Use navigation initiator for calculating the `Origin` header's value.

Before this CL, AddAdditionalRequestHeaders would set the Origin header
to either the destination origin (for main frame navigations) or to the
main frame's origin (for subframe navigations).  Both of these are wrong
and don't match Blink behavior (which correctly uses the initiator of
the navigation to calculate the Origin header's value).

Bug: 915538
Change-Id: Ied0262462f7665d0004da3b298bf0618ae312aec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1761051
Auto-Submit: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Zhongyi Shi <zhongyi@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689562}

chrome/browser/tab_contents/view_source_browsertest.cc

@@ -369,7 +369,7 @@ IN_PROC_BROWSER_TEST_F(ViewSourceTest, HttpPostInMainframe) {
   EXPECT_THAT(source_text,
               HasSubstr("<h1>Request Body:</h1><pre>text=value</pre>"));
   EXPECT_THAT(source_text,
-              HasSubstr("<h1>Request Headers:</h1><pre>POST /echoall HTTP"));
+              HasSubstr("<pre id='request-headers'>POST /echoall HTTP"));
   EXPECT_THAT(source_text,
               ContainsRegex("Request Headers:.*Referer: " + form_url.spec()));
   EXPECT_THAT(
@@ -463,7 +463,7 @@ IN_PROC_BROWSER_TEST_F(ViewSourceTest, HttpPostInSubframe) {
   EXPECT_THAT(source_text,
               HasSubstr("<h1>Request Body:</h1><pre>text=value</pre>"));
   EXPECT_THAT(source_text,
-              HasSubstr("<h1>Request Headers:</h1><pre>POST /echoall HTTP"));
+              HasSubstr("<pre id='request-headers'>POST /echoall HTTP"));
   EXPECT_THAT(source_text,
               ContainsRegex("Request Headers:.*Referer: " + form_url.spec()));
   EXPECT_THAT(

content/browser/frame_host/navigation_controller_impl_browsertest.cc

@@ -6710,11 +6710,20 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, PostViaOpenUrlMsg) {
 // https://crbug.com/860807.
 IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, UncacheablePost) {
   GURL main_url(embedded_test_server()->GetURL(
-      "/form_that_posts_to_echoall_nocache.html"));
+      "initial-page.example.com", "/form_that_posts_to_echoall_nocache.html"));
   EXPECT_TRUE(NavigateToURL(shell(), main_url));
   WebContents* web_contents = shell()->web_contents();
   EXPECT_EQ(0, web_contents->GetController().GetLastCommittedEntryIndex());
 
+  // Tweak the test page, so that it POSTs directly to the right cross-site URL
+  // (without going through the /cross-site-307/host.com handler, because it
+  // seems that such redirects do not preserve the Origin header).
+  GURL target_url(
+      embedded_test_server()->GetURL("another-site.com", "/echoall/nocache"));
+  ASSERT_TRUE(ExecJs(
+      web_contents,
+      JsReplace("document.getElementById('form').action = $1", target_url)));
+
   // Submit the form.
   TestNavigationObserver form_post_observer(web_contents, 1);
   EXPECT_TRUE(
@@ -6722,7 +6731,6 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, UncacheablePost) {
   form_post_observer.Wait();
 
   // Verify that we arrived at the expected location.
-  GURL target_url(embedded_test_server()->GetURL("/echoall/nocache"));
   EXPECT_EQ(target_url, web_contents->GetLastCommittedURL());
   EXPECT_EQ(1, web_contents->GetController().GetLastCommittedEntryIndex());
 
@@ -6746,12 +6754,16 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, UncacheablePost) {
   EXPECT_EQ("text=value\n", body);
 
   // Extract the response nonce.
-  std::string old_response_nonce;
-  std::string response_nonce_extraction_script = R"(
-      domAutomationController.send(
-          document.getElementById('response-nonce').innerText); )";
-  EXPECT_TRUE(ExecuteScriptAndExtractString(
-      web_contents, response_nonce_extraction_script, &old_response_nonce));
+  std::string old_response_nonce =
+      EvalJs(web_contents,
+             "document.getElementById('response-nonce').innerText")
+          .ExtractString();
+
+  // Verify that the Origin header correctly reflects the initial initiator.
+  EXPECT_THAT(EvalJs(web_contents,
+                     "document.getElementById('request-headers').innerText")
+                  .ExtractString(),
+              ::testing::HasSubstr("Origin: http://initial-page.example.com"));
 
   // Go back.
   {
@@ -6808,10 +6820,16 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, UncacheablePost) {
 
   // Extract the new response nonce and verify that it did change (e.g. that the
   // reload did load fresh content).
-  std::string new_response_nonce;
-  EXPECT_TRUE(ExecuteScriptAndExtractString(
-      web_contents, response_nonce_extraction_script, &new_response_nonce));
-  EXPECT_NE(new_response_nonce, old_response_nonce);
+  EXPECT_NE(old_response_nonce,
+            EvalJs(web_contents,
+                   "document.getElementById('response-nonce').innerText"));
+
+  // Verify that the Origin header correctly reflects the initial initiator.
+  // This is a regression test for https://crbug.com/915538.
+  EXPECT_THAT(EvalJs(web_contents,
+                     "document.getElementById('request-headers').innerText")
+                  .ExtractString(),
+              ::testing::HasSubstr("Origin: http://initial-page.example.com"));
 }
 
 // This test verifies that it is possible to reload a POST request that

content/browser/frame_host/navigation_request.cc

@@ -219,9 +219,13 @@ bool IsSecureFrame(FrameTreeNode* frame) {
 // This should match blink::ResourceRequest::needsHTTPOrigin.
 bool NeedsHTTPOrigin(net::HttpRequestHeaders* headers,
                      const std::string& method) {
-  // Don't add an Origin header if it is already present.
-  if (headers->HasHeader(net::HttpRequestHeaders::kOrigin))
-    return false;
+  // Blink version of this function checks if the Origin header might have
+  // already been added to |headers|.  This check is not replicated below
+  // because:
+  // 1. We want to overwrite the old (renderer-provided) header value
+  //    with a new, trustworthy (browser-provided) value.
+  // 2. The rest of the function matches the Blink version, so there should
+  //    be no discrepancies in the Origin value used.
 
   // Don't send an Origin header for GET or HEAD to avoid privacy issues.
   // For example, if an intranet page has a hyperlink to an external web
@@ -248,6 +252,7 @@ void AddAdditionalRequestHeaders(net::HttpRequestHeaders* headers,
                                  const std::string user_agent_override,
                                  bool has_user_gesture,
                                  base::Optional<url::Origin> initiator_origin,
+                                 network::mojom::ReferrerPolicy referrer_policy,
                                  FrameTreeNode* frame_tree_node) {
   if (!url.SchemeIsHTTPOrHTTPS())
     return;
@@ -321,23 +326,13 @@ void AddAdditionalRequestHeaders(net::HttpRequestHeaders* headers,
   }
 
   // Next, set the HTTP Origin if needed.
-  if (!NeedsHTTPOrigin(headers, method))
-    return;
-
-  // Create a unique origin.
-  url::Origin origin;
-  if (frame_tree_node->IsMainFrame()) {
-    // For main frame, the origin is the url currently loading.
-    origin = url::Origin::Create(url);
-  } else if ((frame_tree_node->active_sandbox_flags() &
-              blink::WebSandboxFlags::kOrigin) ==
-             blink::WebSandboxFlags::kNone) {
-    // The origin should be the origin of the root, except for sandboxed
-    // frames which have a unique origin.
-    origin = frame_tree_node->frame_tree()->root()->current_origin();
+  if (NeedsHTTPOrigin(headers, method)) {
+    url::Origin origin_header_value = initiator_origin.value_or(url::Origin());
+    origin_header_value = Referrer::SanitizeOriginForRequest(
+        url, origin_header_value, referrer_policy);
+    headers->SetHeader(net::HttpRequestHeaders::kOrigin,
+                       origin_header_value.Serialize());
   }
-
-  headers->SetHeader(net::HttpRequestHeaders::kOrigin, origin.Serialize());
 }
 
 // Should match the definition of
@@ -858,7 +853,7 @@ NavigationRequest::NavigationRequest(
         frame_tree_node_->navigator()->GetController()->GetBrowserContext(),
         common_params_->method, user_agent_override,
         common_params_->has_user_gesture, common_params_->initiator_origin,
-        frame_tree_node);
+        common_params_->referrer->policy, frame_tree_node);
 
     if (begin_params_->is_form_submission) {
       if (browser_initiated && !commit_params_->post_content_type.empty()) {

content/public/common/referrer.cc

@@ -106,6 +106,16 @@ blink::mojom::ReferrerPtr Referrer::SanitizeForRequest(
   return sanitized_referrer;
 }
 
+// static
+url::Origin Referrer::SanitizeOriginForRequest(
+    const GURL& request,
+    const url::Origin& initiator,
+    network::mojom::ReferrerPolicy policy) {
+  Referrer fake_referrer(initiator.GetURL(), policy);
+  Referrer sanitizied_referrer = SanitizeForRequest(request, fake_referrer);
+  return url::Origin::Create(sanitizied_referrer.url);
+}
+
 // static
 void Referrer::SetReferrerForRequest(net::URLRequest* request,
                                      const Referrer& referrer) {

content/public/common/referrer.h

@@ -35,6 +35,18 @@ struct CONTENT_EXPORT Referrer {
       const GURL& request,
       const blink::mojom::Referrer& referrer);
 
+  // Returns |initiator| origin sanitized by |policy| so that it can be used
+  // when requesting |request| URL.
+  //
+  // Note that the URL-based sanitization (e.g. SanitizeForRequest above) cannot
+  // be used for cases where the referrer URL is missing (e.g. about:blank) but
+  // the initiator origin still needs to be used (e.g. when calculating the
+  // value of the `Origin` header to use in a POST request).
+  static url::Origin SanitizeOriginForRequest(
+      const GURL& request,
+      const url::Origin& initiator,
+      network::mojom::ReferrerPolicy policy);
+
   static void SetReferrerForRequest(net::URLRequest* request,
                                     const Referrer& referrer);
 

net/test/embedded_test_server/default_handlers.cc

@@ -163,7 +163,7 @@ std::unique_ptr<HttpResponse> HandleEchoAll(const HttpRequest& request) {
 
   body +=
       "</pre>"
-      "<h1>Request Headers:</h1><pre>" +
+      "<h1>Request Headers:</h1><pre id='request-headers'>" +
       request.all_headers + "</pre>" +
       "<h1>Response nonce:</h1><pre id='response-nonce'>" +
       base::UnguessableToken::Create().ToString() + "</pre></body></html>";

third_party/blink/web_tests/external/wpt/fetch/origin/assorted.window-expected.txt

 This is a testharness.js-based test.
 PASS Origin header and 308 redirect
 PASS Origin header and GET navigation
-FAIL Origin header and POST navigation assert_equals: expected "http://web-platform.test:8001" but got "null"
+PASS Origin header and POST navigation
 PASS Origin header and POST same-origin navigation with Referrer-Policy no-referrer
 FAIL Origin header and POST same-origin fetch no-cors mode with Referrer-Policy no-referrer assert_equals: expected "null" but got "http://web-platform.test:8001"
 PASS Origin header and POST same-origin fetch cors mode with Referrer-Policy no-referrer