Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(133)

Unified Diff: net/cert/cert_verify_proc_unittest.cc

Issue 2719273002: Disable commonName matching for certificates (Closed)
Patch Set: Update tests & fix small bug with IPvsDNS sans Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: net/cert/cert_verify_proc_unittest.cc
diff --git a/net/cert/cert_verify_proc_unittest.cc b/net/cert/cert_verify_proc_unittest.cc
index 27870e36b7d81c86d0deebba46ea4885c60ab884..a89663256cd2fb3f181846454e2caa09c01f3c83 100644
--- a/net/cert/cert_verify_proc_unittest.cc
+++ b/net/cert/cert_verify_proc_unittest.cc
@@ -1712,33 +1712,21 @@ INSTANTIATE_TEST_CASE_P(VerifyTrustedEE,
CertVerifyProcWeakDigestTest,
testing::ValuesIn(kVerifyTrustedEETestData));
-// For the list of valid hostnames, see
-// net/cert/data/ssl/certificates/subjectAltName_sanity_check.pem
-struct CertVerifyProcNameData {
- const char* hostname;
- bool valid; // Whether or not |hostname| matches a subjectAltName.
-};
-
// Test fixture for verifying certificate names. These tests are run for each
// of the CertVerify implementations.
-class CertVerifyProcNameTest : public CertVerifyProcInternalTest {
- public:
- CertVerifyProcNameTest() {}
- virtual ~CertVerifyProcNameTest() {}
-
+class CertVerifyProcNameTest : public ::testing::Test {
protected:
void VerifyCertName(const char* hostname, bool valid) {
- CertificateList cert_list = CreateCertificateListFromFile(
- GetTestCertsDirectory(), "subjectAltName_sanity_check.pem",
- X509Certificate::FORMAT_AUTO);
- ASSERT_EQ(1U, cert_list.size());
- scoped_refptr<X509Certificate> cert(cert_list[0]);
-
- ScopedTestRoot scoped_root(cert.get());
+ scoped_refptr<X509Certificate> cert(ImportCertFromFile(
+ GetTestCertsDirectory(), "subjectAltName_sanity_check.pem"));
+ ASSERT_TRUE(cert);
+ CertVerifyResult result;
+ result.is_issued_by_known_root = false;
+ scoped_refptr<CertVerifyProc> verify_proc = new MockCertVerifyProc(result);
CertVerifyResult verify_result;
- int error = Verify(cert.get(), hostname, 0, NULL, CertificateList(),
- &verify_result);
+ int error = verify_proc->Verify(cert.get(), hostname, std::string(), 0,
+ nullptr, CertificateList(), &verify_result);
if (valid) {
EXPECT_THAT(error, IsOk());
EXPECT_FALSE(verify_result.cert_status & CERT_STATUS_COMMON_NAME_INVALID);
@@ -1750,74 +1738,138 @@ class CertVerifyProcNameTest : public CertVerifyProcInternalTest {
};
// Don't match the common name
-TEST_P(CertVerifyProcNameTest, DontMatchCommonName) {
+TEST_F(CertVerifyProcNameTest, DontMatchCommonName) {
VerifyCertName("127.0.0.1", false);
}
// Matches the iPAddress SAN (IPv4)
-TEST_P(CertVerifyProcNameTest, MatchesIpSanIpv4) {
+TEST_F(CertVerifyProcNameTest, MatchesIpSanIpv4) {
VerifyCertName("127.0.0.2", true);
}
// Matches the iPAddress SAN (IPv6)
-TEST_P(CertVerifyProcNameTest, MatchesIpSanIpv6) {
+TEST_F(CertVerifyProcNameTest, MatchesIpSanIpv6) {
VerifyCertName("FE80:0:0:0:0:0:0:1", true);
}
// Should not match the iPAddress SAN
-TEST_P(CertVerifyProcNameTest, DoesntMatchIpSanIpv6) {
+TEST_F(CertVerifyProcNameTest, DoesntMatchIpSanIpv6) {
VerifyCertName("[FE80:0:0:0:0:0:0:1]", false);
}
// Compressed form matches the iPAddress SAN (IPv6)
-TEST_P(CertVerifyProcNameTest, MatchesIpSanCompressedIpv6) {
+TEST_F(CertVerifyProcNameTest, MatchesIpSanCompressedIpv6) {
VerifyCertName("FE80::1", true);
}
// IPv6 mapped form should NOT match iPAddress SAN
-TEST_P(CertVerifyProcNameTest, DoesntMatchIpSanIPv6Mapped) {
+TEST_F(CertVerifyProcNameTest, DoesntMatchIpSanIPv6Mapped) {
VerifyCertName("::127.0.0.2", false);
}
// Matches the dNSName SAN
-TEST_P(CertVerifyProcNameTest, MatchesDnsSan) {
+TEST_F(CertVerifyProcNameTest, MatchesDnsSan) {
VerifyCertName("test.example", true);
}
// Matches the dNSName SAN (trailing . ignored)
-TEST_P(CertVerifyProcNameTest, MatchesDnsSanTrailingDot) {
+TEST_F(CertVerifyProcNameTest, MatchesDnsSanTrailingDot) {
VerifyCertName("test.example.", true);
}
// Should not match the dNSName SAN
-TEST_P(CertVerifyProcNameTest, DoesntMatchDnsSan) {
+TEST_F(CertVerifyProcNameTest, DoesntMatchDnsSan) {
VerifyCertName("www.test.example", false);
}
// Should not match the dNSName SAN
-TEST_P(CertVerifyProcNameTest, DoesntMatchDnsSanInvalid) {
+TEST_F(CertVerifyProcNameTest, DoesntMatchDnsSanInvalid) {
VerifyCertName("test..example", false);
}
// Should not match the dNSName SAN
-TEST_P(CertVerifyProcNameTest, DoesntMatchDnsSanTwoTrailingDots) {
+TEST_F(CertVerifyProcNameTest, DoesntMatchDnsSanTwoTrailingDots) {
VerifyCertName("test.example..", false);
}
// Should not match the dNSName SAN
-TEST_P(CertVerifyProcNameTest, DoesntMatchDnsSanLeadingAndTrailingDot) {
+TEST_F(CertVerifyProcNameTest, DoesntMatchDnsSanLeadingAndTrailingDot) {
VerifyCertName(".test.example.", false);
}
// Should not match the dNSName SAN
-TEST_P(CertVerifyProcNameTest, DoesntMatchDnsSanTrailingDot) {
+TEST_F(CertVerifyProcNameTest, DoesntMatchDnsSanTrailingDot) {
VerifyCertName(".test.example", false);
}
-INSTANTIATE_TEST_CASE_P(VerifyName,
- CertVerifyProcNameTest,
- testing::ValuesIn(kAllCertVerifiers),
- VerifyProcTypeToName);
+// Tests that commonName-fallback is handled correctly:
+// - If it's a publicly trusted certificate, the commonName should never
+// match, both with a subjectAltName is present and when it is absent.
+// - If it chains to a private root, the commonName should not match if
+// the subjectAltName is present.
+// - If it chains to a private root, the commonName should not match if
+// the subjectAltName is absent, and the flags don't allow fallback.
+// - If it chains to a private root, the commonName SHOULD match iff the
+// subjectAltName is absent and the flags allow a fallback.
+TEST_F(CertVerifyProcNameTest, HandlesCommonNameFallbackLocalAnchors) {
+ scoped_refptr<X509Certificate> cert(
+ ImportCertFromFile(GetTestCertsDirectory(), "salesforce_com_test.pem"));
+ ASSERT_TRUE(cert);
+
+ CertVerifyResult result;
+ scoped_refptr<CertVerifyProc> verify_proc;
+ CertVerifyResult verify_result;
+ int error;
+
+ // Publicly trusted: Always ignores commonName, regardless of flags.
+ result = CertVerifyResult();
+ verify_result = CertVerifyResult();
+ error = 0;
+ result.is_issued_by_known_root = true;
+ verify_proc = new MockCertVerifyProc(result);
+ error = verify_proc->Verify(cert.get(), "prerelna1.pre.salesforce.com",
+ std::string(), 0, nullptr, CertificateList(),
+ &verify_result);
+ EXPECT_THAT(error, IsError(ERR_CERT_COMMON_NAME_INVALID));
+ EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_COMMON_NAME_INVALID);
+
+ result = CertVerifyResult();
+ verify_result = CertVerifyResult();
+ error = 0;
+ result.is_issued_by_known_root = true;
+ verify_proc = new MockCertVerifyProc(result);
+ error = verify_proc->Verify(
+ cert.get(), "prerelna1.pre.salesforce.com", std::string(),
+ CertVerifier::VERIFY_ENABLE_COMMON_NAME_FALLBACK_LOCAL_ANCHORS, nullptr,
+ CertificateList(), &verify_result);
+ EXPECT_THAT(error, IsError(ERR_CERT_COMMON_NAME_INVALID));
+ EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_COMMON_NAME_INVALID);
+
+ // Privately trusted: Ignores commonName by default.
+ result = CertVerifyResult();
+ verify_result = CertVerifyResult();
+ error = 0;
+ result.is_issued_by_known_root = false;
+ verify_proc = new MockCertVerifyProc(result);
+ error = verify_proc->Verify(cert.get(), "prerelna1.pre.salesforce.com",
+ std::string(), 0, nullptr, CertificateList(),
+ &verify_result);
+ EXPECT_THAT(error, IsError(ERR_CERT_COMMON_NAME_INVALID));
+ EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_COMMON_NAME_INVALID);
+
+ // Privately trusted: Falls back to common name if flags allow.
+ result = CertVerifyResult();
+ verify_result = CertVerifyResult();
+ error = 0;
+ result.is_issued_by_known_root = false;
+ verify_proc = new MockCertVerifyProc(result);
+ error = verify_proc->Verify(
+ cert.get(), "prerelna1.pre.salesforce.com", std::string(),
+ CertVerifier::VERIFY_ENABLE_COMMON_NAME_FALLBACK_LOCAL_ANCHORS, nullptr,
+ CertificateList(), &verify_result);
+ EXPECT_THAT(error, IsOk());
+ EXPECT_FALSE(verify_result.cert_status & CERT_STATUS_COMMON_NAME_INVALID);
+}
// Tests that CertVerifyProc records a histogram correctly when a
// certificate chaining to a private root contains the TLS feature

Powered by Google App Engine
This is Rietveld 408576698