NetworkPkg/HttpDxe: sanity-check the TlsCaCertificate variable before use

In TlsConfigCertificate(), make sure that the set of EFI_SIGNATURE_LIST
objects that the platform stored to "TlsCaCertificate" is well-formed.

In addition, because HttpInstance->TlsConfiguration->SetData() expects
X509 certificates only, ensure that the EFI_SIGNATURE_LIST objects only
report X509 certificates, as described under EFI_CERT_X509_GUID in the
UEFI-2.7 spec.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=909
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>

diff --git a/NetworkPkg/HttpDxe/HttpDxe.inf b/NetworkPkg/HttpDxe/HttpDxe.inf
index 938e894d9f09d42864d11b607b3caa5ca64f18fa..6c0688d1305bb749ef61b8052205b84fdc4098e1 100644 (file)
--- a/NetworkPkg/HttpDxe/HttpDxe.inf
+++ b/NetworkPkg/HttpDxe/HttpDxe.inf
@@ -75,9 +75,10 @@
 [Guids]\r
   gEfiTlsCaCertificateGuid                         ## SOMETIMES_CONSUMES  ## Variable:L"TlsCaCertificate"\r
   gEdkiiHttpTlsCipherListGuid                      ## SOMETIMES_CONSUMES  ## Variable:L"HttpTlsCipherList"\r
+  gEfiCertX509Guid                                 ## SOMETIMES_CONSUMES  ## GUID  # Check the cert type\r
 \r
 [Pcd]\r
   gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections       ## CONSUMES  \r
 \r
 [UserExtensions.TianoCore."ExtraFiles"]\r
-  HttpDxeExtra.uni
\ No newline at end of file
+  HttpDxeExtra.uni\r

diff --git a/NetworkPkg/HttpDxe/HttpsSupport.c b/NetworkPkg/HttpDxe/HttpsSupport.c
index baab77225fdfedf0570e7e6180469fd148cd158f..d658512f6d9fd5e881642b291a751148cc0cbdde 100644 (file)
--- a/NetworkPkg/HttpDxe/HttpsSupport.c
+++ b/NetworkPkg/HttpDxe/HttpsSupport.c
@@ -384,6 +384,7 @@ TlsConfigCertificate (
   UINT32              Index;\r
   EFI_SIGNATURE_LIST  *CertList;\r
   EFI_SIGNATURE_DATA  *Cert;\r
+  UINTN               CertArraySizeInBytes;\r
   UINTN               CertCount;\r
   UINT32              ItemDataSize;\r
 \r
@@ -429,6 +430,70 @@ TlsConfigCertificate (
 \r
   ASSERT (CACert != NULL);\r
 \r
+  //\r
+  // Sanity check\r
+  //\r
+  Status = EFI_INVALID_PARAMETER;\r
+  CertCount = 0;\r
+  ItemDataSize = (UINT32) CACertSize;\r
+  while (ItemDataSize > 0) {\r
+    if (ItemDataSize < sizeof (EFI_SIGNATURE_LIST)) {\r
+      DEBUG ((DEBUG_ERROR, "%a: truncated EFI_SIGNATURE_LIST header\n",\r
+        __FUNCTION__));\r
+      goto FreeCACert;\r
+    }\r
+\r
+    CertList = (EFI_SIGNATURE_LIST *) (CACert + (CACertSize - ItemDataSize));\r
+\r
+    if (CertList->SignatureListSize < sizeof (EFI_SIGNATURE_LIST)) {\r
+      DEBUG ((DEBUG_ERROR,\r
+        "%a: SignatureListSize too small for EFI_SIGNATURE_LIST\n",\r
+        __FUNCTION__));\r
+      goto FreeCACert;\r
+    }\r
+\r
+    if (CertList->SignatureListSize > ItemDataSize) {\r
+      DEBUG ((DEBUG_ERROR, "%a: truncated EFI_SIGNATURE_LIST body\n",\r
+        __FUNCTION__));\r
+      goto FreeCACert;\r
+    }\r
+\r
+    if (!CompareGuid (&CertList->SignatureType, &gEfiCertX509Guid)) {\r
+      DEBUG ((DEBUG_ERROR, "%a: only X509 certificates are supported\n",\r
+        __FUNCTION__));\r
+      Status = EFI_UNSUPPORTED;\r
+      goto FreeCACert;\r
+    }\r
+\r
+    if (CertList->SignatureHeaderSize != 0) {\r
+      DEBUG ((DEBUG_ERROR, "%a: SignatureHeaderSize must be 0 for X509\n",\r
+        __FUNCTION__));\r
+      goto FreeCACert;\r
+    }\r
+\r
+    if (CertList->SignatureSize < sizeof (EFI_SIGNATURE_DATA)) {\r
+      DEBUG ((DEBUG_ERROR,\r
+        "%a: SignatureSize too small for EFI_SIGNATURE_DATA\n", __FUNCTION__));\r
+      goto FreeCACert;\r
+    }\r
+\r
+    CertArraySizeInBytes = (CertList->SignatureListSize -\r
+                            sizeof (EFI_SIGNATURE_LIST));\r
+    if (CertArraySizeInBytes % CertList->SignatureSize != 0) {\r
+      DEBUG ((DEBUG_ERROR,\r
+        "%a: EFI_SIGNATURE_DATA array not a multiple of SignatureSize\n",\r
+        __FUNCTION__));\r
+      goto FreeCACert;\r
+    }\r
+\r
+    CertCount += CertArraySizeInBytes / CertList->SignatureSize;\r
+    ItemDataSize -= CertList->SignatureListSize;\r
+  }\r
+  if (CertCount == 0) {\r
+    DEBUG ((DEBUG_ERROR, "%a: no X509 certificates provided\n", __FUNCTION__));\r
+    goto FreeCACert;\r
+  }\r
+\r
   //\r
   // Enumerate all data and erasing the target item.\r
   //\r