Skip to content

Commit

Permalink
Handle 3p frames properly when shields are disabled for 1p frame.
Browse files Browse the repository at this point in the history
  • Loading branch information
goodov committed Sep 14, 2021
1 parent 8ec30cb commit d7bf804
Show file tree
Hide file tree
Showing 11 changed files with 278 additions and 53 deletions.
74 changes: 74 additions & 0 deletions browser/ephemeral_storage/ephemeral_storage_1p_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#include "brave/browser/ephemeral_storage/ephemeral_storage_browsertest.h"

#include "base/strings/strcat.h"
#include "base/test/bind.h"
#include "brave/components/brave_shields/browser/brave_shields_util.h"
#include "chrome/browser/content_settings/cookie_settings_factory.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/profiles/profile.h"
Expand All @@ -14,8 +16,10 @@
#include "components/content_settings/core/browser/cookie_settings.h"
#include "components/content_settings/core/browser/host_content_settings_map.h"
#include "components/content_settings/core/common/content_settings_pattern.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/test/browser_test.h"
#include "net/base/features.h"
#include "services/network/public/mojom/cookie_manager.mojom.h"

using content::RenderFrameHost;
using content::WebContents;
Expand Down Expand Up @@ -63,6 +67,29 @@ class EphemeralStorage1pBrowserTest : public EphemeralStorageBrowserTest {
return eval_js_result.error.empty();
}

HostContentSettingsMap* content_settings() {
return HostContentSettingsMapFactory::GetForProfile(browser()->profile());
}

network::mojom::CookieManager* CookieManager() {
return browser()
->profile()
->GetDefaultStoragePartition()
->GetCookieManagerForBrowserProcess();
}

std::vector<net::CanonicalCookie> GetAllCookies() {
base::RunLoop run_loop;
std::vector<net::CanonicalCookie> cookies_out;
CookieManager()->GetAllCookies(base::BindLambdaForTesting(
[&](const std::vector<net::CanonicalCookie>& cookies) {
cookies_out = cookies;
run_loop.Quit();
}));
run_loop.Run();
return cookies_out;
}

private:
base::test::ScopedFeatureList scoped_feature_list_;
};
Expand Down Expand Up @@ -432,3 +459,50 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorage1pBrowserTest,
// Cookie values should be empty after a cleanup.
ExpectValuesFromFramesAreEmpty(FROM_HERE, GetValuesFromFrames(site_a_tab2));
}

IN_PROC_BROWSER_TEST_F(
EphemeralStorage1pBrowserTest,
DisabledShieldsAllowsPersistentCookiesFor1PesHostsIn3pFrames) {
// Disable shields on a.com.
brave_shields::SetBraveShieldsEnabled(content_settings(), false,
a_site_ephemeral_storage_url_);
// Enable 1PES on b.com.
SetCookieSetting(b_site_ephemeral_storage_url_, CONTENT_SETTING_SESSION_ONLY);

// Navigate to a.com which includes b.com.
WebContents* site_a_tab_network_cookies =
LoadURLInNewTab(a_site_ephemeral_storage_with_network_cookies_url_);
http_request_monitor_.Clear();

// Cookies should be stored in persistent storage for the main frame and a
// third party frame.
EXPECT_EQ(2u, GetAllCookies().size());

// Navigate to other website and ensure no a.com/b.com cookies are sent (they
// are third-party and ephemeral inside c.com).
ASSERT_TRUE(content::NavigateToURL(site_a_tab_network_cookies,
c_site_ephemeral_storage_url_));
EXPECT_FALSE(http_request_monitor_.HasHttpRequestWithCookie(
a_site_ephemeral_storage_url_, "name=acom_simple"));
EXPECT_FALSE(http_request_monitor_.HasHttpRequestWithCookie(
b_site_ephemeral_storage_url_, "name=bcom_simple"));
WaitForCleanupAfterKeepAlive();
http_request_monitor_.Clear();

// a.com and b.com cookies should be intact.
EXPECT_EQ(2u, GetAllCookies().size());

// Navigate to a.com again and expect a.com and b.com cookies are sent with
// headers.
WebContents* site_a_tab = LoadURLInNewTab(a_site_ephemeral_storage_url_);
EXPECT_TRUE(http_request_monitor_.HasHttpRequestWithCookie(
a_site_ephemeral_storage_url_, "name=acom_simple"));
EXPECT_TRUE(http_request_monitor_.HasHttpRequestWithCookie(
b_site_ephemeral_storage_url_, "name=bcom_simple"));

// Make sure cookies are also accessible via JS.
ValuesFromFrames site_a_tab_values = GetValuesFromFrames(site_a_tab);
EXPECT_EQ("name=acom_simple", site_a_tab_values.main_frame.cookies);
EXPECT_EQ("name=bcom_simple", site_a_tab_values.iframe_1.cookies);
EXPECT_EQ("name=bcom_simple", site_a_tab_values.iframe_2.cookies);
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,18 @@
#include "net/base/features.h"
#include "net/base/url_util.h"

#define BRAVE_COOKIE_SETTINGS_GET_COOKIES_SETTINGS_INTERNAL \
if (setting == CONTENT_SETTING_SESSION_ONLY && !block_third && \
ShouldBlockThirdPartyCookies() && \
!first_party_url.SchemeIs(extension_scheme_) && \
base::FeatureList::IsEnabled( \
net::features::kBraveFirstPartyEphemeralStorage)) { \
block_third = true; \
#define BRAVE_COOKIE_SETTINGS_GET_COOKIES_SETTINGS_INTERNAL \
if (!block && is_third_party_request) { \
block = ShouldBlockThirdPartyIfSettingIsExplicit( \
ShouldBlockThirdPartyCookies(), setting, IsExplicitSetting(info), \
first_party_url.SchemeIs(extension_scheme_)); \
} \
if (auto* setting_with_brave_metadata = \
cookie_setting_with_brave_metadata()) { \
setting_with_brave_metadata->primary_pattern_matches_all_hosts = \
info.primary_pattern.MatchesAllHosts(); \
setting_with_brave_metadata->secondary_pattern_matches_all_hosts = \
info.secondary_pattern.MatchesAllHosts(); \
}

#define ShutdownOnUIThread ShutdownOnUIThread_ChromiumImpl
Expand Down Expand Up @@ -84,5 +89,3 @@ std::vector<url::Origin> CookieSettings::TakeEphemeralStorageOpaqueOrigins(
}

} // namespace content_settings

#undef BRAVE_COOKIE_SETTINGS_GET_COOKIES_SETTINGS_INTERNAL
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,24 @@ bool RendererContentSettingRules::IsRendererContentSetting(
content_type) ||
content_type == ContentSettingsType::AUTOPLAY;
}

namespace content_settings {
namespace {

bool IsExplicitSetting(const ContentSettingsPattern& primary_pattern,
const ContentSettingsPattern& secondary_pattern) {
return !primary_pattern.MatchesAllHosts() ||
!secondary_pattern.MatchesAllHosts();
}

} // namespace

bool IsExplicitSetting(const ContentSettingPatternSource& setting) {
return IsExplicitSetting(setting.primary_pattern, setting.secondary_pattern);
}

bool IsExplicitSetting(const SettingInfo& setting) {
return IsExplicitSetting(setting.primary_pattern, setting.secondary_pattern);
}

} // namespace content_settings
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,11 @@ struct RendererContentSettingRules
ContentSettingsForOneType brave_shields_rules;
};

namespace content_settings {

bool IsExplicitSetting(const ContentSettingPatternSource& setting);
bool IsExplicitSetting(const SettingInfo& setting);

} // namespace content_settings

#endif // BRAVE_CHROMIUM_SRC_COMPONENTS_CONTENT_SETTINGS_CORE_COMMON_CONTENT_SETTINGS_H_
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "components/content_settings/core/common/cookie_settings_base.h"

#include "base/auto_reset.h"
#include "base/feature_list.h"
#include "base/no_destructor.h"
#include "base/stl_util.h"
Expand Down Expand Up @@ -85,6 +86,17 @@ bool IsFirstPartyAccessAllowed(

} // namespace

CookieSettingWithBraveMetadata::CookieSettingWithBraveMetadata() = default;
CookieSettingWithBraveMetadata::CookieSettingWithBraveMetadata(
const CookieSettingWithBraveMetadata&) = default;
CookieSettingWithBraveMetadata::CookieSettingWithBraveMetadata(
CookieSettingWithBraveMetadata&&) = default;
CookieSettingWithBraveMetadata& CookieSettingWithBraveMetadata::operator=(
const CookieSettingWithBraveMetadata&) = default;
CookieSettingWithBraveMetadata& CookieSettingWithBraveMetadata::operator=(
CookieSettingWithBraveMetadata&&) = default;
CookieSettingWithBraveMetadata::~CookieSettingWithBraveMetadata() = default;

bool CookieSettingsBase::ShouldUseEphemeralStorage(
const GURL& url,
const GURL& site_for_cookies,
Expand All @@ -98,6 +110,8 @@ bool CookieSettingsBase::ShouldUseEphemeralStorage(
if (!first_party_url.is_valid())
return false;

// SESSION_ONLY cookie setting works as a "1PES enabler" for the website if
// a 1PES feature is enabled.
if (base::FeatureList::IsEnabled(
net::features::kBraveFirstPartyEphemeralStorage) &&
IsCookieSessionOnly(first_party_url)) {
Expand Down Expand Up @@ -164,16 +178,44 @@ bool CookieSettingsBase::IsCookieAccessAllowedImpl(

const bool is_1p_ephemeral_feature_enabled = base::FeatureList::IsEnabled(
net::features::kBraveFirstPartyEphemeralStorage);
// If 1PES feature is enabled, we should do additional checks below.
if (allow && !is_1p_ephemeral_feature_enabled)
return true;

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;
// Determine whether a first party frame is ephemeral or shields are down.
enum class FirstPartyFrameMode {
kDefault,
kEphemeral,
kShieldsDown,
};
FirstPartyFrameMode first_party_frame_mode = FirstPartyFrameMode::kDefault;
if (is_1p_ephemeral_feature_enabled) {
CookieSettingWithBraveMetadata setting_with_brave_metadata =
GetCookieSettingWithBraveMetadata(first_party_url, first_party_url);

if (setting_with_brave_metadata.setting == CONTENT_SETTING_SESSION_ONLY) {
first_party_frame_mode = FirstPartyFrameMode::kEphemeral;
} else if (setting_with_brave_metadata.setting == CONTENT_SETTING_ALLOW &&
setting_with_brave_metadata.primary_pattern_matches_all_hosts &&
!setting_with_brave_metadata
.secondary_pattern_matches_all_hosts) {
// Disabled shields mode allows everything in nested frames, so we
// determine the fact that shields are disabled by expecting primary and
// secondary patterns to be in a specific state.
first_party_frame_mode = FirstPartyFrameMode::kShieldsDown;
}
}

if (allow) {
// Block all non ephemeral-supported activities (service workers, etc.) if
// 1p is ephemeral.
if (first_party_frame_mode == FirstPartyFrameMode::kEphemeral) {
allow = false;
}
return allow;
}

if (!IsFirstPartyAccessAllowed(first_party_url, this))
Expand All @@ -182,9 +224,48 @@ bool CookieSettingsBase::IsCookieAccessAllowedImpl(
if (BraveIsAllowedThirdParty(url, first_party_url, this))
return true;

if (is_1p_ephemeral_feature_enabled &&
first_party_frame_mode == FirstPartyFrameMode::kShieldsDown &&
IsCookieSessionOnly(url)) {
// Allow 3p session-only frames when shields are disabled.
return true;
}

return false;
}

// Determines whether a 3p cookies block should be applied if a requesting URL
// uses an explicit 1PES setting (CONTENT_SETTING_SESSION_ONLY).
// By default Chromimum allows all 3p cookies if applied CookieSettingsPatterns
// for the URL were explicit. We use explicit setting to enable 1PES mode, but
// in this mode we still want to block 3p frames as usual and not fallback to
// "allow everything" path.
bool CookieSettingsBase::ShouldBlockThirdPartyIfSettingIsExplicit(
bool block_third_party_cookies,
ContentSetting cookie_setting,
bool is_explicit_setting,
bool is_first_party_allowed_scheme) const {
return block_third_party_cookies &&
cookie_setting == CONTENT_SETTING_SESSION_ONLY &&
is_explicit_setting && !is_first_party_allowed_scheme &&
base::FeatureList::IsEnabled(
net::features::kBraveFirstPartyEphemeralStorage);
}

CookieSettingWithBraveMetadata
CookieSettingsBase::GetCookieSettingWithBraveMetadata(
const GURL& url,
const GURL& first_party_url) const {
CookieSettingWithBraveMetadata setting_brave_metadata;
base::AutoReset<CookieSettingWithBraveMetadata*> auto_reset(
&cookie_setting_with_brave_metadata_, &setting_brave_metadata);
// GetCookieSetting fills metadata structure implicitly (implemented in
// GetCookieSettingInternal), the setting value is set explicitly here.
setting_brave_metadata.setting =
GetCookieSetting(url, first_party_url, nullptr);
return setting_brave_metadata;
}

} // namespace content_settings

#define IsFullCookieAccessAllowed IsChromiumFullCookieAccessAllowed
Expand Down
Loading

0 comments on commit d7bf804

Please sign in to comment.