SSL_set1_host() in TlsSetVerifyHost() ignores GEN_IP entries in the peer
certificate's Subject Alternative Name (SAN) extension. This leads to the
rejection of any valid peer certificate that matches the dot-decimal IPv4,
or colon-hexadecimal IPv6, host part of an URL *only* through SAN/GEN_IP,
and not through the Common Name.
Based on David's guidance, replace SSL_set1_host() in TlsSetVerifyHost()
with application specific data ("ExData") that is associated with the SSL
object. Namely, pass the host part of the URL as "application specific
data" into a new peer certificate verification callback. In the callback,
first try to parse the host part of the URL as a numeric IP address, for
certificate subject verification. If that parsing fails, fall back to
interpreting the host part as a DNS hostname.
Cc: Bret Barkelew <bret.barke...@microsoft.com>
Cc: David Woodhouse <dw...@infradead.org>
Cc: Jian J Wang <jian.j.w...@intel.com>
Cc: Jiaxin Wu <jiaxin...@intel.com>
Cc: Richard Levitte <levi...@openssl.org>
Cc: Sivaraman Nainar <sivaram...@amiindia.co.in>
Ref:
http://mid.mail-archive.com/B4DE137BDB63634BAC03BD9DE765F197028B24CA23@VENUS1.in.megatrends.com
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
Ref: https://edk2.groups.io/g/devel/message/42022
Suggested-by: David Woodhouse <dw...@infradead.org>
Signed-off-by: Laszlo Ersek <ler...@redhat.com>
---
Notes:
Unfortunately, there are two problems with this patch:
(1) X509_VERIFY_PARAM_set1_ip_asc() does not accept IPv4 addresses in
dot-decimal notation (unless I messed up the code). My log file
contains:
> TlsDxe:TlsCertVerify: verifying peer certificate with DNS hostname
"192.168.124.2"
> TlsDxe:TlsCertVerify: peer certificate accepted
(2) X509_VERIFY_PARAM_set1_ip_asc() does accept IPv6 addresses. However,
in that case, the server certificate that I had generated with
"genkey" (where I entered the IPv6 address in the Common Name field)
is rejected:
> TlsDxe:TlsCertVerify: verifying peer certificate with numerical IP
address "fd33:eb1b:9b36::2"
> TlsDxe:TlsCertVerify: peer certificate rejected
> TlsDoHandshake SSL_HANDSHAKE_ERROR State=0x4 SSL_ERROR_SSL
> TlsDoHandshake ERROR 0x1416F086=L14:F16F:R86
If I do not apply the present patch on top of Jiaxin's v1 4/4 (at
<20190927034441.3096-5-Jiaxin.wu@intel.com">http://mid.mail-archive.com/20190927034441.3096-5-Jiaxin.wu@intel.com>),
then the certificate is accepted fine.
Not sure how to address these.
CryptoPkg/Library/TlsLib/TlsLib.inf | 1 +
CryptoPkg/Library/TlsLib/InternalTlsLib.h | 33 +++
CryptoPkg/Library/TlsLib/TlsConfig.c | 17 +-
CryptoPkg/Library/TlsLib/TlsExData.c | 301 ++++++++++++++++++++
CryptoPkg/Library/TlsLib/TlsInit.c | 35 ++-
5 files changed, 385 insertions(+), 2 deletions(-)
diff --git a/CryptoPkg/Library/TlsLib/TlsLib.inf
b/CryptoPkg/Library/TlsLib/TlsLib.inf
index 2f3ce695c33e..1f65eea516d4 100644
--- a/CryptoPkg/Library/TlsLib/TlsLib.inf
+++ b/CryptoPkg/Library/TlsLib/TlsLib.inf
@@ -24,12 +24,13 @@ [Defines]
[Sources]
InternalTlsLib.h
TlsInit.c
TlsConfig.c
TlsProcess.c
+ TlsExData.c
[Packages]
MdePkg/MdePkg.dec
CryptoPkg/CryptoPkg.dec
[LibraryClasses]
diff --git a/CryptoPkg/Library/TlsLib/InternalTlsLib.h
b/CryptoPkg/Library/TlsLib/InternalTlsLib.h
index ce7f4ced4a30..c8762befd31c 100644
--- a/CryptoPkg/Library/TlsLib/InternalTlsLib.h
+++ b/CryptoPkg/Library/TlsLib/InternalTlsLib.h
@@ -34,8 +34,41 @@ typedef struct {
//
// Memory BIO for the TLS/SSL Writing operations.
//
BIO *OutBio;
} TLS_CONNECTION;
+//
+// See the documentation for "mPeerSubjectNameKey",
+// TlsPeerSubjectNameDuplicate(), TlsPeerSubjectNameFree(), and TlsCertVerify()
+// in "TlsExData.c".
+//
+extern INT32 mPeerSubjectNameKey;
+
+INT32
+TlsPeerSubjectNameDuplicate (
+ OUT CRYPTO_EX_DATA *DestinationExData,
+ IN CONST CRYPTO_EX_DATA *SourceExData,
+ IN OUT VOID *PeerSubjectNameAddress,
+ IN INT32 ExDataType,
+ IN long ArgLong,
+ IN VOID *ArgPtr
+ );
+
+VOID
+TlsPeerSubjectNameFree (
+ IN VOID *ParentSsl,
+ IN VOID *PeerSubjectName OPTIONAL,
+ IN CRYPTO_EX_DATA *ExData,
+ IN INT32 ExDataType,
+ IN long ArgLong,
+ IN VOID *ArgPtr
+ );
+
+INT32
+TlsCertVerify (
+ IN X509_STORE_CTX *PeerCertificateChain,
+ IN VOID *Arg
+ );
+
#endif
diff --git a/CryptoPkg/Library/TlsLib/TlsConfig.c
b/CryptoPkg/Library/TlsLib/TlsConfig.c
index 2bf5aee7c093..114168dfb020 100644
--- a/CryptoPkg/Library/TlsLib/TlsConfig.c
+++ b/CryptoPkg/Library/TlsLib/TlsConfig.c
@@ -504,32 +504,47 @@ TlsSetVerify (
@param[in] Flags The setting flags during the validation.
@param[in] HostName The specified host name to be verified.
@retval EFI_SUCCESS The HostName setting was set successfully.
@retval EFI_INVALID_PARAMETER The parameter is invalid.
@retval EFI_ABORTED Invalid HostName setting.
+ @retval EFI_OUT_OF_RESOURCES Memory allocation failure.
**/
EFI_STATUS
EFIAPI
TlsSetVerifyHost (
IN VOID *Tls,
IN UINT32 Flags,
IN CHAR8 *HostName
)
{
TLS_CONNECTION *TlsConn;
+ CHAR8 *PeerSubjectName;
TlsConn = (TLS_CONNECTION *) Tls;
if (TlsConn == NULL || TlsConn->Ssl == NULL || HostName == NULL) {
return EFI_INVALID_PARAMETER;
}
+ PeerSubjectName = AllocateCopyPool (
+ AsciiStrSize (HostName),
+ HostName
+ );
+ if (PeerSubjectName == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
SSL_set_hostflags(TlsConn->Ssl, Flags);
- if (SSL_set1_host(TlsConn->Ssl, HostName) == 0) {
+ if (SSL_set_ex_data (
+ TlsConn->Ssl,
+ mPeerSubjectNameKey,
+ PeerSubjectName
+ ) == 0) {
+ FreePool (PeerSubjectName);
return EFI_ABORTED;
}
return EFI_SUCCESS;
}
diff --git a/CryptoPkg/Library/TlsLib/TlsExData.c
b/CryptoPkg/Library/TlsLib/TlsExData.c
new file mode 100644
index 000000000000..9671234f8416
--- /dev/null
+++ b/CryptoPkg/Library/TlsLib/TlsExData.c
@@ -0,0 +1,301 @@
+/** @file
+ OpenSSL callback functions for:
+
+ - duplicating and freeing the Peer Subject Name strings that we associate
+ with SSL objects as application data ("ExData"),
+
+ - verifying peer certificates against the Subject Name stings associated with
+ SSL objects.
+
+ Copyright (C) 2019, Red Hat, Inc.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include "InternalTlsLib.h"
+
+//
+// We attach the Subject Name that we expect the peer certificate to match to
+// the SSL object as an application-specific datum. This type of
+// application-specific data first needs to be registered with OpenSSL. The
+// registration identifier is stored in the object below.
+//
+// We define the associated data type as (CHAR8*), pointing to a
+// dynamically-allocated, NUL-terminated ASCII string. The string may contain a
+// DNS hostname, or an IPv4 address in dotted decimal notation, or an IPv6
+// address in colon-separated hexadecimal notation (without the surrounding
+// brackets used in URLs). The condensed "::" notation is supported for IPv6
+// addresses.
+//
+INT32 mPeerSubjectNameKey;
+
+/**
+ OpenSSL callback function for duplicating the Subject Name when its parent
+ SSL object is duplicated.
+
+ Because this function is an OpenSSL callback, it must not be declared EFIAPI.
+
+ @param[out] DestinationExData The ExData object in the new SSL
+ object. DestinationExData is the
+ dictionary in which
+ mPeerSubjectNameKey identifies the new
+ (duplicated) subject name. Ignored.
+
+ @param[in] SourceExData The ExData object in the original SSL
+ object. SourceExData is the dictionary
+ in which mPeerSubjectNameKey
+ identifies the subject name to
+ duplicate. Ignored.
+
+ @param[in,out] PeerSubjectNameAddress On input,
+ *(VOID**)PeerSubjectNameAddress points
+ to the Subject Name in SourceExData.
+ On output,
+ *(VOID**)PeerSubjectNameAddress points
+ to the newly allocated copy of the
+ Subject Name, to be stored in
+ DestinationExData. On input,
+ PeerSubjectNameAddress must not be
+ NULL, but
+ *(VOID**)PeerSubjectNameAddress may be
+ NULL.
+
+ @param[in] ExDataType Equals mPeerSubjectNameKey. Ignored.
+
+ @param[in] ArgLong Zero; ignored.
+
+ @param[in] ArgPtr NULL; ignored.
+
+ @retval 0 Memory allocation failure.
+
+ @retval 1 Successful duplication (including a NULL subject name, when
+ nothing is done).
+**/
+INT32
+TlsPeerSubjectNameDuplicate (
+ OUT CRYPTO_EX_DATA *DestinationExData,
+ IN CONST CRYPTO_EX_DATA *SourceExData,
+ IN OUT VOID *PeerSubjectNameAddress,
+ IN INT32 ExDataType,
+ IN long ArgLong,
+ IN VOID *ArgPtr
+ )
+{
+ CHAR8 *PeerSubjectName;
+ CHAR8 *NewPeerSubjectName;
+
+ //
+ // Assert that these input parameters match what we passed to
+ // SSL_get_ex_new_index() in TlsInitialize().
+ //
+ ASSERT (ExDataType == mPeerSubjectNameKey);
+ ASSERT (ArgLong == 0);
+ ASSERT (ArgPtr == NULL);
+
+ //
+ // Further assert non-nullity for PeerSubjectNameAddress.
+ //
+ ASSERT (PeerSubjectNameAddress != NULL);
+
+ PeerSubjectName = *(VOID **)PeerSubjectNameAddress;
+ if (PeerSubjectName == NULL) {
+ DEBUG ((DEBUG_VERBOSE, "%a:%a: nothing to copy\n", gEfiCallerBaseName,
+ __FUNCTION__));
+ //
+ // Exit with success.
+ //
+ return 1;
+ }
+
+ NewPeerSubjectName = AllocateCopyPool (
+ AsciiStrSize (PeerSubjectName),
+ PeerSubjectName
+ );
+ if (NewPeerSubjectName == NULL) {
+ DEBUG ((DEBUG_ERROR, "%a:%a: failed to allocate memory\n",
+ gEfiCallerBaseName, __FUNCTION__));
+ return 0;
+ }
+
+ *(VOID **)PeerSubjectNameAddress = NewPeerSubjectName;
+ DEBUG ((DEBUG_VERBOSE,
+ "%a:%a: copied peer subject name \"%a\" from %p to %p\n",
+ gEfiCallerBaseName, __FUNCTION__, PeerSubjectName, (VOID *)PeerSubjectName,
+ (VOID *)NewPeerSubjectName));
+ return 1;
+}
+
+/**
+ OpenSSL callback function for freeing the Subject Name when its parent SSL
+ object is freed.
+
+ Because this function is an OpenSSL callback, it must not be declared EFIAPI.
+
+ @param[in] ParentSsl The parent SSL object being freed. Ignored.
+
+ @param[in] PeerSubjectName The subject name to release. May be NULL.
+
+ @param[in] ExData The ExData object in ParentSsl. ExData is the
+ dictionary in which mPeerSubjectNameKey
+ identifies the subject name to release. Ignored.
+
+ @param[in] ExDataType Equals mPeerSubjectNameKey. Ignored.
+
+ @param[in] ArgLong Zero; ignored.
+
+ @param[in] ArgPtr NULL; ignored.
+**/
+VOID
+TlsPeerSubjectNameFree (
+ IN VOID *ParentSsl,
+ IN VOID *PeerSubjectName OPTIONAL,
+ IN CRYPTO_EX_DATA *ExData,
+ IN INT32 ExDataType,
+ IN long ArgLong,
+ IN VOID *ArgPtr
+ )
+{
+ //
+ // Assert that these input parameters match what we passed to
+ // SSL_get_ex_new_index() in TlsInitialize().
+ //
+ ASSERT (ExDataType == mPeerSubjectNameKey);
+ ASSERT (ArgLong == 0);
+ ASSERT (ArgPtr == NULL);
+
+ if (PeerSubjectName == NULL) {
+ return;
+ }
+
+ DEBUG ((DEBUG_VERBOSE, "%a:%a: freeing peer subject name \"%a\" at %p\n",
+ gEfiCallerBaseName, __FUNCTION__, (CHAR8 *)PeerSubjectName,
+ PeerSubjectName));
+ FreePool (PeerSubjectName);
+}
+
+/**
+ OpenSSL callback function for discovering and verifying the X509 peer
+ certificate chain during SSL/TLS handshake.
+
+ This function wraps the X509_verify_cert() OpenSSL function; it ensures that
+ both DNS host names and numeric IPv4/IPv6 addresses are matched in peer
+ certificates as Subject Names.
+
+ Because this function is an OpenSSL callback, it must not be declared EFIAPI.
+
+ @param[in] PeerCertificateChain The certificate chain of the peer to verify.
+ The function checks whether
+ PeerCertificateChain matches the Peer
+ Subject Name that we've associated with the
+ SSL object of the network connection.
+
+ @param[in] Arg NULL; ignored.
+
+ @retval 1 Verification success.
+
+ @retval 0 Verification failure.
+**/
+INT32
+TlsCertVerify (
+ IN X509_STORE_CTX *PeerCertificateChain,
+ IN VOID *Arg
+ )
+{
+ SSL *Ssl;
+ X509_VERIFY_PARAM *VerifyParams;
+ CHAR8 *SubjectName;
+ INT32 ParamStatus;
+ INT32 VerifyStatus;
+
+ //
+ // Assert that these input parameters match what we passed to
+ // SSL_CTX_set_cert_verify_callback() in TlsCtxNew().
+ //
+ ASSERT (Arg == NULL);
+
+ //
+ // Retrieve the SSL object associated with the network connection for which
+ // the peer certificate is being verified in the SSL/TLS handshake.
+ //
+ Ssl = X509_STORE_CTX_get_ex_data (
+ PeerCertificateChain,
+ SSL_get_ex_data_X509_STORE_CTX_idx ()
+ );
+ if (Ssl == NULL) {
+ DEBUG ((DEBUG_ERROR, "%a:%a: SSL object not found\n", gEfiCallerBaseName,
+ __FUNCTION__));
+ //
+ // Reject the certificate.
+ //
+ return 0;
+ }
+
+ //
+ // Fetch the certificate verification parameters.
+ //
+ VerifyParams = X509_STORE_CTX_get0_param (PeerCertificateChain);
+ if (VerifyParams == NULL) {
+ DEBUG ((DEBUG_ERROR, "%a:%a: verification parameters not found\n",
+ gEfiCallerBaseName, __FUNCTION__));
+ return 0;
+ }
+
+ //
+ // Retrieve the Peer Subject Name that we *may* have associated with the SSL
+ // object in TlsSetVerifyHost().
+ //
+ SubjectName = SSL_get_ex_data (Ssl, mPeerSubjectNameKey);
+ //
+ // If SubjectName is NULL or empty, explicitly clear the list of host names
+ // in VerifyParams, and perform no name checks on the peer certificate.
+ //
+ // Otherwise, attempt to parse the Peer Subject Name as an IPv4 or IPv6
+ // address. If this succeeds, then the parsed address is used for verifying
+ // the peer certificate.
+ //
+ // Otherwise, verify the peer certificate with SubjectName taken as a DNS
+ // hostname.
+ //
+ if (SubjectName == NULL || SubjectName[0] == '\0') {
+ ParamStatus = X509_VERIFY_PARAM_set1_host (VerifyParams, SubjectName, 0);
+
+ DEBUG ((DEBUG_WARN, "%a:%a: verifying peer certificate without subject "
+ "name check (MITM risk)!\n", gEfiCallerBaseName, __FUNCTION__));
+ } else {
+ ParamStatus = X509_VERIFY_PARAM_set1_ip_asc (VerifyParams, SubjectName);
+
+ if (ParamStatus == 1) {
+ DEBUG ((DEBUG_VERBOSE,
+ "%a:%a: verifying peer certificate with numerical IP address \"%a\"\n",
+ gEfiCallerBaseName, __FUNCTION__, SubjectName));
+ } else {
+ ParamStatus = X509_VERIFY_PARAM_set1_host (VerifyParams, SubjectName, 0);
+
+ DEBUG ((DEBUG_VERBOSE,
+ "%a:%a: verifying peer certificate with DNS hostname \"%a\"\n",
+ gEfiCallerBaseName, __FUNCTION__, SubjectName));
+ }
+ }
+
+ if (ParamStatus == 0) {
+ DEBUG ((DEBUG_ERROR,
+ "%a:%a: unexpected failure to set verification parameters\n",
+ gEfiCallerBaseName, __FUNCTION__));
+ //
+ // Reject the certificate.
+ //
+ return 0;
+ }
+
+ VerifyStatus = X509_verify_cert (PeerCertificateChain);
+
+ if (VerifyStatus > 0) {
+ DEBUG ((DEBUG_VERBOSE, "%a:%a: peer certificate accepted\n",
+ gEfiCallerBaseName, __FUNCTION__));
+ return 1;
+ }
+
+ DEBUG ((DEBUG_ERROR, "%a:%a: peer certificate rejected\n",
+ gEfiCallerBaseName, __FUNCTION__));
+ return 0;
+}
diff --git a/CryptoPkg/Library/TlsLib/TlsInit.c
b/CryptoPkg/Library/TlsLib/TlsInit.c
index f9ad6f6b946c..c7918364a4c7 100644
--- a/CryptoPkg/Library/TlsLib/TlsInit.c
+++ b/CryptoPkg/Library/TlsLib/TlsInit.c
@@ -24,29 +24,53 @@ BOOLEAN
EFIAPI
TlsInitialize (
VOID
)
{
INTN Ret;
+ BOOLEAN RandomIsSeeded;
//
// Performs initialization of crypto and ssl library, and loads required
// algorithms.
//
Ret = OPENSSL_init_ssl (
OPENSSL_INIT_LOAD_SSL_STRINGS | OPENSSL_INIT_LOAD_CRYPTO_STRINGS,
NULL
);
if (Ret != 1) {
return FALSE;
}
+ //
+ // OPENSSL_init_ssl() cannot, and need not, be rolled back, if the rest of
+ // this function fails.
+ //
+
+ mPeerSubjectNameKey = SSL_get_ex_new_index (
+ 0, // "argl": unneeded
+ NULL, // "argp": unneeded
+ NULL, // "new_func": unneeded
+ TlsPeerSubjectNameDuplicate, // "dup_func"
+ TlsPeerSubjectNameFree // "free_func"
+ );
+ if (mPeerSubjectNameKey == -1) {
+ return FALSE;
+ }
//
// Initialize the pseudorandom number generator.
//
- return RandomSeed (NULL, 0);
+ RandomIsSeeded = RandomSeed (NULL, 0);
+ if (!RandomIsSeeded) {
+ goto DeregisterPeerSubjectName;
+ }
+ return TRUE;
+
+DeregisterPeerSubjectName:
+ CRYPTO_free_ex_index (CRYPTO_EX_INDEX_SSL, mPeerSubjectNameKey);
+ return FALSE;
}
/**
Free an allocated SSL_CTX object.
@param[in] TlsCtx Pointer to the SSL_CTX object to be released.
@@ -103,12 +127,21 @@ TlsCtxNew (
//
// Treat as minimum accepted versions by setting the minimal bound.
// Client can use higher TLS version if server supports it
//
SSL_CTX_set_min_proto_version (TlsCtx, ProtoVersion);
+ //
+ // Set peer certificate verification procedure.
+ //
+ SSL_CTX_set_cert_verify_callback (
+ TlsCtx,
+ TlsCertVerify,
+ NULL // "arg": unneeded
+ );
+
return (VOID *) TlsCtx;
}
/**
Free an allocated TLS object.
--
2.19.1.3.g30247aa5d201
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#49034): https://edk2.groups.io/g/devel/message/49034
Mute This Topic: https://groups.io/mt/34551672/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
