1629436 - network.cookie.sameSite.laxByDefault breaks my add-on

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect, P2)

Description

[kernp25]

Attachment: firefox_nHZs1FCEwa.png

AR:
network.cookie.sameSite.laxByDefault set to true will break my add-on.

ER:
network.cookie.sameSite.laxByDefault set to true should not affect my WebExtension.

It shows this message in add-on console:
Cookie “AS” has “sameSite” policy set to “lax” because it is missing a “sameSite” attribute, and “sameSite=lax” is the default value for this attribute.

I tried this in my add-on:

        if (name == "set-cookie") {
          let splits = responseHeaders[i].value.split("\n");
          for (let j = 0; j < splits.length; j++) {
            if (splits[j].startsWith("AS=")) {
              splits[j] = splits[j].replace("secure;", "SameSite=None; Secure;");
              break;
            }
          }
          responseHeaders[i].value = splits.join("\n");

The message will not show anymore in add-on console but it doesn't work either.
It will redirect back to https://login.yahoo.com/ after entering the password.

I also found this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1626696

Comment 1

[kernp25]

ER:
I should also not need to make any changes to the Set-Cookie header in order to get this working. It should just work for a WebExtension.
Requests made from WebExtensions should be ignored.

Comment 2

[Andrea Marchesini [:baku]]

Can you tell me the name of your addon? And can you describe why lax-by-default impacts your addon? Does it use cookies somehow (directly or indirectly)?
The best would be to have a test addon, if you have time to provide it. Thanks.

Comment 3

[kernp25]

Attachment: test.zip

Comment 4

[kernp25]

(In reply to Andrea Marchesini [:baku] from comment #2)

Can you tell me the name of your addon?

https://addons.mozilla.org/firefox/addon/yahoo-mail-notifier-we/

Comment 5

[kernp25]

Attachment: anDNmPMSgR.mp4

network.cookie.sameSite.laxByDefault is true

Comment 6

[kernp25]

Attachment: hWLf1qulXR.mp4

network.cookie.sameSite.laxByDefault is false

Comment 7

[Rob Wu [:robwu]]

The issue here is basically that the reporter expects a website to work normally when embedded in an extension. Due to SameSite cookies, this is no longer the case.

To reproduce, open any moz-extension:-document and load a http(s):-iframe in it.

This is a specific instance of the more broader question on what we consider to be first-party. Here are other related bugs:

• bug 1595652 We allowed extensions to embed http(s) frames despite the website's X-Frame-Options header.
• bug 1625599 Tracking protection - we are considering to lift tracking protection (i.e. cookie restrictions) for "requests from extensions".
• bug 1629436 (this bug) - should we treat http(s)-requests from extension pages as same-site requests?

Extension pages are extensions of the browser, so if all ancestor browsing contexts of a http(s):-frame have the moz-extension:-principal, then I believe that it would be reasonable to treat the http(s)-navigation request to be same-site.

:baku Has there been any prior discussions on this topic? What is your opinion on the desired behavior?

Comment 8

[Andrea Marchesini [:baku]]

Attachment: Bug 1629436 - requests with webExtension loading principal are not 3rd party, r?ckerschb

Comment 9

[Andrea Marchesini [:baku]]

Attachment: Bug 1629436 - requests with webExtension loading principal are not 3rd party - tests, r?robwu

Depends on D73027

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:mixedpuppy, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 11

[Andrea Marchesini [:baku]]

I think this is P2. We already have a fix in review. mixedpuppy do you agree?

Comment 12

[Rob Wu [:robwu]]

That's fine. We often use P1 or P2 when there are patches already.

Comment 13

[Miruna Curtean]

Changing status with the mention that this was reproduced on latest FF updates (75.0 - 20200403170909, 76.0 -20200427162639 and 77.0a1 - 20200430082621) on Windows PRO 64-bit and macOS 10.15.4 Catalina.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d14b4bb75db7
requests with webExtension loading principal are not 3rd party, r=ckerschb,robwu,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/18d21d5f63dd
requests with webExtension loading principal are not 3rd party - tests, r=robwu

Comment 15

[Andrei Ciure[:aciure]]

Backed out 2 changesets (bug 1629436) for causing test_chrome_ext_trackingprotection.html failures
https://hg.mozilla.org/integration/autoland/rev/815b76e7c256237e5ca73a5d46bcbd6d5961ac3e

push that caused the backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=18d21d5f63ddfded78b05a45ff1fc64db49bf803&test_paths=est_chrome_ext_trackingprotection.html

log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=300422253&repo=autoland&lineNumber=110854
[task 2020-05-01T20:01:21.744Z] 20:01:21 INFO - TEST-PASS | toolkit/components/extensions/test/mochitest/test_chrome_ext_trackingprotection.html | fetch failure
[task 2020-05-01T20:01:21.744Z] 20:01:21 INFO - add_task | Leaving test
[task 2020-05-01T20:01:21.744Z] 20:01:21 INFO - add_task | Entering test
[task 2020-05-01T20:01:21.744Z] 20:01:21 INFO - Extension loaded
[task 2020-05-01T20:01:21.745Z] 20:01:21 INFO - Buffered messages finished
[task 2020-05-01T20:01:21.748Z] 20:01:21 INFO - TEST-UNEXPECTED-FAIL | toolkit/components/extensions/test/mochitest/test_chrome_ext_trackingprotection.html | fetch received
[task 2020-05-01T20:01:21.750Z] 20:01:21 INFO - SimpleTest.ok@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:299:16
[task 2020-05-01T20:01:21.751Z] 20:01:21 INFO - testHandler@chrome://mochikit/content/tests/SimpleTest/ExtensionTestUtils.js:72:18
[task 2020-05-01T20:01:21.752Z] 20:01:21 INFO - testResult@chrome://mochikit/content/tests/SimpleTest/ExtensionTestUtils.js:82:18
[task 2020-05-01T20:01:21.756Z] 20:01:21 INFO - listener@resource://specialpowers/SpecialPowersChild.jsm:2075:33
[task 2020-05-01T20:01:21.757Z] 20:01:21 INFO - loadExtension/<@resource://specialpowers/SpecialPowersChild.jsm:2017:21
[task 2020-05-01T20:01:21.759Z] 20:01:21 INFO - receiveMessage@resource://specialpowers/SpecialPowersChild.jsm:238:21
[task 2020-05-01T20:01:21.760Z] 20:01:21 INFO - JSActor queryresultListener@resource://specialpowers/SpecialPowersParent.jsm:982:16
[task 2020-05-01T20:01:21.761Z] 20:01:21 INFO - emit@resource://gre/modules/ExtensionCommon.jsm:327:32
[task 2020-05-01T20:01:21.762Z] 20:01:21 INFO - receiveMessage@resource://gre/modules/Extension.jsm:1963:20
[task 2020-05-01T20:01:21.763Z] 20:01:21 INFO - MessageListener.receiveMessageExtension@resource://gre/modules/Extension.jsm:1783:19
[task 2020-05-01T20:01:21.764Z] 20:01:21 INFO - generate@resource://testing-common/ExtensionTestCommon.jsm:458:12
[task 2020-05-01T20:01:21.765Z] 20:01:21 INFO - receiveMessage@resource://specialpowers/SpecialPowersParent.jsm:979:45
[task 2020-05-01T20:01:21.777Z] 20:01:21 INFO - JSActor query*loadExtension@resource://specialpowers/SpecialPowersChild.jsm:2067:10
[task 2020-05-01T20:01:21.780Z] 20:01:21 INFO - ExtensionTestUtils.loadExtension@chrome://mochikit/content/tests/SimpleTest/ExtensionTestUtils.js:113:33
[task 2020-05-01T20:01:21.781Z] 20:01:21 INFO - test_permission@chrome://mochitests/content/chrome/toolkit/components/extensions/test/mochitest/test_chrome_ext_trackingprotection.html:28:38

Comment 16

[Rob Wu [:robwu]]

This part of the test fails: https://searchfox.org/mozilla-central/rev/710d6e1015d03343b067b92e6f1f775a0b1cad6f/toolkit/components/extensions/test/mochitest/test_chrome_ext_trackingprotection.html#52

The test needs to be updated to move that check down, because as of your patch, requests from extensions are considered to be first-party if the extension has the right permission. Consider this comment to be r+ for making the change that I described.

So change from

add_task(async function() { await test_permission(["<all_urls>"], true); });
// Fetch will not be blocked with explicit permissions

to

// Fetch will not be blocked if the extension has host permissions.
add_task(async function() { await test_permission(["<all_urls>"], true); });

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fbab79f74310
requests with webExtension loading principal are not 3rd party, r=ckerschb,robwu,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/f2dee04f06fa
requests with webExtension loading principal are not 3rd party - tests, r=robwu

Comment 19

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/fbab79f74310
https://hg.mozilla.org/mozilla-central/rev/f2dee04f06fa

Comment 20

[Tom S [:evilpie]]

Thank you for fixing this problem. We should document the new behavior (I think I've seen the old behavior documented in some places?) and I think this would also be a good candidate for the about:addons newsletter.

Comment 21

[Richard Bloor]

Please could I get some advice on what documentation might need changing.

Comment 22

[Tom S [:evilpie]]

I think for example https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/permissions#Host_permissions:

bypass tracking protection if the host is a full domain without wildcards. Doesn't work with <all_urls>.

I think the last sentence just doesn't apply anymore I think. Overall I am probably not the best person to help here, because I actually haven't read most of the documentation on MDN.

Comment 23

[Richard Bloor]

Documentation updates completed on host permissions and release notes.