From e1ca690f3d6274dfc18b7480524a2792e6628c52 Mon Sep 17 00:00:00 2001 From: Arthur Edelstein Date: Thu, 14 Nov 2024 21:40:07 -0800 Subject: [PATCH] Fix HttpsFirstModeIncognito so that it doesn't cause infinite redirects (#26574) Fix HttpsFirstModeIncognito so that it doesn't cause infinite redirects (#26542) --- .../ssl/https_upgrades_navigation_throttle.cc | 89 ++++--------------- .../ssl/https_upgrades_navigation_throttle.h | 18 ---- test/filters/browser_tests.filter | 22 ----- 3 files changed, 15 insertions(+), 114 deletions(-) delete mode 100644 chromium_src/chrome/browser/ssl/https_upgrades_navigation_throttle.h diff --git a/chromium_src/chrome/browser/ssl/https_upgrades_navigation_throttle.cc b/chromium_src/chrome/browser/ssl/https_upgrades_navigation_throttle.cc index 23684e7892b6..1e83ecf96a08 100644 --- a/chromium_src/chrome/browser/ssl/https_upgrades_navigation_throttle.cc +++ b/chromium_src/chrome/browser/ssl/https_upgrades_navigation_throttle.cc @@ -6,18 +6,12 @@ #include "chrome/browser/ssl/https_upgrades_navigation_throttle.h" #include "base/time/time.h" -#include "brave/browser/brave_browser_process.h" #include "brave/components/brave_shields/content/browser/brave_shields_util.h" #include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/profiles/profile.h" -#include "chrome/browser/ssl/https_first_mode_settings_tracker.h" -#include "chrome/browser/ssl/https_only_mode_controller_client.h" #include "components/prefs/pref_service.h" -#include "components/security_interstitials/content/stateful_ssl_host_state_delegate.h" #include "content/public/browser/navigation_handle.h" -#include "content/public/browser/page_navigator.h" #include "content/public/browser/web_contents.h" -#include "net/base/features.h" namespace { @@ -30,81 +24,28 @@ bool IsTor(content::NavigationHandle* handle) { return profile->IsTor(); } +bool NormalWindowHttpsOnly(content::NavigationHandle* handle, + Profile* profile) { + if (profile->IsIncognitoProfile()) { + return false; + } + const GURL& request_url = handle->GetURL(); + HostContentSettingsMap* map = + HostContentSettingsMapFactory::GetForProfile(profile); + return brave_shields::ShouldForceHttps(map, request_url); +} + } // namespace -#define MaybeCreateThrottleFor MaybeCreateThrottleFor_ChromiumImpl #define SetNavigationTimeout(DEFAULT_TIMEOUT) \ SetNavigationTimeout(IsTor(navigation_handle()) ? kTorFallbackDelay \ : DEFAULT_TIMEOUT) +#define GetBoolean(ORIGINAL_PREF) \ + GetBooleanOr(ORIGINAL_PREF, NormalWindowHttpsOnly(handle, profile)) + #include "src/chrome/browser/ssl/https_upgrades_navigation_throttle.cc" -#undef MaybeCreateThrottleFor +#undef GetBoolean #undef SetNavigationTimeout -// static -std::unique_ptr -HttpsUpgradesNavigationThrottle::MaybeCreateThrottleFor( - content::NavigationHandle* handle, - std::unique_ptr blocking_page_factory, - Profile* profile) { - DCHECK_CURRENTLY_ON(content::BrowserThread::UI); - - // HTTPS-First Mode is only relevant for primary main-frame HTTP(S) - // navigations. - if (!handle->GetURL().SchemeIsHTTPOrHTTPS() || - !handle->IsInPrimaryMainFrame() || handle->IsSameDocument()) { - return nullptr; - } - - const GURL& request_url = handle->GetURL(); - content::BrowserContext* context = - handle->GetWebContents()->GetBrowserContext(); - HostContentSettingsMap* map = - HostContentSettingsMapFactory::GetForProfile(context); - - PrefService* prefs = profile->GetPrefs(); - security_interstitials::https_only_mode::HttpInterstitialState - interstitial_state; - interstitial_state.enabled_by_pref = - (prefs && prefs->GetBoolean(prefs::kHttpsOnlyModeEnabled)) || - (map && brave_shields::ShouldForceHttps(map, request_url)); - - StatefulSSLHostStateDelegate* state = - static_cast( - profile->GetSSLHostStateDelegate()); - auto* storage_partition = - handle->GetWebContents()->GetPrimaryMainFrame()->GetStoragePartition(); - - HttpsFirstModeService* hfm_service = - HttpsFirstModeServiceFactory::GetForProfile(profile); - if (hfm_service) { - // Can be null in some cases, e.g. when using Ash sign-in profile. - interstitial_state.enabled_by_typically_secure_browsing = - hfm_service->IsInterstitialEnabledByTypicallySecureUserHeuristic(); - } - // StatefulSSLHostStateDelegate can be null during tests. - if (state && - state->IsHttpsEnforcedForUrl(handle->GetURL(), storage_partition)) { - interstitial_state.enabled_by_engagement_heuristic = true; - } - - bool https_upgrades_enabled = - interstitial_state.enabled_by_pref || - (map && brave_shields::ShouldUpgradeToHttps( - map, request_url, - g_brave_browser_process->https_upgrade_exceptions_service())); - if (!https_upgrades_enabled) { - return nullptr; - } - - // Ensure that the HttpsOnlyModeTabHelper has been created (this does nothing - // if it has already been created for the WebContents). There are cases where - // the tab helper won't get created by the initialization in - // chrome/browser/ui/tab_helpers.cc but the criteria for adding the throttle - // are still met (see crbug.com/1233889 for one example). - HttpsOnlyModeTabHelper::CreateForWebContents(handle->GetWebContents()); - - return std::make_unique( - handle, profile, std::move(blocking_page_factory), interstitial_state); -} diff --git a/chromium_src/chrome/browser/ssl/https_upgrades_navigation_throttle.h b/chromium_src/chrome/browser/ssl/https_upgrades_navigation_throttle.h deleted file mode 100644 index 79c9777e22bb..000000000000 --- a/chromium_src/chrome/browser/ssl/https_upgrades_navigation_throttle.h +++ /dev/null @@ -1,18 +0,0 @@ -/* Copyright (c) 2023 The Brave Authors. All rights reserved. - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at https://mozilla.org/MPL/2.0/. */ - -#ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_SSL_HTTPS_UPGRADES_NAVIGATION_THROTTLE_H_ -#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_SSL_HTTPS_UPGRADES_NAVIGATION_THROTTLE_H_ - -#define MaybeCreateThrottleFor(...) \ - MaybeCreateThrottleFor_ChromiumImpl(__VA_ARGS__); \ - static std::unique_ptr \ - MaybeCreateThrottleFor(__VA_ARGS__) - -#include "src/chrome/browser/ssl/https_upgrades_navigation_throttle.h" // IWYU pragma: export - -#undef MaybeCreateThrottleFor - -#endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_SSL_HTTPS_UPGRADES_NAVIGATION_THROTTLE_H_ diff --git a/test/filters/browser_tests.filter b/test/filters/browser_tests.filter index d61b0ee7dc98..be95c3af46e3 100644 --- a/test/filters/browser_tests.filter +++ b/test/filters/browser_tests.filter @@ -1298,45 +1298,25 @@ # These tests fail because we are modifying the HttpsUpgrades feature. -HttpsUpgradesBrowserTest.AllowlistEntryExpires/* --HttpsUpgradesBrowserTest.BadHttpsFollowedByGoodHttps/HttpsFirstBalancedMode --HttpsUpgradesBrowserTest.BadHttpsFollowedByGoodHttps/HttpsFirstModeIncognito --HttpsUpgradesBrowserTest.BrokenSSLOnUpgrade_SecurityLevelWarning/HttpsFirstBalancedMode --HttpsUpgradesBrowserTest.BrokenSSLOnUpgrade_SecurityLevelWarning/HttpsFirstModeIncognito -HttpsUpgradesBrowserTest.CancelTimeoutForFallbackNavigations/* --HttpsUpgradesBrowserTest.CloseInterstitialTab/HttpsFirstBalancedMode --HttpsUpgradesBrowserTest.CloseInterstitialTab/HttpsFirstModeIncognito -HttpsUpgradesBrowserTest.EnterpriseAllowlistDisablesHttpsFirstModeForSiteEngagament/AllAutoHFM -HttpsUpgradesBrowserTest.EnterpriseAllowlistDisablesHttpsFirstModeForSiteEngagament/HttpsFirstModeWithSiteEngagement -HttpsUpgradesBrowserTest.EnterpriseAllowlistDisablesUpgrades/* --HttpsUpgradesBrowserTest.ExemptNetErrorOnUpgrade_ShouldNotFallback/HttpsFirstBalancedMode --HttpsUpgradesBrowserTest.ExemptNetErrorOnUpgrade_ShouldNotFallback/HttpsFirstModeIncognito --HttpsUpgradesBrowserTest.ExemptNetErrorOnUpgrade_UniqueSingleLabelHostname_ShouldFallback/HttpsFirstModeIncognito -HttpsUpgradesBrowserTest.HttpPageHttpPost_NotUpgraded/* -HttpsUpgradesBrowserTest.HttpParentHttpSubframeNavigation_NotUpgraded/* -HttpsUpgradesBrowserTest.HttpsToHttpRedirect_ShouldUpgrade/* -HttpsUpgradesBrowserTest.HttpsUpgradeWithBrokenSSL_ShouldTriggerSSLInterstitial/* --HttpsUpgradesBrowserTest.IncognitoHasSeparateAllowlist/HttpsFirstModeIncognito -HttpsUpgradesBrowserTest.IncognitoInterstitialVariation/HttpsFirstModeIncognito -HttpsUpgradesBrowserTest.InterstitialBypassed_HttpFallbackLoaded/* --HttpsUpgradesBrowserTest.InterstitialFallbackMaintainsHistory/HttpsFirstBalancedMode --HttpsUpgradesBrowserTest.InterstitialFallbackMaintainsHistory/HttpsFirstModeIncognito --HttpsUpgradesBrowserTest.InterstitialGoBack/HttpsFirstBalancedMode --HttpsUpgradesBrowserTest.InterstitialGoBack/HttpsFirstModeIncognito -HttpsUpgradesBrowserTest.InterstitialLearnMoreLink/HttpsFirstBalancedMode -HttpsUpgradesBrowserTest.InterstitialLearnMoreLink/HttpsFirstModeIncognito --HttpsUpgradesBrowserTest.Localhost_ShouldNotUpgrade/HttpsFirstModeIncognito --HttpsUpgradesBrowserTest.NetErrorOnUpgrade_SecurityLevelWarning/HttpsFirstBalancedMode --HttpsUpgradesBrowserTest.NetErrorOnUpgrade_SecurityLevelWarning/HttpsFirstModeIncognito -HttpsUpgradesBrowserTest.NetErrorOnUpgrade_ShouldInterstitial/* -HttpsUpgradesBrowserTest.NonDefaultPorts_NoWarnInBalancedMode/* --HttpsUpgradesBrowserTest.NonUniqueHost_RecordsMetrics/HttpsFirstModeIncognito --HttpsUpgradesBrowserTest.PreferHstsOverHttpsFirstMode/HttpsFirstModeIncognito -HttpsUpgradesBrowserTest.RedirectLoop_ShouldInterstitial/* -HttpsUpgradesBrowserTest.RedirectLoopWithSlowRedirect_ShouldInterstitial/* -HttpsUpgradesBrowserTest.RevisitingBumpsExpiration/* -HttpsUpgradesBrowserTest.SlowHttps_ShouldInterstitial/* -HttpsUpgradesBrowserTest.TogglingSettingClearsAllowlist/AllFeatures --HttpsUpgradesBrowserTest.TogglingSettingClearsAllowlist/HttpsFirstBalancedMode -HttpsUpgradesBrowserTest.UniqueSingleLabel_NoWarnInBalancedMode/* -HttpsUpgradesBrowserTest.URLsAutocompletedWithHttpSchemeAreUpgraded/* -HttpsUpgradesBrowserTest.URLsTypedWithHttpSchemeNoUpgrades/* @@ -1345,9 +1325,7 @@ -HttpsUpgradesBrowserTest.UrlWithHttpScheme_BrokenSSL_ShouldInterstitial_TypicallySecureUser/* -HttpsUpgradesBrowserTest.UrlWithHttpScheme_BrokenSSL_SiteEngagementHeuristic_ShouldIgnoreUrlsWithNonDefaultPorts/* -HttpsUpgradesBrowserTest.UrlWithHttpScheme_BrokenSSL_SiteEngagementHeuristic_ShouldInterstitial/* --HttpsUpgradesBrowserTest.UrlWithHttpScheme_NonUniqueHostname_ShouldNotInterstitial_TypicallySecureUser/HttpsFirstBalancedMode -HttpsUpgradesBrowserTest.UrlWithHttpScheme_ShouldUpgrade/* --HttpsUpgradesBrowserTest.UrlWithHttpsScheme_ShouldLoad/HttpsFirstModeIncognito -HttpsUpgradesBrowserTest.crbug1431026/* -HttpsUpgradesHeuristicsWithoutBalancedModeBrowserTest.UrlWithHttpScheme_BrokenSSL_SiteEngagementHeuristicWithoutBalancedMode_ShouldIgnore/0 -HttpsUpgradesPrefsBrowserTest.PrefStatesRecorded