* [PATCH v2 1/9] OvmfPkg/TlsAuthConfigLib: configure trusted cipher suites for HTTPS boot
2018-04-11 10:42 [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites Laszlo Ersek
@ 2018-04-11 10:42 ` Laszlo Ersek
2018-04-12 7:08 ` Gary Lin
2018-04-11 10:42 ` [PATCH v2 2/9] MdePkg/Include/Protocol/Tls.h: pack structures from the TLS RFC Laszlo Ersek
` (10 subsequent siblings)
11 siblings, 1 reply; 21+ messages in thread
From: Laszlo Ersek @ 2018-04-11 10:42 UTC (permalink / raw)
To: edk2-devel; +Cc: Ard Biesheuvel, Gary Ching-Pang Lin, Jordan Justen
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()).
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Gary Ching-Pang Lin <glin@suse.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
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 <Uefi/UefiBaseType.h>
#include <Uefi/UefiSpec.h>
+#include <Guid/HttpTlsCipherList.h>
#include <Guid/TlsAuthentication.h>
#include <Library/BaseLib.h>
#include <Library/DebugLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/QemuFwCfgLib.h>
@@ -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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 1/9] OvmfPkg/TlsAuthConfigLib: configure trusted cipher suites for HTTPS boot
2018-04-11 10:42 ` [PATCH v2 1/9] OvmfPkg/TlsAuthConfigLib: configure trusted cipher suites for HTTPS boot Laszlo Ersek
@ 2018-04-12 7:08 ` Gary Lin
2018-04-12 8:49 ` Laszlo Ersek
0 siblings, 1 reply; 21+ messages in thread
From: Gary Lin @ 2018-04-12 7:08 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: edk2-devel, Ard Biesheuvel, Jordan Justen
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 <glin@suse.com>
Tested-by: Gary Lin <glin@suse.com>
Cheers,
Gary Lin
[*] https://github.com/nmav/fedora-crypto-policies
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Gary Ching-Pang Lin <glin@suse.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> 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 <Uefi/UefiBaseType.h>
> #include <Uefi/UefiSpec.h>
>
> +#include <Guid/HttpTlsCipherList.h>
> #include <Guid/TlsAuthentication.h>
>
> #include <Library/BaseLib.h>
> #include <Library/DebugLib.h>
> #include <Library/MemoryAllocationLib.h>
> #include <Library/QemuFwCfgLib.h>
> @@ -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
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v2 1/9] OvmfPkg/TlsAuthConfigLib: configure trusted cipher suites for HTTPS boot
2018-04-12 7:08 ` Gary Lin
@ 2018-04-12 8:49 ` Laszlo Ersek
2018-04-12 9:10 ` Gary Lin
0 siblings, 1 reply; 21+ messages in thread
From: Laszlo Ersek @ 2018-04-12 8:49 UTC (permalink / raw)
To: Gary Lin; +Cc: edk2-devel, Ard Biesheuvel, Jordan Justen
On 04/12/18 09:08, Gary Lin wrote:
> 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.
Right, that feature is underway, and the Crypto team has agreed to
implement it for me. My apologies for being unclear about it. Until
then, a small shell script like the following can be used:
-----
export LC_ALL=C
openssl ciphers -V \
| sed -r -n \
-e 's/^ *0x([0-9A-F]{2}),0x([0-9A-F]{2}) - .*$/\\\\x\1 \\\\x\2/p' \
| xargs -r -- printf -- '%b' > ciphers.bin
-----
> 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...)
No, you are right; I figured I'd describe the end-state in the commit
mesage. I guess I can replace
The fw_cfg file is formatted by the "update-crypto-policies" utility
with
The fw_cfg file will be formatted by the "update-crypto-policies"
utility
in the commit message.
>
> BTW, the code looks good and works for me.
>
> Reviewed-by: Gary Lin <glin@suse.com>
> Tested-by: Gary Lin <glin@suse.com>
Thanks Gary!
Laszlo
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v2 1/9] OvmfPkg/TlsAuthConfigLib: configure trusted cipher suites for HTTPS boot
2018-04-12 8:49 ` Laszlo Ersek
@ 2018-04-12 9:10 ` Gary Lin
2018-04-12 9:43 ` Laszlo Ersek
0 siblings, 1 reply; 21+ messages in thread
From: Gary Lin @ 2018-04-12 9:10 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: edk2-devel, Ard Biesheuvel, Jordan Justen
On Thu, Apr 12, 2018 at 10:49:15AM +0200, Laszlo Ersek wrote:
> On 04/12/18 09:08, Gary Lin wrote:
> > 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.
>
> Right, that feature is underway, and the Crypto team has agreed to
> implement it for me. My apologies for being unclear about it. Until
> then, a small shell script like the following can be used:
>
> -----
> export LC_ALL=C
>
> openssl ciphers -V \
> | sed -r -n \
> -e 's/^ *0x([0-9A-F]{2}),0x([0-9A-F]{2}) - .*$/\\\\x\1 \\\\x\2/p' \
> | xargs -r -- printf -- '%b' > ciphers.bin
> -----
>
It would be good to have this script in the description or in the README
so that the person who doesn't have the updated update-crypto-policies,
like me, can easily generate the cipher list.
Cheers,
Gary Lin
> > 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...)
>
> No, you are right; I figured I'd describe the end-state in the commit
> mesage. I guess I can replace
>
> The fw_cfg file is formatted by the "update-crypto-policies" utility
>
> with
>
> The fw_cfg file will be formatted by the "update-crypto-policies"
> utility
>
> in the commit message.
>
> >
> > BTW, the code looks good and works for me.
> >
> > Reviewed-by: Gary Lin <glin@suse.com>
> > Tested-by: Gary Lin <glin@suse.com>
>
> Thanks Gary!
> Laszlo
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v2 1/9] OvmfPkg/TlsAuthConfigLib: configure trusted cipher suites for HTTPS boot
2018-04-12 9:10 ` Gary Lin
@ 2018-04-12 9:43 ` Laszlo Ersek
2018-04-12 10:17 ` Gary Lin
0 siblings, 1 reply; 21+ messages in thread
From: Laszlo Ersek @ 2018-04-12 9:43 UTC (permalink / raw)
To: Gary Lin; +Cc: edk2-devel, Ard Biesheuvel, Jordan Justen
On 04/12/18 11:10, Gary Lin wrote:
> On Thu, Apr 12, 2018 at 10:49:15AM +0200, Laszlo Ersek wrote:
>> On 04/12/18 09:08, Gary Lin wrote:
>>> 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.
>>
>> Right, that feature is underway, and the Crypto team has agreed to
>> implement it for me. My apologies for being unclear about it. Until
>> then, a small shell script like the following can be used:
>>
>> -----
>> export LC_ALL=C
>>
>> openssl ciphers -V \
>> | sed -r -n \
>> -e 's/^ *0x([0-9A-F]{2}),0x([0-9A-F]{2}) - .*$/\\\\x\1 \\\\x\2/p' \
>> | xargs -r -- printf -- '%b' > ciphers.bin
>> -----
>>
> It would be good to have this script in the description or in the README
> so that the person who doesn't have the updated update-crypto-policies,
> like me, can easily generate the cipher list.
I can include this in the commit message, sure.
If you think OvmfPkg/README would be a better place for it: can you
please submit a patch? ;) It's not just that I'm overloaded (although I
am), but I always welcome documentation contributions with enthusiasm.
If the documentation captures real life "user stories", that's for the best.
You could introduce an "HTTPS Boot" section to the README, between
"Network Support" and "OVMF Flash Layout". You contributed quite a bit
to HTTPS enablement anyway!
It's up to you, of course :) If you don't have the time, I'll add the
script to the commit message.
Thanks,
Laszlo
>
> Cheers,
>
> Gary Lin
>
>>> 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...)
>>
>> No, you are right; I figured I'd describe the end-state in the commit
>> mesage. I guess I can replace
>>
>> The fw_cfg file is formatted by the "update-crypto-policies" utility
>>
>> with
>>
>> The fw_cfg file will be formatted by the "update-crypto-policies"
>> utility
>>
>> in the commit message.
>>
>>>
>>> BTW, the code looks good and works for me.
>>>
>>> Reviewed-by: Gary Lin <glin@suse.com>
>>> Tested-by: Gary Lin <glin@suse.com>
>>
>> Thanks Gary!
>> Laszlo
>>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v2 1/9] OvmfPkg/TlsAuthConfigLib: configure trusted cipher suites for HTTPS boot
2018-04-12 9:43 ` Laszlo Ersek
@ 2018-04-12 10:17 ` Gary Lin
2018-04-12 17:10 ` Laszlo Ersek
0 siblings, 1 reply; 21+ messages in thread
From: Gary Lin @ 2018-04-12 10:17 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: edk2-devel, Ard Biesheuvel, Jordan Justen
On Thu, Apr 12, 2018 at 11:43:35AM +0200, Laszlo Ersek wrote:
> On 04/12/18 11:10, Gary Lin wrote:
> > On Thu, Apr 12, 2018 at 10:49:15AM +0200, Laszlo Ersek wrote:
> >> On 04/12/18 09:08, Gary Lin wrote:
> >>> 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.
> >>
> >> Right, that feature is underway, and the Crypto team has agreed to
> >> implement it for me. My apologies for being unclear about it. Until
> >> then, a small shell script like the following can be used:
> >>
> >> -----
> >> export LC_ALL=C
> >>
> >> openssl ciphers -V \
> >> | sed -r -n \
> >> -e 's/^ *0x([0-9A-F]{2}),0x([0-9A-F]{2}) - .*$/\\\\x\1 \\\\x\2/p' \
> >> | xargs -r -- printf -- '%b' > ciphers.bin
> >> -----
> >>
> > It would be good to have this script in the description or in the README
> > so that the person who doesn't have the updated update-crypto-policies,
> > like me, can easily generate the cipher list.
>
> I can include this in the commit message, sure.
>
> If you think OvmfPkg/README would be a better place for it: can you
> please submit a patch? ;) It's not just that I'm overloaded (although I
> am), but I always welcome documentation contributions with enthusiasm.
> If the documentation captures real life "user stories", that's for the best.
>
> You could introduce an "HTTPS Boot" section to the README, between
> "Network Support" and "OVMF Flash Layout". You contributed quite a bit
> to HTTPS enablement anyway!
>
Sounds good. I'm also thinking about collecting the fw_cfg entries in
OVMF and documenting them in README. Currently, those entries look like
black magic to the users.
> It's up to you, of course :) If you don't have the time, I'll add the
> script to the commit message.
>
I can find some time next week. No guarantee though ;)
Thanks,
Gary Lin
> Thanks,
> Laszlo
>
> >
> > Cheers,
> >
> > Gary Lin
> >
> >>> 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...)
> >>
> >> No, you are right; I figured I'd describe the end-state in the commit
> >> mesage. I guess I can replace
> >>
> >> The fw_cfg file is formatted by the "update-crypto-policies" utility
> >>
> >> with
> >>
> >> The fw_cfg file will be formatted by the "update-crypto-policies"
> >> utility
> >>
> >> in the commit message.
> >>
> >>>
> >>> BTW, the code looks good and works for me.
> >>>
> >>> Reviewed-by: Gary Lin <glin@suse.com>
> >>> Tested-by: Gary Lin <glin@suse.com>
> >>
> >> Thanks Gary!
> >> Laszlo
> >>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v2 1/9] OvmfPkg/TlsAuthConfigLib: configure trusted cipher suites for HTTPS boot
2018-04-12 10:17 ` Gary Lin
@ 2018-04-12 17:10 ` Laszlo Ersek
0 siblings, 0 replies; 21+ messages in thread
From: Laszlo Ersek @ 2018-04-12 17:10 UTC (permalink / raw)
To: Gary Lin, Ard Biesheuvel, Jordan Justen; +Cc: edk2-devel
(comment/question at the end for Ard and Jordan)
On 04/12/18 12:17, Gary Lin wrote:
> On Thu, Apr 12, 2018 at 11:43:35AM +0200, Laszlo Ersek wrote:
>> On 04/12/18 11:10, Gary Lin wrote:
>>> On Thu, Apr 12, 2018 at 10:49:15AM +0200, Laszlo Ersek wrote:
>>>> On 04/12/18 09:08, Gary Lin wrote:
>>>>> 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.
>>>>
>>>> Right, that feature is underway, and the Crypto team has agreed to
>>>> implement it for me. My apologies for being unclear about it. Until
>>>> then, a small shell script like the following can be used:
>>>>
>>>> -----
>>>> export LC_ALL=C
>>>>
>>>> openssl ciphers -V \
>>>> | sed -r -n \
>>>> -e 's/^ *0x([0-9A-F]{2}),0x([0-9A-F]{2}) - .*$/\\\\x\1 \\\\x\2/p' \
>>>> | xargs -r -- printf -- '%b' > ciphers.bin
>>>> -----
>>>>
>>> It would be good to have this script in the description or in the README
>>> so that the person who doesn't have the updated update-crypto-policies,
>>> like me, can easily generate the cipher list.
>>
>> I can include this in the commit message, sure.
>>
>> If you think OvmfPkg/README would be a better place for it: can you
>> please submit a patch? ;) It's not just that I'm overloaded (although I
>> am), but I always welcome documentation contributions with enthusiasm.
>> If the documentation captures real life "user stories", that's for the best.
>>
>> You could introduce an "HTTPS Boot" section to the README, between
>> "Network Support" and "OVMF Flash Layout". You contributed quite a bit
>> to HTTPS enablement anyway!
>>
> Sounds good. I'm also thinking about collecting the fw_cfg entries in
> OVMF and documenting them in README. Currently, those entries look like
> black magic to the users.
Indeed, the fw_cfg entries should ultimately not be used by end-users
directly. If that had been the case, I would have chosen different
fw_cfg pathnames. Please refer to the following file under QEMU:
docs/specs/fw_cfg.txt
= Externally Provided Items =
That is, if the -fw_cfg cmdline option had been the intended user
interface for this feature, then I would have chosen an "opt/" prefix. I
chose "etc/" because the pathnames are QEMU-defined (not user-defined),
except the QEMU patches will be written later (and it might not be me
either).
It's too bad that the RHBZs that track this feature (across multiple
components) are all private, so I couldn't link them in the blurb and/or
the commit messages. Those RHBZs provide a more comprehensive
background. They are private because... well don't ask me. I didn't make
them private, they got auto-created like that, and I'm sure you know
that, if *you* flip a BZ from private to public, then the burden is on
*you* to defend your decision facing unpredictable parts of your
organization. While I totally disagree with any of these RHBZs being
private, I know better than to make them public myself :)
>> It's up to you, of course :) If you don't have the time, I'll add the
>> script to the commit message.
>>
> I can find some time next week. No guarantee though ;)
Sure, please take your time. (And thank you for taking on the README
update.) Meanwhile I'll push these patches with the commit message
updates discussed.
Ard, Jordan: Gary tested and reviewed this patch (bunch of kudos for
that!), but still, can one of you guys please ACK it too? I prefer the
patch to go in with maintainer blessing. Similarly to commit
9c7d0d499296 ("OvmfPkg/TlsAuthConfigLib: configure trusted CA certs for
HTTPS boot", 2018-03-30).
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 2/9] MdePkg/Include/Protocol/Tls.h: pack structures from the TLS RFC
2018-04-11 10:42 [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites Laszlo Ersek
2018-04-11 10:42 ` [PATCH v2 1/9] OvmfPkg/TlsAuthConfigLib: configure trusted cipher suites for HTTPS boot Laszlo Ersek
@ 2018-04-11 10:42 ` Laszlo Ersek
2018-04-11 10:42 ` [PATCH v2 3/9] NetworkPkg/TlsDxe: verify DataSize for EfiTlsCipherList Laszlo Ersek
` (9 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Laszlo Ersek @ 2018-04-11 10:42 UTC (permalink / raw)
To: edk2-devel; +Cc: Jiaxin Wu, Liming Gao, Michael D Kinney, Siyuan Fu
The structures defined in RFC 5246 are not to have any padding between
fields or at the end; use the "pack" pragma as necessary.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=915
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
---
Notes:
v2:
- pick up Siyuan's R-b
MdePkg/Include/Protocol/Tls.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/MdePkg/Include/Protocol/Tls.h b/MdePkg/Include/Protocol/Tls.h
index 2119f33c0f5b..dafaabcd2a8b 100644
--- a/MdePkg/Include/Protocol/Tls.h
+++ b/MdePkg/Include/Protocol/Tls.h
@@ -138,33 +138,37 @@ typedef enum {
///
/// EFI_TLS_CIPHER
/// Note: The definition of EFI_TLS_CIPHER definition is from "RFC 5246, A.4.1.
/// Hello Messages". The value of EFI_TLS_CIPHER is from TLS Cipher
/// Suite Registry of IANA.
///
+#pragma pack (1)
typedef struct {
UINT8 Data1;
UINT8 Data2;
} EFI_TLS_CIPHER;
+#pragma pack ()
///
/// EFI_TLS_COMPRESSION
/// Note: The value of EFI_TLS_COMPRESSION definition is from "RFC 3749".
///
typedef UINT8 EFI_TLS_COMPRESSION;
///
/// EFI_TLS_EXTENSION
/// Note: The definition of EFI_TLS_EXTENSION if from "RFC 5246 A.4.1.
/// Hello Messages".
///
+#pragma pack (1)
typedef struct {
UINT16 ExtensionType;
UINT16 Length;
UINT8 Data[1];
} EFI_TLS_EXTENSION;
+#pragma pack ()
///
/// EFI_TLS_VERIFY
/// Use either EFI_TLS_VERIFY_NONE or EFI_TLS_VERIFY_PEER, the last two options
/// are 'ORed' with EFI_TLS_VERIFY_PEER if they are desired.
///
@@ -191,35 +195,41 @@ typedef UINT32 EFI_TLS_VERIFY;
///
/// EFI_TLS_RANDOM
/// Note: The definition of EFI_TLS_RANDOM is from "RFC 5246 A.4.1.
/// Hello Messages".
///
+#pragma pack (1)
typedef struct {
UINT32 GmtUnixTime;
UINT8 RandomBytes[28];
} EFI_TLS_RANDOM;
+#pragma pack ()
///
/// EFI_TLS_MASTER_SECRET
/// Note: The definition of EFI_TLS_MASTER_SECRET is from "RFC 5246 8.1.
/// Computing the Master Secret".
///
+#pragma pack (1)
typedef struct {
UINT8 Data[48];
} EFI_TLS_MASTER_SECRET;
+#pragma pack ()
///
/// EFI_TLS_SESSION_ID
/// Note: The definition of EFI_TLS_SESSION_ID is from "RFC 5246 A.4.1. Hello Messages".
///
#define MAX_TLS_SESSION_ID_LENGTH 32
+#pragma pack (1)
typedef struct {
UINT16 Length;
UINT8 Data[MAX_TLS_SESSION_ID_LENGTH];
} EFI_TLS_SESSION_ID;
+#pragma pack ()
///
/// EFI_TLS_SESSION_STATE
///
typedef enum {
///
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 3/9] NetworkPkg/TlsDxe: verify DataSize for EfiTlsCipherList
2018-04-11 10:42 [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites Laszlo Ersek
2018-04-11 10:42 ` [PATCH v2 1/9] OvmfPkg/TlsAuthConfigLib: configure trusted cipher suites for HTTPS boot Laszlo Ersek
2018-04-11 10:42 ` [PATCH v2 2/9] MdePkg/Include/Protocol/Tls.h: pack structures from the TLS RFC Laszlo Ersek
@ 2018-04-11 10:42 ` Laszlo Ersek
2018-04-11 10:42 ` [PATCH v2 4/9] NetworkPkg/TlsDxe: clean up byte order conversion " Laszlo Ersek
` (8 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Laszlo Ersek @ 2018-04-11 10:42 UTC (permalink / raw)
To: edk2-devel; +Cc: Jiaxin Wu, Siyuan Fu
TlsSetSessionData() shouldn't just ignore an incomplete EFI_TLS_CIPHER
element at the end of "Data":
- Generally speaking, malformed input for a security API is best rejected
explicitly.
- Specifically speaking, the size of EFI_TLS_CIPHER is 2 bytes. If
DataSize is 1 on input, then the initial check for (DataSize == 0) will
fail, but then TlsSetCipherList() will be called with CipherNum=0.
Return EFI_INVALID_PARAMETER from TlsSetSessionData() if "Data" doesn't
contain a whole number of EFI_TLS_CIPHER elements. While at it, introduce
the dedicated variable CipherCount.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=915
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
---
Notes:
v2:
- pick up Siyuan's R-b
NetworkPkg/TlsDxe/TlsProtocol.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/NetworkPkg/TlsDxe/TlsProtocol.c b/NetworkPkg/TlsDxe/TlsProtocol.c
index ad4c922c60bd..a5f95a098345 100644
--- a/NetworkPkg/TlsDxe/TlsProtocol.c
+++ b/NetworkPkg/TlsDxe/TlsProtocol.c
@@ -35,12 +35,13 @@ EFI_TLS_PROTOCOL mTlsProtocol = {
@retval EFI_SUCCESS The TLS session data is set successfully.
@retval EFI_INVALID_PARAMETER One or more of the following conditions is TRUE:
This is NULL.
Data is NULL.
DataSize is 0.
+ DataSize is invalid for DataType.
@retval EFI_UNSUPPORTED The DataType is unsupported.
@retval EFI_ACCESS_DENIED If the DataType is one of below:
EfiTlsClientRandom
EfiTlsServerRandom
EfiTlsKeyMaterial
@retval EFI_NOT_READY Current TLS session state is NOT
@@ -56,12 +57,13 @@ TlsSetSessionData (
IN UINTN DataSize
)
{
EFI_STATUS Status;
TLS_INSTANCE *Instance;
UINT16 *CipherId;
+ UINTN CipherCount;
UINTN Index;
EFI_TPL OldTpl;
Status = EFI_SUCCESS;
CipherId = NULL;
@@ -97,23 +99,29 @@ TlsSetSessionData (
goto ON_EXIT;
}
Status = TlsSetConnectionEnd (Instance->TlsConn, *((EFI_TLS_CONNECTION_END *) Data));
break;
case EfiTlsCipherList:
+ if (DataSize % sizeof (EFI_TLS_CIPHER) != 0) {
+ Status = EFI_INVALID_PARAMETER;
+ goto ON_EXIT;
+ }
+
CipherId = AllocatePool (DataSize);
if (CipherId == NULL) {
Status = EFI_OUT_OF_RESOURCES;
goto ON_EXIT;
}
- for (Index = 0; Index < DataSize / sizeof (EFI_TLS_CIPHER); Index++) {
+ CipherCount = DataSize / sizeof (EFI_TLS_CIPHER);
+ for (Index = 0; Index < CipherCount; Index++) {
*(CipherId +Index) = HTONS (*(((UINT16 *) Data) + Index));
}
- Status = TlsSetCipherList (Instance->TlsConn, CipherId, DataSize / sizeof (EFI_TLS_CIPHER));
+ Status = TlsSetCipherList (Instance->TlsConn, CipherId, CipherCount);
FreePool (CipherId);
break;
case EfiTlsCompressionMethod:
//
// TLS seems only define one CompressionMethod.null, which specifies that data exchanged via the
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 4/9] NetworkPkg/TlsDxe: clean up byte order conversion for EfiTlsCipherList
2018-04-11 10:42 [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites Laszlo Ersek
` (2 preceding siblings ...)
2018-04-11 10:42 ` [PATCH v2 3/9] NetworkPkg/TlsDxe: verify DataSize for EfiTlsCipherList Laszlo Ersek
@ 2018-04-11 10:42 ` Laszlo Ersek
2018-04-11 10:42 ` [PATCH v2 5/9] CryptoPkg/TlsLib: replace TlsGetCipherString() with TlsGetCipherMapping() Laszlo Ersek
` (7 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Laszlo Ersek @ 2018-04-11 10:42 UTC (permalink / raw)
To: edk2-devel; +Cc: Jiaxin Wu, Siyuan Fu
Fix the following style issues:
- "Data" is accessed through a pointer to UINT16 rather than to a pointer
to EFI_TLS_CIPHER. While technically correct, UINT16 is harder to
interpret against the UEFI spec.
- Array subscripting is written with weird *(Pointer + Offset)
expressions, rather than with Pointer[Offset].
- The byte order is converted with HTONS(), while it should be NTOHS().
Either way, use the Data1 and Data2 fields of EFI_TLS_CIPHER instead.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=915
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
---
Notes:
v2:
- pick up Siyuan's R-b
NetworkPkg/TlsDxe/TlsProtocol.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/NetworkPkg/TlsDxe/TlsProtocol.c b/NetworkPkg/TlsDxe/TlsProtocol.c
index a5f95a098345..298ffdd659a2 100644
--- a/NetworkPkg/TlsDxe/TlsProtocol.c
+++ b/NetworkPkg/TlsDxe/TlsProtocol.c
@@ -57,12 +57,13 @@ TlsSetSessionData (
IN UINTN DataSize
)
{
EFI_STATUS Status;
TLS_INSTANCE *Instance;
UINT16 *CipherId;
+ CONST EFI_TLS_CIPHER *TlsCipherList;
UINTN CipherCount;
UINTN Index;
EFI_TPL OldTpl;
Status = EFI_SUCCESS;
@@ -110,15 +111,17 @@ TlsSetSessionData (
CipherId = AllocatePool (DataSize);
if (CipherId == NULL) {
Status = EFI_OUT_OF_RESOURCES;
goto ON_EXIT;
}
+ TlsCipherList = (CONST EFI_TLS_CIPHER *) Data;
CipherCount = DataSize / sizeof (EFI_TLS_CIPHER);
for (Index = 0; Index < CipherCount; Index++) {
- *(CipherId +Index) = HTONS (*(((UINT16 *) Data) + Index));
+ CipherId[Index] = ((TlsCipherList[Index].Data1 << 8) |
+ TlsCipherList[Index].Data2);
}
Status = TlsSetCipherList (Instance->TlsConn, CipherId, CipherCount);
FreePool (CipherId);
break;
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 5/9] CryptoPkg/TlsLib: replace TlsGetCipherString() with TlsGetCipherMapping()
2018-04-11 10:42 [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites Laszlo Ersek
` (3 preceding siblings ...)
2018-04-11 10:42 ` [PATCH v2 4/9] NetworkPkg/TlsDxe: clean up byte order conversion " Laszlo Ersek
@ 2018-04-11 10:42 ` Laszlo Ersek
2018-04-11 10:42 ` [PATCH v2 6/9] CryptoPkg/TlsLib: use binary search in the TlsGetCipherMapping() function Laszlo Ersek
` (6 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Laszlo Ersek @ 2018-04-11 10:42 UTC (permalink / raw)
To: edk2-devel; +Cc: Jiaxin Wu, Qin Long, Siyuan Fu, Ting Ye
In the following patches it will be useful if the IANA CipherId lookup
returns a pointer to the whole matching IANA-to-OpenSSL mapping structure,
not just the OpenSSL cipher suite name. Rename TLS_CIPHER_PAIR and
TlsGetCipherString() to TLS_CIPHER_MAPPING and TlsGetCipherMapping()
respectively, and make the function return a pointer to
TLS_CIPHER_MAPPING.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Qin Long <qin.long@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Ting Ye <ting.ye@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=915
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- no change
CryptoPkg/Library/TlsLib/TlsConfig.c | 37 +++++++++++---------
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/CryptoPkg/Library/TlsLib/TlsConfig.c b/CryptoPkg/Library/TlsLib/TlsConfig.c
index 2ffe58ad29a2..507489386b8e 100644
--- a/CryptoPkg/Library/TlsLib/TlsConfig.c
+++ b/CryptoPkg/Library/TlsLib/TlsConfig.c
@@ -21,19 +21,19 @@ typedef struct {
//
UINT16 IanaCipher;
//
// OpenSSL-used Cipher Suite String
//
CONST CHAR8 *OpensslCipher;
-} TLS_CIPHER_PAIR;
+} TLS_CIPHER_MAPPING;
//
// The mapping table between IANA/IETF Cipher Suite definitions and
// OpenSSL-used Cipher Suite name.
//
-STATIC CONST TLS_CIPHER_PAIR TlsCipherMappingTable[] = {
+STATIC CONST TLS_CIPHER_MAPPING TlsCipherMappingTable[] = {
{ 0x0001, "NULL-MD5" }, /// TLS_RSA_WITH_NULL_MD5
{ 0x0002, "NULL-SHA" }, /// TLS_RSA_WITH_NULL_SHA
{ 0x0004, "RC4-MD5" }, /// TLS_RSA_WITH_RC4_128_MD5
{ 0x0005, "RC4-SHA" }, /// TLS_RSA_WITH_RC4_128_SHA
{ 0x000A, "DES-CBC3-SHA" }, /// TLS_RSA_WITH_3DES_EDE_CBC_SHA, mandatory TLS 1.1
{ 0x0016, "DHE-RSA-DES-CBC3-SHA" }, /// TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA
@@ -54,42 +54,42 @@ STATIC CONST TLS_CIPHER_PAIR TlsCipherMappingTable[] = {
{ 0x0068, "DH-DSS-AES256-SHA256" }, /// TLS_DH_DSS_WITH_AES_256_CBC_SHA256
{ 0x0069, "DH-RSA-AES256-SHA256" }, /// TLS_DH_RSA_WITH_AES_256_CBC_SHA256
{ 0x006B, "DHE-RSA-AES256-SHA256" } /// TLS_DHE_RSA_WITH_AES_256_CBC_SHA256
};
/**
- Gets the OpenSSL cipher suite string for the supplied IANA TLS cipher suite.
+ Gets the OpenSSL cipher suite mapping for the supplied IANA TLS cipher suite.
@param[in] CipherId The supplied IANA TLS cipher suite ID.
- @return The corresponding OpenSSL cipher suite string if found,
+ @return The corresponding OpenSSL cipher suite mapping if found,
NULL otherwise.
**/
STATIC
-CONST CHAR8 *
-TlsGetCipherString (
+CONST TLS_CIPHER_MAPPING *
+TlsGetCipherMapping (
IN UINT16 CipherId
)
{
- CONST TLS_CIPHER_PAIR *CipherEntry;
- UINTN TableSize;
- UINTN Index;
+ CONST TLS_CIPHER_MAPPING *CipherEntry;
+ UINTN TableSize;
+ UINTN Index;
CipherEntry = TlsCipherMappingTable;
- TableSize = sizeof (TlsCipherMappingTable) / sizeof (TLS_CIPHER_PAIR);
+ TableSize = sizeof (TlsCipherMappingTable) / sizeof (TLS_CIPHER_MAPPING);
//
// Search Cipher Mapping Table for IANA-OpenSSL Cipher Translation
//
for (Index = 0; Index < TableSize; Index++, CipherEntry++) {
//
// Translate IANA cipher suite name to OpenSSL name.
//
if (CipherEntry->IanaCipher == CipherId) {
- return CipherEntry->OpensslCipher;
+ return CipherEntry;
}
}
//
// No Cipher Mapping found, return NULL.
//
@@ -226,34 +226,37 @@ EFIAPI
TlsSetCipherList (
IN VOID *Tls,
IN UINT16 *CipherId,
IN UINTN CipherNum
)
{
- TLS_CONNECTION *TlsConn;
- UINTN Index;
- CONST CHAR8 *MappingName;
- CHAR8 CipherString[500];
+ TLS_CONNECTION *TlsConn;
+ UINTN Index;
+ CONST TLS_CIPHER_MAPPING *Mapping;
+ CONST CHAR8 *MappingName;
+ CHAR8 CipherString[500];
TlsConn = (TLS_CONNECTION *) Tls;
if (TlsConn == NULL || TlsConn->Ssl == NULL || CipherId == NULL) {
return EFI_INVALID_PARAMETER;
}
+ Mapping = NULL;
MappingName = NULL;
memset (CipherString, 0, sizeof (CipherString));
for (Index = 0; Index < CipherNum; Index++) {
//
// Handling OpenSSL / RFC Cipher name mapping.
//
- MappingName = TlsGetCipherString (*(CipherId + Index));
- if (MappingName == NULL) {
+ Mapping = TlsGetCipherMapping (*(CipherId + Index));
+ if (Mapping == NULL) {
return EFI_UNSUPPORTED;
}
+ MappingName = Mapping->OpensslCipher;
if (Index != 0) {
//
// The ciphers were separated by a colon.
//
AsciiStrCatS (CipherString, sizeof (CipherString), ":");
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 6/9] CryptoPkg/TlsLib: use binary search in the TlsGetCipherMapping() function
2018-04-11 10:42 [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites Laszlo Ersek
` (4 preceding siblings ...)
2018-04-11 10:42 ` [PATCH v2 5/9] CryptoPkg/TlsLib: replace TlsGetCipherString() with TlsGetCipherMapping() Laszlo Ersek
@ 2018-04-11 10:42 ` Laszlo Ersek
2018-04-11 10:42 ` [PATCH v2 7/9] CryptoPkg/TlsLib: pre-compute OpensslCipherLength in TlsCipherMappingTable Laszlo Ersek
` (5 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Laszlo Ersek @ 2018-04-11 10:42 UTC (permalink / raw)
To: edk2-devel; +Cc: Jiaxin Wu, Qin Long, Siyuan Fu, Ting Ye
Improve the performance of the TlsGetCipherMapping() function by adopting
the binary search from DhcpFindOptionFormat()
[MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Option.c].
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Qin Long <qin.long@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Ting Ye <ting.ye@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=915
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- no change
CryptoPkg/Library/TlsLib/TlsConfig.c | 36 +++++++++++++-------
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/CryptoPkg/Library/TlsLib/TlsConfig.c b/CryptoPkg/Library/TlsLib/TlsConfig.c
index 507489386b8e..c1d91a599482 100644
--- a/CryptoPkg/Library/TlsLib/TlsConfig.c
+++ b/CryptoPkg/Library/TlsLib/TlsConfig.c
@@ -27,12 +27,14 @@ typedef struct {
} TLS_CIPHER_MAPPING;
//
// The mapping table between IANA/IETF Cipher Suite definitions and
// OpenSSL-used Cipher Suite name.
//
+// Keep the table uniquely sorted by the IanaCipher field, in increasing order.
+//
STATIC CONST TLS_CIPHER_MAPPING TlsCipherMappingTable[] = {
{ 0x0001, "NULL-MD5" }, /// TLS_RSA_WITH_NULL_MD5
{ 0x0002, "NULL-SHA" }, /// TLS_RSA_WITH_NULL_SHA
{ 0x0004, "RC4-MD5" }, /// TLS_RSA_WITH_RC4_128_MD5
{ 0x0005, "RC4-SHA" }, /// TLS_RSA_WITH_RC4_128_SHA
{ 0x000A, "DES-CBC3-SHA" }, /// TLS_RSA_WITH_3DES_EDE_CBC_SHA, mandatory TLS 1.1
@@ -68,28 +70,36 @@ STATIC CONST TLS_CIPHER_MAPPING TlsCipherMappingTable[] = {
STATIC
CONST TLS_CIPHER_MAPPING *
TlsGetCipherMapping (
IN UINT16 CipherId
)
{
- CONST TLS_CIPHER_MAPPING *CipherEntry;
- UINTN TableSize;
- UINTN Index;
-
- CipherEntry = TlsCipherMappingTable;
- TableSize = sizeof (TlsCipherMappingTable) / sizeof (TLS_CIPHER_MAPPING);
+ INTN Left;
+ INTN Right;
+ INTN Middle;
//
- // Search Cipher Mapping Table for IANA-OpenSSL Cipher Translation
+ // Binary Search Cipher Mapping Table for IANA-OpenSSL Cipher Translation
//
- for (Index = 0; Index < TableSize; Index++, CipherEntry++) {
- //
- // Translate IANA cipher suite name to OpenSSL name.
- //
- if (CipherEntry->IanaCipher == CipherId) {
- return CipherEntry;
+ Left = 0;
+ Right = ARRAY_SIZE (TlsCipherMappingTable) - 1;
+
+ while (Right >= Left) {
+ Middle = (Left + Right) / 2;
+
+ if (CipherId == TlsCipherMappingTable[Middle].IanaCipher) {
+ //
+ // Translate IANA cipher suite ID to OpenSSL name.
+ //
+ return &TlsCipherMappingTable[Middle];
+ }
+
+ if (CipherId < TlsCipherMappingTable[Middle].IanaCipher) {
+ Right = Middle - 1;
+ } else {
+ Left = Middle + 1;
}
}
//
// No Cipher Mapping found, return NULL.
//
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 7/9] CryptoPkg/TlsLib: pre-compute OpensslCipherLength in TlsCipherMappingTable
2018-04-11 10:42 [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites Laszlo Ersek
` (5 preceding siblings ...)
2018-04-11 10:42 ` [PATCH v2 6/9] CryptoPkg/TlsLib: use binary search in the TlsGetCipherMapping() function Laszlo Ersek
@ 2018-04-11 10:42 ` Laszlo Ersek
2018-04-11 10:42 ` [PATCH v2 8/9] CryptoPkg/TlsLib: sanitize lib classes in internal header and INF Laszlo Ersek
` (4 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Laszlo Ersek @ 2018-04-11 10:42 UTC (permalink / raw)
To: edk2-devel; +Cc: Jiaxin Wu, Qin Long, Siyuan Fu, Ting Ye
In the next patches, we'll need the lengths of the
TLS_CIPHER_MAPPING.OpensslCipher string fields. These lengths can be
computed at build time; add the new field "OpensslCipherLength", and
introduce the MAP() macro for populating it.
While at it, add some horizontal whitespace to "TlsCipherMappingTable",
and add a comma after the last element. This will come handy in a later
patch.
(The patch does not change the first two columns of
"TlsCipherMappingTable", which can be easily verified with "git show
--word-diff".)
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Qin Long <qin.long@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Ting Ye <ting.ye@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=915
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- no change
CryptoPkg/Library/TlsLib/TlsConfig.c | 58 ++++++++++++--------
1 file changed, 35 insertions(+), 23 deletions(-)
diff --git a/CryptoPkg/Library/TlsLib/TlsConfig.c b/CryptoPkg/Library/TlsLib/TlsConfig.c
index c1d91a599482..e2f819b9035f 100644
--- a/CryptoPkg/Library/TlsLib/TlsConfig.c
+++ b/CryptoPkg/Library/TlsLib/TlsConfig.c
@@ -21,44 +21,56 @@ typedef struct {
//
UINT16 IanaCipher;
//
// OpenSSL-used Cipher Suite String
//
CONST CHAR8 *OpensslCipher;
+ //
+ // Length of OpensslCipher
+ //
+ UINTN OpensslCipherLength;
} TLS_CIPHER_MAPPING;
+//
+// Create a TLS_CIPHER_MAPPING initializer from IanaCipher and OpensslCipher so
+// that OpensslCipherLength is filled in automatically. IanaCipher must be an
+// integer constant expression, and OpensslCipher must be a string literal.
+//
+#define MAP(IanaCipher, OpensslCipher) \
+ { (IanaCipher), (OpensslCipher), sizeof (OpensslCipher) - 1 }
+
//
// The mapping table between IANA/IETF Cipher Suite definitions and
// OpenSSL-used Cipher Suite name.
//
// Keep the table uniquely sorted by the IanaCipher field, in increasing order.
//
STATIC CONST TLS_CIPHER_MAPPING TlsCipherMappingTable[] = {
- { 0x0001, "NULL-MD5" }, /// TLS_RSA_WITH_NULL_MD5
- { 0x0002, "NULL-SHA" }, /// TLS_RSA_WITH_NULL_SHA
- { 0x0004, "RC4-MD5" }, /// TLS_RSA_WITH_RC4_128_MD5
- { 0x0005, "RC4-SHA" }, /// TLS_RSA_WITH_RC4_128_SHA
- { 0x000A, "DES-CBC3-SHA" }, /// TLS_RSA_WITH_3DES_EDE_CBC_SHA, mandatory TLS 1.1
- { 0x0016, "DHE-RSA-DES-CBC3-SHA" }, /// TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA
- { 0x002F, "AES128-SHA" }, /// TLS_RSA_WITH_AES_128_CBC_SHA, mandatory TLS 1.2
- { 0x0030, "DH-DSS-AES128-SHA" }, /// TLS_DH_DSS_WITH_AES_128_CBC_SHA
- { 0x0031, "DH-RSA-AES128-SHA" }, /// TLS_DH_RSA_WITH_AES_128_CBC_SHA
- { 0x0033, "DHE-RSA-AES128-SHA" }, /// TLS_DHE_RSA_WITH_AES_128_CBC_SHA
- { 0x0035, "AES256-SHA" }, /// TLS_RSA_WITH_AES_256_CBC_SHA
- { 0x0036, "DH-DSS-AES256-SHA" }, /// TLS_DH_DSS_WITH_AES_256_CBC_SHA
- { 0x0037, "DH-RSA-AES256-SHA" }, /// TLS_DH_RSA_WITH_AES_256_CBC_SHA
- { 0x0039, "DHE-RSA-AES256-SHA" }, /// TLS_DHE_RSA_WITH_AES_256_CBC_SHA
- { 0x003B, "NULL-SHA256" }, /// TLS_RSA_WITH_NULL_SHA256
- { 0x003C, "AES128-SHA256" }, /// TLS_RSA_WITH_AES_128_CBC_SHA256
- { 0x003D, "AES256-SHA256" }, /// TLS_RSA_WITH_AES_256_CBC_SHA256
- { 0x003E, "DH-DSS-AES128-SHA256" }, /// TLS_DH_DSS_WITH_AES_128_CBC_SHA256
- { 0x003F, "DH-RSA-AES128-SHA256" }, /// TLS_DH_RSA_WITH_AES_128_CBC_SHA256
- { 0x0067, "DHE-RSA-AES128-SHA256" }, /// TLS_DHE_RSA_WITH_AES_128_CBC_SHA256
- { 0x0068, "DH-DSS-AES256-SHA256" }, /// TLS_DH_DSS_WITH_AES_256_CBC_SHA256
- { 0x0069, "DH-RSA-AES256-SHA256" }, /// TLS_DH_RSA_WITH_AES_256_CBC_SHA256
- { 0x006B, "DHE-RSA-AES256-SHA256" } /// TLS_DHE_RSA_WITH_AES_256_CBC_SHA256
+ MAP ( 0x0001, "NULL-MD5" ), /// TLS_RSA_WITH_NULL_MD5
+ MAP ( 0x0002, "NULL-SHA" ), /// TLS_RSA_WITH_NULL_SHA
+ MAP ( 0x0004, "RC4-MD5" ), /// TLS_RSA_WITH_RC4_128_MD5
+ MAP ( 0x0005, "RC4-SHA" ), /// TLS_RSA_WITH_RC4_128_SHA
+ MAP ( 0x000A, "DES-CBC3-SHA" ), /// TLS_RSA_WITH_3DES_EDE_CBC_SHA, mandatory TLS 1.1
+ MAP ( 0x0016, "DHE-RSA-DES-CBC3-SHA" ), /// TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA
+ MAP ( 0x002F, "AES128-SHA" ), /// TLS_RSA_WITH_AES_128_CBC_SHA, mandatory TLS 1.2
+ MAP ( 0x0030, "DH-DSS-AES128-SHA" ), /// TLS_DH_DSS_WITH_AES_128_CBC_SHA
+ MAP ( 0x0031, "DH-RSA-AES128-SHA" ), /// TLS_DH_RSA_WITH_AES_128_CBC_SHA
+ MAP ( 0x0033, "DHE-RSA-AES128-SHA" ), /// TLS_DHE_RSA_WITH_AES_128_CBC_SHA
+ MAP ( 0x0035, "AES256-SHA" ), /// TLS_RSA_WITH_AES_256_CBC_SHA
+ MAP ( 0x0036, "DH-DSS-AES256-SHA" ), /// TLS_DH_DSS_WITH_AES_256_CBC_SHA
+ MAP ( 0x0037, "DH-RSA-AES256-SHA" ), /// TLS_DH_RSA_WITH_AES_256_CBC_SHA
+ MAP ( 0x0039, "DHE-RSA-AES256-SHA" ), /// TLS_DHE_RSA_WITH_AES_256_CBC_SHA
+ MAP ( 0x003B, "NULL-SHA256" ), /// TLS_RSA_WITH_NULL_SHA256
+ MAP ( 0x003C, "AES128-SHA256" ), /// TLS_RSA_WITH_AES_128_CBC_SHA256
+ MAP ( 0x003D, "AES256-SHA256" ), /// TLS_RSA_WITH_AES_256_CBC_SHA256
+ MAP ( 0x003E, "DH-DSS-AES128-SHA256" ), /// TLS_DH_DSS_WITH_AES_128_CBC_SHA256
+ MAP ( 0x003F, "DH-RSA-AES128-SHA256" ), /// TLS_DH_RSA_WITH_AES_128_CBC_SHA256
+ MAP ( 0x0067, "DHE-RSA-AES128-SHA256" ), /// TLS_DHE_RSA_WITH_AES_128_CBC_SHA256
+ MAP ( 0x0068, "DH-DSS-AES256-SHA256" ), /// TLS_DH_DSS_WITH_AES_256_CBC_SHA256
+ MAP ( 0x0069, "DH-RSA-AES256-SHA256" ), /// TLS_DH_RSA_WITH_AES_256_CBC_SHA256
+ MAP ( 0x006B, "DHE-RSA-AES256-SHA256" ), /// TLS_DHE_RSA_WITH_AES_256_CBC_SHA256
};
/**
Gets the OpenSSL cipher suite mapping for the supplied IANA TLS cipher suite.
@param[in] CipherId The supplied IANA TLS cipher suite ID.
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 8/9] CryptoPkg/TlsLib: sanitize lib classes in internal header and INF
2018-04-11 10:42 [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites Laszlo Ersek
` (6 preceding siblings ...)
2018-04-11 10:42 ` [PATCH v2 7/9] CryptoPkg/TlsLib: pre-compute OpensslCipherLength in TlsCipherMappingTable Laszlo Ersek
@ 2018-04-11 10:42 ` Laszlo Ersek
2018-04-11 10:42 ` [PATCH v2 9/9] CryptoPkg/TlsLib: rewrite TlsSetCipherList() Laszlo Ersek
` (3 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Laszlo Ersek @ 2018-04-11 10:42 UTC (permalink / raw)
To: edk2-devel; +Cc: Jiaxin Wu, Qin Long, Siyuan Fu, Ting Ye
"InternalTlsLib.h" includes "BaseCryptLib.h", but the lib class is not
listed in the INF file.
The INF file lists a good number of lib classes, but none of the lib class
headers are included by "InternalTlsLib.h".
Synchronize & sort both lists, while removing those library classes that
aren't actually needed. (IntrinsicLib and OpensslLib have no edk2 class
headers.)
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Qin Long <qin.long@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Ting Ye <ting.ye@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=915
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- incorporate patch "CryptoPkg/TlsLib: sort [LibraryClasses] section in
the INF file" from v1
CryptoPkg/Library/TlsLib/TlsLib.inf | 6 ++----
CryptoPkg/Library/TlsLib/InternalTlsLib.h | 3 +++
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/CryptoPkg/Library/TlsLib/TlsLib.inf b/CryptoPkg/Library/TlsLib/TlsLib.inf
index a3f93e7165cb..ae17a7d87444 100644
--- a/CryptoPkg/Library/TlsLib/TlsLib.inf
+++ b/CryptoPkg/Library/TlsLib/TlsLib.inf
@@ -36,20 +36,18 @@ [Sources]
[Packages]
MdePkg/MdePkg.dec
CryptoPkg/CryptoPkg.dec
[LibraryClasses]
+ BaseCryptLib
BaseLib
BaseMemoryLib
- MemoryAllocationLib
- UefiRuntimeServicesTableLib
DebugLib
- OpensslLib
IntrinsicLib
- PrintLib
+ OpensslLib
[BuildOptions]
#
# suppress the following warnings so we do not break the build with warnings-as-errors:
# C4090: 'function' : different 'const' qualifiers
#
diff --git a/CryptoPkg/Library/TlsLib/InternalTlsLib.h b/CryptoPkg/Library/TlsLib/InternalTlsLib.h
index 88c4e3b38e4e..3f18a461a8d1 100644
--- a/CryptoPkg/Library/TlsLib/InternalTlsLib.h
+++ b/CryptoPkg/Library/TlsLib/InternalTlsLib.h
@@ -16,12 +16,15 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#define __INTERNAL_TLS_LIB_H__
#undef _WIN32
#undef _WIN64
#include <Library/BaseCryptLib.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
#include <openssl/ssl.h>
#include <openssl/bio.h>
#include <openssl/err.h>
typedef struct {
//
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 9/9] CryptoPkg/TlsLib: rewrite TlsSetCipherList()
2018-04-11 10:42 [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites Laszlo Ersek
` (7 preceding siblings ...)
2018-04-11 10:42 ` [PATCH v2 8/9] CryptoPkg/TlsLib: sanitize lib classes in internal header and INF Laszlo Ersek
@ 2018-04-11 10:42 ` Laszlo Ersek
2018-04-12 6:32 ` [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites Long, Qin
` (2 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Laszlo Ersek @ 2018-04-11 10:42 UTC (permalink / raw)
To: edk2-devel; +Cc: Jiaxin Wu, Qin Long, Siyuan Fu, Ting Ye
Rewrite the TlsSetCipherList() function in order to fix the following
issues:
- Any cipher identifier in CipherId that is not recognized by
TlsGetCipherMapping() will cause the function to return EFI_UNSUPPORTED.
This is a problem because CipherId is an ordered preference list, and a
caller should not get EFI_UNSUPPORTED just because it has an elaborate
CipherId preference list. Instead, we can filter out cipher identifiers
that we don't recognize, as long as we keep the relative order intact.
- CipherString is allocated on the stack, with 500 bytes.
While processing a large CipherId preference list, this room may not be
enough. Although no buffer overflow is possible, CipherString exhaustion
can lead to a failed TLS connection, because any cipher names that don't
fit on CipherString cannot be negotiated.
Compute CipherStringSize first, and allocate CipherString dynamically.
- Finally, the "@STRENGTH" pseudo cipher name is appended to CipherString.
(Assuming there is enough room left in CipherString.) This causes
OpenSSL to sort the cipher list "in order of encryption algorithm key
length".
This is a bad idea. The caller specifically passes an ordered preference
list in CipherId. Therefore TlsSetCipherList() must not ask OpenSSL to
reorder the list, for any reason. Drop "@STRENGTH".
While at it, fix and unify the documentation of the CipherId parameter.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Qin Long <qin.long@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Ting Ye <ting.ye@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=915
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- incorporate patch "CryptoPkg/TlsLib: clean up leading comment for
TlsSetCipherList()" from v1
CryptoPkg/Library/TlsLib/TlsLib.inf | 3 +-
CryptoPkg/Include/Library/TlsLib.h | 9 +-
CryptoPkg/Library/TlsLib/InternalTlsLib.h | 3 +-
CryptoPkg/Library/TlsLib/TlsConfig.c | 168 +++++++++++++++++---
4 files changed, 157 insertions(+), 26 deletions(-)
diff --git a/CryptoPkg/Library/TlsLib/TlsLib.inf b/CryptoPkg/Library/TlsLib/TlsLib.inf
index ae17a7d87444..4dacb2fab014 100644
--- a/CryptoPkg/Library/TlsLib/TlsLib.inf
+++ b/CryptoPkg/Library/TlsLib/TlsLib.inf
@@ -37,17 +37,18 @@ [Sources]
[Packages]
MdePkg/MdePkg.dec
CryptoPkg/CryptoPkg.dec
[LibraryClasses]
BaseCryptLib
- BaseLib
BaseMemoryLib
DebugLib
IntrinsicLib
+ MemoryAllocationLib
OpensslLib
+ SafeIntLib
[BuildOptions]
#
# suppress the following warnings so we do not break the build with warnings-as-errors:
# C4090: 'function' : different 'const' qualifiers
#
diff --git a/CryptoPkg/Include/Library/TlsLib.h b/CryptoPkg/Include/Library/TlsLib.h
index e19a38a214ce..e71291eaea45 100644
--- a/CryptoPkg/Include/Library/TlsLib.h
+++ b/CryptoPkg/Include/Library/TlsLib.h
@@ -345,19 +345,22 @@ TlsSetConnectionEnd (
/**
Set the ciphers list to be used by the TLS object.
This function sets the ciphers for use by a specified TLS object.
@param[in] Tls Pointer to a TLS object.
- @param[in] CipherId Pointer to a string that contains one or more
- ciphers separated by a colon.
+ @param[in] CipherId Array of UINT16 cipher identifiers. Each UINT16
+ cipher identifier comes from the TLS Cipher Suite
+ Registry of the IANA, interpreting Byte1 and Byte2
+ in network (big endian) byte order.
@param[in] CipherNum The number of cipher in the list.
@retval EFI_SUCCESS The ciphers list was set successfully.
@retval EFI_INVALID_PARAMETER The parameter is invalid.
- @retval EFI_UNSUPPORTED Unsupported TLS cipher in the list.
+ @retval EFI_UNSUPPORTED No supported TLS cipher was found in CipherId.
+ @retval EFI_OUT_OF_RESOURCES Memory allocation failed.
**/
EFI_STATUS
EFIAPI
TlsSetCipherList (
IN VOID *Tls,
diff --git a/CryptoPkg/Library/TlsLib/InternalTlsLib.h b/CryptoPkg/Library/TlsLib/InternalTlsLib.h
index 3f18a461a8d1..b6cf9816aa38 100644
--- a/CryptoPkg/Library/TlsLib/InternalTlsLib.h
+++ b/CryptoPkg/Library/TlsLib/InternalTlsLib.h
@@ -16,15 +16,16 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#define __INTERNAL_TLS_LIB_H__
#undef _WIN32
#undef _WIN64
#include <Library/BaseCryptLib.h>
-#include <Library/BaseLib.h>
#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/SafeIntLib.h>
#include <openssl/ssl.h>
#include <openssl/bio.h>
#include <openssl/err.h>
typedef struct {
//
diff --git a/CryptoPkg/Library/TlsLib/TlsConfig.c b/CryptoPkg/Library/TlsLib/TlsConfig.c
index e2f819b9035f..9154686610e0 100644
--- a/CryptoPkg/Library/TlsLib/TlsConfig.c
+++ b/CryptoPkg/Library/TlsLib/TlsConfig.c
@@ -232,74 +232,200 @@ TlsSetConnectionEnd (
/**
Set the ciphers list to be used by the TLS object.
This function sets the ciphers for use by a specified TLS object.
@param[in] Tls Pointer to a TLS object.
- @param[in] CipherId Pointer to a UINT16 cipher Id.
+ @param[in] CipherId Array of UINT16 cipher identifiers. Each UINT16
+ cipher identifier comes from the TLS Cipher Suite
+ Registry of the IANA, interpreting Byte1 and Byte2
+ in network (big endian) byte order.
@param[in] CipherNum The number of cipher in the list.
@retval EFI_SUCCESS The ciphers list was set successfully.
@retval EFI_INVALID_PARAMETER The parameter is invalid.
- @retval EFI_UNSUPPORTED Unsupported TLS cipher in the list.
+ @retval EFI_UNSUPPORTED No supported TLS cipher was found in CipherId.
+ @retval EFI_OUT_OF_RESOURCES Memory allocation failed.
**/
EFI_STATUS
EFIAPI
TlsSetCipherList (
IN VOID *Tls,
IN UINT16 *CipherId,
IN UINTN CipherNum
)
{
TLS_CONNECTION *TlsConn;
+ EFI_STATUS Status;
+ CONST TLS_CIPHER_MAPPING **MappedCipher;
+ UINTN MappedCipherBytes;
+ UINTN MappedCipherCount;
+ UINTN CipherStringSize;
UINTN Index;
CONST TLS_CIPHER_MAPPING *Mapping;
- CONST CHAR8 *MappingName;
- CHAR8 CipherString[500];
+ CHAR8 *CipherString;
+ CHAR8 *CipherStringPosition;
TlsConn = (TLS_CONNECTION *) Tls;
if (TlsConn == NULL || TlsConn->Ssl == NULL || CipherId == NULL) {
return EFI_INVALID_PARAMETER;
}
- Mapping = NULL;
- MappingName = NULL;
-
- memset (CipherString, 0, sizeof (CipherString));
+ //
+ // Allocate the MappedCipher array for recording the mappings that we find
+ // for the input IANA identifiers in CipherId.
+ //
+ Status = SafeUintnMult (CipherNum, sizeof (*MappedCipher),
+ &MappedCipherBytes);
+ if (EFI_ERROR (Status)) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+ MappedCipher = AllocatePool (MappedCipherBytes);
+ if (MappedCipher == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+ //
+ // Map the cipher IDs, and count the number of bytes for the full
+ // CipherString.
+ //
+ MappedCipherCount = 0;
+ CipherStringSize = 0;
for (Index = 0; Index < CipherNum; Index++) {
//
- // Handling OpenSSL / RFC Cipher name mapping.
+ // Look up the IANA-to-OpenSSL mapping.
//
- Mapping = TlsGetCipherMapping (*(CipherId + Index));
+ Mapping = TlsGetCipherMapping (CipherId[Index]);
if (Mapping == NULL) {
- return EFI_UNSUPPORTED;
- }
- MappingName = Mapping->OpensslCipher;
-
- if (Index != 0) {
+ DEBUG ((DEBUG_VERBOSE, "%a:%a: skipping CipherId=0x%04x\n",
+ gEfiCallerBaseName, __FUNCTION__, CipherId[Index]));
//
- // The ciphers were separated by a colon.
+ // Skipping the cipher is valid because CipherId is an ordered
+ // preference list of ciphers, thus we can filter it as long as we
+ // don't change the relative order of elements on it.
//
- AsciiStrCatS (CipherString, sizeof (CipherString), ":");
+ continue;
+ }
+ //
+ // Accumulate Mapping->OpensslCipherLength into CipherStringSize. If this
+ // is not the first successful mapping, account for a colon (":") prefix
+ // too.
+ //
+ if (MappedCipherCount > 0) {
+ Status = SafeUintnAdd (CipherStringSize, 1, &CipherStringSize);
+ if (EFI_ERROR (Status)) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto FreeMappedCipher;
+ }
+ }
+ Status = SafeUintnAdd (CipherStringSize, Mapping->OpensslCipherLength,
+ &CipherStringSize);
+ if (EFI_ERROR (Status)) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto FreeMappedCipher;
}
+ //
+ // Record the mapping.
+ //
+ MappedCipher[MappedCipherCount++] = Mapping;
+ }
- AsciiStrCatS (CipherString, sizeof (CipherString), MappingName);
+ //
+ // Verify that at least one IANA cipher ID could be mapped; account for the
+ // terminating NUL character in CipherStringSize; allocate CipherString.
+ //
+ if (MappedCipherCount == 0) {
+ DEBUG ((DEBUG_ERROR, "%a:%a: no CipherId could be mapped\n",
+ gEfiCallerBaseName, __FUNCTION__));
+ Status = EFI_UNSUPPORTED;
+ goto FreeMappedCipher;
+ }
+ Status = SafeUintnAdd (CipherStringSize, 1, &CipherStringSize);
+ if (EFI_ERROR (Status)) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto FreeMappedCipher;
+ }
+ CipherString = AllocatePool (CipherStringSize);
+ if (CipherString == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto FreeMappedCipher;
}
- AsciiStrCatS (CipherString, sizeof (CipherString), ":@STRENGTH");
+ //
+ // Go over the collected mappings and populate CipherString.
+ //
+ CipherStringPosition = CipherString;
+ for (Index = 0; Index < MappedCipherCount; Index++) {
+ Mapping = MappedCipher[Index];
+ //
+ // Append the colon (":") prefix except for the first mapping, then append
+ // Mapping->OpensslCipher.
+ //
+ if (Index > 0) {
+ *(CipherStringPosition++) = ':';
+ }
+ CopyMem (CipherStringPosition, Mapping->OpensslCipher,
+ Mapping->OpensslCipherLength);
+ CipherStringPosition += Mapping->OpensslCipherLength;
+ }
+
+ //
+ // NUL-terminate CipherString.
+ //
+ *(CipherStringPosition++) = '\0';
+ ASSERT (CipherStringPosition == CipherString + CipherStringSize);
+
+ //
+ // Log CipherString for debugging. CipherString can be very long if the
+ // caller provided a large CipherId array, so log CipherString in segments of
+ // 79 non-newline characters. (MAX_DEBUG_MESSAGE_LENGTH is usually 0x100 in
+ // DebugLib instances.)
+ //
+ DEBUG_CODE (
+ UINTN FullLength;
+ UINTN SegmentLength;
+
+ FullLength = CipherStringSize - 1;
+ DEBUG ((DEBUG_VERBOSE, "%a:%a: CipherString={\n", gEfiCallerBaseName,
+ __FUNCTION__));
+ for (CipherStringPosition = CipherString;
+ CipherStringPosition < CipherString + FullLength;
+ CipherStringPosition += SegmentLength) {
+ SegmentLength = FullLength - (CipherStringPosition - CipherString);
+ if (SegmentLength > 79) {
+ SegmentLength = 79;
+ }
+ DEBUG ((DEBUG_VERBOSE, "%.*a\n", SegmentLength, CipherStringPosition));
+ }
+ DEBUG ((DEBUG_VERBOSE, "}\n"));
+ //
+ // Restore the pre-debug value of CipherStringPosition by skipping over the
+ // trailing NUL.
+ //
+ CipherStringPosition++;
+ ASSERT (CipherStringPosition == CipherString + CipherStringSize);
+ );
//
// Sets the ciphers for use by the Tls object.
//
if (SSL_set_cipher_list (TlsConn->Ssl, CipherString) <= 0) {
- return EFI_UNSUPPORTED;
+ Status = EFI_UNSUPPORTED;
+ goto FreeCipherString;
}
- return EFI_SUCCESS;
+ Status = EFI_SUCCESS;
+
+FreeCipherString:
+ FreePool (CipherString);
+
+FreeMappedCipher:
+ FreePool (MappedCipher);
+
+ return Status;
}
/**
Set the compression method for TLS/SSL operations.
This function handles TLS/SSL integrated compression methods.
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites
2018-04-11 10:42 [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites Laszlo Ersek
` (8 preceding siblings ...)
2018-04-11 10:42 ` [PATCH v2 9/9] CryptoPkg/TlsLib: rewrite TlsSetCipherList() Laszlo Ersek
@ 2018-04-12 6:32 ` Long, Qin
2018-04-12 8:51 ` Laszlo Ersek
2018-04-12 7:28 ` Wu, Jiaxin
2018-04-13 12:10 ` Laszlo Ersek
11 siblings, 1 reply; 21+ messages in thread
From: Long, Qin @ 2018-04-12 6:32 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@lists.01.org
Cc: Ard Biesheuvel, Gary Ching-Pang Lin, Wu, Jiaxin, Justen, Jordan L,
Gao, Liming, Kinney, Michael D, Fu, Siyuan, Ye, Ting
Hi, Laszlo,
The updated patch series looks good to me.
Reviewed-by: Long Qin <qin.long@intel.com>
Best Regards & Thanks,
LONG, Qin
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Wednesday, April 11, 2018 6:43 PM
To: edk2-devel@lists.01.org
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Gary Ching-Pang Lin <glin@suse.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Long, Qin <qin.long@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Ye, Ting <ting.ye@intel.com>
Subject: [PATCH v2 0/9] {Ovmf,Mde,Network,Crypto}Pkg: fixes+features for setting HTTPS cipher suites
Repo: https://github.com/lersek/edk2.git
Branch: tls_ciphers_v2
This is version 2 of the series posted earlier at
http://mid.mail-archive.com/20180403145149.8925-1-lersek@redhat.com
https://lists.01.org/pipermail/edk2-devel/2018-April/023402.html
Changes are noted per patch. One important change cannot be highlighted that way however, because it involves the dropping of the following two patches from v1:
[edk2] [PATCH 08/13] CryptoPkg/TlsLib: add the "TlsMappingTable.sh"
POSIX shell script
[edk2] [PATCH 09/13] CryptoPkg/TlsLib: extend "TlsCipherMappingTable"
I retested HTTPS boot with this series; it succeeded. The TLS cipher suite preference list came from the system-wide configuration on my
RHEL-7 laptop; basically the binary CipherId array from the command "openssl ciphers -V". The relevant lines from the OVMF log were:
> TlsAuthConfigDxe:SetCipherSuites: stored list of cipher suites (190
> byte(s)) [...]
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC030
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02C
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC028
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC024
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC014
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC00A
> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A5
> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A3
> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A1
> TlsDxe:TlsSetCipherList: skipping CipherId=0x009F
> TlsDxe:TlsSetCipherList: skipping CipherId=0x006A
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0038
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0088
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0087
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0086
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0085
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC032
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02E
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02A
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC026
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC00F
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC005
> TlsDxe:TlsSetCipherList: skipping CipherId=0x009D
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0084
> TlsDxe:TlsSetCipherList: skipping CipherId=0x008D
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02F
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02B
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC027
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC023
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC013
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC009
> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A4
> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A2
> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A0
> TlsDxe:TlsSetCipherList: skipping CipherId=0x009E
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0040
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0032
> TlsDxe:TlsSetCipherList: skipping CipherId=0x009A
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0099
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0098
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0097
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0045
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0044
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0043
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0042
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC031
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02D
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC029
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC025
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC00E
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC004
> TlsDxe:TlsSetCipherList: skipping CipherId=0x009C
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0096
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0041
> TlsDxe:TlsSetCipherList: skipping CipherId=0x008C
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC012
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC008
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0013
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0010
> TlsDxe:TlsSetCipherList: skipping CipherId=0x000D
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC00D
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC003
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0007
> TlsDxe:TlsSetCipherList: skipping CipherId=0x008B
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0021
> TlsDxe:TlsSetCipherList: skipping CipherId=0x001F
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0025
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0023
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC011
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC007
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC00C
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC002
> TlsDxe:TlsSetCipherList: skipping CipherId=0x008A
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0020
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0024
> TlsDxe:TlsSetCipherList: CipherString={
> DHE-RSA-AES256-SHA256:DH-RSA-AES256-SHA256:DH-DSS-AES256-SHA256:DHE-RS
> A-AES256-
> SHA:DH-RSA-AES256-SHA:DH-DSS-AES256-SHA:AES256-SHA256:AES256-SHA:DHE-R
> SA-AES128
> -SHA256:DH-RSA-AES128-SHA256:DH-DSS-AES128-SHA256:DHE-RSA-AES128-SHA:D
> H-RSA-AES
> 128-SHA:DH-DSS-AES128-SHA:AES128-SHA256:AES128-SHA:DHE-RSA-DES-CBC3-SH
> A:DES-CBC
> 3-SHA:RC4-SHA:RC4-MD5
> }
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Gary Ching-Pang Lin <glin@suse.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Qin Long <qin.long@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Ting Ye <ting.ye@intel.com>
Thanks,
Laszlo
Laszlo Ersek (9):
OvmfPkg/TlsAuthConfigLib: configure trusted cipher suites for HTTPS
boot
MdePkg/Include/Protocol/Tls.h: pack structures from the TLS RFC
NetworkPkg/TlsDxe: verify DataSize for EfiTlsCipherList
NetworkPkg/TlsDxe: clean up byte order conversion for EfiTlsCipherList
CryptoPkg/TlsLib: replace TlsGetCipherString() with
TlsGetCipherMapping()
CryptoPkg/TlsLib: use binary search in the TlsGetCipherMapping()
function
CryptoPkg/TlsLib: pre-compute OpensslCipherLength in
TlsCipherMappingTable
CryptoPkg/TlsLib: sanitize lib classes in internal header and INF
CryptoPkg/TlsLib: rewrite TlsSetCipherList()
CryptoPkg/Include/Library/TlsLib.h | 9 +-
CryptoPkg/Library/TlsLib/InternalTlsLib.h | 4 +
CryptoPkg/Library/TlsLib/TlsConfig.c | 279 +++++++++++++++-----
CryptoPkg/Library/TlsLib/TlsLib.inf | 9 +-
MdePkg/Include/Protocol/Tls.h | 10 +
NetworkPkg/TlsDxe/TlsProtocol.c | 17 +-
OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c | 98 +++++++
OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf | 3 +-
8 files changed, 353 insertions(+), 76 deletions(-)
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites
2018-04-12 6:32 ` [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites Long, Qin
@ 2018-04-12 8:51 ` Laszlo Ersek
0 siblings, 0 replies; 21+ messages in thread
From: Laszlo Ersek @ 2018-04-12 8:51 UTC (permalink / raw)
To: Long, Qin, edk2-devel@lists.01.org
Cc: Ard Biesheuvel, Gary Ching-Pang Lin, Wu, Jiaxin, Justen, Jordan L,
Gao, Liming, Kinney, Michael D, Fu, Siyuan, Ye, Ting
On 04/12/18 08:32, Long, Qin wrote:
> Hi, Laszlo,
>
> The updated patch series looks good to me.
>
> Reviewed-by: Long Qin <qin.long@intel.com>
Many thanks; I'll fix up the commit messages as suggested by Jiaxin and
Gary, and push the series sometime later today.
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites
2018-04-11 10:42 [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites Laszlo Ersek
` (9 preceding siblings ...)
2018-04-12 6:32 ` [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites Long, Qin
@ 2018-04-12 7:28 ` Wu, Jiaxin
2018-04-12 8:50 ` Laszlo Ersek
2018-04-13 12:10 ` Laszlo Ersek
11 siblings, 1 reply; 21+ messages in thread
From: Wu, Jiaxin @ 2018-04-12 7:28 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@lists.01.org
Cc: Ard Biesheuvel, Gary Ching-Pang Lin, Justen, Jordan L,
Gao, Liming, Kinney, Michael D, Long, Qin, Fu, Siyuan, Ye, Ting
Hi Laszlo,
In the commit log of patch 0001, "EFI_TLS_CA_CERTIFICATE_VARIABLE" should be "EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE".
Others looks good to me.
Series Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Thanks,
Jiaxin
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Wednesday, April 11, 2018 6:43 PM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Gary Ching-Pang Lin
> <glin@suse.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; Gao, Liming <liming.gao@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Long, Qin <qin.long@intel.com>;
> Fu, Siyuan <siyuan.fu@intel.com>; Ye, Ting <ting.ye@intel.com>
> Subject: [PATCH v2 0/9] {Ovmf,Mde,Network,Crypto}Pkg: fixes+features for
> setting HTTPS cipher suites
>
> Repo: https://github.com/lersek/edk2.git
> Branch: tls_ciphers_v2
>
> This is version 2 of the series posted earlier at
>
> http://mid.mail-archive.com/20180403145149.8925-1-lersek@redhat.com
> https://lists.01.org/pipermail/edk2-devel/2018-April/023402.html
>
> Changes are noted per patch. One important change cannot be highlighted
> that way however, because it involves the dropping of the following two
> patches from v1:
>
> [edk2] [PATCH 08/13] CryptoPkg/TlsLib: add the "TlsMappingTable.sh"
> POSIX shell script
>
> [edk2] [PATCH 09/13] CryptoPkg/TlsLib: extend "TlsCipherMappingTable"
>
> I retested HTTPS boot with this series; it succeeded. The TLS cipher
> suite preference list came from the system-wide configuration on my
> RHEL-7 laptop; basically the binary CipherId array from the command
> "openssl ciphers -V". The relevant lines from the OVMF log were:
>
> > TlsAuthConfigDxe:SetCipherSuites: stored list of cipher suites (190 byte(s))
> > [...]
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC030
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC02C
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC028
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC024
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC014
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC00A
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x00A5
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x00A3
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x00A1
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x009F
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x006A
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0038
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0088
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0087
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0086
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0085
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC032
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC02E
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC02A
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC026
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC00F
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC005
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x009D
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0084
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x008D
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC02F
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC02B
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC027
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC023
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC013
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC009
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x00A4
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x00A2
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x00A0
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x009E
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0040
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0032
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x009A
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0099
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0098
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0097
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0045
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0044
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0043
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0042
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC031
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC02D
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC029
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC025
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC00E
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC004
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x009C
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0096
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0041
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x008C
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC012
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC008
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0013
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0010
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x000D
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC00D
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC003
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0007
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x008B
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0021
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x001F
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0025
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0023
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC011
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC007
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC00C
> > TlsDxe:TlsSetCipherList: skipping CipherId=0xC002
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x008A
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0020
> > TlsDxe:TlsSetCipherList: skipping CipherId=0x0024
> > TlsDxe:TlsSetCipherList: CipherString={
> > DHE-RSA-AES256-SHA256:DH-RSA-AES256-SHA256:DH-DSS-AES256-
> SHA256:DHE-RSA-AES256-
> > SHA:DH-RSA-AES256-SHA:DH-DSS-AES256-SHA:AES256-SHA256:AES256-
> SHA:DHE-RSA-AES128
> > -SHA256:DH-RSA-AES128-SHA256:DH-DSS-AES128-SHA256:DHE-RSA-
> AES128-SHA:DH-RSA-AES
> > 128-SHA:DH-DSS-AES128-SHA:AES128-SHA256:AES128-SHA:DHE-RSA-DES-
> CBC3-SHA:DES-CBC
> > 3-SHA:RC4-SHA:RC4-MD5
> > }
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Gary Ching-Pang Lin <glin@suse.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Qin Long <qin.long@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Ting Ye <ting.ye@intel.com>
>
> Thanks,
> Laszlo
>
> Laszlo Ersek (9):
> OvmfPkg/TlsAuthConfigLib: configure trusted cipher suites for HTTPS
> boot
> MdePkg/Include/Protocol/Tls.h: pack structures from the TLS RFC
> NetworkPkg/TlsDxe: verify DataSize for EfiTlsCipherList
> NetworkPkg/TlsDxe: clean up byte order conversion for EfiTlsCipherList
> CryptoPkg/TlsLib: replace TlsGetCipherString() with
> TlsGetCipherMapping()
> CryptoPkg/TlsLib: use binary search in the TlsGetCipherMapping()
> function
> CryptoPkg/TlsLib: pre-compute OpensslCipherLength in
> TlsCipherMappingTable
> CryptoPkg/TlsLib: sanitize lib classes in internal header and INF
> CryptoPkg/TlsLib: rewrite TlsSetCipherList()
>
> CryptoPkg/Include/Library/TlsLib.h | 9 +-
> CryptoPkg/Library/TlsLib/InternalTlsLib.h | 4 +
> CryptoPkg/Library/TlsLib/TlsConfig.c | 279 +++++++++++++++-----
> CryptoPkg/Library/TlsLib/TlsLib.inf | 9 +-
> MdePkg/Include/Protocol/Tls.h | 10 +
> NetworkPkg/TlsDxe/TlsProtocol.c | 17 +-
> OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c | 98 +++++++
> OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf | 3 +-
> 8 files changed, 353 insertions(+), 76 deletions(-)
>
> --
> 2.14.1.3.gb7cf6e02401b
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites
2018-04-12 7:28 ` Wu, Jiaxin
@ 2018-04-12 8:50 ` Laszlo Ersek
0 siblings, 0 replies; 21+ messages in thread
From: Laszlo Ersek @ 2018-04-12 8:50 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@lists.01.org
Cc: Ard Biesheuvel, Gary Ching-Pang Lin, Justen, Jordan L,
Gao, Liming, Kinney, Michael D, Long, Qin, Fu, Siyuan, Ye, Ting
On 04/12/18 09:28, Wu, Jiaxin wrote:
> Hi Laszlo,
>
> In the commit log of patch 0001, "EFI_TLS_CA_CERTIFICATE_VARIABLE" should be "EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE".
Good catch, thanks!
> Others looks good to me.
>
> Series Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Thank you,
Laszlo
>
>
> Thanks,
> Jiaxin
>
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Wednesday, April 11, 2018 6:43 PM
>> To: edk2-devel@lists.01.org
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Gary Ching-Pang Lin
>> <glin@suse.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Justen, Jordan L
>> <jordan.l.justen@intel.com>; Gao, Liming <liming.gao@intel.com>; Kinney,
>> Michael D <michael.d.kinney@intel.com>; Long, Qin <qin.long@intel.com>;
>> Fu, Siyuan <siyuan.fu@intel.com>; Ye, Ting <ting.ye@intel.com>
>> Subject: [PATCH v2 0/9] {Ovmf,Mde,Network,Crypto}Pkg: fixes+features for
>> setting HTTPS cipher suites
>>
>> Repo: https://github.com/lersek/edk2.git
>> Branch: tls_ciphers_v2
>>
>> This is version 2 of the series posted earlier at
>>
>> http://mid.mail-archive.com/20180403145149.8925-1-lersek@redhat.com
>> https://lists.01.org/pipermail/edk2-devel/2018-April/023402.html
>>
>> Changes are noted per patch. One important change cannot be highlighted
>> that way however, because it involves the dropping of the following two
>> patches from v1:
>>
>> [edk2] [PATCH 08/13] CryptoPkg/TlsLib: add the "TlsMappingTable.sh"
>> POSIX shell script
>>
>> [edk2] [PATCH 09/13] CryptoPkg/TlsLib: extend "TlsCipherMappingTable"
>>
>> I retested HTTPS boot with this series; it succeeded. The TLS cipher
>> suite preference list came from the system-wide configuration on my
>> RHEL-7 laptop; basically the binary CipherId array from the command
>> "openssl ciphers -V". The relevant lines from the OVMF log were:
>>
>>> TlsAuthConfigDxe:SetCipherSuites: stored list of cipher suites (190 byte(s))
>>> [...]
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC030
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02C
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC028
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC024
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC014
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC00A
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A5
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A3
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A1
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x009F
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x006A
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0038
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0088
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0087
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0086
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0085
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC032
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02E
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02A
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC026
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC00F
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC005
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x009D
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0084
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x008D
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02F
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02B
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC027
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC023
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC013
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC009
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A4
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A2
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A0
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x009E
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0040
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0032
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x009A
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0099
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0098
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0097
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0045
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0044
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0043
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0042
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC031
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02D
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC029
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC025
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC00E
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC004
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x009C
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0096
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0041
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x008C
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC012
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC008
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0013
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0010
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x000D
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC00D
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC003
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0007
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x008B
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0021
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x001F
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0025
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0023
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC011
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC007
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC00C
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC002
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x008A
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0020
>>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0024
>>> TlsDxe:TlsSetCipherList: CipherString={
>>> DHE-RSA-AES256-SHA256:DH-RSA-AES256-SHA256:DH-DSS-AES256-
>> SHA256:DHE-RSA-AES256-
>>> SHA:DH-RSA-AES256-SHA:DH-DSS-AES256-SHA:AES256-SHA256:AES256-
>> SHA:DHE-RSA-AES128
>>> -SHA256:DH-RSA-AES128-SHA256:DH-DSS-AES128-SHA256:DHE-RSA-
>> AES128-SHA:DH-RSA-AES
>>> 128-SHA:DH-DSS-AES128-SHA:AES128-SHA256:AES128-SHA:DHE-RSA-DES-
>> CBC3-SHA:DES-CBC
>>> 3-SHA:RC4-SHA:RC4-MD5
>>> }
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Gary Ching-Pang Lin <glin@suse.com>
>> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Qin Long <qin.long@intel.com>
>> Cc: Siyuan Fu <siyuan.fu@intel.com>
>> Cc: Ting Ye <ting.ye@intel.com>
>>
>> Thanks,
>> Laszlo
>>
>> Laszlo Ersek (9):
>> OvmfPkg/TlsAuthConfigLib: configure trusted cipher suites for HTTPS
>> boot
>> MdePkg/Include/Protocol/Tls.h: pack structures from the TLS RFC
>> NetworkPkg/TlsDxe: verify DataSize for EfiTlsCipherList
>> NetworkPkg/TlsDxe: clean up byte order conversion for EfiTlsCipherList
>> CryptoPkg/TlsLib: replace TlsGetCipherString() with
>> TlsGetCipherMapping()
>> CryptoPkg/TlsLib: use binary search in the TlsGetCipherMapping()
>> function
>> CryptoPkg/TlsLib: pre-compute OpensslCipherLength in
>> TlsCipherMappingTable
>> CryptoPkg/TlsLib: sanitize lib classes in internal header and INF
>> CryptoPkg/TlsLib: rewrite TlsSetCipherList()
>>
>> CryptoPkg/Include/Library/TlsLib.h | 9 +-
>> CryptoPkg/Library/TlsLib/InternalTlsLib.h | 4 +
>> CryptoPkg/Library/TlsLib/TlsConfig.c | 279 +++++++++++++++-----
>> CryptoPkg/Library/TlsLib/TlsLib.inf | 9 +-
>> MdePkg/Include/Protocol/Tls.h | 10 +
>> NetworkPkg/TlsDxe/TlsProtocol.c | 17 +-
>> OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c | 98 +++++++
>> OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf | 3 +-
>> 8 files changed, 353 insertions(+), 76 deletions(-)
>>
>> --
>> 2.14.1.3.gb7cf6e02401b
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites
2018-04-11 10:42 [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites Laszlo Ersek
` (10 preceding siblings ...)
2018-04-12 7:28 ` Wu, Jiaxin
@ 2018-04-13 12:10 ` Laszlo Ersek
11 siblings, 0 replies; 21+ messages in thread
From: Laszlo Ersek @ 2018-04-13 12:10 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Ting Ye, Jordan Justen, Liming Gao, Jiaxin Wu,
Gary Ching-Pang Lin, Qin Long, Michael D Kinney, Siyuan Fu
On 04/11/18 12:42, Laszlo Ersek wrote:
> Repo: https://github.com/lersek/edk2.git
> Branch: tls_ciphers_v2
>
> This is version 2 of the series posted earlier at
>
> http://mid.mail-archive.com/20180403145149.8925-1-lersek@redhat.com
> https://lists.01.org/pipermail/edk2-devel/2018-April/023402.html
>
> Changes are noted per patch. One important change cannot be highlighted
> that way however, because it involves the dropping of the following two
> patches from v1:
>
> [edk2] [PATCH 08/13] CryptoPkg/TlsLib: add the "TlsMappingTable.sh"
> POSIX shell script
>
> [edk2] [PATCH 09/13] CryptoPkg/TlsLib: extend "TlsCipherMappingTable"
>
> I retested HTTPS boot with this series; it succeeded. The TLS cipher
> suite preference list came from the system-wide configuration on my
> RHEL-7 laptop; basically the binary CipherId array from the command
> "openssl ciphers -V". The relevant lines from the OVMF log were:
>
>> TlsAuthConfigDxe:SetCipherSuites: stored list of cipher suites (190 byte(s))
>> [...]
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC030
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02C
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC028
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC024
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC014
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC00A
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A5
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A3
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A1
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x009F
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x006A
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0038
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0088
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0087
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0086
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0085
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC032
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02E
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02A
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC026
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC00F
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC005
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x009D
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0084
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x008D
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02F
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02B
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC027
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC023
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC013
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC009
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A4
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A2
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A0
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x009E
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0040
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0032
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x009A
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0099
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0098
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0097
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0045
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0044
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0043
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0042
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC031
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02D
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC029
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC025
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC00E
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC004
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x009C
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0096
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0041
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x008C
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC012
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC008
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0013
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0010
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x000D
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC00D
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC003
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0007
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x008B
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0021
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x001F
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0025
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0023
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC011
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC007
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC00C
>> TlsDxe:TlsSetCipherList: skipping CipherId=0xC002
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x008A
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0020
>> TlsDxe:TlsSetCipherList: skipping CipherId=0x0024
>> TlsDxe:TlsSetCipherList: CipherString={
>> DHE-RSA-AES256-SHA256:DH-RSA-AES256-SHA256:DH-DSS-AES256-SHA256:DHE-RSA-AES256-
>> SHA:DH-RSA-AES256-SHA:DH-DSS-AES256-SHA:AES256-SHA256:AES256-SHA:DHE-RSA-AES128
>> -SHA256:DH-RSA-AES128-SHA256:DH-DSS-AES128-SHA256:DHE-RSA-AES128-SHA:DH-RSA-AES
>> 128-SHA:DH-DSS-AES128-SHA:AES128-SHA256:AES128-SHA:DHE-RSA-DES-CBC3-SHA:DES-CBC
>> 3-SHA:RC4-SHA:RC4-MD5
>> }
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Gary Ching-Pang Lin <glin@suse.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Qin Long <qin.long@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Ting Ye <ting.ye@intel.com>
>
> Thanks,
> Laszlo
>
> Laszlo Ersek (9):
> OvmfPkg/TlsAuthConfigLib: configure trusted cipher suites for HTTPS
> boot
> MdePkg/Include/Protocol/Tls.h: pack structures from the TLS RFC
> NetworkPkg/TlsDxe: verify DataSize for EfiTlsCipherList
> NetworkPkg/TlsDxe: clean up byte order conversion for EfiTlsCipherList
> CryptoPkg/TlsLib: replace TlsGetCipherString() with
> TlsGetCipherMapping()
> CryptoPkg/TlsLib: use binary search in the TlsGetCipherMapping()
> function
> CryptoPkg/TlsLib: pre-compute OpensslCipherLength in
> TlsCipherMappingTable
> CryptoPkg/TlsLib: sanitize lib classes in internal header and INF
> CryptoPkg/TlsLib: rewrite TlsSetCipherList()
>
> CryptoPkg/Include/Library/TlsLib.h | 9 +-
> CryptoPkg/Library/TlsLib/InternalTlsLib.h | 4 +
> CryptoPkg/Library/TlsLib/TlsConfig.c | 279 +++++++++++++++-----
> CryptoPkg/Library/TlsLib/TlsLib.inf | 9 +-
> MdePkg/Include/Protocol/Tls.h | 10 +
> NetworkPkg/TlsDxe/TlsProtocol.c | 17 +-
> OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c | 98 +++++++
> OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf | 3 +-
> 8 files changed, 353 insertions(+), 76 deletions(-)
>
Pushed as commit range 54ec85dd2902..2167c7f7a55b.
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 21+ messages in thread