After seeing Bug 1566295, I think I have a simple solution for this.

When we check for existing Origin headers when can simply remove all that use a disallowed scheme, which includes web-ext://. I suspect the early return for GET and HEAD request is quite and intentional and we don't want to do this.

Depends on D39780

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e0ea03ac10603008052bdaacc603b1338c4554d2&selectedJob=259423530

See the previous try result for dom/security/test/cors/test_CrossSiteXHR_origin.html. Should we allow * and null as origin?

null is a valid value of Origin:, which indicated it's tainted or filtered by referrer-policy or etc. (see [1][2])
[1] https://fetch.spec.whatwg.org/#serializing-a-request-origin
[2] https://fetch.spec.whatwg.org/#append-a-request-origin-header

Depends on D39781

I am going to rewrite the test soon, but I probably won't have enough time before the soft freeze.

https://hg.mozilla.org/mozilla-central/rev/207b97835f60
https://hg.mozilla.org/mozilla-central/rev/71d1cf219835

Backed out 2 changesets (Bug 1405971) for test_sanityRegisteredServiceWorker2.html failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=android%2C4.3%2Capi16%2B%2Copt%2Cmochitests%2Cwithout%2Ce10s%2Ctest-android-em-4.3-arm7-api-16%2Fopt-fennec-mochitest-1proc&tochange=d23599eaa057af2ca33ab658e1cb3c9984c6c14c&fromchange=e41dc7906592504c2596f90fc5db06005be7390b&selectedJob=265054756

Backout link: https://hg.mozilla.org/mozilla-central/rev/7f404d9a40c6b6312fe2f890844bace02c301d61

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=265054756&repo=autoland&lineNumber=1789

[task 2019-09-05T01:21:31.239Z] 01:21:31 INFO - 25 INFO TEST-START | testing/mochitest/tests/Harness_sanity/test_sanityRegisteredServiceWorker2.html
[task 2019-09-05T01:21:31.240Z] 01:21:31 INFO - 26 INFO TEST-UNEXPECTED-FAIL | testing/mochitest/tests/Harness_sanity/test_sanityRegisteredServiceWorker2.html | Registration successfully obtained
[task 2019-09-05T01:21:31.241Z] 01:21:31 INFO - SimpleTest.ok@SimpleTest/SimpleTest.js:277:18
[task 2019-09-05T01:21:31.241Z] 01:21:31 INFO - @testing/mochitest/tests/Harness_sanity/test_sanityRegisteredServiceWorker2.html:17:9
[task 2019-09-05T01:26:34.769Z] 01:26:34 INFO - 27 INFO TEST-UNEXPECTED-FAIL | testing/mochitest/tests/Harness_sanity/test_sanityRegisteredServiceWorker2.html | Test timed out.
[task 2019-09-05T01:26:34.769Z] 01:26:34 INFO - SimpleTest.ok@SimpleTest/SimpleTest.js:277:18
[task 2019-09-05T01:26:34.770Z] 01:26:34 INFO - reportError@SimpleTest/TestRunner.js:121:22
[task 2019-09-05T01:26:34.770Z] 01:26:34 INFO - TestRunner._checkForHangs@SimpleTest/TestRunner.js:142:18
[task 2019-09-05T01:26:34.770Z] 01:26:34 INFO - 28 INFO TEST-OK | testing/mochitest/tests/Harness_sanity/test_sanityRegisteredServiceWorker2.html | took 303455ms

FYI this change has triggered some bustage in webextensions that talk to the FxA OAuth servers, ref https://github.com/mozilla/fxa/issues/2411

I think we can fix it by changing the server behaviour but haven't dug into all the details yet.

To say a few more words about the above bustage: when handling CORS pre-flight requests on the server, it's a common pattern to inspect the Origin header from the incoming request and then echo its value back in the Access-Control-Allow-Origin header of the response. Such a pattern is even documented on the MDN page for the Access-Control-Allow-Origin header.

I don't think it's necessary to do this, but at least one popular web framework (Hapi for nodejs) takes this approach in its default configuration for CORS-enabled routes. With this change to remove the Origin header, webextensions are not able to make CORS requests to such routes.

I think we can fix this in FxA by configuring the route to explicitly send Access-Control-Allow-Origin: * rather than depending on the value of the Origin header, but there may be other websites out there with similar behaviour that will experience similar bustage.

I wonder if using Origin: null would cause fewer issues.

(In reply to Tom Schuster [:evilpie] from comment #25)

I wonder if using Origin: null would cause fewer issues.

This would make it look like a sandboxed document (like <iframe sandbox> or CSP's sandbox directive), so it's nothing one could rely on.

(In reply to Frederik Braun [:freddyb] from comment #26)

(In reply to Tom Schuster [:evilpie] from comment #25)

I wonder if using Origin: null would cause fewer issues.

This would make it look like a sandboxed document (like <iframe sandbox> or CSP's sandbox directive), so it's nothing one could rely on.

Can you clarify this a little bit? Who can rely on what?

So my idea was instead of stripping the Origin header completely we would instead send Origin: null. In theory this should allow extensions to continue to work with CORS using resources which mirror the Origin header to Access-Control-Allow-Origin, without leaking the moz-extension:// URL. Another idea would be to use a different URL like moz-extension://null if we want to avoid conflating sandbox resources and stripped origins. However I actually think we already might be using null for some other cases, maybe blob?

[task 2019-09-05T01:21:31.240Z] 01:21:31 INFO - 26 INFO TEST-UNEXPECTED-FAIL | testing/mochitest/tests/Harness_sanity/test_sanityRegisteredServiceWorker2.html | Registration successfully obtained

Weirdly enough I can reproduce this failure even without my patch applied. Maybe my local environment is somehow bad after running with my patch applied? I am relatively sure this test by itself doesn't even hit my code in nsHttpChannel::SetOriginHeader.

Hi :evilpie! Can I just check what documentation needs updating in light of this change?

(In reply to Tom Schuster [:evilpie] from comment #28)

[task 2019-09-05T01:21:31.240Z] 01:21:31 INFO - 26 INFO TEST-UNEXPECTED-FAIL | testing/mochitest/tests/Harness_sanity/test_sanityRegisteredServiceWorker2.html | Registration successfully obtained

Weirdly enough I can reproduce this failure even without my patch applied. Maybe my local environment is somehow bad after running with my patch applied? I am relatively sure this test by itself doesn't even hit my code in nsHttpChannel::SetOriginHeader.

Are you running all of the tests in this directory or just this one test? This test depends on test_sanityRegisteredServiceWorker.html which runs right before it...

Thank you Ehsan! After running the whole directory everything seems ok locally. Just to make sure I made a try run on Android again and indeed to error reproduces: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1385482164a6566a892a5bb37dd5bd903fc78b28&selectedJob=265884628. Not sure what to do next.

One suggestion: try running the test in desktop in non-e10s mode, perhaps that would help simulate Fennec better? Another suggestion, try running the tests on desktop in an Android simulator and see if you can debug it there?

I have been mostly away for the last two months and I totally forgot about the issue in comment 23. I would first like to see how much fallout this is going to to cause.

I have an alternative idea, which would be to censor moz-extension://998485cf-2a7a-4fc4-ae77-cda5afb0d795 to moz-extension://null if this is going to cause too much fallout.

https://hg.mozilla.org/mozilla-central/rev/3b42f1a5097a
https://hg.mozilla.org/mozilla-central/rev/dd473ab6821e

Was reading backscroll and it looks like no one answered Chris' documentation question in comment 29. If this is going to cause issues in webextensions (e.g bug 1596889) it should be clearly documented / explained.

Apologies if you followed up out of band.

I am going to submit a patch to implement what I suggested in comment 34, otherwise certain uses of CORS might become literally impossible.
I will also follow-up with Chris on documentation: https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/72. I am not sure where else maybe CORS, fetch, XHR?

Tom, can you please write a short summary of changes and potential issues in one comment, there has been a lot of discussion above. It can serve as the basis for MDN updates, and we should probably also mention this in the next blog post (please ni? :Fallen when you post it).

I think it would be best to backout this change for now, until we have a good idea what kind of issues this will cause.

Backed out 2 changesets (Bug 1405971) on evilpies's request

Backout link: https://hg.mozilla.org/integration/autoland/rev/765b9da8b818804ee68c8e18f92d2bdaa1794d8e

Backed out changes: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=dd473ab6821ecc27e748a756dca9ea8ebceaf0c5

Backout merged https://hg.mozilla.org/mozilla-central/rev/765b9da8b818

For reference network.http.referer.hideOnionSource now uses Origin: null: https://bugzilla.mozilla.org/show_bug.cgi?id=1598647

I am not really sure how to to go forward with this bug. Who would be the right person to make a decision on this? I think here are some of our options:

1. Strip Origin header, the previous patch, which caused issue with various compatibility problems.
2. Use Origin: null
3. Use a different censored header like Origin: moz-extension://null
4. Wont Fix and keep this privacy leak. In comparison in Chrome uses the extension ID, which is not unique and "only" leaks which extension an user is using.
5. Something else?

I personally am most in favour of option 1, but since that didn’t go so well, I’d personally go with option 2.

I am wondering if Anne has some specific recommendation.

Origin: null is the way to go here. You cannot strip Origin as that breaks CORS. moz-extension://null is somewhat valid, but it seems better not to reveal Firefox users beyond the obvious places as it'll make any obscuring efforts harder (it's preferable to the status quo, but the status quo probably shouldn't have shipped).

May I ask....... ?

As the developer of FireMonkey (userScript Manager for Firefox), the leak was pointed out to me recently and as a result I decided to filter out any leak by removing any header that includes 'moz-extension://' (this version is not released yet).

The fetch is used by user-content-script via script API and the request is generated from the background script.

Reading from the comment by Anne, should I changed that to Origin: null ?
Is the leak only limited to Origin?

PS. I only noticed the leak in POST (not GET)

(I'm not sure, my advice is for the Origin header only. If say Referer also includes this it'd be okay to omit that header (null would also not be a valid value there).)

Tom, origin:null seems to make sense to me as well. Can you write up a patch and tests? We should probably have a separate bug for referrer.

Apologies for the delay, I wanted to investigate more deeply here before answering, since I wasn't that familiar with all the underlying

(In reply to Tom Schuster [:evilpie] from comment #42)

Who would be the right person to make a decision on this?

Shane can probably make a more informed decision here (and I agree about Origin: null).

Thank you all, I am working on a new patch and seeing how it turns out on try.

https://hg.mozilla.org/mozilla-central/rev/94512ff23703
https://hg.mozilla.org/mozilla-central/rev/1698a498f801

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Hello.

Should the the steps from the description be used for manual testing or are there other test cases to be covered?
Thank you

I don't think there is a good way to test this change manually short of creating your own extension, which would already be covered by the existing test.

Alright. Then could you please set the "qe- verify" flag so that this does not show up on our queries anymore?

We should back this out of Beta, servers replying with Access-Control-Allow-Origin: null don't work. See Bug 1607154.

Backed out 2 changesets (bug 1405971) as requested by evilpie

Backout: https://hg.mozilla.org/releases/mozilla-beta/rev/8a06f7c796f1f61a091489cc88743ba48f2f2438

I don't think there is a good way to test this change manually short of creating your own extension,
which would already be covered by the existing test.

FWIW, installing the Firefox Notes extension and trying to sign in to it will exercise the changes from this bug, and has so far been a good way to repro potential fallout from those changes. For example, I believe it reproduces the issue seen in Bug 1607154.

I currently don't plan on fixing this in the short term. Feel free to take this.

(In reply to Tom Schuster [:evilpie] from comment #42)

For reference network.http.referer.hideOnionSource now uses Origin: null: https://bugzilla.mozilla.org/show_bug.cgi?id=1598647

I am not really sure how to to go forward with this bug. Who would be the right person to make a decision on this? I think here are some of our options:

1. Strip Origin header, the previous patch, which caused issue with various compatibility problems.
2. Use Origin: null
3. Use a different censored header like Origin: moz-extension://null
4. Wont Fix and keep this privacy leak. In comparison in Chrome uses the extension ID, which is not unique and "only" leaks which extension an user is using.
5. Something else?

I prefer provide a extension option to let user select, user can select the behavior like Chrome, which if the extension is install from Firefox Store, then send the Firefox Store deterministic UUID, if the extension install from developer mode, fallback to the random UUID.

• The deterministic UUID can tell the server(the extension owner's server, not 3rd-party), the client code is unmodified, it is trust-able and safe.
• The random UUID tell the server that the client maybe modified, it is untrust-able and unsafe, then the server can reject to response or limit the accessible resources.

A detail comment I post a few hours ago on another post https://bugzilla.mozilla.org/show_bug.cgi?id=1257989#c18

The severity field for this bug is relatively low, S3. However, the bug has 6 See Also bugs.
:robwu, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.