Re: [edk2] [PATCH v3] CryptoPkg: Add new API to retrieve commonName of X.509 certificate

2017-09-28 Thread Long, Qin 
Yes, they are legacy version with old style alignment.
It's first try to address this return status change in this new API.  We may 
update some APIs depending on requirement and impacts evaluations later. 


Best Regards & Thanks,
LONG, Qin

-Original Message-
From: Zhang, Chao B 
Sent: Thursday, September 28, 2017 2:03 PM
To: Long, Qin ; ler...@redhat.com; Ye, Ting 

Cc: edk2-devel@lists.01.org
Subject: RE: [PATCH v3] CryptoPkg: Add new API to retrieve commonName of X.509 
certificate

Qin:  
   What about other X509 related interface, such as X509GetTBSCert, 
X509GetSubjectName. They all return TRUE/FALSE.
   It looks inconsistent between these interfaces


-Original Message-
From: Long, Qin
Sent: Thursday, September 21, 2017 10:48 AM
To: ler...@redhat.com; Ye, Ting ; Zhang, Chao B 

Cc: edk2-devel@lists.01.org
Subject: [PATCH v3] CryptoPkg: Add new API to retrieve commonName of X.509 
certificate

v3: Add extra CommonNameSize check since OpenSSL didn't check this
input parameter. (One openssl issue was filed to address this risk:
https://github.com/openssl/openssl/issues/4392)
v2: Update function interface to return RETURN_STATUS to represent
different error cases.

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

Cc: Laszlo Ersek 
Cc: Ting Ye 
Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long 
---
 CryptoPkg/Application/Cryptest/RsaVerify2.c|  32 --
 CryptoPkg/Include/Library/BaseCryptLib.h   |  35 +++
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c  | 109 +
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c  |  32 ++
 .../Pk/CryptX509Null.c |  34 ++-
 5 files changed, 234 insertions(+), 8 deletions(-)

diff --git a/CryptoPkg/Application/Cryptest/RsaVerify2.c 
b/CryptoPkg/Application/Cryptest/RsaVerify2.c
index 98b5aad900..9db43d6eef 100644
--- a/CryptoPkg/Application/Cryptest/RsaVerify2.c
+++ b/CryptoPkg/Application/Cryptest/RsaVerify2.c
@@ -204,13 +204,17 @@ ValidateCryptRsa2 (
   VOID
   )
 {
-  BOOLEAN  Status;
-  VOID *RsaPrivKey;
-  VOID *RsaPubKey;
-  UINT8*Signature;
-  UINTNSigSize;
-  UINT8*Subject;
-  UINTNSubjectSize;
+  BOOLEANStatus;
+  VOID   *RsaPrivKey;
+  VOID   *RsaPubKey;
+  UINT8  *Signature;
+  UINTN  SigSize;
+  UINT8  *Subject;
+  UINTN  SubjectSize;
+  RETURN_STATUS  ReturnStatus;
+  CHAR8  CommonName[64];
+  CHAR16 CommonNameUnicode[64];
+  UINTN  CommonNameSize;
 
   Print (L"\nUEFI-OpenSSL RSA Key Retrieving Testing: ");
 
@@ -286,6 +290,20 @@ ValidateCryptRsa2 (
 Print (L"[Pass]");
   }
 
+  //
+  // Get CommonName from X509 Certificate Subject  //  CommonNameSize = 
+ 64;  ZeroMem (CommonName, CommonNameSize);  ReturnStatus = 
+ X509GetCommonName (TestCert, sizeof (TestCert), CommonName, 
+ );  if (RETURN_ERROR (ReturnStatus)) {
+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..2366a0218d 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -2171,6 +2171,41 @@ X509GetSubjectName (
   IN OUT  UINTN*SubjectSize
   );
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  @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 RETURN_SUCCESS   The certificate CommonName retrieved 
successfully.
+  @retval RETURN_INVALID_PARAMETER If Cert is NULL.
+   If CommonNameSize is NULL.
+

Re: [edk2] [PATCH v3] CryptoPkg: Add new API to retrieve commonName of X.509 certificate

2017-09-28 Thread Zhang, Chao B 
Qin:  
   What about other X509 related interface, such as X509GetTBSCert, 
X509GetSubjectName. They all return TRUE/FALSE.
   It looks inconsistent between these interfaces


-Original Message-
From: Long, Qin 
Sent: Thursday, September 21, 2017 10:48 AM
To: ler...@redhat.com; Ye, Ting ; Zhang, Chao B 

Cc: edk2-devel@lists.01.org
Subject: [PATCH v3] CryptoPkg: Add new API to retrieve commonName of X.509 
certificate

v3: Add extra CommonNameSize check since OpenSSL didn't check this
input parameter. (One openssl issue was filed to address this risk:
https://github.com/openssl/openssl/issues/4392)
v2: Update function interface to return RETURN_STATUS to represent
different error cases.

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

Cc: Laszlo Ersek 
Cc: Ting Ye 
Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long 
---
 CryptoPkg/Application/Cryptest/RsaVerify2.c|  32 --
 CryptoPkg/Include/Library/BaseCryptLib.h   |  35 +++
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c  | 109 +
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c  |  32 ++
 .../Pk/CryptX509Null.c |  34 ++-
 5 files changed, 234 insertions(+), 8 deletions(-)

diff --git a/CryptoPkg/Application/Cryptest/RsaVerify2.c 
b/CryptoPkg/Application/Cryptest/RsaVerify2.c
index 98b5aad900..9db43d6eef 100644
--- a/CryptoPkg/Application/Cryptest/RsaVerify2.c
+++ b/CryptoPkg/Application/Cryptest/RsaVerify2.c
@@ -204,13 +204,17 @@ ValidateCryptRsa2 (
   VOID
   )
 {
-  BOOLEAN  Status;
-  VOID *RsaPrivKey;
-  VOID *RsaPubKey;
-  UINT8*Signature;
-  UINTNSigSize;
-  UINT8*Subject;
-  UINTNSubjectSize;
+  BOOLEANStatus;
+  VOID   *RsaPrivKey;
+  VOID   *RsaPubKey;
+  UINT8  *Signature;
+  UINTN  SigSize;
+  UINT8  *Subject;
+  UINTN  SubjectSize;
+  RETURN_STATUS  ReturnStatus;
+  CHAR8  CommonName[64];
+  CHAR16 CommonNameUnicode[64];
+  UINTN  CommonNameSize;
 
   Print (L"\nUEFI-OpenSSL RSA Key Retrieving Testing: ");
 
@@ -286,6 +290,20 @@ ValidateCryptRsa2 (
 Print (L"[Pass]");
   }
 
+  //
+  // Get CommonName from X509 Certificate Subject  //  CommonNameSize = 
+ 64;  ZeroMem (CommonName, CommonNameSize);  ReturnStatus = 
+ X509GetCommonName (TestCert, sizeof (TestCert), CommonName, 
+ );  if (RETURN_ERROR (ReturnStatus)) {
+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..2366a0218d 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -2171,6 +2171,41 @@ X509GetSubjectName (
   IN OUT  UINTN*SubjectSize
   );
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  @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 RETURN_SUCCESS   The certificate CommonName retrieved 
successfully.
+  @retval RETURN_INVALID_PARAMETER If Cert is NULL.
+   If CommonNameSize is NULL.
+   If CommonName is not NULL and 
CommonNameSize is 0.
+   If Certificate is invalid.
+  @retval RETURN_NOT_FOUND If no CommonName entry exists.
+  @retval RETURN_BUFFER_TOO_SMALL  If the CommonName is NULL. The required 
buffer size
+   (including the final null) is returned in 
the 
+   CommonNameSize parameter.
+  @retval RETURN_UNSUPPORTED   The operation is not supported.
+
+**/

Re: [edk2] [PATCH v3] CryptoPkg: Add new API to retrieve commonName of X.509 certificate

2017-09-22 Thread Laszlo Ersek 
On 09/21/17 05:50, Ye, Ting wrote:
> Hi Qin,
> 
> I think we might add OPTIONAL attribute to CommonName, as NULL is an valid 
> input for this API.
> In function description, I think we need update below statement to "if *** 
> and *CommonNameSize is 0."
> 
>  "If CommonName is not NULL and CommonNameSize is 0."
> 
> Others are good to me.
> 
> Reviewed-by: Ye Ting 

With those updates, the X509GetCommonName() interface looks good to me
as well.

I didn't check the implementation.

Acked-by: Laszlo Ersek 

Thanks!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3] CryptoPkg: Add new API to retrieve commonName of X.509 certificate

2017-09-20 Thread Ye, Ting 
Hi Qin,

I think we might add OPTIONAL attribute to CommonName, as NULL is an valid 
input for this API.
In function description, I think we need update below statement to "if *** and 
*CommonNameSize is 0."

 "If CommonName is not NULL and CommonNameSize is 0."

Others are good to me.

Reviewed-by: Ye Ting 

Best Regards,
Ting

-Original Message-
From: Long, Qin 
Sent: Thursday, September 21, 2017 10:48 AM
To: ler...@redhat.com; Ye, Ting ; Zhang, Chao B 

Cc: edk2-devel@lists.01.org
Subject: [PATCH v3] CryptoPkg: Add new API to retrieve commonName of X.509 
certificate

v3: Add extra CommonNameSize check since OpenSSL didn't check this
input parameter. (One openssl issue was filed to address this risk:
https://github.com/openssl/openssl/issues/4392)
v2: Update function interface to return RETURN_STATUS to represent
different error cases.

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

Cc: Laszlo Ersek 
Cc: Ting Ye 
Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long 
---
 CryptoPkg/Application/Cryptest/RsaVerify2.c|  32 --
 CryptoPkg/Include/Library/BaseCryptLib.h   |  35 +++
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c  | 109 +
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c  |  32 ++
 .../Pk/CryptX509Null.c |  34 ++-
 5 files changed, 234 insertions(+), 8 deletions(-)

diff --git a/CryptoPkg/Application/Cryptest/RsaVerify2.c 
b/CryptoPkg/Application/Cryptest/RsaVerify2.c
index 98b5aad900..9db43d6eef 100644
--- a/CryptoPkg/Application/Cryptest/RsaVerify2.c
+++ b/CryptoPkg/Application/Cryptest/RsaVerify2.c
@@ -204,13 +204,17 @@ ValidateCryptRsa2 (
   VOID
   )
 {
-  BOOLEAN  Status;
-  VOID *RsaPrivKey;
-  VOID *RsaPubKey;
-  UINT8*Signature;
-  UINTNSigSize;
-  UINT8*Subject;
-  UINTNSubjectSize;
+  BOOLEANStatus;
+  VOID   *RsaPrivKey;
+  VOID   *RsaPubKey;
+  UINT8  *Signature;
+  UINTN  SigSize;
+  UINT8  *Subject;
+  UINTN  SubjectSize;
+  RETURN_STATUS  ReturnStatus;
+  CHAR8  CommonName[64];
+  CHAR16 CommonNameUnicode[64];
+  UINTN  CommonNameSize;
 
   Print (L"\nUEFI-OpenSSL RSA Key Retrieving Testing: ");
 
@@ -286,6 +290,20 @@ ValidateCryptRsa2 (
 Print (L"[Pass]");
   }
 
+  //
+  // Get CommonName from X509 Certificate Subject  //  CommonNameSize = 
+ 64;  ZeroMem (CommonName, CommonNameSize);  ReturnStatus = 
+ X509GetCommonName (TestCert, sizeof (TestCert), CommonName, 
+ );  if (RETURN_ERROR (ReturnStatus)) {
+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..2366a0218d 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -2171,6 +2171,41 @@ X509GetSubjectName (
   IN OUT  UINTN*SubjectSize
   );
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  @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 RETURN_SUCCESS   The certificate CommonName retrieved 
successfully.
+  @retval RETURN_INVALID_PARAMETER If Cert is NULL.
+   If CommonNameSize is NULL.
+   If CommonName is not NULL and 
CommonNameSize is 0.
+   If Certificate is invalid.
+  @retval RETURN_NOT_FOUND If no CommonName entry exists.
+  @retval RETURN_BUFFER_TOO_SMALL  If the CommonName is NULL. The required 
buffer size
+   (including the final

[edk2] [PATCH v3] CryptoPkg: Add new API to retrieve commonName of X.509 certificate

2017-09-20 Thread Qin Long 
v3: Add extra CommonNameSize check since OpenSSL didn't check this
input parameter. (One openssl issue was filed to address this risk:
https://github.com/openssl/openssl/issues/4392)
v2: Update function interface to return RETURN_STATUS to represent
different error cases.

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

Cc: Laszlo Ersek 
Cc: Ting Ye 
Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long 
---
 CryptoPkg/Application/Cryptest/RsaVerify2.c|  32 --
 CryptoPkg/Include/Library/BaseCryptLib.h   |  35 +++
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c  | 109 +
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c  |  32 ++
 .../Pk/CryptX509Null.c |  34 ++-
 5 files changed, 234 insertions(+), 8 deletions(-)

diff --git a/CryptoPkg/Application/Cryptest/RsaVerify2.c 
b/CryptoPkg/Application/Cryptest/RsaVerify2.c
index 98b5aad900..9db43d6eef 100644
--- a/CryptoPkg/Application/Cryptest/RsaVerify2.c
+++ b/CryptoPkg/Application/Cryptest/RsaVerify2.c
@@ -204,13 +204,17 @@ ValidateCryptRsa2 (
   VOID
   )
 {
-  BOOLEAN  Status;
-  VOID *RsaPrivKey;
-  VOID *RsaPubKey;
-  UINT8*Signature;
-  UINTNSigSize;
-  UINT8*Subject;
-  UINTNSubjectSize;
+  BOOLEANStatus;
+  VOID   *RsaPrivKey;
+  VOID   *RsaPubKey;
+  UINT8  *Signature;
+  UINTN  SigSize;
+  UINT8  *Subject;
+  UINTN  SubjectSize;
+  RETURN_STATUS  ReturnStatus;
+  CHAR8  CommonName[64];
+  CHAR16 CommonNameUnicode[64];
+  UINTN  CommonNameSize;
 
   Print (L"\nUEFI-OpenSSL RSA Key Retrieving Testing: ");
 
@@ -286,6 +290,20 @@ ValidateCryptRsa2 (
 Print (L"[Pass]");
   }
 
+  //
+  // Get CommonName from X509 Certificate Subject
+  //
+  CommonNameSize = 64;
+  ZeroMem (CommonName, CommonNameSize);
+  ReturnStatus = X509GetCommonName (TestCert, sizeof (TestCert), CommonName, 
);
+  if (RETURN_ERROR (ReturnStatus)) {
+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..2366a0218d 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -2171,6 +2171,41 @@ X509GetSubjectName (
   IN OUT  UINTN*SubjectSize
   );
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  @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 RETURN_SUCCESS   The certificate CommonName retrieved 
successfully.
+  @retval RETURN_INVALID_PARAMETER If Cert is NULL.
+   If CommonNameSize is NULL.
+   If CommonName is not NULL and 
CommonNameSize is 0.
+   If Certificate is invalid.
+  @retval RETURN_NOT_FOUND If no CommonName entry exists.
+  @retval RETURN_BUFFER_TOO_SMALL  If the CommonName is NULL. The required 
buffer size
+   (including the final null) is returned in 
the 
+   CommonNameSize parameter.
+  @retval RETURN_UNSUPPORTED   The operation is not supported.
+
+**/
+RETURN_STATUS
+EFIAPI
+X509GetCommonName (
+  IN  CONST UINT8  *Cert,
+  IN  UINTNCertSize,
+  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..bcd5ff49fd 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++