Bug 20792 - Add origin header to POST requests
: Add origin header to POST requests
Status: RESOLVED FIXED
: WebKit
Forms
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
: http://crypto.stanford.edu/websec/csrf/
:
:
:
  Show dependency treegraph
 
Reported: 2008-09-11 23:09 PST by
Modified: 2008-10-05 13:46 PST (History)


Attachments
patch (10.91 KB, patch)
2008-09-11 23:10 PST, Adam Barth
no flags Review Patch | Details | Formatted Diff | Diff
Updated patch (27.21 KB, patch)
2008-09-22 02:00 PST, Collin Jackson
sam: review+
Review Patch | Details | Formatted Diff | Diff
baseline for XHR origin behavior (13.21 KB, patch)
2008-09-24 02:04 PST, Adam Barth
no flags Review Patch | Details | Formatted Diff | Diff
updated patch (28.78 KB, patch)
2008-09-24 02:07 PST, Adam Barth
darin: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2008-09-11 23:09:43 PST
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.
------- Comment #1 From 2008-09-11 23:10:35 PST -------
Created an attachment (id=23364) [details]
patch

Here is a simple implementation.
------- Comment #2 From 2008-09-12 02:08:06 PST -------
(From update of attachment 23364 [details])
Removing review flag because this patch doesn't properly handle HTTPS -> HTTP requests.
------- Comment #3 From 2008-09-12 09:36:51 PST -------
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
------- Comment #4 From 2008-09-12 10:58:11 PST -------
What is the desired behavior for HTTPS -> HTTP requests?
------- Comment #5 From 2008-09-12 12:10:54 PST -------
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.
------- Comment #6 From 2008-09-22 02:00:56 PST -------
Created an attachment (id=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.
------- Comment #7 From 2008-09-22 23:55:10 PST -------
(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.  :)
------- Comment #8 From 2008-09-23 01:37:59 PST -------
(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 #9 From 2008-09-23 17:14:40 PST -------
(From update of attachment 23646 [details])
+        // 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?
------- Comment #10 From 2008-09-24 02:04:37 PST -------
Created an attachment (id=23743) [details]
baseline for XHR origin behavior
------- Comment #11 From 2008-09-24 02:07:15 PST -------
Created an attachment (id=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.)
------- Comment #12 From 2008-09-24 16:01:32 PST -------
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.
------- Comment #13 From 2008-09-24 18:15:12 PST -------
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.  :)
------- Comment #14 From 2008-09-25 00:35:26 PST -------
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 #15 From 2008-10-03 13:24:27 PST -------
(From update of attachment 23744 [details])
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.
------- Comment #16 From 2008-10-05 13:45:54 PST -------
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.