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

net/http/http_network_transaction.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/http/http_network_transaction.h"
 
 #include <set>
 #include <vector>
 
 #include "base/compiler_specific.h"
@@ skipping 536 matching lines @@
     next_state_ = STATE_INIT_STREAM;
     DCHECK(stream_.get());
   } else if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {
     result = HandleCertificateRequest(result);
   } else if (result == ERR_HTTPS_PROXY_TUNNEL_RESPONSE) {
     // Return OK and let the caller read the proxy's error page
     next_state_ = STATE_NONE;
     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;
 }
 
 int HttpNetworkTransaction::DoInitStream() {
   DCHECK(stream_.get());
   next_state_ = STATE_INIT_STREAM_COMPLETE;
   return stream_->InitializeStream(request_, net_log_, &io_callback_);
 }
@@ skipping 122 matching lines @@
     result = ERR_CERT_ERROR_IN_SSL_RENEGOTIATION;
   } else if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {
     // TODO(wtc): Need a test case for this code path!
     DCHECK(stream_.get());
     DCHECK(is_https_request());
     response_.cert_request_info = new SSLCertRequestInfo;
     stream_->GetSSLCertRequestInfo(response_.cert_request_info);
     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)
     return HandleIOError(result);
 
   if (result == ERR_CONNECTION_CLOSED && ShouldResendRequest(result)) {
     ResetConnectionAndRequestForResend();
     return OK;
   }
 
@@ skipping 291 matching lines @@
   // SSL session cache.
   ssl_config_.client_cert = client_cert;
   ssl_config_.send_client_cert = true;
   next_state_ = STATE_CREATE_STREAM;
   // Reset the other member variables.
   // Note: this is necessary only with SSL renegotiation.
   ResetStateForRestart();
   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
     // portion of our request) only to find a connection error when we try to
     // read from (or finish writing to) the socket.
     case ERR_CONNECTION_RESET:
     case ERR_CONNECTION_CLOSED:
     case ERR_CONNECTION_ABORTED:
       if (ShouldResendRequest(error)) {
         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;
 }
 
 void HttpNetworkTransaction::ResetStateForRestart() {
   ResetStateForAuthRestart();
   stream_.reset();
 }
 
 void HttpNetworkTransaction::ResetStateForAuthRestart() {
@@ skipping 138 matching lines @@
     default:
       return priority;
   }
 }
 
 
 
 #undef STATE_CASE
 
 }  // namespace net