Support intermediate certificates for OpenSSL client auth.

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 a6f5caf188d96dac7c1a339e732990cb05ebd6f3..68df497f04d7cc046452b19e5ff233f9746bc0a1 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -57,7 +57,13 @@ const int kNoPendingReadResult = 1;
 // the server supports NPN, choosing "http/1.1" is the best answer.
 const char kDefaultSupportedNPNProtocol[] = "http/1.1";
 
+void FreeX509Stack(STACK_OF(X509) * ptr) {

[Ryan Sleevi, 2014/08/14 01:27:17]

STACK_OF(X509)* right?

[davidben, 2014/08/14 16:50:16]

Done.

+  sk_X509_pop_free(ptr, X509_free);
+}
+
 typedef crypto::ScopedOpenSSL<X509, X509_free>::Type ScopedX509;
+typedef crypto::ScopedOpenSSL<STACK_OF(X509), FreeX509Stack>::Type
+    ScopedX509Stack;
 
 #if OPENSSL_VERSION_NUMBER < 0x1000103fL
 // This method doesn't seem to have made it into the OpenSSL headers.
@@ -95,10 +101,6 @@ int GetNetSSLVersion(SSL* ssl) {
   }
 }
 
-void FreeX509Stack(STACK_OF(X509) * ptr) {
-  sk_X509_pop_free(ptr, X509_free);
-}
-
 ScopedX509 OSCertHandleToOpenSSL(
     X509Certificate::OSCertHandle os_handle) {
 #if defined(USE_OPENSSL_CERTS)
@@ -112,6 +114,18 @@ ScopedX509 OSCertHandleToOpenSSL(
 #endif  // defined(USE_OPENSSL_CERTS)
 }
 
+ScopedX509Stack OSCertHandlesToOpenSSL(
+    const X509Certificate::OSCertHandles& os_handles) {
+  ScopedX509Stack stack(sk_X509_new_null());
+  for (size_t i = 0; i < os_handles.size(); i++) {
+    ScopedX509 x509 = OSCertHandleToOpenSSL(os_handles[i]);
+    if (!x509)
+      return ScopedX509Stack();
+    sk_X509_push(stack.get(), x509.release());
+  }
+  return stack.Pass();
+}
+
 }  // namespace
 
 class SSLClientSocketOpenSSL::SSLContext {
@@ -142,7 +156,7 @@ class SSLClientSocketOpenSSL::SSLContext {
     ssl_ctx_.reset(SSL_CTX_new(SSLv23_client_method()));
     session_cache_.Reset(ssl_ctx_.get(), kDefaultSessionCacheConfig);
     SSL_CTX_set_cert_verify_callback(ssl_ctx_.get(), CertVerifyCallback, NULL);
-    SSL_CTX_set_client_cert_cb(ssl_ctx_.get(), ClientCertCallback);
+    SSL_CTX_set_cert_cb(ssl_ctx_.get(), ClientCertRequestCallback, NULL);
     SSL_CTX_set_verify(ssl_ctx_.get(), SSL_VERIFY_PEER, NULL);
     // TODO(kristianm): Only select this if ssl_config_.next_proto is not empty.
     // It would be better if the callback were not a global setting,
@@ -160,10 +174,10 @@ class SSLClientSocketOpenSSL::SSLContext {
 
   static SSLSessionCacheOpenSSL::Config kDefaultSessionCacheConfig;
 
-  static int ClientCertCallback(SSL* ssl, X509** x509, EVP_PKEY** pkey) {
+  static int ClientCertRequestCallback(SSL* ssl, void* arg) {
     SSLClientSocketOpenSSL* socket = GetInstance()->GetClientSocketFromSSL(ssl);
     CHECK(socket);
-    return socket->ClientCertRequestCallback(ssl, x509, pkey);
+    return socket->ClientCertRequestCallback(ssl);
   }
 
   static int CertVerifyCallback(X509_STORE_CTX *store_ctx, void *arg) {
@@ -226,9 +240,6 @@ class SSLClientSocketOpenSSL::PeerCertificateChain {
   bool IsValid() { return os_chain_.get() && openssl_chain_.get(); }
 
  private:
-  typedef crypto::ScopedOpenSSL<STACK_OF(X509), FreeX509Stack>::Type
-      ScopedX509Stack;
-
   ScopedX509Stack openssl_chain_;
 
   scoped_refptr<X509Certificate> os_chain_;
@@ -1395,13 +1406,9 @@ int SSLClientSocketOpenSSL::TransportReadComplete(int result) {
   return result;
 }
 
-int SSLClientSocketOpenSSL::ClientCertRequestCallback(SSL* ssl,
-                                                      X509** x509,
-                                                      EVP_PKEY** pkey) {
+int SSLClientSocketOpenSSL::ClientCertRequestCallback(SSL* ssl) {
   DVLOG(3) << "OpenSSL ClientCertRequestCallback called";
   DCHECK(ssl == ssl_);
-  DCHECK(*x509 == NULL);
-  DCHECK(*pkey == NULL);
 
 #if defined(OS_IOS)
   // TODO(droger): Support client auth on iOS. See http://crbug.com/145954).
@@ -1435,7 +1442,6 @@ int SSLClientSocketOpenSSL::ClientCertRequestCallback(SSL* ssl,
 
   // Second pass: a client certificate should have been selected.
   if (ssl_config_.client_cert.get()) {
-    // TODO(davidben): Configure OpenSSL to also send the intermediates.
     ScopedX509 leaf_x509 =
         OSCertHandleToOpenSSL(ssl_config_.client_cert->os_cert_handle());
     if (!leaf_x509) {
@@ -1444,6 +1450,14 @@ int SSLClientSocketOpenSSL::ClientCertRequestCallback(SSL* ssl,
       return -1;
     }
 
+    ScopedX509Stack chain = OSCertHandlesToOpenSSL(
+        ssl_config_.client_cert->GetIntermediateCertificates());
+    if (!chain) {
+      LOG(WARNING) << "Failed to import intermediate certificates";
+      OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_CERT_BAD_FORMAT);
+      return -1;
+    }
+
     // TODO(davidben): With Linux client auth support, this should be
     // conditioned on OS_ANDROID and then, with https://crbug.com/394131,
     // removed altogether. OpenSSLClientKeyStore is mostly an artifact of the
@@ -1464,16 +1478,18 @@ int SSLClientSocketOpenSSL::ClientCertRequestCallback(SSL* ssl,
       return -1;
     }
 
-    // TODO(joth): (copied from NSS) We should wait for server certificate
-    // verification before sending our credentials. See http://crbug.com/13934
-    *x509 = leaf_x509.release();
-    *pkey = privkey.release();
+    if (!SSL_use_certificate(ssl_, leaf_x509.get()) ||
+        !SSL_use_PrivateKey(ssl_, privkey.get()) ||
+        !SSL_set1_chain(ssl_, chain.get())) {
+      LOG(WARNING) << "Failed to set client certificate";
+      return -1;
+    }
     return 1;
   }
 #endif  // defined(OS_IOS)
 
   // Send no client certificate.
-  return 0;
+  return 1;

[Ryan Sleevi, 2014/08/14 01:27:18]

Does this guarantee that the sockets are cleared (e.g. in the event of
renegotiation)?

[davidben, 2014/08/14 16:50:16]

No, although the old one didn't either. (The old one wouldn't even run as soon
as you'd previously set a certificate.) I've put an SSL_certs_clear at the top
to reset it, although I don't think it can matter much with this code because
ssl_config_.client_cert never changes. Maybe if the key no longer exists and we
want to fail earlier...

 }
 
 int SSLClientSocketOpenSSL::CertVerifyCallback(X509_STORE_CTX* store_ctx) {