1746a920a7ddafcff0f8ea5ddb6d75e54cb7a21a - chromium/src

[davi...@chromium.org]

commit 1746a920a7ddafcff0f8ea5ddb6d75e54cb7a21a
Author: David Benjamin <davi...@chromium.org>
AuthorDate: Thu Oct 14 17:16:57 2021
Commit: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
CommitDate: Thu Oct 14 17:16:57 2021

Unwind kLegacyTLSEnforced feature flag

The feature shipped, so we no longer need the flag. This doesn't change
behavior, just unwinds the old feature flag to toggle the existing
interstitial.

Bug: 1044750
Change-Id: I4af1b81f83e3db9d00f0ac4b8fe4dbd564f85ed1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3216528
Reviewed-by: Chris Thompson <cth...@chromium.org>
Reviewed-by: Ben Kelly <wande...@chromium.org>
Reviewed-by: Mustafa Emre Acer <mea...@chromium.org>
Commit-Queue: David Benjamin <davi...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#931566}

diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
index 4cfe7db..6b653f0 100644
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -6448,10 +6448,6 @@
FEATURE_VALUE_TYPE(features::kRunVideoCaptureServiceInBrowserProcess)},
#endif // defined(OS_WIN)

- {"legacy-tls-enforced", flag_descriptions::kLegacyTLSEnforcedName,
- flag_descriptions::kLegacyTLSEnforcedDescription, kOsDesktop | kOsAndroid,
- FEATURE_VALUE_TYPE(net::features::kLegacyTLSEnforced)},
-
{"double-buffer-compositing",
flag_descriptions::kDoubleBufferCompositingName,
flag_descriptions::kDoubleBufferCompositingDescription, kOsCrOS,
diff --git a/chrome/browser/flag-metadata.json b/chrome/browser/flag-metadata.json
index 7c5250f..e2188bc 100644
--- a/chrome/browser/flag-metadata.json
+++ b/chrome/browser/flag-metadata.json
@@ -3617,11 +3617,6 @@
"expiry_milestone": 96
},
{
- "name": "legacy-tls-enforced",
- "owners": [ "cthomp" ],
- "expiry_milestone": 92
- },
- {
"name": "legacy-tls-interstitial",
"owners": [ "cthomp" ],
"expiry_milestone": 92
diff --git a/chrome/browser/flag_descriptions.cc b/chrome/browser/flag_descriptions.cc
index 6832998..8001a955 100644
--- a/chrome/browser/flag_descriptions.cc
+++ b/chrome/browser/flag_descriptions.cc
@@ -1585,13 +1585,6 @@
const char kJourneysOmniboxActionDescription[] =
"Enables the History Journeys Omnibox Action.";

-const char kLegacyTLSEnforcedName[] =
- "Enforce deprecation of legacy TLS versions";
-const char kLegacyTLSEnforcedDescription[] =
- "Enable connection errors and interstitials for sites that use legacy TLS "
- "versions (TLS 1.0 and TLS 1.1), which are deprecated and will be removed "
- " in the future.";
-
const char kLensCameraAssistedSearchName[] =
"Google Lens in Omnibox and New Tab Page";
const char kLensCameraAssistedSearchDescription[] =
diff --git a/chrome/browser/flag_descriptions.h b/chrome/browser/flag_descriptions.h
index 0bed0a6..0ed58e0 100644
--- a/chrome/browser/flag_descriptions.h
+++ b/chrome/browser/flag_descriptions.h
@@ -909,9 +909,6 @@
extern const char kJourneysOmniboxActionName[];
extern const char kJourneysOmniboxActionDescription[];

-extern const char kLegacyTLSEnforcedName[];
-extern const char kLegacyTLSEnforcedDescription[];
-
extern const char kLensCameraAssistedSearchName[];
extern const char kLensCameraAssistedSearchDescription[];

diff --git a/chrome/browser/ssl/ssl_browsertest.cc b/chrome/browser/ssl/ssl_browsertest.cc
index d6d8bde..2bec1be 100644
--- a/chrome/browser/ssl/ssl_browsertest.cc
+++ b/chrome/browser/ssl/ssl_browsertest.cc
@@ -7890,23 +7890,8 @@
"*should enable TLS 1.2 or later*"));
}

-class LegacyTLSInterstitialTest : public TLSLegacyVersionSSLUITest {
- public:
- LegacyTLSInterstitialTest() {
- feature_list_.InitAndEnableFeature(net::features::kLegacyTLSEnforced);
- }
-
- LegacyTLSInterstitialTest(const LegacyTLSInterstitialTest&) = delete;
- LegacyTLSInterstitialTest& operator=(const LegacyTLSInterstitialTest&) =
- delete;
-
- private:
- base::test::ScopedFeatureList feature_list_;
-};
-
-// When kLegacyTLSEnforcement is enabled, visiting a legacy TLS page should
-// show the legacy TLS specific interstitial.
-IN_PROC_BROWSER_TEST_F(LegacyTLSInterstitialTest, ShowsInterstitial) {
+// Visiting a legacy TLS page should show the legacy TLS specific interstitial.
+IN_PROC_BROWSER_TEST_F(TLSLegacyVersionSSLUITest, ShowsInterstitial) {
SetTLSVersion(net::SSL_PROTOCOL_VERSION_TLS1);
ASSERT_TRUE(https_server()->Start());
base::HistogramTester histograms;
@@ -7946,7 +7931,7 @@
// Test that attempting to set the SSLVersionMin enterprise policy to "tls1"
// (which is no longer supported) does not prevent the legacy TLS interstitial
// from being shown.
-IN_PROC_BROWSER_TEST_F(LegacyTLSInterstitialTest,
+IN_PROC_BROWSER_TEST_F(TLSLegacyVersionSSLUITest,
PolicyDoesNotOverrideInterstitial) {
// Attempt to set the SSLVersionMin policy.
base::Value policy_value("tls1"); // TLS 1.0
@@ -7966,7 +7951,7 @@

// Check that if we have bypassed the legacy TLS error previously and then the
// server responded with TLS 1.2, we drop the error exception.
-IN_PROC_BROWSER_TEST_F(LegacyTLSInterstitialTest, FixedServerDropsBypass) {
+IN_PROC_BROWSER_TEST_F(TLSLegacyVersionSSLUITest, FixedServerDropsBypass) {
GURL kSiteWithLegacyTLS("https://example.test/legacy-tls");
GURL kSiteWithModernTLS("https://example.test/modern-tls");

@@ -8020,7 +8005,7 @@

// Check that cliking the "back to safety" button works for the legacy TLS
// interstitial.
-IN_PROC_BROWSER_TEST_F(LegacyTLSInterstitialTest, BackToSafety) {
+IN_PROC_BROWSER_TEST_F(TLSLegacyVersionSSLUITest, BackToSafety) {
base::HistogramTester histograms;

// Connect over TLS 1.0 and don't proceed through the interstitial.
@@ -8063,7 +8048,7 @@
};

// Tests that resources loaded over legacy TLS should not be cached.
-IN_PROC_BROWSER_TEST_F(LegacyTLSInterstitialTest, LegacyTLSPagesNotCached) {
+IN_PROC_BROWSER_TEST_F(TLSLegacyVersionSSLUITest, LegacyTLSPagesNotCached) {
// Connect over TLS 1.0 and proceed through the interstitial to set an error
// bypass.
SetTLSVersion(net::SSL_PROTOCOL_VERSION_TLS1);
@@ -8091,7 +8076,7 @@

// Tests that a page with legacy TLS and HSTS shows a bypassable interstitial
// rather than a hard non-bypassable HSTS warning.
-IN_PROC_BROWSER_TEST_F(LegacyTLSInterstitialTest, LegacyTLSNotFatal) {
+IN_PROC_BROWSER_TEST_F(TLSLegacyVersionSSLUITest, LegacyTLSNotFatal) {
// Set HSTS for the test page.
ssl_test_util::SetHSTSForHostName(browser()->profile(), kHstsTestHostName);

diff --git a/chrome/browser/ui/page_info/page_info_unittest.cc b/chrome/browser/ui/page_info/page_info_unittest.cc
index 5ca3ad8..828b5e0 100644
--- a/chrome/browser/ui/page_info/page_info_unittest.cc
+++ b/chrome/browser/ui/page_info/page_info_unittest.cc
@@ -997,9 +997,6 @@

// Tests that the site connection status is correctly set for Legacy TLS sites.
TEST_F(PageInfoTest, LegacyTLS) {
- base::test::ScopedFeatureList scoped_feature_list;
- scoped_feature_list.InitAndEnableFeature(net::features::kLegacyTLSEnforced);
-
security_level_ = security_state::WARNING;
visible_security_state_.url = GURL("https://scheme-is-cryptographic.test");
visible_security_state_.certificate = cert();
diff --git a/content/browser/service_worker/service_worker_new_script_loader_unittest.cc b/content/browser/service_worker/service_worker_new_script_loader_unittest.cc
index fa71469..04fc4cd 100644
--- a/content/browser/service_worker/service_worker_new_script_loader_unittest.cc
+++ b/content/browser/service_worker/service_worker_new_script_loader_unittest.cc
@@ -459,19 +459,8 @@
histogram_tester.ExpectTotalCount(kHistogramWriteResponseResult, 0);
}

-class ServiceWorkerNewScriptLoaderTestWithLegacyTLSEnforced
- : public ServiceWorkerNewScriptLoaderTest {
- public:
- ServiceWorkerNewScriptLoaderTestWithLegacyTLSEnforced() {
- feature_list_.InitAndEnableFeature(net::features::kLegacyTLSEnforced);
- }
-
- private:
- base::test::ScopedFeatureList feature_list_;
-};
-
// Tests that service workers fail to load over a connection with legacy TLS.
-TEST_F(ServiceWorkerNewScriptLoaderTestWithLegacyTLSEnforced, Error_LegacyTLS) {
+TEST_F(ServiceWorkerNewScriptLoaderTest, Error_LegacyTLS) {
base::HistogramTester histogram_tester;

std::unique_ptr<network::TestURLLoaderClient> client;
diff --git a/net/base/features.cc b/net/base/features.cc
index 672f24b..7c0bd80 100644
--- a/net/base/features.cc
+++ b/net/base/features.cc
@@ -196,9 +196,6 @@
const base::Feature kTurnOffStreamingMediaCachingAlways{
"TurnOffStreamingMediaCachingAlways", base::FEATURE_DISABLED_BY_DEFAULT};

-const base::Feature kLegacyTLSEnforced{"LegacyTLSEnforced",
- base::FEATURE_ENABLED_BY_DEFAULT};
-
const base::Feature kSchemefulSameSite{"SchemefulSameSite",
base::FEATURE_ENABLED_BY_DEFAULT};

diff --git a/net/base/features.h b/net/base/features.h
index fcdb5c4..caff480 100644
--- a/net/base/features.h
+++ b/net/base/features.h
@@ -295,12 +295,6 @@
// Turns off streaming media caching to disk always.
NET_EXPORT extern const base::Feature kTurnOffStreamingMediaCachingAlways;

-// When enabled, sites that use TLS versions below the |version_min_warn|
-// threshold are marked with the LEGACY_TLS CertStatus and return an
-// ERR_SSL_OBSOLETE_VERSION error. This is used to trigger an interstitial
-// warning for these pages.
-NET_EXPORT extern const base::Feature kLegacyTLSEnforced;
-
// When enabled this feature will cause same-site calculations to take into
// account the scheme of the site-for-cookies and the request/response url.
NET_EXPORT extern const base::Feature kSchemefulSameSite;
diff --git a/net/socket/ssl_client_socket_impl.cc b/net/socket/ssl_client_socket_impl.cc
index daac901..2dccb12 100644
--- a/net/socket/ssl_client_socket_impl.cc
+++ b/net/socket/ssl_client_socket_impl.cc
@@ -1260,8 +1260,7 @@
// If no other errors occurred, check whether the connection used a legacy TLS
// version.
if (result == OK &&
- SSL_version(ssl_.get()) < context_->config().version_min_warn &&
- base::FeatureList::IsEnabled(features::kLegacyTLSEnforced)) {
+ SSL_version(ssl_.get()) < context_->config().version_min_warn) {
server_cert_verify_result_.cert_status |= CERT_STATUS_LEGACY_TLS;

// Only set the resulting net error if it hasn't been previously bypassed.
diff --git a/net/socket/ssl_client_socket_unittest.cc b/net/socket/ssl_client_socket_unittest.cc
index 9197611..0921d61 100644
--- a/net/socket/ssl_client_socket_unittest.cc
+++ b/net/socket/ssl_client_socket_unittest.cc
@@ -5882,19 +5882,9 @@
}
}

-class LegacyTLSDeprecationTest : public SSLClientSocketTest {
- public:
- LegacyTLSDeprecationTest() {
- feature_list_.InitAndEnableFeature(features::kLegacyTLSEnforced);
- }
-
- private:
- base::test::ScopedFeatureList feature_list_;
-};
-
// Set version_min_warn to TLS 1.2 and check that TLS 1.0 and 1.1 fail (with the
// expected error and cert status) but TLS 1.2 and 1.3 pass.
-TEST_F(LegacyTLSDeprecationTest, SetVersionMinWarnToTLS12) {
+TEST_F(SSLClientSocketTest, SetVersionMinWarnToTLS12) {
const struct TestCase {
uint16_t ssl_version;
int expected_net_error;
@@ -5943,7 +5933,7 @@

// Check that TLS 1.0 and TLS 1.1 failure is bypassed when you add
// allowed_bad_certs (with the expected error and cert status).
-TEST_F(LegacyTLSDeprecationTest, NoErrorWhenAddedToAllowedBadCerts) {
+TEST_F(SSLClientSocketTest, NoErrorWhenAddedToAllowedBadCerts) {
SSLServerConfig server_config;
server_config.version_min = SSL_PROTOCOL_VERSION_TLS1;
server_config.version_max = SSL_PROTOCOL_VERSION_TLS1;
@@ -5966,7 +5956,7 @@

// Check that if the we have bypassed a certificate error previously and then
// the server responded with TLS 1.0, we fill in both cert status flags.
-TEST_F(LegacyTLSDeprecationTest, BypassedCertShouldSetLegacyTLSStatus) {
+TEST_F(SSLClientSocketTest, BypassedCertShouldSetLegacyTLSStatus) {
SSLServerConfig server_config;
server_config.version_min = SSL_PROTOCOL_VERSION_TLS1;
server_config.version_max = SSL_PROTOCOL_VERSION_TLS1;
@@ -5991,7 +5981,7 @@
}

// Checks that other errors are prioritized over legacy TLS errors.
-TEST_F(LegacyTLSDeprecationTest, PrioritizeCertErrorsOverLegacyTLS) {
+TEST_F(SSLClientSocketTest, PrioritizeCertErrorsOverLegacyTLS) {
SSLServerConfig server_config;
server_config.version_min = SSL_PROTOCOL_VERSION_TLS1;
server_config.version_max = SSL_PROTOCOL_VERSION_TLS1;
@@ -6013,7 +6003,7 @@
}

// Checks that legacy TLS errors are not fatal.
-TEST_F(LegacyTLSDeprecationTest, LegacyTLSErrorsNotFatal) {
+TEST_F(SSLClientSocketTest, LegacyTLSErrorsNotFatal) {
SSLServerConfig server_config;
server_config.version_min = SSL_PROTOCOL_VERSION_TLS1;
server_config.version_max = SSL_PROTOCOL_VERSION_TLS1;