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
brave · Mar 2, 2023 · 33c3af7 · 33c3af7
1 parent ab9d8f1
commit 33c3af7
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 1 deletion.
diff --git 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
@@ -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
@@ -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
@@ -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);