From 4917adbd814c0570b8ce3038a79222c9ea286ed5 Mon Sep 17 00:00:00 2001 From: Dylan Cutler Date: Thu, 11 Aug 2022 20:44:17 +0000 Subject: [PATCH] Add UMA metric for max SameSite=None cookies per host This metric will be used to get a sense of how many cross-site cookies sites are using. This should give us more information on how many partitioned cookies we should allow domains to use per-partition, which is being discussed in https://github.com/privacycg/CHIPS/issues/48. Since CHIPS is not shipped and only enabled on a small fraction of clients' machines, this metric only tracks unpartitioned cookies. Bug: 1351841 Change-Id: I50f4b6f9bfd6e3bef2ca36d5621bf98aa6476ba9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3822813 Reviewed-by: Chris Fredrickson Commit-Queue: Dylan Cutler Cr-Commit-Position: refs/heads/main@{#1034162} --- net/cookies/cookie_monster.cc | 14 +++++ net/cookies/cookie_monster_unittest.cc | 51 +++++++++++++++++++ .../histograms/metadata/cookie/histograms.xml | 12 +++++ 3 files changed, 77 insertions(+) diff --git a/net/cookies/cookie_monster.cc b/net/cookies/cookie_monster.cc index 65225db56f6fe2..c433232365c938 100644 --- a/net/cookies/cookie_monster.cc +++ b/net/cookies/cookie_monster.cc @@ -2265,6 +2265,20 @@ bool CookieMonster::DoRecordPeriodicStats() { // Can be up to kMaxCookies. UMA_HISTOGRAM_COUNTS_10000("Cookie.NumKeys", num_keys_); + std::map n_same_site_none_cookies; + for (const auto& [host_key, host_cookie] : cookies_) { + if (!host_cookie || !host_cookie->IsEffectivelySameSiteNone()) + continue; + n_same_site_none_cookies[host_key]++; + } + size_t max_n_cookies = 0; + for (const auto& entry : n_same_site_none_cookies) { + max_n_cookies = std::max(max_n_cookies, entry.second); + } + // Can be up to 180 cookies, the max per-domain. + base::UmaHistogramCounts1000("Cookie.MaxSameSiteNoneCookiesPerKey", + max_n_cookies); + // Collect stats for partitioned cookies if they are enabled. if (base::FeatureList::IsEnabled(features::kPartitionedCookies)) { base::UmaHistogramCounts1000("Cookie.PartitionCount", diff --git a/net/cookies/cookie_monster_unittest.cc b/net/cookies/cookie_monster_unittest.cc index 6da3a84cdfb3df..707d0841d8fd6e 100644 --- a/net/cookies/cookie_monster_unittest.cc +++ b/net/cookies/cookie_monster_unittest.cc @@ -3469,6 +3469,57 @@ TEST_F(CookieMonsterTest, NumKeysHistogram) { } } +TEST_F(CookieMonsterTest, MaxSameSiteNoneCookiesPerKey) { + const char kHistogramName[] = "Cookie.MaxSameSiteNoneCookiesPerKey"; + + auto store = base::MakeRefCounted(); + auto cm = std::make_unique(store.get(), net::NetLog::Get(), + kFirstPartySetsDefault); + ASSERT_EQ(0u, GetAllCookies(cm.get()).size()); + + { // Only SameSite cookies should not log a sample. + base::HistogramTester histogram_tester; + + ASSERT_TRUE(CreateAndSetCookie(cm.get(), GURL("https://domain1.test"), + "A=1;SameSite=Lax", + CookieOptions::MakeAllInclusive())); + ASSERT_EQ(1u, GetAllCookies(cm.get()).size()); + ASSERT_TRUE(cm->DoRecordPeriodicStatsForTesting()); + histogram_tester.ExpectUniqueSample(kHistogramName, 0 /* sample */, + 1 /* count */); + } + + { // SameSite=None cookie should log a sample. + base::HistogramTester histogram_tester; + + ASSERT_TRUE(CreateAndSetCookie(cm.get(), GURL("https://domain1.test"), + "B=2;SameSite=None;Secure", + CookieOptions::MakeAllInclusive())); + ASSERT_EQ(2u, GetAllCookies(cm.get()).size()); + ASSERT_TRUE(cm->DoRecordPeriodicStatsForTesting()); + histogram_tester.ExpectUniqueSample(kHistogramName, 1 /* sample */, + 1 /* count */); + } + + { // Should log the maximum number of SameSite=None cookies. + base::HistogramTester histogram_tester; + + ASSERT_TRUE(CreateAndSetCookie(cm.get(), GURL("https://domain2.test"), + "A=1;SameSite=None;Secure", + CookieOptions::MakeAllInclusive())); + ASSERT_TRUE(CreateAndSetCookie(cm.get(), GURL("https://domain2.test"), + "B=2;SameSite=None;Secure", + CookieOptions::MakeAllInclusive())); + ASSERT_TRUE(CreateAndSetCookie(cm.get(), GURL("https://domain3.test"), + "A=1;SameSite=None;Secure", + CookieOptions::MakeAllInclusive())); + ASSERT_EQ(5u, GetAllCookies(cm.get()).size()); + ASSERT_TRUE(cm->DoRecordPeriodicStatsForTesting()); + histogram_tester.ExpectUniqueSample(kHistogramName, 2 /* sample */, + 1 /* count */); + } +} + // Test that localhost URLs can set and get secure cookies, even if // non-cryptographic. TEST_F(CookieMonsterTest, SecureCookieLocalhost) { diff --git a/tools/metrics/histograms/metadata/cookie/histograms.xml b/tools/metrics/histograms/metadata/cookie/histograms.xml index c7f024464ba59d..3514051fe6decb 100644 --- a/tools/metrics/histograms/metadata/cookie/histograms.xml +++ b/tools/metrics/histograms/metadata/cookie/histograms.xml @@ -471,6 +471,18 @@ chromium-metrics-reviews@google.com. + + dylancutler@chromium.org + src/net/cookies/OWNERS + + Maximum number of SameSite=None cookies that belong to a single domain on + the client. This histogram will be used to inform the + per-partition-per-domain limit for partitioned cookies. Recorded every 10 + minutes of active browsing time. + + + cfredric@chromium.org