Mac: Make client-cert picker only show certs the server will accept.

net/base/x509_certificate_mac.cc

 // Copyright (c) 2006-2008 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/base/x509_certificate.h"
 
 #include <CommonCrypto/CommonDigest.h>
+#include <Security/Security.h>
 #include <time.h>
 
 #include "base/scoped_cftyperef.h"
 #include "base/logging.h"
 #include "base/pickle.h"
 #include "base/sys_string_conversions.h"
 #include "net/base/cert_status_flags.h"
 #include "net/base/cert_verify_result.h"
 #include "net/base/net_errors.h"
 
@@ skipping 55 matching lines @@
 
 void SetMacTestCertificate(X509Certificate* cert) {
   Singleton<MacTrustedCertificates>::get()->SetTestCertificate(cert);
 }
 
 namespace {
 
 typedef OSStatus (*SecTrustCopyExtendedResultFuncPtr)(SecTrustRef,
                                                       CFDictionaryRef*);
 
-inline bool CSSMOIDEqual(const CSSM_OID* oid1, const CSSM_OID* oid2) {
-  return oid1->Length == oid2->Length &&
-      (memcmp(oid1->Data, oid2->Data, oid1->Length) == 0);
-}
-
 int NetErrorFromOSStatus(OSStatus status) {
   switch (status) {
     case noErr:
       return OK;
     case errSecNotAvailable:
     case errSecNoCertificateModule:
     case errSecNoPolicyModule:
       return ERR_NOT_IMPLEMENTED;
     case errSecAuthFailed:
       return ERR_ACCESS_DENIED;
@@ skipping 68 matching lines @@
        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;
 }
 
-void ParsePrincipal(const CSSM_X509_NAME* name,
-                    X509Certificate::Principal* principal) {
-  std::vector<std::string> common_names, locality_names, state_names,
-      country_names;
-
-  // TODO(jcampan): add business_category and serial_number.
-  const CSSM_OID* kOIDs[] = { &CSSMOID_CommonName,
-                              &CSSMOID_LocalityName,
-                              &CSSMOID_StateProvinceName,
-                              &CSSMOID_CountryName,
-                              &CSSMOID_StreetAddress,
-                              &CSSMOID_OrganizationName,
-                              &CSSMOID_OrganizationalUnitName,
-                              &CSSMOID_DNQualifier };  // This should be "DC"
-                                                       // but is undoubtedly
-                                                       // wrong. TODO(avi):
-                                                       // Find the right OID.
-
-  std::vector<std::string>* values[] = {
-      &common_names, &locality_names,
-      &state_names, &country_names,
-      &(principal->street_addresses),
-      &(principal->organization_names),
-      &(principal->organization_unit_names),
-      &(principal->domain_components) };
-  DCHECK(arraysize(kOIDs) == arraysize(values));
-
-  for (size_t rdn = 0; rdn < name->numberOfRDNs; ++rdn) {
-    CSSM_X509_RDN rdn_struct = name->RelativeDistinguishedName[rdn];
-    for (size_t pair = 0; pair < rdn_struct.numberOfPairs; ++pair) {
-      CSSM_X509_TYPE_VALUE_PAIR pair_struct =
-          rdn_struct.AttributeTypeAndValue[pair];
-      for (size_t oid = 0; oid < arraysize(kOIDs); ++oid) {
-        if (CSSMOIDEqual(&pair_struct.type, kOIDs[oid])) {
-          std::string value =
-              std::string(reinterpret_cast<std::string::value_type*>
-                              (pair_struct.value.Data),
-                          pair_struct.value.Length);
-          values[oid]->push_back(value);
-          break;
-        }
-      }
-    }
-  }
-
-  // We don't expect to have more than one CN, L, S, and C.
-  std::vector<std::string>* single_value_lists[4] = {
-      &common_names, &locality_names, &state_names, &country_names };
-  std::string* single_values[4] = {
-      &principal->common_name, &principal->locality_name,
-      &principal->state_or_province_name, &principal->country_name };
-  for (size_t i = 0; i < arraysize(single_value_lists); ++i) {
-    DCHECK(single_value_lists[i]->size() <= 1);
-    if (single_value_lists[i]->size() > 0)
-      *(single_values[i]) = (*(single_value_lists[i]))[0];
-  }
-}
-
 struct CSSMFields {
   CSSMFields() : cl_handle(NULL), num_of_fields(0), fields(NULL) {}
   ~CSSMFields() {
     if (cl_handle)
       CSSM_CL_FreeFields(cl_handle, num_of_fields, &fields);
   }
 
   CSSM_CL_HANDLE cl_handle;
   uint32 num_of_fields;
   CSSM_FIELD_PTR fields;
@@ skipping 135 matching lines @@
     };
     err = SecPolicySetValue(*policy, &options_data);
     if (err) {
       CFRelease(*policy);
       return err;
     }
   }
   return noErr;
 }
 
+// Gets the issuer chain of intermediate and root certs for a given cert.
+// This function calls SecTrust but doesn't actually pay attention to the trust
+// result: it shouldn't be used to determine trust, just to traverse the chain.
+// Caller is responsible for releasing the value stored into *out_trust_chain.
+OSStatus CopyCertChain(SecCertificateRef cert_handle,

[wtc, 2010/03/24 23:52:05]

Nit: CopyCertChain also copies the entire cert chain,
including cert_handle itself, but this comment says only the
intermediate and root certs are copied.

Typo: out_trust_chain => out_cert_chain

+                       CFArrayRef* out_cert_chain) {
+  DCHECK(cert_handle && out_cert_chain);
+  // Create an SSL policy ref configured for client cert evaluation.
+  SecPolicyRef ssl_policy;
+  OSStatus result = X509Certificate::CreateSSLClientPolicy(&ssl_policy);
+  if (result)
+    return result;
+  scoped_cftyperef<SecPolicyRef> scoped_ssl_policy(ssl_policy);
+
+  // Create a SecTrustRef.
+  scoped_cftyperef<CFArrayRef> input_certs(
+      CFArrayCreate(NULL, (const void**)&cert_handle, 1,
+                    &kCFTypeArrayCallBacks));
+  SecTrustRef trust_ref = NULL;
+  result = SecTrustCreateWithCertificates(input_certs, ssl_policy, &trust_ref);
+  if (result)
+    return result;
+  scoped_cftyperef<SecTrustRef> trust(trust_ref);
+
+  // Evaluate trust, which creates the cert chain.
+  SecTrustResultType status;
+  CSSM_TP_APPLE_EVIDENCE_INFO* status_chain;
+  result = SecTrustEvaluate(trust, &status);
+  if (result)
+    return result;
+  return SecTrustGetResult(trust, &status, out_cert_chain, &status_chain);
+}
+
 }  // namespace
 
 void X509Certificate::Initialize() {
   const CSSM_X509_NAME* name;
   OSStatus status = SecCertificateGetSubject(cert_handle_, &name);
   if (!status) {
-    ParsePrincipal(name, &subject_);
+    subject_.Parse(name);
   }
   status = SecCertificateGetIssuer(cert_handle_, &name);
   if (!status) {
-    ParsePrincipal(name, &issuer_);
+    issuer_.Parse(name);
   }
 
   GetCertDateForOID(cert_handle_, CSSMOID_X509V1ValidityNotBefore,
                     &valid_start_);
   GetCertDateForOID(cert_handle_, CSSMOID_X509V1ValidityNotAfter,
                     &valid_expiry_);
 
   fingerprint_ = CalculateFingerprint(cert_handle_);
 }
 
@@ skipping 325 matching lines @@
     } else if (CSSMOIDEqual(&field.FieldOid, &CSSMOID_NetscapeCertType)) {
       uint16_t flags =
           *reinterpret_cast<const uint16_t*>(ext->value.parsedValue);
       if (flags & CE_NCT_SSL_Client)
         return true;
     }
   }
   return false;
 }
 
+bool X509Certificate::IsIssuedBy(
+    const std::vector<CertPrincipal>& valid_issuers) {
+  // Get the cert's issuer chain.
+  CFArrayRef trust_chain = NULL;

[wtc, 2010/03/24 23:52:05]

Nit: trust_chain => cert_chain
     scoped_trust_chain => scoped_cert_chain

+  OSStatus result;
+  result = CopyCertChain(os_cert_handle(), &trust_chain);
+  if (result != noErr)
+    return false;
+  scoped_cftyperef<CFArrayRef> scoped_trust_chain(trust_chain);
+
+  // Check all the certs in the chain for a match.
+  int n = CFArrayGetCount(trust_chain);
+  for (int i = 0; i < n; ++i) {
+    SecCertificateRef intermediate_handle = reinterpret_cast<SecCertificateRef>(

[wtc, 2010/03/24 23:52:05]

Nit: "intermediate" is misleading, because this can also be
an end-entity client cert or a root cert.

We can probably start with i = 1 because I think i = 0 is the
end-entity client cert.

[Jens Alfke, 2010/03/26 17:15:04]

It is, but we have to check the issuer of that cert.

[wtc, 2010/03/26 21:03:22]

In each iteration, you are checking intermediate->subject()
(at line 737 below).  Did you mean to check
intermediate->issuer()?

Nit: since the cert at index i=0 is not an intermediate
cert, the variable names |intermediate_handle| and
|intermediate| are inaccurate.  Perhaps rename them
|cert_handle| and |cert|?

+        const_cast<void*>(CFArrayGetValueAtIndex(trust_chain, i)));
+    CFRetain(intermediate_handle);
+    X509Certificate* intermediate = X509Certificate::CreateFromHandle(

[wtc, 2010/03/24 23:52:05]

X509Certificate is reference counted, so always store it in
a scoped_refptr.  Otherwise the reference is leaked.

+        intermediate_handle,
+        X509Certificate::SOURCE_LONE_CERT_IMPORT,
+        X509Certificate::OSCertHandles());
+    for (unsigned j = 0; j < valid_issuers.size(); j++)

[wtc, 2010/03/24 23:52:05]

Nit: use curly braces around the for loop's body, but not
around the if statement's one-liner body.

+      if (intermediate->subject().Matches(valid_issuers[j])) {
+        return true;
+      }
+  }
+  return false;
+}
+
 // static
 OSStatus X509Certificate::CreateSSLClientPolicy(SecPolicyRef* out_policy) {
   CSSM_APPLE_TP_SSL_OPTIONS tp_ssl_options = {
     CSSM_APPLE_TP_SSL_OPTS_VERSION,
     0,
     NULL,
     CSSM_APPLE_TP_SSL_CLIENT
   };
   return CreatePolicy(&CSSMOID_APPLE_TP_SSL,
                       &tp_ssl_options,
                       sizeof(tp_ssl_options),
                       out_policy);
 }
 
 // static
 bool X509Certificate::GetSSLClientCertificates (
     const std::string& server_domain,
+    const std::vector<Principal>& valid_issuers,
     std::vector<scoped_refptr<X509Certificate> >* certs) {
   scoped_cftyperef<SecIdentityRef> preferred_identity;
   if (!server_domain.empty()) {
     // See if there's an identity preference for this domain:
     scoped_cftyperef<CFStringRef> domain_str(
         base::SysUTF8ToCFStringRef("https://" + server_domain));
     SecIdentityRef identity = NULL;
     if (SecIdentityCopyPreference(domain_str,
                                   0,
                                   NULL,

[wtc, 2010/03/24 23:52:05]

IMPORTANT: we can finally pass a real validIssuers
argument to this SecIdentityCopyPreference call now.

[wtc, 2010/03/25 01:00:35]

I looked at the source code of SecIdentityCopyPreference in
Mac OS X 10.6.2 (libsecurity_keychain-37184/lib/SecIdentity.cpp).
It ignores the validIssuers argument.  Sigh.

    // filter on valid issuers, if provided
    if (validIssuers) {
        //%%%TBI
    }

Could you add a comment to note this?  Thanks.

                                   &identity) == noErr)
       preferred_identity.reset(identity);
   }
 
+  // Now enumerate the identities in the available keychains.
   SecIdentitySearchRef search = nil;
   OSStatus err = SecIdentitySearchCreate(NULL, CSSM_KEYUSE_SIGN, &search);
   scoped_cftyperef<SecIdentitySearchRef> scoped_search(search);
   while (!err) {
     SecIdentityRef identity = NULL;
     err = SecIdentitySearchCopyNext(search, &identity);
     if (err)
       break;
     scoped_cftyperef<SecIdentityRef> scoped_identity(identity);
 
@@ skipping 12 matching lines @@
     // Skip duplicates (a cert may be in multiple keychains).
     X509Certificate::Fingerprint fingerprint = cert->fingerprint();
     unsigned i;
     for (i = 0; i < certs->size(); ++i) {
       if ((*certs)[i]->fingerprint().Equals(fingerprint))
         break;
     }
     if (i < certs->size())
       continue;
 
+    bool is_preferred = preferred_identity &&
+        CFEqual(preferred_identity, identity);
+
+    // Make sure the issuer matches valid_issuers, if given.
+    // But an explicit cert preference overrides this.
+    if (!is_preferred &&
+        valid_issuers.size() > 0 &&
+        !cert->IsIssuedBy(valid_issuers))
+      continue;
+
     // The cert passes, so add it to the vector.
     // If it's the preferred identity, add it at the start (so it'll be
     // selected by default in the UI.)
-    if (preferred_identity && CFEqual(preferred_identity, identity))
+    if (is_preferred)
       certs->insert(certs->begin(), cert);
     else
       certs->push_back(cert);
   }
 
   if (err != errSecItemNotFound) {
     LOG(ERROR) << "SecIdentitySearch error " << err;
     return false;
   }
   return true;
 }
 
 CFArrayRef X509Certificate::CreateClientCertificateChain() const {
   // Initialize the result array with just the IdentityRef of the receiver:
   OSStatus result;
   SecIdentityRef identity;
   result = SecIdentityCreateWithCertificate(NULL, cert_handle_, &identity);
   if (result) {
     LOG(ERROR) << "SecIdentityCreateWithCertificate error " << result;
     return NULL;
   }
   scoped_cftyperef<CFMutableArrayRef> chain(
       CFArrayCreateMutable(NULL, 0, &kCFTypeArrayCallBacks));
   CFArrayAppendValue(chain, identity);
 
-  {
-    // Create an SSL policy ref configured for client cert evaluation.
-    SecPolicyRef ssl_policy;
-    result = CreateSSLClientPolicy(&ssl_policy);
-    if (result)
-      goto exit;
-    scoped_cftyperef<SecPolicyRef> scoped_ssl_policy(ssl_policy);
+  CFArrayRef trust_chain = NULL;

[wtc, 2010/03/24 23:52:05]

Nit: trust_chain => cert_chain

+  result = CopyCertChain(cert_handle_, &trust_chain);
+  if (result)
+    goto exit;
 
-    // Use a SecTrust object to find the intermediate certs in the trust chain.
-    scoped_cftyperef<CFArrayRef> input_certs(
-        CFArrayCreate(NULL, (const void**)&cert_handle_, 1,
-                      &kCFTypeArrayCallBacks));
-    SecTrustRef trust_ref = NULL;
-    result = SecTrustCreateWithCertificates(input_certs,
-                                            ssl_policy,
-                                            &trust_ref);
-    if (result)
-      goto exit;
-    scoped_cftyperef<SecTrustRef> trust(trust_ref);
-
-    SecTrustResultType status;
-    CFArrayRef trust_chain = NULL;
-    CSSM_TP_APPLE_EVIDENCE_INFO* status_chain;
-    result = SecTrustEvaluate(trust, &status);
-    if (result)
-      goto exit;
-    result = SecTrustGetResult(trust, &status, &trust_chain, &status_chain);
-    if (result)
-      goto exit;
-
-    // Append the intermediate certs from SecTrust to the result array:
-    if (trust_chain) {
-      int chain_count = CFArrayGetCount(trust_chain);
-      if (chain_count > 1) {
-        CFArrayAppendArray(chain,
-                           trust_chain,
-                           CFRangeMake(1, chain_count - 1));
-      }
-      CFRelease(trust_chain);
+  // Append the intermediate certs from SecTrust to the result array:
+  if (trust_chain) {
+    int chain_count = CFArrayGetCount(trust_chain);
+    if (chain_count > 1) {
+      CFArrayAppendArray(chain,
+                         trust_chain,
+                         CFRangeMake(1, chain_count - 1));
     }
+    CFRelease(trust_chain);
   }
 exit:
   if (result)
     LOG(ERROR) << "CreateIdentityCertificateChain error " << result;
   return chain.release();
 }
 
 }  // namespace net