Mac client-side SSL cert improvements.

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 <time.h>
 
 #include "base/scoped_cftyperef.h"
 #include "base/logging.h"
+#include "base/sys_string_conversions.h"

[wtc, 2010/02/24 01:44:51]

Nit: List "base/sys_string_conversions.h" after
"base/pickle.h"

 #include "base/pickle.h"
 #include "net/base/cert_status_flags.h"
 #include "net/base/cert_verify_result.h"
 #include "net/base/net_errors.h"
 
 using base::Time;
 
 namespace net {
 
 class MacTrustedCertificates {
@@ skipping 327 matching lines @@
       exploded.minute       = time.tm_min;
       exploded.second       = time.tm_sec;
       exploded.millisecond  = 0;
 
       *result = Time::FromUTCExploded(exploded);
       break;
     }
   }
 }
 
-// Returns true if this cert supports a given extended key usage.
-bool CertSupportsUsage(SecCertificateRef cert, const CSSM_OID& usage_oid) {
-  CSSMFields fields;
-  if (GetCertFields(cert, &fields) != noErr)
-    return false;
-  for (unsigned f = 0; f < fields.num_of_fields; ++f) {
-    const CSSM_FIELD &field = fields.fields[f];
-    if (CSSMOIDEqual(&field.FieldOid, &CSSMOID_ExtendedKeyUsage)) {
-      const CSSM_X509_EXTENSION* ext =
-          reinterpret_cast<const CSSM_X509_EXTENSION*>(field.FieldValue.Data);
-      const CE_ExtendedKeyUsage* usage =
-          reinterpret_cast<const CE_ExtendedKeyUsage*>(ext->value.parsedValue);
-      for (unsigned p = 0; p < usage->numPurposes; ++p) {
-        if (CSSMOIDEqual(&usage->purposes[p], &usage_oid))
-          return true;
-      }
-    }
-  }
-  return false;
-}
-
 // Creates a SecPolicyRef for the given OID, with optional value.
 OSStatus CreatePolicy(const CSSM_OID* policy_OID,
                       void* option_data,
                       size_t option_length,
                       SecPolicyRef* policy) {
   SecPolicySearchRef search;
   OSStatus err = SecPolicySearchCreate(CSSM_CERT_X_509v3, policy_OID, NULL,
                                        &search);
   if (err)
     return err;
@@ skipping 335 matching lines @@
     return sha1;
 
   DCHECK(NULL != cert_data.Data);
   DCHECK(0 != cert_data.Length);
 
   CC_SHA1(cert_data.Data, cert_data.Length, sha1.data);
 
   return sha1;
 }
 
+bool X509Certificate::SupportsSSLClientAuth() const {
+  CSSMFields fields;
+  if (GetCertFields(cert_handle_, &fields) != noErr)
+    return false;
+  for (unsigned f = 0; f < fields.num_of_fields; ++f) {
+    const CSSM_FIELD &field = fields.fields[f];

[wtc, 2010/02/24 01:44:51]

Nit: put & next to the type, CSSM_FIELD.

+    const CSSM_X509_EXTENSION* ext =
+        reinterpret_cast<const CSSM_X509_EXTENSION*>(field.FieldValue.Data);
+    if (CSSMOIDEqual(&field.FieldOid, &CSSMOID_ExtendedKeyUsage)) {
+      const CE_ExtendedKeyUsage* usage =
+          reinterpret_cast<const CE_ExtendedKeyUsage*>(ext->value.parsedValue);
+      for (unsigned p = 0; p < usage->numPurposes; ++p) {
+        if (CSSMOIDEqual(&usage->purposes[p], &CSSMOID_ClientAuth))
+          return true;
+      }
+    } 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;
+}
+
 // 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 (
+    std::string domain,
     std::vector<scoped_refptr<X509Certificate> >* certs) {
+  scoped_cftyperef<SecIdentityRef> preferred_identity;
+  if (domain != "") {

[wtc, 2010/02/24 01:44:51]

Is it better to say
  !domain.empty()
?

+    // See if there's an identity preference for this domain:
+    scoped_cftyperef<CFStringRef> domain_str(
+        base::SysUTF8ToCFStringRef("https://" + domain));
+    SecIdentityRef identity = NULL;
+    if (SecIdentityCopyPreference(domain_str,

[wtc, 2010/02/24 01:44:51]

The first argument should also contain the port, if it's not
443.

Where do we call SecIdentitySetPreference?

Is the identity preference stored persistently?

[wtc, 2010/02/24 20:13:51]

I see the difficulty of getting the port.  I forgot that we
pass only the hostname but not the port to the
SSLClientSocket constructor.  Perhaps we should fix that.
The port number is useful for SSL session reuse as well.

Whether different ports on the same post are considered as
the same server depends on the browser.  IE considers
them as the same server, whereas Firefox considers them
as different server instances.  If we want to treat
different ports as different server instances, than the
port needs to be included.

+                                  0,
+                                  NULL,
+                                  &identity) == noErr)
+      preferred_identity.reset(identity);
+  }
+
   SecIdentitySearchRef search = nil;
   OSStatus err = SecIdentitySearchCreate(NULL, CSSM_KEYUSE_SIGN, &search);
   fprintf(stderr,"====Created SecIdentitySearch %p\n",search);//TEMP
   scoped_cftyperef<SecIdentitySearchRef> scoped_search(search);
   while (!err) {
     SecIdentityRef identity = NULL;
     err = SecIdentitySearchCopyNext(search, &identity);
     if (err)
       break;
     scoped_cftyperef<SecIdentityRef> scoped_identity(identity);
 
     SecCertificateRef cert_handle;
     err = SecIdentityCopyCertificate(identity, &cert_handle);
     if (err != noErr)
       continue;
 
     scoped_refptr<X509Certificate> cert(
         CreateFromHandle(cert_handle, SOURCE_LONE_CERT_IMPORT));
     // cert_handle is adoped by cert, so I don't need to release it myself.
-    if (cert->HasExpired() ||
-        !CertSupportsUsage(cert_handle, CSSMOID_ClientAuth))
+    if (cert->HasExpired() || !cert->SupportsSSLClientAuth())
       continue;
 
     // 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;
 
     // The cert passes, so add it to the vector.
-    certs->push_back(cert);
+    // If it's the preferred identity, add it at the start (so it'll be
+    // selected by default in the UI.)

[wtc, 2010/02/24 01:44:51]

Nit: the period (.) should be after the closing parenthesis.

+    if (preferred_identity && CFEqual(preferred_identity, identity))
+      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() {
+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));
@@ skipping 40 matching lines @@
       CFRelease(trust_chain);
     }
   }
 exit:
   if (result)
     LOG(ERROR) << "CreateIdentityCertificateChain error " << result;
   return chain.release();
 }
 
 }  // namespace net