[PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites

public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed

* [PATCH v2 0/9] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites
@ 2018-04-11 10:42 Laszlo Ersek
  2018-04-11 10:42 ` [PATCH v2 1/9] OvmfPkg/TlsAuthConfigLib: configure trusted cipher suites for HTTPS boot Laszlo Ersek
                   ` (11 more replies)
  0 siblings, 12 replies; 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, Jiaxin Wu, Jordan Justen,
	Liming Gao, Michael D Kinney, Qin Long, Siyuan Fu, Ting Ye

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

* [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

* [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 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 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 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 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-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 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

* 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

end of thread, other threads:[~2018-04-13 12:10 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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-12  7:08   ` Gary Lin
2018-04-12  8:49     ` Laszlo Ersek
2018-04-12  9:10       ` Gary Lin
2018-04-12  9:43         ` Laszlo Ersek
2018-04-12 10:17           ` Gary Lin
2018-04-12 17:10             ` 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 ` [PATCH v2 3/9] NetworkPkg/TlsDxe: verify DataSize for EfiTlsCipherList Laszlo Ersek
2018-04-11 10:42 ` [PATCH v2 4/9] NetworkPkg/TlsDxe: clean up byte order conversion " Laszlo Ersek
2018-04-11 10:42 ` [PATCH v2 5/9] CryptoPkg/TlsLib: replace TlsGetCipherString() with TlsGetCipherMapping() Laszlo Ersek
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 ` [PATCH v2 7/9] CryptoPkg/TlsLib: pre-compute OpensslCipherLength in TlsCipherMappingTable Laszlo Ersek
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 ` [PATCH v2 9/9] CryptoPkg/TlsLib: rewrite TlsSetCipherList() 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
2018-04-12  8:51   ` Laszlo Ersek
2018-04-12  7:28 ` Wu, Jiaxin
2018-04-12  8:50   ` Laszlo Ersek
2018-04-13 12:10 ` Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox