Disable the common name fallback on *any* SAN list.
This aligns with the Go crypto/x509 behavior and reduces the cases when
the SAN to CN fallback occurs. If the certificate is new enough to have
a SAN list, even if it only contains email or IP addresses, it is
reasonable to assume the certificate is new enough that the common name
is not a DNS name.
Update-Note: Our certificate verification is getting slightly stricter.
Change-Id: I9e3466d8dd8a722405c546181a589f797efa43f9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35647
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index 93bb582..517635e 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -693,6 +693,40 @@
"pFFXI2B5usgI\n"
"-----END CERTIFICATE-----\n";
+// kCommonNameWithEmailSAN is a leaf certificate signed by kSANTypesRoot, with
+// *.host1.test as the common name and the email address test@host2.test in the
+// SAN list.
+static const char kCommonNameWithEmailSAN[] =
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIIBvDCCASWgAwIBAgIBAjANBgkqhkiG9w0BAQsFADArMRcwFQYDVQQKEw5Cb3Jp\n"
+ "bmdTU0wgVGVzdDEQMA4GA1UEAxMHUm9vdCBDQTAgFw0wMDAxMDEwMDAwMDBaGA8y\n"
+ "MDk5MDEwMTAwMDAwMFowFzEVMBMGA1UEAwwMKi5ob3N0MS50ZXN0MFkwEwYHKoZI\n"
+ "zj0CAQYIKoZIzj0DAQcDQgAEtevOxcTjpPzlNGoUMFfZyr1k03/Hiuh+EsnuScDs\n"
+ "8XLKi6fDkvSaDClI99ycabQZRPIrvyT+dglDC6ugQd+CYqNJMEcwDAYDVR0TAQH/\n"
+ "BAIwADAbBgNVHSMEFDASgBBAN9cB+0AvuBx+VAQnjFkBMBoGA1UdEQQTMBGBD3Rl\n"
+ "c3RAaG9zdDIudGVzdDANBgkqhkiG9w0BAQsFAAOBgQCGbqb78OWJWl4zb+qw0Dz2\n"
+ "HJgZZJt6/+nNG/XJKdaYeS4eofsbwsJI4fuuOF6ZvYCJxVNtGqdfZDgycvFA9hjv\n"
+ "NGosBF1/spP17cmzTahLjxs71jDvHV/EQJbKGl/Zpta1Em1VrzSrwoOFabPXzZTJ\n"
+ "aet/mER21Z/9ZsTUoJQPJw==\n"
+ "-----END CERTIFICATE-----\n";
+
+// kCommonNameWithIPSAN is a leaf certificate signed by kSANTypesRoot, with
+// *.host1.test as the common name and the IP address 127.0.0.1 in the
+// SAN list.
+static const char kCommonNameWithIPSAN[] =
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIIBsTCCARqgAwIBAgIBAjANBgkqhkiG9w0BAQsFADArMRcwFQYDVQQKEw5Cb3Jp\n"
+ "bmdTU0wgVGVzdDEQMA4GA1UEAxMHUm9vdCBDQTAgFw0wMDAxMDEwMDAwMDBaGA8y\n"
+ "MDk5MDEwMTAwMDAwMFowFzEVMBMGA1UEAwwMKi5ob3N0MS50ZXN0MFkwEwYHKoZI\n"
+ "zj0CAQYIKoZIzj0DAQcDQgAEFKrgkxm8PysXbwnHQeTD3p8YY0+sY4ssnZgmj8wX\n"
+ "KTyn893fdBHWlz71GO6t82wMTF5d+ZYwI2XU52pfl4SB2aM+MDwwDAYDVR0TAQH/\n"
+ "BAIwADAbBgNVHSMEFDASgBBAN9cB+0AvuBx+VAQnjFkBMA8GA1UdEQQIMAaHBH8A\n"
+ "AAEwDQYJKoZIhvcNAQELBQADgYEAQWZ8Oj059ZjS109V/ijMYT28xuAN5n6HHxCO\n"
+ "DopTP56Zu9+gme5wTETWEfocspZvgecoUOcedTFoKSQ7JafO09NcVLA+D6ddYpju\n"
+ "mgfuiLy9dDhqvX/NHaLBMxOBWWbOLwWE+ibyX+pOzjWRCw1L7eUXOr6PhZAOQsmU\n"
+ "D0+O6KI=\n"
+ "-----END CERTIFICATE-----\n";
+
// CertFromPEM parses the given, NUL-terminated pem block and returns an
// |X509*|.
static bssl::UniquePtr<X509> CertFromPEM(const char *pem) {
@@ -1785,6 +1819,10 @@
ASSERT_TRUE(with_sans);
bssl::UniquePtr<X509> without_sans = CertFromPEM(kCommonNameWithoutSANs);
ASSERT_TRUE(without_sans);
+ bssl::UniquePtr<X509> with_email = CertFromPEM(kCommonNameWithEmailSAN);
+ ASSERT_TRUE(with_email);
+ bssl::UniquePtr<X509> with_ip = CertFromPEM(kCommonNameWithIPSAN);
+ ASSERT_TRUE(with_ip);
auto verify_cert = [&](X509 *leaf, unsigned flags, const char *host) {
return Verify(
@@ -1804,6 +1842,10 @@
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"));
+ EXPECT_EQ(X509_V_ERR_HOSTNAME_MISMATCH,
+ verify_cert(with_email.get(), 0 /* no flags */, "foo.host1.test"));
+ EXPECT_EQ(X509_V_ERR_HOSTNAME_MISMATCH,
+ verify_cert(with_ip.get(), 0 /* no flags */, "foo.host1.test"));
// X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT is ignored.
EXPECT_EQ(X509_V_ERR_HOSTNAME_MISMATCH,
@@ -1818,6 +1860,12 @@
EXPECT_EQ(X509_V_OK, verify_cert(without_sans.get(),
X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT,
"foo.host1.test"));
+ EXPECT_EQ(X509_V_ERR_HOSTNAME_MISMATCH,
+ verify_cert(with_email.get(), X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT,
+ "foo.host1.test"));
+ EXPECT_EQ(X509_V_ERR_HOSTNAME_MISMATCH,
+ verify_cert(with_ip.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.
@@ -1833,4 +1881,10 @@
EXPECT_EQ(X509_V_ERR_HOSTNAME_MISMATCH,
verify_cert(without_sans.get(), X509_CHECK_FLAG_NEVER_CHECK_SUBJECT,
"foo.host1.test"));
+ EXPECT_EQ(X509_V_ERR_HOSTNAME_MISMATCH,
+ verify_cert(with_email.get(), X509_CHECK_FLAG_NEVER_CHECK_SUBJECT,
+ "foo.host1.test"));
+ EXPECT_EQ(X509_V_ERR_HOSTNAME_MISMATCH,
+ verify_cert(with_ip.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 ebda63a..51db478 100644
--- a/crypto/x509v3/v3_utl.c
+++ b/crypto/x509v3/v3_utl.c
@@ -955,7 +955,6 @@
int j;
int cnid = NID_undef;
int alt_type;
- int san_present = 0;
int rv = 0;
equal_fn equal;
@@ -988,7 +987,6 @@
gen = sk_GENERAL_NAME_value(gens, i);
if (gen->type != check_type)
continue;
- san_present = 1;
if (check_type == GEN_EMAIL)
cstr = gen->d.rfc822Name;
else if (check_type == GEN_DNS)
@@ -1001,10 +999,7 @@
break;
}
GENERAL_NAMES_free(gens);
- if (rv != 0)
- return rv;
- if (san_present)
- return 0;
+ return rv;
}
/* We're done if CN-ID is not pertinent */
