Revert "Add Finch param to control TLS 1.3 Downgrade enforcement on known/unknown roots."

This reverts commit 75ffb181.

Reason for revert: static initializer in about_flags.cc, see https://ci.chromium.org/buildbot/chromium.chrome/Google%20Chrome%20Linux%20x64/37607

Original change's description:
> Add Finch param to control TLS 1.3 Downgrade enforcement on known/unknown roots.
> 
> Change-Id: I88932442b714820944b4608e106eaf47536bf00d
> Reviewed-on: https://chromium-review.googlesource.com/c/1329344
> Commit-Queue: Steven Valdez <svaldez@chromium.org>
> Reviewed-by: David Benjamin <davidben@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#607712}

TBR=davidben@chromium.org,svaldez@chromium.org

Change-Id: I638f9065d117b2885fa947c343234981eacfb991
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/1334412Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607763}

chrome/browser/about_flags.cc

@@ -785,13 +785,6 @@ const FeatureEntry::Choice kTLS13VariantChoices[] = {
      switches::kTLS13VariantFinal},
 };
 
-const FeatureEntry::FeatureParam kEnforceTLS13DowngradeKnownOnly[] = {
-    {"known_roots_only", "true"}};
-
-const FeatureEntry::FeatureVariation kEnforceTLS13DowngradeFeatureVariations[] =
-    {{"(Known Root Only)", kEnforceTLS13DowngradeKnownOnly,
-      base::size(kEnforceTLS13DowngradeKnownOnly), nullptr}};
-
 #if !defined(OS_ANDROID)
 const FeatureEntry::Choice kEnableAudioFocusChoices[] = {
     {flag_descriptions::kEnableAudioFocusDisabled, "", ""},
@@ -2603,10 +2596,7 @@ const FeatureEntry kFeatureEntries[] = {
      MULTI_VALUE_TYPE(kTLS13VariantChoices)},
     {"enforce-tls13-downgrade", flag_descriptions::kEnforceTLS13DowngradeName,
      flag_descriptions::kEnforceTLS13DowngradeDescription, kOsAll,
-     FEATURE_WITH_PARAMS_VALUE_TYPE(
-         net::features::kEnforceTLS13Downgrade,
-         kEnforceTLS13DowngradeFeatureVariations,
-         net::features::kEnforceTLS13Downgrade.name)},
+     FEATURE_VALUE_TYPE(net::features::kEnforceTLS13Downgrade)},
     {"enable-scroll-anchor-serialization",
      flag_descriptions::kEnableScrollAnchorSerializationName,
      flag_descriptions::kEnableScrollAnchorSerializationDescription, kOsAll,

net/socket/ssl_client_socket_impl.cc

@@ -845,9 +845,7 @@ int SSLClientSocketImpl::Init() {
       break;
   }
 
-  if (!base::FeatureList::IsEnabled(features::kEnforceTLS13Downgrade) ||
-      base::GetFieldTrialParamByFeatureAsBool(features::kEnforceTLS13Downgrade,
-                                              "known_roots_only", false)) {
+  if (!base::FeatureList::IsEnabled(features::kEnforceTLS13Downgrade)) {
     SSL_set_ignore_tls13_downgrade(ssl_.get(), 1);
   }
 
@@ -1143,21 +1141,17 @@ int SSLClientSocketImpl::DoVerifyCertComplete(int result) {
       }
     }
 
-    bool enforce_tls13_downgrade_known_roots_only =
-        base::GetFieldTrialParamByFeatureAsBool(
-            features::kEnforceTLS13Downgrade, "known_roots_only", false);
-    if (!base::FeatureList::IsEnabled(features::kEnforceTLS13Downgrade) ||
-        enforce_tls13_downgrade_known_roots_only) {
+    if (!base::FeatureList::IsEnabled(features::kEnforceTLS13Downgrade)) {
       // Record metrics on the TLS 1.3 anti-downgrade mechanism. This is only
       // recorded when enforcement is disabled. (When enforcement is enabled,
       // the connection will fail with ERR_TLS13_DOWNGRADE_DETECTED.) See
       // https://crbug.com/boringssl/226.
       //
       // Record metrics for both servers overall and the TLS 1.3 experiment
-      // set. These metrics are only useful on TLS 1.3 servers, so the latter
-      // is more precise, but there is a large enough TLS 1.3 deployment that
-      // the overall numbers may be more robust. In particular, the
-      // DowngradeType metrics do not need to be filtered.
+      // set. These metrics are only useful on TLS 1.3 servers, so the latter is
+      // more precise, but there is a large enough TLS 1.3 deployment that the
+      // overall numbers may be more robust. In particular, the DowngradeType
+      // metrics do not need to be filtered.
       bool is_downgrade = !!SSL_is_tls13_downgrade(ssl_.get());
       UMA_HISTOGRAM_BOOLEAN("Net.SSLTLS13Downgrade", is_downgrade);
       bool is_tls13_experiment_host =
@@ -1171,8 +1165,7 @@ int SSLClientSocketImpl::DoVerifyCertComplete(int result) {
         // Record whether connections which hit the downgrade used known vs
         // unknown roots and which key exchange type.
 
-        // This enum is persisted into histograms. Values may not be
-        // renumbered.
+        // This enum is persisted into histograms. Values may not be renumbered.
         enum class DowngradeType {
           kKnownRootRSA = 0,
           kKnownRootECDHE = 1,
@@ -1197,14 +1190,6 @@ int SSLClientSocketImpl::DoVerifyCertComplete(int result) {
                                     type);
         }
       }
-
-      if (enforce_tls13_downgrade_known_roots_only &&
-          server_cert_verify_result_.is_issued_by_known_root) {
-        // Exit DoHandshakeLoop and return the result to the caller to
-        // Connect.
-        DCHECK_EQ(STATE_NONE, next_handshake_state_);
-        return ERR_TLS13_DOWNGRADE_DETECTED;
-      }
     }
   }
 

net/socket/ssl_client_socket_unittest.cc

@@ -4846,64 +4846,6 @@ TEST_F(SSLClientSocketTest, TLS13DowngradeIgnoredAtTLS10) {
             SSLConnectionStatusToVersion(info.connection_status));
 }
 
-// Test downgrade enforcement enforces known roots when configured to.
-TEST_F(SSLClientSocketTest, TLS13DowngradeEnforcedKnownKnown) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeatureWithParameters(
-      features::kEnforceTLS13Downgrade, {{"known_roots_only", "true"}});
-
-  SpawnedTestServer::SSLOptions ssl_options;
-  ssl_options.simulate_tls13_downgrade = true;
-  ssl_options.tls_max_version =
-      SpawnedTestServer::SSLOptions::TLS_MAX_VERSION_TLS1_2;
-  ASSERT_TRUE(StartTestServer(ssl_options));
-
-  scoped_refptr<X509Certificate> server_cert =
-      spawned_test_server()->GetCertificate();
-
-  // Certificate is trusted and chains to a public root.
-  CertVerifyResult verify_result;
-  verify_result.is_issued_by_known_root = true;
-  verify_result.verified_cert = server_cert;
-  cert_verifier_->AddResultForCert(server_cert.get(), verify_result, OK);
-
-  SSLConfig config;
-  config.version_max = SSL_PROTOCOL_VERSION_TLS1_3;
-  int rv;
-  ASSERT_TRUE(CreateAndConnectSSLClientSocket(config, &rv));
-  EXPECT_THAT(rv, IsError(ERR_TLS13_DOWNGRADE_DETECTED));
-  EXPECT_FALSE(sock_->IsConnected());
-}
-
-// Test downgrade enforcement ignores unknown roots when configured to.
-TEST_F(SSLClientSocketTest, TLS13DowngradeEnforcedKnownUnknown) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeatureWithParameters(
-      features::kEnforceTLS13Downgrade, {{"known_roots_only", "true"}});
-
-  SpawnedTestServer::SSLOptions ssl_options;
-  ssl_options.simulate_tls13_downgrade = true;
-  ssl_options.tls_max_version =
-      SpawnedTestServer::SSLOptions::TLS_MAX_VERSION_TLS1_2;
-  ASSERT_TRUE(StartTestServer(ssl_options));
-
-  scoped_refptr<X509Certificate> server_cert =
-      spawned_test_server()->GetCertificate();
-
-  // Certificate is trusted and chains to a public root.
-  CertVerifyResult verify_result;
-  verify_result.is_issued_by_known_root = false;
-  verify_result.verified_cert = server_cert;
-  cert_verifier_->AddResultForCert(server_cert.get(), verify_result, OK);
-
-  SSLConfig config;
-  config.version_max = SSL_PROTOCOL_VERSION_TLS1_3;
-  int rv;
-  ASSERT_TRUE(CreateAndConnectSSLClientSocket(config, &rv));
-  EXPECT_THAT(rv, IsOk());
-  EXPECT_TRUE(sock_->IsConnected());
-}
-
 struct TLS13DowngradeMetricsParams {
   bool downgrade;
   bool known_root;