1750972 - Re-enable same-site schemeful

Status: RESOLVED FIXED

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

Description

[Frederik Braun [:freddy]]

+++ This bug was initially created as a clone and follow-up of Bug #1748693 +++

Bug 1748693 is only a hotfix. This is the second part from bug 1748693 comment 13:

There is still the potential for cookie injection attacks from http://insecure.site.example framing https://good.site.example that "schemeful samesite" (along with "lax by default") was intended to prevent. There are several existing cookie features that https://good.site.example could be using for protection, like secure cookies and cookie prefixes, but "lax by default" is trying to help sites that haven't thought about these issues and schemeful is part of that.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Note that we've only disabled samesite schemeful on Fx96 at the moment. Is this bug indication that we should be disabling it everywhere until this is resolved?

Comment 2

[Ryan VanderMeulen [:RyanVM]]

I'm wondering if we should put it behind EARLY_BETA_OR_EARLIER for 97+ for the time-being.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Not sure if we'll be ready to do this for 98 or not, but 97 isn't going to happen.

Comment 4

[Frederik Braun [:freddy]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #1)

Note that we've only disabled samesite schemeful on Fx96 at the moment. Is this bug indication that we should be disabling it everywhere until this is resolved?

I shoulda removed this flag much earlier. This happened in bug 1751435.

Comment 5

[Frederik Braun [:freddy]]

Re-enabling this is urgent, but it's not a defect. We won't make Firefox 98 either, so marking that as wontfix.

Comment 6

[Tom Schuster (MoCo)]

We decided to re-enable network.cookie.sameSite.schemeful and network.cookie.sameSite.noneRequiresSecure even without necessarily enabling lax-by-default. This spreads the risks around multiple releases.

Comment 7

[Tom Schuster (MoCo)]

Attachment: Bug 1750972 - Enable same-site schemeful by default. r?freddyb

Depends on D150600

Comment 8

[Tom Schuster (MoCo)]

Attachment: Bug 1750972 - Remove tests setting noneRequiresSecure = true. r?freddyb

Depends on D150602

Comment 9

[Tom Schuster (MoCo)]

Attachment: Bug 1750972 - Remove the same-site schemeful experimental feature. r?#fluent-reviewers,#preferences-reviewers

Depends on D150603

Comment 10

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/b1a8adcb9099
https://hg.mozilla.org/mozilla-central/rev/3b7d15317952
https://hg.mozilla.org/mozilla-central/rev/90d51383665c

Comment 11

[Tom Schuster (MoCo)]

Release Note Request (optional, but appreciated)
[Why is this notable]: Sites using SameSite=None or different schemes for cookies might break.
[Affects Firefox for Android]: Yes
[Suggested wording]: Cookies with SameSite=None must now also specify the Secure attribute. Cookies are now only considered same-site if they use the same scheme (http or https). (This is heavily inspired by the docs in MDN below)
[Links (documentation, blog post, etc)]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite

Comment 12

[Dianna Smith [:diannaS]]

Added to 104.0a1 release notes https://www.mozilla.org/en-US/firefox/104.0a1/releasenotes/

Comment 13

[Tom Schuster (MoCo)]

In bug 1679318 we saw site breakage caused by none-requires-secure. SameSite=None is also less useful without also shipping lax-by-default, because the website can just specify no SameSite attribute at all.

On the schemeful side I realized that with bug 1756213 still missing we are actually missing at least on piece of the implementation, which we should probably ship together.

For now just backing this out completely seems like the easiest way forward.

Comment 14

[Cosmin Sabou [:CosminS]]

Backed out as requested by dev: https://hg.mozilla.org/integration/autoland/rev/56ff9d9fe4233290960d6e035a72ecef1374127f

Comment 15

[Tom Schuster (MoCo)]

Hi Dianna, this was backed out, so we should probably also update the release notes.

Comment 16

[Dianna Smith [:diannaS]]

Hi, Removed from 104.0a1 relnotes. Thank you!

Comment 17

[Cosmin Sabou [:CosminS]]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/56ff9d9fe4233290960d6e035a72ecef1374127f

Comment 18

[Tom Schuster (MoCo)]

Re-using this bug to only land same-site schemeful.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/28957fbb64d5
Enable same-site schemeful by default. r=freddyb
https://hg.mozilla.org/integration/autoland/rev/ff07f346788a
Remove the same-site schemeful experimental feature. r=preferences-reviewers,Gijs

Comment 20

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/28957fbb64d5
https://hg.mozilla.org/mozilla-central/rev/ff07f346788a

Comment 21

[Dianna Smith [:diannaS]]

:tom can you revise the relnote with the new patch?

Comment 22

[Tom Schuster (MoCo)]

This "feature" by itself is more of a bug fix and probably doesn't require a release note.