Plumbing SSLPrivateKey

net/ssl/ssl_platform_key_mac.cc

 // Copyright 2015 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/ssl_platform_key.h"
 
 #include <openssl/ecdsa.h>
 #include <openssl/obj.h>
 #include <openssl/rsa.h>
 
@@ skipping 18 matching lines @@
 #include "crypto/scoped_openssl_types.h"
 #include "net/base/net_errors.h"
 #include "net/cert/x509_certificate.h"
 #include "net/ssl/ssl_private_key.h"
 #include "net/ssl/threaded_ssl_private_key.h"
 
 namespace net {
 
 namespace {
 
+base::LazyInstance<SSLPlatformKeyTaskRunner>::Leaky g_platform_key_task_runner =
+    LAZY_INSTANCE_INITIALIZER;
+
 class ScopedCSSM_CC_HANDLE {
  public:
   ScopedCSSM_CC_HANDLE() : handle_(0) {}
   explicit ScopedCSSM_CC_HANDLE(CSSM_CC_HANDLE handle) : handle_(handle) {}
 
   ~ScopedCSSM_CC_HANDLE() { reset(); }
 
   CSSM_CC_HANDLE get() const { return handle_; }
 
   void reset() {
@@ skipping 156 matching lines @@
 
  private:
   base::ScopedCFTypeRef<SecKeyRef> key_;
   const CSSM_KEY* cssm_key_;
 
   DISALLOW_COPY_AND_ASSIGN(SSLPlatformKeyMac);
 };
 
 }  // namespace
 
-scoped_ptr<SSLPrivateKey> FetchClientCertPrivateKey(
-    X509Certificate* certificate,
-    scoped_refptr<base::SequencedTaskRunner> task_runner) {
+scoped_refptr<SSLPrivateKey> FetchClientCertPrivateKey(
+    X509Certificate* certificate) {
+  if (!certificate) {

[davidben, 2015/09/25 20:10:12]

Any reason not to have the caller handle null? This seems the sort of function
that doesn't really have a reason to allow null when you ask it to do something
with a certificate.

[svaldez, 2015/09/28 16:54:53]

Then we'd have to check both at the caller that the argument isn't null and also
check that the private key returned isn't null. A null cert and a null private
key should continue down the same code path, since they both mean we can't use
this cert.

[davidben, 2015/10/13 20:32:15]

In general, APIs shouldn't silently try to handle null unless what the function
does inherently wants a nullable type. I'm not sure this does. There also aren't
that many callers of FetchClientCertPrivateKey which actually can have a null
input it seems. Just the SSLClientAuthObserver thing as far as I can tell.

There's even more implementations of this function, actually. :-)

[svaldez, 2015/10/14 15:06:18]

Done.

+    return nullptr;
+  }
+
   // Look up the private key.
   base::ScopedCFTypeRef<SecKeyRef> private_key(
       FetchSecKeyRefForCertificate(certificate));
   if (!private_key)
     return nullptr;
 
   const CSSM_KEY* cssm_key;
   OSStatus status = SecKeyGetCSSMKey(private_key.get(), &cssm_key);
   if (status != noErr)
     return nullptr;
 
   if (cssm_key->KeyHeader.AlgorithmId != CSSM_ALGID_RSA &&
       cssm_key->KeyHeader.AlgorithmId != CSSM_ALGID_ECDSA) {
     LOG(ERROR) << "Unknown key type: " << cssm_key->KeyHeader.AlgorithmId;
     return nullptr;
   }
-  return make_scoped_ptr(new ThreadedSSLPrivateKey(
+  return make_scoped_refptr(new ThreadedSSLPrivateKey(
       make_scoped_ptr(new SSLPlatformKeyMac(private_key.get(), cssm_key)),
-      task_runner.Pass()));
+      g_platform_key_task_runner.Get().task_runner().Pass()));
 }
 
 }  // namespace net