From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
"lersek@redhat.com" <lersek@redhat.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>,
David Woodhouse <dwmw2@infradead.org>,
"Wang, Jian J" <jian.j.wang@intel.com>,
"Richard Levitte" <levitte@openssl.org>,
Sivaraman Nainar <sivaramann@amiindia.co.in>
Subject: Re: [edk2-devel] [RFC v1 5/4] CryptoPkg/TlsLib: accept peer certs via both DNS names and IP addresses
Date: Wed, 16 Oct 2019 05:18:28 +0000 [thread overview]
Message-ID: <895558F6EA4E3B41AC93A00D163B727416F81251@SHSMSX107.ccr.corp.intel.com> (raw)
In-Reply-To: <20191015230839.27708-1-lersek@redhat.com>
These days I'm too busy with other things. Just take the time to review the email discussion & researched the correct behavior of HTTPS cert verification.
I did never though my patch caused the function regression, and I'm also not loop in the long ago email discussion reported from AMI.
Now, I understand the testing *gap* between us is that we set the IP address string in the CN or dNSName filed of SAN when creating the certificate, while you don't prefer an IP address string setting in dNSName but setting the OCTET string to iPAddress of SAN. That's why the failure happen with different certificates setting (I've explained it many times).
With my patches to UEFI part, I believe you can pass the test if setting the IP in CN or dNSName of SAN, but no one shared me any significant evidence to indicate it's the mistake idea if URI is specified as an IP address rather than a Hostname. On the contrary, my explains to my thought/viewpoint when creating the patch was treated as poor excuse:(.
Now, I post my own investigation here to correct my mistake thought:
According my previous experiment and RFC5280, the dNSName of subjectAltName is IA5String, which means the string of an IPv4 or IPv6 address is also legal to be included in the dNSName:
SubjectAltName ::= GeneralNames
GeneralName ::= CHOICE {
otherName [0] OtherName,
rfc822Name [1] IA5String,
dNSName [2] IA5String,
x400Address [3] ORAddress,
directoryName [4] Name,
ediPartyName [5] EDIPartyName,
uniformResourceIdentifier [6] IA5String,
iPAddress [7] OCTET STRING,
registeredID [8] OBJECT IDENTIFIER }
If so, I thought it should be doable and make sense to treat the IP in URI as hostname in CN or dNSName of SAN. Besides, it can also pass the verification if the certificate only has the CN (NO SAN case, I think it's also the failure case Laszlo met in Note(2)). I believed it's the implementation choice before if no below finding in RFC 6125. But with the series discussion here, I'm starting to wonder whether there is any RFC or document to restrict the TLS & certificate checking roles , for example:
1) IP in the URI can be treated as Hostname or it only can be used for the iPAddress of SAN verification?
2) Certificate must have the iPAddress or dNSName of SAN?
Fortunately, I get my wanted answer in RFC6125, SECTION 3.1 :
If a subjectAltName extension of type dNSName is present, that MUST
be used as the identity. Otherwise, the (most specific) Common Name
field in the Subject field of the certificate MUST be used. Although
the use of the Common Name is existing practice, it is deprecated and
Certification Authorities are encouraged to use the dNSName instead.
...
In some cases, the URI is specified as an IP address rather than a
Hostname . In this case, the iPAddress subjectAltName must be present
in the certificate and must exactly match the IP in the URI.
So, the question is more clear here:
1) IP in the URI is used for the iPAddress of SAN verification, but *not say no* if upper driver set to dNSName of SAN.
2) iPAddress of SAN MUST be present in the certificate and must exactly match the IP in the URI. If not, we can treat as failure.
Ok, with above declaim in RFC, I agree there is the function issue in my series patches to treat IP address in URI as string setting to hostname, which breaks the above 2 rules defined in RFC 6125.
Now, to resolve the problem, I think the best compatibility can actually be reached by setting the IP address both as iPAddress and dNSName in SAN. To achieve that, we have two methods:
1) TLS provides the separate interfaces to upper driver to set the iPAddress & dNSName (even email address), which means HTTPS driver need follow RFC6125. But that will need spec changes.
2) Just as David's suggestion and the patch send out by Laszlo, fix it in UEFI TLS part. I reviewed the existing UEFI 2.8, looks we can treat the HostName in Spec as the widely name (can be IP, DNS or email address name).
To make things simple & easier, I also prefer 2). So, I reviewed the patch from Laszlo:
Comment1: How about setting the IP address both as iPAddress and dNSName in SAN for the compatibility, if so, I think below note2 failure will be gone.
Comment2: do we really need the app_verify_callback function setting? Why not call X509_VERIFY_PARAM_set1_ip_asc (TlsConn->Ssl->param, HostName) in TlsSetVerifyHost directly? anything I missed in the discussion?
Please correct me if anything wrong.
Thanks,
Jiaxin
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
> Ersek
> Sent: Wednesday, October 16, 2019 7:09 AM
> To: edk2-devel-groups-io <devel@edk2.groups.io>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; David Woodhouse
> <dwmw2@infradead.org>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Jiaxin
> <jiaxin.wu@intel.com>; Richard Levitte <levitte@openssl.org>; Sivaraman
> Nainar <sivaramann@amiindia.co.in>
> Subject: [edk2-devel] [RFC v1 5/4] CryptoPkg/TlsLib: accept peer certs via
> both DNS names and IP addresses
>
> 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.Barkelew@microsoft.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Richard Levitte <levitte@openssl.org>
> Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
> Ref: http://mid.mail-
> archive.com/B4DE137BDB63634BAC03BD9DE765F197028B24CA23@VENUS1.i
> n.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 <dwmw2@infradead.org>
> Signed-off-by: Laszlo Ersek <lersek@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
> <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/1787330
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [jiaxin.wu@intel.com]
> -=-=-=-=-=-=
next prev parent reply other threads:[~2019-10-16 5:18 UTC|newest]
Thread overview: 61+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-27 3:44 [PATCH v1 0/4] Support HTTPS HostName validation feature(CVE-2019-14553) Wu, Jiaxin
2019-09-27 3:44 ` [PATCH v1 1/4] MdePkg/Include/Protocol/Tls.h: Add the data type of EfiTlsVerifyHost(CVE-2019-14553) Wu, Jiaxin
2019-09-27 3:44 ` [PATCH v1 2/4] CryptoPkg/TlsLib: Add the new API "TlsSetVerifyHost"(CVE-2019-14553) Wu, Jiaxin
2019-09-27 3:44 ` [PATCH v1 3/4] NetworkPkg/TlsDxe: Add the support of host validation to TlsDxe driver(CVE-2019-14553) Wu, Jiaxin
2019-09-27 3:44 ` [PATCH v1 4/4] NetworkPkg/HttpDxe: Set the HostName for the verification(CVE-2019-14553) Wu, Jiaxin
2019-09-29 6:09 ` [edk2-devel] [PATCH v1 0/4] Support HTTPS HostName validation feature(CVE-2019-14553) Wang, Jian J
2019-09-30 23:21 ` Laszlo Ersek
2019-10-01 9:02 ` David Woodhouse
2019-10-08 6:19 ` Wu, Jiaxin
2019-10-09 7:53 ` David Woodhouse
2019-10-09 20:24 ` Laszlo Ersek
2019-10-09 20:34 ` David Woodhouse
2019-10-10 3:11 ` Wu, Jiaxin
2019-10-10 8:00 ` Laszlo Ersek
2019-10-10 15:45 ` David Woodhouse
2019-10-10 18:03 ` Laszlo Ersek
2019-10-11 2:24 ` Wu, Jiaxin
2019-10-11 6:58 ` David Woodhouse
2019-10-11 8:04 ` Wu, Jiaxin
2019-10-11 10:55 ` Laszlo Ersek
2019-10-11 11:16 ` David Woodhouse
2019-10-11 15:36 ` Laszlo Ersek
2019-10-11 16:01 ` David Woodhouse
2019-10-14 16:15 ` Laszlo Ersek
2019-10-14 16:20 ` Laszlo Ersek
2019-10-14 16:53 ` David Woodhouse
2019-10-15 11:03 ` David Woodhouse
2019-10-15 11:06 ` David Woodhouse
2019-10-15 13:54 ` Laszlo Ersek
2019-10-15 15:29 ` David Woodhouse
2019-10-15 16:56 ` Laszlo Ersek
2019-10-15 17:34 ` Laszlo Ersek
2019-10-16 9:40 ` David Woodhouse
2019-10-16 10:27 ` Laszlo Ersek
2019-10-15 15:57 ` David Woodhouse
2019-10-15 17:28 ` Laszlo Ersek
2019-10-10 2:45 ` Wu, Jiaxin
2019-10-09 15:54 ` Laszlo Ersek
2019-10-10 2:46 ` Wu, Jiaxin
2019-10-15 23:08 ` [RFC v1 5/4] CryptoPkg/TlsLib: accept peer certs via both DNS names and IP addresses Laszlo Ersek
2019-10-16 5:18 ` Wu, Jiaxin [this message]
2019-10-16 7:36 ` [edk2-devel] " Laszlo Ersek
2019-10-16 7:54 ` Laszlo Ersek
2019-10-16 7:56 ` David Woodhouse
2019-10-16 8:08 ` Laszlo Ersek
2019-10-16 9:19 ` David Woodhouse
2019-10-16 11:41 ` Laszlo Ersek
2019-10-16 13:35 ` David Woodhouse
2019-10-16 14:43 ` Laszlo Ersek
2019-10-16 15:25 ` David Woodhouse
2019-10-17 15:35 ` Laszlo Ersek
2019-10-17 15:49 ` David Woodhouse
2019-10-18 13:25 ` Laszlo Ersek
2019-10-25 2:12 ` Wu, Jiaxin
2019-10-25 8:14 ` Laszlo Ersek
2019-10-24 19:47 ` Laszlo Ersek
2019-10-25 2:13 ` Wu, Jiaxin
2019-10-25 2:12 ` Wu, Jiaxin
2019-10-25 2:12 ` Wu, Jiaxin
2019-10-16 8:45 ` David Woodhouse
2019-10-16 11:01 ` David Woodhouse
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=895558F6EA4E3B41AC93A00D163B727416F81251@SHSMSX107.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox