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

net/base/openssl_private_key_store.h

Index: net/base/openssl_private_key_store.h
diff --git a/net/base/openssl_private_key_store.h b/net/base/openssl_private_key_store.h
index edd54f38aa150159336a088bbc74475106a0c211..afd6c6fc76fe436f4accbfd343d35ce3e929107e 100644
--- a/net/base/openssl_private_key_store.h
+++ b/net/base/openssl_private_key_store.h
@@ -5,43 +5,47 @@
 #ifndef NET_BASE_OPENSSL_PRIVATE_KEY_STORE_H_
 #define NET_BASE_OPENSSL_PRIVATE_KEY_STORE_H_
 
-#include "base/basictypes.h"
+#include <openssl/evp.h>
+
+#include <vector>
 
-// Avoid including <openssl/evp.h> here.
-typedef struct evp_pkey_st EVP_PKEY;
+#include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/synchronization/lock.h"
+#include "crypto/openssl_util.h"
+#include "net/base/net_export.h"
 
 class GURL;
 
 namespace net {
 
-// Defines an abstract store for private keys; the OpenSSL library does not
-// provide this service so it is left to individual platforms to provide it.
-//
-// The contract is that the private key will be stored in an appropriate secure
-// system location, and be available to the SSLClientSocketOpenSSL when using a
-// client certificate created against the associated public key for client
-// authentication.
-class OpenSSLPrivateKeyStore {
+class X509Certificate;
+
+// OpenSSLPrivateKeyStore provides an interface for storing
+// public/private key pairs to system storage on platforms where
+// OpenSSL is used.
+// This class shall only be used from the network thread.
+class NET_EXPORT OpenSSLPrivateKeyStore {
  public:
   // Platforms must define this factory function as appropriate.
   static OpenSSLPrivateKeyStore* GetInstance();
 
-  virtual ~OpenSSLPrivateKeyStore() {}
-
-  // Called to store a private key generated via <keygen> while visiting |url|.
-  // Does not takes ownership of |pkey|, the caller reamins responsible to
-  // EVP_PKEY_free it. (Internally, a copy maybe made or the reference count
-  // incremented).
+  // Called to permanently store a private/public key pair, generated
+  // via <keygen> while visiting |url|, to an appropriate system
+  // location. Increments |pkey|'s reference count, so the caller is still
+  // responsible for calling EVP_PKEY_free on it. Note that this has
+  // nothing to do with the methods RecordClientCertPrivateKey() and
+  // FetchClientCertPrivateKey() below.
+  // |url| is the corresponding server URL.
+  // |pkey| is the key pair handle.
   // Returns false if an error occurred whilst attempting to store the key.
-  virtual bool StorePrivateKey(const GURL& url, EVP_PKEY* pkey) = 0;
-
-  // Given a |public_key| part returns the corresponding private key, or NULL
-  // if no key found. Does NOT return ownership.
-  virtual EVP_PKEY* FetchPrivateKey(EVP_PKEY* public_key) = 0;
+  virtual bool StoreKeyPair(const GURL& url, EVP_PKEY* pkey) = 0;
 
  protected:
   OpenSSLPrivateKeyStore() {}
 
+  virtual ~OpenSSLPrivateKeyStore() {}

[Ryan Sleevi, 2013/02/27 01:08:20]

Does this really need to be virtual with multiple implementations?

Seems like the (Android || Memory) are mutually exclusive, and can be both
concrete implementations (eg: no virtuals needed)

If you intended to be able to mock this out (eg: in higher-layer unit tests),
that makes sense, but I didn't see any Mock versions, nor am I sure how/where
the unit tests would fit, so I wanted to make sure it's really what you
intended.

[digit1, 2013/02/27 10:24:27]

I was just trying to make a minimally viable change here. There are no plans to
mock this class for now, so I've 'simplified' the implementation.

(just to be clear, I intend to introduce net::TestClientCertStore instead for
higher-level unit tests).

+
  private:
   DISALLOW_COPY_AND_ASSIGN(OpenSSLPrivateKeyStore);
 };