From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=195.135.221.5; helo=smtp.nue.novell.com; envelope-from=glin@suse.com; receiver=edk2-devel@lists.01.org Received: from smtp.nue.novell.com (smtp.nue.novell.com [195.135.221.5]) (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 6FB66226EAC63 for ; Thu, 12 Apr 2018 00:09:05 -0700 (PDT) Received: from emea4-mta.ukb.novell.com ([10.120.13.87]) by smtp.nue.novell.com with ESMTP (TLS encrypted); Thu, 12 Apr 2018 09:09:03 +0200 Received: from GaryWorkstation (nwb-a10-snat.microfocus.com [10.120.13.202]) by emea4-mta.ukb.novell.com with ESMTP (TLS encrypted); Thu, 12 Apr 2018 08:08:50 +0100 Date: Thu, 12 Apr 2018 15:08:46 +0800 From: Gary Lin To: Laszlo Ersek Cc: edk2-devel@lists.01.org, Ard Biesheuvel , Jordan Justen Message-ID: <20180412070825.46rnknrjmg46sw3j@GaryWorkstation> References: <20180411104247.3758-1-lersek@redhat.com> <20180411104247.3758-2-lersek@redhat.com> MIME-Version: 1.0 In-Reply-To: <20180411104247.3758-2-lersek@redhat.com> User-Agent: NeoMutt/20170912 (1.9.0) Subject: Re: [PATCH v2 1/9] OvmfPkg/TlsAuthConfigLib: configure trusted cipher suites for HTTPS boot X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Apr 2018 07:09:06 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Apr 11, 2018 at 12:42:39PM +0200, Laszlo Ersek wrote: > Read the list of trusted cipher suites from fw_cfg and to store it to > EFI_TLS_CA_CERTIFICATE_VARIABLE. > > The fw_cfg file is formatted by the "update-crypto-policies" utility on > the host side, so that the host settings take effect in guest HTTPS boot > as well. QEMU forwards the file intact to the firmware. The contents are > forwarded by NetworkPkg/HttpDxe (in TlsConfigCipherList()) to > NetworkPkg/TlsDxe (TlsSetSessionData()) and TlsLib (TlsSetCipherList()). > Hi Laszlo, The description mentioned "update-crypto-policies" to format the cipher list. The command is not available in openSUSE and I downloaded the command from github repo[*]. However, I didn't find any command in the repo could create the binary cipher list. Anyway, I found you also mentioned "openssl ciphers -V" in the cover letter, and I managed to convert the plaintext cipher list to the binary array. Maybe the description can be improved to avoid the confusion. (Or, I just found the wrong program...) BTW, the code looks good and works for me. Reviewed-by: Gary Lin Tested-by: Gary Lin Cheers, Gary Lin [*] https://github.com/nmav/fedora-crypto-policies > Cc: Ard Biesheuvel > Cc: Gary Ching-Pang Lin > Cc: Jordan Justen > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Laszlo Ersek > --- > > Notes: > v2: > - no change > > OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf | 3 +- > OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c | 98 ++++++++++++++++++++ > 2 files changed, 100 insertions(+), 1 deletion(-) > > diff --git a/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf b/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf > index 5f83582a8313..40754ea5a2f3 100644 > --- a/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf > +++ b/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf > @@ -46,10 +46,11 @@ [LibraryClasses] > DebugLib > MemoryAllocationLib > QemuFwCfgLib > UefiRuntimeServicesTableLib > > [Guids] > - gEfiTlsCaCertificateGuid ## PRODUCES ## Variable:L"TlsCaCertificate" > + gEdkiiHttpTlsCipherListGuid ## PRODUCES ## Variable:L"HttpTlsCipherList" > + gEfiTlsCaCertificateGuid ## PRODUCES ## Variable:L"TlsCaCertificate" > > [Depex] > gEfiVariableWriteArchProtocolGuid > diff --git a/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c b/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c > index b5b33bc4fc69..74c393e5462f 100644 > --- a/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c > +++ b/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c > @@ -17,12 +17,13 @@ > > **/ > > #include > #include > > +#include > #include > > #include > #include > #include > #include > @@ -118,16 +119,113 @@ SetCaCerts ( > gEfiCallerBaseName, __FUNCTION__, (UINT64)HttpsCaCertsSize)); > > FreeHttpsCaCerts: > FreePool (HttpsCaCerts); > } > > +/** > + Read the list of trusted cipher suites from the fw_cfg file > + "etc/edk2/https/ciphers", and store it to > + gEdkiiHttpTlsCipherListGuid:EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE. > + > + The contents are propagated by NetworkPkg/HttpDxe to NetworkPkg/TlsDxe; the > + list is processed by the latter. > +**/ > +STATIC > +VOID > +SetCipherSuites ( > + VOID > + ) > +{ > + EFI_STATUS Status; > + FIRMWARE_CONFIG_ITEM HttpsCiphersItem; > + UINTN HttpsCiphersSize; > + VOID *HttpsCiphers; > + > + Status = QemuFwCfgFindFile ("etc/edk2/https/ciphers", &HttpsCiphersItem, > + &HttpsCiphersSize); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_VERBOSE, "%a:%a: not touching cipher suites\n", > + gEfiCallerBaseName, __FUNCTION__)); > + return; > + } > + // > + // From this point on, any failure is fatal. An ordered cipher preference > + // list is available from QEMU, thus we cannot let the firmware attempt HTTPS > + // boot with either pre-existent or non-existent preferences. An empty set of > + // cipher suites does not fail HTTPS boot automatically; the default cipher > + // suite preferences would take effect, and we must prevent that. > + // > + // Delete the current EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE if it exists. If > + // the variable exists with EFI_VARIABLE_NON_VOLATILE attribute, we cannot > + // make it volatile without deleting it first. > + // > + Status = gRT->SetVariable ( > + EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE, // VariableName > + &gEdkiiHttpTlsCipherListGuid, // VendorGuid > + 0, // Attributes > + 0, // DataSize > + NULL // Data > + ); > + if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) { > + DEBUG ((DEBUG_ERROR, "%a:%a: failed to delete %g:\"%s\"\n", > + gEfiCallerBaseName, __FUNCTION__, &gEdkiiHttpTlsCipherListGuid, > + EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE)); > + goto Done; > + } > + > + if (HttpsCiphersSize == 0) { > + DEBUG ((DEBUG_ERROR, "%a:%a: list of cipher suites must not be empty\n", > + gEfiCallerBaseName, __FUNCTION__)); > + Status = EFI_INVALID_PARAMETER; > + goto Done; > + } > + > + HttpsCiphers = AllocatePool (HttpsCiphersSize); > + if (HttpsCiphers == NULL) { > + DEBUG ((DEBUG_ERROR, "%a:%a: failed to allocate HttpsCiphers\n", > + gEfiCallerBaseName, __FUNCTION__)); > + Status = EFI_OUT_OF_RESOURCES; > + goto Done; > + } > + > + QemuFwCfgSelectItem (HttpsCiphersItem); > + QemuFwCfgReadBytes (HttpsCiphersSize, HttpsCiphers); > + > + Status = gRT->SetVariable ( > + EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE, // VariableName > + &gEdkiiHttpTlsCipherListGuid, // VendorGuid > + EFI_VARIABLE_BOOTSERVICE_ACCESS, // Attributes > + HttpsCiphersSize, // DataSize > + HttpsCiphers // Data > + ); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a:%a: failed to set %g:\"%s\"\n", > + gEfiCallerBaseName, __FUNCTION__, &gEdkiiHttpTlsCipherListGuid, > + EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE)); > + goto FreeHttpsCiphers; > + } > + > + DEBUG ((DEBUG_VERBOSE, "%a:%a: stored list of cipher suites (%Lu byte(s))\n", > + gEfiCallerBaseName, __FUNCTION__, (UINT64)HttpsCiphersSize)); > + > +FreeHttpsCiphers: > + FreePool (HttpsCiphers); > + > +Done: > + if (EFI_ERROR (Status)) { > + ASSERT_EFI_ERROR (Status); > + CpuDeadLoop (); > + } > +} > + > RETURN_STATUS > EFIAPI > TlsAuthConfigInit ( > VOID > ) > { > SetCaCerts (); > + SetCipherSuites (); > > return RETURN_SUCCESS; > } > -- > 2.14.1.3.gb7cf6e02401b > > >