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 60A7081E10 for ; Wed, 18 Jan 2017 00:30:55 -0800 (PST) Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (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 7D6283F1FB; Wed, 18 Jan 2017 08:30:56 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-38.phx2.redhat.com [10.3.116.38]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v0I8UrX7029938; Wed, 18 Jan 2017 03:30:54 -0500 To: "Wu, Jiaxin" , "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> <885aab27-f660-768b-59da-4d3e33f099ec@redhat.com> <895558F6EA4E3B41AC93A00D163B7274162948A8@SHSMSX103.ccr.corp.intel.com> Cc: "Ni, Ruiyu" , "Ye, Ting" , "Kinney, Michael D" , "Fu, Siyuan" , Gary Ching-Pang Lin , "Justen, Jordan L" From: Laszlo Ersek Message-ID: <9cacd428-ac00-bab8-2329-6ea78c389dd2@redhat.com> Date: Wed, 18 Jan 2017 09:30:52 +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: <895558F6EA4E3B41AC93A00D163B7274162948A8@SHSMSX103.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Wed, 18 Jan 2017 08:30:56 +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: Wed, 18 Jan 2017 08:30:55 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 01/18/17 03:16, Wu, Jiaxin wrote: >> Subject: Re: [edk2] [PATCH v2 2/2] Nt32Pkg.dsc: Add flag to control HTTP >> connections >> >> 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.PcdResetOnMemoryTypeInformationChan >> ge|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? >> > > Laszlo, > > For my perspective, I think it also make sense since the platform owner make the decision to disable the HTTP connections. In such a case, if TLS is not enabled, HTTP stack should be useless since HTTP connections have been disabled. > > > > >> 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? >> > > The default value of PcdAllowHttpConnections is crucial to ensure the real platform security by default. So, we set the default value to FALSE. > > In order to facilitate control (Just like Nt32), platform owner can define the flag to make the decision whether allow the HTTP connections. > > For Nt32 simulation platform, the default value of flag ALLOW_HTTP_CONNECTIONS is TRUE. For OVMF, we can also define the flag with the TRUE value, which would achieve your purpose that HTTP working with just -D HTTP_BOOT_ENABLE and both HTTP and HTTPS working with -D HTTP_BOOT_ENABLE -D TLS_ENABLE. > > > >> 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? >> > > Laszlo, > > As I talked above and according your requirement, we have the below update choice: > > 1) The flag definition (ALLOW_HTTP_CONNECTIONS) with TRUE value to allow the HTTP connections (the same to NT32). > > DEFINE ALLOW_HTTP_CONNECTIONS = TRUE > !if $(ALLOW_HTTP_CONNECTIONS) == TRUE > gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE > !endif > > 2) Sets PcdAllowHttpConnections to TRUE whenever HTTP_BOOT_ENABLE is TRUE > !if $( HTTP_BOOT_ENABLE) == TRUE > gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE > !endif > > For 1), Flexible control! > For 2), we have no way to stop the HTTP connections while HTTPS is allowed. That means no HTTP connections control switch. > > I still prefer 1), but that's depends on you since you are the OVMF platform owner:). > > What's your opinion? I agree that for a security-oriented approach, for a production firmware, both the DEC default *and* the separate ALLOW_HTTP_CONNECTIONS buid flag make sense. For the default -D HTTP_BOOT_ENABLE build of upstream OVMF however, I think ease of use is more important. In a home or company or team intranet setting, booting virtual machines from plain HTTP is acceptable, I think; forcing users to set up HTTPS on the server side, and mess with keys, would be an inconvenience, in my opionion. I guess we could introduce ALLOW_HTTP_CONNECTIONS with a TRUE default, but in general I try to minimize the number of different build flags (same way as MdeModulePkg seeks to minimize new PCDs); I think they quickly become confusing. Serious users (like distros shipping OVMF) can flip the PCD in the DSC files anyway. So, I prefer (2). Jordan, Gary, what do you guys think? Thanks! Laszlo > >> (Note that in "OvmfPkgIa32X64.dsc", the setting should likely go under >> [PcdsFixedAtBuild.X64].) >> >> Thanks! >> Laszlo