From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 42D8681B84 for ; Tue, 17 Jan 2017 02:02:21 -0800 (PST) Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2C5378553D; Tue, 17 Jan 2017 10:02:22 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-70.phx2.redhat.com [10.3.116.70]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v0HA2JGA005763; Tue, 17 Jan 2017 05:02:20 -0500 To: Jiaxin Wu , edk2-devel@ml01.01.org References: <1484623992-52988-1-git-send-email-jiaxin.wu@intel.com> <1484623992-52988-3-git-send-email-jiaxin.wu@intel.com> Cc: Ruiyu Ni , Ye Ting , Kinney Michael D , Fu Siyuan , Gary Ching-Pang Lin , "Jordan Justen (Intel address)" From: Laszlo Ersek Message-ID: <885aab27-f660-768b-59da-4d3e33f099ec@redhat.com> Date: Tue, 17 Jan 2017 11:02:17 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <1484623992-52988-3-git-send-email-jiaxin.wu@intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Tue, 17 Jan 2017 10:02:22 +0000 (UTC) Subject: Re: [PATCH v2 2/2] Nt32Pkg.dsc: Add flag to control HTTP connections X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Jan 2017 10:02:21 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit CC Jordan and Gary On 01/17/17 04:33, Jiaxin Wu wrote: > v2: > * Rename the flag. > > This flag is used to overwrite the PcdAllowHttpConnections > value, then the platform can make a decision whether to allow > HTTP connections or not. > > Cc: Ye Ting > Cc: Fu Siyuan > Cc: Ruiyu Ni > Cc: Laszlo Ersek > Cc: Kinney Michael D > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Wu Jiaxin > --- > Nt32Pkg/Nt32Pkg.dsc | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/Nt32Pkg/Nt32Pkg.dsc b/Nt32Pkg/Nt32Pkg.dsc > index 134afb8..88b1ea9 100644 > --- a/Nt32Pkg/Nt32Pkg.dsc > +++ b/Nt32Pkg/Nt32Pkg.dsc > @@ -2,11 +2,11 @@ > # EFI/Framework Emulation Platform with UEFI HII interface supported. > # > # The Emulation Platform can be used to debug individual modules, prior to creating > # a real platform. This also provides an example for how an DSC is created. > # > -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
> # Copyright (c) 2015, Hewlett-Packard Development Company, L.P.
> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP
> # > # This program and the accompanying materials > # are licensed and made available under the terms and conditions of the BSD License > @@ -57,11 +57,21 @@ > # > # Note: TLS feature highly depends on the OpenSSL building. To enable this > # feature, please follow the instructions found in the file "Patch-HOWTO.txt" > # located in CryptoPkg\Library\OpensslLib to enable the OpenSSL building first. > # > - DEFINE TLS_ENABLE = FALSE > + DEFINE TLS_ENABLE = FALSE > + > + # > + # Indicates whether HTTP connections (i.e., unsecured) are permitted or not. > + # -D FLAG=VALUE > + # > + # Note: If ALLOW_HTTP_CONNECTIONS is TRUE, HTTP connections is allowed. Both > + # the "https://" and "http://" URI schemes are permitted. Otherwise, HTTP > + # connections is denied. Only the "https://" URI scheme is permitted. > + # > + DEFINE ALLOW_HTTP_CONNECTIONS = TRUE > > ################################################################################ > # > # SKU Identification section - list of all SKU IDs supported by this > # Platform. > @@ -252,10 +262,14 @@ > gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE > !if $(SECURE_BOOT_ENABLE) == TRUE || $(TLS_ENABLE) == TRUE > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 > !endif > > +!if $(ALLOW_HTTP_CONNECTIONS) == TRUE > + gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE > +!endif > + > !ifndef $(USE_OLD_SHELL) > gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 0x7C, 0x3E, 0x9E, 0x1C, 0x4F, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 } > !endif > > !if $(SECURE_BOOT_ENABLE) == TRUE > Does the following combination make sense? TLS_ENABLE=FALSE and ALLOW_HTTP_CONNECTIONS=FALSE In this case, only the https:// scheme would be accepted, however the TLS facility that underlies HTTPS is missing. I think this would render the HTTP stack useless. Is that correct? I'm asking mainly for OVMF's sake. (I have nothing against this patch in Nt32Pkg.) Namely, in OvmfPkg, I would dislike the additional complexity of an ALLOW_HTTP_CONNECTIONS build flag. Instead, I think we should set PcdAllowHttpConnections to TRUE, whenever HTTP_BOOT_ENABLE is defined (and we shouldn't override the DEC default otherwise). This would result in HTTP working with just -D HTTP_BOOT_ENABLE, and both HTTP and HTTPS working with -D HTTP_BOOT_ENABLE -D TLS_ENABLE. I don't see any downsides to always permitting HTTP in OVMF. Thoughts? If everyone agrees, then Jiaxin, can you please append a third patch for OvmfPkg, which sets PcdAllowHttpConnections to TRUE whenever HTTP_BOOT_ENABLE is TRUE? (Note that in "OvmfPkgIa32X64.dsc", the setting should likely go under [PcdsFixedAtBuild.X64].) Thanks! Laszlo