* [PATCH v2] CryptoPkg: Add new API to retrieve commonName of X.509 certificate
@ 2017-09-20 16:05 Qin Long
2017-09-20 16:38 ` Laszlo Ersek
0 siblings, 1 reply; 4+ messages in thread
From: Qin Long @ 2017-09-20 16:05 UTC (permalink / raw)
To: lersek, ting.ye, chao.b.zhang; +Cc: edk2-devel
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 <lersek@redhat.com>
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 | 32 +++++--
CryptoPkg/Include/Library/BaseCryptLib.h | 34 +++++++
CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 106 +++++++++++++++++++++
CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c | 32 +++++++
.../Pk/CryptX509Null.c | 34 ++++++-
5 files changed, 230 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;
- UINTN SigSize;
- UINT8 *Subject;
- UINTN SubjectSize;
+ BOOLEAN Status;
+ 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, &CommonNameSize);
+ 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..48e9531758 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -2171,6 +2171,40 @@ 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 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 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..c3cf97b262 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
@@ -297,6 +297,112 @@ _Exit:
return Status;
}
+/**
+ 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 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 UINTN CertSize,
+ OUT CHAR8 *CommonName,
+ IN OUT UINTN *CommonNameSize
+ )
+{
+ RETURN_STATUS ReturnStatus;
+ BOOLEAN Status;
+ X509 *X509Cert;
+ X509_NAME *X509Name;
+ INTN Length;
+
+ ReturnStatus = RETURN_INVALID_PARAMETER;
+
+ //
+ // Check input parameters.
+ //
+ if ((Cert == NULL) || (CommonNameSize == NULL)) {
+ return ReturnStatus;
+ }
+
+ 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) {
+ //
+ // Fail to retrieve subject name content
+ //
+ ReturnStatus = RETURN_INVALID_PARAMETER;
+ 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;
+ ReturnStatus = RETURN_NOT_FOUND;
+ goto _Exit;
+ }
+
+ *CommonNameSize = (UINTN)(Length + 1);
+ if (CommonName == NULL) {
+ ReturnStatus = RETURN_BUFFER_TOO_SMALL;
+ } else {
+ ReturnStatus = RETURN_SUCCESS;
+ }
+
+_Exit:
+ //
+ // Release Resources.
+ //
+ if (X509Cert != NULL) {
+ X509_free (X509Cert);
+ }
+
+ return ReturnStatus;
+}
+
/**
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..25879d3578 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 RETURN_UNSUPPORTED 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 RETURN_UNSUPPORTED The operation is not supported.
+
+**/
+RETURN_STATUS
+EFIAPI
+X509GetCommonName (
+ IN CONST UINT8 *Cert,
+ IN UINTN CertSize,
+ OUT CHAR8 *CommonName,
+ IN OUT UINTN *CommonNameSize
+ )
+{
+ ASSERT (FALSE);
+ return RETURN_UNSUPPORTED;
+}
+
/**
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..25879d3578 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 RETURN_UNSUPPORTED 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 RETURN_UNSUPPORTED The operation is not supported.
+
+**/
+RETURN_STATUS
+EFIAPI
+X509GetCommonName (
+ IN CONST UINT8 *Cert,
+ IN UINTN CertSize,
+ OUT CHAR8 *CommonName,
+ IN OUT UINTN *CommonNameSize
+ )
+{
+ ASSERT (FALSE);
+ return RETURN_UNSUPPORTED;
+}
+
/**
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] 4+ messages in thread
* Re: [PATCH v2] CryptoPkg: Add new API to retrieve commonName of X.509 certificate
2017-09-20 16:05 [PATCH v2] CryptoPkg: Add new API to retrieve commonName of X.509 certificate Qin Long
@ 2017-09-20 16:38 ` Laszlo Ersek
2017-09-20 17:04 ` Long, Qin
0 siblings, 1 reply; 4+ messages in thread
From: Laszlo Ersek @ 2017-09-20 16:38 UTC (permalink / raw)
To: Qin Long, ting.ye, chao.b.zhang; +Cc: edk2-devel
Hello Qin,
On 09/20/17 18:05, Qin Long wrote:
> 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 <lersek@redhat.com>
> 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 | 32 +++++--
> CryptoPkg/Include/Library/BaseCryptLib.h | 34 +++++++
> CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 106 +++++++++++++++++++++
> CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c | 32 +++++++
> .../Pk/CryptX509Null.c | 34 ++++++-
> 5 files changed, 230 insertions(+), 8 deletions(-)
>
> diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h
> index 9c5ffcd9cf..48e9531758 100644
> --- a/CryptoPkg/Include/Library/BaseCryptLib.h
> +++ b/CryptoPkg/Include/Library/BaseCryptLib.h
> @@ -2171,6 +2171,40 @@ 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 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 UINTN CertSize,
> + OUT CHAR8 *CommonName,
> + IN OUT UINTN *CommonNameSize
> + );
> +
> /**
> Verify one X509 certificate was issued by the trusted CA.
>
I think the RETURN_BUFFER_TOO_SMALL description is incorrect -- it
shouldn't only cover the (CommonName == NULL) case, but any other case
when *CommonNameSize is not large enough, for formatting the full CN,
plus the terminating '\0'.
Relatedly, the output value of *CommonNameSize should always be the
number of bytes required to format the NUL-terminated common name,
regardless if there is enough room or not. The return status will tell
the caller:
- if the return status is BUFFER_TOO_SMALL, then a larger buffer is
needed -- how large is explained by *CommonNameSize
- if the return status is SUCCESS, then the buffer was large enough, and
*CommonNameSize bytes have been used from it.
Additional question: does the code handle the case when *CommonNameSize
is zero, on input? (I.e., when there isn't even room for storing a '\0'
character.)
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] CryptoPkg: Add new API to retrieve commonName of X.509 certificate
2017-09-20 16:38 ` Laszlo Ersek
@ 2017-09-20 17:04 ` Long, Qin
2017-09-20 17:12 ` Laszlo Ersek
0 siblings, 1 reply; 4+ messages in thread
From: Long, Qin @ 2017-09-20 17:04 UTC (permalink / raw)
To: Laszlo Ersek, Ye, Ting, Zhang, Chao B; +Cc: edk2-devel@lists.01.org
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Thursday, September 21, 2017 12:38 AM
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: [PATCH v2] CryptoPkg: Add new API to retrieve commonName of X.509 certificate
Hello Qin,
On 09/20/17 18:05, Qin Long wrote:
> 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 <lersek@redhat.com<mailto:lersek@redhat.com>>
> 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 | 32 +++++--
> CryptoPkg/Include/Library/BaseCryptLib.h | 34 +++++++
> CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 106 +++++++++++++++++++++
> CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c | 32 +++++++
> .../Pk/CryptX509Null.c | 34 ++++++-
> 5 files changed, 230 insertions(+), 8 deletions(-)
>
> diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h
> index 9c5ffcd9cf..48e9531758 100644
> --- a/CryptoPkg/Include/Library/BaseCryptLib.h
> +++ b/CryptoPkg/Include/Library/BaseCryptLib.h
> @@ -2171,6 +2171,40 @@ 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 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 UINTN CertSize,
> + OUT CHAR8 *CommonName,
> + IN OUT UINTN *CommonNameSize
> + );
> +
> /**
> Verify one X509 certificate was issued by the trusted CA.
>
I think the RETURN_BUFFER_TOO_SMALL description is incorrect -- it
shouldn't only cover the (CommonName == NULL) case, but any other case
when *CommonNameSize is not large enough, for formatting the full CN,
plus the terminating '\0'.
Relatedly, the output value of *CommonNameSize should always be the
number of bytes required to format the NUL-terminated common name,
regardless if there is enough room or not. The return status will tell
the caller:
- if the return status is BUFFER_TOO_SMALL, then a larger buffer is
needed -- how large is explained by *CommonNameSize
- if the return status is SUCCESS, then the buffer was large enough, and
*CommonNameSize bytes have been used from it.
[qlong] good catch.
The current implementation is based on OpenSSL X509_NAME_get_text_by_OBJ
API, and we can only get the real written data size or required size (by passing
NULL CommonName) with this interface.
I didn’t want to introduce additional handling (e.g. extra ASN1_STRING parsing) in
this API. For fixed CommonNameSize buffer, it’s acceptable to receive the truncated
string (e.g. for display purpose) depending on the API usage (and the CN string should
be less than 64 char per the standard).
Additional question: does the code handle the case when *CommonNameSize
is zero, on input? (I.e., when there isn't even room for storing a '\0'
character.)
[qlong] It’s one missed case. And more interesting is this may expose one OpenSSL issue.
Let me double-check this. ☺
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] CryptoPkg: Add new API to retrieve commonName of X.509 certificate
2017-09-20 17:04 ` Long, Qin
@ 2017-09-20 17:12 ` Laszlo Ersek
0 siblings, 0 replies; 4+ messages in thread
From: Laszlo Ersek @ 2017-09-20 17:12 UTC (permalink / raw)
To: Long, Qin, Ye, Ting, Zhang, Chao B; +Cc: edk2-devel@lists.01.org
On 09/20/17 19:04, Long, Qin wrote:
>
>
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, September 21, 2017 12:38 AM
> 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: [PATCH v2] CryptoPkg: Add new API to retrieve commonName of X.509 certificate
>
> Hello Qin,
>
> On 09/20/17 18:05, Qin Long wrote:
>> 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 <lersek@redhat.com<mailto:lersek@redhat.com>>
>> 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 | 32 +++++--
>> CryptoPkg/Include/Library/BaseCryptLib.h | 34 +++++++
>> CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 106 +++++++++++++++++++++
>> CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c | 32 +++++++
>> .../Pk/CryptX509Null.c | 34 ++++++-
>> 5 files changed, 230 insertions(+), 8 deletions(-)
>>
>> diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h
>> index 9c5ffcd9cf..48e9531758 100644
>> --- a/CryptoPkg/Include/Library/BaseCryptLib.h
>> +++ b/CryptoPkg/Include/Library/BaseCryptLib.h
>> @@ -2171,6 +2171,40 @@ 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 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 UINTN CertSize,
>> + OUT CHAR8 *CommonName,
>> + IN OUT UINTN *CommonNameSize
>> + );
>> +
>> /**
>> Verify one X509 certificate was issued by the trusted CA.
>>
>
> I think the RETURN_BUFFER_TOO_SMALL description is incorrect -- it
> shouldn't only cover the (CommonName == NULL) case, but any other case
> when *CommonNameSize is not large enough, for formatting the full CN,
> plus the terminating '\0'.
>
> Relatedly, the output value of *CommonNameSize should always be the
> number of bytes required to format the NUL-terminated common name,
> regardless if there is enough room or not. The return status will tell
> the caller:
> - if the return status is BUFFER_TOO_SMALL, then a larger buffer is
> needed -- how large is explained by *CommonNameSize
> - if the return status is SUCCESS, then the buffer was large enough, and
> *CommonNameSize bytes have been used from it.
>
> [qlong] good catch.
> The current implementation is based on OpenSSL X509_NAME_get_text_by_OBJ
> API, and we can only get the real written data size or required size (by passing
> NULL CommonName) with this interface.
> I didn’t want to introduce additional handling (e.g. extra ASN1_STRING parsing) in
> this API. For fixed CommonNameSize buffer, it’s acceptable to receive the truncated
> string (e.g. for display purpose) depending on the API usage (and the CN string should
> be less than 64 char per the standard).
OK. If the most obvious / straight-forward implementation for the
interface does not support these elaborate use cases, then I agree it's
fine to stick with what's readily available from the underlying features.
> Additional question: does the code handle the case when *CommonNameSize
> is zero, on input? (I.e., when there isn't even room for storing a '\0'
> character.)
>
> [qlong] It’s one missed case. And more interesting is this may expose one OpenSSL issue.
> Let me double-check this. ☺
Heh, great :) Thanks!
Laszlo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-09-20 17:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-20 16:05 [PATCH v2] CryptoPkg: Add new API to retrieve commonName of X.509 certificate Qin Long
2017-09-20 16:38 ` Laszlo Ersek
2017-09-20 17:04 ` Long, Qin
2017-09-20 17:12 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox