1752475 - Store distinct "default" samesite attribute value even if the laxByDefault pref is false

Status: RESOLVED FIXED

(Core :: Networking: Cookies, task, P2)

Description

[Daniel Veditz [:dveditz]]

We currently only support three values for samesite: lax, strict, and none. Since the time that code was implemented in bug 1551798 the early spec was merged into rfc6265bis and clarified. The spec now requires storing an explicit "default" value
https://www.ietf.org/archive/id/draft-ietf-httpbis-rfc6265bis-09.html#section-5.5-3.16

We can track that state, but currently we only do so if the laxByDefault pref is true. The result is that by time we turn laxByDefault on, users with old profiles have a bunch of old cookies whose samesite value is interpreted as an explicit "None". As long as those cookies are not updated this will hide sites that are broken by the "laxByDefault" setting, possibly delaying detection until after we ship and fresh users start using the broken site.

Our storage of this attribute is goofy (a "raw" and "interpreted" attribute), but we can use it to indicate "default" samesite attributes -- essentially we do that when the laxByDefault pref is set to true.

This bug proposes to change the storing of cookies to ignore the laxByDefault pref and always store the 4 values (lax, strict, none, and default) described in the spec. The use of cookies does not need to be changed because it already does the right thing based on the state of the laxByDefault pref.

I am not proposing to rewrite the storage to use a single column with 4 values at this time. That can be a later clean up, but at the moment getting rid of one of the columns would cause an older Firefox to decide the cookie database is corrupt and delete them all. Plus that would be a bigger change requiring a lot more testing and fingernail biting about regressions.

Comment 1

[Frederik Braun [:freddy] (OOO until August 18th)]

Attachment: Bug 1752475 - Change nsICookie/CookieStruct to implicitly flip SameSite in getter func r=dragana!,dveditz

Comment 2

[Tom S. (please needinfo tschuster)]

Attachment: WIP: Bug 1752475 - Review updates

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0acc23548adf
Change nsICookie/CookieStruct to implicitly flip SameSite in getter func r=dveditz,dragana

Comment 4

[Natalia Csoregi [:nataliaCs]]

Backed out for causing multiple cookies failures.

• backout: https://hg.mozilla.org/integration/autoland/rev/615bf7a32f729d58f65cdbdfd8d0ccc81dde1199
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=0acc23548adff4283f954d9e433d41f8a1bc6ece
• push with more failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=e881ef466324323602dbb3dac1882dc732ec7388&searchStr=mochitest-plain
• failure logs:
  • TEST-UNEXPECTED-FAIL | toolkit/components/url-classifier/tests/mochitest/test_classify_by_default.html | Tracker request img-srcreceived with cookie
  • TEST-UNEXPECTED-FAIL | devtools/client/webconsole/test/browser/browser_webconsole_warning_group_cookies.js | Test timed out -

Comment 5

[Tom Schuster (MoCo) [OoO]]

Sorry, updated the patch.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b09a364bdf81
Change nsICookie/CookieStruct to implicitly flip SameSite in getter func r=dveditz,dragana,freddyb

Comment 7

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/b09a364bdf81