summaryrefslogtreecommitdiffstats
path: root/net/cert
diff options
context:
space:
mode:
Diffstat (limited to 'net/cert')
-rw-r--r--net/cert/cert_verify_proc_nss.cc9
-rw-r--r--net/cert/cert_verify_proc_unittest.cc66
-rw-r--r--net/cert/cert_verify_proc_win.cc55
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