Add X509_CHECK_FLAG_NEVER_CHECK_SUBJECT.
cryptography.io uses this and it's also the correct behavior. Ideally it would
be default, but start with just adding the flag. See also
dd60efea955e41a6f0926f93ec1503c6f83c4e58 from upstream.
Change-Id: I9e13cdbfd44c904ba5bd69a5a66c68c4b7596867
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35645
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index a53ed7a..ca86ef4 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -660,6 +660,39 @@
"KStYq7X9PKseN+PvmfeoffIKc5R/Ha39oi7cGMVHCr8aiEhsf94=\n"
"-----END CERTIFICATE-----\n";
+// kCommonNameWithSANs is a leaf certificate signed by kSANTypesRoot, with
+// *.host1.test as the common name and a SAN list of *.host2.test and
+// foo.host3.test.
+static const char kCommonNameWithSANs[] =
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIIB2zCCAUSgAwIBAgIBAzANBgkqhkiG9w0BAQsFADArMRcwFQYDVQQKEw5Cb3Jp\n"
+ "bmdTU0wgVGVzdDEQMA4GA1UEAxMHUm9vdCBDQTAgFw0wMDAxMDEwMDAwMDBaGA8y\n"
+ "MDk5MDEwMTAwMDAwMFowNzEeMBwGA1UEChMVQ29tbW9uIG5hbWUgd2l0aCBTQU5z\n"
+ "MRUwEwYDVQQDDAwqLmhvc3QxLnRlc3QwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNC\n"
+ "AASgWzfnFnpQrokSLIC+LhCKJDUAY/2usfIDpOnafYoYCasbYetkmOslgyY4Nn07\n"
+ "zjvjNROprA/0bdULXAkdL9bNo0gwRjAbBgNVHSMEFDASgBBAN9cB+0AvuBx+VAQn\n"
+ "jFkBMCcGA1UdEQQgMB6CDCouaG9zdDIudGVzdIIOZm9vLmhvc3QzLnRlc3QwDQYJ\n"
+ "KoZIhvcNAQELBQADgYEAtv2e3hBhsslXB1HTxgusjoschWOVtvGZUaYlhkKzKTCL\n"
+ "4YpDn50BccnucBU/b9phYvaEZtyzOv4ZXhxTGyLnLrIVB9x5ikfCcfl+LNYNjDwM\n"
+ "enm/h1zOfJ7wXLyscD4kU29Wc/zxBd70thIgLYn16CC1S9NtXKsXXDXv5VVH/bg=\n"
+ "-----END CERTIFICATE-----\n";
+
+// kCommonNameWithSANs is a leaf certificate signed by kSANTypesRoot, with
+// *.host1.test as the common name and no SAN list.
+static const char kCommonNameWithoutSANs[] =
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIIBtTCCAR6gAwIBAgIBAzANBgkqhkiG9w0BAQsFADArMRcwFQYDVQQKEw5Cb3Jp\n"
+ "bmdTU0wgVGVzdDEQMA4GA1UEAxMHUm9vdCBDQTAgFw0wMDAxMDEwMDAwMDBaGA8y\n"
+ "MDk5MDEwMTAwMDAwMFowOjEhMB8GA1UEChMYQ29tbW9uIG5hbWUgd2l0aG91dCBT\n"
+ "QU5zMRUwEwYDVQQDDAwqLmhvc3QxLnRlc3QwWTATBgcqhkjOPQIBBggqhkjOPQMB\n"
+ "BwNCAARt2vjlIrPE+kr11VS1rRP/AYQu4fvf1bNw/K9rwYlVBhmLMPYasEmpCtKE\n"
+ "0bDIFydtDYC3wZDpSS+YiaG40sdAox8wHTAbBgNVHSMEFDASgBBAN9cB+0AvuBx+\n"
+ "VAQnjFkBMA0GCSqGSIb3DQEBCwUAA4GBAHRbIeaCEytOpJpw9O2dlB656AHe1+t5\n"
+ "4JiS5mvtzoVOLn7fFk5EFQtZS7sG1Uc2XjlSw+iyvFoTFEqfKyU/mIdc2vBuPwA2\n"
+ "+YXT8aE4S+UZ9oz5j0gDpikGnkSCW0cyHD8L8fntNjaQRSaM482JpmtdmuxClmWO\n"
+ "pFFXI2B5usgI\n"
+ "-----END CERTIFICATE-----\n";
+
// CertFromPEM parses the given, NUL-terminated pem block and returns an
// |X509*|.
static bssl::UniquePtr<X509> CertFromPEM(const char *pem) {
@@ -1744,3 +1777,61 @@
ASSERT_TRUE(cert2);
EXPECT_EQ(0, X509_cmp(cert.get(), cert2.get()));
}
+
+TEST(X509Test, CommonNameFallback) {
+ bssl::UniquePtr<X509> root = CertFromPEM(kSANTypesRoot);
+ ASSERT_TRUE(root);
+ bssl::UniquePtr<X509> with_sans = CertFromPEM(kCommonNameWithSANs);
+ ASSERT_TRUE(with_sans);
+ bssl::UniquePtr<X509> without_sans = CertFromPEM(kCommonNameWithoutSANs);
+ ASSERT_TRUE(without_sans);
+
+ auto verify_cert = [&](X509 *leaf, unsigned flags, const char *host) {
+ return Verify(
+ leaf, {root.get()}, {}, {}, 0, false, [&](X509_VERIFY_PARAM *param) {
+ ASSERT_TRUE(X509_VERIFY_PARAM_set1_host(param, host, strlen(host)));
+ X509_VERIFY_PARAM_set_hostflags(param, flags);
+ });
+ };
+
+ // By default, the common name is ignored if the SAN list is present but
+ // otherwise is checked.
+ EXPECT_EQ(X509_V_ERR_HOSTNAME_MISMATCH,
+ verify_cert(with_sans.get(), 0 /* no flags */, "foo.host1.test"));
+ EXPECT_EQ(X509_V_OK,
+ verify_cert(with_sans.get(), 0 /* no flags */, "foo.host2.test"));
+ EXPECT_EQ(X509_V_OK,
+ verify_cert(with_sans.get(), 0 /* no flags */, "foo.host3.test"));
+ EXPECT_EQ(X509_V_OK, verify_cert(without_sans.get(), 0 /* no flags */,
+ "foo.host1.test"));
+
+ // X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT causes the common name to always be
+ // checked.
+ EXPECT_EQ(X509_V_OK,
+ verify_cert(with_sans.get(), X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT,
+ "foo.host1.test"));
+ EXPECT_EQ(X509_V_OK,
+ verify_cert(with_sans.get(), X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT,
+ "foo.host2.test"));
+ EXPECT_EQ(X509_V_OK,
+ verify_cert(with_sans.get(), X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT,
+ "foo.host3.test"));
+ EXPECT_EQ(X509_V_OK, verify_cert(without_sans.get(),
+ X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT,
+ "foo.host1.test"));
+
+ // X509_CHECK_FLAG_NEVER_CHECK_SUBJECT implements the correct behavior: the
+ // common name is never checked.
+ EXPECT_EQ(X509_V_ERR_HOSTNAME_MISMATCH,
+ verify_cert(with_sans.get(), X509_CHECK_FLAG_NEVER_CHECK_SUBJECT,
+ "foo.host1.test"));
+ EXPECT_EQ(X509_V_OK,
+ verify_cert(with_sans.get(), X509_CHECK_FLAG_NEVER_CHECK_SUBJECT,
+ "foo.host2.test"));
+ EXPECT_EQ(X509_V_OK,
+ verify_cert(with_sans.get(), X509_CHECK_FLAG_NEVER_CHECK_SUBJECT,
+ "foo.host3.test"));
+ EXPECT_EQ(X509_V_ERR_HOSTNAME_MISMATCH,
+ verify_cert(without_sans.get(), X509_CHECK_FLAG_NEVER_CHECK_SUBJECT,
+ "foo.host1.test"));
+}
diff --git a/crypto/x509v3/v3_utl.c b/crypto/x509v3/v3_utl.c
index 2a293dc..b38f49f 100644
--- a/crypto/x509v3/v3_utl.c
+++ b/crypto/x509v3/v3_utl.c
@@ -1003,14 +1003,12 @@
GENERAL_NAMES_free(gens);
if (rv != 0)
return rv;
- if (cnid == NID_undef
- || (san_present
- && !(flags & X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT)))
+ if (san_present && !(flags & X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT))
return 0;
}
/* We're done if CN-ID is not pertinent */
- if (cnid == NID_undef)
+ if (cnid == NID_undef || (flags & X509_CHECK_FLAG_NEVER_CHECK_SUBJECT))
return 0;
j = -1;
diff --git a/include/openssl/x509v3.h b/include/openssl/x509v3.h
index d2d39f8..77af8c3 100644
--- a/include/openssl/x509v3.h
+++ b/include/openssl/x509v3.h
@@ -713,6 +713,8 @@
#define X509_CHECK_FLAG_MULTI_LABEL_WILDCARDS 0x8
/* 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. */
+#define X509_CHECK_FLAG_NEVER_CHECK_SUBJECT 0x20
/*
* Match reference identifiers starting with "." to any sub-domain.
* This is a non-public flag, turned on implicitly when the subject
