On Mac, compare certificate host names according to RFC6125.

Additionally, add more aggressive checks for invalid certificates in the chain.

This re-lands the portion of r93346 that was reverted in r94007.

BUG=88135, 90146
TEST=none


Review URL: http://codereview.chromium.org/7467038

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

2 files changed, 27 insertions, 93 deletions

diff --git a/net/base/x509_certificate.h b/net/base/x509_certificate.h
index ad0849b..ca2abec 100644
--- a/net/base/x509_certificate.h
+++ b/net/base/x509_certificate.h
@@ -255,15 +255,6 @@ class NET_API X509Certificate
   // |*policy| and ownership transferred to the caller.
   static OSStatus CreateSSLClientPolicy(SecPolicyRef* policy);
 
-  // Creates a security policy for certificates used by SSL servers.
-  // |hostname| is an optionally-supplied string indicating the name to verify
-  // the server certificate as; if it is empty, no hostname verification will
-  // happen.
-  // If a policy is successfully created, it will be stored in |*policy| and
-  // ownership transferred to the caller.
-  static OSStatus CreateSSLServerPolicy(const std::string& hostname,
-                                        SecPolicyRef* policy);
-
   // Creates a security policy for basic X.509 validation. If the policy is
   // successfully created, it will be stored in |*policy| and ownership
   // transferred to the caller.
diff --git a/net/base/x509_certificate_mac.cc b/net/base/x509_certificate_mac.cc
index 1f16f20..88ff95f 100644
--- a/net/base/x509_certificate_mac.cc
+++ b/net/base/x509_certificate_mac.cc
@@ -120,29 +120,6 @@ int CertStatusFromOSStatus(OSStatus status) {
   }
 }
 
-bool OverrideHostnameMismatch(const std::string& hostname,
-                              std::vector<std::string>* dns_names) {
-  // SecTrustEvaluate() does not check dotted IP addresses. If
-  // hostname is provided as, say, 127.0.0.1, then the error
-  // CSSMERR_APPLETP_HOSTNAME_MISMATCH will always be returned,
-  // even if the certificate contains 127.0.0.1 as one of its names.
-  // We, however, want to allow that behavior. SecTrustEvaluate()
-  // only checks for digits and dots when considering whether a
-  // hostname is an IP address, so IPv6 and hex addresses go through
-  // its normal comparison.
-  bool is_dotted_ip = true;
-  bool override_hostname_mismatch = false;
-  for (std::string::const_iterator c = hostname.begin();
-       c != hostname.end() && is_dotted_ip; ++c)
-    is_dotted_ip = (*c >= '0' && *c <= '9') || *c == '.';
-  if (is_dotted_ip) {
-    for (std::vector<std::string>::const_iterator name = dns_names->begin();
-         name != dns_names->end() && !override_hostname_mismatch; ++name)
-      override_hostname_mismatch = (*name == hostname);
-  }
-  return override_hostname_mismatch;
-}
-
 struct CSSMFields {
   CSSMFields() : cl_handle(NULL), num_of_fields(0), fields(NULL) {}
   ~CSSMFields() {
@@ -263,24 +240,31 @@ OSStatus CreatePolicy(const CSSM_OID* policy_OID,
 }
 
 // Creates a series of SecPolicyRefs to be added to a SecTrustRef used to
-// validate a certificate for an SSL server. |hostname| contains the name of
-// the SSL server that the certificate should be verified against. |flags| is
-// a bitwise-OR of VerifyFlags that can further alter how trust is
-// validated, such as how revocation is checked. If successful, returns
-// noErr, and stores the resultant array of SecPolicyRefs in |policies|.
-OSStatus CreateTrustPolicies(const std::string& hostname, int flags,
+// validate a certificate for an SSL server. Hostname verification is not
+// performed and is the responsibility of the caller. |flags| is a bitwise-OR
+// of VerifyFlags that can further alter how trust is validated, such as how
+// revocation is checked. If successful, returns noErr, and stores the
+// resultant array of SecPolicyRefs in |policies|.
+OSStatus CreateTrustPolicies(int flags,
                              ScopedCFTypeRef<CFArrayRef>* policies) {
   ScopedCFTypeRef<CFMutableArrayRef> local_policies(
       CFArrayCreateMutable(kCFAllocatorDefault, 0, &kCFTypeArrayCallBacks));
   if (!local_policies)
     return memFullErr;
 
-  // Create an SSL SecPolicyRef, and configure it to perform hostname
-  // validation. The hostname check does 99% of what we want, with the
-  // exception of dotted IPv4 addreses, which we handle ourselves below.
+  // Create an SSL server policy. Certificate name validation is not performed
+  // using SecTrustEvalute(), as it contains several limitations that are not
+  // desirable:
+  // - Doesn't support IP addresses in dotted-quad literals (127.0.0.1)
+  // - Doesn't support IPv6 addresses
+  // - Doesn't support the iPAddress subjectAltName
+  CSSM_APPLE_TP_SSL_OPTIONS tp_ssl_options;
+  memset(&tp_ssl_options, 0, sizeof(tp_ssl_options));
+  tp_ssl_options.Version = CSSM_APPLE_TP_SSL_OPTS_VERSION;
+
   SecPolicyRef ssl_policy;
-  OSStatus status = X509Certificate::CreateSSLServerPolicy(hostname,
-                                                           &ssl_policy);
+  OSStatus status = CreatePolicy(&CSSMOID_APPLE_TP_SSL, &tp_ssl_options,
+                                 sizeof(tp_ssl_options), &ssl_policy);
   if (status)
     return status;
   CFArrayAppendValue(local_policies, ssl_policy);
@@ -750,7 +734,7 @@ int X509Certificate::VerifyInternal(const std::string& hostname,
                                     int flags,
                                     CertVerifyResult* verify_result) const {
   ScopedCFTypeRef<CFArrayRef> trust_policies;
-  OSStatus status = CreateTrustPolicies(hostname, flags, &trust_policies);
+  OSStatus status = CreateTrustPolicies(flags, &trust_policies);
   if (status)
     return NetErrorFromOSStatus(status);
 
@@ -850,7 +834,6 @@ int X509Certificate::VerifyInternal(const std::string& hostname,
 
   // Evaluate the results
   OSStatus cssm_result;
-  bool got_certificate_error = false;
   switch (trust_result) {
     case kSecTrustResultUnspecified:
     case kSecTrustResultProceed:
@@ -871,23 +854,7 @@ int X509Certificate::VerifyInternal(const std::string& hostname,
       status = SecTrustGetCssmResultCode(trust_ref, &cssm_result);
       if (status)
         return NetErrorFromOSStatus(status);
-      switch (cssm_result) {
-        case CSSMERR_TP_NOT_TRUSTED:
-        case CSSMERR_TP_INVALID_ANCHOR_CERT:
-          verify_result->cert_status |= CERT_STATUS_AUTHORITY_INVALID;
-          break;
-        case CSSMERR_TP_CERT_EXPIRED:
-        case CSSMERR_TP_CERT_NOT_VALID_YET:
-          verify_result->cert_status |= CERT_STATUS_DATE_INVALID;
-          break;
-        case CSSMERR_TP_CERT_REVOKED:
-        case CSSMERR_TP_CERT_SUSPENDED:
-          verify_result->cert_status |= CERT_STATUS_REVOKED;
-          break;
-        default:
-          // Look for specific per-certificate errors below.
-          break;
-      }
+      verify_result->cert_status |= CertStatusFromOSStatus(cssm_result);
       // Walk the chain of error codes in the CSSM_TP_APPLE_EVIDENCE_INFO
       // structure which can catch multiple errors from each certificate.
       for (CFIndex index = 0, chain_count = CFArrayGetCount(completed_chain);
@@ -904,24 +871,11 @@ int X509Certificate::VerifyInternal(const std::string& hostname,
         for (uint32 status_code_index = 0;
              status_code_index < chain_info[index].NumStatusCodes;
              ++status_code_index) {
-          got_certificate_error = true;
-          int cert_status = CertStatusFromOSStatus(
+          verify_result->cert_status |= CertStatusFromOSStatus(
               chain_info[index].StatusCodes[status_code_index]);
-          if (cert_status == CERT_STATUS_COMMON_NAME_INVALID) {
-            std::vector<std::string> names;
-            GetDNSNames(&names);
-            if (OverrideHostnameMismatch(hostname, &names))
-              cert_status = 0;
-          }
-          verify_result->cert_status |= cert_status;
         }
       }
-      // Be paranoid and ensure that we recorded at least one certificate
-      // status on receiving kSecTrustResultRecoverableTrustFailure. The
-      // call to SecTrustGetCssmResultCode() should pick up when the chain
-      // is not trusted and the loop through CSSM_TP_APPLE_EVIDENCE_INFO
-      // should pick up everything else, but let's be safe.
-      if (!verify_result->cert_status && !got_certificate_error) {
+      if (!IsCertStatusError(verify_result->cert_status)) {
         LOG(ERROR) << "cssm_result=" << cssm_result;
         verify_result->cert_status |= CERT_STATUS_INVALID;
         NOTREACHED();
@@ -933,13 +887,17 @@ int X509Certificate::VerifyInternal(const std::string& hostname,
       if (status)
         return NetErrorFromOSStatus(status);
       verify_result->cert_status |= CertStatusFromOSStatus(cssm_result);
-      if (!verify_result->cert_status) {
+      if (!IsCertStatusError(verify_result->cert_status)) {
         LOG(WARNING) << "trust_result=" << trust_result;
         verify_result->cert_status |= CERT_STATUS_INVALID;
       }
       break;
   }
 
+  // Perform hostname verification independent of SecTrustEvaluate.
+  if (!VerifyNameMatch(hostname))
+    verify_result->cert_status |= CERT_STATUS_COMMON_NAME_INVALID;
+
   // TODO(wtc): Suppress CERT_STATUS_NO_REVOCATION_MECHANISM for now to be
   // compatible with Windows, which in turn implements this behavior to be
   // compatible with WinHTTP, which doesn't report this error (bug 3004).
@@ -1159,21 +1117,6 @@ OSStatus X509Certificate::CreateSSLClientPolicy(SecPolicyRef* policy) {
 }
 
 // static
-OSStatus X509Certificate::CreateSSLServerPolicy(const std::string& hostname,
-                                                SecPolicyRef* policy) {
-  CSSM_APPLE_TP_SSL_OPTIONS tp_ssl_options;
-  memset(&tp_ssl_options, 0, sizeof(tp_ssl_options));
-  tp_ssl_options.Version = CSSM_APPLE_TP_SSL_OPTS_VERSION;
-  if (!hostname.empty()) {
-    tp_ssl_options.ServerName = hostname.data();
-    tp_ssl_options.ServerNameLen = hostname.size();
-  }
-
-  return CreatePolicy(&CSSMOID_APPLE_TP_SSL, &tp_ssl_options,
-                      sizeof(tp_ssl_options), policy);
-}
-
-// static
 OSStatus X509Certificate::CreateBasicX509Policy(SecPolicyRef* policy) {
   return CreatePolicy(&CSSMOID_APPLE_X509_BASIC, NULL, 0, policy);
 }