Revert "Revert "Post-launch ECH cleanup""

This reverts commit d871c7a.

chrome/browser/about_flags.cc

@@ -8067,12 +8067,6 @@ const FeatureEntry kFeatureEntries[] = {
      FEATURE_VALUE_TYPE(ash::features::kSettingsAppNotificationSettings)},
 #endif  // BUILDFLAG(IS_CHROMEOS_ASH)
 
-    {"encrypted-client-hello", "Encrypted ClientHello",
-     "When enabled, Chrome will enable Encrypted ClientHello support. This will "
-     "encrypt TLS ClientHello if the server enables the extension via the HTTPS "
-     "DNS record.", kOsAll,
-     FEATURE_VALUE_TYPE(net::features::kEncryptedClientHello)},
-
     {"use-dns-https-svcb-alpn", flag_descriptions::kUseDnsHttpsSvcbAlpnName,
      flag_descriptions::kUseDnsHttpsSvcbAlpnDescription,
      kOsLinux | kOsMac | kOsWin | kOsCrOS | kOsAndroid,

chrome/browser/policy/policy_network_browsertest.cc

@@ -160,8 +160,7 @@ class ECHPolicyTest : public SSLPolicyTest {
   ECHPolicyTest() : ech_server_{net::EmbeddedTestServer::TYPE_HTTPS} {
     scoped_feature_list_.InitWithFeaturesAndParameters(
         /*enabled_features=*/
-        {{net::features::kEncryptedClientHello, {}},
-         {net::features::kUseDnsHttpsSvcb,
+        {{net::features::kUseDnsHttpsSvcb,
           {{"UseDnsHttpsSvcbEnforceSecureResponse", "true"}}}},
         /*disabled_features=*/{});
   }

content/browser/devtools/protocol/devtools_protocol_browsertest.cc

@@ -4116,8 +4116,7 @@ class NetworkResponseProtocolECHTest : public NetworkResponseProtocolTest {
       : ech_server_{net::EmbeddedTestServer::TYPE_HTTPS} {
     scoped_feature_list_.InitWithFeaturesAndParameters(
         /*enabled_features=*/
-        {{net::features::kEncryptedClientHello, {}},
-         {net::features::kUseDnsHttpsSvcb,
+        {{net::features::kUseDnsHttpsSvcb,
           {{"UseDnsHttpsSvcbEnforceSecureResponse", "true"}}}},
         /*disabled_features=*/{});
   }

net/base/features.cc

@@ -101,14 +101,6 @@ BASE_FEATURE(kEnableTLS13EarlyData,
              "EnableTLS13EarlyData",
              base::FEATURE_DISABLED_BY_DEFAULT);
 
-BASE_FEATURE(kEncryptedClientHello,
-             "EncryptedClientHello",
-             base::FEATURE_ENABLED_BY_DEFAULT);
-
-BASE_FEATURE(kEncryptedClientHelloQuic,
-             "EncryptedClientHelloQuic",
-             base::FEATURE_ENABLED_BY_DEFAULT);
-
 BASE_FEATURE(kNetworkQualityEstimator,
              "NetworkQualityEstimator",
              base::FEATURE_DISABLED_BY_DEFAULT);

net/base/features.h

@@ -117,17 +117,6 @@ NET_EXPORT BASE_DECLARE_FEATURE(kEnableIPv6ReachabilityOverride);
 // Enables TLS 1.3 early data.
 NET_EXPORT BASE_DECLARE_FEATURE(kEnableTLS13EarlyData);
 
-// Enables the TLS Encrypted ClientHello feature.
-// https://datatracker.ietf.org/doc/html/draft-ietf-tls-esni-13
-NET_EXPORT BASE_DECLARE_FEATURE(kEncryptedClientHello);
-
-// Enables the TLS Encrypted ClientHello feature for QUIC. Only takes effect if
-// kEncryptedClientHello is also enabled.
-//
-// TODO(crbug.com/1287248): Remove this flag when ECH for QUIC is fully
-// implemented. This flag is just a temporary mechanism for now.
-NET_EXPORT BASE_DECLARE_FEATURE(kEncryptedClientHelloQuic);
-
 // Enables optimizing the network quality estimation algorithms in network
 // quality estimator (NQE).
 NET_EXPORT BASE_DECLARE_FEATURE(kNetworkQualityEstimator);

net/http/http_stream_factory_job_controller_unittest.cc

@@ -6202,15 +6202,6 @@ TEST_F(HttpStreamFactoryJobControllerDnsHttpsAlpnTest, PreconnectNoDnsAlpnH3) {
   EXPECT_TRUE(HttpStreamFactoryPeer::IsJobControllerDeleted(factory_));
 }
 
-class HttpStreamFactoryJobControllerDnsHttpsAlpnEchTest
-    : public HttpStreamFactoryJobControllerDnsHttpsAlpnTest {
- public:
-  HttpStreamFactoryJobControllerDnsHttpsAlpnEchTest()
-      : HttpStreamFactoryJobControllerDnsHttpsAlpnTest(
-            {features::kEncryptedClientHello,
-             features::kEncryptedClientHelloQuic}) {}
-};
-
 // Test that, when an Alt-Svc-based preconnect fails with
 // `ERR_DNS_NO_MATCHING_SUPPORTED_ALPN`, the job controller handles it
 // correctly. This is a regression test for https://crbug.com/1420202.
@@ -6219,7 +6210,7 @@ class HttpStreamFactoryJobControllerDnsHttpsAlpnEchTest
 // was no A/AAAA route. However, we do not implement HTTPS-RR in full yet (see
 // https://crbug.com/1417033), so instead this is only possible in a corner case
 // with ECH.
-TEST_F(HttpStreamFactoryJobControllerDnsHttpsAlpnEchTest,
+TEST_F(HttpStreamFactoryJobControllerDnsHttpsAlpnTest,
        PreconnectAlternateNoDnsAlpn) {
   const char kAlternateHost[] = "alt.example.com";
 

net/quic/quic_chromium_client_session.cc

@@ -1600,9 +1600,7 @@ void QuicChromiumClientSession::OnCanCreateNewOutgoingStream(
 
 quic::QuicSSLConfig QuicChromiumClientSession::GetSSLConfig() const {
   quic::QuicSSLConfig config = quic::QuicSpdyClientSessionBase::GetSSLConfig();
-  if (ssl_config_service_->GetSSLContextConfig()
-          .EncryptedClientHelloEnabled() &&
-      base::FeatureList::IsEnabled(features::kEncryptedClientHelloQuic)) {
+  if (ssl_config_service_->GetSSLContextConfig().ech_enabled) {
     config.ech_grease_enabled = true;
     config.ech_config_list.assign(ech_config_list_.begin(),
                                   ech_config_list_.end());

net/quic/quic_session_pool.cc

@@ -1951,4 +1951,4 @@ bool QuicSessionPool::CryptoConfigCacheIsEmptyForTesting(
   return !cached || cached->IsEmpty();
 }
 
-}  // namespace net
+}  // namespace net

net/quic/quic_session_pool_direct_job.cc

@@ -240,8 +240,7 @@ bool QuicSessionPool::DirectJob::IsSvcbOptional(
   // If SVCB/HTTPS resolution succeeded, the client supports ECH, and all
   // routes support ECH, disable the A/AAAA fallback. See Section 10.1 of
   // draft-ietf-dnsop-svcb-https-11.
-  if (!pool_->ssl_config_service_->GetSSLContextConfig().ech_enabled ||
-      !base::FeatureList::IsEnabled(features::kEncryptedClientHelloQuic)) {
+  if (!pool_->ssl_config_service_->GetSSLContextConfig().ech_enabled) {
     return true;  // ECH is not supported for this request.
   }
 

net/quic/quic_session_pool_test.cc

@@ -13870,30 +13870,6 @@ TEST_P(QuicSessionPoolDnsAliasPoolingTest, IPPooling) {
   EXPECT_EQ(expected_dns_aliases2_, stream2->GetDnsAliases());
 }
 
-class QuicStreamFactoryEchTest : public QuicStreamFactoryTestBase,
-                                 public ::testing::TestWithParam<TestParams> {
- protected:
-  QuicStreamFactoryEchTest()
-      : QuicStreamFactoryTestBase(GetParam().version,
-                                  /*enabled_features=*/
-                                  {features::kEncryptedClientHello,
-                                   features::kEncryptedClientHelloQuic}) {
-    if (GetParam().priority_header_enabled) {
-      feature_list_.InitAndEnableFeature(net::features::kPriorityHeader);
-    } else {
-      feature_list_.InitAndDisableFeature(net::features::kPriorityHeader);
-    }
-  }
-
- private:
-  base::test::ScopedFeatureList feature_list_;
-};
-
-INSTANTIATE_TEST_SUITE_P(VersionIncludeStreamDependencySequence,
-                         QuicStreamFactoryEchTest,
-                         ::testing::ValuesIn(GetTestParams()),
-                         ::testing::PrintToStringParamName());
-
 // Test that, even if DNS does not provide ECH keys, ECH GREASE is enabled.
 TEST_P(QuicSessionPoolTest, EchGrease) {
   Initialize();

net/socket/ssl_client_socket_impl.cc

@@ -809,7 +809,7 @@ int SSLClientSocketImpl::Init() {
     SSL_set_enable_ech_grease(ssl_.get(), 1);
   }
   if (!ssl_config_.ech_config_list.empty()) {
-    DCHECK(context_->config().EncryptedClientHelloEnabled());
+    DCHECK(context_->config().ech_enabled);
     net_log_.AddEvent(NetLogEventType::SSL_ECH_CONFIG_LIST, [&] {
       return base::Value::Dict().Set(
           "bytes", NetLogBinaryValue(ssl_config_.ech_config_list));

net/socket/ssl_client_socket_unittest.cc

@@ -5412,9 +5412,6 @@ TEST_F(SSLClientSocketTest, Tag) {
 }
 
 TEST_F(SSLClientSocketTest, ECH) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kEncryptedClientHello);
-
   SSLServerConfig server_config;
   SSLConfig client_config;
   server_config.ech_keys = MakeTestEchKeys(
@@ -5472,9 +5469,6 @@ TEST_F(SSLClientSocketTest, ECH) {
 // Test that, on key mismatch, the public name can be used to authenticate
 // replacement keys.
 TEST_F(SSLClientSocketTest, ECHWrongKeys) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kEncryptedClientHello);
-
   static const char kPublicName[] = "public.example";
   std::vector<uint8_t> ech_config_list1, ech_config_list2;
   bssl::UniquePtr<SSL_ECH_KEYS> keys1 =
@@ -5516,9 +5510,6 @@ TEST_F(SSLClientSocketTest, ECHWrongKeys) {
 // via the public name. This allows recovery if the server needed to
 // rollback ECH support.
 TEST_F(SSLClientSocketTest, ECHSecurelyDisabled) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kEncryptedClientHello);
-
   static const char kPublicName[] = "public.example";
   std::vector<uint8_t> ech_config_list;
   bssl::UniquePtr<SSL_ECH_KEYS> keys =
@@ -5553,9 +5544,6 @@ TEST_F(SSLClientSocketTest, ECHSecurelyDisabled) {
 // The same as the above, but testing that it also works in TLS 1.2, which
 // otherwise does not support ECH.
 TEST_F(SSLClientSocketTest, ECHSecurelyDisabledTLS12) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kEncryptedClientHello);
-
   static const char kPublicName[] = "public.example";
   std::vector<uint8_t> ech_config_list;
   bssl::UniquePtr<SSL_ECH_KEYS> keys =
@@ -5591,9 +5579,6 @@ TEST_F(SSLClientSocketTest, ECHSecurelyDisabledTLS12) {
 
 // Test that the ECH fallback handshake rejects bad certificates.
 TEST_F(SSLClientSocketTest, ECHFallbackBadCert) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kEncryptedClientHello);
-
   static const char kPublicName[] = "public.example";
   std::vector<uint8_t> ech_config_list1, ech_config_list2;
   bssl::UniquePtr<SSL_ECH_KEYS> keys1 =
@@ -5623,9 +5608,6 @@ TEST_F(SSLClientSocketTest, ECHFallbackBadCert) {
 }
 
 TEST_F(SSLClientSocketTest, InvalidECHConfigList) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kEncryptedClientHello);
-
   ASSERT_TRUE(
       StartEmbeddedTestServer(EmbeddedTestServer::CERT_OK, SSLServerConfig()));
 
@@ -5640,9 +5622,6 @@ TEST_F(SSLClientSocketTest, InvalidECHConfigList) {
 
 // Test that, if no ECHConfigList is available, the client sends ECH GREASE.
 TEST_F(SSLClientSocketTest, ECHGreaseEnabled) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kEncryptedClientHello);
-
   // Configure the server to expect an ECH extension.
   bool ran_callback = false;
   SSLServerConfig server_config;
@@ -5663,11 +5642,11 @@ TEST_F(SSLClientSocketTest, ECHGreaseEnabled) {
   EXPECT_TRUE(ran_callback);
 }
 
-// Test that, if the feature flag is disabled, the client does not send ECH
-// GREASE.
+// Test that, if ECH is disabled, the client does not send ECH GREASE.
 TEST_F(SSLClientSocketTest, ECHGreaseDisabled) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndDisableFeature(features::kEncryptedClientHello);
+  SSLContextConfig context_config;
+  context_config.ech_enabled = false;
+  ssl_config_service_->UpdateSSLConfigAndNotify(context_config);
 
   // Configure the server not to expect an ECH extension.
   bool ran_callback = false;

net/socket/ssl_connect_job.cc

@@ -247,7 +247,7 @@ int SSLConnectJob::DoTransportConnect() {
   std::optional<TransportConnectJob::EndpointResultOverride>
       endpoint_result_override;
   if (ech_retry_configs_) {
-    DCHECK(ssl_client_context()->config().EncryptedClientHelloEnabled());
+    DCHECK(ssl_client_context()->config().ech_enabled);
     DCHECK(endpoint_result_);
     endpoint_result_override.emplace(*endpoint_result_, dns_aliases_);
   }
@@ -360,7 +360,7 @@ int SSLConnectJob::DoSSLConnect() {
       *common_connect_job_params()->ignore_certificate_errors;
   ssl_config.network_anonymization_key = params_->network_anonymization_key();
 
-  if (ssl_client_context()->config().EncryptedClientHelloEnabled()) {
+  if (ssl_client_context()->config().ech_enabled) {
     if (ech_retry_configs_) {
       ssl_config.ech_config_list = *ech_retry_configs_;
     } else if (endpoint_result_) {
@@ -413,9 +413,9 @@ int SSLConnectJob::DoSSLConnectComplete(int result) {
   // control and experiment group.
   const bool is_ech_capable =
       endpoint_result_ && !endpoint_result_->metadata.ech_config_list.empty();
+  const bool ech_enabled = ssl_client_context()->config().ech_enabled;
 
-  if (!ech_retry_configs_ && result == ERR_ECH_NOT_NEGOTIATED &&
-      ssl_client_context()->config().EncryptedClientHelloEnabled()) {
+  if (!ech_retry_configs_ && result == ERR_ECH_NOT_NEGOTIATED && ech_enabled) {
     // 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
@@ -432,15 +432,12 @@ int SSLConnectJob::DoSSLConnectComplete(int result) {
               "bytes", NetLogBinaryValue(*ech_retry_configs_));
         });
 
-    // TODO(https://crbug.com/1091403): Add histograms for how often this
-    // happens.
     ResetStateForRestart();
     next_state_ = GetInitialState(params_->GetConnectionType());
     return OK;
   }
 
-  if (is_ech_capable &&
-      base::FeatureList::IsEnabled(features::kEncryptedClientHello)) {
+  if (is_ech_capable && ech_enabled) {
     // These values are persisted to logs. Entries should not be renumbered
     // and numeric values should never be reused.
     enum class ECHResult {