Do not roll back to SSL 3.0 for Google properties.

net/http/http_network_transaction.cc

 // Copyright (c) 2012 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/bind.h"
@@ skipping 29 matching lines @@
 #include "net/http/http_proxy_client_socket_pool.h"
 #include "net/http/http_request_headers.h"
 #include "net/http/http_request_info.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_response_info.h"
 #include "net/http/http_server_properties.h"
 #include "net/http/http_status_code.h"
 #include "net/http/http_stream_base.h"
 #include "net/http/http_stream_factory.h"
 #include "net/http/http_util.h"
+#include "net/http/transport_security_state.h"
 #include "net/http/url_security_manager.h"
 #include "net/socket/client_socket_factory.h"
 #include "net/socket/socks_client_socket_pool.h"
 #include "net/socket/ssl_client_socket.h"
 #include "net/socket/ssl_client_socket_pool.h"
 #include "net/socket/transport_client_socket_pool.h"
 #include "net/spdy/spdy_http_stream.h"
 #include "net/spdy/spdy_session.h"
 #include "net/spdy/spdy_session_pool.h"
 #include "net/ssl/ssl_cert_request_info.h"
@@ skipping 1013 matching lines @@
         total_duration, base::TimeDelta::FromMilliseconds(1),
         base::TimeDelta::FromMinutes(10), 100);
 
     if (!reused_socket) {
       UMA_HISTOGRAM_CUSTOM_TIMES(
           base::FieldTrial::MakeName("Net.Transaction_Connected_New_b",
                                      "SpdyImpact"),
           total_duration, base::TimeDelta::FromMilliseconds(1),
           base::TimeDelta::FromMinutes(10), 100);
     }
   }

[thaidn_google, 2013/04/19 01:42:55]

Not me.

 
   // Currently, non-HIGHEST priority requests are frame or sub-frame resource
   // types.  This will change when we also prioritize certain subresources like
   // css, js, etc.
   if (priority_ != HIGHEST) {
     UMA_HISTOGRAM_CUSTOM_TIMES(
         "Net.Priority_High_Latency_b",
         total_duration,
         base::TimeDelta::FromMilliseconds(1), base::TimeDelta::FromMinutes(10),
         100);
@@ skipping 32 matching lines @@
         duration,
         base::TimeDelta::FromMilliseconds(1),
         base::TimeDelta::FromMinutes(10),
         100);
     UMA_HISTOGRAM_CUSTOM_TIMES(
         base::FieldTrial::MakeName("Net.Transaction_Latency_Total",
                                    "WarmSocketImpact"),
         total_duration,
         base::TimeDelta::FromMilliseconds(1),
         base::TimeDelta::FromMinutes(10), 100);
   }

[thaidn_google, 2013/04/19 01:42:55]

Not me.

 
   if (!stream_->IsConnectionReused()) {
     UMA_HISTOGRAM_CUSTOM_TIMES(
         "Net.Transaction_Latency_Total_New_Connection",
         total_duration, base::TimeDelta::FromMilliseconds(1),
         base::TimeDelta::FromMinutes(10), 100);
   }
 }
 
 int HttpNetworkTransaction::HandleCertificateRequest(int error) {
@@ skipping 62 matching lines @@
 // 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_);
   if (server_ssl_config_.send_client_cert &&
       (error == ERR_SSL_PROTOCOL_ERROR || IsClientCertificateError(error))) {
     session_->ssl_client_auth_cache()->Remove(
         GetHostAndPort(request_->url));
   }
 
+  uint16 version_max = server_ssl_config_.version_max;
+
   switch (error) {
     case ERR_SSL_PROTOCOL_ERROR:
     case ERR_SSL_VERSION_OR_CIPHER_MISMATCH:
-      if (server_ssl_config_.version_max >= SSL_PROTOCOL_VERSION_TLS1 &&
-          server_ssl_config_.version_max > server_ssl_config_.version_min) {
+      if (version_max >= SSL_PROTOCOL_VERSION_TLS1 &&
+          version_max > server_ssl_config_.version_min) {
         // This could be a TLS-intolerant server or a server that chose a
         // cipher suite defined only for higher protocol versions (such as
         // an SSL 3.0 server that chose a TLS-only cipher suite).  Fall
         // back to the next lower version and retry.
         // NOTE: if the SSLClientSocket class doesn't support TLS 1.1,
         // specifying TLS 1.1 in version_max will result in a TLS 1.0
         // handshake, so falling back from TLS 1.1 to TLS 1.0 will simply
         // repeat the TLS 1.0 handshake. To avoid this problem, the default
         // version_max should match the maximum protocol version supported
         // by the SSLClientSocket class.
-        uint16 version_before = server_ssl_config_.version_max;
-        server_ssl_config_.version_max--;
-        net_log_.AddEvent(
-            NetLog::TYPE_SSL_VERSION_FALLBACK,
-            base::Bind(&NetLogSSLVersionFallbackCallback,
-                       &request_->url, error, version_before,
-                       server_ssl_config_.version_max));
-        server_ssl_config_.version_fallback = true;
-        ResetConnectionAndRequestForResend();
-        error = OK;
-      }
-      break;
-    case ERR_SSL_DECOMPRESSION_FAILURE_ALERT:
-    case ERR_SSL_BAD_RECORD_MAC_ALERT:
-      if (server_ssl_config_.version_max >= SSL_PROTOCOL_VERSION_TLS1 &&
-          server_ssl_config_.version_min == SSL_PROTOCOL_VERSION_SSL3) {
-        // This could be a server with buggy DEFLATE support. Turn off TLS,
-        // DEFLATE support and retry.
-        // TODO(wtc): turn off DEFLATE support only. Do not tie it to TLS.
-        uint16 version_before = server_ssl_config_.version_max;
-        server_ssl_config_.version_max = SSL_PROTOCOL_VERSION_SSL3;
-        net_log_.AddEvent(
-            NetLog::TYPE_SSL_VERSION_FALLBACK,
-            base::Bind(&NetLogSSLVersionFallbackCallback,
-                       &request_->url, error, version_before,
-                       server_ssl_config_.version_max));
-        server_ssl_config_.version_fallback = true;
-        ResetConnectionAndRequestForResend();
-        error = OK;
+        version_max--;
+
+        // Fallback to the lower SSL version.
+        // While SSL 3.0 fallback should be eliminated because of security
+        // reasons, there is a high risk of breaking the servers if this is
+        // done in general.
+        // For now SSL 3.0 fallback is disabled for Google servers first,
+        // and will be expanded to other servers after enough experiences
+        // have been gained showing that this experiment works well with
+        // today's Internet.
+        if (version_max > SSL_PROTOCOL_VERSION_SSL3 ||
+            (version_max == SSL_PROTOCOL_VERSION_SSL3 &&

[wtc, 2013/04/18 18:15:34]

Delete "(version_max == SSL_PROTOCOL_VERSION_SSL3 &&" and the
corresponding closing ")".

This is implied by how C/C++ evaluates the two operands of
the || operator, and the fact that SSL_PROTOCOL_VERSION_SSL3
is the smallest SSL version used by Chrome.

[thaidn_google, 2013/04/19 01:20:43]

Nice catch.

Thanks!
On 2013/04/18 18:15:34, wtc wrote:

+             (server_ssl_config_.ssl3_fallback_enabled ||
+              !TransportSecurityState::IsGooglePinnedProperty(
+                  request_->url.host(), true /* include SNI */)))) {
+          net_log_.AddEvent(
+              NetLog::TYPE_SSL_VERSION_FALLBACK,
+              base::Bind(&NetLogSSLVersionFallbackCallback,
+                         &request_->url, error, server_ssl_config_.version_max,
+                         version_max));
+          server_ssl_config_.version_max = version_max;
+          server_ssl_config_.version_fallback = true;
+          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
@@ skipping 185 matching lines @@
       description = base::StringPrintf("Unknown state 0x%08X (%u)", state,
                                        state);
       break;
   }
   return description;
 }
 
 #undef STATE_CASE
 
 }  // namespace net