Skip to content

Commit

Permalink
Fix HttpsFirstModeIncognito so that it doesn't cause infinite redirec…
Browse files Browse the repository at this point in the history
…ts (#26574)

Fix HttpsFirstModeIncognito so that it doesn't cause infinite redirects (#26542)
  • Loading branch information
arthuredelstein authored Nov 15, 2024
1 parent 1996348 commit e1ca690
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 114 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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>
HttpsUpgradesNavigationThrottle::MaybeCreateThrottleFor(
content::NavigationHandle* handle,
std::unique_ptr<SecurityBlockingPageFactory> 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<StatefulSSLHostStateDelegate*>(
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<HttpsUpgradesNavigationThrottle>(
handle, profile, std::move(blocking_page_factory), interstitial_state);
}

This file was deleted.

22 changes: 0 additions & 22 deletions test/filters/browser_tests.filter
Original file line number Diff line number Diff line change
Expand Up @@ -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/*
Expand All @@ -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
Expand Down

0 comments on commit e1ca690

Please sign in to comment.