Request a new client certificate if a cached one is stale.

If an SSLPrivateKey is backed by a smartcard or other interesting
module, the handle may eventually stop working. In particular, the
smartcard may be removed at some point.

Ideally, the OS would provide reliable fine-grained signals to clear
relevant the cache entries, but the OS tends not to provide these APIs.
We do drop the cache entry on failure, but the user is required to retry
the operation.

Instead, if an SSLPrivateKey was grabbed from the SSLClientAuthCache,
assume it is potentially stale. Should the signing operation fail, we
can not only drop the cache entry, but retry the request.

This CL does not implement this logic for proxy client certificates,
only server client certificates. Proxy client certificates a missing the
cache clearing logic (https://crbug.com/814911), so we can fill this in
once the plumbing is in place.

Along the way, fill in some URLRequest-level client certificate unit
tests.

Bug: 813022
Change-Id: I9f0450e9f4df1383dd8b73d0297ebea5e3368fec
Reviewed-on: https://chromium-review.googlesource.com/935723Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Commit-Queue: David Benjamin <davidben@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539022}

net/http/http_network_transaction.cc

@@ -88,6 +88,7 @@ HttpNetworkTransaction::HttpNetworkTransaction(RequestPriority priority,
       priority_(priority),
       headers_valid_(false),
       can_send_early_data_(false),
+      server_ssl_client_cert_was_cached_(false),
       request_headers_(),
       read_buf_len_(0),
       total_received_bytes_(0),
@@ -899,9 +900,9 @@ int HttpNetworkTransaction::DoCreateStreamComplete(int result) {
     return HandleHttp11Required(result);
   }
 
-  // Handle possible handshake errors that may have occurred if the stream
-  // used SSL for one or more of the layers.
-  result = HandleSSLHandshakeError(result);
+  // Handle possible client certificate errors that may have occurred if the
+  // stream used SSL for one or more of the layers.
+  result = HandleSSLClientAuthError(result);
 
   // At this point we are done with the stream_request_.
   stream_request_.reset();
@@ -1486,6 +1487,10 @@ int HttpNetworkTransaction::HandleCertificateRequest(int error) {
       return error;
   }
 
+  if (!response_.cert_request_info->is_proxy) {
+    server_ssl_client_cert_was_cached_ = true;
+  }
+
   // TODO(davidben): Add a unit test which covers this path; we need to be
   // able to send a legitimate certificate and also bypass/clear the
   // SSL session cache.
@@ -1514,21 +1519,31 @@ int HttpNetworkTransaction::HandleHttp11Required(int error) {
   return OK;
 }
 
-void HttpNetworkTransaction::HandleClientAuthError(int error) {
+int HttpNetworkTransaction::HandleSSLClientAuthError(int error) {
+  // TODO(davidben): This does handle client certificate errors from the
+  // proxy. https://crbug.com/814911.
   if (server_ssl_config_.send_client_cert &&
       (error == ERR_SSL_PROTOCOL_ERROR || IsClientCertificateError(error))) {
     session_->ssl_client_auth_cache()->Remove(
         HostPortPair::FromURL(request_->url));
-  }
-}
 
-// TODO(rch): This does not correctly handle errors when an SSL proxy is
-// being used, as all of the errors are handled as if they were generated
-// by the endpoint host, request_->url, rather than considering if they were
-// generated by the SSL proxy. http://crbug.com/69329
-int HttpNetworkTransaction::HandleSSLHandshakeError(int error) {
-  DCHECK(request_);
-  HandleClientAuthError(error);
+    // The private key handle may have gone stale due to, e.g., the user
+    // unplugging their smartcard. Operating systems do not provide reliable
+    // notifications for this, so if the signature failed and the private key
+    // came from SSLClientAuthCache, retry to ask the user for a new one.
+    if (error == ERR_SSL_CLIENT_AUTH_SIGNATURE_FAILED &&
+        server_ssl_client_cert_was_cached_ && !HasExceededMaxRetries()) {
+      server_ssl_client_cert_was_cached_ = false;
+      server_ssl_config_.send_client_cert = false;
+      server_ssl_config_.client_cert = nullptr;
+      server_ssl_config_.client_private_key = nullptr;
+      retry_attempts_++;
+      net_log_.AddEventWithNetErrorCode(
+          NetLogEventType::HTTP_TRANSACTION_RESTART_AFTER_ERROR, error);
+      ResetConnectionAndRequestForResend();
+      return OK;
+    }
+  }
   return error;
 }
 
@@ -1539,7 +1554,7 @@ int HttpNetworkTransaction::HandleSSLHandshakeError(int error) {
 int HttpNetworkTransaction::HandleIOError(int error) {
   // Because the peer may request renegotiation with client authentication at
   // any time, check and handle client authentication errors.
-  HandleClientAuthError(error);
+  error = HandleSSLClientAuthError(error);
 
   switch (error) {
     // If we try to reuse a connection that the server is in the process of

net/http/http_network_transaction.h

@@ -232,13 +232,10 @@ class NET_EXPORT_PRIVATE HttpNetworkTransaction
   // ERR_PROXY_HTTP_1_1_REQUIRED has to be handled.
   int HandleHttp11Required(int error);
 
-  // Called to possibly handle a client authentication error.
-  void HandleClientAuthError(int error);
-
-  // Called to possibly recover from an SSL handshake error.  Sets next_state_
+  // Called to possibly handle a client authentication error. Sets next_state_
   // and returns OK if recovering from the error.  Otherwise, the same error
   // code is returned.
-  int HandleSSLHandshakeError(int error);
+  int HandleSSLClientAuthError(int error);
 
   // Called to possibly recover from the given error.  Sets next_state_ and
   // returns OK if recovering from the error.  Otherwise, the same error code
@@ -342,6 +339,10 @@ class NET_EXPORT_PRIVATE HttpNetworkTransaction
   // True if we can send the request over early data.
   bool can_send_early_data_;
 
+  // True if |server_ssl_config_.client_cert| was looked up from the
+  // SSLClientAuthCache, rather than provided externally by the caller.
+  bool server_ssl_client_cert_was_cached_;
+
   SSLConfig server_ssl_config_;
   SSLConfig proxy_ssl_config_;