public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate
@ 2017-09-19  3:38 Long Qin
  2017-09-20  6:57 ` Zhang, Chao B
  2017-09-20 12:09 ` Laszlo Ersek
  0 siblings, 2 replies; 6+ messages in thread
From: Long Qin @ 2017-09-19  3:38 UTC (permalink / raw)
  To: ting.ye, chao.b.zhang; +Cc: edk2-devel, Qin Long

Add one new API (X509GetCommonName()) to retrieve the subject commonName
string from one X.509 certificate.

Cc: Ting Ye <ting.ye@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long <qin.long@intel.com>
---
 CryptoPkg/Application/Cryptest/RsaVerify2.c        | 17 ++++
 CryptoPkg/Include/Library/BaseCryptLib.h           | 32 ++++++++
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c      | 93 ++++++++++++++++++++++
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c  | 32 ++++++++
 .../Pk/CryptX509Null.c                             | 34 +++++++-
 5 files changed, 207 insertions(+), 1 deletion(-)

diff --git a/CryptoPkg/Application/Cryptest/RsaVerify2.c b/CryptoPkg/Application/Cryptest/RsaVerify2.c
index 98b5aad900..f9b70d5794 100644
--- a/CryptoPkg/Application/Cryptest/RsaVerify2.c
+++ b/CryptoPkg/Application/Cryptest/RsaVerify2.c
@@ -211,6 +211,9 @@ ValidateCryptRsa2 (
   UINTN    SigSize;
   UINT8    *Subject;
   UINTN    SubjectSize;
+  CHAR8    CommonName[64];
+  CHAR16   CommonNameUnicode[64];
+  UINTN    CommonNameSize;
 
   Print (L"\nUEFI-OpenSSL RSA Key Retrieving Testing: ");
 
@@ -286,6 +289,20 @@ ValidateCryptRsa2 (
     Print (L"[Pass]");
   }
 
+  //
+  // Get CommonName from X509 Certificate Subject
+  //
+  CommonNameSize = 64;
+  ZeroMem (CommonName, CommonNameSize);
+  Status = X509GetCommonName (TestCert, sizeof (TestCert), CommonName, &CommonNameSize);
+  if (!Status) {
+    Print (L"\n  - Retrieving Common Name - [Fail]");
+    return EFI_ABORTED;
+  } else {
+    AsciiStrToUnicodeStrS (CommonName, CommonNameUnicode, CommonNameSize);
+    Print (L"\n  - Retrieving Common Name = \"%s\" (Size = %d)", CommonNameUnicode, CommonNameSize);
+  }
+
   //
   // X509 Certificate Verification.
   //
diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h
index 9c5ffcd9cf..d861be6725 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -2171,6 +2171,38 @@ X509GetSubjectName (
   IN OUT  UINTN        *SubjectSize
   );
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  If Cert or CommonNameSize is NULL, then return FALSE.
+  If this interface is not supported, then return FALSE.
+
+  @param[in]      Cert            Pointer to the DER-encoded X509 certificate.
+  @param[in]      CertSize        Size of the X509 certificate in bytes.
+  @param[out]     CommonName      Buffer to contain the retrieved certificate common
+                                  name string. At most CommonNameSize bytes will be
+                                  written and the string will be null terminated. May be
+                                  NULL in order to determine the size buffer needed.
+  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer on input,
+                                  and the size of buffer returned CommonName on output.
+                                  if CommonName is NULL then the amount of space needed
+                                  in buffer (including the final null) is returned.
+
+  @retval  TRUE   The certificate CommonName retrieved successfully.
+  @retval  FALSE  Invalid certificate, or CommonNameSize is NULL,
+                  or no CommonName entry exists.
+  @retval  FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+X509GetCommonName (
+  IN      CONST UINT8  *Cert,
+  IN      UINTN        CertSize,
+  OUT     CHAR8        *CommonName,
+  IN OUT  UINTN        *CommonNameSize
+  );
+
 /**
   Verify one X509 certificate was issued by the trusted CA.
 
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
index 7d275977c5..e45c214bd1 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
@@ -297,6 +297,99 @@ _Exit:
   return Status;
 }
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  If Cert or CommonNameSize is NULL, then return FALSE.
+  If this interface is not supported, then return FALSE.
+
+  @param[in]      Cert            Pointer to the DER-encoded X509 certificate.
+  @param[in]      CertSize        Size of the X509 certificate in bytes.
+  @param[out]     CommonName      Buffer to contain the retrieved certificate common
+                                  name string. At most CommonNameSize bytes will be
+                                  written and the string will be null terminated. May be
+                                  NULL in order to determine the size buffer needed.
+  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer on input,
+                                  and the size of buffer returned CommonName on output.
+                                  if CommonName is NULL then the amount of space needed
+                                  in buffer (including the final null) is returned.
+
+  @retval  TRUE   The certificate CommonName retrieved successfully.
+  @retval  FALSE  Invalid certificate, or CommonNameSize is NULL,
+                  or no CommonName entry exists.
+  @retval  FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+X509GetCommonName (
+  IN      CONST UINT8  *Cert,
+  IN      UINTN        CertSize,
+  OUT     CHAR8        *CommonName,
+  IN OUT  UINTN        *CommonNameSize
+  )
+{
+  BOOLEAN    Status;
+  X509       *X509Cert;
+  X509_NAME  *X509Name;
+  INTN       Length;
+
+  //
+  // Check input parameters.
+  //
+  if ((Cert == NULL) || (CommonNameSize == NULL)) {
+    return FALSE;
+  }
+
+  X509Cert = NULL;
+
+  //
+  // Read DER-encoded X509 Certificate and Construct X509 object.
+  //
+  Status = X509ConstructCertificate (Cert, CertSize, (UINT8 **) &X509Cert);
+  if ((X509Cert == NULL) || (!Status)) {
+    //
+    // Invalid X.509 Certificate
+    //
+    goto _Exit;
+  }
+
+  Status = FALSE;
+
+  //
+  // Retrieve subject name from certificate object.
+  //
+  X509Name = X509_get_subject_name (X509Cert);
+  if (X509Name == NULL) {
+    goto _Exit;
+  }
+
+  //
+  // Retrieve the CommonName information from X.509 Subject
+  //
+  Length = (INTN) X509_NAME_get_text_by_NID (X509Name, NID_commonName, CommonName, (int)(*CommonNameSize));
+  if (Length < 0) {
+    //
+    // No CommonName entry exists in X509_NAME object
+    //
+    *CommonNameSize = 0;
+    goto _Exit;
+  }
+
+  *CommonNameSize = (UINTN)(Length + 1);
+  Status = TRUE;
+
+_Exit:
+  //
+  // Release Resources.
+  //
+  if (X509Cert != NULL) {
+    X509_free (X509Cert);
+  }
+
+  return Status;
+}
+
 /**
   Retrieve the RSA Public Key from one DER-encoded X509 certificate.
 
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
index 51aa0633a8..81587003f2 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
@@ -127,6 +127,38 @@ X509GetSubjectName (
   return FALSE;
 }
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  Return FALSE to indicate this interface is not supported.
+
+  @param[in]      Cert            Pointer to the DER-encoded X509 certificate.
+  @param[in]      CertSize        Size of the X509 certificate in bytes.
+  @param[out]     CommonName      Buffer to contain the retrieved certificate common
+                                  name string. At most CommonNameSize bytes will be
+                                  written and the string will be null terminated. May be
+                                  NULL in order to determine the size buffer needed.
+  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer on input,
+                                  and the size of buffer returned CommonName on output.
+                                  if CommonName is NULL then the amount of space needed
+                                  in buffer (including the final null) is returned.
+
+  @retval  FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+X509GetCommonName (
+  IN      CONST UINT8  *Cert,
+  IN      UINTN        CertSize,
+  OUT     CHAR8        *CommonName,
+  IN OUT  UINTN        *CommonNameSize
+  )
+{
+  ASSERT (FALSE);
+  return FALSE;
+}
+
 /**
   Retrieve the RSA Public Key from one DER-encoded X509 certificate.
 
diff --git a/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c b/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c
index f5d9aa1076..81587003f2 100644
--- a/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c
+++ b/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c
@@ -127,6 +127,38 @@ X509GetSubjectName (
   return FALSE;
 }
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  Return FALSE to indicate this interface is not supported.
+
+  @param[in]      Cert            Pointer to the DER-encoded X509 certificate.
+  @param[in]      CertSize        Size of the X509 certificate in bytes.
+  @param[out]     CommonName      Buffer to contain the retrieved certificate common
+                                  name string. At most CommonNameSize bytes will be
+                                  written and the string will be null terminated. May be
+                                  NULL in order to determine the size buffer needed.
+  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer on input,
+                                  and the size of buffer returned CommonName on output.
+                                  if CommonName is NULL then the amount of space needed
+                                  in buffer (including the final null) is returned.
+
+  @retval  FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+X509GetCommonName (
+  IN      CONST UINT8  *Cert,
+  IN      UINTN        CertSize,
+  OUT     CHAR8        *CommonName,
+  IN OUT  UINTN        *CommonNameSize
+  )
+{
+  ASSERT (FALSE);
+  return FALSE;
+}
+
 /**
   Retrieve the RSA Public Key from one DER-encoded X509 certificate.
 
@@ -203,4 +235,4 @@ X509GetTBSCert (
 {
   ASSERT (FALSE);
   return FALSE;
-}
\ No newline at end of file
+}
-- 
2.14.1.windows.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate
  2017-09-19  3:38 [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate Long Qin
@ 2017-09-20  6:57 ` Zhang, Chao B
  2017-09-20  8:25   ` Long, Qin
  2017-09-20 12:09 ` Laszlo Ersek
  1 sibling, 1 reply; 6+ messages in thread
From: Zhang, Chao B @ 2017-09-20  6:57 UTC (permalink / raw)
  To: Long, Qin, Ye, Ting; +Cc: edk2-devel@lists.01.org

Qin:
   For cryptest, do we need to support 64 maximum CN name and NULL? That makes buffer size 65 instead of 64.
    Others are good to me.
 


-----Original Message-----
From: Long, Qin 
Sent: Tuesday, September 19, 2017 11:39 AM
To: Ye, Ting <ting.ye@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>
Cc: edk2-devel@lists.01.org; Long, Qin <qin.long@intel.com>
Subject: [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate

Add one new API (X509GetCommonName()) to retrieve the subject commonName string from one X.509 certificate.

Cc: Ting Ye <ting.ye@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long <qin.long@intel.com>
---
 CryptoPkg/Application/Cryptest/RsaVerify2.c        | 17 ++++
 CryptoPkg/Include/Library/BaseCryptLib.h           | 32 ++++++++
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c      | 93 ++++++++++++++++++++++
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c  | 32 ++++++++
 .../Pk/CryptX509Null.c                             | 34 +++++++-
 5 files changed, 207 insertions(+), 1 deletion(-)

diff --git a/CryptoPkg/Application/Cryptest/RsaVerify2.c b/CryptoPkg/Application/Cryptest/RsaVerify2.c
index 98b5aad900..f9b70d5794 100644
--- a/CryptoPkg/Application/Cryptest/RsaVerify2.c
+++ b/CryptoPkg/Application/Cryptest/RsaVerify2.c
@@ -211,6 +211,9 @@ ValidateCryptRsa2 (
   UINTN    SigSize;
   UINT8    *Subject;
   UINTN    SubjectSize;
+  CHAR8    CommonName[64];
+  CHAR16   CommonNameUnicode[64];
+  UINTN    CommonNameSize;
 
   Print (L"\nUEFI-OpenSSL RSA Key Retrieving Testing: ");
 
@@ -286,6 +289,20 @@ ValidateCryptRsa2 (
     Print (L"[Pass]");
   }
 
+  //
+  // Get CommonName from X509 Certificate Subject  //  CommonNameSize = 
+ 64;  ZeroMem (CommonName, CommonNameSize);  Status = X509GetCommonName 
+ (TestCert, sizeof (TestCert), CommonName, &CommonNameSize);  if 
+ (!Status) {
+    Print (L"\n  - Retrieving Common Name - [Fail]");
+    return EFI_ABORTED;
+  } else {
+    AsciiStrToUnicodeStrS (CommonName, CommonNameUnicode, CommonNameSize);
+    Print (L"\n  - Retrieving Common Name = \"%s\" (Size = %d)", 
+ CommonNameUnicode, CommonNameSize);  }
+
   //
   // X509 Certificate Verification.
   //
diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h
index 9c5ffcd9cf..d861be6725 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -2171,6 +2171,38 @@ X509GetSubjectName (
   IN OUT  UINTN        *SubjectSize
   );
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  If Cert or CommonNameSize is NULL, then return FALSE.
+  If this interface is not supported, then return FALSE.
+
+  @param[in]      Cert            Pointer to the DER-encoded X509 certificate.
+  @param[in]      CertSize        Size of the X509 certificate in bytes.
+  @param[out]     CommonName      Buffer to contain the retrieved certificate common
+                                  name string. At most CommonNameSize bytes will be
+                                  written and the string will be null terminated. May be
+                                  NULL in order to determine the size buffer needed.
+  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer on input,
+                                  and the size of buffer returned CommonName on output.
+                                  if CommonName is NULL then the amount of space needed
+                                  in buffer (including the final null) is returned.
+
+  @retval  TRUE   The certificate CommonName retrieved successfully.
+  @retval  FALSE  Invalid certificate, or CommonNameSize is NULL,
+                  or no CommonName entry exists.
+  @retval  FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+X509GetCommonName (
+  IN      CONST UINT8  *Cert,
+  IN      UINTN        CertSize,
+  OUT     CHAR8        *CommonName,
+  IN OUT  UINTN        *CommonNameSize
+  );
+
 /**
   Verify one X509 certificate was issued by the trusted CA.
 
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
index 7d275977c5..e45c214bd1 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
@@ -297,6 +297,99 @@ _Exit:
   return Status;
 }
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  If Cert or CommonNameSize is NULL, then return FALSE.
+  If this interface is not supported, then return FALSE.
+
+  @param[in]      Cert            Pointer to the DER-encoded X509 certificate.
+  @param[in]      CertSize        Size of the X509 certificate in bytes.
+  @param[out]     CommonName      Buffer to contain the retrieved certificate common
+                                  name string. At most CommonNameSize bytes will be
+                                  written and the string will be null terminated. May be
+                                  NULL in order to determine the size buffer needed.
+  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer on input,
+                                  and the size of buffer returned CommonName on output.
+                                  if CommonName is NULL then the amount of space needed
+                                  in buffer (including the final null) is returned.
+
+  @retval  TRUE   The certificate CommonName retrieved successfully.
+  @retval  FALSE  Invalid certificate, or CommonNameSize is NULL,
+                  or no CommonName entry exists.
+  @retval  FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+X509GetCommonName (
+  IN      CONST UINT8  *Cert,
+  IN      UINTN        CertSize,
+  OUT     CHAR8        *CommonName,
+  IN OUT  UINTN        *CommonNameSize
+  )
+{
+  BOOLEAN    Status;
+  X509       *X509Cert;
+  X509_NAME  *X509Name;
+  INTN       Length;
+
+  //
+  // Check input parameters.
+  //
+  if ((Cert == NULL) || (CommonNameSize == NULL)) {
+    return FALSE;
+  }
+
+  X509Cert = NULL;
+
+  //
+  // Read DER-encoded X509 Certificate and Construct X509 object.
+  //
+  Status = X509ConstructCertificate (Cert, CertSize, (UINT8 **) 
+ &X509Cert);  if ((X509Cert == NULL) || (!Status)) {
+    //
+    // Invalid X.509 Certificate
+    //
+    goto _Exit;
+  }
+
+  Status = FALSE;
+
+  //
+  // Retrieve subject name from certificate object.
+  //
+  X509Name = X509_get_subject_name (X509Cert);  if (X509Name == NULL) {
+    goto _Exit;
+  }
+
+  //
+  // Retrieve the CommonName information from X.509 Subject  //  Length 
+ = (INTN) X509_NAME_get_text_by_NID (X509Name, NID_commonName, 
+ CommonName, (int)(*CommonNameSize));  if (Length < 0) {
+    //
+    // No CommonName entry exists in X509_NAME object
+    //
+    *CommonNameSize = 0;
+    goto _Exit;
+  }
+
+  *CommonNameSize = (UINTN)(Length + 1);  Status = TRUE;
+
+_Exit:
+  //
+  // Release Resources.
+  //
+  if (X509Cert != NULL) {
+    X509_free (X509Cert);
+  }
+
+  return Status;
+}
+
 /**
   Retrieve the RSA Public Key from one DER-encoded X509 certificate.
 
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
index 51aa0633a8..81587003f2 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
@@ -127,6 +127,38 @@ X509GetSubjectName (
   return FALSE;
 }
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  Return FALSE to indicate this interface is not supported.
+
+  @param[in]      Cert            Pointer to the DER-encoded X509 certificate.
+  @param[in]      CertSize        Size of the X509 certificate in bytes.
+  @param[out]     CommonName      Buffer to contain the retrieved certificate common
+                                  name string. At most CommonNameSize bytes will be
+                                  written and the string will be null terminated. May be
+                                  NULL in order to determine the size buffer needed.
+  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer on input,
+                                  and the size of buffer returned CommonName on output.
+                                  if CommonName is NULL then the amount of space needed
+                                  in buffer (including the final null) is returned.
+
+  @retval  FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+X509GetCommonName (
+  IN      CONST UINT8  *Cert,
+  IN      UINTN        CertSize,
+  OUT     CHAR8        *CommonName,
+  IN OUT  UINTN        *CommonNameSize
+  )
+{
+  ASSERT (FALSE);
+  return FALSE;
+}
+
 /**
   Retrieve the RSA Public Key from one DER-encoded X509 certificate.
 
diff --git a/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c b/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c
index f5d9aa1076..81587003f2 100644
--- a/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c
+++ b/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Nul
+++ l.c
@@ -127,6 +127,38 @@ X509GetSubjectName (
   return FALSE;
 }
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  Return FALSE to indicate this interface is not supported.
+
+  @param[in]      Cert            Pointer to the DER-encoded X509 certificate.
+  @param[in]      CertSize        Size of the X509 certificate in bytes.
+  @param[out]     CommonName      Buffer to contain the retrieved certificate common
+                                  name string. At most CommonNameSize bytes will be
+                                  written and the string will be null terminated. May be
+                                  NULL in order to determine the size buffer needed.
+  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer on input,
+                                  and the size of buffer returned CommonName on output.
+                                  if CommonName is NULL then the amount of space needed
+                                  in buffer (including the final null) is returned.
+
+  @retval  FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+X509GetCommonName (
+  IN      CONST UINT8  *Cert,
+  IN      UINTN        CertSize,
+  OUT     CHAR8        *CommonName,
+  IN OUT  UINTN        *CommonNameSize
+  )
+{
+  ASSERT (FALSE);
+  return FALSE;
+}
+
 /**
   Retrieve the RSA Public Key from one DER-encoded X509 certificate.
 
@@ -203,4 +235,4 @@ X509GetTBSCert (
 {
   ASSERT (FALSE);
   return FALSE;
-}
\ No newline at end of file
+}
--
2.14.1.windows.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate
  2017-09-20  6:57 ` Zhang, Chao B
@ 2017-09-20  8:25   ` Long, Qin
  2017-09-20  8:33     ` Zhang, Chao B
  0 siblings, 1 reply; 6+ messages in thread
From: Long, Qin @ 2017-09-20  8:25 UTC (permalink / raw)
  To: Zhang, Chao B, Ye, Ting; +Cc: edk2-devel@lists.01.org

Thanks, Chao.
Cryptest just simply use the hard-coded test vectors for API usage demonstration. So 64 is big enough for the given test X.509 data.


Best Regards & Thanks,
LONG, Qin

-----Original Message-----
From: Zhang, Chao B 
Sent: Wednesday, September 20, 2017 2:57 PM
To: Long, Qin <qin.long@intel.com>; Ye, Ting <ting.ye@intel.com>
Cc: edk2-devel@lists.01.org
Subject: RE: [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate

Qin:
   For cryptest, do we need to support 64 maximum CN name and NULL? That makes buffer size 65 instead of 64.
    Others are good to me.
 


-----Original Message-----
From: Long, Qin
Sent: Tuesday, September 19, 2017 11:39 AM
To: Ye, Ting <ting.ye@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>
Cc: edk2-devel@lists.01.org; Long, Qin <qin.long@intel.com>
Subject: [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate

Add one new API (X509GetCommonName()) to retrieve the subject commonName string from one X.509 certificate.

Cc: Ting Ye <ting.ye@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long <qin.long@intel.com>
---
 CryptoPkg/Application/Cryptest/RsaVerify2.c        | 17 ++++
 CryptoPkg/Include/Library/BaseCryptLib.h           | 32 ++++++++
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c      | 93 ++++++++++++++++++++++
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c  | 32 ++++++++
 .../Pk/CryptX509Null.c                             | 34 +++++++-
 5 files changed, 207 insertions(+), 1 deletion(-)

diff --git a/CryptoPkg/Application/Cryptest/RsaVerify2.c b/CryptoPkg/Application/Cryptest/RsaVerify2.c
index 98b5aad900..f9b70d5794 100644
--- a/CryptoPkg/Application/Cryptest/RsaVerify2.c
+++ b/CryptoPkg/Application/Cryptest/RsaVerify2.c
@@ -211,6 +211,9 @@ ValidateCryptRsa2 (
   UINTN    SigSize;
   UINT8    *Subject;
   UINTN    SubjectSize;
+  CHAR8    CommonName[64];
+  CHAR16   CommonNameUnicode[64];
+  UINTN    CommonNameSize;
 
   Print (L"\nUEFI-OpenSSL RSA Key Retrieving Testing: ");
 
@@ -286,6 +289,20 @@ ValidateCryptRsa2 (
     Print (L"[Pass]");
   }
 
+  //
+  // Get CommonName from X509 Certificate Subject  //  CommonNameSize = 
+ 64;  ZeroMem (CommonName, CommonNameSize);  Status = X509GetCommonName 
+ (TestCert, sizeof (TestCert), CommonName, &CommonNameSize);  if
+ (!Status) {
+    Print (L"\n  - Retrieving Common Name - [Fail]");
+    return EFI_ABORTED;
+  } else {
+    AsciiStrToUnicodeStrS (CommonName, CommonNameUnicode, CommonNameSize);
+    Print (L"\n  - Retrieving Common Name = \"%s\" (Size = %d)", 
+ CommonNameUnicode, CommonNameSize);  }
+
   //
   // X509 Certificate Verification.
   //
diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h
index 9c5ffcd9cf..d861be6725 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -2171,6 +2171,38 @@ X509GetSubjectName (
   IN OUT  UINTN        *SubjectSize
   );
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  If Cert or CommonNameSize is NULL, then return FALSE.
+  If this interface is not supported, then return FALSE.
+
+  @param[in]      Cert            Pointer to the DER-encoded X509 certificate.
+  @param[in]      CertSize        Size of the X509 certificate in bytes.
+  @param[out]     CommonName      Buffer to contain the retrieved certificate common
+                                  name string. At most CommonNameSize bytes will be
+                                  written and the string will be null terminated. May be
+                                  NULL in order to determine the size buffer needed.
+  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer on input,
+                                  and the size of buffer returned CommonName on output.
+                                  if CommonName is NULL then the amount of space needed
+                                  in buffer (including the final null) is returned.
+
+  @retval  TRUE   The certificate CommonName retrieved successfully.
+  @retval  FALSE  Invalid certificate, or CommonNameSize is NULL,
+                  or no CommonName entry exists.
+  @retval  FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+X509GetCommonName (
+  IN      CONST UINT8  *Cert,
+  IN      UINTN        CertSize,
+  OUT     CHAR8        *CommonName,
+  IN OUT  UINTN        *CommonNameSize
+  );
+
 /**
   Verify one X509 certificate was issued by the trusted CA.
 
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
index 7d275977c5..e45c214bd1 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
@@ -297,6 +297,99 @@ _Exit:
   return Status;
 }
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  If Cert or CommonNameSize is NULL, then return FALSE.
+  If this interface is not supported, then return FALSE.
+
+  @param[in]      Cert            Pointer to the DER-encoded X509 certificate.
+  @param[in]      CertSize        Size of the X509 certificate in bytes.
+  @param[out]     CommonName      Buffer to contain the retrieved certificate common
+                                  name string. At most CommonNameSize bytes will be
+                                  written and the string will be null terminated. May be
+                                  NULL in order to determine the size buffer needed.
+  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer on input,
+                                  and the size of buffer returned CommonName on output.
+                                  if CommonName is NULL then the amount of space needed
+                                  in buffer (including the final null) is returned.
+
+  @retval  TRUE   The certificate CommonName retrieved successfully.
+  @retval  FALSE  Invalid certificate, or CommonNameSize is NULL,
+                  or no CommonName entry exists.
+  @retval  FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+X509GetCommonName (
+  IN      CONST UINT8  *Cert,
+  IN      UINTN        CertSize,
+  OUT     CHAR8        *CommonName,
+  IN OUT  UINTN        *CommonNameSize
+  )
+{
+  BOOLEAN    Status;
+  X509       *X509Cert;
+  X509_NAME  *X509Name;
+  INTN       Length;
+
+  //
+  // Check input parameters.
+  //
+  if ((Cert == NULL) || (CommonNameSize == NULL)) {
+    return FALSE;
+  }
+
+  X509Cert = NULL;
+
+  //
+  // Read DER-encoded X509 Certificate and Construct X509 object.
+  //
+  Status = X509ConstructCertificate (Cert, CertSize, (UINT8 **) 
+ &X509Cert);  if ((X509Cert == NULL) || (!Status)) {
+    //
+    // Invalid X.509 Certificate
+    //
+    goto _Exit;
+  }
+
+  Status = FALSE;
+
+  //
+  // Retrieve subject name from certificate object.
+  //
+  X509Name = X509_get_subject_name (X509Cert);  if (X509Name == NULL) {
+    goto _Exit;
+  }
+
+  //
+  // Retrieve the CommonName information from X.509 Subject  //  Length 
+ = (INTN) X509_NAME_get_text_by_NID (X509Name, NID_commonName, 
+ CommonName, (int)(*CommonNameSize));  if (Length < 0) {
+    //
+    // No CommonName entry exists in X509_NAME object
+    //
+    *CommonNameSize = 0;
+    goto _Exit;
+  }
+
+  *CommonNameSize = (UINTN)(Length + 1);  Status = TRUE;
+
+_Exit:
+  //
+  // Release Resources.
+  //
+  if (X509Cert != NULL) {
+    X509_free (X509Cert);
+  }
+
+  return Status;
+}
+
 /**
   Retrieve the RSA Public Key from one DER-encoded X509 certificate.
 
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
index 51aa0633a8..81587003f2 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
@@ -127,6 +127,38 @@ X509GetSubjectName (
   return FALSE;
 }
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  Return FALSE to indicate this interface is not supported.
+
+  @param[in]      Cert            Pointer to the DER-encoded X509 certificate.
+  @param[in]      CertSize        Size of the X509 certificate in bytes.
+  @param[out]     CommonName      Buffer to contain the retrieved certificate common
+                                  name string. At most CommonNameSize bytes will be
+                                  written and the string will be null terminated. May be
+                                  NULL in order to determine the size buffer needed.
+  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer on input,
+                                  and the size of buffer returned CommonName on output.
+                                  if CommonName is NULL then the amount of space needed
+                                  in buffer (including the final null) is returned.
+
+  @retval  FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+X509GetCommonName (
+  IN      CONST UINT8  *Cert,
+  IN      UINTN        CertSize,
+  OUT     CHAR8        *CommonName,
+  IN OUT  UINTN        *CommonNameSize
+  )
+{
+  ASSERT (FALSE);
+  return FALSE;
+}
+
 /**
   Retrieve the RSA Public Key from one DER-encoded X509 certificate.
 
diff --git a/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c b/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c
index f5d9aa1076..81587003f2 100644
--- a/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c
+++ b/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Nul
+++ l.c
@@ -127,6 +127,38 @@ X509GetSubjectName (
   return FALSE;
 }
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  Return FALSE to indicate this interface is not supported.
+
+  @param[in]      Cert            Pointer to the DER-encoded X509 certificate.
+  @param[in]      CertSize        Size of the X509 certificate in bytes.
+  @param[out]     CommonName      Buffer to contain the retrieved certificate common
+                                  name string. At most CommonNameSize bytes will be
+                                  written and the string will be null terminated. May be
+                                  NULL in order to determine the size buffer needed.
+  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer on input,
+                                  and the size of buffer returned CommonName on output.
+                                  if CommonName is NULL then the amount of space needed
+                                  in buffer (including the final null) is returned.
+
+  @retval  FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+X509GetCommonName (
+  IN      CONST UINT8  *Cert,
+  IN      UINTN        CertSize,
+  OUT     CHAR8        *CommonName,
+  IN OUT  UINTN        *CommonNameSize
+  )
+{
+  ASSERT (FALSE);
+  return FALSE;
+}
+
 /**
   Retrieve the RSA Public Key from one DER-encoded X509 certificate.
 
@@ -203,4 +235,4 @@ X509GetTBSCert (
 {
   ASSERT (FALSE);
   return FALSE;
-}
\ No newline at end of file
+}
--
2.14.1.windows.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate
  2017-09-20  8:25   ` Long, Qin
@ 2017-09-20  8:33     ` Zhang, Chao B
  0 siblings, 0 replies; 6+ messages in thread
From: Zhang, Chao B @ 2017-09-20  8:33 UTC (permalink / raw)
  To: Long, Qin, Ye, Ting; +Cc: edk2-devel@lists.01.org

Qin:
   Thanks for your explanation. It makes sense to me.
    Reviewed-by: Chao Zhang <chao.b.zhang@intel.com> 

-----Original Message-----
From: Long, Qin 
Sent: Wednesday, September 20, 2017 4:25 PM
To: Zhang, Chao B <chao.b.zhang@intel.com>; Ye, Ting <ting.ye@intel.com>
Cc: edk2-devel@lists.01.org
Subject: RE: [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate

Thanks, Chao.
Cryptest just simply use the hard-coded test vectors for API usage demonstration. So 64 is big enough for the given test X.509 data.


Best Regards & Thanks,
LONG, Qin

-----Original Message-----
From: Zhang, Chao B
Sent: Wednesday, September 20, 2017 2:57 PM
To: Long, Qin <qin.long@intel.com>; Ye, Ting <ting.ye@intel.com>
Cc: edk2-devel@lists.01.org
Subject: RE: [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate

Qin:
   For cryptest, do we need to support 64 maximum CN name and NULL? That makes buffer size 65 instead of 64.
    Others are good to me.
 


-----Original Message-----
From: Long, Qin
Sent: Tuesday, September 19, 2017 11:39 AM
To: Ye, Ting <ting.ye@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>
Cc: edk2-devel@lists.01.org; Long, Qin <qin.long@intel.com>
Subject: [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate

Add one new API (X509GetCommonName()) to retrieve the subject commonName string from one X.509 certificate.

Cc: Ting Ye <ting.ye@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long <qin.long@intel.com>
---
 CryptoPkg/Application/Cryptest/RsaVerify2.c        | 17 ++++
 CryptoPkg/Include/Library/BaseCryptLib.h           | 32 ++++++++
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c      | 93 ++++++++++++++++++++++
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c  | 32 ++++++++
 .../Pk/CryptX509Null.c                             | 34 +++++++-
 5 files changed, 207 insertions(+), 1 deletion(-)

diff --git a/CryptoPkg/Application/Cryptest/RsaVerify2.c b/CryptoPkg/Application/Cryptest/RsaVerify2.c
index 98b5aad900..f9b70d5794 100644
--- a/CryptoPkg/Application/Cryptest/RsaVerify2.c
+++ b/CryptoPkg/Application/Cryptest/RsaVerify2.c
@@ -211,6 +211,9 @@ ValidateCryptRsa2 (
   UINTN    SigSize;
   UINT8    *Subject;
   UINTN    SubjectSize;
+  CHAR8    CommonName[64];
+  CHAR16   CommonNameUnicode[64];
+  UINTN    CommonNameSize;
 
   Print (L"\nUEFI-OpenSSL RSA Key Retrieving Testing: ");
 
@@ -286,6 +289,20 @@ ValidateCryptRsa2 (
     Print (L"[Pass]");
   }
 
+  //
+  // Get CommonName from X509 Certificate Subject  //  CommonNameSize = 
+ 64;  ZeroMem (CommonName, CommonNameSize);  Status = X509GetCommonName 
+ (TestCert, sizeof (TestCert), CommonName, &CommonNameSize);  if
+ (!Status) {
+    Print (L"\n  - Retrieving Common Name - [Fail]");
+    return EFI_ABORTED;
+  } else {
+    AsciiStrToUnicodeStrS (CommonName, CommonNameUnicode, CommonNameSize);
+    Print (L"\n  - Retrieving Common Name = \"%s\" (Size = %d)", 
+ CommonNameUnicode, CommonNameSize);  }
+
   //
   // X509 Certificate Verification.
   //
diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h
index 9c5ffcd9cf..d861be6725 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -2171,6 +2171,38 @@ X509GetSubjectName (
   IN OUT  UINTN        *SubjectSize
   );
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  If Cert or CommonNameSize is NULL, then return FALSE.
+  If this interface is not supported, then return FALSE.
+
+  @param[in]      Cert            Pointer to the DER-encoded X509 certificate.
+  @param[in]      CertSize        Size of the X509 certificate in bytes.
+  @param[out]     CommonName      Buffer to contain the retrieved certificate common
+                                  name string. At most CommonNameSize bytes will be
+                                  written and the string will be null terminated. May be
+                                  NULL in order to determine the size buffer needed.
+  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer on input,
+                                  and the size of buffer returned CommonName on output.
+                                  if CommonName is NULL then the amount of space needed
+                                  in buffer (including the final null) is returned.
+
+  @retval  TRUE   The certificate CommonName retrieved successfully.
+  @retval  FALSE  Invalid certificate, or CommonNameSize is NULL,
+                  or no CommonName entry exists.
+  @retval  FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+X509GetCommonName (
+  IN      CONST UINT8  *Cert,
+  IN      UINTN        CertSize,
+  OUT     CHAR8        *CommonName,
+  IN OUT  UINTN        *CommonNameSize
+  );
+
 /**
   Verify one X509 certificate was issued by the trusted CA.
 
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
index 7d275977c5..e45c214bd1 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
@@ -297,6 +297,99 @@ _Exit:
   return Status;
 }
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  If Cert or CommonNameSize is NULL, then return FALSE.
+  If this interface is not supported, then return FALSE.
+
+  @param[in]      Cert            Pointer to the DER-encoded X509 certificate.
+  @param[in]      CertSize        Size of the X509 certificate in bytes.
+  @param[out]     CommonName      Buffer to contain the retrieved certificate common
+                                  name string. At most CommonNameSize bytes will be
+                                  written and the string will be null terminated. May be
+                                  NULL in order to determine the size buffer needed.
+  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer on input,
+                                  and the size of buffer returned CommonName on output.
+                                  if CommonName is NULL then the amount of space needed
+                                  in buffer (including the final null) is returned.
+
+  @retval  TRUE   The certificate CommonName retrieved successfully.
+  @retval  FALSE  Invalid certificate, or CommonNameSize is NULL,
+                  or no CommonName entry exists.
+  @retval  FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+X509GetCommonName (
+  IN      CONST UINT8  *Cert,
+  IN      UINTN        CertSize,
+  OUT     CHAR8        *CommonName,
+  IN OUT  UINTN        *CommonNameSize
+  )
+{
+  BOOLEAN    Status;
+  X509       *X509Cert;
+  X509_NAME  *X509Name;
+  INTN       Length;
+
+  //
+  // Check input parameters.
+  //
+  if ((Cert == NULL) || (CommonNameSize == NULL)) {
+    return FALSE;
+  }
+
+  X509Cert = NULL;
+
+  //
+  // Read DER-encoded X509 Certificate and Construct X509 object.
+  //
+  Status = X509ConstructCertificate (Cert, CertSize, (UINT8 **) 
+ &X509Cert);  if ((X509Cert == NULL) || (!Status)) {
+    //
+    // Invalid X.509 Certificate
+    //
+    goto _Exit;
+  }
+
+  Status = FALSE;
+
+  //
+  // Retrieve subject name from certificate object.
+  //
+  X509Name = X509_get_subject_name (X509Cert);  if (X509Name == NULL) {
+    goto _Exit;
+  }
+
+  //
+  // Retrieve the CommonName information from X.509 Subject  //  Length 
+ = (INTN) X509_NAME_get_text_by_NID (X509Name, NID_commonName, 
+ CommonName, (int)(*CommonNameSize));  if (Length < 0) {
+    //
+    // No CommonName entry exists in X509_NAME object
+    //
+    *CommonNameSize = 0;
+    goto _Exit;
+  }
+
+  *CommonNameSize = (UINTN)(Length + 1);  Status = TRUE;
+
+_Exit:
+  //
+  // Release Resources.
+  //
+  if (X509Cert != NULL) {
+    X509_free (X509Cert);
+  }
+
+  return Status;
+}
+
 /**
   Retrieve the RSA Public Key from one DER-encoded X509 certificate.
 
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
index 51aa0633a8..81587003f2 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
@@ -127,6 +127,38 @@ X509GetSubjectName (
   return FALSE;
 }
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  Return FALSE to indicate this interface is not supported.
+
+  @param[in]      Cert            Pointer to the DER-encoded X509 certificate.
+  @param[in]      CertSize        Size of the X509 certificate in bytes.
+  @param[out]     CommonName      Buffer to contain the retrieved certificate common
+                                  name string. At most CommonNameSize bytes will be
+                                  written and the string will be null terminated. May be
+                                  NULL in order to determine the size buffer needed.
+  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer on input,
+                                  and the size of buffer returned CommonName on output.
+                                  if CommonName is NULL then the amount of space needed
+                                  in buffer (including the final null) is returned.
+
+  @retval  FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+X509GetCommonName (
+  IN      CONST UINT8  *Cert,
+  IN      UINTN        CertSize,
+  OUT     CHAR8        *CommonName,
+  IN OUT  UINTN        *CommonNameSize
+  )
+{
+  ASSERT (FALSE);
+  return FALSE;
+}
+
 /**
   Retrieve the RSA Public Key from one DER-encoded X509 certificate.
 
diff --git a/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c b/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c
index f5d9aa1076..81587003f2 100644
--- a/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Null.c
+++ b/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptX509Nul
+++ l.c
@@ -127,6 +127,38 @@ X509GetSubjectName (
   return FALSE;
 }
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  Return FALSE to indicate this interface is not supported.
+
+  @param[in]      Cert            Pointer to the DER-encoded X509 certificate.
+  @param[in]      CertSize        Size of the X509 certificate in bytes.
+  @param[out]     CommonName      Buffer to contain the retrieved certificate common
+                                  name string. At most CommonNameSize bytes will be
+                                  written and the string will be null terminated. May be
+                                  NULL in order to determine the size buffer needed.
+  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer on input,
+                                  and the size of buffer returned CommonName on output.
+                                  if CommonName is NULL then the amount of space needed
+                                  in buffer (including the final null) is returned.
+
+  @retval  FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+X509GetCommonName (
+  IN      CONST UINT8  *Cert,
+  IN      UINTN        CertSize,
+  OUT     CHAR8        *CommonName,
+  IN OUT  UINTN        *CommonNameSize
+  )
+{
+  ASSERT (FALSE);
+  return FALSE;
+}
+
 /**
   Retrieve the RSA Public Key from one DER-encoded X509 certificate.
 
@@ -203,4 +235,4 @@ X509GetTBSCert (
 {
   ASSERT (FALSE);
   return FALSE;
-}
\ No newline at end of file
+}
--
2.14.1.windows.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate
  2017-09-19  3:38 [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate Long Qin
  2017-09-20  6:57 ` Zhang, Chao B
@ 2017-09-20 12:09 ` Laszlo Ersek
  2017-09-20 12:45   ` Long, Qin
  1 sibling, 1 reply; 6+ messages in thread
From: Laszlo Ersek @ 2017-09-20 12:09 UTC (permalink / raw)
  To: Long Qin, ting.ye, chao.b.zhang; +Cc: edk2-devel

Hello Qin,

On 09/19/17 05:38, Long Qin wrote:
> Add one new API (X509GetCommonName()) to retrieve the subject commonName
> string from one X.509 certificate.
> 
> Cc: Ting Ye <ting.ye@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Qin Long <qin.long@intel.com>
> ---
>  CryptoPkg/Application/Cryptest/RsaVerify2.c        | 17 ++++
>  CryptoPkg/Include/Library/BaseCryptLib.h           | 32 ++++++++
>  CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c      | 93 ++++++++++++++++++++++
>  CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c  | 32 ++++++++
>  .../Pk/CryptX509Null.c                             | 34 +++++++-
>  5 files changed, 207 insertions(+), 1 deletion(-)

> diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h
> index 9c5ffcd9cf..d861be6725 100644
> --- a/CryptoPkg/Include/Library/BaseCryptLib.h
> +++ b/CryptoPkg/Include/Library/BaseCryptLib.h
> @@ -2171,6 +2171,38 @@ X509GetSubjectName (
>    IN OUT  UINTN        *SubjectSize
>    );
>  
> +/**
> +  Retrieve the common name (CN) string from one X.509 certificate.
> +
> +  If Cert or CommonNameSize is NULL, then return FALSE.
> +  If this interface is not supported, then return FALSE.
> +
> +  @param[in]      Cert            Pointer to the DER-encoded X509 certificate.
> +  @param[in]      CertSize        Size of the X509 certificate in bytes.
> +  @param[out]     CommonName      Buffer to contain the retrieved certificate common
> +                                  name string. At most CommonNameSize bytes will be
> +                                  written and the string will be null terminated. May be
> +                                  NULL in order to determine the size buffer needed.
> +  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer on input,
> +                                  and the size of buffer returned CommonName on output.
> +                                  if CommonName is NULL then the amount of space needed
> +                                  in buffer (including the final null) is returned.
> +
> +  @retval  TRUE   The certificate CommonName retrieved successfully.
> +  @retval  FALSE  Invalid certificate, or CommonNameSize is NULL,
> +                  or no CommonName entry exists.
> +  @retval  FALSE  This interface is not supported.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +X509GetCommonName (
> +  IN      CONST UINT8  *Cert,
> +  IN      UINTN        CertSize,
> +  OUT     CHAR8        *CommonName,
> +  IN OUT  UINTN        *CommonNameSize
> +  );
> +
>  /**
>    Verify one X509 certificate was issued by the trusted CA.
>  

I hope my questions / suggestions aren't unwelcome (or misguided) --
have you considered returning RETURN_STATUS from this function?

Currently FALSE is returned for several error cases, but we have good
RETURN_xxx macros for telling them apart:

- RETURN_BUFFER_TOO_SMALL: "The buffer was not large enough to hold the
requested data. The required buffer size is returned in the appropriate
parameter when this error occurs."

- RETURN_UNSUPPORTED: "The operation is not supported."

- RETURN_NOT_FOUND: "The item was not found." -- this can be used for
"no CommonName entry exists".

- RETURN_INVALID_PARAMETER: "The parameter was incorrect." -- this can
be used for "CommonNameSize is NULL", and likely for "Invalid
certificate" as well.

If you don't want to update the interface, I'm OK with that of course; I
just figured I'd raise the question.

Thanks!
Laszlo


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate
  2017-09-20 12:09 ` Laszlo Ersek
@ 2017-09-20 12:45   ` Long, Qin
  0 siblings, 0 replies; 6+ messages in thread
From: Long, Qin @ 2017-09-20 12:45 UTC (permalink / raw)
  To: Laszlo Ersek, Ye, Ting, Zhang,  Chao B; +Cc: edk2-devel@lists.01.org

Laszlo.

It's one good feedback.

This is one historical design issue. We choose to use simple BOOLEAN as the return value, because OpenSSL has complicated return data (reason) with extra api (e.g. ERR_get_error()...). It's hard to map these error messages directly, then we just used one simplest way before, and always kept this kind of API style in late updates.

I also think the return value (true/false) in current BaseCryptLib is really ambiguous to tell any more useful information. RETURN_xxx is more valuable in this new-added case. I would like to update the patch per your suggestion.

Thanks for raising this.

Best Regards & Thanks,
LONG, Qin

From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
Sent: Wednesday, September 20, 2017 8:09 PM
To: Long, Qin <qin.long@intel.com>; Ye, Ting <ting.ye@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate

Hello Qin,

On 09/19/17 05:38, Long Qin wrote:
> Add one new API (X509GetCommonName()) to retrieve the subject commonName
> string from one X.509 certificate.
>
> Cc: Ting Ye <ting.ye@intel.com<mailto:ting.ye@intel.com>>
> Cc: Chao Zhang <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Qin Long <qin.long@intel.com<mailto:qin.long@intel.com>>
> ---
>  CryptoPkg/Application/Cryptest/RsaVerify2.c        | 17 ++++
>  CryptoPkg/Include/Library/BaseCryptLib.h           | 32 ++++++++
>  CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c      | 93 ++++++++++++++++++++++
>  CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c  | 32 ++++++++
>  .../Pk/CryptX509Null.c                             | 34 +++++++-
>  5 files changed, 207 insertions(+), 1 deletion(-)

> diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h
> index 9c5ffcd9cf..d861be6725 100644
> --- a/CryptoPkg/Include/Library/BaseCryptLib.h
> +++ b/CryptoPkg/Include/Library/BaseCryptLib.h
> @@ -2171,6 +2171,38 @@ X509GetSubjectName (
>    IN OUT  UINTN        *SubjectSize
>    );
>
> +/**
> +  Retrieve the common name (CN) string from one X.509 certificate.
> +
> +  If Cert or CommonNameSize is NULL, then return FALSE.
> +  If this interface is not supported, then return FALSE.
> +
> +  @param[in]      Cert            Pointer to the DER-encoded X509 certificate.
> +  @param[in]      CertSize        Size of the X509 certificate in bytes.
> +  @param[out]     CommonName      Buffer to contain the retrieved certificate common
> +                                  name string. At most CommonNameSize bytes will be
> +                                  written and the string will be null terminated. May be
> +                                  NULL in order to determine the size buffer needed.
> +  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer on input,
> +                                  and the size of buffer returned CommonName on output.
> +                                  if CommonName is NULL then the amount of space needed
> +                                  in buffer (including the final null) is returned.
> +
> +  @retval  TRUE   The certificate CommonName retrieved successfully.
> +  @retval  FALSE  Invalid certificate, or CommonNameSize is NULL,
> +                  or no CommonName entry exists.
> +  @retval  FALSE  This interface is not supported.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +X509GetCommonName (
> +  IN      CONST UINT8  *Cert,
> +  IN      UINTN        CertSize,
> +  OUT     CHAR8        *CommonName,
> +  IN OUT  UINTN        *CommonNameSize
> +  );
> +
>  /**
>    Verify one X509 certificate was issued by the trusted CA.
>

I hope my questions / suggestions aren't unwelcome (or misguided) --
have you considered returning RETURN_STATUS from this function?

Currently FALSE is returned for several error cases, but we have good
RETURN_xxx macros for telling them apart:

- RETURN_BUFFER_TOO_SMALL: "The buffer was not large enough to hold the
requested data. The required buffer size is returned in the appropriate
parameter when this error occurs."

- RETURN_UNSUPPORTED: "The operation is not supported."

- RETURN_NOT_FOUND: "The item was not found." -- this can be used for
"no CommonName entry exists".

- RETURN_INVALID_PARAMETER: "The parameter was incorrect." -- this can
be used for "CommonNameSize is NULL", and likely for "Invalid
certificate" as well.

If you don't want to update the interface, I'm OK with that of course; I
just figured I'd raise the question.

Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-09-20 12:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-19  3:38 [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate Long Qin
2017-09-20  6:57 ` Zhang, Chao B
2017-09-20  8:25   ` Long, Qin
2017-09-20  8:33     ` Zhang, Chao B
2017-09-20 12:09 ` Laszlo Ersek
2017-09-20 12:45   ` Long, Qin

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