Bug 1763073 - Add telemetry (and messaging) for SameSite cookies blocked due to redirects. r=freddyb,dveditz

Differential Revision: https://phabricator.services.mozilla.com/D143034
This commit is contained in:
Tom Schuster 2022-06-15 14:51:16 +00:00
Родитель 87b5fee4a6
Коммит 934b379383
11 изменённых файлов: 138 добавлений и 36 удалений

Просмотреть файл

@ -84,6 +84,10 @@ class Cookie final : public nsICookie {
}
inline int32_t SameSite() const { return mData.sameSite(); }
inline int32_t RawSameSite() const { return mData.rawSameSite(); }
inline bool IsDefaultSameSite() const {
return SameSite() == nsICookie::SAMESITE_LAX &&
RawSameSite() == nsICookie::SAMESITE_NONE;
}
inline uint8_t SchemeMap() const { return mData.schemeMap(); }
// setters

Просмотреть файл

@ -518,7 +518,10 @@ bool IsSameSiteSchemeEqual(const nsACString& aFirstScheme,
return aFirstScheme.Equals(aSecondScheme);
}
bool CookieCommons::IsSameSiteForeign(nsIChannel* aChannel, nsIURI* aHostURI) {
bool CookieCommons::IsSameSiteForeign(nsIChannel* aChannel, nsIURI* aHostURI,
bool* aHadCrossSiteRedirects) {
*aHadCrossSiteRedirects = false;
if (!aChannel) {
return false;
}
@ -595,6 +598,7 @@ bool CookieCommons::IsSameSiteForeign(nsIChannel* aChannel, nsIURI* aHostURI) {
rv = redirectPrincipal->IsThirdPartyChannel(aChannel, &isForeign);
// if at any point we encounter a cross-origin redirect we can return.
if (NS_FAILED(rv) || isForeign) {
*aHadCrossSiteRedirects = true;
return true;
}
@ -603,6 +607,7 @@ bool CookieCommons::IsSameSiteForeign(nsIChannel* aChannel, nsIURI* aHostURI) {
if (!IsSameSiteSchemeEqual(redirectScheme, hostScheme)) {
// If the two schemes are not of the same http(s) scheme then we
// consider the request as foreign.
*aHadCrossSiteRedirects = true;
return true;
}
}

Просмотреть файл

@ -127,8 +127,11 @@ class CookieCommons final {
// Returns true if the channel is a foreign with respect to the host-uri.
// For loads of TYPE_DOCUMENT, this function returns true if it's a cross
// origin navigation.
static bool IsSameSiteForeign(nsIChannel* aChannel, nsIURI* aHostURI);
// site navigation.
// `aHadCrossSiteRedirects` will be true iff the channel had a cross-site
// redirect before the final URI.
static bool IsSameSiteForeign(nsIChannel* aChannel, nsIURI* aHostURI,
bool* aHadCrossSiteRedirects);
};
} // namespace net

Просмотреть файл

@ -152,22 +152,25 @@ bool ProcessSameSiteCookieForForeignRequest(nsIChannel* aChannel,
Cookie* aCookie,
bool aIsSafeTopLevelNav,
bool aLaxByDefault) {
int32_t sameSiteAttr = 0;
aCookie->GetSameSite(&sameSiteAttr);
// it if's a cross origin request and the cookie is same site only (strict)
// don't send it
if (sameSiteAttr == nsICookie::SAMESITE_STRICT) {
// If it's a cross-site request and the cookie is same site only (strict)
// don't send it.
if (aCookie->SameSite() == nsICookie::SAMESITE_STRICT) {
return false;
}
// Explicit SameSite=None cookies are always processed. When laxByDefault
// is OFF then so are default cookies.
if (aCookie->SameSite() == nsICookie::SAMESITE_NONE ||
(!aLaxByDefault && aCookie->IsDefaultSameSite())) {
return true;
}
int64_t currentTimeInUsec = PR_Now();
// 2 minutes of tolerance for 'SameSite=Lax by default' for cookies set
// without a SameSite value when used for unsafe http methods.
if (StaticPrefs::network_cookie_sameSite_laxPlusPOST_timeout() > 0 &&
aLaxByDefault && sameSiteAttr == nsICookie::SAMESITE_LAX &&
aCookie->RawSameSite() == nsICookie::SAMESITE_NONE &&
if (aLaxByDefault && aCookie->IsDefaultSameSite() &&
StaticPrefs::network_cookie_sameSite_laxPlusPOST_timeout() > 0 &&
currentTimeInUsec - aCookie->CreationTime() <=
(StaticPrefs::network_cookie_sameSite_laxPlusPOST_timeout() *
PR_USEC_PER_SEC) &&
@ -175,9 +178,11 @@ bool ProcessSameSiteCookieForForeignRequest(nsIChannel* aChannel,
return true;
}
// if it's a cross origin request, the cookie is same site lax, but it's not a
// top-level navigation, don't send it
return sameSiteAttr != nsICookie::SAMESITE_LAX || aIsSafeTopLevelNav;
MOZ_ASSERT((aLaxByDefault && aCookie->IsDefaultSameSite()) ||
aCookie->SameSite() == nsICookie::SAMESITE_LAX);
// We only have SameSite=Lax or lax-by-default cookies at this point. These
// are processed only if it's a top-level navigation
return aIsSafeTopLevelNav;
}
} // namespace
@ -486,7 +491,9 @@ CookieService::GetCookieStringFromHttp(nsIURI* aHostURI, nsIChannel* aChannel,
aChannel, attrs, StoragePrincipalHelper::eStorageAccessPrincipal);
bool isSafeTopLevelNav = CookieCommons::IsSafeTopLevelNav(aChannel);
bool isSameSiteForeign = CookieCommons::IsSameSiteForeign(aChannel, aHostURI);
bool hadCrossSiteRedirects = false;
bool isSameSiteForeign = CookieCommons::IsSameSiteForeign(
aChannel, aHostURI, &hadCrossSiteRedirects);
AutoTArray<Cookie*, 8> foundCookieList;
GetCookiesForURI(
@ -494,8 +501,8 @@ CookieService::GetCookieStringFromHttp(nsIURI* aHostURI, nsIChannel* aChannel,
result.contains(ThirdPartyAnalysis::IsThirdPartyTrackingResource),
result.contains(ThirdPartyAnalysis::IsThirdPartySocialTrackingResource),
result.contains(ThirdPartyAnalysis::IsStorageAccessPermissionGranted),
rejectedReason, isSafeTopLevelNav, isSameSiteForeign, true, attrs,
foundCookieList);
rejectedReason, isSafeTopLevelNav, isSameSiteForeign,
hadCrossSiteRedirects, true, attrs, foundCookieList);
ComposeCookieString(foundCookieList, aCookieString);
@ -895,12 +902,26 @@ CookieService::RemoveNative(const nsACString& aHost, const nsACString& aName,
return NS_OK;
}
enum class CookieProblem : uint32_t {
None = 0,
// Same-Site cookies (default or explicit) blocked due to redirect
RedirectDefault = 1 << 0,
RedirectExplicit = 1 << 1,
// Special case for googleads Same-Site cookies
RedirectGoogleAds = 1 << 2,
// Blocked due to other reasons
OtherDefault = 1 << 3,
OtherExplicit = 1 << 4
};
MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(CookieProblem)
void CookieService::GetCookiesForURI(
nsIURI* aHostURI, nsIChannel* aChannel, bool aIsForeign,
bool aIsThirdPartyTrackingResource,
bool aIsThirdPartySocialTrackingResource,
bool aStorageAccessPermissionGranted, uint32_t aRejectedReason,
bool aIsSafeTopLevelNav, bool aIsSameSiteForeign, bool aHttpBound,
bool aIsSafeTopLevelNav, bool aIsSameSiteForeign,
bool aHadCrossSiteRedirects, bool aHttpBound,
const OriginAttributes& aOriginAttrs, nsTArray<Cookie*>& aCookieList) {
NS_ASSERTION(aHostURI, "null host!");
@ -1003,6 +1024,13 @@ void CookieService::GetCookiesForURI(
!nsContentUtils::IsURIInPrefList(
aHostURI, "network.cookie.sameSite.laxByDefault.disabledHosts");
// We are counting blocked SameSite cookies to get an idea of
// potential website breakage before we reintroduce "laxByDefault".
// Special attention is paid to redirects where our behavior
// differs from Chrome (bug 1763073, crbug/1221316).
//
CookieProblem sameSiteProblems = CookieProblem::None;
// iterate the cookies!
for (Cookie* cookie : *cookies) {
// check the host, since the base domain lookup is conservative.
@ -1015,12 +1043,6 @@ void CookieService::GetCookiesForURI(
continue;
}
if (aHttpBound && aIsSameSiteForeign &&
!ProcessSameSiteCookieForForeignRequest(
aChannel, cookie, aIsSafeTopLevelNav, laxByDefault)) {
continue;
}
// if the cookie is httpOnly and it's not going directly to the HTTP
// connection, don't send it
if (cookie->IsHttpOnly() && !aHttpBound) {
@ -1037,6 +1059,49 @@ void CookieService::GetCookiesForURI(
continue;
}
if (aHttpBound && aIsSameSiteForeign) {
bool blockCookie = !ProcessSameSiteCookieForForeignRequest(
aChannel, cookie, aIsSafeTopLevelNav, laxByDefault);
// Record telemetry for blocked sameSite cookies. If laxByDefault is off,
// re-run the check to see if we would get different results with it
// turned on.
if (blockCookie || (!laxByDefault && cookie->IsDefaultSameSite() &&
!ProcessSameSiteCookieForForeignRequest(
aChannel, cookie, aIsSafeTopLevelNav, true))) {
if (aHadCrossSiteRedirects) {
// Count the known problematic Google cookies from
// adservice.google.{com, de} etc. separately. This is not an exact
// domain match, but good enough for telemetry.
if (StringBeginsWith(hostFromURI, "adservice.google."_ns)) {
sameSiteProblems |= CookieProblem::RedirectGoogleAds;
} else {
if (cookie->IsDefaultSameSite()) {
sameSiteProblems |= CookieProblem::RedirectDefault;
} else {
sameSiteProblems |= CookieProblem::RedirectExplicit;
}
}
} else {
if (cookie->IsDefaultSameSite()) {
sameSiteProblems |= CookieProblem::OtherDefault;
} else {
sameSiteProblems |= CookieProblem::OtherExplicit;
}
}
}
if (blockCookie) {
CookieLogging::LogMessageToConsole(
crc, aHostURI, nsIScriptError::warningFlag,
CONSOLE_REJECTION_CATEGORY, "CookieBlockedCrossSiteRedirect"_ns,
AutoTArray<nsString, 1>{
NS_ConvertUTF8toUTF16(cookie->Name()),
});
continue;
}
}
// all checks passed - add to list and check if lastAccessed stamp needs
// updating
aCookieList.AppendElement(cookie);
@ -1045,6 +1110,9 @@ void CookieService::GetCookiesForURI(
}
}
Telemetry::Accumulate(Telemetry::COOKIE_RETRIEVAL_SAMESITE_PROBLEM,
static_cast<uint32_t>(sameSiteProblems));
if (aCookieList.IsEmpty()) {
return;
}

Просмотреть файл

@ -84,8 +84,8 @@ class CookieService final : public nsICookieService,
bool aIsThirdPartySocialTrackingResource,
bool aStorageAccessPermissionGranted,
uint32_t aRejectedReason, bool aIsSafeTopLevelNav,
bool aIsSameSiteForeign, bool aHttpBound,
const OriginAttributes& aOriginAttrs,
bool aIsSameSiteForeign, bool aHadCrossSiteRedirects,
bool aHttpBound, const OriginAttributes& aOriginAttrs,
nsTArray<Cookie*>& aCookieList);
/**

Просмотреть файл

@ -145,13 +145,16 @@ void CookieServiceChild::TrackCookieLoad(nsIChannel* aChannel) {
aChannel, attrs);
bool isSafeTopLevelNav = CookieCommons::IsSafeTopLevelNav(aChannel);
bool isSameSiteForeign = CookieCommons::IsSameSiteForeign(aChannel, uri);
bool hadCrossSiteRedirects = false;
bool isSameSiteForeign =
CookieCommons::IsSameSiteForeign(aChannel, uri, &hadCrossSiteRedirects);
SendPrepareCookieList(
uri, result.contains(ThirdPartyAnalysis::IsForeign),
result.contains(ThirdPartyAnalysis::IsThirdPartyTrackingResource),
result.contains(ThirdPartyAnalysis::IsThirdPartySocialTrackingResource),
result.contains(ThirdPartyAnalysis::IsStorageAccessPermissionGranted),
rejectedReason, isSafeTopLevelNav, isSameSiteForeign, attrs);
rejectedReason, isSafeTopLevelNav, isSameSiteForeign,
hadCrossSiteRedirects, attrs);
}
IPCResult CookieServiceChild::RecvRemoveAll() {

Просмотреть файл

@ -82,7 +82,9 @@ void CookieServiceParent::TrackCookieLoad(nsIChannel* aChannel) {
nsCOMPtr<nsILoadInfo> loadInfo = aChannel->LoadInfo();
OriginAttributes attrs = loadInfo->GetOriginAttributes();
bool isSafeTopLevelNav = CookieCommons::IsSafeTopLevelNav(aChannel);
bool aIsSameSiteForeign = CookieCommons::IsSameSiteForeign(aChannel, uri);
bool hadCrossSiteRedirects = false;
bool isSameSiteForeign =
CookieCommons::IsSameSiteForeign(aChannel, uri, &hadCrossSiteRedirects);
StoragePrincipalHelper::PrepareEffectiveStoragePrincipalOriginAttributes(
aChannel, attrs);
@ -101,8 +103,8 @@ void CookieServiceParent::TrackCookieLoad(nsIChannel* aChannel) {
result.contains(ThirdPartyAnalysis::IsThirdPartyTrackingResource),
result.contains(ThirdPartyAnalysis::IsThirdPartySocialTrackingResource),
result.contains(ThirdPartyAnalysis::IsStorageAccessPermissionGranted),
rejectedReason, isSafeTopLevelNav, aIsSameSiteForeign, false, attrs,
foundCookieList);
rejectedReason, isSafeTopLevelNav, isSameSiteForeign,
hadCrossSiteRedirects, false, attrs, foundCookieList);
nsTArray<CookieStruct> matchingCookiesList;
SerialializeCookieList(foundCookieList, matchingCookiesList);
Unused << SendTrackCookiesLoad(matchingCookiesList, attrs);
@ -129,7 +131,8 @@ IPCResult CookieServiceParent::RecvPrepareCookieList(
const bool& aIsThirdPartySocialTrackingResource,
const bool& aStorageAccessPermissionGranted,
const uint32_t& aRejectedReason, const bool& aIsSafeTopLevelNav,
const bool& aIsSameSiteForeign, const OriginAttributes& aAttrs) {
const bool& aIsSameSiteForeign, const bool& aHadCrossSiteRedirects,
const OriginAttributes& aAttrs) {
// Send matching cookies to Child.
if (!aHost) {
return IPC_FAIL(this, "aHost must not be null");
@ -142,8 +145,8 @@ IPCResult CookieServiceParent::RecvPrepareCookieList(
mCookieService->GetCookiesForURI(
aHost, nullptr, aIsForeign, aIsThirdPartyTrackingResource,
aIsThirdPartySocialTrackingResource, aStorageAccessPermissionGranted,
aRejectedReason, aIsSafeTopLevelNav, aIsSameSiteForeign, false, aAttrs,
foundCookieList);
aRejectedReason, aIsSafeTopLevelNav, aIsSameSiteForeign,
aHadCrossSiteRedirects, false, aAttrs, foundCookieList);
nsTArray<CookieStruct> matchingCookiesList;
SerialializeCookieList(foundCookieList, matchingCookiesList);
Unused << SendTrackCookiesLoad(matchingCookiesList, aAttrs);

Просмотреть файл

@ -56,7 +56,8 @@ class CookieServiceParent : public PCookieServiceParent {
const bool& aIsThirdPartySocialTrackingResource,
const bool& aStorageAccessPermissionGranted,
const uint32_t& aRejectedReason, const bool& aIsSafeTopLevelNav,
const bool& aIsSameSiteForeign, const OriginAttributes& aAttrs);
const bool& aIsSameSiteForeign, const bool& aHadCrossSiteRedirects,
const OriginAttributes& aAttrs);
static void SerialializeCookieList(const nsTArray<Cookie*>& aFoundCookieList,
nsTArray<CookieStruct>& aCookiesList);

Просмотреть файл

@ -46,6 +46,7 @@ parent:
uint32_t rejectedReason,
bool isSafeTopLevelNav,
bool isSameSiteForeign,
bool hadCrossSiteRedirects,
OriginAttributes attrs);
async __delete__();

Просмотреть файл

@ -85,5 +85,8 @@ CookieRejectedExpired=Cookie “%1$S” has been rejected because it is already
# LOCALIZATION NOTE (CookieRejectedForNonSameSiteness): %1$S is the cookie name.
CookieRejectedForNonSameSiteness=Cookie “%1$S” has been rejected because it is in a cross-site context and its “SameSite” is “Lax” or “Strict”.
# LOCALIZATION NOTE (CookieBlockedCrossSiteRedirect): %1$S is the cookie name. Do not translate "SameSite", "Lax" or "Strict".
CookieBlockedCrossSiteRedirect=Cookie “%1$S” with the “SameSite” attribute value “Lax” or “Strict” was omitted because of a cross-site redirect.
# LOCALIZATION NOTE (APIDeprecationWarning): %1$S is the deprecated API; %2$S is the API function that should be used.
APIDeprecationWarning=Warning: %1$S deprecated, please use %2$S

Просмотреть файл

@ -16614,6 +16614,17 @@
"n_buckets": 56,
"description": "How much time (in hours) passed between the current cookie purging activity and the one before that (cookie purging is run on 'daily idle')"
},
"COOKIE_RETRIEVAL_SAMESITE_PROBLEM": {
"record_in_processes": ["main", "content"],
"products": ["firefox"],
"alert_emails": ["fbraun@mozilla.com", "tschuster@mozilla.com", "seceng-telemetry@mozilla.com"],
"bug_numbers": [1763367],
"expires_in_version": "106",
"kind" : "enumerated",
"n_values": 32,
"description": "Whether a cookie was skipped in GetCookiesForURI because of a same-site issue. This is a bit field.",
"releaseChannelCollection": "opt-out"
},
"REFERRER_POLICY_COUNT" : {
"products": ["firefox"],
"record_in_processes": ["main"],