Normalize certificate name verification across all platforms

This brings Linux/ChromeOS, iOS, and Windows in line with the Android/OS X
implementations by using Chromium's internal RFC 6125 name validation
routines, rather than the platform-specific routines.

In particular, this adds support for iPAddress subjectAltName matching
on Windows, ignores trailing dots for dNSNames, and on Windows, removes
support for matching against non-IDNA commonNames when no subjectAltName
is present.

BUG=72726, 91072
R=wtc@chromium.org

Review URL: https://chromiumcodereview.appspot.com/22893021

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@218121 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 117 insertions, 89 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
diff --git a/net/data/ssl/certificates/subjectAltName_sanity_check.pem b/net/data/ssl/certificates/subjectAltName_sanity_check.pem
index 46cf58d..bb7f31b4 100644
--- a/net/data/ssl/certificates/subjectAltName_sanity_check.pem
+++ b/net/data/ssl/certificates/subjectAltName_sanity_check.pem
@@ -1,54 +1,55 @@
 Certificate:
     Data:
         Version: 3 (0x2)
-        Serial Number:
-            f2:f1:e7:8b:cf:09:30:f1
-        Signature Algorithm: sha1WithRSAEncryption
+        Serial Number: 17778064637999560130 (0xf6b85f9895e5b5c2)
+    Signature Algorithm: sha1WithRSAEncryption
         Issuer: C=US, ST=California, L=Mountain View, O=Test CA, CN=127.0.0.1
         Validity
-            Not Before: Apr  3 00:46:54 2012 GMT
-            Not After : Apr  1 00:46:54 2022 GMT
+            Not Before: Aug 16 02:31:34 2013 GMT
+            Not After : Aug 14 02:31:34 2023 GMT
         Subject: C=US, ST=California, L=Mountain View, O=Test CA, CN=127.0.0.1
         Subject Public Key Info:
             Public Key Algorithm: rsaEncryption
-            RSA Public Key: (1024 bit)
-                Modulus (1024 bit):
-                    00:c8:0e:13:bb:da:d5:5a:d4:68:a2:11:90:ae:c3:
-                    b3:f9:72:52:7d:e9:73:5c:49:60:ef:d3:49:05:9a:
-                    c7:4e:01:4f:b0:c8:4c:18:34:2f:7b:84:27:ad:94:
-                    12:9b:e7:3d:38:6b:49:15:55:f6:c7:3a:8d:03:ec:
-                    3e:59:90:5c:b9:a6:41:af:f0:12:b8:87:b9:54:4d:
-                    1e:18:ba:41:96:d0:f3:bb:a0:d6:80:8e:29:10:72:
-                    eb:3c:4c:c0:e2:f7:d8:61:2f:d8:63:c7:a7:79:f5:
-                    74:e0:2a:f0:5d:3e:eb:a2:36:09:4b:5d:35:31:56:
-                    1c:86:0e:8a:22:ad:1b:3f:27
+                Public-Key: (1024 bit)
+                Modulus:
+                    00:bf:11:d3:18:37:84:53:8b:07:d3:7d:0a:dc:f7:
+                    fc:ed:ce:8d:72:3a:29:af:17:e2:2b:d0:99:5f:3c:
+                    7b:29:a9:a8:3d:02:42:19:82:0b:df:5d:95:ac:60:
+                    d9:08:69:ed:90:36:42:57:39:87:4c:cc:1e:8a:1d:
+                    7e:92:bb:7e:02:df:02:80:48:3f:38:21:cc:e9:d1:
+                    b5:34:01:8f:92:17:ed:97:1d:11:2b:dd:df:fc:74:
+                    f4:d6:66:9f:e3:e5:10:ea:ea:53:b2:a7:78:4b:96:
+                    31:06:38:0b:fa:0f:d8:58:9b:ff:2a:1f:2d:8c:ae:
+                    6c:42:73:4c:d2:cf:1b:b7:d1
                 Exponent: 65537 (0x10001)
         X509v3 extensions:
+            X509v3 Basic Constraints: critical
+                CA:TRUE
             X509v3 Subject Alternative Name: 
                 IP Address:127.0.0.2, IP Address:FE80:0:0:0:0:0:0:1, DNS:test.example, email:test@test.example, othername:<unsupported>, DirName:/CN=127.0.0.3
     Signature Algorithm: sha1WithRSAEncryption
-        32:46:49:70:be:e4:db:05:0e:7e:7a:e4:ea:5c:90:c6:4c:65:
-        2d:03:ac:fb:d1:de:e4:26:e5:83:dc:5a:c8:4f:ff:b5:10:4e:
-        39:21:7f:c8:37:f3:c6:7a:de:96:b3:30:e7:c7:87:6d:75:1e:
-        14:30:17:6b:d2:76:0b:b8:43:39:c4:63:4c:50:8e:e1:0f:09:
-        ff:6c:7d:ab:c8:97:46:e8:04:70:9d:f5:e5:8c:b6:8c:b7:3d:
-        8e:0f:59:1f:6a:fd:03:c2:be:a1:40:b7:9b:38:ca:55:f5:18:
-        c3:0d:35:01:12:a0:8d:ba:1b:41:a3:6e:68:8c:cf:52:f9:96:
-        90:64
+         ad:99:a8:25:29:15:1f:b8:c7:27:f0:c8:d7:2a:2a:66:54:07:
+         2b:2c:b4:1e:fe:27:07:29:da:22:3d:7a:d8:4d:81:72:78:3e:
+         96:5d:4c:42:ce:8c:c5:d1:d9:b3:ac:92:99:19:e5:2a:32:8a:
+         bc:ce:fb:58:a0:b9:e7:4b:44:d8:0c:2c:30:2f:fa:6c:48:7e:
+         23:77:4f:67:e9:72:83:39:22:6f:2b:4d:25:16:3d:98:be:01:
+         31:a0:55:0a:85:78:b8:b9:9c:66:e6:cb:7b:81:1c:fc:84:d1:
+         79:1b:41:12:21:f8:c9:5b:fd:3c:a6:e4:6d:36:5b:0a:4c:aa:
+         bb:2b
 -----BEGIN CERTIFICATE-----
-MIICsDCCAhmgAwIBAgIJAPLx54vPCTDxMA0GCSqGSIb3DQEBBQUAMGAxCzAJBgNV
+MIICwzCCAiygAwIBAgIJAPa4X5iV5bXCMA0GCSqGSIb3DQEBBQUAMGAxCzAJBgNV
 BAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMRYwFAYDVQQHDA1Nb3VudGFpbiBW
-aWV3MRAwDgYDVQQKDAdUZXN0IENBMRIwEAYDVQQDDAkxMjcuMC4wLjEwHhcNMTIw
-NDAzMDA0NjU0WhcNMjIwNDAxMDA0NjU0WjBgMQswCQYDVQQGEwJVUzETMBEGA1UE
+aWV3MRAwDgYDVQQKDAdUZXN0IENBMRIwEAYDVQQDDAkxMjcuMC4wLjEwHhcNMTMw
+ODE2MDIzMTM0WhcNMjMwODE0MDIzMTM0WjBgMQswCQYDVQQGEwJVUzETMBEGA1UE
 CAwKQ2FsaWZvcm5pYTEWMBQGA1UEBwwNTW91bnRhaW4gVmlldzEQMA4GA1UECgwH
 VGVzdCBDQTESMBAGA1UEAwwJMTI3LjAuMC4xMIGfMA0GCSqGSIb3DQEBAQUAA4GN
-ADCBiQKBgQDIDhO72tVa1GiiEZCuw7P5clJ96XNcSWDv00kFmsdOAU+wyEwYNC97
-hCetlBKb5z04a0kVVfbHOo0D7D5ZkFy5pkGv8BK4h7lUTR4YukGW0PO7oNaAjikQ
-cus8TMDi99hhL9hjx6d59XTgKvBdPuuiNglLXTUxVhyGDooirRs/JwIDAQABo3Iw
-cDBuBgNVHREEZzBlhwR/AAAChxD+gAAAAAAAAAAAAAAAAAABggx0ZXN0LmV4YW1w
-bGWBEXRlc3RAdGVzdC5leGFtcGxloBIGAyoDBKALDAlpZ25vcmUgbWWkFjAUMRIw
-EAYDVQQDDAkxMjcuMC4wLjMwDQYJKoZIhvcNAQEFBQADgYEAMkZJcL7k2wUOfnrk
-6lyQxkxlLQOs+9He5Cblg9xayE//tRBOOSF/yDfzxnrelrMw58eHbXUeFDAXa9J2
-C7hDOcRjTFCO4Q8J/2x9q8iXRugEcJ315Yy2jLc9jg9ZH2r9A8K+oUC3mzjKVfUY
-ww01ARKgjbobQaNuaIzPUvmWkGQ=
+ADCBiQKBgQC/EdMYN4RTiwfTfQrc9/ztzo1yOimvF+Ir0JlfPHspqag9AkIZggvf
+XZWsYNkIae2QNkJXOYdMzB6KHX6Su34C3wKASD84Iczp0bU0AY+SF+2XHREr3d/8
+dPTWZp/j5RDq6lOyp3hLljEGOAv6D9hYm/8qHy2MrmxCc0zSzxu30QIDAQABo4GE
+MIGBMA8GA1UdEwEB/wQFMAMBAf8wbgYDVR0RBGcwZYcEfwAAAocQ/oAAAAAAAAAA
+AAAAAAAAAYIMdGVzdC5leGFtcGxlgRF0ZXN0QHRlc3QuZXhhbXBsZaASBgMqAwSg
+CwwJaWdub3JlIG1lpBYwFDESMBAGA1UEAwwJMTI3LjAuMC4zMA0GCSqGSIb3DQEB
+BQUAA4GBAK2ZqCUpFR+4xyfwyNcqKmZUBysstB7+Jwcp2iI9ethNgXJ4PpZdTELO
+jMXR2bOskpkZ5SoyirzO+1iguedLRNgMLDAv+mxIfiN3T2fpcoM5Im8rTSUWPZi+
+ATGgVQqFeLi5nGbmy3uBHPyE0XkbQRIh+Mlb/Tym5G02WwpMqrsr
 -----END CERTIFICATE-----
diff --git a/net/data/ssl/scripts/ee.cnf b/net/data/ssl/scripts/ee.cnf
index ad786c8..5214f9e 100644
--- a/net/data/ssl/scripts/ee.cnf
+++ b/net/data/ssl/scripts/ee.cnf
@@ -29,7 +29,8 @@ CN = Duplicate
 subjectAltName = IP:127.0.0.1
 
 [req_san_sanity]
-subjectAltName = @san_sanity
+basicConstraints = critical, CA:true
+subjectAltName   = @san_sanity
 
 [san_sanity]
 IP.1  = 127.0.0.2