From 33c3af7947b0142b3cc2e581ffe361d1a4454962 Mon Sep 17 00:00:00 2001 From: Arthur Edelstein Date: Thu, 2 Mar 2023 02:45:57 -0500 Subject: [PATCH] Prevent permissive HTTPS Upgrade settings from leaking from normal to incognito windows (#17421) Prevent permissive HTTPS Upgrade settings from leaking from normal to private windows --- .../https_upgrade_browsertest.cc | 70 +++++++++++++++++++ .../brave_content_settings_browsertest.cc | 1 + .../core/browser/content_settings_registry.cc | 2 +- .../core/browser/host_content_settings_map.cc | 2 + 4 files changed, 74 insertions(+), 1 deletion(-) diff --git a/browser/brave_shields/https_upgrade_browsertest.cc b/browser/brave_shields/https_upgrade_browsertest.cc index c2e276230616..a38add5e0565 100644 --- a/browser/brave_shields/https_upgrade_browsertest.cc +++ b/browser/brave_shields/https_upgrade_browsertest.cc @@ -229,3 +229,73 @@ IN_PROC_BROWSER_TEST_F(HttpsUpgradeBrowserTest_FlagDisabled, CheckUpgrades) { } } } + +IN_PROC_BROWSER_TEST_F(HttpsUpgradeBrowserTest, IsolateSettings) { + // Test host URLs. + GURL host1("https://example1.test"); + GURL host2("https://example2.test"); + + // Test profiles + Profile* normal_profile = chrome_test_utils::GetProfile(this); + Profile* incognito_profile = normal_profile->GetOffTheRecordProfile( + Profile::OTRProfileID::PrimaryID(), /*create_if_needed=*/true); + + auto* normal_map = + HostContentSettingsMapFactory::GetForProfile(normal_profile); + auto* incognito_map = + HostContentSettingsMapFactory::GetForProfile(incognito_profile); + + // Disable upgrades for a site. + brave_shields::SetHttpsUpgradeControlType(normal_map, ControlType::ALLOW, + host1); + brave_shields::SetHttpsUpgradeControlType(incognito_map, ControlType::ALLOW, + host2); + + // Disabled upgrade per-site in normal windows should not apply to incognito + // windows, nor vice versa. + EXPECT_EQ(ControlType::ALLOW, + brave_shields::GetHttpsUpgradeControlType(normal_map, host1)); + EXPECT_EQ(ControlType::BLOCK_THIRD_PARTY, + brave_shields::GetHttpsUpgradeControlType(incognito_map, host1)); + EXPECT_EQ(ControlType::BLOCK_THIRD_PARTY, + brave_shields::GetHttpsUpgradeControlType(normal_map, host2)); + EXPECT_EQ(ControlType::ALLOW, + brave_shields::GetHttpsUpgradeControlType(incognito_map, host2)); + + // Set strict per-site settings. + brave_shields::SetHttpsUpgradeControlType(normal_map, ControlType::BLOCK, + host1); + brave_shields::SetHttpsUpgradeControlType(incognito_map, ControlType::BLOCK, + host2); + + // A strict per-site setting in normal windows does apply to incognito + // windows, but not vice versa. + EXPECT_EQ(ControlType::BLOCK, + brave_shields::GetHttpsUpgradeControlType(normal_map, host1)); + EXPECT_EQ(ControlType::BLOCK, + brave_shields::GetHttpsUpgradeControlType(incognito_map, host1)); + EXPECT_EQ(ControlType::BLOCK, + brave_shields::GetHttpsUpgradeControlType(incognito_map, host2)); + EXPECT_EQ(ControlType::BLOCK_THIRD_PARTY, + brave_shields::GetHttpsUpgradeControlType(normal_map, host2)); + + // Set global setting to strict. + brave_shields::SetHttpsUpgradeControlType(normal_map, ControlType::BLOCK, + GURL()); + + // Strict global upgrades should apply to both normal and incognito windows. + EXPECT_EQ(ControlType::BLOCK, + brave_shields::GetHttpsUpgradeControlType(normal_map, GURL())); + EXPECT_EQ(ControlType::BLOCK, + brave_shields::GetHttpsUpgradeControlType(incognito_map, GURL())); + + // Set global setting to disabled. + brave_shields::SetHttpsUpgradeControlType(normal_map, ControlType::ALLOW, + GURL()); + + // Disabled global upgrades should apply to normal windows but not incognito. + EXPECT_EQ(ControlType::ALLOW, + brave_shields::GetHttpsUpgradeControlType(normal_map, GURL())); + EXPECT_EQ(ControlType::BLOCK_THIRD_PARTY, + brave_shields::GetHttpsUpgradeControlType(incognito_map, GURL())); +} diff --git a/browser/content_settings/brave_content_settings_browsertest.cc b/browser/content_settings/brave_content_settings_browsertest.cc index 91d912232469..b96ebc2dd9f8 100644 --- a/browser/content_settings/brave_content_settings_browsertest.cc +++ b/browser/content_settings/brave_content_settings_browsertest.cc @@ -51,6 +51,7 @@ class BraveContentSettingsBrowserTest : public InProcessBrowserTest { ContentSettingsType::NOTIFICATIONS, ContentSettingsType::PROTECTED_MEDIA_IDENTIFIER, ContentSettingsType::IDLE_DETECTION, + ContentSettingsType::BRAVE_HTTPS_UPGRADE, }; if (base::Contains(kOffTheRecordAwareTypes, content_type)) { return current_setting; diff --git a/chromium_src/components/content_settings/core/browser/content_settings_registry.cc b/chromium_src/components/content_settings/core/browser/content_settings_registry.cc index 642b6806cdf2..5625d4a6c76e 100644 --- a/chromium_src/components/content_settings/core/browser/content_settings_registry.cc +++ b/chromium_src/components/content_settings/core/browser/content_settings_registry.cc @@ -41,7 +41,7 @@ void ContentSettingsRegistry::BraveInit() { WebsiteSettingsInfo::TOP_ORIGIN_ONLY_SCOPE, WebsiteSettingsRegistry::DESKTOP | WebsiteSettingsRegistry::PLATFORM_ANDROID, - ContentSettingsInfo::INHERIT_IN_INCOGNITO, + ContentSettingsInfo::INHERIT_IF_LESS_PERMISSIVE, ContentSettingsInfo::EXCEPTIONS_ON_SECURE_AND_INSECURE_ORIGINS); Register(ContentSettingsType::BRAVE_HTTP_UPGRADABLE_RESOURCES, diff --git a/chromium_src/components/content_settings/core/browser/host_content_settings_map.cc b/chromium_src/components/content_settings/core/browser/host_content_settings_map.cc index dbd102ec9d44..52cbb6eb72fa 100644 --- a/chromium_src/components/content_settings/core/browser/host_content_settings_map.cc +++ b/chromium_src/components/content_settings/core/browser/host_content_settings_map.cc @@ -6,6 +6,7 @@ #include "base/containers/contains.h" #include "build/build_config.h" #include "components/content_settings/core/browser/content_settings_utils.h" +#include "components/content_settings/core/common/content_settings_types.h" #include "components/content_settings/core/common/features.h" #if !BUILDFLAG(IS_IOS) @@ -29,6 +30,7 @@ bool IsMorePermissive_BraveImpl(ContentSettingsType content_type, ContentSettingsType::NOTIFICATIONS, ContentSettingsType::PROTECTED_MEDIA_IDENTIFIER, ContentSettingsType::IDLE_DETECTION, + ContentSettingsType::BRAVE_HTTPS_UPGRADE, }; const bool is_more_permissive = IsMorePermissive(setting, initial_setting);