Preserve transport errors for OpenSSL sockets.

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 1279 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_);
   HandleClientAuthError(error);
 
   bool should_fallback = false;
   uint16 version_max = server_ssl_config_.version_max;
 
   switch (error) {
+    case ERR_CONNECTION_CLOSED:
     case ERR_SSL_PROTOCOL_ERROR:
     case ERR_SSL_VERSION_OR_CIPHER_MISMATCH:
       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
@@ skipping 14 matching lines @@
       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;
+    case ERR_CONNECTION_RESET:
+      if (version_max >= SSL_PROTOCOL_VERSION_TLS1_1 &&
+          version_max > server_ssl_config_.version_min) {
+        // Some network devices that inspect application-layer packets seem to
+        // inject TCP reset packets to break the connections when they see TLS
+        // 1.1 in ClientHello or ServerHello. See http://crbug.com/130293.
+        //
+        // Only allow ERR_CONNECTION_RESET to trigger a fallback from TLS 1.1 or
+        // 1.2. We don't lose much in this fallback because the explicit IV for
+        // CBC mode in TLS 1.1 is approximated by record splitting in TLS
+        // 1.0. The fallback will be more painful for TLS 1.2 when we have GCM
+        // support.
+        //
+        // ERR_CONNECTION_RESET is a common network error, so we don't want it
+        // to trigger a version fallback in general, especially the TLS 1.0 ->
+        // SSL 3.0 fallback, which would drop TLS extensions.
+        version_max--;
+        should_fallback = true;

[Ryan Sleevi, 2014/06/18 01:39:52]

For future reference, changes like this should really belong in a separate CL.

It's subtle, full of danger, and requires separate reasoning about to ensure
it's correctness. I'm aware that NSS and OpenSSL may have divergent behaviour
here, and you're fixing that behaviour, but that's not a strong enough
justification for lumping this in. It's perfectly OK to have CL chains.



Independent of that, I do wonder whether this logic belongs associated with 1324
- that is, in both cases, we want to do minimal fallback.

[davidben, 2014/06/18 22:27:22]

Yeah, I'll split this one out.
1324?

+      }
+      break;
     case ERR_SSL_INAPPROPRIATE_FALLBACK:
       // The server told us that we should not have fallen back. A buggy server
       // could trigger ERR_SSL_INAPPROPRIATE_FALLBACK with the initial
       // connection. |fallback_error_code_| is initialised to
       // ERR_SSL_INAPPROPRIATE_FALLBACK to catch this case.
       error = fallback_error_code_;
       break;
   }
 
   if (should_fallback) {
@@ skipping 207 matching lines @@
       description = base::StringPrintf("Unknown state 0x%08X (%u)", state,
                                        state);
       break;
   }
   return description;
 }
 
 #undef STATE_CASE
 
 }  // namespace net