Set |request_initiator| to the website, not to the content script's origin.
Desirability of setting |request_initiator| to the website
==========================================================
At a high-level, this change means that content script scripts behave more like the
webpage that they are injected to. This kind of high-level change is desirable
because:
1) the renderer process hosting the webpage is locked to the webpage's
origin - anything running in such renderer (including content
scripts) should be bound by webpage's capabilities
2) it simplifies reasoning about content scripts for extension authors
(after this change content scripts run as webpage's origin [unless
allowlisted]; this makes it easier for content scripts to issue
SameSite and/or CORS requests on behalf the webpage)
At a low-level, this change allows cleaning up some technical debt
related to how same-*web*-origin XHRs have to be handled when initiated
from content scripts. In particular:
A) after this CL, we can remove hacks that OOR-CORS and CORB currently
use to (heuristically, inaccurately) attempt to recognize same-origin
requests (i.e. we can consider reverting https://crrev.com/c/1642752
and https://crrev.com/c/1394432). For more detailed comparison of
how CORB, OOR-CORS and Sec-Fetch-Site behavior is affected by this
CL, see the comparison table in
https://docs.google.com/document/d/1ENNu-kRSCrgfaqrGO8T-lUqvItnS6VYw-jAdtRVUXic/edit#heading=h.h4a3p9fb5fo6
B) after this CL, requests initiated on behalf of the translate feature
will use a request_initiator compatible with
request_initiator_site_lock. This means that the translate feature
no longer needs to provide a separate URLLoaderFactory (i.e. we can
consider reverting https://crrev.com/c/1441277 and follow-ups).
C) after this CL, we no longer need to suppress attaching of the Origin
header for requests initiated by a content script
(i.e. we can consider reverting https://crrev.com/c/1610667).
Main change
===========
The main change in this CL is in
third_party/blink/renderer/core/fetch/request.cc.
After this CL, |request->SetOrigin(...)| will be unconditionally called
with the main world's origin.
Retaining isolated world's origin
=================================
The isolated world's origin needs to be retained for 2 reasons:
i) For selecting the right URLLoaderFactory in case the extension
is allowlisted (see URLLoaderFactoryBundle::GetFactory in
//third_party/blink/common/loader/url_loader_factory_bundle.cc).
ii) For finding CORS extension policies (see changes related
to OriginAccessList)
iii) For allowing the content scripts to access blob URLs belonging
to *either* of 1) the main world or 2) the extension
(see changes related to CanDisplay)
Because of the above the CL introduces the new fields:
- network::ResourceRequest::isolated_world_origin
- blink::WebURLRequest::IsolatedWorldOrigin
- blink::ResourceRequest::IsolatedWorldOrigin
- blink::FetchRequestData::IsolatedWorldOrigin
While it may seem that having two origin fields is more confusing, it is
better than the old state because:
*) There really *are* two origins associated with each request. Trying
to represent this reality with just one origin in code is confusing
and error-prone (as evidenced by the technical debt listed above).
**) Forgetting to consider the other origin (e.g. isolated world origin)
is safe - it might lead to functional bugs but shouldn't lead to
security bugs. If code forgets to consider the other origin then it
will only consider and/or grant capabilities of the web origin
(which should be safe from security perspective).
Bug: 940068
Change-Id: I9c3638070861e760227fb62af63f3d8a2c655f88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1633232
Reviewed-by: Devlin <[email protected]>
Reviewed-by: Kinuko Yasuda <[email protected]>
Reviewed-by: Nasko Oskov <[email protected]>
Reviewed-by: Yutaka Hirano <[email protected]>
Commit-Queue: Łukasz Anforowicz <[email protected]>
Cr-Commit-Position: refs/heads/master@{#694827}
diff --git a/chrome/browser/extensions/extension_unload_browsertest.cc b/chrome/browser/extensions/extension_unload_browsertest.cc
index 8fa6d34c..d99aee2 100644
--- a/chrome/browser/extensions/extension_unload_browsertest.cc
+++ b/chrome/browser/extensions/extension_unload_browsertest.cc
@@ -112,8 +112,9 @@
GURL test_url = embedded_test_server()->GetURL("/title1.html");
ui_test_utils::NavigateToURL(browser(), test_url);
- // Sending an XHR with the extension's Origin header should succeed when the
- // extension is installed.
+ // The content script sends an XHR with the webpage's (rather than
+ // extension's) Origin header - this should succeed (given that
+ // xhr.txt.mock-http-headers says `Access-Control-Allow-Origin: *`).
const char kSendXhrScript[] = "document.getElementById('xhrButton').click();";
bool xhr_result = false;
EXPECT_TRUE(content::ExecuteScriptAndExtractBool(
@@ -129,12 +130,13 @@
test_url,
browser()->tab_strip_model()->GetWebContentsAt(0)->GetLastCommittedURL());
- // Sending an XHR with the extension's Origin header should fail but not kill
- // the tab.
+ // The content script sends an XHR with the webpage's (rather than
+ // extension's) Origin header - this should succeed (given that
+ // xhr.txt.mock-http-headers says `Access-Control-Allow-Origin: *`).
EXPECT_TRUE(content::ExecuteScriptAndExtractBool(
browser()->tab_strip_model()->GetActiveWebContents(), kSendXhrScript,
&xhr_result));
- EXPECT_FALSE(xhr_result);
+ EXPECT_TRUE(xhr_result);
// Ensure the process has not been killed.
EXPECT_TRUE(browser()
