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>
This commit is contained in:
David Benjamin 2019-04-16 13:57:11 -05:00 committed by Adam Langley
parent 9df41ae953
commit c60b42bf7e
3 changed files with 95 additions and 4 deletions

View File

@ -660,6 +660,39 @@ static const char kSelfSignedMismatchAlgorithms[] =
"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 @@ TEST(X509Test, PartialBIOReturn) {
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"));
}

View File

@ -1003,14 +1003,12 @@ static int do_x509_check(X509 *x, const char *chk, size_t chklen,
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;

View File

@ -713,6 +713,8 @@ OPENSSL_EXPORT STACK_OF(OPENSSL_STRING) *X509_get1_ocsp(X509 *x);
#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