On Wed, Jul 9, 2008 at 6:59 PM, Jonas Sicking wrote on the WHATWG mailing list: > Hi All, > > The Access-Control spec [1] adds an 'Origin' header that is submitted with > all requests. I propose that we specify that <form> POSTs should do the > same. This would be a very powerful mechanism to prevent CSRF attacks as it > would allow CSRF prevention to happen in the server, rather than in the > application layer. > > This way servers could be configured to reject all POST requests that have > an Origin header from a different site. > > This wouldn't replace the normal CSRF protections sites need to do for now, > but eventually enough UAs implement this that servers can just reject POSTs > that doesn't have 'Origin' set. This would be especially true if we can get > this feature backported into old browsers (we'll likely backport it to FF3). > > / Jonas > > [1] http://dev.w3.org/2006/waf/access-control/ I would like to see this proposal implemented in Firefox. Adam Barth, John Mitchell, and I have posted our paper in support of the Origin header as a CSRF defense at <http://crypto.stanford.edu/websec/csrf/>. Here is an excerpt: To prevent CSRF attacks, we propose modifying browsers to send a Origin header with POST requests that identifies the origin that initiated the request. If the browser cannot determine the origin, the browser sends the value "null". == Privacy == The Origin header improves on the Referer header by respecting the user’s privacy: 1. The Origin header includes only the information required to identify the principal that initiated the request (typically the scheme, host, and port of the active document’s URL). In particular, the Origin header does not contain the path or query portions of the URL included in the Referer header that invade privacy without providing additional security. 2. The Origin header is sent only for POST requests, whereas the Referer header is sent for all requests. Simply following a hyperlink (e.g., from a list of search results or from a corporate intranet) does not send the Origin header, preventing the majority of accidental leakage of sensitive information. By responding to privacy concerns, the Origin header will likely not be widely suppressed. == Server Behavior == To use the Origin header as a CSRF defense, servers should behave as follows: 1. All state-modifying requests, including login requests, must be sent using the POST method. In particular, state-modifying GET requests must be blocked in order to address the forum poster theat model. 2. If the Origin header is present, the server must reject any requests whose Origin header contains an undesired value (including null). For example, a site could reject all requests whose Origin indicated the request was initiated from another site. == Security Analysis == Although the Origin header has a simple design, the use of the header as a CSRF defense has a number of subtleties. * Rollback and Suppression. Because a supporting browser will always include the Origin header when making POST requests, sites can detect that a request was initiated by a supporting browser by observing the presence of the header. This design prevents an attacker from making a supporting browser appear to be a non-supporting browser. Unlike the Referer header, which is absent when suppressed by the browser, the Origin header takes on the value null when suppressed by the browser. * DNS Rebinding. In existing browsers, the Origin header can be spoofed for same-site XMLHttpRequests. Sites that rely only on network connectivity for authentication should use one of the existing DNS rebinding defenses, such as validating the Host header. This requirement is complementary to CSRF protection and also applies to all the other existing CSRF defenses. * Plug-ins. If a site opts into cross-site HTTP requests via crossdomain.xml, an attacker can use Flash Player to set the Origin header in cross-site requests. Opting into cross-site HTTP requests also defeats secret token validation CSRF defenses because the tokens leak during cross-site HTTP requests. To prevent these (and other) attacks, sites should not opt into cross-site HTTP requests from untrusted origins.

Jesse says: "The proposal sounds good to me. I was never a fan of referrer-hiding, so you might want to get an opinion from someone who understood that." Gerv

Here is a patch that attaches the Origin header to POST requests.

Comment on attachment 336013 [details] [diff] [review] Patch >+++ b/netwerk/protocol/http/src/nsHttpChannel.cpp Fri Aug 29 01:23:09 2008 -0700 >@@ -324,24 +324,35 @@ Please add -p to your diff flags? >+ nsCAutoString method; >+ GetRequestMethod(method); >+ if (method.Equals("POST")) { if (mRequestHead.Method() == nsHttp::Post) { is probably better >+ nsCAutoString origin; >+ rv = mRequestHead.GetHeader(nsHttp::Origin, origin); >+ if (NS_FAILED(rv) || origin.IsEmpty()) { It seems like: if (!mRequestHead.PeekHeader(nsHttp::Origin)) { is enough, no? > nsHttpChannel::SetReferrer(nsIURI *referrer) So... this won't get called in a lot of cases. In particular, there's some docshell code that skips calling this function under certain circumstances. However, docshell will always set the "docshell.internalReferrer" property. I don't recall the details of how this stuff works, so you might want to check with dveditz or whoever touched it last. But it's worth looking into whether that should be used if present. >+ nsCAutoString method; >+ GetRequestMethod(method); >+ if (method.Equals("POST")) { See above for the faster way to check this. >+ nsCAutoString origin; >+ rv = clone->GetPrePath(origin); This doesn't look right to me, for several reasons: 1) This should probably use the innermost URI, not the referrer URI itself to handle jar:, view-source:, etc. 2) GetPrePath doesn't ASCII-fy, which as I recall is a problem when putting stuff in http headers. As far as I know the Origin spec talks about ascii-fied hostnames. 3) I don't recall that GetPrePath canonicalizes default ports, which we need to do for Origin, right? 4) GetPrePath will return things like "data:" or "about:" for non-hierarchical URIs, whereas I suspect we want to send "null" in those cases, though maybe we want to send the entire URI instead? I'm not really happy with the latter, to be honest. Ideally, we'd have an nsNetUtil function to compute the origin of a URI, to be used here, in nsPrincipal::GetOrigin, and in GetPrincipalDomainOrigin. It would more or less follow whatever the Origin spec says. As far as that goes, do we set data: URIs as referrers? What about wyciwyg? Or do we get the URI from the principal in those cases? This might just need a followup bug, since I bet we do and that seems wrong to me. The function I mention above might need, as a first cut, a way of indicating to the caller that it used the full spec as the origin so this particular caller can use "null" instead. This probably also needs tests and review from biesi.

Thanks for the comments. Unusual schemes aren't much of an issue because we only attach the Origin header for http, https, ftp, and gopher schemes. Thanks for the pointer to internalReferrer. I'll investigate an post a new patch.

The point is that you can have something like: jar:http://foo.com/bar.jar!/bin.html I don't really know how that works with POSTs, but I'm pretty sure that you really do want to get the innermost URI. You can use NS_GetInnermostURI to do it.

Comment on attachment 336013 [details] [diff] [review] Patch Clearing the review flag while I take another go at this.

Addresses most of the comments above. Not sure how to deal with schemes like "about" and "data". Next stop: tests.

Hmm.. ideally I think we want to get the origin out of an nsIPrincipal, rather than an nsIURI. But we can't really change nsIHttpChannel at this point (though we will in the future). So what I think we should do is to go through all callers of SetReferrer to make sure that they get their referrer URI out of a principal, rather than a GetDocumentURI for example. It would help here with an NS_SetHTTPReferrer(nsIHTTPChannel*, nsIPrincipal*) This should elliminate most cases when the URI is a data uri or a javascript uri. After that I think we can make the Origin header have the value "null" for those cases, including when the principal is chrome. Another thing, should we set this on all non-GET/HEAD channels, not just on POST? I.e. should DELETE have an origin as well? I would think so.

Thanks for the comments. > So what I think we should do is to go through all callers of SetReferrer to > make sure that they get their referrer URI out of a principal, Maybe we should do this in a separate patch? That looks like it will involve touching a lot of code. > This should elliminate most cases when the URI is a data uri or a javascript > uri. After that I think we can make the Origin header have the value "null" for > those cases, including when the principal is chrome. Turns out we already have a white list of allowed referrer schemes. I factored out that code and used it here too. > Another thing, should we set this on all non-GET/HEAD channels, not just on > POST? I.e. should DELETE have an origin as well? I would think so. Yeah, that's a good idea. Fixed.

separate patch seems good for the referred thing. Is the patch ready for review then? If so, please request reviews

> separate patch seems good for the referred thing. Is the patch ready for review > then? If so, please request reviews Almost there. I'm going to add some tests first.

Here is an updated patch with tests.

Comment on attachment 338278 [details] [diff] [review] patch with tests I assume the test can't be a necko unit test because we rely on the docshell referrer-setting behavior? Might be good to also have a unit test where we set the referrer manually, too. Or file a followup bug to create that, as well as to create a test for a jar: referrer. > +++ b/content/base/test/test_bug446344.xhtml > + is(window.frames[0].document.getElementsByTagName("pre")[0].innerHTML ... How about window.frames[0].document.textContent ? And perhaps some line-breaking within 80 chars? Same for the window.frames[1] case. >+addLoadEvent(submitPOSTForm, ok); What's the 'ok' part doing there? >+NS_GetOriginFromURI(nsIURI *uri, So for a non-nsIURL nsIURI this function will return an error, but more or less by accident (the SetUserPass will return error). Is this the behavior we want? If so, we should document it, and perhaps make it less accidental. >+++ b/netwerk/protocol/http/src/nsHttpChannel.cpp Fri Sep 12 01:46:02 2008 -0700 >+ // Make sure we send an Origin header for POST requests to tell the server s/POST requests/requests which are not GET or HEAD/ >+ origin = NS_LITERAL_CSTRING("null"); origin.AssignLiteral("null"); >+ nsCAutoString originASCII; >+ rv = NS_StringToACE(origin, originASCII); Hmm... Should we be using GetAsciiHost instead of GetPrePath while building up the origin string?

Honza, I think you also have a patch with a GetOrigin function in it. You and Adam should coordinate on this.

As I remember the GetOrigin (or NS_GetSchemaHostPort) function has been removed because it was no longer necessary. Anyway, it seems its need for it raises. So, putting function for extraction of an origin with respect to NS_SecurityCompareURIs function to nsNetUtils.h would be appropriate. Adam, I can create a patch on top of your patch that will introduce this new function. Would you agree with that? Other option is to submit a new bug and make it blocking this one. I know about some patches that would need this "GetOrigin" function too.

(In reply to comment #13) > I assume the test can't be a necko unit test because we rely on the docshell > referrer-setting behavior? This is my understanding. > Might be good to also have a unit test where we set > the referrer manually, too. Or file a followup bug to create that, as well as > to create a test for a jar: referrer. Ok. I'll create a follow-up bug. > How about window.frames[0].document.textContent ? Fixed. > And perhaps some line-breaking within 80 chars? Same for the window.frames[1] > case. Fixed. > >+addLoadEvent(submitPOSTForm, ok); > > What's the 'ok' part doing there? No idea. Copy/paste from another test. Removed. > >+NS_GetOriginFromURI(nsIURI *uri, > > So for a non-nsIURL nsIURI this function will return an error, but more or less > by accident (the SetUserPass will return error). Is this the behavior we want? > If so, we should document it, and perhaps make it less accidental. What is a non-nsIURL nsIURI (that sounds paradoxical)? I just moved this code from the postMessage implementation so I could reuse it. It looks like setReferrer calls the same function... Is there a way to achieve the same effect without generating the error? > s/POST requests/requests which are not GET or HEAD/ Fixed. > >+ origin = NS_LITERAL_CSTRING("null"); > > origin.AssignLiteral("null"); Fixed. > >+ nsCAutoString originASCII; > >+ rv = NS_StringToACE(origin, originASCII); > > Hmm... Should we be using GetAsciiHost instead of GetPrePath while building up > the origin string? postMessage was using GetPrePath (which I think was introduced for this purpose). (In reply to comment #15) > Adam, I can create a patch on top of your patch that will introduce this new > function. Would you agree with that? Other option is to submit a new bug and > make it blocking this one. I know about some patches that would need this > "GetOrigin" function too. This patch already contains a NS_GetOriginFromURI in nsNetUtils.h. Is there some reason to move it to a separate patch (maybe to easy integration)?

Comment on attachment 338331 [details] [diff] [review] patch updated with bz's comments >diff -r 018b8ac99a4c netwerk/base/public/nsNetUtil.h >--- a/netwerk/base/public/nsNetUtil.h Thu Aug 28 17:17:55 2008 -0700 >+++ b/netwerk/base/public/nsNetUtil.h Fri Sep 12 09:08:47 2008 -0700 >@@ -1411,24 +1411,62 @@ NS_GetInnermostURI(nsIURI *uri) > NS_ADDREF(uri); > return uri; > } > > nsresult rv = nestedURI->GetInnermostURI(&uri); > if (NS_FAILED(rv)) { > return nsnull; > } > > return uri; > } > >+inline nsresult >+NS_GetOriginFromURI(nsIURI *uri, >+ nsACString &origin) >+{ >+ NS_PRECONDITION(uri, "Must have URI"); >+ >+ nsCOMPtr<nsIURI> innermostURI = NS_GetInnermostURI(uri); >+ if (!innermostURI) >+ return NS_ERROR_FAILURE; >+ >+ // We need to clone innermostURI here because NS_GetInnermostURI could >+ // return an alias of uri. >+ nsCOMPtr<nsIURI> originURI; >+ nsresult rv = innermostURI->Clone(getter_AddRefs(originURI)); >+ NS_ENSURE_SUCCESS(rv, rv); >+ >+ // Normalize default ports, if necessary. >+ PRInt32 port; >+ if (NS_SUCCEEDED(originURI->GetPort(&port))) { >+ nsCAutoString scheme; >+ originURI->GetScheme(scheme); >+ NS_ENSURE_SUCCESS(rv, rv); >+ PRInt32 defaultPort = NS_GetDefaultPort(scheme.get()); >+ if (port == defaultPort) { >+ rv = originURI->SetPort(-1); >+ NS_ENSURE_SUCCESS(rv, rv); >+ } >+ } >+ >+ // Extract the PrePath portion of the URI (sans UserPass). >+ rv = originURI->SetUserPass(EmptyCString()); >+ NS_ENSURE_SUCCESS(rv, rv); >+ rv = originURI->GetPrePath(origin); >+ NS_ENSURE_SUCCESS(rv, rv); >+ >+ return NS_OK; >+} >+ > /** > * Get the "final" URI for a channel. This is either the same as GetURI or > * GetOriginalURI, depending on whether this channel has > * nsIChanel::LOAD_REPLACE set. For channels without that flag set, the final > * URI is the original URI, while for ones with the flag the final URI is the > * channel URI. > */ > inline nsresult > NS_GetFinalChannelURI(nsIChannel* channel, nsIURI** uri) > { > *uri = nsnull; > nsLoadFlags loadFlags = 0; This function does not reflect NS_SecurityCompareURIs. Origin is not just the schema/host/port triple but more in case of file:// and some other schemes. Please see also bug 451081. It would probably be best if NS_GetOriginFromURI would return the same parts that NS_SecurityCompareURIs works with. Other problem is that GetPrePath doesn't work for some schemes at all from their nature.

(In reply to comment #18) > This function does not reflect NS_SecurityCompareURIs. Origin is not just the > schema/host/port triple but more in case of file:// and some other schemes. I see. We might not be able to use the same function then. This function has to produce the string "file://" for file URLs to work properly with postMessage.

So you want to have to functions: one dedicated to what is needed for postMessage and another just for what is needed to have an Origin header? The former, what I propose, could easily be used to fix bug 357323. Actually, I start to believe that "GetOrigin" should be method of nsIURI implementations via new interface as a string. I don't much like the way how "imap", "mailbox" and "news" URIs have to be handled a different way using a flag. Also there must be a special code for "file" URIs. The new interface could also provide method to compare two origins, but it is disputable because we are dependent on implementers to obey the origin rules.

I do think we should have a GetOrigin function, but it should live on nsIPrincipal rather than nsIURI. You really should never use nsIURIs as bases for security checks, but rather nsIPrincipals. (yes, lots of code currently using nsIURIs, and we've had bugs because of it)

You are right, we have both methods on nsIPrincipal: http://hg.mozilla.org/mozilla-central/annotate/9b2a99adc05e/caps/idl/nsIPrincipal.idl#l152 and a way to compare here: http://hg.mozilla.org/mozilla-central/annotate/9b2a99adc05e/caps/idl/nsIPrincipal.idl#l88 I believe we can find a way to use nsIPrincipal for all issues mentioned here including this one bug. But, I don't know how to get the principal in nsHttpChannel.

Jonas, the problem is that for a lot of these cases the HTML5 spec defines same-origin checks between an entity we have a principal for (e.g. a document) and a URI-to-be-loaded (hence no principal yet). We've been using CheckMayLoad for this, basically....

So, when I need to get origin string for purposes like database storage or hash table mapping (or purposes of this bug) I should do following? principal = nsScriptSecurityManager.getCodebasePrincipal(URI); origin = principal.origin; And by the way, principal.checkMayLoad calls NS_SecurityCompareURIs, when it fails then it tries to still return success in case both URIs (the principal's and the tested one) are file URIs and passes some others, less strict conditions (see the code).

I'm a bit unclear how to modify the patch to take this feedback into account. Would you prefer that I: 1) Remove NS_GetOriginFromURI, leave postMessage as is, and implement the URI -> Origin header conversion in nsHttpChannel.cpp. 2) Rename NS_GetOriginFromURI to something that makes it clear the function is for converting URIs to the string needed for the Origin header. 3) Leave the patch as is.

I'm fine either way personally.

> principal = nsScriptSecurityManager.getCodebasePrincipal(URI); > origin = principal.origin; That depends on whether you want an origin string that takes document.domain into account or not. If not, then this would work fine. If we need to take document.domain into account, then it's no good. Note that ideally (very ideally) we would pass along all the information we have (the nsIPrincipal or nsIURI or whatever we happen to have) and then at the last possible moment actually do anything like extracting information from them. So if the hashtable can be keyed on nsIPrincipal, say, then it should be. Adam, if the Origin header needs something different than other places that need origins, then it needs a separate function. Otherwise I would like us to have no more than one place where we extract origins from URIs in any one given way.

Sounds good. We don't need to take document.domain into account for this purpose. I'll test the getCodebasePrincipal approach and post a new patch.

Yeah, we definitely should not take document.domain into account. In fact, we basically never ever should except in the very few cases where it's needed for web compat. (In fact, now with postMessage implemented we ideally should kill support for setting document.domain. Unfortunately that would currently break sites)

This patch leaves postMessage as is, and implements the URI -> Origin header conversion in nsHttpChannel.cpp. (We tried the GetCodebasePrincipal approach, but our attempt to include nsIScriptSecurityManager.h failed. According to Gavin the netwerk module isn't allowed to depends on caps.)

This should probably get beta exposure, marking P2.

You can't use the GetCodebasePrincipal thing, right. But I'm still pretty unhappy about having multiple places in our codebase doing this. It doesn't help that this caller actually wants different output than the others in some cases(!). Could we please raise that issue with the spec author? Then again, I don't see this "null" stuff in the spec.... It sounds to me like we should get that straightened out (and perhaps straighten out the fact that the ascii serialization in html5 is empty). I just realized that I totally missed comment 16. My apologies. Responses follow. > What is a non-nsIURL nsIURI Any object that implements nsIURI but not nsIURL, pretty much. In particular nsSimpleURI. This is commonly used for protocols that have no concept of "host". The real question here is what you want behavior to be for such URIs, and making it very explicit (with comments or something, if nothing else, but better yet not relying on a side-effect of SetUserPass()) so that people don't "fix" away the desired behavior. > postMessage was using GetPrePath (which I think was introduced for this > purpose). prePath has been on nsIURI since 2002-Mar-05. But more to the point, GetPrePath will just not give the same output as the results of GetAsciiHost. The spec pretty clearly indicates we want the latter here, as far as I can tell. See http://dev.w3.org/2006/waf/access-control/#source-origin and follow the links. Note that by contrast the spec for postMessage explicitly talks about the unicode serialization of the origin. Now that I think about this more, by the way, and actually read the Origin spec, it seems to me that we should really be serializing the URI of the principal of the form node or some such here (guaranteed to have one in docshell, since we're limiting this to POST; the only other place we do POST is in XHR, and we can use its principal there), and not the referrer URI, which is a totally different beastie in some cases. That is, maybe the way to go is to create the origin in the callers and set it as another property in the HTTP channel property bag, rather than trying to get to security information that HTTP simply doesn't have from inside the HTTP channel.

So I think we should use the implementation of uri->origin-string mapping that is in this patch. That is IMHO the one that makes the most sense out of all the ones i've heard. We should then lobby to get it adopted in the various specs, I think that should be doable. We can migrate the other origin implementations over to this new method as the specs are updated. Ideally I would like to change the signature of nsIHttpChannel to either make SetReferrer take an nsIPrincipal, or add a SetOrigin method that takes an nsIPrincipal. This seems hard right now for a number of reasons (interfaces frozen, don't want necko to depend on caps). For now I think we should add a method somewhere reachable (nsContentUtils.h? caps somewhere?) with the signature: NS_SetOriginOnChannel(nsIHttpChannel* aChannel, nsIPrincipal* aOrigin) And call that in the places that need it and have the information.

This is the implementation that will put non-ASCII chars into the HTTP header? I don't think that's desirable, since it violates the HTTP spec.

Oh, crap, i missed that. You need to call AsciiHost or AsciiSpec. It is actually a good question of what origin should be when exposed to javascript, such as in postMessage. In any even we should use the punycode when sending as a HTTP header.

The JavaScript question is a tricky one. As I understand it, the current attitude from the IETF is that IDN is a display technology, and underlying plumbing uses the xn-- form throughout. So that would suggest exposing it as xn--. However, I think Hixie wants to make it possible in HTML5 to use the proper forms directly in HTML. So perhaps it would be best to consult with the HTML5WG. Gerv

Not blocking on this. We'd definitely want this done as soon as possible, but there needs to be a spec first. And we can always land a fix in a dot-release.

(In reply to comment #37) > And we can always land a fix in a dot-release. Please don't use this to determine blocking/non-blocking status. We're much more cautious on what we take in "dot-releases" and tend to now only take security, stability, and regression-from-previous-dot-release fixes. If this is something you want fixed at all in 3.1.x, it should land in 3.1 final (3.1.0?).

This would be a security fix. We can't block yet since there is no spec, and possibly won't be until it's too late to get into 3.1 (i.e. might be after we ship 3.1)

(In reply to comment #39) > This would be a security fix. Then it should have an sg marking in the status whiteboard. By "security fix", I mean a fix to a security vulnerability found in the codebase, not an enhancement in function to existing features (unless that's the safest or only way to fix a reported vulnerability).

Absent the conversation about whether it blocks or not, is this something that's still being actively worked on?

Based on Comment #37, I'm working with the IETF's HTTPbis working group on an Internet-Draft for this. You can see the latest draft here: http://webblaze.cs.berkeley.edu/2009/origin/origin.txt I'm happy to resume work on the patch.

For what it's worth, we now have origin-string-getting functions in nsContentUtils. If we need to move them to nsNetUtil, we can.

I think we've converged on how this should work, and we've harmonized with CORS. It's probably time to try writing a new patch.

Adam, is the policy that the WebKit implementation of Origin follows documented anywhere? I noticed that [1] states that "The user agent MAY include an Origin header in any HTTP request" and it appears that WebKit is only sending it with POST. Does WebKit have any long-term plans to send it in other contexts? [1] http://tools.ietf.org/html/draft-abarth-origin-09

I haven't updated the WebKit implementation since the original prototype, but the plan is to match the spec: https://datatracker.ietf.org/doc/draft-ietf-websec-origin/?include_text=1 If you'd like to have input on what the spec says, feel free to share your thoughts on websec@ietf.org or to email me directly.

Any plans to implement this? Would be very helpful since Chrome supports it already and CSRF mitigation via Origin header over tokens is a much cleaner solution.

Mounir?

So from what I heard from Ian, and it would be great if Adam could confirm, is that this was only done for POST because including it everywhere broke some services. https://tools.ietf.org/html/rfc6454 is unfortunately still vague on when exactly it should be included, but is clear on the serialization questions. My plan is to make it crystal clear in http://fetch.spec.whatwg.org/ for which requests it is to be included and what its value should be.

Anything yet from Adam?

We add it for non-GET requests.

It seems that this neendinfo is a mistake given that I have not been involved with that. Boris, if your intent was to have me look deeply into this and give my feedback, let me know.

In our app (non-public yet) I create dynamically a form to the logout address and then submit it. Chrome adds the Origin header in such a case and Firefox doesn't. The lack of this header causes Django 1.6 to check for CSRFToken so it breaks the site when run on localhost (e.g. during automated tests). I was wondering what makes the test break for some time...

This probably needs a different owner... It's not clear whether necko should do this for all POSTs or whether docshell should do it. That's more or less a necko API decision.

Hi These two pages seem to suggest that Origin is set on POST requests in Firefox. However, in my testing (testing tool: http://homakov.github.io/) this appears not to be the case. https://wiki.mozilla.org/Security/Origin http://www.petefreitag.com/item/702.cfm Implementing this seems like an important security improvement that would make it vastly easier to protect your website against CSRF attacks. Is this on Firefox's roadmap?

http://www.browserscope.org/security/test This is another site testing origin header and other security related browser features. There are another 2-3 of them missing in firefox 31

I just had a long conversation with wgrant, lead developer of launchpad, one of the few websites that I (try to) use that require referers. He's saying he can't reasonably fix this https://bugs.launchpad.net/launchpad/+bug/560246 . But if Firefox were to send Origin: for all same-origin (!) HTTP POST requests, Launchpad could check for that header. If it exists, and matches, the POST can be allowed even in the absense of Referer. Otherwise, Launchpad could still do its normal dirty tricks for other browser. But at least it would be fixed for Firefox and Chrome. Chromium already does this. Please note that Origin: should be sent only (!) for same-origin requests. If they were to be sent for cross-origin requests, we'd have (almost) the same privacy problem as with Referer. (To a lesser extend, because it's only the hostname, not the URL, but still the site can see which site I was on.) The warnings of RFC 2616 Section 15.1.3 apply here as well.

Henri Sivonen wrote in bug 704320 comment 7: > > network.http.sendRefererHeader set to zero > I did so, too, for years with success, but had to stop when I started using Launchpad > for reporting Ubuntu bugs. Exactly the same for me. So, the lack of this feature has a direct effect on people's privacy.

Origin is included for cross-origin HTTP POST as well (when issued through XMLHttpRequest, not <form>), as part of the CORS protocol. Chrome implements that as well. Including when done through <img crossorigin> or some such. However, in such cases the two parties have already decided to cooperate.

What is the status of this bug? Do we still want to use the Origin header for CSRF Mitigation? What happens if a user turns off referrers completely in the browser for privacy reasons? The Origin header should also then be stripped, and hence using it for CSRF mitigation would be as error prone as using the referrer for CSRF mitigation. Given we have added ways to reduce referrer information (ex: meta referrer https://bugzilla.mozilla.org/show_bug.cgi?id=704320) and are considering changing the default referrer policy, does this bug still make sense?

I think always sending Origin header in all HTTP POST request should be fine even regards the privacy consideration, because I don't see any reason why a normal website, out of CSRF attack, would want to send cross-origin post request, but not want the target to know it.

If the user turns off referrers completely, are you asserting we'd break every site that uses CORS? I doubt that. Note also that 1) Origin includes less information than Referer by design. No path or query is revealed. 2) As Xidorn indicates this only affects <form method=POST>.

> are you asserting we'd break every site that uses CORS? Arguably, we should... The "disable referrers completely" preference is a bit of a footgun if you take it seriously. More precisely, break sites that don't just want to expose their stuff to the whole world. > Origin includes less information than Referer by design. Not necessarily true given the fine-grained control possible over Referer now... In any case, we should really figure out what our "disable referer" stuff should mean in terms of the Origin header... we can have an argument about whether things are opt-in or not, but it really would help most to just understand the set of problems the Referer preferences are trying to solve.

So the only preferences I find for Referer are in about:config which makes them essentially always be the default values for our users. It's not entirely clear to me why a single header warrants four preferences, but I guess that's a separate bug. The case Tanvi came up with was about changing the default referrer policy. Even if we change the default of that, CORS will have to remain working. Given that, adding modest XSRF protection to <form method=POST> seems like a good idea.

Agreed with Anne. There is a difference between passive, generally accidental or unintended, leaking of information via Referrer, vs cross-origin requests with direct information access via CORS or taking actions via POST, and the origin is a pretty limited set of information. While token based approaches are superior for XSRF in many cases, there are some cases where tokens are more error-prone and Origin can be very useful. e.g. if an attacker is trying to force a user to login to an account they control via CSRF, the attacker likely already knows the value of the CSRF token.

In light of https://tom.vg/papers/heist_blackhat2016.pdf an Firefox at least agree to send the Origin header when a request is same-origin? That provides a simple, browser-level mitigation and does not have the privacy concerns of revealing the exact non-same-origin location a request came from.

Brad, per the update from Patrick we are willing to accept patches already (for the full behavior; we should figure out if Chrome also includes it for "no-cors" or just "navigate" coupled with POST). Nobody picked it up thus far. I also haven't completed the specification work on this mostly because it's hard to get clarity from the Chrome networking team. Having said that, including this for all POST navigations would be a good first step.

See https://github.com/whatwg/fetch/issues/225 for a proposal.

It's probably worth reading through https://github.com/w3c/resource-timing/issues/64 and the proposal I linked above. In short, it's not clear that implementing the Origin header the way Chrome supports it actually helps with CSRF and it makes it harder (impossible really) to distinguish CORS requests. https://tools.ietf.org/html/draft-ietf-httpbis-cookie-same-site and/or a new Origin-like header seem like the way to go.

We use Origin header (with fallback to Referer) for CSRF prevention at my place of employment. Not sending Origin consistently makes Firefox a second-class browser for us. Sending Origin for same-origin requests would be a welcome addition.

I get that Origin header is not the perfect CSRF counter-measure. I get the point about breaking some aspects of CORs. However, it would seem that using Origin header for CSRF prevention would be a big easy security win for the vast majority of users. The paralysis of waiting for a better security mechanism against CSRF has meant that users of Firefox have been more vulnerable than their WebKit counter-parts for years now. Chromium experimented with using this header in a beneficial way, and the Internet doesn't seem worse-ff because of it, it seems better off. I deeply respect Firefox for what it stands for, but in this case, I'm scratching my head.

I would like to echo Xidorn, Brad and Justin's sentiments. Not sending the Origin header is making FireFox a "second class citizen" when it comes to CSRF defense and browser security. I feel this header - especially for same origin POST requests - is critical.

And please note, XHR should NOT be able to over-write the Origin header, add a second Origin header or add one where one does not exist or it will undermine this defense.

After talking to Anne, Brad and Mike West at TPAC, I think it's a worthwhile thing to implement even though better anti-CSRF approaches exist. I'll take it on this quarter.

May I ask what the status of this bug is? I'm really looking forward to seeing Firefox send the "Origin" header with POST requests.

(In reply to Steffen Weber from comment #76) > May I ask what the status of this bug is? I didn't get to it last quarter, but it's on my list for Q1.

FYI, https://fetch.spec.whatwg.org/ was updated to make it very clear when Origin is to be included in a request, per the agreement as in comment 75. Namely for all cross-origin CORS requests and any requests whose method is neither GET nor HEAD.

Any update here? Our Atlassian Confluence does not work because of lack of Origin headers in POST requests. :(

Comment 79 sounds like this has become a web-compat issue.

In reply to comment #75, François, so far as I am aware, CORS is the proper method to prevent CSRF. Can you elaborate as to what the better anti-csrf approaches you referred to might be?

(In reply to lasersox from comment #81) > Can you elaborate as to what the better anti-csrf approaches you referred to > might be? The CSRF protection enabled by the Origin header is described here: https://www.owasp.org/index.php/CSRF_Prevention_Cheat_Sheet#Verifying_Same_Origin_with_Standard_Headers For anybody interested in the status of this (in progress) bug, my detailed TODO list is here: https://public.etherpad-mozilla.org/p/bug446344

Any update on this? We are in Q3..

François, can you please provide a rough ETA on this?

(In reply to Florian Bender from comment #84) > François, can you please provide a rough ETA on this? It was de-prioritized earlier this year. I would still like to finish this year, but I can't promise anything. If someone wants to take it from me, I can post my patch and share notes about what remains to be done.

(In reply to François Marier [:francois] from comment #85) > If someone wants to take it from me, I can post my patch and share notes > about what remains to be done. That'd be much appreciated anyway, maybe this will prompt someone to jump in even more!

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

P5? Is this never getting implemented then?

(In reply to lasersox from comment #88) > P5? Is this never getting implemented then? 34 Votes only, we probably need more votes.

Francois is cleaning up the patch for review. Additional work will be done in Q4 and Q1.

Dragana, the patch I posted should more or less match the Webkit/Blink implementation. There will be follow-up work once we have refactored how we handle the referrer (bug to be filed later). That's why I'm landing this initial support pref'ed off.

Comment on attachment 8928798 [details] Bug 446344 - Implement Origin header CSRF mitigation. https://reviewboard.mozilla.org/r/200072/#review205258 ::: netwerk/protocol/http/nsHttpChannel.cpp:8982 (Diff revision 1) > + if (sSendOriginHeader == 0) { > + return; > + } > + > + // See nsHttpChannel::ProcessResponse() for why we use > + // GetReferringPage() here. The use in ProcessResponse is for the predictor, so in some ways it doesn't matter if it does nonsensical things, since it's not web-exposed. But this is web-exposed. Why is the document URI the right thing to use here (as opposed to the URI the document was loaded from, or the principal URI, etc)? ::: netwerk/protocol/http/nsHttpChannel.cpp:8989 (Diff revision 1) > + if (!referrer) { > + referrer = mReferrer; > + } > + > + nsAutoCString origin; > + if (referrer) { Why are we getting the Origin from the document URI? This seems like it will do the wrong thing for srcdoc documents, sandboxed documents, etc. There should be tests for all these cases, by the way. Note that CORS uses the LoadInfo's LoadingPrincipal() as the source of the Origin header.

Comment on attachment 8928798 [details] Bug 446344 - Implement Origin header CSRF mitigation. https://reviewboard.mozilla.org/r/200072/#review205378 Please also ask for a review from bz or someone from the security team as well. THe necko code looks good, I will take another look when origin uri get fixed. ::: netwerk/protocol/http/nsHttpChannel.cpp:8982 (Diff revision 1) > + if (sSendOriginHeader == 0) { > + return; > + } > + > + // See nsHttpChannel::ProcessResponse() for why we use > + // GetReferringPage() here. should this be uri from the triggering principal? I agree that the document URI is maybe not the right uri. This will need a review from someone form security team as well. ::: netwerk/protocol/http/nsHttpChannel.cpp:9011 (Diff revision 1) > + if (sSendOriginHeader == 1) { > + nsAutoCString currentOrigin; > + rv = nsContentUtils::GetASCIIOrigin(mURI, currentOrigin); > + if (NS_FAILED(rv) || currentOrigin.IsEmpty() || > + !origin.Equals(currentOrigin)) { > + return; do you want to set the header to "null" here?

Comment on attachment 8928798 [details] Bug 446344 - Implement Origin header CSRF mitigation. https://reviewboard.mozilla.org/r/200072/#review205378 > do you want to set the header to "null" here? Good point, it was a little confusing so I added a comment to explain why it's different.

Just for my own bookkeeping, comment 92 indicates there will be a follow-up bug to deal with https://github.com/whatwg/fetch/issues/593 and needed tests for that? If so, great! (Also, really great in general that we're tackling this. There's quite a few folks that need this.)

Comment on attachment 8928798 [details] Bug 446344 - Implement Origin header CSRF mitigation. https://reviewboard.mozilla.org/r/200072/#review208928 Approach looks good, but I have to r- this first iteration. Please address my concerns and re-flag me whenever you are ready; thanks for working on this. ::: netwerk/protocol/http/HttpBaseChannel.h:171 (Diff revision 2) > NS_IMETHOD GetRequestMethod(nsACString& aMethod) override; > NS_IMETHOD SetRequestMethod(const nsACString& aMethod) override; > NS_IMETHOD GetReferrer(nsIURI **referrer) override; > NS_IMETHOD SetReferrer(nsIURI *referrer) override; > NS_IMETHOD GetReferrerPolicy(uint32_t *referrerPolicy) override; > + NS_IMETHOD IsReferrerSchemeAllowed(nsIURI *referrer, bool* value); this is only a helper right? Why not use static bool IsReferrerSchemeAllowed(nsIURI* aReferrerURI); ::: netwerk/protocol/http/HttpBaseChannel.cpp:1587 (Diff revision 2) > + rv = referrer->SchemeIs(*scheme, value); > + if (NS_FAILED(rv)) { > + return rv; > + } > + } > + return NS_OK; I don't like that whole function to be honest and I am not even sure it does the right thing because it never breaks out of the loop, it always runs through the entire loop, no? Anyway, why not have bool HttpBaseChannel::IsReferrerSchemeAllowed(nsIURI *aReferrer) { NS_ENSURE_TRUE(aReferrer, false); nsAutoCString scheme; nsresult rv = aReferrer->GetScheme(&scheme); NS_ENSURE_SUCCESS(rv, false); if (scheme.LowerCaseEqualsLiteral("https") || scheme.LowerCaseEqualsLiteral("http") || scheme.LowerCaseEqualsLiteral("ftp")) { return true; } return false; } ::: netwerk/protocol/http/nsHttpChannel.cpp:8962 (Diff revision 2) > + return; > + } > + nsAutoCString existingHeader; > + Unused << mRequestHead.GetHeader(nsHttp::Origin, existingHeader); > + if (!existingHeader.IsEmpty()) { > + LOG(("nsHttpChannel::SetOriginHeader Origin header already present")); when could that be the case? Should we have a stronger assertion than just a log? I am not sure, just asking. ::: netwerk/protocol/http/nsHttpChannel.cpp:8984 (Diff revision 2) > + return; > + } > + > + nsCOMPtr<nsIURI> referrer; > + mLoadInfo->TriggeringPrincipal()->GetURI(getter_AddRefs(referrer)); > + can we restructure that function a little bit to have early returns? E.g. there is nothing to be done in case there is no referrer, right? if (!referrer) { mRequestHead.SetHeader(nsHttp::Origin, origin, ... return; } ::: netwerk/protocol/http/nsHttpChannel.cpp:8990 (Diff revision 2) > + nsAutoCString origin; > + if (referrer) { > + rv = nsContentUtils::GetASCIIOrigin(referrer, origin); > + if (NS_FAILED(rv) || origin.IsEmpty()) { > + origin.AssignLiteral("null"); > + } same here: if failed or orign.Empty() SetHeader to null, return; ::: netwerk/protocol/http/nsHttpChannel.cpp:8994 (Diff revision 2) > + origin.AssignLiteral("null"); > + } > + > + // Enforce referrer whitelist > + bool schemeAllowed = false; > + rv = IsReferrerSchemeAllowed(referrer, &schemeAllowed); maybe add a comment here as well why we are doing this? Because it could be a moz-nullprincipal{...}, right? ::: netwerk/protocol/http/nsHttpChannel.cpp:9006 (Diff revision 2) > + > + // Restrict Origin to same-origin loads if requested by user > + if (sSendOriginHeader == 1) { > + nsAutoCString currentOrigin; > + rv = nsContentUtils::GetASCIIOrigin(mURI, currentOrigin); > + if (NS_FAILED(rv) || currentOrigin.IsEmpty() || do we really only care about origin? I thought it should be scheme, host and port, no?

Comment on attachment 8928798 [details] Bug 446344 - Implement Origin header CSRF mitigation. https://reviewboard.mozilla.org/r/200072/#review208928 > this is only a helper right? Why not use > static bool IsReferrerSchemeAllowed(nsIURI* aReferrerURI); That changes the logic slightly (it doesn't propagate the exact IsScheme() error anymore), but that's probably OK and is cleaner. > I don't like that whole function to be honest and I am not even sure it does the right thing because it never breaks out of the loop, it always runs through the entire loop, no? > > Anyway, why not have > > bool > HttpBaseChannel::IsReferrerSchemeAllowed(nsIURI *aReferrer) > { > NS_ENSURE_TRUE(aReferrer, false); > nsAutoCString scheme; > nsresult rv = aReferrer->GetScheme(&scheme); > NS_ENSURE_SUCCESS(rv, false); > if (scheme.LowerCaseEqualsLiteral("https") || > scheme.LowerCaseEqualsLiteral("http") || > scheme.LowerCaseEqualsLiteral("ftp")) { > return true; > } > return false; > } You're right, it's a messy function. I was trying to avoid touching too much unrelated code and just moved the existing check into a function, but we may as well rewrite it properly since it's so short. > when could that be the case? Should we have a stronger assertion than just a log? I am not sure, just asking. CORS also uses that header, so if it's already set, we may as well keep it. > can we restructure that function a little bit to have early returns? > > E.g. there is nothing to be done in case there is no referrer, right? > if (!referrer) { > mRequestHead.SetHeader(nsHttp::Origin, origin, ... > return; > } We still need to check for `sSendOriginHeader == 1` to determine whether or not we need to send a header at all. So I could add early returns, but I would likely have to duplicate that check. I was able to shrink this block a little due to `IsReferrerSchemeAllowed()` now returning a boolean. Do you think that's enough? > same here: > if failed or orign.Empty() > SetHeader to null, return; We can almost do that. The only exception is when `sSendOriginHeader == 1` where we don't send a header at all. > maybe add a comment here as well why we are doing this? Because it could be a moz-nullprincipal{...}, right? I wanted to mirror what we do in the `Referer` header to avoid exposing anything that we wouldn't expose there. My guess is that it's to avoid schemes like `about:` or `file:` getting in there. > do we really only care about origin? I thought it should be scheme, host and port, no? `GetASCIIOrigin()` includes scheme, host and port: https://searchfox.org/mozilla-central/rev/9f3bd430c2b132c86c46126a0548661de876799a/dom/base/nsContentUtils.cpp#6437-6455

Comment on attachment 8928798 [details] Bug 446344 - Implement Origin header CSRF mitigation. https://reviewboard.mozilla.org/r/200072/#review209362 Hey Francois, thanks for incorporating the suggestions. I know that you didn't want to touch code that are not necessarily part of that patch, anyway, thanks for cleaning that up. Sorry I have to r- this patch one more time. The only reason for that is because I have been biten by 'camelcase' quite so many times before. As things stand right now we might run into the same problem. I guess we have to use .EqualsIgnoreCase in all of our functions. Please also update the tests to add some camelCase for scheme, host and port to make sure we don't regress that. Sorry I haven't realized that the first time around when I reviewed your patch. ::: netwerk/protocol/http/HttpBaseChannel.cpp:3328 (Diff revision 3) > + nsresult rv = aReferrer->GetScheme(scheme); > + NS_ENSURE_SUCCESS(rv, false); > + > + if (scheme.LowerCaseEqualsLiteral("https") || > + scheme.LowerCaseEqualsLiteral("http") || > + scheme.LowerCaseEqualsLiteral("ftp")) { sorry, I think .EqualsLiteralIgnoreCase is what we want ::: netwerk/protocol/http/nsHttpChannel.cpp:8989 (Diff revision 3) > + > + nsAutoCString origin; > + if (referrer && IsReferrerSchemeAllowed(referrer)) { > + rv = nsContentUtils::GetASCIIOrigin(referrer, origin); > + if (NS_FAILED(rv) || origin.IsEmpty()) { > + origin.AssignLiteral("null"); It seems that GetASCIIOrigin(...) assigns "null" in case it can not query the origin anyway, so you don't need to explicitly assign it here and will never be Empty(). Just remove that case. If I can make one suggestion, then I would do: // Default referrer value is "null" nsAutoCString origin("null"); if (referrer && IsReferrerSchemeAllowed(referrer)) { // Query the actual origin from the referrer nsContentUtils::GetASCIIOrigin(referrer, origin); } ::: netwerk/protocol/http/nsHttpChannel.cpp:9006 (Diff revision 3) > + if (NS_FAILED(rv) || currentOrigin.IsEmpty() || > + !origin.Equals(currentOrigin)) { > + // Origin header suppressed by user setting > + return; > + } > + } And do a similar thing here: if (sSendOriginHeader == 1) { nsAutoCString currentOrigin; nsContentUtils::GetASCIIOrigin(mURI, currentOrigin); if (!origin.EqualsIgnoreCase(currentOrigin) { return; } } ::: netwerk/protocol/http/nsHttpChannel.cpp:9007 (Diff revision 3) > + !origin.Equals(currentOrigin)) { > + // Origin header suppressed by user setting > + return; > + } > + } > + Ideally it would be great to add an assertion here that the value is 2, but I guess we can't because == 1 also falls through. I leave that up to you, you could pull in the mRequestHead.SetHeader(bla) into the if-case and return after that. The above change would allow you to add an assertion that we only get down here if ==2. As I said I leave that up to you, I am fine either way.

Comment on attachment 8928798 [details] Bug 446344 - Implement Origin header CSRF mitigation. https://reviewboard.mozilla.org/r/200072/#review209544 thanks. r=me

Comment on attachment 8928798 [details] Bug 446344 - Implement Origin header CSRF mitigation. https://reviewboard.mozilla.org/r/200072/#review210162 ::: netwerk/protocol/http/nsHttpChannel.cpp:9000 (Diff revision 5) > + // Origin header suppressed by user setting > + return; > + } > + } > + > + MOZ_ASSERT(sSendOriginHeader > 0 && sSendOriginHeader <= 2); this is not necessary. It is not a release assert so it is not important.

Try generally looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=82070be0199136f5d03ec4c02d99ba8531b11b25 but the test takes a long time on Android and times out: https://treeherder.mozilla.org/#/jobs?repo=try&revision=82070be0199136f5d03ec4c02d99ba8531b11b25&selectedJob=149016042 https://treeherder.mozilla.org/logviewer.html#?job_id=149016042&repo=try&lineNumber=1839 https://treeherder.mozilla.org/#/jobs?repo=try&revision=82070be0199136f5d03ec4c02d99ba8531b11b25&selectedJob=148973674 https://treeherder.mozilla.org/logviewer.html#?job_id=148973674&repo=try&lineNumber=1925

After increasing the timeout, Try looks better: https://treeherder.mozilla.org/#/jobs?repo=try&revision=456ee2a47297b7c51ae50ee9918a4455e411df4e The only failing tests are already known intermittents on Android: https://treeherder.mozilla.org/#/jobs?repo=try&revision=456ee2a47297b7c51ae50ee9918a4455e411df4e&selectedJob=149464576 https://treeherder.mozilla.org/#/jobs?repo=try&revision=456ee2a47297b7c51ae50ee9918a4455e411df4e&selectedJob=149457217

Pushed by fmarier@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7d2792c6423a Implement Origin header CSRF mitigation. r=ckerschb,dragana

For comment 97 and filing the follow-up bug, if any.

(In reply to Anne (:annevk) from comment #97) > Just for my own bookkeeping, comment 92 indicates there will be a follow-up > bug to deal with https://github.com/whatwg/fetch/issues/593 and needed tests > for that? I added this explicitly on line 10 of the etherpad (https://public.etherpad-mozilla.org/p/bug446344).

To document this, I've added a note to the Fx59 rel notes: https://developer.mozilla.org/en-US/Firefox/Releases/59#Security I've also checked the Origin reference page, and seen that in the compat data we mention this change too: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin#Browser_compatibility Let me know if this looks OK, and what else you think we need to say about this (I'm not really sure, but the above does seem a bit inadequate). Thanks!

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #113) > To document this, I've added a note to the Fx59 rel notes: > https://developer.mozilla.org/en-US/Firefox/Releases/59#Security In terms of release notes, we probably don't need to mention anything right now because this feature is currently pref'ed off by default (see bug 1424076). > I've also checked the Origin reference page, and seen that in the compat > data we mention this change too: > https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/ > Origin#Browser_compatibility Saying that we don't set the header on POST requests isn't quite right. I'd probably say that we don't send the Origin header unless it's a CORS request.

(In reply to François Marier [:francois] from comment #114) > (In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #113) > > To document this, I've added a note to the Fx59 rel notes: > > https://developer.mozilla.org/en-US/Firefox/Releases/59#Security > > In terms of release notes, we probably don't need to mention anything right > now because this feature is currently pref'ed off by default (see bug > 1424076). Oh, OK. In that case I've removed it from the release notes and added a note to the experimental features page: https://developer.mozilla.org/en-US/Firefox/Experimental_features#Security > > > I've also checked the Origin reference page, and seen that in the compat > > data we mention this change too: > > https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/ > > Origin#Browser_compatibility > > Saying that we don't set the header on POST requests isn't quite right. I'd > probably say that we don't send the Origin header unless it's a CORS request. OK, right. So can you just let me know if I've got the wording right in the experimental features note, and I'll then update this browser compat note too.

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #115) > OK, right. So can you just let me know if I've got the wording right in the > experimental features note, and I'll then update this browser compat note > too. Here's how I would describe this feature: Origin header sent for non-CORS requests To mitigate CSRF attacks, the Origin header is sent with non-CORS requests unless they are GET or HEAD (bug 446344). (Sending the header is the part that helps defend against CSRFs.) The above behaviour is when the pref is enabled (default = off). So we currently only send Origin on CORS requests (this is not new of course), but we plan to finish and turn this feature on by default soon.

(In reply to François Marier [:francois] from comment #116) > (In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #115) > > OK, right. So can you just let me know if I've got the wording right in the > > experimental features note, and I'll then update this browser compat note > > too. > > Here's how I would describe this feature: > > Origin header sent for non-CORS requests > To mitigate CSRF attacks, the Origin header is sent with non-CORS > requests unless they are GET or HEAD (bug 446344). > > (Sending the header is the part that helps defend against CSRFs.) > > The above behaviour is when the pref is enabled (default = off). So we > currently only send Origin on CORS requests (this is not new of course), but > we plan to finish and turn this feature on by default soon. Perfect, thanks. I've updated the wording.

The last update to this issue was 5mo ago that mentioned a change to default this to on "soon". Are there any updates to making this defaulted on? <3

I have seen Bug 446344. Just notifying that the lack of Origin is creating the webcompat issue https://webcompat.com/issues/19248 And that manually activating it is fixing the issue.

Bug 1424076 tracks turning this on by default. It includes a list of the remaining work, for example.