Merge 216836 "Handle the TLS version fallback on the bad_record_..."

> Handle the TLS version fallback on the bad_record_mac alert error in
> http_network_transaction.cc, so that it applies to SSLClientSockets
> based on both NSS and OpenSSL.
> 
> R=agl@chromium.org,rsleevi@chromium.org
> BUG=260358
> TEST=net_unittests, plus manual testing: visit https://www.web-secured.com/.
> Should get a successful TLS 1.0 connection, rather than
> ERR_SSL_BAD_RECORD_MAC_ALERT.
> 
> Review URL: https://chromiumcodereview.appspot.com/22633004

TBR=wtc@chromium.org

Review URL: https://codereview.chromium.org/22896005

git-svn-id: svn://svn.chromium.org/chrome/branches/1547/src@217296 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 25 insertions, 18 deletions

diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 55cfb2e..8fa7d0d 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -1218,6 +1218,7 @@ int HttpNetworkTransaction::HandleSSLHandshakeError(int error) {
         GetHostAndPort(request_->url));
   }
 
+  bool should_fallback = false;
   uint16 version_max = server_ssl_config_.version_max;
 
   switch (error) {
@@ -1249,18 +1250,33 @@ int HttpNetworkTransaction::HandleSSLHandshakeError(int error) {
             (server_ssl_config_.unrestricted_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;
+          should_fallback = true;
         }
       }
       break;
+    case ERR_SSL_BAD_RECORD_MAC_ALERT:
+      if (version_max >= SSL_PROTOCOL_VERSION_TLS1_1 &&
+          version_max > server_ssl_config_.version_min) {
+        // Some broken SSL devices negotiate TLS 1.0 when sent a TLS 1.1 or
+        // 1.2 ClientHello, but then return a bad_record_mac alert. See
+        // crbug.com/260358. In order to make the fallback as minimal as
+        // possible, this fallback is only triggered for >= TLS 1.1.
+        version_max--;
+        should_fallback = true;
+      }
+      break;
+  }
+
+  if (should_fallback) {
+    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;
   }
 
   return error;
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index 6783999..a7d2ab6 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -1915,15 +1915,6 @@ int SSLClientSocketNSS::Core::DoHandshake() {
       net_error = ERR_SSL_PROTOCOL_ERROR;
     }
 
-    // Some broken SSL devices negotiate TLS 1.0 when sent a TLS 1.1 or 1.2
-    // ClientHello, but then return a bad-record-MAC alert. See
-    // crbug.com/260358. In order to make the fallback as minimal as possible,
-    // this fallback is only triggered for >= TLS 1.1.
-    if (net_error == ERR_SSL_BAD_RECORD_MAC_ALERT &&
-        ssl_config_.version_max >= SSL_PROTOCOL_VERSION_TLS1_1) {
-      net_error = ERR_SSL_PROTOCOL_ERROR;
-    }
-
     // If not done, stay in this state
     if (net_error == ERR_IO_PENDING) {
       GotoState(STATE_HANDSHAKE);