Fix client certificate authentication on Mac and Linux introduced in r178732

net/ssl/client_cert_store_impl_mac.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/ssl/client_cert_store_impl.h"
 
 #include <CommonCrypto/CommonDigest.h>
 #include <CoreFoundation/CFArray.h>
 #include <CoreServices/CoreServices.h>
 #include <Security/SecBase.h>
 #include <Security/Security.h>
 
 #include <algorithm>
 #include <string>
 
 #include "base/logging.h"
+#include "base/mac/foundation_util.h"
 #include "base/mac/mac_logging.h"
 #include "base/mac/scoped_cftyperef.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/synchronization/lock.h"
 #include "crypto/mac_security_services_lock.h"
 #include "net/base/host_port_pair.h"
 #include "net/cert/x509_util.h"
 
 using base::mac::ScopedCFTypeRef;
 
 namespace net {
 
 namespace {
 
+// Gets the issuer for a given cert, starting with the cert itself and
+// including the intermediate and finally root certificates (if any).
+// 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_cert_chain.
+OSStatus CopyCertChain(SecCertificateRef cert_handle,
+                       CFArrayRef* out_cert_chain) {
+  DCHECK(cert_handle);
+  DCHECK(out_cert_chain);
+
+  // Create an SSL policy ref configured for client cert evaluation.
+  SecPolicyRef ssl_policy;
+  OSStatus result = x509_util::CreateSSLClientPolicy(&ssl_policy);
+  if (result)
+    return result;
+  ScopedCFTypeRef<SecPolicyRef> scoped_ssl_policy(ssl_policy);
+
+  // Create a SecTrustRef.
+  ScopedCFTypeRef<CFArrayRef> input_certs(CFArrayCreate(
+      NULL, const_cast<const void**>(reinterpret_cast<void**>(&cert_handle)),
+      1, &kCFTypeArrayCallBacks));
+  SecTrustRef trust_ref = NULL;
+  {
+    base::AutoLock lock(crypto::GetMacSecurityServicesLock());
+    result = SecTrustCreateWithCertificates(input_certs, ssl_policy,
+                                            &trust_ref);
+  }
+  if (result)
+    return result;
+  ScopedCFTypeRef<SecTrustRef> trust(trust_ref);
+
+  // Evaluate trust, which creates the cert chain.
+  SecTrustResultType status;
+  CSSM_TP_APPLE_EVIDENCE_INFO* status_chain;
+  {
+    base::AutoLock lock(crypto::GetMacSecurityServicesLock());
+    result = SecTrustEvaluate(trust, &status);
+  }
+  if (result)
+    return result;
+  {
+    base::AutoLock lock(crypto::GetMacSecurityServicesLock());
+    result = SecTrustGetResult(trust, &status, out_cert_chain, &status_chain);
+  }
+  return result;
+}
+
+// Returns true if |*cert| is issued by an authority in |valid_issuers|
+// according to Keychain Services, rather than using |cert|'s intermediate
+// certificates. If it is, |*cert| is updated to point to the completed
+// certificate
+bool IsIssuedByInKeychain(const std::vector<std::string>& valid_issuers,
+                          scoped_refptr<X509Certificate>* cert) {
+  DCHECK(cert);
+  DCHECK(*cert);
+
+  X509Certificate::OSCertHandle cert_handle = (*cert)->os_cert_handle();
+  CFArrayRef cert_chain = NULL;
+  OSStatus result = CopyCertChain(cert_handle, &cert_chain);
+  if (result) {
+    OSSTATUS_LOG(ERROR, result) << "CopyCertChain error";
+    return false;
+  }
+
+  if (!cert_chain)
+    return false;
+
+  X509Certificate::OSCertHandles intermediates;
+  for (CFIndex i = 1, chain_count = CFArrayGetCount(cert_chain);
+       i < chain_count; ++i) {
+    SecCertificateRef cert = base::mac::CFCastStrict<SecCertificateRef>(
+        CFArrayGetValueAtIndex(cert_chain, i));
+    intermediates.push_back(cert);
+  }
+
+  scoped_refptr<X509Certificate> new_cert(X509Certificate::CreateFromHandle(
+      cert_handle, intermediates));
+  CFRelease(cert_chain);  // Also frees |intermediates|.
+
+  if (!new_cert->IsIssuedByEncoded(valid_issuers))

[Ryan Sleevi, 2013/04/16 02:32:50]

The downside to this approach is that |new_cert->os_cert_handle()| is checked
twice (first on line 147, since it's the same OSCertHandle, then again here). On
OS X, this forces the cert to be parsed twice into its NSS
representation/CachedCert, but I think this is a minor overhead for the code
simplicity - and for keeping this merge friendly (eg: not changing
IsIssuedByEncoded)

+    return false;
+
+  cert->swap(new_cert);
+  return true;
+}
+
 bool GetClientCertsImpl(const scoped_refptr<X509Certificate>& preferred_cert,
                         const CertificateList& regular_certs,
                         const SSLCertRequestInfo& request,
+                        bool query_keychain,

[wtc, 2013/04/16 18:50:32]

query_keychain probably should be documented.

It's not clear to me when query_keychain should be true.
Based on the three callers of this function, I can see the
rule is whether the caller is trying to *get* or *select*
client certs. But I don't know the rationale of this rule.
It would be nice to document this for future maintainers of
this code.

[Ryan Sleevi, 2013/04/16 19:00:26]

Good point. Will add comments to these functions. Also, see reply to NSS code
for more explanation.

                         CertificateList* selected_certs) {
   CertificateList preliminary_list;
   if (preferred_cert)
     preliminary_list.push_back(preferred_cert);
   preliminary_list.insert(preliminary_list.end(), regular_certs.begin(),
                           regular_certs.end());
 
   selected_certs->clear();
   for (size_t i = 0; i < preliminary_list.size(); ++i) {
     scoped_refptr<X509Certificate>& cert = preliminary_list[i];
     if (cert->HasExpired() || !cert->SupportsSSLClientAuth())
       continue;
 
     // Skip duplicates (a cert may be in multiple keychains).
     const SHA1HashValue& fingerprint = cert->fingerprint();
     size_t pos;
     for (pos = 0; pos < selected_certs->size(); ++pos) {
       if ((*selected_certs)[pos]->fingerprint().Equals(fingerprint))
         break;
     }
     if (pos < selected_certs->size())
       continue;
 
     // Check if the certificate issuer is allowed by the server.
-    if (!request.cert_authorities.empty() &&
-        !cert->IsIssuedByEncoded(request.cert_authorities)) {
-      continue;
+    if (request.cert_authorities.empty() ||
+        cert->IsIssuedByEncoded(request.cert_authorities) ||
+        (query_keychain &&
+         IsIssuedByInKeychain(request.cert_authorities, &cert))) {
+      selected_certs->push_back(cert);
     }
-    selected_certs->push_back(cert);
   }
 
   // Preferred cert should appear first in the ui, so exclude it from the
   // sorting.
   CertificateList::iterator sort_begin = selected_certs->begin();
   CertificateList::iterator sort_end = selected_certs->end();
   if (preferred_cert &&
       sort_begin != sort_end &&
       *sort_begin == preferred_cert) {
     ++sort_begin;
@@ skipping 67 matching lines @@
     } else {
       regular_certs.push_back(cert);
     }
   }
 
   if (err != errSecItemNotFound) {
     OSSTATUS_LOG(ERROR, err) << "SecIdentitySearch error";
     return false;
   }
 
-  return GetClientCertsImpl(preferred_cert, regular_certs, request,
+  return GetClientCertsImpl(preferred_cert, regular_certs, request, true,
                             selected_certs);
 }
 
 bool ClientCertStoreImpl::SelectClientCerts(const CertificateList& input_certs,
                                             const SSLCertRequestInfo& request,
                                             CertificateList* selected_certs) {
-  return GetClientCertsImpl(NULL, input_certs, request,
+  return GetClientCertsImpl(NULL, input_certs, request, false,
                             selected_certs);
 }
 
 #if defined(OS_MACOSX) && !defined(OS_IOS)
 bool ClientCertStoreImpl::SelectClientCertsGivenPreferred(
       const scoped_refptr<X509Certificate>& preferred_cert,
       const CertificateList& regular_certs,
       const SSLCertRequestInfo& request,
       CertificateList* selected_certs) {
-  return GetClientCertsImpl(preferred_cert, regular_certs, request,
+  return GetClientCertsImpl(preferred_cert, regular_certs, request, false,
                             selected_certs);
 }
 #endif
 
 }  // namespace net