Remove non-standard X.509 DNS wildcard matching.
Always enable X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS and never enable
X509_CHECK_FLAG_MULTI_LABEL_WILDCARDS.
Update-Note: BoringSSL will no longer accept wildcard patterns like
*www.example.com or www*.example.com. (It already did not accept
ww*w.example.com.) X509_CHECK_FLAG_MULTI_LABEL_WILDCARDS will also be
ignored and can no longer be used to allow foo.bar.example.com to match
*.example.com.
Fixes: 462
Change-Id: I004e087bf70f4c3f249235cd864d9e19cc9a5102
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50705
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/x509v3/v3_utl.c b/crypto/x509v3/v3_utl.c
index 5d91aed..7e7c549 100644
--- a/crypto/x509v3/v3_utl.c
+++ b/crypto/x509v3/v3_utl.c
@@ -821,7 +821,6 @@
const unsigned char *wildcard_start;
const unsigned char *wildcard_end;
const unsigned char *p;
- int allow_multi = 0;
int allow_idna = 0;
if (subject_len < prefix_len + suffix_len)
@@ -840,8 +839,6 @@
if (wildcard_start == wildcard_end)
return 0;
allow_idna = 1;
- if (flags & X509_CHECK_FLAG_MULTI_LABEL_WILDCARDS)
- allow_multi = 1;
}
/* IDNA labels cannot match partial wildcards */
if (!allow_idna &&
@@ -853,14 +850,13 @@
return 1;
/*
* Check that the part matched by the wildcard contains only
- * permitted characters and only matches a single label unless
- * allow_multi is set.
+ * permitted characters and only matches a single label.
*/
for (p = wildcard_start; p != wildcard_end; ++p)
if (!(('0' <= *p && *p <= '9') ||
('A' <= *p && *p <= 'Z') ||
('a' <= *p && *p <= 'z') ||
- *p == '-' || (allow_multi && *p == '.')))
+ *p == '-'))
return 0;
return 1;
}
@@ -892,12 +888,8 @@
*/
if (star != NULL || (state & LABEL_IDNA) != 0 || dots)
return NULL;
- /* Only full-label '*.example.com' wildcards? */
- if ((flags & X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS)
- && (!atstart || !atend))
- return NULL;
- /* No 'foo*bar' wildcards */
- if (!atstart && !atend)
+ /* Only full-label '*.example.com' wildcards. */
+ if (!atstart || !atend)
return NULL;
star = &p[i];
state &= ~LABEL_START;
diff --git a/include/openssl/x509v3.h b/include/openssl/x509v3.h
index acff637..3e8cf1b 100644
--- a/include/openssl/x509v3.h
+++ b/include/openssl/x509v3.h
@@ -890,10 +890,12 @@
#define X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT 0
// Disable wildcard matching for dnsName fields and common name.
#define X509_CHECK_FLAG_NO_WILDCARDS 0x2
-// Wildcards must not match a partial label.
-#define X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS 0x4
-// Allow (non-partial) wildcards to match multiple labels.
-#define X509_CHECK_FLAG_MULTI_LABEL_WILDCARDS 0x8
+// X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS does nothing, but is necessary in
+// OpenSSL to enable standard wildcard matching. In BoringSSL, this behavior is
+// always enabled.
+#define X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS 0
+// Deprecated: this flag does nothing
+#define X509_CHECK_FLAG_MULTI_LABEL_WILDCARDS 0
// Constraint verifier subdomain patterns to match a single labels.
#define X509_CHECK_FLAG_SINGLE_LABEL_SUBDOMAINS 0x10
// Skip the subject common name fallback if subjectAltNames is missing.
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 6ef80cb..d0bd680 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -8012,76 +8012,99 @@
}
TEST(SSLTest, HostMatching) {
- static const char kCertPEM[] =
- "-----BEGIN CERTIFICATE-----\n"
- "MIIB9jCCAZ2gAwIBAgIQeudG9R61BOxUvWkeVhU5DTAKBggqhkjOPQQDAjApMRAw\n"
- "DgYDVQQKEwdBY21lIENvMRUwEwYDVQQDEwxleGFtcGxlMy5jb20wHhcNMjExMjA2\n"
- "MjA1NjU2WhcNMjIxMjA2MjA1NjU2WjApMRAwDgYDVQQKEwdBY21lIENvMRUwEwYD\n"
- "VQQDEwxleGFtcGxlMy5jb20wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAS7l2VO\n"
- "Bl2TjVm9WfGk24+hMbVFUNB+RVHWbCvFvNZAoWiIJ2z34RLGInyZvCZ8xLAvsuaW\n"
- "ULDDaoeDl1M0t4Hmo4GmMIGjMA4GA1UdDwEB/wQEAwIChDATBgNVHSUEDDAKBggr\n"
- "BgEFBQcDATAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBTTJWurcc1t+VPQBko3\n"
- "Gsw6cbcWSTBMBgNVHREERTBDggxleGFtcGxlMS5jb22CDGV4YW1wbGUyLmNvbYIP\n"
- "YSouZXhhbXBsZTQuY29tgg4qLmV4YW1wbGU1LmNvbYcEAQIDBDAKBggqhkjOPQQD\n"
- "AgNHADBEAiAAv0ljHJGrgyzZDkG6XvNZ5ewxRfnXcZuD0Y7E4giCZgIgNK1qjilu\n"
- "5DyVbfKeeJhOCtGxqE1dWLXyJBnoRomSYBY=\n"
- "-----END CERTIFICATE-----\n";
+ static const char kCertPEM[] = R"(
+-----BEGIN CERTIFICATE-----
+MIIB9jCCAZ2gAwIBAgIQeudG9R61BOxUvWkeVhU5DTAKBggqhkjOPQQDAjApMRAw
+DgYDVQQKEwdBY21lIENvMRUwEwYDVQQDEwxleGFtcGxlMy5jb20wHhcNMjExMjA2
+MjA1NjU2WhcNMjIxMjA2MjA1NjU2WjApMRAwDgYDVQQKEwdBY21lIENvMRUwEwYD
+VQQDEwxleGFtcGxlMy5jb20wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAS7l2VO
+Bl2TjVm9WfGk24+hMbVFUNB+RVHWbCvFvNZAoWiIJ2z34RLGInyZvCZ8xLAvsuaW
+ULDDaoeDl1M0t4Hmo4GmMIGjMA4GA1UdDwEB/wQEAwIChDATBgNVHSUEDDAKBggr
+BgEFBQcDATAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBTTJWurcc1t+VPQBko3
+Gsw6cbcWSTBMBgNVHREERTBDggxleGFtcGxlMS5jb22CDGV4YW1wbGUyLmNvbYIP
+YSouZXhhbXBsZTQuY29tgg4qLmV4YW1wbGU1LmNvbYcEAQIDBDAKBggqhkjOPQQD
+AgNHADBEAiAAv0ljHJGrgyzZDkG6XvNZ5ewxRfnXcZuD0Y7E4giCZgIgNK1qjilu
+5DyVbfKeeJhOCtGxqE1dWLXyJBnoRomSYBY=
+-----END CERTIFICATE-----
+)";
bssl::UniquePtr<X509> cert(CertFromPEM(kCertPEM));
ASSERT_TRUE(cert);
+ static const char kCertNoSANsPEM[] = R"(
+-----BEGIN CERTIFICATE-----
+MIIBqzCCAVGgAwIBAgIQeudG9R61BOxUvWkeVhU5DTAKBggqhkjOPQQDAjArMRIw
+EAYDVQQKEwlBY21lIENvIDIxFTATBgNVBAMTDGV4YW1wbGUzLmNvbTAeFw0yMTEy
+MDYyMDU2NTZaFw0yMjEyMDYyMDU2NTZaMCsxEjAQBgNVBAoTCUFjbWUgQ28gMjEV
+MBMGA1UEAxMMZXhhbXBsZTMuY29tMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE
+u5dlTgZdk41ZvVnxpNuPoTG1RVDQfkVR1mwrxbzWQKFoiCds9+ESxiJ8mbwmfMSw
+L7LmllCww2qHg5dTNLeB5qNXMFUwDgYDVR0PAQH/BAQDAgKEMBMGA1UdJQQMMAoG
+CCsGAQUFBwMBMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYEFNMla6txzW35U9AG
+SjcazDpxtxZJMAoGCCqGSM49BAMCA0gAMEUCIG3YWGWtpVhbcGV7wFKQwTfmvwHW
+pw4qCFZlool4hCwsAiEA+2fc6NfSbNpFEtQkDOMJW2ANiScAVEmImNqPfb2klz4=
+-----END CERTIFICATE-----
+)";
+ bssl::UniquePtr<X509> cert_no_sans(CertFromPEM(kCertNoSANsPEM));
+ ASSERT_TRUE(cert_no_sans);
- static const char kKeyPEM[] =
- "-----BEGIN PRIVATE KEY-----\n"
- "MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQghsaSZhUzZAcQlLyJ\n"
- "MDuy7WPdyqNsAX9rmEP650LF/q2hRANCAAS7l2VOBl2TjVm9WfGk24+hMbVFUNB+\n"
- "RVHWbCvFvNZAoWiIJ2z34RLGInyZvCZ8xLAvsuaWULDDaoeDl1M0t4Hm\n"
- "-----END PRIVATE KEY-----\n";
+ static const char kKeyPEM[] = R"(
+-----BEGIN PRIVATE KEY-----
+MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQghsaSZhUzZAcQlLyJ
+MDuy7WPdyqNsAX9rmEP650LF/q2hRANCAAS7l2VOBl2TjVm9WfGk24+hMbVFUNB+
+RVHWbCvFvNZAoWiIJ2z34RLGInyZvCZ8xLAvsuaWULDDaoeDl1M0t4Hm
+-----END PRIVATE KEY-----
+)";
bssl::UniquePtr<EVP_PKEY> key(KeyFromPEM(kKeyPEM));
ASSERT_TRUE(key);
- bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method()));
- ASSERT_TRUE(server_ctx);
- ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), cert.get()));
- ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()));
-
bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
ASSERT_TRUE(client_ctx);
ASSERT_TRUE(X509_STORE_add_cert(SSL_CTX_get_cert_store(client_ctx.get()),
cert.get()));
+ ASSERT_TRUE(X509_STORE_add_cert(SSL_CTX_get_cert_store(client_ctx.get()),
+ cert_no_sans.get()));
SSL_CTX_set_verify(client_ctx.get(),
SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT,
nullptr);
struct TestCase {
+ X509 *cert;
std::string hostname;
unsigned flags;
bool should_match;
};
std::vector<TestCase> kTests = {
// These two names are present as SANs in the certificate.
- {"example1.com", 0, true},
- {"example2.com", 0, true},
+ {cert.get(), "example1.com", 0, true},
+ {cert.get(), "example2.com", 0, true},
// This is the CN of the certificate, but that shouldn't matter if a SAN
// extension is present.
- {"example3.com", 0, false},
- // a*.example4.com is a SAN, which is invalid because partial wildcards
- // aren't a thing except for the OpenSSL verifier.
- {"abc.example4.com", 0, true},
- // ... but they can be turned off.
- {"abc.example4.com", X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS, false},
+ {cert.get(), "example3.com", 0, false},
+ // If the SAN is not present, we, for now, look for DNS names in the CN.
+ {cert_no_sans.get(), "example3.com", 0, true},
+ // ... but this can be turned off.
+ {cert_no_sans.get(), "example3.com", X509_CHECK_FLAG_NEVER_CHECK_SUBJECT,
+ false},
+ // a*.example4.com is a SAN, but is invalid.
+ {cert.get(), "abc.example4.com", 0, false},
// *.example5.com is a SAN in the certificate, which is a normal and valid
// wildcard.
- {"abc.example5.com", 0, true},
+ {cert.get(), "abc.example5.com", 0, true},
// This name is not present.
- {"notexample1.com", 0, false},
+ {cert.get(), "notexample1.com", 0, false},
// The IPv4 address 1.2.3.4 is a SAN, but that shouldn't match against a
// hostname that happens to be its textual representation.
- {"1.2.3.4", 0, false},
+ {cert.get(), "1.2.3.4", 0, false},
};
- bssl::UniquePtr<SSL> client, server;
- ClientConfig config;
for (const TestCase &test : kTests) {
SCOPED_TRACE(test.hostname);
+
+ bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method()));
+ ASSERT_TRUE(server_ctx);
+ ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), test.cert));
+ ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()));
+
+ ClientConfig config;
+ bssl::UniquePtr<SSL> client, server;
config.verify_hostname = test.hostname;
config.hostflags = test.flags;
EXPECT_EQ(test.should_match,
