1504085 - Origin header should honor ReferrerPolicy

Status: RESOLVED FIXED

(Core :: DOM: Security, defect, P3)

Description

[Junior [inactive]]

"That's the Chrome behavior, but the OWASP wiki says it's not supposed to be like that for some reason.
https://www.owasp.org/index.php/CSRF_Prevention_Cheat_Sheet#Checking_the_Origin_Header"

Note that there's an exception:

HTTPS to http://127.0.0.1 should get the full origin since 127.0.0.1 is considered a potentially secure Origin.

Comment 1

[Daniel Veditz [:dveditz]]

Do you have steps to reproduce this, or an example site? One case of extra Origin: headers we do know about is bug 1502904 but I'm not sure that's what you're talking about.

The spec is part of Fetch:
https://fetch.spec.whatwg.org/#origin-header

Comment 2

[Junior [inactive]]

This bug is from bug 1424076 comment 1.
We want to make sure the origin header is good before we turn on pref network.http.sendOriginHeader 

I don't have anything on this.
I'll try to validate this when I get a chance.

Moreover, it's not a "fetch-only" feature. It applies to form POST, too.
Now we have three entries (fetch, websocket, bug 446344) to add the Origin header, making things complicated.

bug 1502904 is talking about the same-origin, and HTTPS and HTTP are cross origin.

Comment 3

[Daniel Veditz [:dveditz]]

bug 1424076 comment 1 was just verbatim notes taken in a meeting and probably missing a lot of context, copied months later because the big warning on Etherpad said we couldn't rely on it. As thoughts from a meeting they are not necessarily correct without investigations. For example, I don't remember why we said the Origin: header should be immune to the pref for Referrer spoofing. No point in spoofing if the Origin: header is going to give you away. No point in a ReferrerPolicy of noreferrer if the Origin: header leaks the origin anyway.

> Moreover, it's not a "fetch-only" feature. It applies to form POST, too.

Wasn't originally clear if you were talking about CORS use of Origin: or not. So not Fetch then, this is the rfc6454 sense of Origin: header. Except that's old and only defines the header but doesn't say when it should or shouldn't be sent. It is where this language about sending "null" comes from though.

  Whenever a user agent issues an HTTP request from a "privacy-
  sensitive" context, the user agent MUST send the value "null" in
  the Origin header field.

  NOTE: This document does not define the notion of a privacy-
  sensitive context.  Applications that generate HTTP requests can
  designate contexts as privacy-sensitive to impose restrictions on
  how user agents generate Origin header fields.

So if we assume "HTTPS" is a privacy-sensitive context then we should set Origin: to null. But "HTTP" in this context doesn't mean the http:// scheme, it means the protocol and is saying we should send null from "privacy-sensitive" https:// documents to even other https:// endpoints. I'm guessing that was a hook to account for browsers who wanted to suppress referrer-like information in "Private Browsing Mode".

I worry we're implementing this without a clear spec, even if that spec is just a reverse-engineered "What does Chrome do?".

Given mixed-content blocking this mostly only applies to image and similar requests in practice. Those could still be CORS requests, though, so we need to worry about compatibility.

Comment 4

[Junior [inactive]]

Hello David,
It's true that bug 1424076 comment 1 isn't extracted from spec. What I did is to review all the items there and to file the gap we might have. The bug is open in the processes.
I'm with you for the spoofing and ReferrerPolicy part. Moreover, I'd like to change this bug to "Let Origin: header follow the ReferrerPolicy" after some investigation. It makes more sense. Sorry for filing the bug before fully understanding.

Three things are involved to enable network.http.sendOriginHeader 
(a) Implementing something without a spec is concerned. That is utter true.
(b) We did encounter the web-compat issue for years like [1] and tons of discussion in bug 446344.
(c) Privacy which we care a lot.

Given too many sites checking (and wanna checking) the Origin: to mitigating CSRF in the field, I'd like to send Origin Header for the form POST with caring of leak information.

My plan is:
(a) Finish the bugs for leaving from .onion (bug 1503736), referrerPolicy(this bug) and spoofing (new bug)
    Good to wait for bug 1502904 and bug 1444278
(b) Turn on the pref network.http.sendOriginHeader to 1 (i.e., same site only)
(c) Wait for signal to change the pref to 0 or 2 (cors).

What do you think, David?
[1] https://webcompat.com/issues/19248


> Given mixed-content blocking this mostly only applies to image and similar
> requests in practice. Those could still be CORS requests, though, so we need
> to worry about compatibility.

I didn't fully understand what compat we worry about. Like the spec change or we dance with the prefs?

Comment 5

[Daniel Veditz [:dveditz]]

I'm going to bounce this to François who originally implemented this pref. My gut feel is that if we send the Origin header it should match the referrer we send based on the ReferrerPolicy. That might mean sending (null) from https -> http, but if the referrer policy says to send an origin unsafely then we should send the Origin header in that case too.

Yes, localhost can be consider a secure context and not get stripped referrers. If the referrer is correct then the Origin will automatically be correct.

There's an alternate spec proposal to address CSRF that we might want to do instead or in addition: Sec-Metadata. It carries more context but less specific origin information and is intended to always be sent. Because it's a new header we shouldn't break anything which is a risk we need to watch for when we start adding Origin: headers

Also tagging Anne in case he knows what the compat-state for non-CORS use of the Origin: header is.

Also Christoph noticed that our current Origin header might be wrong. Tricky because we don't want to lie in the CORS case, but otherwise the Origin: might be spoofed or limited and should use the "effective" referrer we're going to send in the header and not the internal record of the "real" referrer that we use for our own security checks.

https://searchfox.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannel.cpp#9430

Comment 6

[Daniel Veditz [:dveditz]]

Can't needinfo? francois so assigning this to him temporarily to answer the questions when he gets back.

Comment 7

[Anne (:annevk)]

The relevant bit in the standard around Origin is in https://fetch.spec.whatwg.org/#http-network-or-cache-fetch:

> If the CORS flag is set, httpRequest’s method is neither `GET` nor `HEAD`, or httpRequest’s mode is "websocket", then append `Origin`/the result of serializing a request origin with httpRequest, to httpRequest’s header list.

That was meant to supersede the RFC and tie together the various requirements around CORS, CSRF, and WebSocket.

However, this doesn't do any censoring for HTTPS -> HTTP or use the referrer policy. I might not have done enough testing around those cases to account for them. It should be doable to expand the requirements here to account for that though, but I'd need help with somewhat exhaustive tests as I won't have time to write this in the foreseeable future.

Comment 8

[Junior [inactive]]

Given the privacy concern and web-compat, let's honor the referalPolicy first.
If a sound spec shows it's good to exposing origin, it's easy to revert.

Comment 9

[François Marier [:francois]]

I'm not sure what the right answer is here.

On the one hand, honoring referrerpolicy seems like the right thing to do from a privacy point of view when the policy is stronger then the default one, but I'm not sure we should be sending more info if the policy is weaker.

Also, if a site wants to use a very strong referrerpolicy (maybe no-referrer or strict-origin) because they use the Origin header in their CSRF checks, then it could harm adoption. Although, if Chrome already does this, then I guess that boat has sailed.

It might be worth bringing up on the WebAppSec list.

Comment 10

[Junior [inactive]]

Assign myself for reaching a conclusion

Comment 11

[Junior [inactive]]

After discussion, we want to restrict the Origin: and not to expose information more than Referer:

The plan is:
(a) don't expose CORS with Origin:  (i.e. network.http.sendOriginHeader = 1)
(b) Based on (a), we want to honor the referral policy.
    The only problem is ReferrerPolicy: "no-referrer", which limits even same origin.
    Will have some tests and modify the spec.
(c) Fix tests.

Comment 12

[Junior [inactive]]

(In reply to Junior [:junior] from comment #11)

After discussion, we want to restrict the Origin: and not to expose
information more than Referer:

The plan is:
(a) don't expose CORS with Origin: (i.e. network.http.sendOriginHeader = 1)

From the view of the spec. 4.4.5.10, we need to add Origin: if CORS flag is set [1]
This might leads changes [2]

[1] https://fetch.spec.whatwg.org/#http-network-or-cache-fetch
[2] https://github.com/whatwg/fetch/issues/871

Comment 13

[Junior [inactive]]

Test is ready.
https://github.com/web-platform-tests/wpt/pull/15937

Comment 14

[Junior [inactive]]

Spec work
https://github.com/whatwg/fetch/pull/908

Comment 15

[Junior [inactive]]

Attachment: Bug 1504085 - P1 refactor ReferrerInfo for reuse referrer-policy algorithm

Comment 16

[Junior [inactive]]

Attachment: Bug 1504085 - P2 Let Origin: honor ReferrerPolicy for non-CORS

Comment 17

[Junior [inactive]]

(In reply to Junior [:junior] from comment #13)

Test is ready.
https://github.com/web-platform-tests/wpt/pull/15937

Test-fix patch
https://github.com/web-platform-tests/wpt/pull/17280

Comment 18

[Junior [inactive]]

Attachment: Bug 1504085 - P3 fix test

Origin: honors ReferrerPolicy: so we should honor defaultPolicy set by user

Comment 19

[Junior [inactive]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e02c5cc3fcfe17e8a6adfcc6f4be3908146322ad

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b1068cbce572
P1 refactor ReferrerInfo for reuse referrer-policy algorithm r=tnguyen

Comment 21

[Junior [inactive]]

Should have landed the whole stack in one shot.

Comment 22

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e84b3d7d69e8
Backed out changeset b1068cbce572 requested by the dev. CLOSED TREE

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/94e779e03d4f
P2 Let Origin: honor ReferrerPolicy for non-CORS r=tnguyen,valentin

Comment 24

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b68317f66cc6
Backed out changeset 94e779e03d4f Requested by the dev. CLOSED TREE

Comment 25

[Junior [inactive]]

Please land the three patches in one shot. Thanks.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ca62c730aa4
P1 refactor ReferrerInfo for reuse referrer-policy algorithm r=tnguyen
https://hg.mozilla.org/integration/autoland/rev/2c610261152d
P2 Let Origin: honor ReferrerPolicy for non-CORS r=tnguyen,valentin
https://hg.mozilla.org/integration/autoland/rev/601362e2871d
P3 fix test r=tnguyen

Comment 27

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/7ca62c730aa4
https://hg.mozilla.org/mozilla-central/rev/2c610261152d
https://hg.mozilla.org/mozilla-central/rev/601362e2871d