Normalize certificate name verification across all platforms

net/cert/cert_verify_proc_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/cert/cert_verify_proc_win.h"
 
 #include <string>
 #include <vector>
 
 #include "base/memory/scoped_ptr.h"
@@ skipping 699 matching lines @@
         CERT_TRUST_IS_OFFLINE_REVOCATION) {
       verify_result->cert_status |= CERT_STATUS_REVOKED;
     }
   }
 
   ScopedPCCERT_CHAIN_CONTEXT scoped_chain_context(chain_context);
 
   verify_result->cert_status |= MapCertChainErrorStatusToCertStatus(
       chain_context->TrustStatus.dwErrorStatus);
 
   // Flag certificates that have a Subject common name with a NULL character.

[wtc, 2013/08/16 19:59:19]

Perhaps this workaround can be removed?

[Ryan Sleevi, 2013/08/16 20:38:28]

I think it's still reasonable to bounce such certs as invalid, independent of
any name matching. It's more of a defense in depth thing, and matches other SSL
libraries behaviours.

   if (CertSubjectCommonNameHasNull(cert_handle))
     verify_result->cert_status |= CERT_STATUS_INVALID;
 
   std::wstring wstr_hostname = ASCIIToWide(hostname);
 
   SSL_EXTRA_CERT_CHAIN_POLICY_PARA extra_policy_para;
   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;
+  extra_policy_para.fdwChecks =
+      0x00001000;  // SECURITY_FLAG_IGNORE_CERT_CN_INVALID

[wtc, 2013/08/16 19:59:19]

Is this macro not defined in Wincrypt.h?

A comment that announces we will check the cert name below
might be helpful. Alternative, we can call cert->VerifyNameMatch(hostname)
first.

[Ryan Sleevi, 2013/08/16 20:38:28]

No. The flags here are similar to the flags on the (old) line 769-786, which are
handled by WinHttp/WinInet.h, not Wincrypt

   extra_policy_para.pwszServerName =
       const_cast<wchar_t*>(wstr_hostname.c_str());
 
   CERT_CHAIN_POLICY_PARA policy_para;
   memset(&policy_para, 0, sizeof(policy_para));
   policy_para.cbSize = sizeof(policy_para);
   policy_para.dwFlags = 0;
   policy_para.pvExtraPolicyPara = &extra_policy_para;
 
   CERT_CHAIN_POLICY_STATUS policy_status;
   memset(&policy_status, 0, sizeof(policy_status));
   policy_status.cbSize = sizeof(policy_status);
 
   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));
-
-    // 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;
+
+

[wtc, 2013/08/16 19:59:19]

Nit: delete one blank line.

   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
     // information for every certificate. We only want to put up revoked
     // statuses from the offline checks so we squash this error.
     verify_result->cert_status &= ~CERT_STATUS_UNABLE_TO_CHECK_REVOCATION;
   }
 
   AppendPublicKeyHashes(chain_context, &verify_result->public_key_hashes);
   verify_result->is_issued_by_known_root = IsIssuedByKnownRoot(chain_context);
 
   if (IsCertStatusError(verify_result->cert_status))
     return MapCertStatusToNetError(verify_result->cert_status);
 
   if (ev_policy_oid &&
       CheckEV(chain_context, rev_checking_enabled, ev_policy_oid)) {
     verify_result->cert_status |= CERT_STATUS_IS_EV;
   }
   return OK;
 }
 
 }  // namespace net