Add a dedicated error code for TLS 1.3 interference.

net/socket/ssl_client_socket_pool.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/socket/ssl_client_socket_pool.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 112 matching lines @@
       client_socket_factory_(client_socket_factory),
       context_(context.cert_verifier,
                context.channel_id_service,
                context.transport_security_state,
                context.cert_transparency_verifier,
                context.ct_policy_enforcer,
                (params->privacy_mode() == PRIVACY_MODE_ENABLED
                     ? "pm/" + context.ssl_session_cache_shard
                     : context.ssl_session_cache_shard)),
       callback_(
-          base::Bind(&SSLConnectJob::OnIOComplete, base::Unretained(this))) {}
+          base::Bind(&SSLConnectJob::OnIOComplete, base::Unretained(this))),
+      version_interference_probe_(false),
+      version_interference_probe_error_(OK) {}
 
 SSLConnectJob::~SSLConnectJob() {
 }
 
 LoadState SSLConnectJob::GetLoadState() const {
   switch (next_state_) {
     case STATE_TUNNEL_CONNECT_COMPLETE:
       if (transport_socket_handle_->socket())
         return LOAD_STATE_ESTABLISHING_PROXY_TUNNEL;
       // else, fall through.
@@ skipping 85 matching lines @@
   next_state_ = STATE_TRANSPORT_CONNECT_COMPLETE;
   transport_socket_handle_.reset(new ClientSocketHandle());
   scoped_refptr<TransportSocketParams> direct_params =
       params_->GetDirectConnectionParams();
   return transport_socket_handle_->Init(group_name(), direct_params, priority(),
                                         respect_limits(), callback_,
                                         transport_pool_, net_log());
 }
 
 int SSLConnectJob::DoTransportConnectComplete(int result) {
-  connection_attempts_ = transport_socket_handle_->connection_attempts();
+  connection_attempts_.insert(
+      connection_attempts_.end(),
+      transport_socket_handle_->connection_attempts().begin(),
+      transport_socket_handle_->connection_attempts().end());
   if (result == OK) {
     next_state_ = STATE_SSL_CONNECT;
     transport_socket_handle_->socket()->GetPeerAddress(&server_address_);
   }
 
   return result;
 }
 
 int SSLConnectJob::DoSOCKSConnect() {
   DCHECK(socks_pool_);
@@ skipping 64 matching lines @@
     // Overwriting |connect_start| serves two purposes - it adjusts timing so
     // |connect_start| doesn't include dns times, and it adjusts the time so
     // as not to include time spent waiting for an idle socket.
     connect_timing_.connect_start = socket_connect_timing.connect_start;
     connect_timing_.dns_start = socket_connect_timing.dns_start;
     connect_timing_.dns_end = socket_connect_timing.dns_end;
   }
 
   connect_timing_.ssl_start = base::TimeTicks::Now();
 
+  SSLConfig ssl_config = params_->ssl_config();
+  if (version_interference_probe_) {
+    DCHECK_EQ(SSL_PROTOCOL_VERSION_TLS1_3, ssl_config.version_max);
+    ssl_config.version_max = SSL_PROTOCOL_VERSION_TLS1_2;
+    ssl_config.version_interference_probe = true;

[mmenke, 2017/04/10 19:08:46]

It seems a little bit silly that we're exposing this to the next layer down.  I
don't suppose we could just map successful 1.2 handshakes to the new error code
at this layer?  Or does the next layer down have magic that has to go there?

[davidben, 2017/04/10 19:52:25]

Very little magic, but some. I'd prefer the SSL session caches sharded between
the two modes.

[mmenke, 2017/04/10 19:55:05]

That's unfortunate, but I'll defer to you on that.

+  }
   ssl_socket_ = client_socket_factory_->CreateSSLClientSocket(
-      std::move(transport_socket_handle_), params_->host_and_port(),
-      params_->ssl_config(), context_);
+      std::move(transport_socket_handle_), params_->host_and_port(), ssl_config,
+      context_);
   return ssl_socket_->Connect(callback_);
 }
 
 int SSLConnectJob::DoSSLConnectComplete(int result) {

[mmenke, 2017/04/10 19:08:46]

// Version interference probes should not result in success.
DCHECK(!version_interference_probe_ || result != OK);

[davidben, 2017/04/10 19:52:25]

Done.

   // TODO(rvargas): Remove ScopedTracker below once crbug.com/462784 is fixed.
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "462784 SSLConnectJob::DoSSLConnectComplete"));
 
   connect_timing_.ssl_end = base::TimeTicks::Now();
 
   if (result != OK && !server_address_.address().empty()) {
     connection_attempts_.push_back(ConnectionAttempt(server_address_, result));
     server_address_ = IPEndPoint();
   }
 
   // If we want SPDY over ALPN, make sure it succeeded.
   if (params_->expect_spdy() &&
       ssl_socket_->GetNegotiatedProtocol() != kProtoHTTP2) {
     return ERR_ALPN_NEGOTIATION_FAILED;
   }
 
+  // Perform a TLS 1.3 version interference probe on various connection
+  // errors. The retry will never produce a successful connection but may map
+  // errors to ERR_SSL_VERSION_INTERFERENCE, which signals a probable
+  // version-interfering middlebox.
+  if (params_->ssl_config().version_max == SSL_PROTOCOL_VERSION_TLS1_3 &&
+      !params_->ssl_config().deprecated_cipher_suites_enabled &&
+      !version_interference_probe_) {
+    if (result == ERR_CONNECTION_CLOSED || result == ERR_SSL_PROTOCOL_ERROR ||
+        result == ERR_SSL_VERSION_OR_CIPHER_MISMATCH ||
+        result == ERR_CONNECTION_RESET ||
+        result == ERR_SSL_BAD_RECORD_MAC_ALERT) {
+      // Report the error code for each time a version interference probe is
+      // triggered.
+      UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSLVersionInterferenceProbeTrigger",
+                                  std::abs(result));

[mmenke, 2017/04/10 19:08:46]

I think std::abs is declared in <math>?

[davidben, 2017/04/10 19:52:25]

cstdlib, apparently.

+      net_log().AddEventWithNetErrorCode(
+          NetLogEventType::SSL_VERSION_INTERFERENCE_PROBE, result);
+
+      ResetStateForRetry();
+      version_interference_probe_ = true;
+      version_interference_probe_error_ = result;

[mmenke, 2017/04/10 19:08:46]

This was the error that triggered the probe, not the result of the probe, which
is not what I would have expected from the name.

Maybe original_error?  Or version_interference_probe_triggering_error_?  Not
sure.  The comment for the variable is certainly fine.

[davidben, 2017/04/10 19:52:25]

What about version_interference_error_ just so it's not so long and to match the
histogram below?

[mmenke, 2017/04/10 19:55:06]

SGTM

+      next_state_ = GetInitialState(params_->GetConnectionType());
+      return OK;
+    }
+  }
+
   const std::string& host = params_->host_and_port().host();
   bool is_google =
       host == "google.com" ||
       (host.size() > 11 && host.rfind(".google.com") == host.size() - 11);
 
   // These are hosts that we intend to use in the initial TLS 1.3 deployment.
   // TLS connections to them, whether or not this browser is in the experiment
   // group, form the basis of our comparisons.
   bool tls13_supported =
       (host == "drive.google.com" || host == "mail.google.com");
@@ skipping 84 matching lines @@
   if (is_google) {
     UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSL_Connection_Error_Google",
                                 std::abs(result));
   }
 
   if (tls13_supported) {
     UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSL_Connection_Error_TLS13Experiment",
                                 std::abs(result));
   }
 
+  if (result == ERR_SSL_VERSION_INTERFERENCE) {
+    // Record the error code version interference was detected at.
+    DCHECK(version_interference_probe_);
+    DCHECK_NE(OK, version_interference_probe_error_);
+    UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSLVersionInterferenceError",
+                                std::abs(version_interference_probe_error_));
+  }
+
   if (result == OK || IsCertificateError(result)) {
     SetSocket(std::move(ssl_socket_));
   } else if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {
     error_response_info_.cert_request_info = new SSLCertRequestInfo;
     ssl_socket_->GetSSLCertRequestInfo(
         error_response_info_.cert_request_info.get());
   }
 
   return result;
 }
@@ skipping 10 matching lines @@
   }
   NOTREACHED();
   return STATE_NONE;
 }
 
 int SSLConnectJob::ConnectInternal() {
   next_state_ = GetInitialState(params_->GetConnectionType());
   return DoLoop(OK);
 }
 
+void SSLConnectJob::ResetStateForRetry() {
+  transport_socket_handle_.reset();
+  ssl_socket_.reset();
+  error_response_info_ = HttpResponseInfo();
+  server_address_ = IPEndPoint();
+}
+
 SSLClientSocketPool::SSLConnectJobFactory::SSLConnectJobFactory(
     TransportClientSocketPool* transport_pool,
     SOCKSClientSocketPool* socks_pool,
     HttpProxyClientSocketPool* http_proxy_pool,
     ClientSocketFactory* client_socket_factory,
     const SSLClientSocketContext& context,
     NetLog* net_log)
     : transport_pool_(transport_pool),
       socks_pool_(socks_pool),
       http_proxy_pool_(http_proxy_pool),
@@ skipping 210 matching lines @@
   if (base_.CloseOneIdleSocket())
     return true;
   return base_.CloseOneIdleConnectionInHigherLayeredPool();
 }
 
 void SSLClientSocketPool::OnSSLConfigChanged() {
   FlushWithError(ERR_NETWORK_CHANGED);
 }
 
 }  // namespace net