[SAA] Add use counter for cross-origin, same-site SAA usage
By "cross-origin, same-site", I mean that the iframe that called
`document.requestStorageAccess()` and then sent a network fetch, is
cross-origin but same-site to the origin that it's sending the network
fetch to. This is an interesting case because there ought to be a
security boundary between those origins, but there's no privacy boundary
since they're the same site.
This will give more actionable breakage metrics (including UKM) which
will be useful to decide whether we can ship this feature
(http://chromestatus/5169937372676096).
Bug: 379030052
Change-Id: If2938c3ac64e6abefcda3b0e7c3e3a0f371e4bfc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6397807
Reviewed-by: Maks Orlovich <[email protected]>
Reviewed-by: Ken Buchanan <[email protected]>
Reviewed-by: Alex Moshchuk <[email protected]>
Auto-Submit: Chris Fredrickson <[email protected]>
Reviewed-by: Theresa Sullivan <[email protected]>
Reviewed-by: Robert Kaplow <[email protected]>
Commit-Queue: Robert Kaplow <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1439029}
diff --git a/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher_impl_unittest.cc b/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher_impl_unittest.cc
index 87564d29..15e0302b 100644
--- a/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher_impl_unittest.cc
+++ b/chrome/browser/signin/bound_session_credentials/bound_session_refresh_cookie_fetcher_impl_unittest.cc
@@ -202,8 +202,8 @@
network::mojom::CookieAccessDetails::Type access_type) {
std::vector<network::mojom::CookieAccessDetailsPtr> cookie_access_details;
cookie_access_details.emplace_back(network::mojom::CookieAccessDetails::New(
- access_type, kGaiaUrl, url::Origin(), net::SiteForCookies(),
- CreateReportedCookies(cookies_), std::nullopt,
+ access_type, kGaiaUrl, url::Origin(), url::Origin(),
+ net::SiteForCookies(), CreateReportedCookies(cookies_), std::nullopt,
/*is_ad_tagged=*/false, net::CookieSettingOverrides()));
fetcher_->OnCookiesAccessed(std::move(cookie_access_details));
}
diff --git a/chrome/browser/storage_access_api/api_browsertest.cc b/chrome/browser/storage_access_api/api_browsertest.cc
index 6c37ec3..cf34319 100644
--- a/chrome/browser/storage_access_api/api_browsertest.cc
+++ b/chrome/browser/storage_access_api/api_browsertest.cc
@@ -869,6 +869,7 @@
IN_PROC_BROWSER_TEST_F(StorageAccessAPIBrowserTest,
ThirdPartyCookiesIFrameRequestsAccess_CrossSiteIframe) {
SetBlockThirdPartyCookies(true);
+ base::HistogramTester histogram_tester;
NavigateToPageWithFrame(kHostA);
NavigateFrameTo(EchoCookiesURL(kHostB));
@@ -881,6 +882,12 @@
EXPECT_TRUE(storage::test::RequestAndCheckStorageAccessForFrame(GetFrame()));
EXPECT_EQ(ReadCookies(GetFrame(), kHostB), CookieBundle("cross-site=b.test"));
+
+ histogram_tester.ExpectBucketCount(
+ "Blink.UseCounter.Features",
+ blink::mojom::WebFeature::
+ kCrossOriginSameSiteCookieAccessViaStorageAccessAPI,
+ 0);
}
IN_PROC_BROWSER_TEST_F(StorageAccessAPIBrowserTest,
@@ -989,6 +996,7 @@
IN_PROC_BROWSER_TEST_F(StorageAccessAPIBrowserTest,
ThirdPartyCookiesIFrameRequestsAccess_CrossOriginFetch) {
SetBlockThirdPartyCookies(true);
+ base::HistogramTester histogram_tester;
NavigateToPageWithFrame(kHostA);
NavigateFrameTo(EchoCookiesURL(kHostBSubdomain));
@@ -1003,6 +1011,12 @@
EXPECT_EQ(CookiesFromFetch(GetFrame(), kHostBSubdomain2),
"cross-site=b.test");
+
+ histogram_tester.ExpectBucketCount(
+ "Blink.UseCounter.Features",
+ blink::mojom::WebFeature::
+ kCrossOriginSameSiteCookieAccessViaStorageAccessAPI,
+ 1);
}
// Validate that in a A(B(B)) frame tree, the middle B iframe can obtain access,
diff --git a/components/page_load_metrics/browser/observers/use_counter/ukm_features.cc b/components/page_load_metrics/browser/observers/use_counter/ukm_features.cc
index b0ca0f3..46c62b8 100644
--- a/components/page_load_metrics/browser/observers/use_counter/ukm_features.cc
+++ b/components/page_load_metrics/browser/observers/use_counter/ukm_features.cc
@@ -467,6 +467,7 @@
WebFeature::kLanguageModel_Create,
WebFeature::kLanguageModel_Prompt,
WebFeature::kLanguageModel_PromptStreaming,
+ WebFeature::kCrossOriginSameSiteCookieAccessViaStorageAccessAPI,
// NOTE: before adding new use counters here, verify in UMA that their
// emissions are very rare, e.g. <1% of page loads.
}));
diff --git a/content/browser/renderer_host/cookie_utils.cc b/content/browser/renderer_host/cookie_utils.cc
index b389dfb..13e0fce7 100644
--- a/content/browser/renderer_host/cookie_utils.cc
+++ b/content/browser/renderer_host/cookie_utils.cc
@@ -22,9 +22,12 @@
#include "content/public/browser/legacy_tech_cookie_issue_details.h"
#include "content/public/common/content_client.h"
#include "content/public/common/content_features.h"
+#include "net/cookies/cookie_constants.h"
#include "net/cookies/cookie_inclusion_status.h"
+#include "net/cookies/cookie_setting_override.h"
#include "services/metrics/public/cpp/metrics_utils.h"
#include "services/metrics/public/cpp/ukm_builders.h"
+#include "third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom-shared.h"
#include "url/gurl.h"
namespace content {
@@ -274,6 +277,21 @@
net::CookieInclusionStatus::ExemptionReason::k3PCDHeuristics;
}
+bool IsCrossOriginSameSiteNetworkAccessWithStorageAccessEligible(
+ const network::mojom::CookieAccessDetailsPtr& cookie_details) {
+ if (!cookie_details->frame_origin ||
+ !cookie_details->cookie_setting_overrides.Has(
+ net::CookieSettingOverride::kStorageAccessGrantEligible)) {
+ // `frame_origin` is unset for script accesses, and network accesses whose
+ // IsolationInfo's `frame_origin` was nullptr.
+ return false;
+ }
+ const url::Origin origin = url::Origin::Create(cookie_details->url);
+ return !origin.IsSameOriginWith(cookie_details->frame_origin.value()) &&
+ net::SchemefulSite::IsSameSite(origin,
+ cookie_details->frame_origin.value());
+}
+
} // namespace
void SplitCookiesIntoAllowedAndBlocked(
@@ -375,6 +393,11 @@
int cookies_exempted_by_top_level_storage_access = 0;
+ const bool cross_origin_same_site_with_storage_access_eligible =
+ IsCrossOriginSameSiteNetworkAccessWithStorageAccessEligible(
+ cookie_details);
+ bool cross_origin_same_site_cookie_via_storage_access_api = false;
+
for (const network::mojom::CookieOrLineWithAccessResultPtr& cookie :
cookie_details->cookie_list) {
const net::CookieInclusionStatus& status = cookie->access_result.status;
@@ -499,6 +522,12 @@
cookie_has_not_been_refreshed_in_351_to_400_days |=
days_since_refresh > 350 && days_since_refresh <= 400;
}
+
+ cross_origin_same_site_cookie_via_storage_access_api |=
+ cross_origin_same_site_with_storage_access_eligible &&
+ cookie->access_result.status.IsInclude() &&
+ cookie->access_result.status.exemption_reason() ==
+ net::CookieInclusionStatus::ExemptionReason::kStorageAccess;
}
if (samesite_treated_as_lax_cookies) {
@@ -560,6 +589,12 @@
rfh->GetPageUkmSourceId(),
cookies_exempted_by_top_level_storage_access);
}
+
+ if (cross_origin_same_site_cookie_via_storage_access_api) {
+ GetContentClient()->browser()->LogWebFeatureForCurrentPage(
+ rfh, blink::mojom::WebFeature::
+ kCrossOriginSameSiteCookieAccessViaStorageAccessAPI);
+ }
}
} // namespace content
diff --git a/services/network/public/mojom/cookie_access_observer.mojom b/services/network/public/mojom/cookie_access_observer.mojom
index 36a98f2..f124a24 100644
--- a/services/network/public/mojom/cookie_access_observer.mojom
+++ b/services/network/public/mojom/cookie_access_observer.mojom
@@ -24,6 +24,12 @@
// worker script looking them up.
url.mojom.Url url;
+ // If non-null, then the cookies are accessed by a network transfer and
+ // |frame_origin| is the origin of the frame executing the fetch. May be null
+ // for network transfers whose IsolationInfo's `frame_origin()` is
+ // std::nullopt.
+ url.mojom.Origin? frame_origin;
+
// The top frame origin for the given request, we use top_frame_origin from
// net::IsolationInfo.
url.mojom.Origin top_frame_origin;
diff --git a/services/network/restricted_cookie_manager.cc b/services/network/restricted_cookie_manager.cc
index f29b11f..29d3589 100644
--- a/services/network/restricted_cookie_manager.cc
+++ b/services/network/restricted_cookie_manager.cc
@@ -663,7 +663,8 @@
if (cookie_observer_ && !on_cookies_accessed_result.empty()) {
OnCookiesAccessed(mojom::CookieAccessDetails::New(
- mojom::CookieAccessDetails::Type::kRead, url, isolated_top_frame_origin,
+ mojom::CookieAccessDetails::Type::kRead, url,
+ /*frame_origin=*/std::nullopt, isolated_top_frame_origin,
site_for_cookies, std::move(on_cookies_accessed_result), std::nullopt,
is_ad_tagged, cookie_setting_overrides));
}
@@ -770,9 +771,9 @@
net::CookieAccessResult(status)));
OnCookiesAccessed(mojom::CookieAccessDetails::New(
mojom::CookieAccessDetails::Type::kChange, url,
- isolated_top_frame_origin, site_for_cookies,
- std::move(result_with_access_result), std::nullopt, is_ad_tagged,
- cookie_setting_overrides));
+ /*frame_origin=*/std::nullopt, isolated_top_frame_origin,
+ site_for_cookies, std::move(result_with_access_result), std::nullopt,
+ is_ad_tagged, cookie_setting_overrides));
}
std::move(callback).Run(false);
return;
@@ -893,8 +894,9 @@
mojom::CookieOrLine::NewCookie(cookie), access_result));
OnCookiesAccessed(mojom::CookieAccessDetails::New(
mojom::CookieAccessDetails::Type::kChange, url,
- isolated_top_frame_origin, site_for_cookies, std::move(notify),
- std::nullopt, is_ad_tagged, cookie_setting_overrides));
+ /*frame_origin=*/std::nullopt, isolated_top_frame_origin,
+ site_for_cookies, std::move(notify), std::nullopt, is_ad_tagged,
+ cookie_setting_overrides));
}
}
std::move(user_callback).Run(access_result.status.IsInclude());
@@ -979,6 +981,7 @@
net::CookieAccessResult(status)));
OnCookiesAccessed(mojom::CookieAccessDetails::New(
mojom::CookieAccessDetails::Type::kChange, url,
+ /*frame_origin=*/std::nullopt,
isolation_info_.top_frame_origin().value_or(url::Origin()),
site_for_cookies, std::move(result_with_access_result), std::nullopt,
is_ad_tagged,
diff --git a/services/network/url_loader.cc b/services/network/url_loader.cc
index ede0531..5450162 100644
--- a/services/network/url_loader.cc
+++ b/services/network/url_loader.cc
@@ -3121,6 +3121,7 @@
if (!reported_cookies.empty()) {
cookie_access_details_.emplace_back(mojom::CookieAccessDetails::New(
mojom::CookieAccessDetails::Type::kRead, url_request_->url(),
+ url_request_->isolation_info().frame_origin(),
url_request_->isolation_info().top_frame_origin().value_or(
url::Origin()),
url_request_->site_for_cookies(), std::move(reported_cookies),
@@ -3456,6 +3457,7 @@
if (!reported_cookies.empty()) {
cookie_access_details_.emplace_back(mojom::CookieAccessDetails::New(
mojom::CookieAccessDetails::Type::kChange, url_request_->url(),
+ url_request_->isolation_info().frame_origin(),
url_request_->isolation_info().top_frame_origin().value_or(
url::Origin()),
url_request_->site_for_cookies(), std::move(reported_cookies),
diff --git a/third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom b/third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom
index cf47a7a..c58e7000 100644
--- a/third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom
+++ b/third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom
@@ -4842,6 +4842,7 @@
kSummarizer_OutputLanguage = 5456,
kSummarizer_SharedContext = 5457,
kSummarizer_Type = 5458,
+ kCrossOriginSameSiteCookieAccessViaStorageAccessAPI = 5459,
// Add new features immediately above this line. Don't change assigned
// numbers of any item, and don't reuse removed slots. Also don't add extra
diff --git a/tools/metrics/histograms/metadata/blink/enums.xml b/tools/metrics/histograms/metadata/blink/enums.xml
index b53d58f..28b8f58c 100644
--- a/tools/metrics/histograms/metadata/blink/enums.xml
+++ b/tools/metrics/histograms/metadata/blink/enums.xml
@@ -6082,6 +6082,7 @@
<int value="5456" label="Summarizer_OutputLanguage"/>
<int value="5457" label="Summarizer_SharedContext"/>
<int value="5458" label="Summarizer_Type"/>
+ <int value="5459" label="CrossOriginSameSiteCookieAccessViaStorageAccessAPI"/>
</enum>
<!-- LINT.ThenChange(//third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom:WebFeature) -->
