From e0b9da0442fc0fa4f0b4043ba2510839f721af1c Mon Sep 17 00:00:00 2001 From: Tom Schuster Date: Fri, 18 Nov 2022 11:08:45 +0000 Subject: [PATCH] Bug 1774857 - Send (only) laxByDefault cookies on boomerang-redirects. r=dveditz Differential Revision: https://phabricator.services.mozilla.com/D150253 --- modules/libpref/init/StaticPrefList.yaml | 8 ++++++ netwerk/cookie/CookieCommons.cpp | 2 +- netwerk/cookie/CookieService.cpp | 26 ++++++++++++++----- .../cookies/samesite/fetch.https.html.ini | 6 +++++ .../samesite/form-post-blank.https.html.ini | 6 +++++ .../cookies/samesite/iframe.https.html.ini | 6 +++++ .../meta/cookies/samesite/img.https.html.ini | 6 +++++ ...ultiple-samesite-attributes.https.html.ini | 6 +++++ 8 files changed, 59 insertions(+), 7 deletions(-) diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index fe254cb5b5cc..a43bba9b878e 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -10746,6 +10746,14 @@ value: 120 mirror: always +# For lax-by-default cookies ignore cross-site redirects when the final +# redirect is same-site again. +# https://github.com/httpwg/http-extensions/issues/2104 +- name: network.cookie.sameSite.laxByDefault.allowBoomerangRedirect + type: bool + value: true + mirror: always + - name: network.cookie.sameSite.noneRequiresSecure type: bool value: @IS_EARLY_BETA_OR_EARLIER@ diff --git a/netwerk/cookie/CookieCommons.cpp b/netwerk/cookie/CookieCommons.cpp index 0fb658443f99..803297dbfa25 100644 --- a/netwerk/cookie/CookieCommons.cpp +++ b/netwerk/cookie/CookieCommons.cpp @@ -610,7 +610,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; + *aHadCrossSiteRedirects = isForeign; return true; } diff --git a/netwerk/cookie/CookieService.cpp b/netwerk/cookie/CookieService.cpp index 715bbc8a0a0e..52547b546e19 100644 --- a/netwerk/cookie/CookieService.cpp +++ b/netwerk/cookie/CookieService.cpp @@ -1093,13 +1093,27 @@ void CookieService::GetCookiesForURI( } } + // Lax-by-default cookies are processed even with an intermediate + // cross-site redirect (they are treated like aIsSameSiteForeign = false). + // + // This is outside of ProcessSameSiteCookieForForeignRequest to still + // collect the same telemetry. + if (blockCookie && aHadCrossSiteRedirects && + cookie->IsDefaultSameSite() && + StaticPrefs:: + network_cookie_sameSite_laxByDefault_allowBoomerangRedirect()) { + blockCookie = false; + } + if (blockCookie) { - CookieLogging::LogMessageToConsole( - crc, aHostURI, nsIScriptError::warningFlag, - CONSOLE_REJECTION_CATEGORY, "CookieBlockedCrossSiteRedirect"_ns, - AutoTArray{ - NS_ConvertUTF8toUTF16(cookie->Name()), - }); + if (aHadCrossSiteRedirects) { + CookieLogging::LogMessageToConsole( + crc, aHostURI, nsIScriptError::warningFlag, + CONSOLE_REJECTION_CATEGORY, "CookieBlockedCrossSiteRedirect"_ns, + AutoTArray{ + NS_ConvertUTF8toUTF16(cookie->Name()), + }); + } continue; } } diff --git a/testing/web-platform/meta/cookies/samesite/fetch.https.html.ini b/testing/web-platform/meta/cookies/samesite/fetch.https.html.ini index a6f29ed3ce2a..0b1471cbd5b4 100644 --- a/testing/web-platform/meta/cookies/samesite/fetch.https.html.ini +++ b/testing/web-platform/meta/cookies/samesite/fetch.https.html.ini @@ -1,2 +1,8 @@ [fetch.https.html] prefs: [network.cookie.sameSite.laxByDefault:true, network.cookie.sameSite.noneRequiresSecure:true] + + [Cross-site redirecting to same-host fetches are cross-site] + expected: FAIL + + [Cross-site redirecting to subdomain fetches are cross-site] + expected: FAIL diff --git a/testing/web-platform/meta/cookies/samesite/form-post-blank.https.html.ini b/testing/web-platform/meta/cookies/samesite/form-post-blank.https.html.ini index bda3ee42f66d..e0de736bb869 100644 --- a/testing/web-platform/meta/cookies/samesite/form-post-blank.https.html.ini +++ b/testing/web-platform/meta/cookies/samesite/form-post-blank.https.html.ini @@ -3,3 +3,9 @@ expected: if (os == "android") and fission: [OK, TIMEOUT] if (os == "mac") and not debug: [OK, TIMEOUT] + [Cross-site redirecting to same-host top-level form POSTs are cross-site] + expected: FAIL + + [Cross-site redirecting to subdomain top-level form POSTs are cross-site] + expected: FAIL + diff --git a/testing/web-platform/meta/cookies/samesite/iframe.https.html.ini b/testing/web-platform/meta/cookies/samesite/iframe.https.html.ini index db69363d1903..4d8b8978ba7f 100644 --- a/testing/web-platform/meta/cookies/samesite/iframe.https.html.ini +++ b/testing/web-platform/meta/cookies/samesite/iframe.https.html.ini @@ -2,3 +2,9 @@ prefs: [network.cookie.sameSite.laxByDefault:true, network.cookie.sameSite.noneRequiresSecure:true] expected: if (os == "android") and fission: [OK, TIMEOUT] + [Cross-site redirecting to same-host fetches are cross-site] + expected: FAIL + + [Cross-site redirecting to subdomain fetches are cross-site] + expected: FAIL + diff --git a/testing/web-platform/meta/cookies/samesite/img.https.html.ini b/testing/web-platform/meta/cookies/samesite/img.https.html.ini index 512a9a09bcff..eed175f22e99 100644 --- a/testing/web-platform/meta/cookies/samesite/img.https.html.ini +++ b/testing/web-platform/meta/cookies/samesite/img.https.html.ini @@ -3,3 +3,9 @@ expected: if (os == "android") and fission: [OK, TIMEOUT] if (os == "mac") and not debug: [OK, TIMEOUT] + [Cross-site redirecting to same-host images are cross-site] + expected: FAIL + + [Cross-site redirecting to subdomain images are cross-site] + expected: FAIL + diff --git a/testing/web-platform/meta/cookies/samesite/multiple-samesite-attributes.https.html.ini b/testing/web-platform/meta/cookies/samesite/multiple-samesite-attributes.https.html.ini index bbe13fc91396..94b187431108 100644 --- a/testing/web-platform/meta/cookies/samesite/multiple-samesite-attributes.https.html.ini +++ b/testing/web-platform/meta/cookies/samesite/multiple-samesite-attributes.https.html.ini @@ -3,3 +3,9 @@ expected: if (os == "mac") and not debug: [OK, TIMEOUT] if (os == "android") and fission: [OK, TIMEOUT] + [Cross-site redirecting to same-host images are cross-site] + expected: FAIL + + [Cross-site redirecting to subdomain images are cross-site] + expected: FAIL +