Implement the ECH recovery flow in SSLConnectJob

This implements the connection setup part of the ECH recovery flow
described in
https://www.ietf.org/archive/id/draft-ietf-tls-esni-13.html#section-6.1.6

This allows us to recover if the DNS and server configuration get out of
sync.

Bug: 1091403
Change-Id: Id5327b9bbdb5a8e6138da5825e56d64bde06ad2f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3510303
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: David Benjamin <davidben@chromium.org>
Cr-Commit-Position: refs/heads/main@{#985059}

net/base/features.h

@@ -153,9 +153,6 @@ NET_EXPORT extern const base::Feature kEnableTLS13EarlyData;
 
 // Enables the TLS Encrypted ClientHello feature.
 // https://datatracker.ietf.org/doc/html/draft-ietf-tls-esni-13
-//
-// TODO(https://crbug.com/1091403): This flag does not currently do much yet.
-// ECH is still in development.
 NET_EXPORT extern const base::Feature kEncryptedClientHello;
 
 // Enables optimizing the network quality estimation algorithms in network

net/log/net_log_event_type_list.h

@@ -788,6 +788,16 @@ EVENT_TYPE(HTTP_PROXY_CONNECT_JOB_CONNECT)
 // The start/end of the WebSocketConnectJob::Connect().
 EVENT_TYPE(WEB_SOCKET_TRANSPORT_CONNECT_JOB_CONNECT)
 
+// A TLS connection attempt failed because ECH was not negotiated. The
+// connection will be retried with a new ECHConfigList from the client-facing
+// server. If the new ECHConfigList is the empty string, the server has rolled
+// back ECH and the new attempt will have ECH disabled.
+//
+//   {
+//     "bytes": <The new ECHConfigList, base64-encoded>
+//   }
+EVENT_TYPE(SSL_CONNECT_JOB_RESTART_WITH_ECH_CONFIG_LIST)
+
 // ------------------------------------------------------------------------
 // ClientSocketPoolBaseHelper
 // ------------------------------------------------------------------------

net/socket/connect_job.cc

@@ -7,11 +7,11 @@
 #include <set>
 #include <utility>
 
-#include "base/no_destructor.h"
 #include "base/trace_event/trace_event.h"
 #include "net/base/connection_endpoint_metadata.h"
 #include "net/base/net_errors.h"
 #include "net/base/trace_constants.h"
+#include "net/dns/host_resolver_results.h"
 #include "net/dns/public/secure_dns_policy.h"
 #include "net/http/http_auth_controller.h"
 #include "net/http/http_proxy_connect_job.h"
@@ -136,9 +136,9 @@ scoped_refptr<SSLCertRequestInfo> ConnectJob::GetCertRequestInfo() {
   return nullptr;
 }
 
-const ConnectionEndpointMetadata& ConnectJob::GetEndpointMetadata() const {
-  static const base::NoDestructor<ConnectionEndpointMetadata> empty_metadata;
-  return *empty_metadata;
+absl::optional<HostResolverEndpointResult>
+ConnectJob::GetHostResolverEndpointResult() const {
+  return absl::nullopt;
 }
 
 void ConnectJob::SetSocket(std::unique_ptr<StreamSocket> socket,

net/socket/connect_job.h

@@ -30,9 +30,9 @@
 namespace net {
 
 class ClientSocketFactory;
-struct ConnectionEndpointMetadata;
 class HostPortPair;
 class HostResolver;
+struct HostResolverEndpointResult;
 class HttpAuthCache;
 class HttpAuthController;
 class HttpAuthHandlerFactory;
@@ -233,9 +233,13 @@ class NET_EXPORT_PRIVATE ConnectJob {
   // SSLCertRequestInfo received. Otherwise, returns nullptr.
   virtual scoped_refptr<SSLCertRequestInfo> GetCertRequestInfo();
 
-  // Returns the `ConnectionEndpointMetadata` structure corresponding to the
-  // chosen route. Should only be called on a successful connect.
-  virtual const ConnectionEndpointMetadata& GetEndpointMetadata() const;
+  // Returns the `HostResolverEndpointResult` structure corresponding to the
+  // chosen route. Should only be called on a successful connect. If the
+  // `ConnectJob` does not make DNS queries, or does not use the SVCB/HTTPS
+  // record, it may return `absl::nullopt`, to avoid callers getting confused by
+  // an empty `IPEndPoint` list.
+  virtual absl::optional<HostResolverEndpointResult>
+  GetHostResolverEndpointResult() const;
 
   const LoadTimingInfo::ConnectTiming& connect_timing() const {
     return connect_timing_;

net/socket/socket_test_util.cc

@@ -791,8 +791,8 @@ MockClientSocketFactory::CreateTransportClientSocket(
   SocketDataProvider* data_provider = mock_tcp_data_.GetNextWithoutAsserting();
   if (!data_provider)
     data_provider = mock_data_.GetNext();
-  std::unique_ptr<MockTCPClientSocket> socket(
-      new MockTCPClientSocket(addresses, net_log, data_provider));
+  auto socket =
+      std::make_unique<MockTCPClientSocket>(addresses, net_log, data_provider);
   if (enable_read_if_ready_)
     socket->set_enable_read_if_ready(enable_read_if_ready_);
   return std::move(socket);
@@ -850,6 +850,10 @@ std::unique_ptr<SSLClientSocket> MockClientSocketFactory::CreateSSLClientSocket(
     EXPECT_EQ(*next_ssl_data->expected_disable_legacy_crypto,
               ssl_config.disable_legacy_crypto);
   }
+  if (next_ssl_data->expected_ech_config_list) {
+    EXPECT_EQ(*next_ssl_data->expected_ech_config_list,
+              ssl_config.ech_config_list);
+  }
   return std::unique_ptr<SSLClientSocket>(new MockSSLClientSocket(
       std::move(stream_socket), host_and_port, ssl_config, next_ssl_data));
 }
@@ -953,6 +957,9 @@ MockTCPClientSocket::MockTCPClientSocket(const AddressList& addresses,
   DCHECK(data_);
   peer_addr_ = data->connect_data().peer_addr;
   data_->Initialize(this);
+  if (data_->expected_addresses()) {
+    EXPECT_EQ(*data_->expected_addresses(), addresses);
+  }
 }
 
 MockTCPClientSocket::~MockTCPClientSocket() {
@@ -1502,10 +1509,7 @@ int MockSSLClientSocket::ExportKeyingMaterial(const base::StringPiece& label,
 }
 
 std::vector<uint8_t> MockSSLClientSocket::GetECHRetryConfigs() {
-  // TODO(crbug.com/1091403): Add a mechanism to specify this, when testing the
-  // retry portions of the recovery flow.
-  NOTIMPLEMENTED();
-  return {};
+  return data_->ech_retry_configs;
 }
 
 void MockSSLClientSocket::RunCallbackAsync(CompletionOnceCallback callback,

net/socket/socket_test_util.h

@@ -287,6 +287,13 @@ class SocketDataProvider {
   }
   bool set_keep_alive_result() const { return set_keep_alive_result_; }
 
+  const absl::optional<AddressList>& expected_addresses() const {
+    return expected_addresses_;
+  }
+  void set_expected_addresses(net::AddressList addresses) {
+    expected_addresses_ = std::move(addresses);
+  }
+
   // Returns true if the request should be considered idle, for the purposes of
   // IsConnectedAndIdle.
   virtual bool IsIdle() const;
@@ -325,6 +332,7 @@ class SocketDataProvider {
   int set_send_buffer_size_result_ = net::OK;
   bool set_no_delay_result_ = true;
   bool set_keep_alive_result_ = true;
+  absl::optional<AddressList> expected_addresses_;
 };
 
 // The AsyncSocket is an interface used by the SocketDataProvider to
@@ -487,6 +495,9 @@ struct SSLSocketDataProvider {
   // Result for GetSSLCertRequestInfo().
   SSLCertRequestInfo* cert_request_info;
 
+  // Result for GetECHRetryConfigs().
+  std::vector<uint8_t> ech_retry_configs;
+
   absl::optional<NextProtoVector> next_protos_expected_in_ssl_config;
 
   uint16_t expected_ssl_version_min;
@@ -496,6 +507,7 @@ struct SSLSocketDataProvider {
   absl::optional<HostPortPair> expected_host_and_port;
   absl::optional<NetworkIsolationKey> expected_network_isolation_key;
   absl::optional<bool> expected_disable_legacy_crypto;
+  absl::optional<std::vector<uint8_t>> expected_ech_config_list;
 
   bool is_connect_data_consumed = false;
   bool is_confirm_data_consumed = false;

net/socket/ssl_connect_job.cc

@@ -23,12 +23,14 @@
 #include "net/cert/x509_util.h"
 #include "net/http/http_proxy_connect_job.h"
 #include "net/log/net_log_source_type.h"
+#include "net/log/net_log_values.h"
 #include "net/log/net_log_with_source.h"
 #include "net/socket/client_socket_factory.h"
 #include "net/socket/client_socket_handle.h"
 #include "net/socket/socks_connect_job.h"
 #include "net/socket/ssl_client_socket.h"
 #include "net/socket/transport_connect_job.h"
+#include "net/socket/websocket_transport_connect_job.h"
 #include "net/ssl/ssl_cert_request_info.h"
 #include "net/ssl/ssl_connection_status_flags.h"
 #include "net/ssl/ssl_info.h"
@@ -274,9 +276,26 @@ int SSLConnectJob::DoTransportConnect() {
   DCHECK(!TimerIsRunning());
 
   next_state_ = STATE_TRANSPORT_CONNECT_COMPLETE;
-  nested_connect_job_ = TransportConnectJob::CreateTransportConnectJob(
-      params_->GetDirectConnectionParams(), priority(), socket_tag(),
-      common_connect_job_params(), this, &net_log());
+  if (!common_connect_job_params()->websocket_endpoint_lock_manager) {
+    // If this is an ECH retry, connect to the same server as before.
+    absl::optional<TransportConnectJob::EndpointResultOverride>
+        endpoint_result_override;
+    if (ech_retry_configs_) {
+      DCHECK(base::FeatureList::IsEnabled(features::kEncryptedClientHello));
+      DCHECK(endpoint_result_);
+      endpoint_result_override.emplace(*endpoint_result_, dns_aliases_);
+    }
+    nested_connect_job_ = std::make_unique<TransportConnectJob>(
+        priority(), socket_tag(), common_connect_job_params(),
+        params_->GetDirectConnectionParams(), this, &net_log(),
+        std::move(endpoint_result_override));
+  } else {
+    // TODO(https://crbug.com/1291352): Support ECH for WebSockets.
+    DCHECK(!ech_retry_configs_);
+    nested_connect_job_ = std::make_unique<WebSocketTransportConnectJob>(
+        priority(), socket_tag(), common_connect_job_params(),
+        params_->GetDirectConnectionParams(), this, &net_log());
+  }
   return nested_connect_job_->Connect();
 }
 
@@ -374,14 +393,30 @@ int SSLConnectJob::DoSSLConnect() {
   ssl_negotiation_started_ = true;
   connect_timing_.ssl_start = base::TimeTicks::Now();
 
+  // Save the `HostResolverEndpointResult`. `nested_connect_job_` is destroyed
+  // at the end of this function.
+  endpoint_result_ = nested_connect_job_->GetHostResolverEndpointResult();
+
   SSLConfig ssl_config = params_->ssl_config();
   ssl_config.network_isolation_key = params_->network_isolation_key();
   ssl_config.privacy_mode = params_->privacy_mode();
   ssl_config.disable_legacy_crypto = disable_legacy_crypto_with_fallback_;
+
   if (base::FeatureList::IsEnabled(features::kEncryptedClientHello)) {
-    ssl_config.ech_config_list =
-        nested_connect_job_->GetEndpointMetadata().ech_config_list;
+    if (ech_retry_configs_) {
+      ssl_config.ech_config_list = *ech_retry_configs_;
+    } else if (endpoint_result_) {
+      ssl_config.ech_config_list = endpoint_result_->metadata.ech_config_list;
+    }
+    if (!ssl_config.ech_config_list.empty()) {
+      // Overriding the DNS lookup only works for direct connections. We
+      // currently do not support ECH with other connection types.
+      DCHECK_EQ(params_->GetConnectionType(), SSLSocketParams::DIRECT);
+      // TODO(https://crbug.com/1291352): Support ECH for WebSockets.
+      DCHECK(!common_connect_job_params()->websocket_endpoint_lock_manager);
+    }
   }
+
   ssl_socket_ = client_socket_factory()->CreateSSLClientSocket(
       ssl_client_context(), std::move(nested_socket_), params_->host_and_port(),
       ssl_config);
@@ -416,6 +451,33 @@ int SSLConnectJob::DoSSLConnectComplete(int result) {
     return OK;
   }
 
+  if (base::FeatureList::IsEnabled(features::kEncryptedClientHello) &&
+      !ech_retry_configs_ && result == ERR_ECH_NOT_NEGOTIATED) {
+    // We used ECH, and the server could not decrypt the ClientHello. However,
+    // it was able to handshake with the public name and send authenticated
+    // retry configs. If this is not the first time around, retry the connection
+    // with the new ECHConfigList, or with ECH disabled (empty retry configs),
+    // as directed.
+    //
+    // See
+    // https://www.ietf.org/archive/id/draft-ietf-tls-esni-13.html#section-6.1.6
+    DCHECK(endpoint_result_ &&
+           !endpoint_result_->metadata.ech_config_list.empty());
+    ech_retry_configs_ = ssl_socket_->GetECHRetryConfigs();
+    net_log().AddEvent(
+        NetLogEventType::SSL_CONNECT_JOB_RESTART_WITH_ECH_CONFIG_LIST, [&] {
+          base::Value dict(base::Value::Type::DICTIONARY);
+          dict.SetKey("bytes", NetLogBinaryValue(*ech_retry_configs_));
+          return dict;
+        });
+
+    // TODO(https://crbug.com/1091403): Add histograms for how often this
+    // happens.
+    ResetStateForRestart();
+    next_state_ = GetInitialState(params_->GetConnectionType());
+    return OK;
+  }
+
   const std::string& host = params_->host_and_port().host();
 
   if (result == OK) {

net/socket/ssl_connect_job.h

@@ -5,9 +5,12 @@
 #ifndef NET_SOCKET_SSL_CONNECT_JOB_H_
 #define NET_SOCKET_SSL_CONNECT_JOB_H_
 
+#include <stdint.h>
+
 #include <memory>
 #include <set>
 #include <string>
+#include <vector>
 
 #include "base/memory/ref_counted.h"
 #include "base/time/time.h"
@@ -16,12 +19,14 @@
 #include "net/base/net_export.h"
 #include "net/base/network_isolation_key.h"
 #include "net/base/privacy_mode.h"
+#include "net/dns/host_resolver_results.h"
 #include "net/dns/public/resolve_error_info.h"
 #include "net/socket/connect_job.h"
 #include "net/socket/connection_attempts.h"
 #include "net/socket/ssl_client_socket.h"
 #include "net/ssl/ssl_cert_request_info.h"
 #include "net/ssl/ssl_config_service.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
 
 namespace net {
 
@@ -205,6 +210,14 @@ class NET_EXPORT_PRIVATE SSLConnectJob : public ConnectJob,
   // lifetime and the aliases can no longer be retrieved from there by by the
   // time that the aliases are needed to be passed in SetSocket.
   std::set<std::string> dns_aliases_;
+
+  // The endpoint result used by `nested_connect_job_`. Stored because
+  // `nested_connect_job_` has a limited lifetime.
+  absl::optional<HostResolverEndpointResult> endpoint_result_;
+
+  // If not `absl::nullopt`, the ECH retry configs to use in the ECH recovery
+  // flow. `endpoint_result_` will then contain the endpoint to reconnect to.
+  absl::optional<std::vector<uint8_t>> ech_retry_configs_;
 };
 
 }  // namespace net