Re: [PATCH 1/2] CryptoPkg/BaseCryptLib: Add C-structure to matching certificate stack

["Long, Qin" <qin.long@intel.com>]

Reviewed-by: Long Qin <qin.long@intel.com>

One minor comment: please leave one space before the structure name:
+} EFI_CERT_DATA;
and
+} EFI_CERT_STACK;


Best Regards & Thanks,
LONG, Qin

-----Original Message-----
From: Chen, Chen A 
Sent: Tuesday, November 7, 2017 9:05 AM
To: edk2-devel@lists.01.org
Cc: Chen, Chen A <chen.a.chen@intel.com>; Long, Qin <qin.long@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>
Subject: [PATCH 1/2] CryptoPkg/BaseCryptLib: Add C-structure to matching certificate stack

The parameter CertStack of Pkcs7GetSigners will return all embedded X.509 certificate in one given PKCS7 signature. The format is:
//
// UINT8  CertNumber;
// UINT32 Cert1Length;
// UINT8  Cert1[];
// UINT32 Cert2Length;
// UINT8  Cert2[];
// ...
// UINT32 CertnLength;
// UINT8  Certn[];
//
Add EFI_CERT_STACK and EFI_CERT_DATA structure, these two C-structure are used for parsing CertStack more clearly.

Cc: Long Qin <qin.long@intel.com>
Cc: Zhang Chao <chao.b.zhang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: chenc2 <chen.a.chen@intel.com>
---
 CryptoPkg/Include/Library/BaseCryptLib.h           | 33 ++++++++++++++++++++++
 .../Library/BaseCryptLib/Pk/CryptPkcs7Verify.c     |  3 ++
 .../Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c |  3 ++
 3 files changed, 39 insertions(+)

diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h
index e2b6a95666..3fd9a3c911 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -2377,6 +2377,36 @@ Pkcs5HashPassword (
   );
 
 /**
+  The 3rd parameter of Pkcs7GetSigners will return all embedded
+  X.509 certificate in one given PKCS7 signature. The format is:
+  //
+  // UINT8  CertNumber;
+  // UINT32 Cert1Length;
+  // UINT8  Cert1[];
+  // UINT32 Cert2Length;
+  // UINT8  Cert2[];
+  // ...
+  // UINT32 CertnLength;
+  // UINT8  Certn[];
+  //
+
+  The two following C-structure are used for parsing CertStack more clearly.
+**/
+#pragma pack(1)
+
+typedef struct {
+  UINT32    CertDataLength;       // The length in bytes of X.509 certificate.
+  UINT8     CertDataBuffer[0];    // The X.509 certificate content (DER).
+}EFI_CERT_DATA;
+
+typedef struct {
+  UINT8             CertNumber;   // Number of X.509 certificate.
+  //EFI_CERT_DATA   CertArray[];  // An array of X.509 certificate.
+}EFI_CERT_STACK;
+
+#pragma pack()
+
+/**
   Get the signer's certificates from PKCS#7 signed data as described in "PKCS #7:
   Cryptographic Message Syntax Standard". The input signed data could be wrapped
   in a ContentInfo structure.
@@ -2390,6 +2420,7 @@ Pkcs5HashPassword (
   @param[out] CertStack    Pointer to Signer's certificates retrieved from P7Data.
                            It's caller's responsibility to free the buffer with
                            Pkcs7FreeSigners().
+                           This data structure is EFI_CERT_STACK type.
   @param[out] StackLength  Length of signer's certificates in bytes.
   @param[out] TrustedCert  Pointer to a trusted certificate from Signer's certificates.
                            It's caller's responsibility to free the buffer with @@ -2437,9 +2468,11 @@ Pkcs7FreeSigners (
   @param[out] SignerChainCerts  Pointer to the certificates list chained to signer's
                                 certificate. It's caller's responsibility to free the buffer
                                 with Pkcs7FreeSigners().
+                                This data structure is EFI_CERT_STACK type.
   @param[out] ChainLength       Length of the chained certificates list buffer in bytes.
   @param[out] UnchainCerts      Pointer to the unchained certificates lists. It's caller's
                                 responsibility to free the buffer with Pkcs7FreeSigners().
+                                This data structure is EFI_CERT_STACK type.
   @param[out] UnchainLength     Length of the unchained certificates list buffer in bytes.
 
   @retval  TRUE         The operation is finished successfully.
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c
index 296df028b1..fe8e5950f9 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c
@@ -242,6 +242,7 @@ _Exit:
   @param[out] CertStack    Pointer to Signer's certificates retrieved from P7Data.
                            It's caller's responsibility to free the buffer with
                            Pkcs7FreeSigners().
+                           This data structure is EFI_CERT_STACK type.
   @param[out] StackLength  Length of signer's certificates in bytes.
   @param[out] TrustedCert  Pointer to a trusted certificate from Signer's certificates.
                            It's caller's responsibility to free the buffer with @@ -442,9 +443,11 @@ Pkcs7FreeSigners (
   @param[out] SignerChainCerts  Pointer to the certificates list chained to signer's
                                 certificate. It's caller's responsibility to free the buffer
                                 with Pkcs7FreeSigners().
+                                This data structure is EFI_CERT_STACK type.
   @param[out] ChainLength       Length of the chained certificates list buffer in bytes.
   @param[out] UnchainCerts      Pointer to the unchained certificates lists. It's caller's
                                 responsibility to free the buffer with Pkcs7FreeSigners().
+                                This data structure is EFI_CERT_STACK type.
   @param[out] UnchainLength     Length of the unchained certificates list buffer in bytes.
 
   @retval  TRUE         The operation is finished successfully.
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
index d3e8ec89a7..5490b1f3d6 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
@@ -27,6 +27,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
   @param[out] CertStack    Pointer to Signer's certificates retrieved from P7Data.
                            It's caller's responsibility to free the buffer with
                            Pkcs7FreeSigners().
+                           This data structure is EFI_CERT_STACK type.
   @param[out] StackLength  Length of signer's certificates in bytes.
   @param[out] TrustedCert  Pointer to a trusted certificate from Signer's certificates.
                            It's caller's responsibility to free the buffer with @@ -79,9 +80,11 @@ Pkcs7FreeSigners (
   @param[out] SignerChainCerts  Pointer to the certificates list chained to signer's
                                 certificate. It's caller's responsibility to free the buffer
                                 with Pkcs7FreeSigners().
+                                This data structure is EFI_CERT_STACK type.
   @param[out] ChainLength       Length of the chained certificates list buffer in bytes.
   @param[out] UnchainCerts      Pointer to the unchained certificates lists. It's caller's
                                 responsibility to free the buffer with Pkcs7FreeSigners().
+                                This data structure is EFI_CERT_STACK type.
   @param[out] UnchainLength     Length of the unchained certificates list buffer in bytes.
 
   @retval  TRUE         The operation is finished successfully.
--
2.13.2.windows.1