Currently CORB makes an exception for content script and this exception is based on checking the initiator's scheme (and comparing it against chrome-extension://).  This probably means that this bug is blocked on issue 846346 (which talks about having a separate URLLoaderFactory for content scripts - one with a separate initiator and/or separate CORB exceptions).

A partial attempt to lock down the request initiator is here: https://crrev.com/c/1234967

+alexmos@ who is investigating whether html imports 1) from foo.com 2) of bar.com components, would use foo.com VS bar.com in their Origin header and/or network::ResourceRequest::request_initiator field when making requests on behalf of the bar.com component.

The default URLLoaderFactory for a frame is created at ready-to-commit time.  If we wanted to lock such factory to a specific request_initiator origin, then it would have to be done at ready-to-commit time.  The problem is that today, the browser process doesn't know which origin will commit - fixing this depends on the "precursor origin" work.

#4: the test I was investigating here was http/tests/htmlimports/import-script-block-crossorigin-dynamic.html.  IIUC, the test works as follows (sample cmdline to run it at [1]):

1. Starts running at http://127.0.0.1:8000 and gets locked to this origin when running with --site-per-process.
2. Does a html import for http://localhost:8000/htmlimports/resources/cors-having-crossorigin-scripts.cgi.  This is allowed to go through because the response has Access-Control-Allow-Origin: http://127.0.0.1:8000.
3. The html import dynamically injects three external scripts into the imported document (i.e., into document.currentScript.ownerDocument, distinct from document that was loaded in step 1).  
4. These scripts are all at http://127.0.0.1:8000/ and differ in their CORS responses: 
  a. http://127.0.0.1:8000/htmlimports/resources/external-script.js has no CORS headers
  b. http://127.0.0.1:8000/htmlimports/resources/cors-js.cgi has "Access-Control-Allow-Origin: http://127.0.0.1:8000".
  c. http://127.0.0.1:8000/htmlimports/resources/cors-js-for-localhost.cgi has "Access-Control-Allow-Origin: http://localhost:8000"
5. The test expects scripts (a) and (b) to *fail* loading, and (c) to succeed.

For all three scripts, the network request comes through with a network::ResourceRequest::request_initiator of "http://localhost:8000", while in a process locked to http://127.0.0.1:8000.

I'm concerned how this affects computing the request_initiator in the browser process, like in comment #3: it seems a renderer for foo.com can include html imports from arbitrary sites which can then pose as request initiators.

[1] third_party/blink/tools/run_web_tests.py --additional-driver-flag=--site-per-process --no-show-results --driver-logging -t <your_out_dir> http/tests/htmlimports/import-script-block-crossorigin-dynamic.html

Maybe some good news re: #6: according to https://www.chromestatus.com/features/5144752345317376, HTML Imports are deprecated at M70 and will be removed in M73.

+yoichio@ who AFAIK works on deprecation of HTML imports in M73 (per 
https://groups.google.com/a/chromium.org/d/topic/blink-dev/h-JwMiPUnuU/discussion and https://www.chromestatus.com/feature/5144752345317376).

HTML imports make it difficult to realize full security benefits of Site Isolation, because even if a renderer process is locked to a single site (say foo.com) it may still issue subresource requests for another site (say bar.com) because of HTML imports.

Going forward, I'd like to have Cross-Origin Read Blocking (CORB) depend on the trustworthy |request_initiator_origin_lock| (being introduced in https://crrev.com/1234967) instead of depending on untrustworthy |request_initiator| coming from the renderer.  This seems likely to break a scenario where foo.com uses a HTML import from bar.com and the import tries to use fetch API to fetch CORB-protected resource (e.g. JSON or XML) from bar.com (I assume that such fetch is possible via HTML imported scripts + that the fetch would be considered same-origin and wouldn't require CORS).  If CORB depends on |request_initiator_origin_lock| then it would block the JSON/XML response from bar.com, because the request would be initiated from a process locked to foo.com.

Given above, I hope deprecation of HTML imports can proceed as planned (i.e. without any further delays).  Looking at the deprecation thread, I see that while the feature is planned to be deprecated in M73, it will still be available via reverse origin trials until 2020Q1 - this makes me sad (I'll try to think if any workarounds are possible for CORB;  any suggestions are welcome).

The fact that HTML Imports can cause a security issue is a significant motivation to deprecate the feature though the usage is still high.

Thanks for the investigation.

Removing Restrict-View-Google - there is no sensitive information in the bug.  In particular, this bug tracks a step needed to implement a *new* security feature / *new* security guarantees (rather than tracking a security bug in an existing security feature / guarantee).

Note to self (and to nasko@): I've verified that subresource requests made from a sandboxed frame use request_initiator set to an opaque/unique origin.

REPRO STEPS:

0. Add ad-hoc logging to network::URLLoader's constructor to log request_initiator
   Launch Chrome with --enable-features=NetworkService

1. Navigate to example.com

2. In DevTools, in main frame:
     var frame = document.createElement('iframe')
     frame.sandbox = "allow-scripts"
     document.body.appendChild(frame)

3. In DevTools, in the new subframe:
     window.origin  // optional: verify that the frame has opaque/unique/null origin
     
     // verify that CORS + CORB kick-in (and observe the request_initiator):
     fetch('http://example.com')
          .then(() => console.log('success'))
          .catch(e => console.log('error: ' + e));

     // verify that CORB kicks in (and observe the request_initiator):
     fetch('http://example.com', {mode: "no-cors"})
          .then(() => console.log('success'))
          .catch(e => console.log('error: ' + e));

EXPECTED=ACTUAL BEHAVIOR:

A. CORS + CORB are kicking in
B. request_initiator is set to an opaque/unique/null origin:

    [...:url_loader.cc(349)] URLLoader::ctor; request.url = http://example.com/; request_initiator = null [internally: (EE9A09A...186365) anonymous]

This bug is marked as being blocked by issue 888079 (Browser process should calculate the origin to commit).  Let me expand that a little bit below.


Main problem is that at the time of RenderFrameHostImpl::CommitNavigation, the browser process needs to know or compute request_initiator_origin_lock to be used by the default URLLoaderFactory that will be sent down to the renderer process as part of the commit IPC.  This is problematic in case of about:blank pages - the origin they will commit with is currently not known until DidCommit IPC from the renderer.

Examples of about:blank navigations that go through RenderFrameHostImpl::CommitNavigation and need to commit with a non-opaque origin:
- FrameTreeBrowserTest.NavigateChildToAboutBlank a b.com frame uses window.open('about:blank', 'targetFrame') to navigate its remote c.com frame to about:blank (in b.com origin).
- NavigationControllerBrowserTest.FrameNavigationEntry_BackWithRedirect navigates to about:blank during a browser-initiated "back" navigation.


Therefore, until precursor origins and issue 888079 are resolved, RenderFrameHostImpl::CommitNavigation may need to continue falling back to the site URL:

1. Note that without site-per-process site URL is an okay fallback for current code (e.g. for deciding whether to turn CORB on or off), but is *not* a good fallback for |request_initiator_origin_lock|:

1.1. For example https://foo.com may commit in a process associated with http://bar.com SiteInstance (difference in scheme and hostname is intentional) - in this case we don't want enforce requests from https://foo.com by treating them as if they came from an opaque origin).

1.2. Another example is process reuse for extensions (we don't want to stamp an opaque request_initiator, because that may break OOR-CORS that needs to look at extension-specific CORS exemptions).

1.3. We may consider working around that by only enforcing |request_initiator_origin_lock| on desktop platforms (ignoring switches::kDisableSiteIsolation) - not sure if this is a good option.

2. Also note that the network service is not able to translate |request_initiator| into a site URL.  This means that the enforcement of |request_initiator_origin_lock| will need to be relaxed (e.g. limited to scheme-equality checks and eTLD+1 checks for http/https locks).

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1e686969e6098169e9306ae8cbc245e80c416dad

commit 1e686969e6098169e9306ae8cbc245e80c416dad
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri Dec 07 01:14:19 2018

Introducing URLLoaderFactoryParams::request_initiator_origin_lock.

This CL is a step toward the world where we don't trust the origin
reported by the renderer in network::ResourceRequest::request_initiator
and instead know the request initiator a priori (e.g. it gets computed
by the browser process).

This CL only logs UMA that says whether the newly introduced lock is
compatible with the request initiator requested by the user of the
NetworkService (e.g. by the renderer process).  This CL doesn't yet
enforce the request_initiator_origin_lock - the enforcement will be
enabled in a separate, follow-up CL.

This CL builds upon the fact that in most cases
RenderProcessHostImpl::CreateURLLoaderFactory knows which origin will
use the factory to initiate requests.  In presence of NetworkService,
this knowledge allows locking of URLLoaderFactory to this specific
origin.

The only cases where the origin is not known should eventually disappear
after https://crbug.com/891872 is dealt with.  Note that CL has to deal
with the fact that because of https://crbug.com/888079 sometimes only a
site URL (rather than origin) may be known at the commit time.  This is
why the lock is temporarily called |request_initiator_site_lock| rather
than |request_initiator_origin_lock|.

Bug: 871827
Change-Id: I97cc8285ace65e072bd7780da45522b10cb5ba57
Reviewed-on: https://chromium-review.googlesource.com/c/1351574
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614535}
[modify] https://crrev.com/1e686969e6098169e9306ae8cbc245e80c416dad/content/browser/DEPS
[modify] https://crrev.com/1e686969e6098169e9306ae8cbc245e80c416dad/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/1e686969e6098169e9306ae8cbc245e80c416dad/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/1e686969e6098169e9306ae8cbc245e80c416dad/content/browser/loader/cross_site_document_blocking_browsertest.cc
[modify] https://crrev.com/1e686969e6098169e9306ae8cbc245e80c416dad/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/1e686969e6098169e9306ae8cbc245e80c416dad/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/1e686969e6098169e9306ae8cbc245e80c416dad/content/public/browser/render_process_host.h
[modify] https://crrev.com/1e686969e6098169e9306ae8cbc245e80c416dad/content/public/test/mock_render_process_host.cc
[modify] https://crrev.com/1e686969e6098169e9306ae8cbc245e80c416dad/content/public/test/mock_render_process_host.h
[modify] https://crrev.com/1e686969e6098169e9306ae8cbc245e80c416dad/extensions/browser/url_loader_factory_manager.cc
[modify] https://crrev.com/1e686969e6098169e9306ae8cbc245e80c416dad/services/network/public/mojom/network_context.mojom
[modify] https://crrev.com/1e686969e6098169e9306ae8cbc245e80c416dad/services/network/url_loader.cc
[modify] https://crrev.com/1e686969e6098169e9306ae8cbc245e80c416dad/services/network/url_loader.h
[modify] https://crrev.com/1e686969e6098169e9306ae8cbc245e80c416dad/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/1e686969e6098169e9306ae8cbc245e80c416dad/tools/metrics/histograms/histograms.xml

Hmmm... in #c11 I wanted to remove Restrict-View-Google, but didn't actually do it... :-/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2d3f85193a309457c923e1edc053c5d7a9d214e6

commit 2d3f85193a309457c923e1edc053c5d7a9d214e6
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Thu Jan 03 06:39:30 2019

Enforce |request_initiator_site_lock| for Cross-Origin Read Blocking.

In the long-term we want |request_initiator_site_lock| to take
precedence over |request_initiator| everywhere in the NetworkService.
This is risky however, because there are known cases where
|request_initiator| may differ from |request_initiator_site_lock| even
without a malicious/compromised renderer in the picture (e.g. HTML
Imports).  Therefore, in the short-term, we start with enforcing
|request_initiator_site_lock| only for Cross-Origin Read Blocking
(CORB) - this is safer because:
- CORB only applies to a subset of requests and so this approach seems
  safer and okay to enable by default (while measuring the impact via UMA
  and having a kill switch ready).
- CORB is only web-observable when blocking subresource requests
  trigerred by XHR/fetch API (and these requests seem to always use a
  |request_initiator| compatible with |request_initiator_site_lock|;
  this is covered by new tests added by this CL).

Bug: 871827
Change-Id: If9459bfbd09411d70c3547de0e50a58832e75503
Reviewed-on: https://chromium-review.googlesource.com/c/1377385
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619597}
[modify] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/content/browser/DEPS
[modify] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/content/browser/loader/cross_site_document_blocking_browsertest.cc
[modify] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/content/browser/loader/cross_site_document_resource_handler.cc
[add] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/content/test/data/cross_site_document_blocking/fetch_nosniff_json.js
[add] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/content/test/data/cross_site_document_blocking/html_import.html
[add] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/content/test/data/cross_site_document_blocking/html_import.html.mock-http-headers
[add] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/content/test/data/cross_site_document_blocking/html_import2.html
[add] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/content/test/data/cross_site_document_blocking/html_import2.html.mock-http-headers
[add] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/content/test/data/cross_site_document_blocking/html_import3.html
[add] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/content/test/data/cross_site_document_blocking/html_import3.html.mock-http-headers
[modify] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/services/network/BUILD.gn
[modify] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/services/network/OWNERS
[modify] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/services/network/cross_origin_read_blocking.cc
[modify] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/services/network/cross_origin_read_blocking.h
[add] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/services/network/initiator_lock_compatibility.cc
[add] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/services/network/initiator_lock_compatibility.h
[add] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/services/network/initiator_lock_compatibility_unittest.cc
[modify] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/services/network/public/cpp/features.cc
[modify] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/services/network/public/cpp/features.h
[modify] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/services/network/url_loader.cc
[modify] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/services/network/url_loader.h
[modify] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/services/network/url_loader_unittest.cc
[modify] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/2d3f85193a309457c923e1edc053c5d7a9d214e6/tools/metrics/histograms/histograms.xml

We've found good workarounds and so this issue is no longer blocked by issue 888079 and issue 891872 (there issues only block switching from site to origin, but they don't block CORB enforcement / unbypassability).  I've opened a separate bug to track switching to using an origin instead of site - issue 918967.

The CL in #c19 (Enforce |request_initiator_site_lock| for Cross-Origin Read Blocking) landed in 73.0.3661.0.  We should monitor the newly added SiteIsolation.XSD.NetworkService.InitiatorLockCompatibility for the next few weeks.

SiteIsolation.XSD.NetworkService.InitiatorLockCompatibility shows that BlockingWhenIncorrectLock is non-zero as expected - this is probably caused by HTML Imports using <script> or <img> tag that points to non-script/non-image.  The impact is very low though:
- BlockingWhenIncorrectLock is <0.5% of BenignBlocking+BlockingWhenCompatibleLock
- BlockingWhenIncorrectLock is <0.005% of NoBlocking