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..a0c9b2361da7524683a48147095eb60f6c0bac56 100644
--- a/net/base/openssl_private_key_store.h
+++ b/net/base/openssl_private_key_store.h
@@ -14,13 +14,11 @@ class GURL;
 
 namespace net {
 
+class X509Certificate;
+
 // 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 {
  public:
   // Platforms must define this factory function as appropriate.
@@ -28,16 +26,34 @@ class OpenSSLPrivateKeyStore {
 
   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 store a private/public key pair, generated via <keygen> while
+  // visiting |url|, to an appropriate secure system location.
+  // Increments |pkey|'s reference count, so the caller is still responsible
+  // for calling EVP_PKEY_free on it.
+  // |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;
+  virtual bool StoreKeyPair(const GURL& url, EVP_PKEY* pkey) = 0;
+
+  // Record the association between a certificate and its private key.
+  // This method should be called _before_ FetchPrivateKey to ensure that
+  // the private key is returned when it is called later.
+  // |cert| is a handle to a certificate object.
+  // |private_key| is an OpenSSL EVP_PKEY that corresponds to the
+  // certificate's private key.
+  // Returns false if an error occured.
+  // This function does not take ownership of the private_key, but may
+  // increment its internal reference count.

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

comment nit: That is the very definition of taking ownership in a refcounted
system.

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

I'll rephrase that. I was just paraphrasing the original comment to make it
clearer. Obviously this didn't work as expected :)

+  virtual bool RecordClientCertPrivateKey(const X509Certificate& cert,
+                                          EVP_PKEY* private_key) = 0;

[Ryan Sleevi, 2013/02/12 20:12:58]

Nowhere in Chromium do we pass a "const X509Certificate&". It's always treated
as a "const X509Certificate*" or a "X509Certificate*".

[digit1, 2013/02/13 18:24:34]

I think a simple grep would show otherwise. However, that's true for the net/
code, so I've changed it too.

 
-  // 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;
+  // Given a certificate's |public_key|, return the corresponding private
+  // key that has been recorded previously by RecordClientCertPrivateKey().
+  // |public_key| must contain the certificate's public key.
+  // Returns a handle to the private key's EVP_PKEY object. Caller must
+  // call EVP_PKEY_free() to free it.

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

Can you not return a scoped_ptr<EVP_PKEY, EVP_PKEY_free> then?

[digit1, 2013/02/13 18:24:34]

Done.

+  virtual EVP_PKEY* FetchClientCertPrivateKey(
+      const X509Certificate& cert) = 0;
 
  protected:
   OpenSSLPrivateKeyStore() {}