diff --git a/chrome/browser/extensions/api/web_navigation/web_navigation_apitest.cc b/chrome/browser/extensions/api/web_navigation/web_navigation_apitest.cc index 3af5f5fda6289..6510db2e14f4f 100644 --- a/chrome/browser/extensions/api/web_navigation/web_navigation_apitest.cc +++ b/chrome/browser/extensions/api/web_navigation/web_navigation_apitest.cc @@ -26,7 +26,6 @@ #include "chrome/browser/extensions/extension_apitest.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/renderer_context_menu/render_view_context_menu_test_util.h" -#include "chrome/browser/ssl/https_upgrades_util.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_navigator_params.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" @@ -282,6 +281,9 @@ class WebNavigationApiPrerenderTestWithContextType const WebNavigationApiPrerenderTestWithContextType&) = delete; WebNavigationApiPrerenderTestWithContextType& operator=( const WebNavigationApiPrerenderTestWithContextType&) = delete; + + private: + content::test::ScopedPrerenderFeatureList prerender_feature_list_; }; IN_PROC_BROWSER_TEST_P(WebNavigationApiTestWithContextType, Api) { @@ -301,10 +303,6 @@ IN_PROC_BROWSER_TEST_P(WebNavigationApiPrerenderTestWithContextType, GetFrame) { IN_PROC_BROWSER_TEST_P(WebNavigationApiPrerenderTestWithContextType, Prerendering) { - // TODO(crbug.com/1394910): Use https in the test and remove this allowlist - // entry. - AllowHttpForHostnamesForTesting({"a.test"}, browser()->profile()->GetPrefs()); - ASSERT_TRUE(StartEmbeddedTestServer()); ASSERT_TRUE(RunExtensionTest("webnavigation/prerendering")) << message_; } @@ -371,11 +369,6 @@ IN_PROC_BROWSER_TEST_P(WebNavigationApiTestWithContextType, MAYBE_Download) { IN_PROC_BROWSER_TEST_P(WebNavigationApiTestWithContextType, ServerRedirectSingleProcess) { - // TODO(crbug.com/1394910): Use https in the test and remove these allowlist - // entries. - AllowHttpForHostnamesForTesting({"www.a.com", "www.b.com"}, - browser()->profile()->GetPrefs()); - ASSERT_TRUE(StartEmbeddedTestServer()); // Set max renderers to 1 to force running out of processes. @@ -681,11 +674,6 @@ IN_PROC_BROWSER_TEST_P(WebNavigationApiTestWithContextType, PendingDeletion) { } IN_PROC_BROWSER_TEST_P(WebNavigationApiTestWithContextType, Crash) { - // TODO(crbug.com/1394910): Use https in the test and remove this allowlist - // entry. - AllowHttpForHostnamesForTesting({"www.a.com"}, - browser()->profile()->GetPrefs()); - content::ScopedAllowRendererCrashes scoped_allow_renderer_crashes; ASSERT_TRUE(StartEmbeddedTestServer()); diff --git a/chrome/browser/extensions/content_script_apitest.cc b/chrome/browser/extensions/content_script_apitest.cc index b88024d59f6c1..d259e6976481e 100644 --- a/chrome/browser/extensions/content_script_apitest.cc +++ b/chrome/browser/extensions/content_script_apitest.cc @@ -23,7 +23,6 @@ #include "chrome/browser/extensions/identifiability_metrics_test_util.h" #include "chrome/browser/search/search.h" #include "chrome/browser/ssl/https_upgrades_interceptor.h" -#include "chrome/browser/ssl/https_upgrades_util.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_navigator.h" #include "chrome/browser/ui/search/ntp_test_utils.h" @@ -146,15 +145,6 @@ class ContentScriptApiTest : public ExtensionApiTest { https_test_server_->port()); HttpsUpgradesInterceptor::SetHttpPortForTesting( embedded_test_server()->port()); - - // Test extensions use these hostnames. Allow them to be loaded over - // HTTP so that HTTPS-Upgrades feature doesn't upgrade their URLs. - // TODO(crbug.com/1394910): Use https in these tests and remove these - // allowlist entries. - AllowHttpForHostnamesForTesting( - {"a.com", "b.com", "default.test", "bar.com", "path-test.example", - "example.com", "chromium.org", "example1.com"}, - browser()->profile()->GetPrefs()); } void SetUpCommandLine(base::CommandLine* command_line) override { @@ -1004,8 +994,13 @@ IN_PROC_BROWSER_TEST_F(ContentScriptApiTest, CannotScriptTheNewTabPage) { // The extension should inject on "normal" urls. - // Test on an HTTP URL. HTTPS upgrades is disabled on example1.com so it - // loads over http instead of https. example2.com loads over https. + // Test on an HTTP URL. Disable HTTPS upgrades on example1.com for this test + // to work. + auto* prefs = browser()->profile()->GetPrefs(); + base::Value::List allowlist; + allowlist.Append("example1.com"); + prefs->SetList(prefs::kHttpAllowlist, std::move(allowlist)); + GURL unprotected_url1 = embedded_test_server()->GetURL( "example1.com", "/extensions/test_file.html"); ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), unprotected_url1)); diff --git a/chrome/browser/ssl/https_only_mode_browsertest.cc b/chrome/browser/ssl/https_only_mode_browsertest.cc index d2051a1979b9e..3aabdc634b8c6 100644 --- a/chrome/browser/ssl/https_only_mode_browsertest.cc +++ b/chrome/browser/ssl/https_only_mode_browsertest.cc @@ -56,8 +56,7 @@ class HttpsOnlyModeBrowserTest : public InProcessBrowserTest { void SetUp() override { feature_list_.InitWithFeatures( /*enabled_features=*/{features::kHttpsOnlyMode}, - /*disabled_features=*/{features::kHttpsFirstModeV2, - features::kHttpsUpgrades}); + /*disabled_features=*/{features::kHttpsFirstModeV2}); InProcessBrowserTest::SetUp(); } diff --git a/chrome/browser/ssl/https_only_mode_upgrade_interceptor.cc b/chrome/browser/ssl/https_only_mode_upgrade_interceptor.cc index b9db43a0157f4..12be8068983ad 100644 --- a/chrome/browser/ssl/https_only_mode_upgrade_interceptor.cc +++ b/chrome/browser/ssl/https_only_mode_upgrade_interceptor.cc @@ -111,8 +111,9 @@ void HttpsOnlyModeUpgradeInterceptor::MaybeCreateLoader( } // Check if the hostname is in the enterprise policy HTTP allowlist. - if (IsHostnameInHttpAllowlist(tentative_resource_request.url, - profile->GetPrefs())) { + PrefService* prefs = profile->GetPrefs(); + if (IsHostnameInAllowlist(tentative_resource_request.url, + prefs->GetList(prefs::kHttpAllowlist))) { std::move(callback).Run({}); return; } diff --git a/chrome/browser/ssl/https_upgrades_browsertest.cc b/chrome/browser/ssl/https_upgrades_browsertest.cc index 47f16b5dae6c3..2385d79dbd115 100644 --- a/chrome/browser/ssl/https_upgrades_browsertest.cc +++ b/chrome/browser/ssl/https_upgrades_browsertest.cc @@ -77,10 +77,6 @@ class HttpsUpgradesBrowserTest /*enabled_features=*/{features::kHttpsFirstModeV2, features::kHttpsUpgrades}, /*disabled_features=*/{}); - } else if (GetParam() == HttpsUpgradesTestType::kNeither) { - feature_list_.InitWithFeatures( - /*enabled_features=*/{features::kHttpsFirstModeV2}, - /*disabled_features=*/{features::kHttpsUpgrades}); } else { feature_list_.InitAndEnableFeature(features::kHttpsFirstModeV2); } diff --git a/chrome/browser/ssl/https_upgrades_interceptor.cc b/chrome/browser/ssl/https_upgrades_interceptor.cc index 3ee93ab64dc05..0d71b4eb97afd 100644 --- a/chrome/browser/ssl/https_upgrades_interceptor.cc +++ b/chrome/browser/ssl/https_upgrades_interceptor.cc @@ -266,8 +266,8 @@ void HttpsUpgradesInterceptor::MaybeCreateLoaderOnHstsQueryCompleted( // Don't upgrade navigation if it is allowlisted. // First, check the enterprise policy HTTP allowlist. PrefService* prefs = profile->GetPrefs(); - if (IsHostnameInHttpAllowlist(tentative_resource_request.url, - profile->GetPrefs())) { + if (IsHostnameInAllowlist(tentative_resource_request.url, + prefs->GetList(prefs::kHttpAllowlist))) { RecordNavigationRequestSecurityLevel( NavigationRequestSecurityLevel::kInsecure); std::move(callback).Run({}); diff --git a/chrome/browser/ssl/https_upgrades_util.cc b/chrome/browser/ssl/https_upgrades_util.cc index e47faa4960d7b..b20c4eb907141 100644 --- a/chrome/browser/ssl/https_upgrades_util.cc +++ b/chrome/browser/ssl/https_upgrades_util.cc @@ -5,15 +5,11 @@ #include "chrome/browser/ssl/https_upgrades_util.h" #include "base/values.h" -#include "chrome/common/pref_names.h" #include "components/content_settings/core/common/content_settings_pattern.h" -#include "components/prefs/pref_service.h" #include "url/gurl.h" -bool IsHostnameInHttpAllowlist(const GURL& url, PrefService* prefs) { - const base::Value::List& allowed_hosts = - prefs->GetList(prefs::kHttpAllowlist); - +bool IsHostnameInAllowlist(const GURL& url, + const base::Value::List& allowed_hosts) { // Though this is not technically a Content Setting, ContentSettingsPattern // aligns better than URLMatcher with the rules from // https://chromeenterprise.google/policies/url-patterns/. @@ -32,12 +28,3 @@ bool IsHostnameInHttpAllowlist(const GURL& url, PrefService* prefs) { } return false; } - -void AllowHttpForHostnamesForTesting(const std::vector& hostnames, - PrefService* prefs) { - base::Value::List allowed_hosts; - for (const std::string& hostname : hostnames) { - allowed_hosts.Append(hostname); - } - prefs->SetList(prefs::kHttpAllowlist, std::move(allowed_hosts)); -} diff --git a/chrome/browser/ssl/https_upgrades_util.h b/chrome/browser/ssl/https_upgrades_util.h index 0e1bd1ca6af6e..06db64c7c908f 100644 --- a/chrome/browser/ssl/https_upgrades_util.h +++ b/chrome/browser/ssl/https_upgrades_util.h @@ -8,8 +8,6 @@ #include "base/values.h" #include "url/gurl.h" -class PrefService; - // Helper for applying the HttpAllowlist enterprise policy. Checks if the // hostname of `url` matches any of the hostnames or hostname patterns in the // `allowed_hosts` list. Does not allow blanket host wildcards (i.e., "*" which @@ -17,10 +15,7 @@ class PrefService; // "[*.]example.com"). Entries in `allowed_hosts` should follow the rules in // https://chromeenterprise.google/policies/url-patterns/ (or they'll be // ignored). -bool IsHostnameInHttpAllowlist(const GURL& url, PrefService* prefs); - -// Adds `hostnames` to the HttpAllowlist enterprise policy for testing. -void AllowHttpForHostnamesForTesting(const std::vector& hostnames, - PrefService* prefs); +bool IsHostnameInAllowlist(const GURL& url, + const base::Value::List& allowed_hosts); #endif // CHROME_BROWSER_SSL_HTTPS_UPGRADES_UTIL_H_ diff --git a/chrome/browser/ssl/security_state_tab_helper_browsertest.cc b/chrome/browser/ssl/security_state_tab_helper_browsertest.cc index bdb063bdd9bd1..3db0dd370c127 100644 --- a/chrome/browser/ssl/security_state_tab_helper_browsertest.cc +++ b/chrome/browser/ssl/security_state_tab_helper_browsertest.cc @@ -1363,13 +1363,6 @@ IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperIncognitoTest, HttpErrorPage) { SecurityStateTabHelper::FromWebContents(contents); ASSERT_TRUE(helper); - // Disable HTTPS upgrades on nonexistent.test for this test to work. - auto* profile = Profile::FromBrowserContext(contents->GetBrowserContext()); - auto* prefs = profile->GetPrefs(); - base::Value::List allowlist; - allowlist.Append("nonexistent.test"); - prefs->SetList(prefs::kHttpAllowlist, std::move(allowlist)); - // Navigate to a URL that results in an error page. Even though the displayed // URL is http://, there shouldn't be a Not Secure warning because the browser // hasn't really navigated to an http:// page. diff --git a/chrome/browser/ssl/ssl_browsertest.cc b/chrome/browser/ssl/ssl_browsertest.cc index d0eea1eb072d8..81a37efe93c6b 100644 --- a/chrome/browser/ssl/ssl_browsertest.cc +++ b/chrome/browser/ssl/ssl_browsertest.cc @@ -109,7 +109,6 @@ #include "components/security_interstitials/content/ssl_error_handler.h" #include "components/security_interstitials/content/stateful_ssl_host_state_delegate.h" #include "components/security_interstitials/core/controller_client.h" -#include "components/security_interstitials/core/https_only_mode_metrics.h" #include "components/security_interstitials/core/metrics_helper.h" #include "components/security_interstitials/core/pref_names.h" #include "components/security_state/core/security_state.h" @@ -6458,12 +6457,10 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, ASSERT_TRUE(ui_test_utils::NavigateToURL( browser(), https_server_.GetURL(replacement_path))); - WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents(); content::TestNavigationObserver nav_observer(tab, 1); ASSERT_TRUE(content::ExecuteScript(tab, "submitForm();")); nav_observer.Wait(); - security_interstitials::SecurityInterstitialTabHelper* helper = security_interstitials::SecurityInterstitialTabHelper::FromWebContents( tab); @@ -6472,27 +6469,12 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, // Check this was logged correctly as a redirect mixed form that would not // expose form data. - // Since Https Upgrades are enabled, metrics should be recorded twice, first - // for the original HTTP navigation and then for the fallback navigation. - histograms.ExpectTotalCount(interstitial_histogram, 2); + histograms.ExpectTotalCount(interstitial_histogram, 1); histograms.ExpectBucketCount( interstitial_histogram, InsecureFormNavigationThrottle::InterstitialTriggeredState:: kMixedFormRedirectNoFormData, - 2); - - // Also check HTTPS upgrade metrics. - histograms.ExpectTotalCount( - security_interstitials::https_only_mode::kEventHistogram, 3); - histograms.ExpectBucketCount( - security_interstitials::https_only_mode::kEventHistogram, - security_interstitials::https_only_mode::Event::kUpgradeAttempted, 1); - histograms.ExpectBucketCount( - security_interstitials::https_only_mode::kEventHistogram, - security_interstitials::https_only_mode::Event::kUpgradeFailed, 1); - histograms.ExpectBucketCount( - security_interstitials::https_only_mode::kEventHistogram, - security_interstitials::https_only_mode::Event::kUpgradeNetError, 1); + 1); } // Checks no interstitial is shown for mixed forms caused for a POST form with a @@ -6523,27 +6505,12 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, // Check this was logged correctly as a redirect mixed form that would not // expose form data. - // Since Https Upgrades are enabled, metrics should be recorded twice, first - // for the original HTTP navigation and then for the fallback navigation. - histograms.ExpectTotalCount(interstitial_histogram, 2); + histograms.ExpectTotalCount(interstitial_histogram, 1); histograms.ExpectBucketCount( interstitial_histogram, InsecureFormNavigationThrottle::InterstitialTriggeredState:: kMixedFormRedirectNoFormData, - 2); - - // Also check HTTPS upgrade metrics. - histograms.ExpectTotalCount( - security_interstitials::https_only_mode::kEventHistogram, 3); - histograms.ExpectBucketCount( - security_interstitials::https_only_mode::kEventHistogram, - security_interstitials::https_only_mode::Event::kUpgradeAttempted, 1); - histograms.ExpectBucketCount( - security_interstitials::https_only_mode::kEventHistogram, - security_interstitials::https_only_mode::Event::kUpgradeFailed, 1); - histograms.ExpectBucketCount( - security_interstitials::https_only_mode::kEventHistogram, - security_interstitials::https_only_mode::Event::kUpgradeNetError, 1); + 1); } namespace { @@ -6602,27 +6569,12 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, // Check this was logged correctly as a redirect mixed form that would not // expose form data. - // Since Https Upgrades are enabled, metrics should be recorded twice, first - // for the original HTTP navigation and then for the fallback navigation. - histograms.ExpectTotalCount(interstitial_histogram, 2); + histograms.ExpectTotalCount(interstitial_histogram, 1); histograms.ExpectBucketCount( interstitial_histogram, InsecureFormNavigationThrottle::InterstitialTriggeredState:: kMixedFormRedirectNoFormData, - 2); - - // Also check HTTPS upgrade metrics. - histograms.ExpectTotalCount( - security_interstitials::https_only_mode::kEventHistogram, 3); - histograms.ExpectBucketCount( - security_interstitials::https_only_mode::kEventHistogram, - security_interstitials::https_only_mode::Event::kUpgradeAttempted, 1); - histograms.ExpectBucketCount( - security_interstitials::https_only_mode::kEventHistogram, - security_interstitials::https_only_mode::Event::kUpgradeFailed, 1); - histograms.ExpectBucketCount( - security_interstitials::https_only_mode::kEventHistogram, - security_interstitials::https_only_mode::Event::kUpgradeCertError, 1); + 1); } class MixedFormsPolicyTest : public policy::PolicyTest {}; diff --git a/testing/variations/fieldtrial_testing_config.json b/testing/variations/fieldtrial_testing_config.json index 5cc4a6528c32a..616213ba3a36e 100644 --- a/testing/variations/fieldtrial_testing_config.json +++ b/testing/variations/fieldtrial_testing_config.json @@ -6049,26 +6049,6 @@ ] } ], - "HttpsUpgrades": [ - { - "platforms": [ - "android", - "windows", - "chromeos", - "chromeos_lacros", - "mac", - "linux" - ], - "experiments": [ - { - "name": "Enabled", - "enable_features": [ - "HttpsUpgrades" - ] - } - ] - } - ], "IOSAddToHomeScreen": [ { "platforms": [