* [PATCH] CryptoPkg: Remove deprecated function usage in X509GetCommonName()
@ 2018-05-24 8:11 Long Qin
2018-05-29 3:16 ` Ye, Ting
0 siblings, 1 reply; 2+ messages in thread
From: Long Qin @ 2018-05-24 8:11 UTC (permalink / raw)
To: edk2-devel; +Cc: ting.ye, Michael.Turner
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=923
X509_NAME_get_text_by_NID() used in X509GetCommonName() implementation
is one legacy function which have various limitations. The returned
data may be not usable when the target cert contains multicharacter
string type like a BMPString or a UTF8String.
This patch replaced the legacy function usage with more general
X509_NAME_get_index_by_NID() / X509_NAME_get_entry() APIs for X509
CommonName retrieving.
Tests: Validated the commonName retrieving with test certificates
containing PrintableString or BMPString data.
Cc: Ye Ting <ting.ye@intel.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Long Qin <qin.long@intel.com>
---
CryptoPkg/Include/Library/BaseCryptLib.h | 4 +-
CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 53 ++++++++++++++++++-----
CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c | 4 +-
3 files changed, 47 insertions(+), 14 deletions(-)
diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h
index 027ea09feb..dc6aaf0635 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -4,7 +4,7 @@
primitives (Hash Serials, HMAC, RSA, Diffie-Hellman, etc) for UEFI security
functionality enabling.
-Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -2177,7 +2177,7 @@ X509GetSubjectName (
@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
+ name string (UTF8). 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,
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
index 56e66308ae..c137df357f 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
@@ -1,7 +1,7 @@
/** @file
X.509 Certificate Handler Wrapper Implementation over OpenSSL.
-Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -303,7 +303,7 @@ _Exit:
@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
+ name string (UTF8). 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,
@@ -332,13 +332,18 @@ X509GetCommonName (
IN OUT UINTN *CommonNameSize
)
{
- RETURN_STATUS ReturnStatus;
- BOOLEAN Status;
- X509 *X509Cert;
- X509_NAME *X509Name;
- INTN Length;
+ RETURN_STATUS ReturnStatus;
+ BOOLEAN Status;
+ X509 *X509Cert;
+ X509_NAME *X509Name;
+ INT32 Index;
+ INTN Length;
+ X509_NAME_ENTRY *Entry;
+ ASN1_STRING *EntryData;
+ UINT8 *UTF8Name;
ReturnStatus = RETURN_INVALID_PARAMETER;
+ UTF8Name = NULL;
//
// Check input parameters.
@@ -378,8 +383,8 @@ X509GetCommonName (
//
// 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) {
+ Index = X509_NAME_get_index_by_NID (X509Name, NID_commonName, -1);
+ if (Index < 0) {
//
// No CommonName entry exists in X509_NAME object
//
@@ -388,10 +393,35 @@ X509GetCommonName (
goto _Exit;
}
- *CommonNameSize = (UINTN)(Length + 1);
+ Entry = X509_NAME_get_entry (X509Name, Index);
+ if (Entry == NULL) {
+ //
+ // Fail to retrieve name entry data
+ //
+ *CommonNameSize = 0;
+ ReturnStatus = RETURN_NOT_FOUND;
+ goto _Exit;
+ }
+
+ EntryData = X509_NAME_ENTRY_get_data (Entry);
+
+ Length = ASN1_STRING_to_UTF8 (&UTF8Name, EntryData);
+ if (Length < 0) {
+ //
+ // Fail to convert the commonName string
+ //
+ *CommonNameSize = 0;
+ ReturnStatus = RETURN_INVALID_PARAMETER;
+ goto _Exit;
+ }
+
if (CommonName == NULL) {
+ *CommonNameSize = Length + 1;
ReturnStatus = RETURN_BUFFER_TOO_SMALL;
} else {
+ *CommonNameSize = MIN ((UINTN)Length, *CommonNameSize - 1) + 1;
+ CopyMem (CommonName, UTF8Name, *CommonNameSize - 1);
+ CommonName[*CommonNameSize - 1] = '\0';
ReturnStatus = RETURN_SUCCESS;
}
@@ -402,6 +432,9 @@ _Exit:
if (X509Cert != NULL) {
X509_free (X509Cert);
}
+ if (UTF8Name != NULL) {
+ OPENSSL_free (UTF8Name);
+ }
return ReturnStatus;
}
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
index d00f38daa8..d86c784a7f 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
@@ -2,7 +2,7 @@
X.509 Certificate Handler Wrapper Implementation which does not provide
real capabilities.
-Copyright (c) 2012 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -135,7 +135,7 @@ X509GetSubjectName (
@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
+ name string (UTF8). 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,
--
2.16.1.windows.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] CryptoPkg: Remove deprecated function usage in X509GetCommonName()
2018-05-24 8:11 [PATCH] CryptoPkg: Remove deprecated function usage in X509GetCommonName() Long Qin
@ 2018-05-29 3:16 ` Ye, Ting
0 siblings, 0 replies; 2+ messages in thread
From: Ye, Ting @ 2018-05-29 3:16 UTC (permalink / raw)
To: Long, Qin, edk2-devel@lists.01.org; +Cc: Michael.Turner@microsoft.com
Reviewed-by: Ye Ting <ting.ye@intel.com>
-----Original Message-----
From: Long, Qin
Sent: Thursday, May 24, 2018 4:11 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting <ting.ye@intel.com>; Michael.Turner@microsoft.com
Subject: [PATCH] CryptoPkg: Remove deprecated function usage in X509GetCommonName()
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=923
X509_NAME_get_text_by_NID() used in X509GetCommonName() implementation is one legacy function which have various limitations. The returned data may be not usable when the target cert contains multicharacter string type like a BMPString or a UTF8String.
This patch replaced the legacy function usage with more general
X509_NAME_get_index_by_NID() / X509_NAME_get_entry() APIs for X509 CommonName retrieving.
Tests: Validated the commonName retrieving with test certificates
containing PrintableString or BMPString data.
Cc: Ye Ting <ting.ye@intel.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Long Qin <qin.long@intel.com>
---
CryptoPkg/Include/Library/BaseCryptLib.h | 4 +-
CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 53 ++++++++++++++++++-----
CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c | 4 +-
3 files changed, 47 insertions(+), 14 deletions(-)
diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h
index 027ea09feb..dc6aaf0635 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -4,7 +4,7 @@
primitives (Hash Serials, HMAC, RSA, Diffie-Hellman, etc) for UEFI security
functionality enabling.
-Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at @@ -2177,7 +2177,7 @@ X509GetSubjectName (
@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
+ name string (UTF8). 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,
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
index 56e66308ae..c137df357f 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
@@ -1,7 +1,7 @@
/** @file
X.509 Certificate Handler Wrapper Implementation over OpenSSL.
-Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at @@ -303,7 +303,7 @@ _Exit:
@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
+ name string (UTF8). 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,
@@ -332,13 +332,18 @@ X509GetCommonName (
IN OUT UINTN *CommonNameSize
)
{
- RETURN_STATUS ReturnStatus;
- BOOLEAN Status;
- X509 *X509Cert;
- X509_NAME *X509Name;
- INTN Length;
+ RETURN_STATUS ReturnStatus;
+ BOOLEAN Status;
+ X509 *X509Cert;
+ X509_NAME *X509Name;
+ INT32 Index;
+ INTN Length;
+ X509_NAME_ENTRY *Entry;
+ ASN1_STRING *EntryData;
+ UINT8 *UTF8Name;
ReturnStatus = RETURN_INVALID_PARAMETER;
+ UTF8Name = NULL;
//
// Check input parameters.
@@ -378,8 +383,8 @@ X509GetCommonName (
//
// 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) {
+ Index = X509_NAME_get_index_by_NID (X509Name, NID_commonName, -1);
+ if (Index < 0) {
//
// No CommonName entry exists in X509_NAME object
//
@@ -388,10 +393,35 @@ X509GetCommonName (
goto _Exit;
}
- *CommonNameSize = (UINTN)(Length + 1);
+ Entry = X509_NAME_get_entry (X509Name, Index); if (Entry == NULL) {
+ //
+ // Fail to retrieve name entry data
+ //
+ *CommonNameSize = 0;
+ ReturnStatus = RETURN_NOT_FOUND;
+ goto _Exit;
+ }
+
+ EntryData = X509_NAME_ENTRY_get_data (Entry);
+
+ Length = ASN1_STRING_to_UTF8 (&UTF8Name, EntryData); if (Length < 0)
+ {
+ //
+ // Fail to convert the commonName string
+ //
+ *CommonNameSize = 0;
+ ReturnStatus = RETURN_INVALID_PARAMETER;
+ goto _Exit;
+ }
+
if (CommonName == NULL) {
+ *CommonNameSize = Length + 1;
ReturnStatus = RETURN_BUFFER_TOO_SMALL;
} else {
+ *CommonNameSize = MIN ((UINTN)Length, *CommonNameSize - 1) + 1;
+ CopyMem (CommonName, UTF8Name, *CommonNameSize - 1);
+ CommonName[*CommonNameSize - 1] = '\0';
ReturnStatus = RETURN_SUCCESS;
}
@@ -402,6 +432,9 @@ _Exit:
if (X509Cert != NULL) {
X509_free (X509Cert);
}
+ if (UTF8Name != NULL) {
+ OPENSSL_free (UTF8Name);
+ }
return ReturnStatus;
}
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
index d00f38daa8..d86c784a7f 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c
@@ -2,7 +2,7 @@
X.509 Certificate Handler Wrapper Implementation which does not provide
real capabilities.
-Copyright (c) 2012 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at @@ -135,7 +135,7 @@ X509GetSubjectName (
@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
+ name string (UTF8). 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,
--
2.16.1.windows.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-05-29 4:35 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-24 8:11 [PATCH] CryptoPkg: Remove deprecated function usage in X509GetCommonName() Long Qin
2018-05-29 3:16 ` Ye, Ting
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox