Obsolete Q043 and Q046 in Chrome

This CL exists to work around an issue where some users are being
configured with an outdated Finch config. It will ensure that we
cannot accidentally configure Chrome to use these old obsolete
versions. This CL includes an escape-hatch, it will be possible
to add the obsolete_versions_allowed flag in Finch in case we
want to intentionally enable obsolete versions for some future
reason. This CL also applies to cronet, though with the exception
that cronet currently allows Q043 as some cronet applications
currently rely on it.

R=renjietang@chromium.org

Change-Id: I275e1797c3cab1e0ddd411520e113cb957365896
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2429868
Commit-Queue: David Schinazi <dschinazi@chromium.org>
Reviewed-by: Renjie Tang <renjietang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810543}

components/cronet/url_request_context_config.cc

@@ -90,6 +90,7 @@ const char kQuicHostWhitelist[] = "host_whitelist";
 const char kQuicEnableSocketRecvOptimization[] =
     "enable_socket_recv_optimization";
 const char kQuicVersion[] = "quic_version";
+const char kQuicObsoleteVersionsAllowed[] = "obsolete_versions_allowed";
 const char kQuicFlags[] = "set_quic_flags";
 
 // AsyncDNS experiment dictionary name.
@@ -316,6 +317,27 @@ void URLRequestContextConfig::ParseAndSetExperimentalOptions(
       if (quic_args->GetString(kQuicVersion, &quic_version_string)) {
         quic::ParsedQuicVersionVector supported_versions =
             quic::ParseQuicVersionVectorString(quic_version_string);
+        bool obsolete_versions_allowed = false;
+        if (!quic_args->GetBoolean(kQuicObsoleteVersionsAllowed,
+                                   &obsolete_versions_allowed) ||
+            !obsolete_versions_allowed) {
+          quic::ParsedQuicVersionVector filtered_versions;
+          quic::ParsedQuicVersionVector obsolete_versions =
+              net::ObsoleteQuicVersions();
+          for (const quic::ParsedQuicVersion& version : supported_versions) {
+            if (version == quic::ParsedQuicVersion::Q043()) {
+              // TODO(dschinazi) Remove this special-casing of Q043 once we no
+              // longer have cronet applications that require it.
+              filtered_versions.push_back(version);
+              continue;
+            }
+            if (std::find(obsolete_versions.begin(), obsolete_versions.end(),
+                          version) == obsolete_versions.end()) {
+              filtered_versions.push_back(version);
+            }
+          }
+          supported_versions = filtered_versions;
+        }
         if (!supported_versions.empty())
           quic_params->supported_versions = supported_versions;
       }

components/cronet/url_request_context_config_unittest.cc

@@ -452,6 +452,112 @@ TEST(URLRequestContextConfigTest, SetUnsupportedQuicVersion) {
             net::DefaultSupportedQuicVersions());
 }
 
+TEST(URLRequestContextConfigTest, SetObsoleteQuicVersion) {
+  // This test configures cronet with an obsolete QUIC version and validates
+  // that cronet ignores that version and uses the default versions.
+  base::test::TaskEnvironment task_environment_(
+      base::test::TaskEnvironment::MainThreadType::IO);
+
+  URLRequestContextConfig config(
+      // Enable QUIC.
+      true,
+      // QUIC User Agent ID.
+      "Default QUIC User Agent ID",
+      // Enable SPDY.
+      true,
+      // Enable Brotli.
+      false,
+      // Type of http cache.
+      URLRequestContextConfig::HttpCacheType::DISK,
+      // Max size of http cache in bytes.
+      1024000,
+      // Disable caching for HTTP responses. Other information may be stored in
+      // the cache.
+      false,
+      // Storage path for http cache and cookie storage.
+      "/data/data/org.chromium.net/app_cronet_test/test_storage",
+      // Accept-Language request header field.
+      "foreign-language",
+      // User-Agent request header field.
+      "fake agent",
+      // JSON encoded experimental options.
+      std::string("{\"QUIC\":{\"quic_version\":\"") +
+          quic::AlpnForVersion(net::ObsoleteQuicVersions().back()) + "\"}}",
+      // MockCertVerifier to use for testing purposes.
+      std::unique_ptr<net::CertVerifier>(),
+      // Enable network quality estimator.
+      false,
+      // Enable Public Key Pinning bypass for local trust anchors.
+      true,
+      // Optional network thread priority.
+      base::Optional<double>());
+
+  net::URLRequestContextBuilder builder;
+  config.ConfigureURLRequestContextBuilder(&builder);
+  // Set a ProxyConfigService to avoid DCHECK failure when building.
+  builder.set_proxy_config_service(
+      std::make_unique<net::ProxyConfigServiceFixed>(
+          net::ProxyConfigWithAnnotation::CreateDirect()));
+  std::unique_ptr<net::URLRequestContext> context(builder.Build());
+  const net::QuicParams* quic_params = context->quic_context()->params();
+  EXPECT_EQ(quic_params->supported_versions,
+            net::DefaultSupportedQuicVersions());
+}
+
+TEST(URLRequestContextConfigTest, SetObsoleteQuicVersionWhenAllowed) {
+  // This test configures cronet with an obsolete QUIC version and explicitly
+  // allows it, then validates that cronet uses that version.
+  base::test::TaskEnvironment task_environment_(
+      base::test::TaskEnvironment::MainThreadType::IO);
+
+  URLRequestContextConfig config(
+      // Enable QUIC.
+      true,
+      // QUIC User Agent ID.
+      "Default QUIC User Agent ID",
+      // Enable SPDY.
+      true,
+      // Enable Brotli.
+      false,
+      // Type of http cache.
+      URLRequestContextConfig::HttpCacheType::DISK,
+      // Max size of http cache in bytes.
+      1024000,
+      // Disable caching for HTTP responses. Other information may be stored in
+      // the cache.
+      false,
+      // Storage path for http cache and cookie storage.
+      "/data/data/org.chromium.net/app_cronet_test/test_storage",
+      // Accept-Language request header field.
+      "foreign-language",
+      // User-Agent request header field.
+      "fake agent",
+      // JSON encoded experimental options.
+      std::string("{\"QUIC\":{\"quic_version\":\"") +
+          quic::AlpnForVersion(net::ObsoleteQuicVersions().back()) +
+          "\",\"obsolete_versions_allowed\":true}}",
+      // MockCertVerifier to use for testing purposes.
+      std::unique_ptr<net::CertVerifier>(),
+      // Enable network quality estimator.
+      false,
+      // Enable Public Key Pinning bypass for local trust anchors.
+      true,
+      // Optional network thread priority.
+      base::Optional<double>());
+
+  net::URLRequestContextBuilder builder;
+  config.ConfigureURLRequestContextBuilder(&builder);
+  // Set a ProxyConfigService to avoid DCHECK failure when building.
+  builder.set_proxy_config_service(
+      std::make_unique<net::ProxyConfigServiceFixed>(
+          net::ProxyConfigWithAnnotation::CreateDirect()));
+  std::unique_ptr<net::URLRequestContext> context(builder.Build());
+  const net::QuicParams* quic_params = context->quic_context()->params();
+  quic::ParsedQuicVersionVector supported_versions = {
+      net::ObsoleteQuicVersions().back()};
+  EXPECT_EQ(quic_params->supported_versions, supported_versions);
+}
+
 TEST(URLRequestContextConfigTest, SetQuicServerMigrationOptions) {
   base::test::TaskEnvironment task_environment_(
       base::test::TaskEnvironment::MainThreadType::IO);

components/network_session_configurator/browser/network_session_configurator.cc

@@ -458,8 +458,25 @@ size_t GetQuicMaxPacketLength(const VariationParameters& quic_trial_params) {
 
 quic::ParsedQuicVersionVector GetQuicVersions(
     const VariationParameters& quic_trial_params) {
-  return quic::ParseQuicVersionVectorString(
-      GetVariationParam(quic_trial_params, "quic_version"));
+  quic::ParsedQuicVersionVector trial_versions =
+      quic::ParseQuicVersionVectorString(
+          GetVariationParam(quic_trial_params, "quic_version"));
+  const bool obsolete_versions_allowed = base::LowerCaseEqualsASCII(
+      GetVariationParam(quic_trial_params, "obsolete_versions_allowed"),
+      "true");
+  if (!obsolete_versions_allowed) {
+    quic::ParsedQuicVersionVector filtered_versions;
+    quic::ParsedQuicVersionVector obsolete_versions =
+        net::ObsoleteQuicVersions();
+    for (const quic::ParsedQuicVersion& version : trial_versions) {
+      if (std::find(obsolete_versions.begin(), obsolete_versions.end(),
+                    version) == obsolete_versions.end()) {
+        filtered_versions.push_back(version);
+      }
+    }
+    trial_versions = filtered_versions;
+  }
+  return trial_versions;
 }
 
 bool ShouldEnableServerPushCancelation(

components/network_session_configurator/browser/network_session_configurator_unittest.cc

@@ -511,35 +511,11 @@ TEST_F(NetworkSessionConfiguratorTest, QuicVersionFromFieldTrialParamsAlpn) {
   EXPECT_EQ(supported_versions, quic_params_.supported_versions);
 }
 
-TEST_F(NetworkSessionConfiguratorTest,
-       MultipleQuicVersionFromFieldTrialParams) {
-  // Note that this test covers the legacy field param mechanism which relies on
-  // QuicVersionToString. We should now be using ALPNs instead.
-  quic::ParsedQuicVersionVector versions_with_quic_crypto =
-      quic::AllSupportedVersionsWithQuicCrypto();
-  ASSERT_LE(2u, versions_with_quic_crypto.size());
-
-  quic::ParsedQuicVersion version1 = versions_with_quic_crypto.front();
-  quic::ParsedQuicVersion version2 = versions_with_quic_crypto.back();
-  std::string quic_versions =
-      quic::QuicVersionToString(version1.transport_version) + "," +
-      quic::QuicVersionToString(version2.transport_version);
-
-  std::map<std::string, std::string> field_trial_params;
-  field_trial_params["quic_version"] = quic_versions;
-  variations::AssociateVariationParams("QUIC", "Enabled", field_trial_params);
-  base::FieldTrialList::CreateFieldTrial("QUIC", "Enabled");
-
-  ParseFieldTrials();
-
-  quic::ParsedQuicVersionVector supported_versions = {version1, version2};
-  EXPECT_EQ(supported_versions, quic_params_.supported_versions);
-}
-
 TEST_F(NetworkSessionConfiguratorTest,
        MultipleQuicVersionFromFieldTrialParamsAlpn) {
-  quic::ParsedQuicVersion version1 = quic::AllSupportedVersions().front();
-  quic::ParsedQuicVersion version2 = quic::AllSupportedVersions().back();
+  ASSERT_LE(2u, quic::AllSupportedVersions().size());
+  quic::ParsedQuicVersion version1 = quic::AllSupportedVersions()[0];
+  quic::ParsedQuicVersion version2 = quic::AllSupportedVersions()[1];
   std::string quic_versions =
       quic::AlpnForVersion(version1) + "," + quic::AlpnForVersion(version2);
 
@@ -924,7 +900,8 @@ class NetworkSessionConfiguratorWithQuicVersionTest
 
 INSTANTIATE_TEST_SUITE_P(QuicVersion,
                          NetworkSessionConfiguratorWithQuicVersionTest,
-                         ::testing::ValuesIn(quic::AllSupportedVersions()));
+                         ::testing::ValuesIn(quic::AllSupportedVersions()),
+                         ::testing::PrintToStringParamName());
 
 TEST_P(NetworkSessionConfiguratorWithQuicVersionTest, QuicVersion) {
   // Note that this test covers the legacy mechanism which relies on
@@ -932,16 +909,14 @@ TEST_P(NetworkSessionConfiguratorWithQuicVersionTest, QuicVersion) {
   if (!version_.UsesQuicCrypto()) {
     return;
   }
-
   base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
   command_line.AppendSwitch(switches::kEnableQuic);
   command_line.AppendSwitchASCII(
       switches::kQuicVersion,
       quic::QuicVersionToString(version_.transport_version));
   ParseCommandLineAndFieldTrials(command_line);
-
-  ASSERT_EQ(1u, quic_params_.supported_versions.size());
-  EXPECT_EQ(version_, quic_params_.supported_versions[0]);
+  quic::ParsedQuicVersionVector expected_versions = {version_};
+  EXPECT_EQ(expected_versions, quic_params_.supported_versions);
 }
 
 TEST_P(NetworkSessionConfiguratorWithQuicVersionTest, QuicVersionAlpn) {
@@ -950,9 +925,8 @@ TEST_P(NetworkSessionConfiguratorWithQuicVersionTest, QuicVersionAlpn) {
   command_line.AppendSwitchASCII(switches::kQuicVersion,
                                  quic::AlpnForVersion(version_));
   ParseCommandLineAndFieldTrials(command_line);
-
-  ASSERT_EQ(1u, quic_params_.supported_versions.size());
-  EXPECT_EQ(version_, quic_params_.supported_versions[0]);
+  quic::ParsedQuicVersionVector expected_versions = {version_};
+  EXPECT_EQ(expected_versions, quic_params_.supported_versions);
 }
 
 TEST_P(NetworkSessionConfiguratorWithQuicVersionTest,
@@ -962,36 +936,126 @@ TEST_P(NetworkSessionConfiguratorWithQuicVersionTest,
   if (!version_.UsesQuicCrypto()) {
     return;
   }
-
+  quic::ParsedQuicVersionVector obsolete_versions = net::ObsoleteQuicVersions();
+  if (std::find(obsolete_versions.begin(), obsolete_versions.end(), version_) !=
+      obsolete_versions.end()) {
+    // Do not test obsolete versions here as those are covered by the
+    // ObsoleteQuicVersion tests.
+    return;
+  }
   std::string quic_versions =
       quic::QuicVersionToString(version_.transport_version) + "," +
       quic::QuicVersionToString(version_.transport_version);
-
   std::map<std::string, std::string> field_trial_params;
   field_trial_params["quic_version"] = quic_versions;
   variations::AssociateVariationParams("QUIC", "Enabled", field_trial_params);
   base::FieldTrialList::CreateFieldTrial("QUIC", "Enabled");
-
   ParseFieldTrials();
-
-  ASSERT_EQ(1u, quic_params_.supported_versions.size());
-  EXPECT_EQ(version_, quic_params_.supported_versions[0]);
+  quic::ParsedQuicVersionVector expected_versions = {version_};
+  EXPECT_EQ(expected_versions, quic_params_.supported_versions);
 }
 
 TEST_P(NetworkSessionConfiguratorWithQuicVersionTest,
        SameQuicVersionsFromFieldTrialParamsAlpn) {
+  quic::ParsedQuicVersionVector obsolete_versions = net::ObsoleteQuicVersions();
+  if (std::find(obsolete_versions.begin(), obsolete_versions.end(), version_) !=
+      obsolete_versions.end()) {
+    // Do not test obsolete versions here as those are covered by the
+    // ObsoleteQuicVersion tests.
+    return;
+  }
   std::string quic_versions =
       quic::AlpnForVersion(version_) + "," + quic::AlpnForVersion(version_);
+  std::map<std::string, std::string> field_trial_params;
+  field_trial_params["quic_version"] = quic_versions;
+  variations::AssociateVariationParams("QUIC", "Enabled", field_trial_params);
+  base::FieldTrialList::CreateFieldTrial("QUIC", "Enabled");
+  ParseFieldTrials();
+  quic::ParsedQuicVersionVector expected_versions = {version_};
+  EXPECT_EQ(expected_versions, quic_params_.supported_versions);
+}
 
+TEST_P(NetworkSessionConfiguratorWithQuicVersionTest, ObsoleteQuicVersion) {
+  // Test that a single obsolete version causes us to use default versions.
+  quic::ParsedQuicVersionVector obsolete_versions = net::ObsoleteQuicVersions();
+  if (std::find(obsolete_versions.begin(), obsolete_versions.end(), version_) ==
+      obsolete_versions.end()) {
+    // Only test obsolete versions here.
+    return;
+  }
+  std::string quic_versions = quic::AlpnForVersion(version_);
   std::map<std::string, std::string> field_trial_params;
   field_trial_params["quic_version"] = quic_versions;
   variations::AssociateVariationParams("QUIC", "Enabled", field_trial_params);
   base::FieldTrialList::CreateFieldTrial("QUIC", "Enabled");
+  ParseFieldTrials();
+  EXPECT_EQ(net::DefaultSupportedQuicVersions(),
+            quic_params_.supported_versions);
+}
 
+TEST_P(NetworkSessionConfiguratorWithQuicVersionTest,
+       ObsoleteQuicVersionAllowed) {
+  // Test that a single obsolete version is used when explicitly allowed.
+  quic::ParsedQuicVersionVector obsolete_versions = net::ObsoleteQuicVersions();
+  if (std::find(obsolete_versions.begin(), obsolete_versions.end(), version_) ==
+      obsolete_versions.end()) {
+    // Only test obsolete versions here.
+    return;
+  }
+  std::string quic_versions = quic::AlpnForVersion(version_);
+  std::map<std::string, std::string> field_trial_params;
+  field_trial_params["quic_version"] = quic_versions;
+  field_trial_params["obsolete_versions_allowed"] = "true";
+  variations::AssociateVariationParams("QUIC", "Enabled", field_trial_params);
+  base::FieldTrialList::CreateFieldTrial("QUIC", "Enabled");
+  ParseFieldTrials();
+  quic::ParsedQuicVersionVector expected_versions = {version_};
+  EXPECT_EQ(expected_versions, quic_params_.supported_versions);
+}
+
+TEST_P(NetworkSessionConfiguratorWithQuicVersionTest,
+       ObsoleteQuicVersionWithGoodVersion) {
+  // Test that when using one obsolete version and a supported version, the
+  // supported version is used.
+  quic::ParsedQuicVersionVector obsolete_versions = net::ObsoleteQuicVersions();
+  if (std::find(obsolete_versions.begin(), obsolete_versions.end(), version_) ==
+      obsolete_versions.end()) {
+    // Only test obsolete versions here.
+    return;
+  }
+  quic::ParsedQuicVersion good_version = quic::AllSupportedVersions().front();
+  std::string quic_versions =
+      quic::AlpnForVersion(version_) + "," + quic::AlpnForVersion(good_version);
+  std::map<std::string, std::string> field_trial_params;
+  field_trial_params["quic_version"] = quic_versions;
+  variations::AssociateVariationParams("QUIC", "Enabled", field_trial_params);
+  base::FieldTrialList::CreateFieldTrial("QUIC", "Enabled");
   ParseFieldTrials();
+  quic::ParsedQuicVersionVector expected_versions = {good_version};
+  EXPECT_EQ(expected_versions, quic_params_.supported_versions);
+}
 
-  ASSERT_EQ(1u, quic_params_.supported_versions.size());
-  EXPECT_EQ(version_, quic_params_.supported_versions[0]);
+TEST_P(NetworkSessionConfiguratorWithQuicVersionTest,
+       ObsoleteQuicVersionAllowedWithGoodVersion) {
+  // Test that when using one obsolete version and a non-obsolete version, and
+  // obsolete versions are allowed, then both are used.
+  quic::ParsedQuicVersionVector obsolete_versions = net::ObsoleteQuicVersions();
+  if (std::find(obsolete_versions.begin(), obsolete_versions.end(), version_) ==
+      obsolete_versions.end()) {
+    // Only test obsolete versions here.
+    return;
+  }
+  quic::ParsedQuicVersion good_version = quic::AllSupportedVersions().front();
+  std::string quic_versions =
+      quic::AlpnForVersion(version_) + "," + quic::AlpnForVersion(good_version);
+  std::map<std::string, std::string> field_trial_params;
+  field_trial_params["quic_version"] = quic_versions;
+  field_trial_params["obsolete_versions_allowed"] = "true";
+  variations::AssociateVariationParams("QUIC", "Enabled", field_trial_params);
+  base::FieldTrialList::CreateFieldTrial("QUIC", "Enabled");
+  ParseFieldTrials();
+  quic::ParsedQuicVersionVector expected_versions = {version_, good_version};
+  EXPECT_EQ(expected_versions, quic_params_.supported_versions);
 }
 
 }  // namespace network_session_configurator

net/quic/quic_context.h

@@ -12,12 +12,21 @@
 
 namespace net {
 
-// Default QUIC supprted versions used in absence of any external configuration.
+// Default QUIC supported versions used in absence of any external
+// configuration.
 inline NET_EXPORT_PRIVATE quic::ParsedQuicVersionVector
 DefaultSupportedQuicVersions() {
   return quic::ParsedQuicVersionVector{quic::ParsedQuicVersion::Q050()};
 }
 
+// Obsolete QUIC supported versions are versions that are supported by the
+// QUIC shared code but that Chrome refuses to use because modern clients
+// should only use versions at least as recent as the oldest default version.
+inline NET_EXPORT_PRIVATE quic::ParsedQuicVersionVector ObsoleteQuicVersions() {
+  return quic::ParsedQuicVersionVector{quic::ParsedQuicVersion::Q043(),
+                                       quic::ParsedQuicVersion::Q046()};
+}
+
 // When a connection is idle for 30 seconds it will be closed.
 constexpr base::TimeDelta kIdleConnectionTimeout =
     base::TimeDelta::FromSeconds(30);