Sign CertificateVerify messages on a background thread.

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 d06f5ac48a67fd0e282b820a0269ed53e78aef18..b9198c621700ce97e3f8416b87c5670d13699a07 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -20,9 +20,11 @@
 #include "base/memory/singleton.h"
 #include "base/metrics/histogram.h"
 #include "base/profiler/scoped_tracker.h"
+#include "base/stl_util.h"
 #include "base/strings/string_piece.h"
 #include "base/synchronization/lock.h"
 #include "base/threading/thread_local.h"
+#include "base/threading/worker_pool.h"
 #include "base/values.h"
 #include "crypto/ec_private_key.h"
 #include "crypto/openssl_util.h"
@@ -41,6 +43,7 @@
 #include "net/ssl/ssl_connection_status_flags.h"
 #include "net/ssl/ssl_failure_state.h"
 #include "net/ssl/ssl_info.h"
+#include "net/ssl/ssl_private_key.h"
 
 #if defined(OS_WIN)
 #include "base/win/windows_version.h"
@@ -49,7 +52,7 @@
 #if defined(USE_OPENSSL_CERTS)
 #include "net/ssl/openssl_client_key_store.h"
 #else
-#include "net/ssl/openssl_platform_key.h"
+#include "net/ssl/ssl_platform_key.h"
 #endif
 
 namespace net {
@@ -67,7 +70,7 @@ namespace {
 
 // This constant can be any non-negative/non-zero value (eg: it does not
 // overlap with any value of the net::Error range, including net::OK).
-const int kNoPendingReadResult = 1;
+const int kNoPendingResult = 1;
 
 // If a client doesn't have a list of protocols that it supports, but
 // the server supports NPN, choosing "http/1.1" is the best answer.
@@ -144,6 +147,34 @@ int LogErrorCallback(const char* str, size_t len, void* context) {
   return 1;
 }
 
+bool EVP_MDToPrivateKeyHash(const EVP_MD* md, SSLPrivateKey::Hash* hash) {
+  switch (EVP_MD_type(md)) {
+    case NID_md5_sha1:
+      *hash = SSLPrivateKey::Hash::MD5_SHA1;
+      return true;
+    case NID_md5:
+      *hash = SSLPrivateKey::Hash::MD5;
+      return true;
+    case NID_sha1:
+      *hash = SSLPrivateKey::Hash::SHA1;
+      return true;
+    case NID_sha224:
+      *hash = SSLPrivateKey::Hash::SHA224;
+      return true;
+    case NID_sha256:
+      *hash = SSLPrivateKey::Hash::SHA256;
+      return true;
+    case NID_sha384:
+      *hash = SSLPrivateKey::Hash::SHA384;
+      return true;
+    case NID_sha512:
+      *hash = SSLPrivateKey::Hash::SHA512;
+      return true;
+    default:
+      return false;
+  }
+}
+
 }  // namespace
 
 class SSLClientSocketOpenSSL::SSLContext {
@@ -349,7 +380,7 @@ SSLClientSocketOpenSSL::SSLClientSocketOpenSSL(
     const SSLClientSocketContext& context)
     : transport_send_busy_(false),
       transport_recv_busy_(false),
-      pending_read_error_(kNoPendingReadResult),
+      pending_read_error_(kNoPendingResult),
       pending_read_ssl_error_(SSL_ERROR_NONE),
       transport_read_error_(OK),
       transport_write_error_(OK),
@@ -371,6 +402,7 @@ SSLClientSocketOpenSSL::SSLClientSocketOpenSSL(
       handshake_completed_(false),
       certificate_verified_(false),
       ssl_failure_state_(SSL_FAILURE_NONE),
+      signature_result_(kNoPendingResult),
       transport_security_state_(context.transport_security_state),
       policy_enforcer_(context.cert_policy_enforcer),
       net_log_(transport_->socket()->NetLog()),
@@ -492,7 +524,7 @@ void SSLClientSocketOpenSSL::Disconnect() {
   user_write_buf_        = NULL;
   user_write_buf_len_    = 0;
 
-  pending_read_error_ = kNoPendingReadResult;
+  pending_read_error_ = kNoPendingResult;
   pending_read_ssl_error_ = SSL_ERROR_NONE;
   pending_read_error_info_ = OpenSSLErrorInfo();
 
@@ -515,6 +547,10 @@ void SSLClientSocketOpenSSL::Disconnect() {
   certificate_verified_ = false;
   channel_id_request_.Cancel();
   ssl_failure_state_ = SSL_FAILURE_NONE;
+
+  private_key_.reset();
+  signature_result_ = kNoPendingResult;
+  signature_.clear();
 }
 
 bool SSLClientSocketOpenSSL::IsConnected() const {
@@ -928,6 +964,14 @@ int SSLClientSocketOpenSSL::DoHandshake() {
         !ssl_config_.send_client_cert) {
       return ERR_SSL_CLIENT_AUTH_CERT_NEEDED;
     }
+    if (ssl_error == SSL_ERROR_WANT_PRIVATE_KEY_OPERATION) {
+      // FIXME!!! The handshake state machine *cannot* handle things happening
+      // in parallel.

[Ryan Sleevi, 2015/06/12 23:37:20]

?

[davidben, 2015/06/15 21:28:24]

Oops. Removed. Remnant of when I was toying with adding a separate
HandshakeState, but that's a dud anyway because renego doesn't go through
HandshakeState. I later determined that this didn't matter, although it is a
very confusing and slightly broken part of our current state machine.

-- TANGENTIAL STUFF FOLLOWS --

Our state machine is patterned after SSLClientSocketNSS's which is patterned
after the SChannel implementation. From what I can tell, SChannel's API style is
very different from NSS and OpenSSL, so we can reasonably have separate states
for every kind of event we wait on. Notably it doesn't own the transport and
just consumes data that you give it and tells you what to write.

That is NOT true of these state machines. It *almost* is, but due to the write
buffer, we can have a transport Write in flight while another asynchronous
operation is being waited on, like private key operation or channel ID lookup.
That means that OnSendComplete might get called while we're actually waiting for
a different event like cert verify or channel ID lookup. This turns out to be
fine, though sketchy, because either we're in STATE_HANDSHAKE and we pump the
handshake state machine (it'll notice that whatever operation it was waiting on
isn't done yet) or we're not and it'll pump the read loop. Which is either
correct or horribly wrong because Connect isn't done. But that's fine because
then there won't be any read payload so we don't do anything.

Incidentally, Adam and I intend that BoringSSL will have a lower-level
buffer-free API with in-place encryptions and decryptions (and the consumer
explicitly switches to pumping renego). After toying with this state machine,
I'm now thinking I want to move Chromium to it too when the time comes. We can
then keep track of:

- ConnectState = what is Connect() waiting on
- ReadState = what is Read() waiting on
- WriteState = what is Write() waiting on
- HandshakeState = what is the current handshake waiting on.

And then each of the three higher-level states will have an option for "I am
waiting on the handshake" so we know which loops to wake up when the current
handshake completes.

Also we can finally fix all the craziness around write errors. (If we want to.
We might be getting some perf win by starting to listen for Read() before
Write() actually completes?? But if that's true, presumably we'd want this
optimization elsewhere too like in HttpBasicStream.)

+      DCHECK(private_key_);
+      DCHECK_NE(kNoPendingResult, signature_result_);
+      GotoState(STATE_HANDSHAKE);
+      return ERR_IO_PENDING;
+    }
 
     OpenSSLErrorInfo error_info;
     net_error = MapOpenSSLErrorWithDetails(ssl_error, err_tracer, &error_info);
@@ -1256,32 +1300,9 @@ void SSLClientSocketOpenSSL::OnSendComplete(int result) {
     return;
   }
 
-  // OnSendComplete may need to call DoPayloadRead while the renegotiation
-  // handshake is in progress.
-  int rv_read = ERR_IO_PENDING;
-  int rv_write = ERR_IO_PENDING;
-  bool network_moved;
-  do {
-    if (user_read_buf_.get())
-      rv_read = DoPayloadRead();
-    if (user_write_buf_.get())
-      rv_write = DoPayloadWrite();
-    network_moved = DoTransportIO();
-  } while (rv_read == ERR_IO_PENDING && rv_write == ERR_IO_PENDING &&
-           (user_read_buf_.get() || user_write_buf_.get()) && network_moved);
-
-  // Performing the Read callback may cause |this| to be deleted. If this
-  // happens, the Write callback should not be invoked. Guard against this by
-  // holding a WeakPtr to |this| and ensuring it's still valid.
-  base::WeakPtr<SSLClientSocketOpenSSL> guard(weak_factory_.GetWeakPtr());
-  if (user_read_buf_.get() && rv_read != ERR_IO_PENDING)
-    DoReadCallback(rv_read);
-
-  if (!guard.get())
-    return;
-
-  if (user_write_buf_.get() && rv_write != ERR_IO_PENDING)
-    DoWriteCallback(rv_write);
+  // OnSendComplete may need to run the Read state machine while a
+  // renegotiation handshake is in progress.
+  RunReadWriteLoops();
 }
 
 void SSLClientSocketOpenSSL::OnRecvComplete(int result) {
@@ -1379,9 +1400,9 @@ int SSLClientSocketOpenSSL::DoPayloadRead() {
   DCHECK(user_read_buf_.get());
 
   int rv;
-  if (pending_read_error_ != kNoPendingReadResult) {
+  if (pending_read_error_ != kNoPendingResult) {
     rv = pending_read_error_;
-    pending_read_error_ = kNoPendingReadResult;
+    pending_read_error_ = kNoPendingResult;
     if (rv == 0) {
       net_log_.AddByteTransferEvent(NetLog::TYPE_SSL_SOCKET_BYTES_RECEIVED,
                                     rv, user_read_buf_->data());
@@ -1423,6 +1444,11 @@ int SSLClientSocketOpenSSL::DoPayloadRead() {
     } else if (pending_read_ssl_error_ == SSL_ERROR_WANT_X509_LOOKUP &&
                !ssl_config_.send_client_cert) {
       pending_read_error_ = ERR_SSL_CLIENT_AUTH_CERT_NEEDED;
+    } else if (pending_read_ssl_error_ ==
+               SSL_ERROR_WANT_PRIVATE_KEY_OPERATION) {
+      DCHECK(private_key_);
+      DCHECK_NE(kNoPendingResult, signature_result_);
+      pending_read_error_ = ERR_IO_PENDING;
     } else {
       pending_read_error_ = MapOpenSSLErrorWithDetails(
           pending_read_ssl_error_, err_tracer, &pending_read_error_info_);
@@ -1447,12 +1473,12 @@ int SSLClientSocketOpenSSL::DoPayloadRead() {
     // call to DoPayloadRead(), and thus it is important to check SSL_read() on
     // subsequent invocations to see if a complete record may now be read.
     if (pending_read_error_ == ERR_IO_PENDING)
-      pending_read_error_ = kNoPendingReadResult;
+      pending_read_error_ = kNoPendingResult;
   } else {
     // No bytes were returned. Return the pending read error immediately.
-    DCHECK_NE(kNoPendingReadResult, pending_read_error_);
+    DCHECK_NE(kNoPendingResult, pending_read_error_);
     rv = pending_read_error_;
-    pending_read_error_ = kNoPendingReadResult;
+    pending_read_error_ = kNoPendingResult;
   }
 
   if (rv >= 0) {
@@ -1480,6 +1506,8 @@ int SSLClientSocketOpenSSL::DoPayloadWrite() {
   }
 
   int ssl_error = SSL_get_error(ssl_, rv);
+  if (ssl_error == SSL_ERROR_WANT_PRIVATE_KEY_OPERATION)
+    return ERR_IO_PENDING;
   OpenSSLErrorInfo error_info;
   int net_error = MapOpenSSLErrorWithDetails(ssl_error, err_tracer,
                                              &error_info);
@@ -1492,6 +1520,33 @@ int SSLClientSocketOpenSSL::DoPayloadWrite() {
   return net_error;
 }
 
+void SSLClientSocketOpenSSL::RunReadWriteLoops() {
+  int rv_read = ERR_IO_PENDING;
+  int rv_write = ERR_IO_PENDING;
+  bool network_moved;
+  do {
+    if (user_read_buf_.get())
+      rv_read = DoPayloadRead();
+    if (user_write_buf_.get())
+      rv_write = DoPayloadWrite();
+    network_moved = DoTransportIO();
+  } while (rv_read == ERR_IO_PENDING && rv_write == ERR_IO_PENDING &&
+           (user_read_buf_.get() || user_write_buf_.get()) && network_moved);
+
+  // Performing the Read callback may cause |this| to be deleted. If this
+  // happens, the Write callback should not be invoked. Guard against this by
+  // holding a WeakPtr to |this| and ensuring it's still valid.
+  base::WeakPtr<SSLClientSocketOpenSSL> guard(weak_factory_.GetWeakPtr());
+  if (user_read_buf_.get() && rv_read != ERR_IO_PENDING)
+    DoReadCallback(rv_read);
+
+  if (!guard.get())
+    return;
+
+  if (user_write_buf_.get() && rv_write != ERR_IO_PENDING)
+    DoWriteCallback(rv_write);
+}
+
 int SSLClientSocketOpenSSL::BufferSend(void) {
   if (transport_send_busy_)
     return ERR_IO_PENDING;
@@ -1675,18 +1730,16 @@ int SSLClientSocketOpenSSL::ClientCertRequestCallback(SSL* ssl) {
       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
-    // net/ client auth API lacking a private key handle.
+    if (!SSL_use_certificate(ssl_, leaf_x509.get()) ||
+        !SSL_set1_chain(ssl_, chain.get())) {
+      LOG(WARNING) << "Failed to set client certificate";
+      return -1;
+    }
+
 #if defined(USE_OPENSSL_CERTS)
     crypto::ScopedEVP_PKEY privkey =
         OpenSSLClientKeyStore::GetInstance()->FetchClientCertPrivateKey(
             ssl_config_.client_cert.get());
-#else  // !defined(USE_OPENSSL_CERTS)
-    crypto::ScopedEVP_PKEY privkey =
-        FetchClientCertPrivateKey(ssl_config_.client_cert.get());
-#endif  // defined(USE_OPENSSL_CERTS)
     if (!privkey) {
       // Could not find the private key. Fail the handshake and surface an
       // appropriate error to the caller.
@@ -1694,14 +1747,37 @@ int SSLClientSocketOpenSSL::ClientCertRequestCallback(SSL* ssl) {
       OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_CERT_NO_PRIVATE_KEY);
       return -1;
     }
-
-    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";
+    if (!SSL_use_PrivateKey(ssl_, privkey.get())) {
+      LOG(WARNING) << "Failed to set private key";
+      return -1;
+    }
+#else   // !USE_OPENSSL_CERTS
+    // TODO(davidben): Move Android to the SSLPrivateKey codepath and disable
+    // client auth on NaCl altogether.
+    //
+    // TODO(davidben): Lift this call up to the embedder so we can actually test
+    // this code. https://crbug.com/394131
+    private_key_ = FetchClientCertPrivateKey(
+        ssl_config_.client_cert.get(),
+        base::WorkerPool::GetTaskRunner(true /* task_is_slow */));
+    if (!private_key_) {
+      // Could not find the private key. Fail the handshake and surface an
+      // appropriate error to the caller.
+      LOG(WARNING) << "Client cert found without private key";
+      OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_CERT_NO_PRIVATE_KEY);
       return -1;
     }
 
+    static const SSL_PRIVATE_KEY_METHOD kPrivateKeyMethod = {
+        &SSLClientSocketOpenSSL::PrivateKeyTypeCallback,
+        &SSLClientSocketOpenSSL::PrivateKeySupportsDigestCallback,
+        &SSLClientSocketOpenSSL::PrivateKeyMaxSignatureLenCallback,
+        &SSLClientSocketOpenSSL::PrivateKeySignCallback,
+        &SSLClientSocketOpenSSL::PrivateKeySignCompleteCallback,
+    };
+    SSL_set_private_key_method(ssl_, &kPrivateKeyMethod);
+#endif  // USE_OPENSSL_CERTS
+
     int cert_count = 1 + sk_X509_num(chain.get());
     net_log_.AddEvent(NetLog::TYPE_SSL_CLIENT_CERT_PROVIDED,
                       NetLog::IntegerCallback("cert_count", cert_count));
@@ -1922,6 +1998,126 @@ bool SSLClientSocketOpenSSL::IsRenegotiationAllowed() const {
   return false;
 }
 
+// static
+int SSLClientSocketOpenSSL::PrivateKeyTypeCallback(SSL* ssl) {
+  SSLClientSocketOpenSSL* socket =
+      SSLContext::GetInstance()->GetClientSocketFromSSL(ssl);
+
+  switch (socket->private_key_->GetType()) {
+    case SSLPrivateKey::Type::RSA:
+      return EVP_PKEY_RSA;
+    case SSLPrivateKey::Type::ECDSA:
+      return EVP_PKEY_EC;
+    default:
+      NOTREACHED();
+      return EVP_PKEY_NONE;

[Ryan Sleevi, 2015/06/12 23:37:19]

remove the default, and move the return outside the switch.

If you add any new types to the return of GetType(), clang will start yelling at
compile-time. Better than a NOTREACHED.

[davidben, 2015/06/15 21:28:24]

Done.

+  }
+}
+
+// static
+int SSLClientSocketOpenSSL::PrivateKeySupportsDigestCallback(SSL* ssl,
+                                                             const EVP_MD* md) {
+  SSLClientSocketOpenSSL* socket =
+      SSLContext::GetInstance()->GetClientSocketFromSSL(ssl);
+
+  SSLPrivateKey::Hash hash;
+  return EVP_MDToPrivateKeyHash(md, &hash) &&
+         socket->private_key_->SupportsHash(hash);
+}
+
+// static
+size_t SSLClientSocketOpenSSL::PrivateKeyMaxSignatureLenCallback(SSL* ssl) {
+  SSLClientSocketOpenSSL* socket =
+      SSLContext::GetInstance()->GetClientSocketFromSSL(ssl);
+
+  return socket->private_key_->GetMaxSignatureLength();
+}
+
+// static
+ssl_private_key_result_t SSLClientSocketOpenSSL::PrivateKeySignCallback(
+    SSL* ssl,
+    uint8_t* out,
+    size_t* out_len,
+    size_t max_out,
+    const EVP_MD* md,
+    const uint8_t* in,
+    size_t in_len) {
+  SSLClientSocketOpenSSL* socket =
+      SSLContext::GetInstance()->GetClientSocketFromSSL(ssl);
+
+  DCHECK_EQ(kNoPendingResult, socket->signature_result_);
+  DCHECK(socket->signature_.empty());
+  DCHECK(socket->private_key_);
+
+  socket->net_log_.BeginEvent(NetLog::TYPE_SSL_PRIVATE_KEY_OPERATION);
+
+  SSLPrivateKey::Hash hash;
+  if (!EVP_MDToPrivateKeyHash(md, &hash)) {
+    OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_SIGNATURE_FAILED);
+    return ssl_private_key_failure;
+  }
+
+  socket->signature_result_ = ERR_IO_PENDING;
+  socket->private_key_->SignDigest(
+      hash, base::StringPiece(reinterpret_cast<const char*>(in), in_len),
+      base::Bind(&SSLClientSocketOpenSSL::OnPrivateKeySignComplete,
+                 socket->weak_factory_.GetWeakPtr()));
+  return ssl_private_key_retry;
+}
+
+// static
+ssl_private_key_result_t SSLClientSocketOpenSSL::PrivateKeySignCompleteCallback(
+    SSL* ssl,
+    uint8_t* out,
+    size_t* out_len,
+    size_t max_out) {
+  SSLClientSocketOpenSSL* socket =
+      SSLContext::GetInstance()->GetClientSocketFromSSL(ssl);
+
+  DCHECK_NE(kNoPendingResult, socket->signature_result_);
+  DCHECK(socket->private_key_);
+
+  if (socket->signature_result_ == ERR_IO_PENDING)
+    return ssl_private_key_retry;
+  if (socket->signature_result_ != OK) {
+    OpenSSLPutNetError(FROM_HERE, socket->signature_result_);
+    return ssl_private_key_failure;
+  }
+  if (socket->signature_.size() > max_out) {
+    LOG(ERROR) << "Buffer too small";
+    OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_SIGNATURE_FAILED);
+    return ssl_private_key_failure;
+  }
+  memcpy(out, vector_as_array(&socket->signature_), socket->signature_.size());
+  *out_len = socket->signature_.size();
+  socket->signature_.clear();
+  return ssl_private_key_success;
+}
+
+void SSLClientSocketOpenSSL::OnPrivateKeySignComplete(
+    Error error,
+    const std::vector<uint8_t>& signature) {
+  DCHECK_EQ(ERR_IO_PENDING, signature_result_);
+  DCHECK(signature_.empty());
+  DCHECK(private_key_);
+
+  net_log_.EndEventWithNetErrorCode(NetLog::TYPE_SSL_PRIVATE_KEY_OPERATION,
+                                    error);
+
+  signature_result_ = error;
+  if (signature_result_ == OK)
+    signature_ = signature;
+
+  if (next_handshake_state_ == STATE_HANDSHAKE) {
+    OnHandshakeIOComplete(signature_result_);
+    return;
+  }
+
+  // Either Read or Write may be blocked on an asynchronous private
+  // key operation during a renegotiation.
+  RunReadWriteLoops();
+}
+
 scoped_refptr<X509Certificate>
 SSLClientSocketOpenSSL::GetUnverifiedServerCertificateChain() const {
   return server_cert_;