Hi Laszlo, The HttpDxe driver needs to install the Driver Binding Protocol so as to check if a specific controller is supported by HttpDxe. HttpDxe can only be started if the TcpServiceBindingProtocol existed. So, it has to follow the UEFI Driver Model.
For the PCD usage, I think it should be fine to cover the configuration of UEFI Drivers through the PCD settings. The requirement of *.inf needs to include the PcdLib and the section of [Pcd]. We already have the similar pattern for this usage, for example, Ps2KeyboardDxe, PciBusDxe, PciSioSerialDxe, and etc in MdeModulePkg. Besides, there are some advantages by using PCD compared to the variable. First, PCD is one kind of interface that more formal than a private variable, the setting by PCD is more acceptable by the consumer. Secondly, from a *security* standpoint, variable can be dumped easily from the flash region. Here, even though it's no security impact towards the cipher list storage because it will be public shared to remote server, but we need to think and *align* with other configurations for TLS in HTTPS level. For example, in the future, we might support the HTTPS mutual authentication, than the host PrivateKey/Password (EfiTlsConfigDataTypeHostPrivateKey) *mustn't* be s aved as a variable due to its confidentiality, while PCD is good choice. At that time, we will also provide the PCD for EfiTlsConfigDataTypeCACertificate, which is currently setting by the variable (TlsCaCertificate), so as to align all the configuration setting on one line, which can reduce the complexity of platform usage. Finally, we can also save the variable space. >From the above, the dynamic PCD is a solution I still preferred. Thanks, Jiaxin > -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Thursday, January 25, 2018 12:13 AM > To: Wu, Jiaxin <jiaxin...@intel.com>; Fu, Siyuan <siyuan...@intel.com>; Ye, > Ting <ting...@intel.com>; Long, Qin <qin.l...@intel.com>; Yao, Jiewen > <jiewen....@intel.com>; Hsiung, Harry L <harry.l.hsi...@intel.com> > Cc: edk2-devel-01 <edk2-devel@lists.01.org> > Subject: Re: setting the TLS cipher list for HTTPS booting > > On 01/24/18 07:50, Wu, Jiaxin wrote: > > Hi Laszlo, > > > > After the discussion with team member, we still prefer to use the PCD > > solution. In HttpDxe driver, we don't want to locate/use a > > nonstandard protocol. We think It's not a general solution for the > > UEFI driver. > Ah, I totally missed that NetworkPkg/HttpDxe was a UEFI_DRIVER! :) > > In that case, I think *neither* the LocateProtocol() call for an > edk2-specific protocol, *nor* a PcdGetPtr() call are appropriate. > UEFI_DRIVER modules should preferably only use facilities from the UEFI > spec, and the protocol for the dynamic PCDs comes from the PI spec, not > the UEFI spec. > > (This would be different if HttpDxe was a DXE_DRIVER -- in that case > both approaches would be valid. I assumed HttpDxe was a DXE_DRIVER, not > sure why.) > > (1) So, given that HttpDxe is a UEFI_DRIVER, I think the right approach > would be -- which I believe I also mentioned earlier -- to introduce > another UEFI variable for the list of cipher suites, similarly to > "TlsCaCertificate" (in a custom variable namespace GUID). This would > stay within the framework of the UEFI spec. > > > (2) Regarding the order between setting these UEFI variables in OVMF, > and consuming them in HttpDxe, I think your argument is good. We can set > the variables in some platform code (DXE_DRIVER) in OVMF, before > End-of-DXE, and HttpDxe will only read them later in BDS. > > "OvmfPkg/PlatformDxe" seems like a good candidate for setting these > variables (both considering PlatformDxe's purpose, and because it > already depends on "gEfiVariableWriteArchProtocolGuid"). > > > (3) Regardig the format (EFI_TLS_CIPHER): I agree with you. It seems we > can modify the host environment to pass QEMU (and OVMF) a cipher list > that is already in EFI_TLS_CIPHER format. > > > So I think the only remaining question is if you like a new UEFI > variable instead of the dynamic PCD, for the cipher list. > > Thanks! > Laszlo > > > > > > > Thanks, > > Jiaxin > > > >> -----Original Message----- > >> From: Wu, Jiaxin > >> Sent: Wednesday, January 24, 2018 11:40 AM > >> To: Wu, Jiaxin <jiaxin...@intel.com>; Laszlo Ersek <ler...@redhat.com>; > Fu, > >> Siyuan <siyuan...@intel.com>; Ye, Ting <ting...@intel.com>; Long, Qin > >> <qin.l...@intel.com>; Yao, Jiewen <jiewen....@intel.com>; Hsiung, Harry > L > >> <harry.l.hsi...@intel.com> > >> Cc: edk2-devel-01 <edk2-devel@lists.01.org> > >> Subject: RE: setting the TLS cipher list for HTTPS booting > >> > >> Hi Laszlo, > >> > >> More comments: > >> > >>> > >>> Dynamic PCDs is just one of the solutions for the required settings, just > >>> like > >> the > >>> platform protocol (HTTPS_CONFIG_PROTOCOL), provides the capability to > >>> support the global HTTPS configuration. > >>> > >>> Each solutions have its own advantages and disadvantages: > >>> 1) PCD can simplify the problem and it's easy to use for the other > >>> platform > >> not > >>> only OVMF, but as you said, it's perhaps overkill. > >>> 2) The additional platform protocol looks flexible and reasonably, but it > >> makes > >>> the specific platform have the optional dependency ["OVMF hooks a NULL- > >> class > >>> library into HttpDxe that introduces a new DEPEX on the protocol. Other > >>> platforms would not delay HttpDxe."]. If the user doesn't want HTTPS > feature > >>> but only HTTP, it has to include one NULL protocol. > >>> > >> > >> I checked the PciHostBridgeDxe driver to call the EDKII_IOMMU_PROTOCOL, > >> which makes me better understanding your comments. > >> > >> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf { > >> <LibraryClasses> > >> ... > >> > NULL|OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf > >> } > >> > >> So, as you said, it's just the platform behavior so as to hook the platform > >> produces the EDKII_IOMMU_PROTOCOL protocol first, then dispatch > >> PciHostBridgeDxe driver. That's good to me. > >> > >> For HTTPS configuration, the HttpDxe configuration is only happened during > the > >> protocol instance calling, which is created by service binding protocol. > >> So, it > >> looks only happened after EndofDxe phase. If so, it will be no such > >> optional > >> dependency to wait for the platform DXE driver produces the > >> EDKII_PLATFORM_HTTPS_CONFIG_PROTOCOL. > >> > >> Anyway, for above two solutions, I need review them with other colleagues > and > >> help to collect the comments for both of them, then feedback to you. Thank > you > >> so such. > >> > >> > >>> Now, I think we are discussing the most appropriate way for the HTTPS > >>> controlling. It's NOT related to who should be responsible for the > >>> solution > >>> coding, you know we are always thinking from the user's perspective:). > >>> > >>> > >>>>> > >>>>> If you really think that HttpDxe should only care about these two items > >>>>> (CA cert and cipher list), then I have another question: do you think it > >>>>> makes sense to introduce another non-volatile UEFI variable, for the > >>>>> cipher suites too? This would make things uniform, and perhaps > >>>>> TlsAuthConfigDxe could expose the cipher suites too, as a list of > >>>>> checkboxes. Just an idea. > >>>> > >>>> So, apparently we indeed care about these two options mostly, i.e., the > >>>> CA certs, and the cipher suites. > >>>> > >>>> However, I was informed that OVMF should simply forward the *textual* > >>>> cipher list representation, with preferably no processing at all before > >>>> the string reaches the OpenSSL code. In other words, OVMF should set the > >>>> PCD -- or, even better, variable -- to a *character string* like this: > >>>> > >>>> > >>> > >> > "kEECDH:kRSA:kEDH:kPSK:kDHEPSK:kECDHEPSK:!EXP:!DES:!RC4:!RC2:!IDEA:!SEE > >>>> D:!eNULL:!aNULL:!MD5:!SSLv2" > >>>> > >>>> Is this feasible? > >>> > >>> It looks impossible to simply forward the *textual*cipher list to OpenSSL > from > >>> the aspect of EFI_TLS_PROTOCOL. > >>> > >>> > //************************************************************ > >>> // EFI_TLS_CIPHER > >>> > //************************************************************ > >>> typedef struct { > >>> UINT8 Data1; > >>> UINT8 Data2; > >>> } EFI_TLS_CIPHER; > >>> Note: The definition of EFI_TLS_CIPHER is from RFC 5246 A.4.1.Hello > >> Messages. > >>> The value of > >>> EFI_TLS_CIPHER is from TLS Cipher Suite Registry of IANA. > >>> > >>> > >>>> > >>>> Thanks, > >>>> Laszlo > >>> > >>> > >> > >> > >> Thanks, > >> Jiaxin > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
