Use X509_check_host when available instead of a custom implementation.

X509_check_host was added in OpenSSL 1.0.2, but the Debian 8 line is still
on 1.0.1, so keep our version as a fallback.

The local_ implementation was changed to support wildcard CN values in
3+ label names, because OpenSSL X509_check_host does, too.

Commit migrated from https://github.com/dotnet/corefx/commit/af5a54a64406d40e2108dfc521dcea7867342f69

diff --git a/src/libraries/Common/src/System/Net/Security/CertificateValidation.Unix.cs b/src/libraries/Common/src/System/Net/Security/CertificateValidation.Unix.cs
index a6cb181..7313b72 100644 (file)
--- a/src/libraries/Common/src/System/Net/Security/CertificateValidation.Unix.cs
+++ b/src/libraries/Common/src/System/Net/Security/CertificateValidation.Unix.cs
@@ -45,6 +45,11 @@ namespace System.Net.Security
                     // like "all characters being within [a-z0-9.-]+"
                     string matchName = s_idnMapping.GetAscii(hostName);
                     hostNameMatch = Interop.Crypto.CheckX509Hostname(certHandle, matchName, matchName.Length);
+
+                    if (hostNameMatch < 0)
+                    {
+                        throw Interop.Crypto.CreateOpenSslCryptographicException();
+                    }
                 }
             }
 

diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c
index 0f5ee6d..167de7f 100644 (file)
--- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c
+++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c
@@ -6,6 +6,9 @@
 #include "pal_crypto_types.h"
 #include "pal_types.h"
 
+#include "../Common/pal_safecrt.h"
+#include <assert.h>
+
 #ifdef NEED_OPENSSL_1_0
 
 #include "apibridge.h"
@@ -24,6 +27,8 @@
 #define SSL_ST_OK 3
 #endif
 
+c_static_assert(X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS == 4);
+
 const ASN1_TIME* local_X509_get0_notBefore(const X509* x509)
 {
     if (x509 && x509->cert_info && x509->cert_info->validity)
@@ -479,6 +484,198 @@ int32_t local_SSL_is_init_finished(const SSL* ssl)
     return SSL_state(ssl) == SSL_ST_OK;
 }
 
+/*
+Function:
+CheckX509HostnameMatch
+
+Checks if a particular ASN1_STRING represents the entry in a certificate which would match against
+the requested hostname.
+
+Parameter sanRules: 0 for match rules against the subject CN, 1 for match rules against a SAN entry
+
+Return values:
+1 if the hostname is a match
+0 if the hostname is not a match
+Any negative number indicates an error in the arguments.
+*/
+static int CheckX509HostnameMatch(ASN1_STRING* candidate, const char* hostname, int cchHostname, int typeMatch)
+{
+    assert(candidate != NULL);
+    assert(hostname != NULL);
+
+    if (!candidate->data || !candidate->length)
+    {
+        return 0;
+    }
+
+    // If the candidate is *.example.org then the smallest we would match is a.example.org, which is the same
+    // length. So anything longer than what we're matching against isn't valid.
+
+    // Since the IDNA punycode conversion was applied already this holds even
+    // in Unicode requests.
+    if (candidate->length > cchHostname)
+    {
+        return 0;
+    }
+
+    char* candidateStr;
+    int i;
+    int hostnameFirstDot = -1;
+
+    if (candidate->type != typeMatch)
+    {
+        return 0;
+    }
+
+    // Great, candidateStr is just candidate->data!
+    candidateStr = (char*)(candidate->data);
+
+    // First, verify that the string is alphanumeric, plus hyphens or periods and maybe starting with an asterisk.
+    for (i = 0; i < candidate->length; ++i)
+    {
+        char c = candidateStr[i];
+
+        if ((c < 'A' || c > 'Z') && (c < 'a' || c > 'z') && (c < '0' || c > '9') && (c != '.') && (c != '-') &&
+            (c != '*' || i != 0))
+        {
+            return 0;
+        }
+    }
+
+    if (candidateStr[0] != '*')
+    {
+        if (candidate->length != cchHostname)
+        {
+            return 0;
+        }
+
+        return !strncasecmp((const char*)candidateStr, hostname, (size_t)cchHostname);
+    }
+
+    for (i = 0; i < cchHostname; ++i)
+    {
+        if (hostname[i] == '.')
+        {
+            hostnameFirstDot = i;
+            break;
+        }
+    }
+
+    if (hostnameFirstDot < 0)
+    {
+        // It's possible that this should be considered a match if the entire SAN entry is '*',
+        // aka candidate->length == 1; but nothing talks about this case.
+        return 0;
+    }
+
+    int foundSecondDot = 0;
+
+    for (i = hostnameFirstDot + 1; i < cchHostname; ++i)
+    {
+        if (hostname[i] == '.')
+        {
+            foundSecondDot = 1;
+            break;
+        }
+    }
+
+    // OpenSSL requires two dots for their hostname match.
+    if (!foundSecondDot)
+    {
+        return 0;
+    }
+
+    {
+        // Determine how many characters exist after the portion the wildcard would match. For example,
+        // if hostname is 10 bytes long, and the '.' was at index 3, then we eliminate the first 3
+        // characters (www) from the match constraint.  This forces the wildcard to be the last
+        // character before the . in its match group.
+        int matchLength = cchHostname - hostnameFirstDot;
+
+        // If what's left over from hostname isn't as long as what's left over from the candidate
+        // after the first character was an asterisk, it can't match.
+        if (matchLength != (candidate->length - 1))
+        {
+            return 0;
+        }
+
+        return !strncasecmp(candidateStr + 1, hostname + hostnameFirstDot, (size_t)matchLength);
+    }
+}
+
+int32_t local_X509_check_host(X509* x509, const char* name, size_t namelen, unsigned int flags, char** peername)
+{
+    assert(peername == NULL);
+    assert(flags == X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS);
+    (void)flags;
+    (void)peername;
+
+    GENERAL_NAMES* san = (GENERAL_NAMES*)(X509_get_ext_d2i(x509, NID_subject_alt_name, NULL, NULL));
+    int readSubject = 1;
+    int success = 0;
+
+    // RFC2818 says that if ANY dNSName alternative name field is present then
+    // we should ignore the subject common name.
+
+    if (san != NULL)
+    {
+        int count = sk_GENERAL_NAME_num(san);
+
+        for (int i = 0; i < count; ++i)
+        {
+            GENERAL_NAME* sanEntry = sk_GENERAL_NAME_value(san, i);
+
+            if (sanEntry->type != GEN_DNS)
+            {
+                continue;
+            }
+
+            readSubject = 0;
+
+            // A GEN_DNS name is supposed to be a V_ASN1_IA5STRING.
+            // If it isn't, we don't know how to read it.
+            if (CheckX509HostnameMatch(sanEntry->d.dNSName, name, (int)namelen, V_ASN1_IA5STRING))
+            {
+                success = 1;
+                break;
+            }
+        }
+
+        GENERAL_NAMES_free(san);
+    }
+
+    if (readSubject)
+    {
+        assert(success == 0);
+
+        // This is a shared/interor pointer, do not free!
+        X509_NAME* subject = X509_get_subject_name(x509);
+
+        if (subject != NULL)
+        {
+            int i = -1;
+
+            while ((i = X509_NAME_get_index_by_NID(subject, NID_commonName, i)) >= 0)
+            {
+                // Shared/interior pointers, do not free!
+                X509_NAME_ENTRY* nameEnt = X509_NAME_get_entry(subject, i);
+                ASN1_STRING* cn = X509_NAME_ENTRY_get_data(nameEnt);
+
+                // For compatibility with previous .NET Core builds, allow any type of
+                // string for CN, provided it ended up with a single-byte encoding (otherwise
+                // strncasecmp simply won't match).
+                if (CheckX509HostnameMatch(cn, name, (int)namelen, cn->type))
+                {
+                    success = 1;
+                    break;
+                }
+            }
+        }
+    }
+
+    return success;
+}
+
 X509Stack* local_X509_STORE_CTX_get0_chain(X509_STORE_CTX* ctx)
 {
     return ctx ? ctx->chain : NULL;

diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.h
index c7f4405..5f62864 100644 (file)
--- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.h
+++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.h
@@ -30,6 +30,7 @@ int32_t local_SSL_is_init_finished(const SSL* ssl);
 unsigned long local_SSL_CTX_set_options(SSL_CTX* ctx, unsigned long options);
 void local_SSL_CTX_set_security_level(SSL_CTX* ctx, int32_t level);
 int local_SSL_session_reused(SSL* ssl);
+int32_t local_X509_check_host(X509* x509, const char* name, size_t namelen, unsigned int flags, char** peername);
 const ASN1_TIME* local_X509_CRL_get0_nextUpdate(const X509_CRL* crl);
 int32_t local_X509_NAME_get0_der(X509_NAME* x509Name, const uint8_t** pder, size_t* pderlen);
 int32_t local_X509_PUBKEY_get0_param(

diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/openssl.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/openssl.c
index 9bbd920..82d0011 100644 (file)
--- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/openssl.c
+++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/openssl.c
@@ -705,126 +705,6 @@ BIO* CryptoNative_GetX509NameInfo(X509* x509, int32_t nameType, int32_t forIssue
 
 /*
 Function:
-CheckX509HostnameMatch
-
-Checks if a particular ASN1_STRING represents the entry in a certificate which would match against
-the requested hostname.
-
-Prameter sanRules: 0 for match rules against the subject CN, 1 for match rules against a SAN entry
-
-Return values:
-1 if the hostname is a match
-0 if the hostname is not a match
-Any negative number indicates an error in the arguments.
-*/
-static int CheckX509HostnameMatch(ASN1_STRING* candidate, const char* hostname, int cchHostname, char sanRules)
-{
-    assert(candidate);
-    assert(hostname);
-
-    if (!candidate->data || !candidate->length)
-    {
-        return 0;
-    }
-
-    // If the candidate is *.example.org then the smallest we would match is a.example.org, which is the same
-    // length. So anything longer than what we're matching against isn't valid.
-
-    // Since the IDNA punycode conversion was applied already this holds even
-    // in Unicode requests.
-    if (candidate->length > cchHostname)
-    {
-        return 0;
-    }
-
-    if (sanRules)
-    {
-        // RFC2818 says to use RFC2595 matching rules, but then gives an example that f*.com would match foo.com
-        // RFC2595 says that '*' may be used as the left name component, in which case it is a wildcard that does
-        // not match a '.'.
-        // The recommendation from the Windows Crypto team was not to match f*.com with foo.com.
-
-        char* candidateStr;
-        int i;
-        int hostnameFirstDot = -1;
-
-        // A GEN_DNS name is supposed to be a V_ASN1_IA5STRING.  If it isn't, we don't know how to read it.
-        if (candidate->type != V_ASN1_IA5STRING)
-        {
-            return 0;
-        }
-
-        // Great, candidateStr is just candidate->data!
-        candidateStr = (char*)(candidate->data);
-
-        // First, verify that the string is alphanumeric, plus hyphens or periods and maybe starting with an asterisk.
-        for (i = 0; i < candidate->length; ++i)
-        {
-            char c = candidateStr[i];
-
-            if ((c < 'A' || c > 'Z') && (c < 'a' || c > 'z') && (c < '0' || c > '9') && (c != '.') && (c != '-') &&
-                (c != '*' || i != 0))
-            {
-                return 0;
-            }
-        }
-
-        if (candidateStr[0] != '*')
-        {
-            if (candidate->length != cchHostname)
-            {
-                return 0;
-            }
-
-            return !strncasecmp((const char*)candidateStr, hostname, (size_t)cchHostname);
-        }
-
-        for (i = 0; i < cchHostname; ++i)
-        {
-            if (hostname[i] == '.')
-            {
-                hostnameFirstDot = i;
-                break;
-            }
-        }
-
-        if (hostnameFirstDot < 0)
-        {
-            // It's possible that this should be considered a match if the entire SAN entry is '*',
-            // aka candidate->length == 1; but nothing talks about this case.
-            return 0;
-        }
-
-        {
-            // Determine how many characters exist after the portion the wildcard would match. For example,
-            // if hostname is 10 bytes long, and the '.' was at index 3, then we eliminate the first 3
-            // characters (www) from the match constraint.  This forces the wildcard to be the last
-            // character before the . in its match group.
-            int matchLength = cchHostname - hostnameFirstDot;
-
-            // If what's left over from hostname isn't as long as what's left over from the candidate
-            // after the first character was an asterisk, it can't match.
-            if (matchLength != (candidate->length - 1))
-            {
-                return 0;
-            }
-
-            return !strncasecmp(candidateStr + 1, hostname + hostnameFirstDot, (size_t)matchLength);
-        }
-    }
-
-    // Not SAN-rules, much simpler:
-
-    if (candidate->length != cchHostname)
-    {
-        return 0;
-    }
-
-    return !strncasecmp((const char*)candidate->data, hostname, (size_t)cchHostname);
-}
-
-/*
-Function:
 CheckX509Hostname
 
 Used by System.Net.Security's Unix CertModule to identify if the certificate presented by
@@ -837,73 +717,28 @@ Any negative number indicates an error in the arguments.
 */
 int32_t CryptoNative_CheckX509Hostname(X509* x509, const char* hostname, int32_t cchHostname)
 {
+    // Input errors.  OpenSSL might return -1 or -2, so skip those.
     if (!x509)
-        return -2;
-    if (cchHostname > 0 && !hostname)
         return -3;
-    if (cchHostname < 0)
+    if (cchHostname > 0 && !hostname)
         return -4;
+    if (cchHostname < 0)
+        return -5;
 
-    int subjectNid = NID_commonName;
-    int sanGenType = GEN_DNS;
-    GENERAL_NAMES* san = (GENERAL_NAMES*)(X509_get_ext_d2i(x509, NID_subject_alt_name, NULL, NULL));
-    char readSubject = 1;
-    int success = 0;
-
-    // RFC2818 says that if ANY dNSName alternative name field is present then
-    // we should ignore the subject common name.
-
-    if (san)
-    {
-        int i;
-        int count = sk_GENERAL_NAME_num(san);
-
-        for (i = 0; i < count; ++i)
-        {
-            GENERAL_NAME* sanEntry = sk_GENERAL_NAME_value(san, i);
-
-            if (sanEntry->type != sanGenType)
-            {
-                continue;
-            }
-
-            readSubject = 0;
-
-            if (CheckX509HostnameMatch(sanEntry->d.dNSName, hostname, cchHostname, 1))
-            {
-                success = 1;
-                break;
-            }
-        }
-
-        GENERAL_NAMES_free(san);
-    }
-
-    if (readSubject && !success)
+    // OpenSSL will treat a target hostname starting with '.' as special.
+    // We don't expect target hostnames to start with '.', but if one gets in here, the fallback
+    // and the mainline won't be the same... so just make it report false.
+    if (cchHostname > 0 && hostname[0] == '.')
     {
-        // This is a shared/interor pointer, do not free!
-        X509_NAME* subject = X509_get_subject_name(x509);
-
-        if (subject)
-        {
-            int i = -1;
-
-            while ((i = X509_NAME_get_index_by_NID(subject, subjectNid, i)) >= 0)
-            {
-                // Shared/interior pointers, do not free!
-                X509_NAME_ENTRY* nameEnt = X509_NAME_get_entry(subject, i);
-                ASN1_STRING* cn = X509_NAME_ENTRY_get_data(nameEnt);
-
-                if (CheckX509HostnameMatch(cn, hostname, cchHostname, 0))
-                {
-                    success = 1;
-                    break;
-                }
-            }
-        }
+        return 0;
     }
 
-    return success;
+    return X509_check_host(
+        x509,
+        hostname,
+        (size_t)cchHostname,
+        X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS,
+        NULL);
 }
 
 /*
@@ -984,7 +819,8 @@ int32_t CryptoNative_CheckX509IpAddress(
                 X509_NAME_ENTRY* nameEnt = X509_NAME_get_entry(subject, i);
                 ASN1_STRING* cn = X509_NAME_ENTRY_get_data(nameEnt);
 
-                if (CheckX509HostnameMatch(cn, hostname, cchHostname, 0))
+                if (cn->length == cchHostname &&
+                    !strncasecmp((const char*)cn->data, hostname, (size_t)cchHostname))
                 {
                     success = 1;
                     break;

diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h
index 585ff67..7d88cd7 100644 (file)
--- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h
+++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h
@@ -172,6 +172,9 @@ int32_t X509_up_ref(X509* x509);
 
 #if OPENSSL_VERSION_NUMBER < OPENSSL_VERSION_1_0_2_RTM
 X509_STORE* X509_STORE_CTX_get0_store(X509_STORE_CTX* ctx);
+int32_t X509_check_host(X509* x509, const char* name, size_t namelen, unsigned int flags, char** peername);
+#define X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS 4
+
 #endif
 
 #if !HAVE_OPENSSL_ALPN
@@ -487,6 +490,7 @@ void SSL_get0_alpn_selected(const SSL* ssl, const unsigned char** protocol, unsi
     LEGACY_FUNCTION(SSLeay) \
     RENAMED_FUNCTION(TLS_method, SSLv23_method) \
     REQUIRED_FUNCTION(SSL_write) \
+    FALLBACK_FUNCTION(X509_check_host) \
     REQUIRED_FUNCTION(X509_check_issued) \
     REQUIRED_FUNCTION(X509_check_purpose) \
     REQUIRED_FUNCTION(X509_cmp_current_time) \
@@ -877,6 +881,7 @@ FOR_ALL_OPENSSL_FUNCTIONS
 #define SSLeay SSLeay_ptr
 #define SSL_write SSL_write_ptr
 #define TLS_method TLS_method_ptr
+#define X509_check_host X509_check_host_ptr
 #define X509_check_issued X509_check_issued_ptr
 #define X509_check_purpose X509_check_purpose_ptr
 #define X509_cmp_current_time X509_cmp_current_time_ptr
@@ -1034,6 +1039,7 @@ FOR_ALL_OPENSSL_FUNCTIONS
 
 #if OPENSSL_VERSION_NUMBER < OPENSSL_VERSION_1_0_2_RTM
 
+#define X509_check_host local_X509_check_host
 #define X509_STORE_CTX_get0_store local_X509_STORE_CTX_get0_store
 
 #endif

diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/HostnameMatchTests.Unix.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/HostnameMatchTests.Unix.cs
index 2a3b8b1..b6acc0d 100644 (file)
--- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/HostnameMatchTests.Unix.cs
+++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/HostnameMatchTests.Unix.cs
@@ -16,7 +16,20 @@ namespace System.Security.Cryptography.X509Certificates.Tests
         [InlineData(false, true)]
         [InlineData(true, false)]
         [InlineData(true, true)]
-        public static void MatchCN_NoWildcards(bool wantsWildcard, bool mixedCase)
+        public static void MatchCN_ThreeLabels(bool wantsWildcard, bool mixedCase)
+        {
+            string targetName = "LocalHost.loCAldoMaIn.exAmple";
+            string subjectCN = wantsWildcard ? "*.LOcaLdomain.exAMPle" : targetName;
+
+            RunTest(targetName, subjectCN, null, !mixedCase, true);
+        }
+
+        [Theory]
+        [InlineData(false, false)]
+        [InlineData(false, true)]
+        [InlineData(true, false)]
+        [InlineData(true, true)]
+        public static void MatchCN_TwoLabels(bool wantsWildcard, bool mixedCase)
         {
             string targetName = "LocalHost.loCAldoMaIn";
             string subjectCN = wantsWildcard ? "*.LOcaLdomain" : targetName;