To prevent CSRF attacks, we should send an 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.
Created attachment 23364 [details] patch Here is a simple implementation.
Comment on attachment 23364 [details] patch Removing review flag because this patch doesn't properly handle HTTPS -> HTTP requests.
For reference, here is the Mozilla bug on this topic: https://bugzilla.mozilla.org/show_bug.cgi?id=446344 Here are Brandon Sterne's thoughts on the topic: http://people.mozilla.org/~bsterne/content-security-policy/origin-header-proposal.html
What is the desired behavior for HTTPS -> HTTP requests?
we'd like the header to be like Origin: https://www.example.com The current patch shows "null". Keep in mind that this case only occurs if the user clicks through the "insecure form submission" dialog.
Created attachment 23646 [details] Updated patch We updated the patch to attach an Origin header to POST requests even if the referrer is suppressed. We also added some more tests. The origin-header-for-xmlhttprequest.html test shows the Origin header getting attached to XMLHttpRequest GET requests -- I need to investigate whether this is a bug or a feature.
(In reply to comment #6) > The origin-header-for-xmlhttprequest.html test shows the Origin header getting > attached to XMLHttpRequest GET requests -- I need to investigate whether this > is a bug or a feature. This occurs in the latest nightly without this patch, so it's a feature. :)
(In reply to comment #7) > This occurs in the latest nightly without this patch, so it's a feature. :) This patch is currently attaching an Origin header to same-origin GET XMLHttpRequests that's not already present in the nightly. Cross-origin GET requests are unaffected by this patch.
Comment on attachment 23646 [details] Updated patch + // Don't send an Origin header for GET or HEAD to avoid privacy issues. This comment could use a little more information. What privacy issues future me may wonder? - request.setHTTPHeaderField("Origin", accessControlOrigin()); + request.setHTTPHeaderField("Origin", m_doc->securityOrigin()->toHTTPOrigin()); This can use request .setHTTPOrigin(). - request.setHTTPHeaderField("Origin", accessControlOrigin()); + request.setHTTPHeaderField("Origin", m_doc->securityOrigin()->toHTTPOrigin()); As can this. This doesn't break sending the origin for XHR does it? Otherwise, awesome! I think one other person should review these changes as well, as I am far from an authority on FrameLoader. Brady, Anders, Darin, or Maciej?
Created attachment 23743 [details] baseline for XHR origin behavior
Created attachment 23744 [details] updated patch Patch updated with Sam's comments. Also, I added a bunch of tests for the Origin header behavior for XHR. I split this into two patches so you can see exactly how this patch changes the behavior. (The previous patch messed up the XHR behavior a bit, so I tweaked a few things.)
Has this been proposed to the HTTP Working Group or another appropriate standards body? I like the idea but I'd like to make sure we put it on track to being officially placed in the appropriate standard.
We proposed this informally to the webapps working group a number of months ago, which (I think) is how Mozilla got interested in implementing this. It wasn't clear which spec would be most appropriate. The computation of the header value is speced in HTML 5 for postMessage and for use by cross-site XHR. I doubt the HTTP Working Group want an dependency on HTML 5. :)
It is part of Access Control for Cross-Site Requests and I will make sure it is registered as an official HTTP header in due course. (It won't be part of the HTTP specification of course, but other specifications can register header names.)
Comment on attachment 23744 [details] updated patch The biggest problem is that this sprinkles calls to a new function all over the loader classes. I'm concerned that we have to add the addHTTPOriginIfNeeded function call in so many places. It seems to me that will be hard to maintain -- it will be very easy to introduce a security problem by forgetting to add a call to that function. Is there any way to streamline this? I suppose we have the same issue with the referrer, but it's not even true that these are all the sites that set the referrer! The loader is already not so great in this respect, but it seems this patch makes it even worse. + RefPtr<SecurityOrigin> origin = SecurityOrigin::createEmpty(); + return origin->toHTTPOrigin(); The local variable isn't needed and the code would read more cleanly without it. + // Make sure we send the Origin header properly. + addHTTPOriginIfNeeded(request, String()); I don't understand the meaning of the word "properly" in this comment. + // will leak the internal host name. Similar privacy concerns have lead + // header. This is similar to toString(), except that the empty We normally don't use two spaces between sentences in comments. + RefPtr<SecurityOrigin> emptyOrigin = SecurityOrigin::createEmpty(); + origin = emptyOrigin->toHTTPOrigin(); The local variable isn't needed and the code would read more cleanly without it. Maybe also a helper function named SecurityOrigin::emptyHTTPOrigin() would be good? + // SecurityOrigin is represetned as the string "null". Typo: "represetned". I'm going to say review+ despite my comments and minor concerns.
Fixed in r37317. I definitely agree that FrameLoader needs to be reorganized, but it's a bit too big a project for me to take right now.