diff options
Diffstat (limited to 'net/cert')
-rw-r--r-- | net/cert/cert_verify_proc_nss.cc | 9 | ||||
-rw-r--r-- | net/cert/cert_verify_proc_unittest.cc | 66 | ||||
-rw-r--r-- | net/cert/cert_verify_proc_win.cc | 55 |
3 files changed, 78 insertions, 52 deletions
diff --git a/net/cert/cert_verify_proc_nss.cc b/net/cert/cert_verify_proc_nss.cc index f63297e..0a0743c 100644 --- a/net/cert/cert_verify_proc_nss.cc +++ b/net/cert/cert_verify_proc_nss.cc @@ -764,8 +764,7 @@ int CertVerifyProcNSS::VerifyInternal( #endif // defined(OS_IOS) // Make sure that the hostname matches with the common name of the cert. - SECStatus status = CERT_VerifyCertName(cert_handle, hostname.c_str()); - if (status != SECSuccess) + if (!cert->VerifyNameMatch(hostname)) verify_result->cert_status |= CERT_STATUS_COMMON_NAME_INVALID; // Make sure that the cert is valid now. @@ -805,9 +804,9 @@ int CertVerifyProcNSS::VerifyInternal( CertificateListToCERTCertList(additional_trust_anchors)); } - status = PKIXVerifyCert(cert_handle, check_revocation, false, - cert_io_enabled, NULL, 0, trust_anchors.get(), - cvout); + SECStatus status = PKIXVerifyCert(cert_handle, check_revocation, false, + cert_io_enabled, NULL, 0, + trust_anchors.get(), cvout); if (status == SECSuccess && (flags & CertVerifier::VERIFY_REV_CHECKING_REQUIRED_LOCAL_ANCHORS) && diff --git a/net/cert/cert_verify_proc_unittest.cc b/net/cert/cert_verify_proc_unittest.cc index a53d10a..45a35736 100644 --- a/net/cert/cert_verify_proc_unittest.cc +++ b/net/cert/cert_verify_proc_unittest.cc @@ -245,7 +245,6 @@ TEST_F(CertVerifyProcTest, MAYBE_IntermediateCARequireExplicitPolicy) { EXPECT_EQ(0u, verify_result.cert_status); } - // Test for bug 58437. // This certificate will expire on 2011-12-21. The test will still // pass if error == ERR_CERT_DATE_INVALID. @@ -1356,4 +1355,69 @@ WRAPPED_INSTANTIATE_TEST_CASE_P( CertVerifyProcWeakDigestTest, testing::ValuesIn(kVerifyMixedTestData)); +// For the list of valid hostnames, see +// net/cert/data/ssl/certificates/subjectAltName_sanity_check.pem +static const struct CertVerifyProcNameData { + const char* hostname; + bool valid; // Whether or not |hostname| matches a subjectAltName. +} kVerifyNameData[] = { + { "127.0.0.1", false }, // Don't match the common name + { "127.0.0.2", true }, // Matches the iPAddress SAN (IPv4) + { "FE80:0:0:0:0:0:0:1", true }, // Matches the iPAddress SAN (IPv6) + { "[FE80:0:0:0:0:0:0:1]", false }, // Should not match the iPAddress SAN + { "FE80::1", true }, // Compressed form matches the iPAddress SAN (IPv6) + { "::127.0.0.2", false }, // IPv6 mapped form should NOT match iPAddress SAN + { "test.example", true }, // Matches the dNSName SAN + { "test.example.", true }, // Matches the dNSName SAN (trailing . ignored) + { "www.test.example", false }, // Should not match the dNSName SAN + { "test..example", false }, // Should not match the dNSName SAN + { "test.example..", false }, // Should not match the dNSName SAN + { ".test.example.", false }, // Should not match the dNSName SAN + { ".test.example", false }, // Should not match the dNSName SAN +}; + +// GTest 'magic' pretty-printer, so that if/when a test fails, it knows how +// to output the parameter that was passed. Without this, it will simply +// attempt to print out the first twenty bytes of the object, which depending +// on platform and alignment, may result in an invalid read. +void PrintTo(const CertVerifyProcNameData& data, std::ostream* os) { + *os << "Hostname: " << data.hostname << "; valid=" << data.valid; +} + +class CertVerifyProcNameTest + : public CertVerifyProcTest, + public testing::WithParamInterface<CertVerifyProcNameData> { + public: + CertVerifyProcNameTest() {} + virtual ~CertVerifyProcNameTest() {} +}; + +TEST_P(CertVerifyProcNameTest, VerifyCertName) { + CertVerifyProcNameData data = GetParam(); + + 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()); + + CertVerifyResult verify_result; + int error = Verify(cert.get(), data.hostname, 0, NULL, empty_cert_list_, + &verify_result); + if (data.valid) { + EXPECT_EQ(OK, error); + EXPECT_FALSE(verify_result.cert_status & CERT_STATUS_COMMON_NAME_INVALID); + } else { + EXPECT_EQ(ERR_CERT_COMMON_NAME_INVALID, error); + EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_COMMON_NAME_INVALID); + } +} + +WRAPPED_INSTANTIATE_TEST_CASE_P( + VerifyName, + CertVerifyProcNameTest, + testing::ValuesIn(kVerifyNameData)); + } // namespace net diff --git a/net/cert/cert_verify_proc_win.cc b/net/cert/cert_verify_proc_win.cc index 7e94246..d3e8b62 100644 --- a/net/cert/cert_verify_proc_win.cc +++ b/net/cert/cert_verify_proc_win.cc @@ -727,7 +727,10 @@ int CertVerifyProcWin::VerifyInternal( memset(&extra_policy_para, 0, sizeof(extra_policy_para)); extra_policy_para.cbSize = sizeof(extra_policy_para); extra_policy_para.dwAuthType = AUTHTYPE_SERVER; - extra_policy_para.fdwChecks = 0; + // Certificate name validation happens separately, later, using an internal + // routine that has better support for RFC 6125 name matching. + extra_policy_para.fdwChecks = + 0x00001000; // SECURITY_FLAG_IGNORE_CERT_CN_INVALID extra_policy_para.pwszServerName = const_cast<wchar_t*>(wstr_hostname.c_str()); @@ -752,57 +755,17 @@ int CertVerifyProcWin::VerifyInternal( if (policy_status.dwError) { verify_result->cert_status |= MapNetErrorToCertStatus( MapSecurityError(policy_status.dwError)); - - // CertVerifyCertificateChainPolicy reports only one error (in - // policy_status.dwError) if the certificate has multiple errors. - // CertGetCertificateChain doesn't report certificate name mismatch, so - // CertVerifyCertificateChainPolicy is the only function that can report - // certificate name mismatch. - // - // To prevent a potential certificate name mismatch from being hidden by - // some other certificate error, if we get any other certificate error, - // we call CertVerifyCertificateChainPolicy again, ignoring all other - // certificate errors. Both extra_policy_para.fdwChecks and - // policy_para.dwFlags allow us to ignore certificate errors, so we set - // them both. - if (policy_status.dwError != CERT_E_CN_NO_MATCH) { - const DWORD extra_ignore_flags = - 0x00000080 | // SECURITY_FLAG_IGNORE_REVOCATION - 0x00000100 | // SECURITY_FLAG_IGNORE_UNKNOWN_CA - 0x00002000 | // SECURITY_FLAG_IGNORE_CERT_DATE_INVALID - 0x00000200; // SECURITY_FLAG_IGNORE_WRONG_USAGE - extra_policy_para.fdwChecks = extra_ignore_flags; - const DWORD ignore_flags = - CERT_CHAIN_POLICY_IGNORE_ALL_NOT_TIME_VALID_FLAGS | - CERT_CHAIN_POLICY_IGNORE_INVALID_BASIC_CONSTRAINTS_FLAG | - CERT_CHAIN_POLICY_ALLOW_UNKNOWN_CA_FLAG | - CERT_CHAIN_POLICY_IGNORE_WRONG_USAGE_FLAG | - CERT_CHAIN_POLICY_IGNORE_INVALID_NAME_FLAG | - CERT_CHAIN_POLICY_IGNORE_INVALID_POLICY_FLAG | - CERT_CHAIN_POLICY_IGNORE_ALL_REV_UNKNOWN_FLAGS | - CERT_CHAIN_POLICY_ALLOW_TESTROOT_FLAG | - CERT_CHAIN_POLICY_TRUST_TESTROOT_FLAG | - CERT_CHAIN_POLICY_IGNORE_NOT_SUPPORTED_CRITICAL_EXT_FLAG | - CERT_CHAIN_POLICY_IGNORE_PEER_TRUST_FLAG; - policy_para.dwFlags = ignore_flags; - if (!CertVerifyCertificateChainPolicy( - CERT_CHAIN_POLICY_SSL, - chain_context, - &policy_para, - &policy_status)) { - return MapSecurityError(GetLastError()); - } - if (policy_status.dwError) { - verify_result->cert_status |= MapNetErrorToCertStatus( - MapSecurityError(policy_status.dwError)); - } - } } // TODO(wtc): Suppress CERT_STATUS_NO_REVOCATION_MECHANISM for now to be // compatible with WinHTTP, which doesn't report this error (bug 3004). verify_result->cert_status &= ~CERT_STATUS_NO_REVOCATION_MECHANISM; + // Perform hostname verification independent of + // CertVerifyCertificateChainPolicy. + if (!cert->VerifyNameMatch(hostname)) + verify_result->cert_status |= CERT_STATUS_COMMON_NAME_INVALID; + if (!rev_checking_enabled) { // If we didn't do online revocation checking then Windows will report // CERT_UNABLE_TO_CHECK_REVOCATION unless it had cached OCSP or CRL |