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

net/socket/ssl_client_socket_openssl.cc

Index: net/socket/ssl_client_socket_openssl.cc
diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc
index c49326d89fefd85971c4b8ba3ee8b8ab15a4296e..061098261aaf0aa3dab1847cada3ec5e2f1c067d 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -590,15 +590,18 @@ int SSLClientSocketOpenSSL::ClientCertRequestCallback(SSL* ssl,
 
   // Second pass: a client certificate should have been selected.
   if (ssl_config_.client_cert) {
-    EVP_PKEY* privkey = OpenSSLPrivateKeyStore::GetInstance()->FetchPrivateKey(
-        X509_PUBKEY_get(X509_get_X509_PUBKEY(
-            ssl_config_.client_cert->os_cert_handle())));
-    if (privkey) {
+    // A note about ownership: FetchClientCertPrivateKey() increments
+    // the reference count of the EVP_PKEY. Ownership of this reference
+    // is passed directly to OpenSSL, which will release the reference
+    // using EVP_PKEY_free() when the SSL object is destroyed.
+    OpenSSLPrivateKeyStore::ScopedEVP_PKEY privkey;
+    if (OpenSSLPrivateKeyStore::GetInstance()->FetchClientCertPrivateKey(
+            ssl_config_.client_cert.get(), &privkey)) {
       // TODO(joth): (copied from NSS) We should wait for server certificate
       // verification before sending our credentials. See http://crbug.com/13934
       *x509 = X509Certificate::DupOSCertHandle(
           ssl_config_.client_cert->os_cert_handle());
-      *pkey = privkey;
+      *pkey = privkey.release();
       return 1;
     }
     LOG(WARNING) << "Client cert found without private key";
@@ -612,6 +615,10 @@ int SSLClientSocketOpenSSL::ClientCertRequestCallback(SSL* ssl,
 
 bool SSLClientSocketOpenSSL::GetSSLInfo(SSLInfo* ssl_info) {
   ssl_info->Reset();
+
+  ssl_info->client_cert_sent =
+      ssl_config_.send_client_cert && ssl_config_.client_cert;

[Ryan Sleevi, 2013/02/25 19:51:07]

Why did you move this here? This seems to violate the invariants that previously
existed.

[digit1, 2013/02/26 11:03:13]

Because this would always return a false value in |client_cert_sent| even if the
client certificates were actually sent, but the connection was refused by the
server.

Give that we don't have a unit test where the server actually refuses the
connection anymore, I've removed this.

+
   if (!server_cert_)
     return false;
 
@@ -621,8 +628,6 @@ bool SSLClientSocketOpenSSL::GetSSLInfo(SSLInfo* ssl_info) {
       server_cert_verify_result_.is_issued_by_known_root;
   ssl_info->public_key_hashes =
     server_cert_verify_result_.public_key_hashes;
-  ssl_info->client_cert_sent =
-      ssl_config_.send_client_cert && ssl_config_.client_cert;
   ssl_info->channel_id_sent = WasChannelIDSent();
 
   const SSL_CIPHER* cipher = SSL_get_current_cipher(ssl_);