diff --git a/devtools/client/webconsole/test/browser/browser_webconsole_warning_group_cookies.js b/devtools/client/webconsole/test/browser/browser_webconsole_warning_group_cookies.js index 40ae1d89eddc..7b4d047e4151 100644 --- a/devtools/client/webconsole/test/browser/browser_webconsole_warning_group_cookies.js +++ b/devtools/client/webconsole/test/browser/browser_webconsole_warning_group_cookies.js @@ -38,10 +38,10 @@ add_task(async function testSameSiteCookieMessage() { groupLabel: "Some cookies are misusing the recommended “SameSite“ attribute", message1: - "Cookie “a” will be soon rejected because it has the “SameSite” attribute set to “None” or an invalid value, without the “secure” attribute.", + "Cookie “a” does not have a proper “SameSite” attribute value. Soon, cookies without the “SameSite” attribute or with an invalid value will be treated as “Lax”. This means that the cookie will no longer be sent in third-party contexts. If your application depends on this cookie being available in such contexts, please add the “SameSite=None“ attribute to it. To know more about the “SameSite“ attribute, read https://developer.mozilla.org/docs/Web/HTTP/Headers/Set-Cookie/SameSite", typeMessage1: ".warn", message2: - "Cookie “b” will be soon rejected because it has the “SameSite” attribute set to “None” or an invalid value, without the “secure” attribute.", + "Cookie “b” does not have a proper “SameSite” attribute value. Soon, cookies without the “SameSite” attribute or with an invalid value will be treated as “Lax”. This means that the cookie will no longer be sent in third-party contexts. If your application depends on this cookie being available in such contexts, please add the “SameSite=None“ attribute to it. To know more about the “SameSite“ attribute, read https://developer.mozilla.org/docs/Web/HTTP/Headers/Set-Cookie/SameSite", }, ]; diff --git a/dom/security/test/https-first/test_toplevel_cookies.html b/dom/security/test/https-first/test_toplevel_cookies.html index 74005cc33fe1..2c0c64db469f 100644 --- a/dom/security/test/https-first/test_toplevel_cookies.html +++ b/dom/security/test/https-first/test_toplevel_cookies.html @@ -108,8 +108,7 @@ async function runTest() { SpecialPowers.pushPrefEnv({ set: [ ["dom.security.https_first", true], - // Bug 1617611: Fix all the tests broken by "cookies SameSite=lax by default" - ["network.cookie.sameSite.laxByDefault", false], + ["network.cookie.sameSite.noneRequiresSecure", false], ]}, runTest); diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index e2f3487c5518..0a7cdbcab256 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -9844,7 +9844,7 @@ mirror: always - name: network.cookie.sameSite.laxByDefault - type: bool + type: RelaxedAtomicBool value: @IS_EARLY_BETA_OR_EARLIER@ mirror: always diff --git a/netwerk/cookie/Cookie.cpp b/netwerk/cookie/Cookie.cpp index e83a2fcae212..6f3b4e4f2f97 100644 --- a/netwerk/cookie/Cookie.cpp +++ b/netwerk/cookie/Cookie.cpp @@ -4,6 +4,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "Cookie.h" +#include "CookieStorage.h" #include "mozilla/Encoding.h" #include "mozilla/dom/ToJSValue.h" #include "mozilla/StaticPrefs_network.h" @@ -56,14 +57,10 @@ already_AddRefed Cookie::Create( gLastCreationTime = cookie->mData.creationTime(); } - // If sameSite is not a sensible value, assume strict - if (cookie->mData.sameSite() < 0 || - cookie->mData.sameSite() > nsICookie::SAMESITE_STRICT) { - cookie->mData.sameSite() = nsICookie::SAMESITE_STRICT; - } - - // If rawSameSite is not a sensible value, assume equal to sameSite. - if (!Cookie::ValidateRawSame(cookie->mData)) { + // If sameSite/rawSameSite values aren't sensible reset to Default + // cf. 5.4.7 in draft-ietf-httpbis-rfc6265bis-09 + if (!Cookie::ValidateSameSite(cookie->mData)) { + cookie->mData.sameSite() = nsICookie::SAMESITE_LAX; cookie->mData.rawSameSite() = nsICookie::SAMESITE_NONE; } @@ -204,9 +201,16 @@ Cookie::GetExpires(uint64_t* aExpires) { } // static -bool Cookie::ValidateRawSame(const CookieStruct& aCookieData) { - return aCookieData.rawSameSite() == aCookieData.sameSite() || - aCookieData.rawSameSite() == nsICookie::SAMESITE_NONE; +bool Cookie::ValidateSameSite(const CookieStruct& aCookieData) { + // For proper migration towards a laxByDefault world, + // sameSite is initialized to LAX even though the server + // has never sent it. + if (aCookieData.rawSameSite() == aCookieData.sameSite()) { + return aCookieData.rawSameSite() >= nsICookie::SAMESITE_NONE && + aCookieData.rawSameSite() <= nsICookie::SAMESITE_STRICT; + } + return aCookieData.rawSameSite() == nsICookie::SAMESITE_NONE && + aCookieData.sameSite() == nsICookie::SAMESITE_LAX; } already_AddRefed Cookie::Clone() const { diff --git a/netwerk/cookie/Cookie.h b/netwerk/cookie/Cookie.h index 94a49db7c65b..51d9d1673c70 100644 --- a/netwerk/cookie/Cookie.h +++ b/netwerk/cookie/Cookie.h @@ -46,7 +46,7 @@ class Cookie final : public nsICookie { public: // Returns false if rawSameSite has an invalid value, compared to sameSite. - static bool ValidateRawSame(const CookieStruct& aCookieData); + static bool ValidateSameSite(const CookieStruct& aCookieData); // Generate a unique and monotonically increasing creation time. See comment // in Cookie.cpp. diff --git a/netwerk/cookie/CookieService.cpp b/netwerk/cookie/CookieService.cpp index 4d32c51767a0..8d7db43afbad 100644 --- a/netwerk/cookie/CookieService.cpp +++ b/netwerk/cookie/CookieService.cpp @@ -1194,7 +1194,13 @@ bool CookieService::CanSetCookie( // If the new cookie is same-site but in a cross site context, // browser must ignore the cookie. - if ((aCookieData.sameSite() != nsICookie::SAMESITE_NONE) && + bool laxByDefault = + StaticPrefs::network_cookie_sameSite_laxByDefault() && + !nsContentUtils::IsURIInPrefList( + aHostURI, "network.cookie.sameSite.laxByDefault.disabledHosts"); + auto effectiveSameSite = + laxByDefault ? aCookieData.sameSite() : aCookieData.rawSameSite(); + if ((effectiveSameSite != nsICookie::SAMESITE_NONE) && aIsForeignAndNotAddon) { COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, savedCookieHeader, "failed the samesite tests"); @@ -1363,14 +1369,17 @@ bool CookieService::GetTokenValue(nsACString::const_char_iterator& aIter, return false; } -static inline void SetSameSiteDefaultAttribute(CookieStruct& aCookieData, - bool laxByDefault) { +static inline void SetSameSiteAttributeDefault(CookieStruct& aCookieData) { + // Set cookie with SameSite attribute that is treated as Default + // and doesn't requires changing the DB schema. + aCookieData.sameSite() = nsICookie::SAMESITE_LAX; aCookieData.rawSameSite() = nsICookie::SAMESITE_NONE; - if (laxByDefault) { - aCookieData.sameSite() = nsICookie::SAMESITE_LAX; - } else { - aCookieData.sameSite() = nsICookie::SAMESITE_NONE; - } +} + +static inline void SetSameSiteAttribute(CookieStruct& aCookieData, + int32_t aValue) { + aCookieData.sameSite() = aValue; + aCookieData.rawSameSite() = aValue; } // Parses attributes from cookie header. expires/max-age attributes aren't @@ -1403,11 +1412,7 @@ bool CookieService::ParseAttributes(nsIConsoleReportCollector* aCRC, aCookieData.isSecure() = false; aCookieData.isHttpOnly() = false; - bool laxByDefault = - StaticPrefs::network_cookie_sameSite_laxByDefault() && - !nsContentUtils::IsURIInPrefList( - aHostURI, "network.cookie.sameSite.laxByDefault.disabledHosts"); - SetSameSiteDefaultAttribute(aCookieData, laxByDefault); + SetSameSiteAttributeDefault(aCookieData); nsDependentCSubstring tokenString(cookieStart, cookieStart); nsDependentCSubstring tokenValue(cookieStart, cookieStart); @@ -1457,17 +1462,14 @@ bool CookieService::ParseAttributes(nsIConsoleReportCollector* aCRC, } else if (tokenString.LowerCaseEqualsLiteral(kSameSite)) { if (tokenValue.LowerCaseEqualsLiteral(kSameSiteLax)) { - aCookieData.sameSite() = nsICookie::SAMESITE_LAX; - aCookieData.rawSameSite() = nsICookie::SAMESITE_LAX; + SetSameSiteAttribute(aCookieData, nsICookie::SAMESITE_LAX); } else if (tokenValue.LowerCaseEqualsLiteral(kSameSiteStrict)) { - aCookieData.sameSite() = nsICookie::SAMESITE_STRICT; - aCookieData.rawSameSite() = nsICookie::SAMESITE_STRICT; + SetSameSiteAttribute(aCookieData, nsICookie::SAMESITE_STRICT); } else if (tokenValue.LowerCaseEqualsLiteral(kSameSiteNone)) { - aCookieData.sameSite() = nsICookie::SAMESITE_NONE; - aCookieData.rawSameSite() = nsICookie::SAMESITE_NONE; + SetSameSiteAttribute(aCookieData, nsICookie::SAMESITE_NONE); } else { - // Reset to defaults if unknown token value (see Bug 1682450) - SetSameSiteDefaultAttribute(aCookieData, laxByDefault); + // Reset to Default if unknown token value (see Bug 1682450) + SetSameSiteAttributeDefault(aCookieData); CookieLogging::LogMessageToConsole( aCRC, aHostURI, nsIScriptError::infoFlag, CONSOLE_SAMESITE_CATEGORY, "CookieSameSiteValueInvalid2"_ns, @@ -1479,29 +1481,32 @@ bool CookieService::ParseAttributes(nsIConsoleReportCollector* aCRC, // re-assign aCookieHeader, in case we need to process another cookie aCookieHeader.Assign(Substring(cookieStart, cookieEnd)); - // If same-site is set to 'none' but this is not a secure context, let's abort - // the parsing. + // If same-site is explicitly set to 'none' but this is not a secure context, + // let's abort the parsing. if (!aCookieData.isSecure() && aCookieData.sameSite() == nsICookie::SAMESITE_NONE) { - if (laxByDefault && - StaticPrefs::network_cookie_sameSite_noneRequiresSecure()) { + if (StaticPrefs::network_cookie_sameSite_noneRequiresSecure()) { CookieLogging::LogMessageToConsole( - aCRC, aHostURI, nsIScriptError::infoFlag, CONSOLE_SAMESITE_CATEGORY, + aCRC, aHostURI, nsIScriptError::errorFlag, CONSOLE_SAMESITE_CATEGORY, "CookieRejectedNonRequiresSecure2"_ns, AutoTArray{NS_ConvertUTF8toUTF16(aCookieData.name())}); return newCookie; } - // if SameSite=Lax by default is disabled, we want to warn the user. + // Still warn about the missing Secure attribute when not enforcing. CookieLogging::LogMessageToConsole( aCRC, aHostURI, nsIScriptError::warningFlag, CONSOLE_SAMESITE_CATEGORY, - "CookieRejectedNonRequiresSecureForBeta2"_ns, + "CookieRejectedNonRequiresSecureForBeta3"_ns, AutoTArray{NS_ConvertUTF8toUTF16(aCookieData.name()), SAMESITE_MDN_URL}); } if (aCookieData.rawSameSite() == nsICookie::SAMESITE_NONE && aCookieData.sameSite() == nsICookie::SAMESITE_LAX) { + bool laxByDefault = + StaticPrefs::network_cookie_sameSite_laxByDefault() && + !nsContentUtils::IsURIInPrefList( + aHostURI, "network.cookie.sameSite.laxByDefault.disabledHosts"); if (laxByDefault) { CookieLogging::LogMessageToConsole( aCRC, aHostURI, nsIScriptError::infoFlag, CONSOLE_SAMESITE_CATEGORY, @@ -1519,7 +1524,7 @@ bool CookieService::ParseAttributes(nsIConsoleReportCollector* aCRC, // Cookie accepted. aAcceptedByParser = true; - MOZ_ASSERT(Cookie::ValidateRawSame(aCookieData)); + MOZ_ASSERT(Cookie::ValidateSameSite(aCookieData)); return newCookie; } diff --git a/netwerk/cookie/CookieServiceChild.cpp b/netwerk/cookie/CookieServiceChild.cpp index e2e5d2ca4bf5..fe869303511b 100644 --- a/netwerk/cookie/CookieServiceChild.cpp +++ b/netwerk/cookie/CookieServiceChild.cpp @@ -288,6 +288,7 @@ void CookieServiceChild::RecordDocumentCookie(Cookie* aCookie, cookie->Expiry() == aCookie->Expiry() && cookie->IsSecure() == aCookie->IsSecure() && cookie->SameSite() == aCookie->SameSite() && + cookie->RawSameSite() == aCookie->RawSameSite() && cookie->IsSession() == aCookie->IsSession() && cookie->IsHttpOnly() == aCookie->IsHttpOnly()) { cookie->SetLastAccessed(aCookie->LastAccessed()); diff --git a/netwerk/cookie/CookieStorage.cpp b/netwerk/cookie/CookieStorage.cpp index 9389e67f80df..606e339c7bb4 100644 --- a/netwerk/cookie/CookieStorage.cpp +++ b/netwerk/cookie/CookieStorage.cpp @@ -495,6 +495,7 @@ void CookieStorage::AddCookie(nsIConsoleReportCollector* aCRC, oldCookie->IsSession() == aCookie->IsSession() && oldCookie->IsHttpOnly() == aCookie->IsHttpOnly() && oldCookie->SameSite() == aCookie->SameSite() && + oldCookie->RawSameSite() == aCookie->RawSameSite() && oldCookie->SchemeMap() == aCookie->SchemeMap() && // We don't want to perform this optimization if the cookie is // considered stale, since in this case we would need to update the diff --git a/netwerk/cookie/test/unit/test_getCookieSince.js b/netwerk/cookie/test/unit/test_getCookieSince.js index 7a0ae9c7aa50..e96cc4ef2312 100644 --- a/netwerk/cookie/test/unit/test_getCookieSince.js +++ b/netwerk/cookie/test/unit/test_getCookieSince.js @@ -5,7 +5,7 @@ const cs = Cc["@mozilla.org/cookieService;1"].getService(Ci.nsICookieService); const cm = cs.QueryInterface(Ci.nsICookieManager); function setCookie(name, url) { - let value = `${name}=${Math.random()}; Path=/; Max-Age=1000; sameSite=none`; + let value = `${name}=${Math.random()}; Path=/; Max-Age=1000; sameSite=none; Secure`; info(`Setting cookie ${value} for ${url.spec}`); let channel = NetUtil.newChannel({ @@ -37,9 +37,6 @@ add_task(async function() { true ); - // Bug 1617611 - Fix all the tests broken by "cookies SameSite=Lax by default" - Services.prefs.setBoolPref("network.cookie.sameSite.laxByDefault", false); - await setCookie("A", Services.io.newURI("https://example.com/A/")); await sleep(); @@ -72,6 +69,4 @@ add_task(async function() { someCookies = cm.getCookiesSince(cookies[3].creationTime + 1); Assert.equal(someCookies.length, 0, "We retrieve some cookies"); - - Services.prefs.clearUserPref("network.cookie.sameSite.laxByDefault"); }); diff --git a/netwerk/locales/en-US/necko.properties b/netwerk/locales/en-US/necko.properties index c60d3a9b03e0..8ff47ab18541 100644 --- a/netwerk/locales/en-US/necko.properties +++ b/netwerk/locales/en-US/necko.properties @@ -50,8 +50,8 @@ CookieAllowedForFpiByHeuristic=Storage access automatically granted for First-Pa # LOCALIZATION NOTE(CookieRejectedNonRequiresSecure2): %1$S is the cookie name. Do not localize "SameSite=None" and "secure". CookieRejectedNonRequiresSecure2=Cookie “%1$S” rejected because it has the “SameSite=None” attribute but is missing the “secure” attribute. -# LOCALIZATION NOTE(CookieRejectedNonRequiresSecureForBeta2): %1$S is the cookie name. %2$S is a URL. Do not localize "SameSite", "SameSite=None" and "secure". -CookieRejectedNonRequiresSecureForBeta2=Cookie “%1$S” will be soon rejected because it has the “SameSite” attribute set to “None” or an invalid value, without the “secure” attribute. To know more about the “SameSite“ attribute, read %2$S +# LOCALIZATION NOTE(CookieRejectedNonRequiresSecureForBeta3): %1$S is the cookie name. %2$S is a URL. Do not localize "SameSite", "SameSite=None" and "secure". +CookieRejectedNonRequiresSecureForBeta3=Cookie “%1$S” will be soon rejected because it has the “SameSite” attribute set to “None” without the “secure” attribute. To know more about the “SameSite“ attribute, read %2$S # LOCALIZATION NOTE(CookieLaxForced2): %1$S is the cookie name. Do not localize "SameSite", "Lax" and "SameSite=Lax". CookieLaxForced2=Cookie “%1$S” has “SameSite” policy set to “Lax” because it is missing a “SameSite” attribute, and “SameSite=Lax” is the default value for this attribute. # LOCALIZATION NOTE(CookieLaxForcedForBeta2): %1$S is the cookie name. %2$S is a URL. Do not localize "SameSite", "Lax" and "SameSite=Lax", "SameSite=None". diff --git a/netwerk/test/gtest/TestCookie.cpp b/netwerk/test/gtest/TestCookie.cpp index 2b37c0a4a8b2..f65dc6ef9212 100644 --- a/netwerk/test/gtest/TestCookie.cpp +++ b/netwerk/test/gtest/TestCookie.cpp @@ -1024,7 +1024,7 @@ TEST(TestCookie, SameSiteLax) cookie = static_cast(cookies[0].get()); EXPECT_EQ(cookie->RawSameSite(), nsICookie::SAMESITE_NONE); - EXPECT_EQ(cookie->SameSite(), nsICookie::SAMESITE_NONE); + EXPECT_EQ(cookie->SameSite(), nsICookie::SAMESITE_LAX); } TEST(TestCookie, OnionSite) diff --git a/toolkit/components/url-classifier/tests/mochitest/test_classify_by_default.html b/toolkit/components/url-classifier/tests/mochitest/test_classify_by_default.html index 7b8971b09962..c10a0c62f9d8 100644 --- a/toolkit/components/url-classifier/tests/mochitest/test_classify_by_default.html +++ b/toolkit/components/url-classifier/tests/mochitest/test_classify_by_default.html @@ -94,8 +94,6 @@ async function cleanup(topLevelSite = TEST_TOP_SITE) { // Ensure we clear the stylesheet cache so that we re-classify. SpecialPowers.DOMWindowUtils.clearSharedStyleSheetCache(); - SpecialPowers.clearUserPref("network.cookie.sameSite.laxByDefault"); - await clearServiceWorker(); } @@ -110,8 +108,6 @@ async function runTests() { ["dom.serviceWorkers.exemptFromPerDomainMax", true], ["dom.serviceWorkers.enabled", true], ["dom.serviceWorkers.testing.enabled", true], - // Bug 1617611: Fix all the tests broken by "cookies SameSite=lax by default" - ["network.cookie.sameSite.laxByDefault", false], ]}); await classifierHelper.waitForInit(); diff --git a/toolkit/components/url-classifier/tests/mochitest/trackerFrame.sjs b/toolkit/components/url-classifier/tests/mochitest/trackerFrame.sjs index 2f25fd2f7475..c8942eba8617 100644 --- a/toolkit/components/url-classifier/tests/mochitest/trackerFrame.sjs +++ b/toolkit/components/url-classifier/tests/mochitest/trackerFrame.sjs @@ -31,7 +31,7 @@ function handleRequest(aRequest, aResponse) { aResponse.setHeader("Content-Type", "text/plain", false); // Prepare the cookie - aResponse.setHeader("Set-Cookie", "cookie=1234"); + aResponse.setHeader("Set-Cookie", "cookie=1234; SameSite=None; Secure"); aResponse.setHeader( "Access-Control-Allow-Origin", aRequest.getHeader("Origin"),