Wire up SSL client authentication for OpenSSL/Android through the net/ stack

net/base/openssl_memory_private_key_store.cc

Index: net/base/openssl_memory_private_key_store.cc
diff --git a/net/base/openssl_memory_private_key_store.cc b/net/base/openssl_memory_private_key_store.cc
index 92716f236e1d341edc1f392bdcaa40bd44c0f726..16a85ad098a5f681b302fe88933bf7396e5ae54f 100644
--- a/net/base/openssl_memory_private_key_store.cc
+++ b/net/base/openssl_memory_private_key_store.cc
@@ -11,6 +11,7 @@
 #include "base/logging.h"
 #include "base/memory/singleton.h"
 #include "base/synchronization/lock.h"
+#include "net/base/openssl_util.h"
 #include "net/base/x509_certificate.h"
 
 namespace net {
@@ -22,36 +23,53 @@ class OpenSSLMemoryKeyStore : public OpenSSLPrivateKeyStore {
   OpenSSLMemoryKeyStore() {}
 
   static OpenSSLMemoryKeyStore* GetInstance() {
-    return Singleton<OpenSSLMemoryKeyStore>::get();
+    return Singleton<OpenSSLMemoryKeyStore,
+        OpenSSLMemoryKeyStoreLeakyTraits>::get();

[Ryan Sleevi, 2013/02/12 00:25:17]

style: indent to the <

[digit1, 2013/02/12 15:05:25]

Done.

   }
 
-  virtual ~OpenSSLMemoryKeyStore() {
+  virtual ~OpenSSLMemoryKeyStore() { }
+
+  virtual bool StoreKeyPair(const GURL& url, EVP_PKEY* pkey) OVERRIDE {
     base::AutoLock lock(lock_);
-    for (std::vector<EVP_PKEY*>::iterator it = keys_.begin();
-         it != keys_.end(); ++it) {
-      EVP_PKEY_free(*it);
-    }
+    // The 'pkey' holds both a private and a public key.
+    return pairs_.AddKeyPair(pkey, pkey);
   }
 
-  virtual bool StorePrivateKey(const GURL& url, EVP_PKEY* pkey) {
-    CRYPTO_add(&pkey->references, 1, CRYPTO_LOCK_EVP_PKEY);
+  virtual bool RecordClientCertPrivateKey(
+      const net::X509Certificate& client_cert,
+      EVP_PKEY* private_key) OVERRIDE {
+    // Sanity check.
+    if (private_key == NULL) {
+      LOG(ERROR) << "NULL private key for client certificate!";
+      return false;
+    }
+    // Get public key from certificate.
+    ScopedEVP_PKEY pub_key(GetCertificatePublicKeyOpenSSL(client_cert));
+    if (!pub_key.get()) {
+      LOG(ERROR) << "Can't extract public key from certificate!";
+      return false;
+    }
     base::AutoLock lock(lock_);
-    keys_.push_back(pkey);
-    return true;
+    return pairs_.AddKeyPair(pub_key.get(), private_key);
   }
 
-  virtual EVP_PKEY* FetchPrivateKey(EVP_PKEY* pkey) {
-    base::AutoLock lock(lock_);
-    for (std::vector<EVP_PKEY*>::iterator it = keys_.begin();
-         it != keys_.end(); ++it) {
-      if (EVP_PKEY_cmp(*it, pkey) == 1)
-        return *it;
+  virtual EVP_PKEY* FetchClientCertPrivateKey(
+      const X509Certificate& client_cert) {
+    ScopedEVP_PKEY pub_key(GetCertificatePublicKeyOpenSSL(client_cert));
+    if (!pub_key.get()) {
+      LOG(ERROR) << "Could not extract public key from client certificate";
+      return NULL;
     }
-    return NULL;
+    base::AutoLock lock(lock_);
+    return pairs_.FindPrivateKey(pub_key.get());
   }
 
  private:
-  std::vector<EVP_PKEY*> keys_;
+  friend struct DefaultSingletonTraits<OpenSSLMemoryKeyStore>;
+  typedef LeakySingletonTraits<OpenSSLMemoryKeyStore>
+      OpenSSLMemoryKeyStoreLeakyTraits;

[Ryan Sleevi, 2013/02/12 00:25:17]

Why is this typedef hanging off the class, and not off GetInstance?

Why are you friending DefaultSingletonTraits if you're leaking, by virtue of
LeakyTraits?

[digit1, 2013/02/12 15:05:25]

Most of this code new actually comes from the existing
openssl_private_key_store_android.cc implementation. Looking at the log, it
seems a leaky singleton is needed because the key store is being used from a
non-joinable worker thread that may still be running at shutdown. At least on
Android. I'm not too sure about linux_redux though. I've removed this to get
back to the previous state of the code. Let me know if this isn't correct.

+
+  OpenSSLKeyPairList pairs_;
   base::Lock lock_;
 
   DISALLOW_COPY_AND_ASSIGN(OpenSSLMemoryKeyStore);