Set Origin header to "null" for cross origin redirects.

content/child/web_url_loader_impl.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/child/web_url_loader_impl.h"
 
 #include <algorithm>
 #include <deque>
 #include <string>
 
@@ skipping 16 matching lines @@
 #include "content/child/weburlresponse_extradata_impl.h"
 #include "content/common/resource_messages.h"
 #include "content/common/resource_request_body.h"
 #include "content/common/service_worker/service_worker_types.h"
 #include "content/public/child/request_peer.h"
 #include "content/public/common/content_switches.h"
 #include "net/base/data_url.h"
 #include "net/base/filename_util.h"
 #include "net/base/mime_util.h"
 #include "net/base/net_errors.h"
+#include "net/http/http_request_headers.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_util.h"
 #include "net/url_request/redirect_info.h"
 #include "net/url_request/url_request_data_job.h"
 #include "third_party/WebKit/public/platform/WebHTTPLoadInfo.h"
+#include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/platform/WebURL.h"
 #include "third_party/WebKit/public/platform/WebURLError.h"
 #include "third_party/WebKit/public/platform/WebURLLoadTiming.h"
 #include "third_party/WebKit/public/platform/WebURLLoaderClient.h"
 #include "third_party/WebKit/public/platform/WebURLRequest.h"
 #include "third_party/WebKit/public/platform/WebURLResponse.h"
 #include "third_party/WebKit/public/web/WebSecurityPolicy.h"
 #include "third_party/mojo/src/mojo/public/cpp/system/data_pipe.h"
 
 using base::Time;
@@ skipping 503 matching lines @@
   new_request.setFetchCredentialsMode(request_.fetchCredentialsMode());
 
   new_request.setHTTPReferrer(WebString::fromUTF8(redirect_info.new_referrer),
                               referrer_policy_);
 
   std::string old_method = request_.httpMethod().utf8();
   new_request.setHTTPMethod(WebString::fromUTF8(redirect_info.new_method));
   if (redirect_info.new_method == old_method)
     new_request.setHTTPBody(request_.httpBody());
 
+  // This is necessary to avoid laundering the Origin header across redirects,
+  // which would break some CSRF protections. See the comment in
+  // URLRequest::Redirect in //net/url_request.cc for more information.
+  WebString origin_header =
+      WebString::fromUTF8(net::HttpRequestHeaders::kOrigin);
+  new_request.setHTTPHeaderField(origin_header,
+                                 request_.httpHeaderField(origin_header));

[davidben, 2015/03/24 23:47:38]

I think this does the opposite of what the comment says. The renderer doesn't
really find out about any changes that happen during redirects. That's what I
meant by we end up unfortunately duplicating a lot of stuff, and why there's a
net::RedirectInfo now. This *is* where that all gets applied.

In fact, this is kind of odd... I wonder if we've been failing to copy over all
the extra headers (including Origin) from request leg to request leg (from
Blink's perspective) all this time...

To make matters worse, having URLRequest send a copy of the new extra request
headers back down to renderer doesn't quite work either because  if code in
between staples more headers, Blink should be blind to them. And I don't even
want to think about what happens if ServiceWorker mucks about with this.

I *think* the thing to do for now is to remove this code (sorry!) and only put
logic in URLRequest. We have a giant hairball to untangle around redirects and
process boundaries and CORS... Blink apparently has its own version of this
logic here. Which... actually if I'm reading it right, that does something
completely different!

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

Except I rather suspect this doesn't do anything. Certainly this never made its
way to the network stack because Blink has no influence on redirects. I also
don't think the Origin header got copied over anyway because this code never
copies over random headers...

...which means that probably doing a form submission which then does a
method-preserving redirect elsewhere and then pressing refresh causes all kinds
of problems. Until all these navigation refactors go through and then they might
not! (I suspect long term, we might want to move CORS out of the renderer
process? If we can get rid of all the places where the renderer cares about the
post-redirect request headers, that would resolve a lot of this.)

[jww, 2015/03/27 22:16:15]

Hm, okay, I'm happy to remove it. All of the tests that I wrote still pass, so
that does make me think it's unnecessary :-)

[davidben, 2015/03/27 22:46:51]

Well, the cases where Blink's version of the request headers are important are
pretty rare. I'm sure we have zero tests for those. I know of two (reloads and
document.referrer), but I'm sure there's more. :-)

+
   // Protect from deletion during call to willSendRequest.
   scoped_refptr<Context> protect(this);
 
   client_->willSendRequest(loader_, new_request, response);
   request_ = new_request;
 
   // Only follow the redirect if WebKit left the URL unmodified.
   if (redirect_info.new_url == GURL(new_request.url())) {
     // First-party cookie logic moved from DocumentLoader in Blink to
     // net::URLRequest in the browser. Assert that Blink didn't try to change it
@@ skipping 563 matching lines @@
                                          int intra_priority_value) {
   context_->DidChangePriority(new_priority, intra_priority_value);
 }
 
 bool WebURLLoaderImpl::attachThreadedDataReceiver(
     blink::WebThreadedDataReceiver* threaded_data_receiver) {
   return context_->AttachThreadedDataReceiver(threaded_data_receiver);
 }
 
 }  // namespace content