Handle 3p frames properly when shields are disabled for 1p frame.

brave · Sep 7, 2021 · 2da1b2a · 2da1b2a
1 parent 34c1dfa
commit 2da1b2a
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 11 deletions.
diff --git a/chromium_src/components/content_settings/core/browser/cookie_settings.cc b/chromium_src/components/content_settings/core/browser/cookie_settings.cc
@@ -10,6 +10,8 @@
 
 #define BRAVE_COOKIE_SETTINGS_GET_COOKIES_SETTINGS_INTERNAL      \
   if (setting == CONTENT_SETTING_SESSION_ONLY && !block_third && \
+      (!info.primary_pattern.MatchesAllHosts() ||                \
+       !info.secondary_pattern.MatchesAllHosts()) &&             \
       ShouldBlockThirdPartyCookies() &&                          \
       !first_party_url.SchemeIs(extension_scheme_) &&            \
       base::FeatureList::IsEnabled(                              \
@@ -83,6 +85,28 @@ std::vector<url::Origin> CookieSettings::TakeEphemeralStorageOpaqueOrigins(
   return result;
 }
 
-}  // namespace content_settings
+ContentSetting CookieSettings::GetDetailedCookieSetting(
+    const GURL& url,
+    bool* is_shields_disable_rule) const {
+  if (ShouldAlwaysAllowCookies(url, url)) {
+    return CONTENT_SETTING_ALLOW;
+  }
 
-#undef BRAVE_COOKIE_SETTINGS_GET_COOKIES_SETTINGS_INTERNAL
+  SettingInfo info;
+  std::unique_ptr<base::Value> value =
+      host_content_settings_map_->GetWebsiteSetting(
+          url, url, ContentSettingsType::COOKIES, &info);
+  DCHECK(value);
+
+  ContentSetting cookie_setting = ValueToContentSetting(value.get());
+
+  if (is_shields_disable_rule) {
+    *is_shields_disable_rule = cookie_setting == CONTENT_SETTING_ALLOW &&
+                               info.primary_pattern.MatchesAllHosts() &&
+                               !info.secondary_pattern.MatchesAllHosts();
+  }
+
+  return cookie_setting;
+}
+
+}  // namespace content_settings
diff --git a/chromium_src/components/content_settings/core/browser/cookie_settings.h b/chromium_src/components/content_settings/core/browser/cookie_settings.h
@@ -10,6 +10,7 @@
 
 #include "base/containers/flat_map.h"
 #include "components/content_settings/core/browser/content_settings_provider.h"
+#include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/keyed_service/core/refcounted_keyed_service.h"
 #include "url/origin.h"
 
@@ -21,6 +22,8 @@
       url::Origin& storage_origin);                                          \
   std::vector<url::Origin> TakeEphemeralStorageOpaqueOrigins(                \
       const std::string& ephemeral_storage_domain);                          \
+  ContentSetting GetDetailedCookieSetting(                                   \
+      const GURL& url, bool* is_shields_disable_rule) const override;        \
                                                                              \
  private:                                                                    \
   /* Ephemeral storage domain to non_opaque->opaque origins map. */          \

diff --git a/chromium_src/components/content_settings/core/common/cookie_settings_base.cc b/chromium_src/components/content_settings/core/common/cookie_settings_base.cc
@@ -169,19 +169,32 @@ bool CookieSettingsBase::IsCookieAccessAllowedImpl(
 
   const GURL first_party_url = GetFirstPartyURL(
       site_for_cookies, base::OptionalOrNullptr(top_frame_origin));
-  const bool is_1p_ephemeral =
-      is_1p_ephemeral_feature_enabled && IsCookieSessionOnly(first_party_url);
 
-  if (is_1p_ephemeral && allow) {
-    return false;
+  bool are_first_party_shields_disabled = false;
+  if (allow && is_1p_ephemeral_feature_enabled) {
+    const bool is_1p_ephemeral =
+        GetDetailedCookieSetting(first_party_url,
+                                 &are_first_party_shields_disabled) ==
+        CONTENT_SETTING_SESSION_ONLY;
+    // Block all non ephemeral activities (service workers, etc.) if 1p is
+    // ephemeral.
+    return !is_1p_ephemeral;
   }
 
+  DCHECK(!allow);
+
   if (!IsFirstPartyAccessAllowed(first_party_url, this))
     return false;
 
   if (BraveIsAllowedThirdParty(url, first_party_url, this))
     return true;
 
+  if (is_1p_ephemeral_feature_enabled && are_first_party_shields_disabled &&
+      IsCookieSessionOnly(url)) {
+    // Allow 3p session-only frames as is when shields are disabled.
+    return true;
+  }
+
   return false;
 }
 

diff --git a/chromium_src/components/content_settings/core/common/cookie_settings_base.h b/chromium_src/components/content_settings/core/common/cookie_settings_base.h
@@ -31,6 +31,8 @@ using ScopedEphemeralStorageAwareness = base::AutoReset<bool>;
   bool IsChromiumFullCookieAccessAllowed(                                    \
       const GURL& url, const GURL& site_for_cookies,                         \
       const absl::optional<url::Origin>& top_frame_origin) const;            \
+  virtual ContentSetting GetDetailedCookieSetting(                           \
+      const GURL& url, bool* is_shields_disabled_rule) const = 0;            \
                                                                              \
  private:                                                                    \
   bool IsCookieAccessAllowedImpl(                                            \

diff --git a/chromium_src/services/network/cookie_settings.cc b/chromium_src/services/network/cookie_settings.cc
@@ -10,6 +10,7 @@
 
 #define BRAVE_COOKIE_SETTINGS_GET_COOKIE_SETTINGS_INTERNAL    \
   if (cookie_setting == CONTENT_SETTING_SESSION_ONLY &&       \
+      IsExplicitSetting(*entry) &&                            \
       base::FeatureList::IsEnabled(                           \
           net::features::kBraveFirstPartyEphemeralStorage)) { \
     /* Do nothing */                                          \
@@ -69,4 +70,31 @@ bool CookieSettings::AnnotateAndMoveUserBlockedEphemeralCookies(
       excluded_cookies);
 }
 
+ContentSetting CookieSettings::GetDetailedCookieSetting(
+    const GURL& url,
+    bool* is_shields_disable_rule) const {
+  if (ShouldAlwaysAllowCookies(url, url)) {
+    return CONTENT_SETTING_ALLOW;
+  }
+
+  // Default to allowing cookies.
+  ContentSetting cookie_setting = CONTENT_SETTING_ALLOW;
+  const auto& entry = base::ranges::find_if(
+      content_settings_, [&](const ContentSettingPatternSource& entry) {
+        return entry.primary_pattern.Matches(url) &&
+               entry.secondary_pattern.Matches(url);
+      });
+
+  if (entry != content_settings_.end()) {
+    cookie_setting = entry->GetContentSetting();
+    if (is_shields_disable_rule) {
+      *is_shields_disable_rule = cookie_setting == CONTENT_SETTING_ALLOW &&
+                                 entry->primary_pattern.MatchesAllHosts() &&
+                                 !entry->secondary_pattern.MatchesAllHosts();
+    }
+  }
+
+  return cookie_setting;
+}
+
 }  // namespace network
diff --git a/chromium_src/services/network/cookie_settings.h b/chromium_src/services/network/cookie_settings.h
@@ -11,13 +11,15 @@
       const net::CanonicalCookie& cookie, const GURL& url,        \
       const GURL& site_for_cookies,                               \
       const absl::optional<url::Origin>& top_frame_origin) const; \
+  ContentSetting GetDetailedCookieSetting(                            \
+      const GURL& url, bool* is_shields_disable_rule) const override; \
   bool IsCookieAccessible
 
-#define IsPrivacyModeEnabled                                      \
-  IsEphemeralPrivacyModeEnabled(                                  \
-      const GURL& url, const GURL& site_for_cookies,              \
-      const absl::optional<url::Origin>& top_frame_origin,        \
-      net::SamePartyContext::Type same_party_context_type) const; \
+#define IsPrivacyModeEnabled                                             \
+  IsEphemeralPrivacyModeEnabled(                                         \
+      const GURL& url, const GURL& site_for_cookies,                     \
+      const absl::optional<url::Origin>& top_frame_origin,               \
+      net::SamePartyContext::Type same_party_cookie_context_type) const; \
   bool IsPrivacyModeEnabled
 
 #define AnnotateAndMoveUserBlockedCookies                   \