Skip to content

Commit d27d506

Browse files
DCtheTallChromium LUCI CQ
authored and
Chromium LUCI CQ
committed
Make Clear-Site-Data: "cookies" respect third-party cookie blocking
This will prevent Clear-Site-Data from being abused for cross-site tracking [1] when partitioned cookies are enabled by default. [1]: privacycg/storage-partitioning#11 Old behavior: Clear-Site-Data could clear unpartitioned cookies from any context. New behavior: Clear-Site-Data cannot clear unpartitioned cookies if it came from a response where 3P cookie blocking applies. In both cases, CSD will delete partitioned cookies in the current partition. Low-Coverage-Reason:Some files have trial changes that do not impact behavior. Bug: 1416655 Change-Id: Ieed1e050f8f376b7d7704b4948c8f59adc21a17f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4312585 Reviewed-by: Nasko Oskov <[email protected]> Reviewed-by: Andrey Zaytsev <[email protected]> Reviewed-by: Maks Orlovich <[email protected]> Reviewed-by: Daniel Murphy <[email protected]> Commit-Queue: Dylan Cutler <[email protected]> Reviewed-by: Christian Dullweber <[email protected]> Reviewed-by: Oleh Lamzin <[email protected]> Cr-Commit-Position: refs/heads/main@{#1118080}
1 parent 7dad3cc commit d27d506

31 files changed

+348
-52
lines changed

chrome/browser/ash/wilco_dtc_supportd/wilco_dtc_supportd_network_context.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ void WilcoDtcSupportdNetworkContextImpl::OnClearSiteData(
114114
const std::string& header_value,
115115
int32_t load_flags,
116116
const absl::optional<net::CookiePartitionKey>& cookie_partition_key,
117+
bool partitioned_state_allowed_only,
117118
OnClearSiteDataCallback callback) {
118119
std::move(callback).Run();
119120
}

chrome/browser/ash/wilco_dtc_supportd/wilco_dtc_supportd_network_context.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ class WilcoDtcSupportdNetworkContextImpl
8080
const std::string& header_value,
8181
int32_t load_flags,
8282
const absl::optional<net::CookiePartitionKey>& cookie_partition_key,
83+
bool partitioned_state_allowed_only,
8384
OnClearSiteDataCallback callback) override;
8485
void OnLoadingStateUpdate(network::mojom::LoadInfoPtr info,
8586
OnLoadingStateUpdateCallback callback) override;

chrome/browser/browsing_data/browsing_data_remover_browsertest.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -786,6 +786,7 @@ class BrowsingDataRemoverWithPasswordsAccountStorageBrowserTest
786786
/*avoid_closing_connections=*/true,
787787
/*cookie_partition_key=*/cookie_partition_key,
788788
/*storage_key=*/storage_key,
789+
/*partitioned_state_allowed_only=*/false,
789790
/*callback=*/loop.QuitClosure());
790791
loop.Run();
791792
}
@@ -972,6 +973,7 @@ class BrowsingDataRemoverStorageBucketsBrowserTest
972973
/*avoid_closing_connections=*/true,
973974
/*cookie_partition_key=*/absl::nullopt,
974975
/*storage_key=*/storage_key,
976+
/*partitioned_state_allowed_only=*/false,
975977
/*callback=*/loop.QuitClosure());
976978
loop.Run();
977979
}

chrome/browser/ui/views/web_apps/web_app_uninstall_dialog_view.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,9 @@ void WebAppUninstallDialogDelegateView::ClearWebAppSiteData() {
196196
/*storage_buckets_to_remove=*/{},
197197
/*avoid_closing_connections=*/false,
198198
/*cookie_partition_key=*/absl::nullopt,
199-
/*storage_key=*/absl::nullopt, base::DoNothing());
199+
/*storage_key=*/absl::nullopt,
200+
/*partitioned_state_allowed_only=*/false,
201+
base::DoNothing());
200202
}
201203

202204
void WebAppUninstallDialogDelegateView::ProcessAutoConfirmValue() {

chrome/browser/web_applications/app_service/web_app_publisher_helper.cc

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -772,16 +772,17 @@ void WebAppPublisherHelper::UninstallWebApp(
772772
constexpr bool kClearCache = true;
773773
constexpr bool kAvoidClosingConnections = false;
774774

775-
content::ClearSiteData(base::BindRepeating(
776-
[](content::BrowserContext* browser_context) {
777-
return browser_context;
778-
},
779-
base::Unretained(profile())),
780-
origin, kClearCookies, kClearStorage, kClearCache,
781-
/*storage_buckets_to_remove=*/{},
782-
kAvoidClosingConnections,
783-
/*cookie_partition_key=*/absl::nullopt,
784-
/*storage_key=*/absl::nullopt, base::DoNothing());
775+
content::ClearSiteData(
776+
base::BindRepeating(
777+
[](content::BrowserContext* browser_context) {
778+
return browser_context;
779+
},
780+
base::Unretained(profile())),
781+
origin, kClearCookies, kClearStorage, kClearCache,
782+
/*storage_buckets_to_remove=*/{}, kAvoidClosingConnections,
783+
/*cookie_partition_key=*/absl::nullopt,
784+
/*storage_key=*/absl::nullopt,
785+
/*partitioned_state_allowed_only=*/false, base::DoNothing());
785786
}
786787

787788
void WebAppPublisherHelper::SetIconEffect(const std::string& app_id) {

content/browser/browsing_data/browsing_data_filter_builder_impl.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,10 @@ bool BrowsingDataFilterBuilderImpl::MatchesAllOriginsAndDomains() {
213213
return mode_ == Mode::kPreserve && origins_.empty() && domains_.empty();
214214
}
215215

216+
void BrowsingDataFilterBuilderImpl::SetPartitionedStateAllowedOnly(bool value) {
217+
partitioned_state_only_ = value;
218+
}
219+
216220
base::RepeatingCallback<bool(const GURL&)>
217221
BrowsingDataFilterBuilderImpl::BuildUrlFilter() {
218222
if (MatchesAllOriginsAndDomains())
@@ -274,6 +278,8 @@ BrowsingDataFilterBuilderImpl::BuildCookieDeletionFilter() {
274278
deletion_filter->cookie_partition_key_collection =
275279
cookie_partition_key_collection_;
276280

281+
deletion_filter->partitioned_state_only = partitioned_state_only_;
282+
277283
switch (mode_) {
278284
case Mode::kDelete:
279285
deletion_filter->including_domains.emplace(domains_.begin(),

content/browser/browsing_data/browsing_data_filter_builder_impl.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ class CONTENT_EXPORT BrowsingDataFilterBuilderImpl
4141
bool MatchesWithSavedStorageKey(
4242
const blink::StorageKey& other_key) const override;
4343
bool MatchesAllOriginsAndDomains() override;
44+
void SetPartitionedStateAllowedOnly(bool value) override;
4445
base::RepeatingCallback<bool(const GURL&)> BuildUrlFilter() override;
4546
content::StoragePartition::StorageKeyMatcherFunction BuildStorageKeyFilter()
4647
override;
@@ -68,6 +69,7 @@ class CONTENT_EXPORT BrowsingDataFilterBuilderImpl
6869
net::CookiePartitionKeyCollection cookie_partition_key_collection_ =
6970
net::CookiePartitionKeyCollection::ContainsAll();
7071
absl::optional<blink::StorageKey> storage_key_ = absl::nullopt;
72+
bool partitioned_state_only_ = false;
7173
};
7274

7375
} // content

content/browser/browsing_data/browsing_data_filter_builder_impl_unittest.cc

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -869,4 +869,51 @@ TEST(BrowsingDataFilterBuilderImplTest, GetRegisterableDomains) {
869869
UnorderedElementsAre(kGoogleDomain, kLongETLDDomain));
870870
}
871871

872+
TEST(BrowsingDataFilterBuilderImplTest, ExcludeUnpartitionedCookies) {
873+
BrowsingDataFilterBuilderImpl builder(
874+
BrowsingDataFilterBuilderImpl::Mode::kDelete);
875+
876+
builder.SetPartitionedStateAllowedOnly(true);
877+
878+
CookieDeletionInfo delete_info =
879+
network::DeletionFilterToInfo(builder.BuildCookieDeletionFilter());
880+
881+
// Unpartitioned cookie should NOT match.
882+
std::unique_ptr<net::CanonicalCookie> cookie = net::CanonicalCookie::Create(
883+
GURL("https://www.cookie.com/"),
884+
"__Host-A=B; Secure; SameSite=None; Path=/;", base::Time::Now(),
885+
absl::nullopt, absl::nullopt);
886+
EXPECT_TRUE(cookie);
887+
EXPECT_FALSE(delete_info.Matches(
888+
*cookie, net::CookieAccessParams{
889+
net::CookieAccessSemantics::NONLEGACY, false,
890+
net::CookieSamePartyStatus::kNoSamePartyEnforcement}));
891+
892+
// Partitioned cookie should match.
893+
cookie = net::CanonicalCookie::Create(
894+
GURL("https://www.cookie.com/"),
895+
"__Host-A=B; Secure; SameSite=None; Path=/; Partitioned;",
896+
base::Time::Now(), absl::nullopt,
897+
net::CookiePartitionKey::FromURLForTesting(
898+
GURL("https://toplevelsite.com")));
899+
EXPECT_TRUE(cookie);
900+
EXPECT_TRUE(delete_info.Matches(
901+
*cookie, net::CookieAccessParams{
902+
net::CookieAccessSemantics::NONLEGACY, false,
903+
net::CookieSamePartyStatus::kNoSamePartyEnforcement}));
904+
905+
// Nonced partitioned cookie should match.
906+
cookie = net::CanonicalCookie::Create(
907+
GURL("https://www.cookie.com/"),
908+
"__Host-A=B; Secure; SameSite=None; Path=/;", base::Time::Now(),
909+
absl::nullopt,
910+
net::CookiePartitionKey::FromURLForTesting(
911+
GURL("https://toplevelsite.com"), base::UnguessableToken::Create()));
912+
EXPECT_TRUE(cookie);
913+
EXPECT_TRUE(delete_info.Matches(
914+
*cookie, net::CookieAccessParams{
915+
net::CookieAccessSemantics::NONLEGACY, false,
916+
net::CookieSamePartyStatus::kNoSamePartyEnforcement}));
917+
}
918+
872919
} // namespace content

content/browser/browsing_data/browsing_data_remover_impl_browsertest.cc

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,36 @@ IN_PROC_BROWSER_TEST_F(CookiesBrowsingDataRemoverImplBrowserTest,
407407
EXPECT_EQ("F", cookies[2].Name());
408408
}
409409

410+
IN_PROC_BROWSER_TEST_F(CookiesBrowsingDataRemoverImplBrowserTest,
411+
ClearSiteData_PartitionedStateAllowedOnly) {
412+
// Unpartitioned cookie should not be removed when third-party cookie blocking
413+
// applies to the request that sent Clear-Site-Data.
414+
ASSERT_TRUE(SetCookie(GURL("https://a.com"), "A=0; secure;",
415+
/*cookie_partition_key=*/absl::nullopt));
416+
// Partitioned cookies should still be removed.
417+
ASSERT_TRUE(SetCookie(
418+
GURL("https://a.com"), "B=1; secure; partitioned",
419+
net::CookiePartitionKey::FromURLForTesting(GURL("https://b.com"))));
420+
// Nonced partitioned cookies should still be removed.
421+
ASSERT_TRUE(
422+
SetCookie(GURL("https://a.com"), "C=2; secure;",
423+
net::CookiePartitionKey::FromURLForTesting(
424+
GURL("https://b.com"), base::UnguessableToken::Create())));
425+
426+
std::unique_ptr<BrowsingDataFilterBuilder> builder(
427+
BrowsingDataFilterBuilder::Create(
428+
BrowsingDataFilterBuilder::Mode::kDelete));
429+
builder->AddRegisterableDomain("a.com");
430+
builder->SetPartitionedStateAllowedOnly(true);
431+
432+
RemoveWithFilterAndWait(BrowsingDataRemover::DATA_TYPE_COOKIES,
433+
std::move(builder));
434+
435+
auto cookies = GetAllCookies();
436+
EXPECT_EQ(1u, cookies.size());
437+
EXPECT_EQ("A", cookies[0].Name());
438+
}
439+
410440
namespace {
411441
// Provide BrowsingDataRemoverImplTrustTokenTest the Trust Tokens
412442
// feature as a mixin so that it gets set before the superclass initializes

content/browser/browsing_data/clear_site_data_handler.cc

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,12 @@ void ClearSiteDataHandler::HandleHeader(
104104
int load_flags,
105105
const absl::optional<net::CookiePartitionKey>& cookie_partition_key,
106106
const absl::optional<blink::StorageKey>& storage_key,
107+
bool partitioned_state_allowed_only,
107108
base::OnceClosure callback) {
108109
ClearSiteDataHandler handler(browser_context_getter, web_contents_getter, url,
109110
header_value, load_flags, cookie_partition_key,
110-
storage_key, std::move(callback),
111+
storage_key, partitioned_state_allowed_only,
112+
std::move(callback),
111113
std::make_unique<ConsoleMessagesDelegate>());
112114
handler.HandleHeaderAndOutputConsoleMessages();
113115
}
@@ -134,6 +136,7 @@ ClearSiteDataHandler::ClearSiteDataHandler(
134136
int load_flags,
135137
const absl::optional<net::CookiePartitionKey>& cookie_partition_key,
136138
const absl::optional<blink::StorageKey>& storage_key,
139+
bool partitioned_state_allowed_only,
137140
base::OnceClosure callback,
138141
std::unique_ptr<ConsoleMessagesDelegate> delegate)
139142
: browser_context_getter_(browser_context_getter),
@@ -143,6 +146,7 @@ ClearSiteDataHandler::ClearSiteDataHandler(
143146
load_flags_(load_flags),
144147
cookie_partition_key_(cookie_partition_key),
145148
storage_key_(storage_key),
149+
partitioned_state_allowed_only_(partitioned_state_allowed_only),
146150
callback_(std::move(callback)),
147151
delegate_(std::move(delegate)) {
148152
DCHECK(browser_context_getter_);
@@ -336,7 +340,8 @@ void ClearSiteDataHandler::ExecuteClearingTask(
336340
ClearSiteData(browser_context_getter_, origin, clear_cookies, clear_storage,
337341
clear_cache, storage_buckets_to_remove,
338342
true /*avoid_closing_connections*/, cookie_partition_key_,
339-
storage_key_, std::move(callback));
343+
storage_key_, partitioned_state_allowed_only_,
344+
std::move(callback));
340345
}
341346

342347
// static

content/browser/browsing_data/clear_site_data_handler.h

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ class CONTENT_EXPORT ClearSiteDataHandler {
8383
int load_flags,
8484
const absl::optional<net::CookiePartitionKey>& cookie_partition_key,
8585
const absl::optional<blink::StorageKey>& storage_key,
86+
bool partitioned_state_allowed_only,
8687
base::OnceClosure callback);
8788

8889
// Exposes ParseHeader() publicly for testing.
@@ -104,6 +105,7 @@ class CONTENT_EXPORT ClearSiteDataHandler {
104105
int load_flags,
105106
const absl::optional<net::CookiePartitionKey>& cookie_partition_key,
106107
const absl::optional<blink::StorageKey>& storage_key,
108+
bool partitioned_state_allowed_only,
107109
base::OnceClosure callback,
108110
std::unique_ptr<ConsoleMessagesDelegate> delegate);
109111
virtual ~ClearSiteDataHandler();
@@ -163,27 +165,35 @@ class CONTENT_EXPORT ClearSiteDataHandler {
163165
return storage_key_;
164166
}
165167

168+
bool PartitionedStateOnlyForTesting() const {
169+
return partitioned_state_allowed_only_;
170+
}
171+
166172
private:
167173
// Required to clear the data.
168174
base::RepeatingCallback<BrowserContext*()> browser_context_getter_;
169175
base::RepeatingCallback<WebContents*()> web_contents_getter_;
170176

171177
// Target URL whose data will be cleared.
172-
GURL url_;
178+
const GURL url_;
173179

174180
// Raw string value of the 'Clear-Site-Data' header.
175-
std::string header_value_;
181+
const std::string header_value_;
176182

177183
// Load flags of the current request, used to check cookie policies.
178-
int load_flags_;
184+
const int load_flags_;
179185

180186
// The cookie partition key for which we need to clear partitioned cookies
181187
// when we receive the Clear-Site-Data header.
182-
absl::optional<net::CookiePartitionKey> cookie_partition_key_;
188+
const absl::optional<net::CookiePartitionKey> cookie_partition_key_;
183189

184190
// The storage key for which we need to clear partitioned storage when we
185191
// receive the Clear-Site-Data header.
186-
absl::optional<blink::StorageKey> storage_key_;
192+
const absl::optional<blink::StorageKey> storage_key_;
193+
194+
// If third-party cookie blocking is enabled and applies to the response that
195+
// sent Clear-Site-Data.
196+
const bool partitioned_state_allowed_only_;
187197

188198
// Used to notify that the clearing has completed. Callers could resuming
189199
// loading after this point.

content/browser/browsing_data/clear_site_data_handler_browsertest.cc

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -174,14 +174,20 @@ class ClearSiteDataHandlerBrowserTest : public ContentBrowserTest {
174174
}
175175

176176
// Adds a cookie for the |url|. Used in the cookie integration tests.
177-
void AddCookie(const GURL& url) {
177+
void AddCookie(const GURL& url,
178+
const absl::optional<net::CookiePartitionKey>&
179+
cookie_partition_key = absl::nullopt) {
178180
DCHECK_CURRENTLY_ON(BrowserThread::UI);
179181
network::mojom::CookieManager* cookie_manager =
180182
storage_partition()->GetCookieManagerForBrowserProcess();
181183

184+
std::string cookie_line = "A=1";
185+
if (cookie_partition_key) {
186+
cookie_line += "; Secure; Partitioned";
187+
}
182188
std::unique_ptr<net::CanonicalCookie> cookie(net::CanonicalCookie::Create(
183-
url, "A=1", base::Time::Now(), absl::nullopt /* server_time */,
184-
absl::nullopt /* cookie_partition_key */));
189+
url, cookie_line, base::Time::Now(), /*server_time=*/absl::nullopt,
190+
cookie_partition_key));
185191

186192
base::RunLoop run_loop;
187193
cookie_manager->SetCanonicalCookie(
@@ -296,6 +302,12 @@ class ClearSiteDataHandlerBrowserTest : public ContentBrowserTest {
296302
response->AddCustomHeader("X-XSS-Protection", "0");
297303
}
298304

305+
if (net::GetValueForKeyInQuery(request.GetURL(),
306+
"access-control-allow-origin", &value)) {
307+
response->AddCustomHeader("Access-Control-Allow-Origin", value);
308+
response->AddCustomHeader("Access-Control-Allow-Credentials", "true");
309+
}
310+
299311
browsing_data_browsertest_utils::SetResponseContent(request.GetURL(),
300312
&value, response.get());
301313

@@ -744,6 +756,57 @@ IN_PROC_BROWSER_TEST_F(ClearSiteDataHandlerBrowserTest,
744756
EXPECT_EQ(cookies[1].Domain(), "subdomain.origin2.com");
745757
}
746758

759+
IN_PROC_BROWSER_TEST_F(ClearSiteDataHandlerBrowserTest,
760+
ThirdPartyCookieBlocking) {
761+
// When third-party cookie blocking is disabled, both cookies should be
762+
// cleared.
763+
AddCookie(https_server()->GetURL("origin1.com", "/"));
764+
AddCookie(
765+
https_server()->GetURL("origin1.com", "/"),
766+
net::CookiePartitionKey::FromURLForTesting(GURL("https://origin2.com")));
767+
768+
GURL url = https_server()->GetURL("origin2.com", "/");
769+
EXPECT_TRUE(NavigateToURL(shell(), url));
770+
771+
GURL csd_url = https_server()->GetURL("origin1.com", "/clear-site-data");
772+
AddQuery(&csd_url, "header", kClearCookiesHeader);
773+
std::string origin = url.spec();
774+
// Pop the last character to remove trailing /.
775+
origin.erase(origin.size() - 1);
776+
AddQuery(&csd_url, "access-control-allow-origin", origin);
777+
778+
// Script that makes a cross-site subresource request that responds with
779+
// Clear-Site-Data.
780+
std::string script =
781+
"fetch('" + csd_url.spec() + "', {credentials: 'include'})";
782+
script += ".then(resp => resp.ok)";
783+
script += ".catch(err => { console.error(err); return false; });";
784+
785+
EXPECT_EQ(true, EvalJs(shell()->web_contents(), script));
786+
787+
auto cookies = GetCookies();
788+
ASSERT_EQ(0u, cookies.size());
789+
790+
// Now enable third-party cookie blocking.
791+
network::mojom::CookieManager* cookie_manager =
792+
storage_partition()->GetCookieManagerForBrowserProcess();
793+
cookie_manager->BlockThirdPartyCookies(true);
794+
795+
// Unpartitioned cookie, should not be removed.
796+
AddCookie(https_server()->GetURL("origin1.com", "/"));
797+
// Partitioned cookie set in the partition we are clearing, should still
798+
// be removed.
799+
AddCookie(
800+
https_server()->GetURL("origin1.com", "/"),
801+
net::CookiePartitionKey::FromURLForTesting(GURL("https://origin2.com")));
802+
803+
EXPECT_EQ(true, EvalJs(shell()->web_contents(), script));
804+
805+
cookies = GetCookies();
806+
ASSERT_EQ(1u, cookies.size());
807+
EXPECT_FALSE(cookies[0].IsPartitioned());
808+
}
809+
747810
// Integration test for the unregistering of service workers.
748811
IN_PROC_BROWSER_TEST_F(ClearSiteDataHandlerBrowserTest,
749812
StorageServiceWorkersIntegrationTest) {

0 commit comments

Comments
 (0)