Ensure that when using False Start + client auth, bad client certificates are not cached

net/http/http_network_transaction.cc

Index: net/http/http_network_transaction.cc
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 64a4fa7985669de75c1ef7c7a02d99f23602c1cf..51d962f983e79a93f1344dd4531b8c6902bab7e6 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -554,6 +554,10 @@ int HttpNetworkTransaction::DoCreateStreamComplete(int result) {
     return OK;
   }
 
+  // Handle possible handshake errors that may have occurred if the stream
+  // used SSL for one or more of the layers.
+  result = HandleSSLHandshakeError(result);
+
   // At this point we are done with the stream_request_.
   stream_request_.reset();
   return result;
@@ -696,23 +700,6 @@ int HttpNetworkTransaction::DoReadHeadersComplete(int result) {
     result = HandleCertificateRequest(result);
     if (result == OK)
       return result;
-  } else if ((result == ERR_SSL_DECOMPRESSION_FAILURE_ALERT ||
-              result == ERR_SSL_BAD_RECORD_MAC_ALERT) &&
-             ssl_config_.tls1_enabled &&
-             !SSLConfigService::IsKnownStrictTLSServer(request_->url.host())) {
-    // Some buggy servers select DEFLATE compression when offered and then
-    // fail to ever decompress anything. They will send a fatal alert telling
-    // us this. Normally we would pick this up during the handshake because
-    // our Finished message is compressed and we'll never get the server's
-    // Finished if it fails to process ours.
-    //
-    // However, with False Start, we'll believe that the handshake is
-    // complete as soon as we've /sent/ our Finished message. In this case,
-    // we only find out that the server is buggy here, when we try to read
-    // the initial reply.
-    session_->http_stream_factory()->AddTLSIntolerantServer(request_->url);
-    ResetConnectionAndRequestForResend();
-    return OK;
   }
 
   if (result < 0 && result != ERR_CONNECTION_CLOSED)
@@ -1024,11 +1011,61 @@ int HttpNetworkTransaction::HandleCertificateRequest(int error) {
   return OK;
 }
 
+// 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/66424
+int HttpNetworkTransaction::HandleSSLHandshakeError(int error) {
+  DCHECK(request_);
+  if (ssl_config_.send_client_cert &&
+      (error == ERR_SSL_PROTOCOL_ERROR ||
+       error == ERR_BAD_SSL_CLIENT_AUTH_CERT)) {
+    session_->ssl_client_auth_cache()->Remove(
+        GetHostAndPort(request_->url));
+  }
+
+  switch (error) {
+    case ERR_SSL_PROTOCOL_ERROR:
+    case ERR_SSL_VERSION_OR_CIPHER_MISMATCH:
+    case ERR_SSL_DECOMPRESSION_FAILURE_ALERT:
+    case ERR_SSL_BAD_RECORD_MAC_ALERT:
+      if (ssl_config_.tls1_enabled &&
+          !SSLConfigService::IsKnownStrictTLSServer(request_->url.host())) {
+        // This could be a TLS-intolerant server, an SSL 3.0 server that
+        // chose a TLS-only cipher suite or a server with buggy DEFLATE
+        // support. Turn off TLS 1.0, DEFLATE support and retry.
+        session_->http_stream_factory()->AddTLSIntolerantServer(request_->url);
+        ResetConnectionAndRequestForResend();
+        error = OK;
+      }
+      break;
+    case ERR_SSL_SNAP_START_NPN_MISPREDICTION:
+      // This means that we tried to Snap Start a connection, but we
+      // mispredicted the NPN result. This isn't a problem from the point of
+      // view of the SSL layer because the server will ignore the application
+      // data in the Snap Start extension. However, at the HTTP layer, we have
+      // already decided that it's a HTTP or SPDY connection and it's easier to
+      // abort and start again.
+      ResetConnectionAndRequestForResend();
+      error = OK;
+      break;
+  }
+  return error;
+}
+
 // This method determines whether it is safe to resend the request after an
 // IO error.  It can only be called in response to request header or body
 // write errors or response header read errors.  It should not be used in
 // other cases, such as a Connect error.
 int HttpNetworkTransaction::HandleIOError(int error) {
+  // SSL errors may happen at any time during the stream and indicate issues
+  // with the underlying connection. Because the peer may request
+  // renegotiation at any time, check and handle any possible SSL handshake
+  // related errors. In addition to renegotiation, TLS False/Snap Start may
+  // cause SSL handshake errors to be delayed until the first Read on the

[wtc, 2011/01/12 00:21:09]

This should be "first Write or Read", right?

+  // underlying connection.
+  error = HandleSSLHandshakeError(error);
+
   switch (error) {
     // If we try to reuse a connection that the server is in the process of
     // closing, we may end up successfully writing out our request (or a
@@ -1042,16 +1079,6 @@ int HttpNetworkTransaction::HandleIOError(int error) {
         error = OK;
       }
       break;
-    case ERR_SSL_SNAP_START_NPN_MISPREDICTION:
-      // This means that we tried to Snap Start a connection, but we
-      // mispredicted the NPN result. This isn't a problem from the point of
-      // view of the SSL layer because the server will ignore the application
-      // data in the Snap Start extension. However, at the HTTP layer, we have
-      // already decided that it's a HTTP or SPDY connection and it's easier to
-      // abort and start again.
-      ResetConnectionAndRequestForResend();
-      error = OK;
-      break;
   }
   return error;
 }