CORB: Need to ensure that requests from plugins are not blocked |
||||||||
Issue description
CrossSiteDocumentResourceHandler tries to ensure that prefetching/caching still works for responses blocked by Cross-Origin Read Blocking (CORB):
bool CrossSiteDocumentResourceHandler::ShouldBlockBasedOnHeaders(
const network::ResourceResponse& response) {
...
// Don't block plugin requests with universal access (e.g., Flash). Such
// requests are made without CORS, and thus dont have an Origin request
// header. Other plugin requests (e.g., NaCl) are made using CORS and have an
// Origin request header. If they fail the CORS check above, they should be
// blocked.
if (info->GetResourceType() == RESOURCE_TYPE_PLUGIN_RESOURCE &&
is_nocors_plugin_request_) {
return false;
}
There are 2 things missing AFAIK:
1. There is no regresion test for this behavior
2. I am not sure how to preserve this behavior in CORB/NetworkService integration
- NetworkService layer isn't aware of the resource type
- Quite possibly plugins should get a separate URLLoaderFactory that is initialized
with URLLoaderFactoryParams::is_corb_enabled set to false. OTOH, I think all
non-browser factories today set is_corb_enabled to true: RPHI::CreateURLLoaderFactory
and RFHI::CreateNetworkServiceDefaultFactoryAndObserve.
,
Jun 14 2018
WIP CL with a test that highlights the problem: https://crrev.com/c/1101300
,
Jun 14 2018
Some notes on how the request is handled: 1) The plugin process (URLLoaderResource::Open) sends an IPC (PpapiHostMsg_URLLoader_Open) with a URL request to the renderer process (PepperURLLoaderHost::OnHostMsgOpen). 2) PepperURLLoaderHost::InternalOnHostMsgOpen in the renderer process checks |has_universal_access_| and then builds a regular renderer URL request (handled either by the browser process, or in the network-service process). The new URL request is marked as WebURLRequest::kRequestContextPlugin (later translated into RESOURCE_TYPE_PLUGIN_RESOURCE). Printf-debugging output confirming different PIDs (browser=123931, plugin=124012, renderer=123974): [123931:123931:0614/115136.740933:WARNING:password_store_factory.cc(250)] ... ... [124012:124012:0614/115137.506697:ERROR:url_loader_resource.cc(104)] ##### Sending PpapiHostMsg_URLLoader_Open [123974:123974:0614/115137.507350:ERROR:pepper_url_loader_host.cc(202)] ##### Received PpapiHostMsg_URLLoader_Open
,
Jun 14 2018
,
Jun 14 2018
As long as plugin requests are routed through the renderer process, an exploited renderer (where an attacker has ability to execute arbitrary code) will be able to disable CORB (by pretending that it is acting on behalf of a plugin). Therefore, vending URLLoaderFactory directly from the browser process to the plugin process is necessary to ensure that Site Isolation protects against exploited renderers.
,
Jun 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f4345313caebb6bb561fe3983531f29a4a37eb4b commit f4345313caebb6bb561fe3983531f29a4a37eb4b Author: Lukasz Anforowicz <lukasza@chromium.org> Date: Mon Jun 18 18:23:38 2018 New test: Plugins (e.g. Flash) VS Cross-Origin Read Blocking (CORB). This CL adds tests that verify that Cross-Origin Read Blocking (CORB) won't block HTTP responses for requests initiated by plugins that have been granted universal access (like Flash which has its own CORS-like mechanism - crossdomain.xml). In addition to running tryjobs, I've also tested this CL by temporarily editing CrossSiteDocumentResourceHandler::ShouldBlockBasedOnHeaders to comment out the exception carved out for plugins (the new TestTrustedCorbEligibleRequest test expectedly failed with such an edit both with and without NetworkService). Bug: 846339 Change-Id: I7b7befff3018ed5a7bd834b2bf0507c2e1f6365b Reviewed-on: https://chromium-review.googlesource.com/1101300 Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Raymes Khoury <raymes@chromium.org> Cr-Commit-Position: refs/heads/master@{#568081} [modify] https://crrev.com/f4345313caebb6bb561fe3983531f29a4a37eb4b/chrome/test/ppapi/ppapi_browsertest.cc [modify] https://crrev.com/f4345313caebb6bb561fe3983531f29a4a37eb4b/content/browser/loader/cross_site_document_resource_handler.cc [modify] https://crrev.com/f4345313caebb6bb561fe3983531f29a4a37eb4b/ppapi/BUILD.gn [add] https://crrev.com/f4345313caebb6bb561fe3983531f29a4a37eb4b/ppapi/tests/corb_eligible_resource.json [add] https://crrev.com/f4345313caebb6bb561fe3983531f29a4a37eb4b/ppapi/tests/corb_eligible_resource.json.mock-http-headers [modify] https://crrev.com/f4345313caebb6bb561fe3983531f29a4a37eb4b/ppapi/tests/test_url_loader.cc [modify] https://crrev.com/f4345313caebb6bb561fe3983531f29a4a37eb4b/ppapi/tests/test_url_loader.h [modify] https://crrev.com/f4345313caebb6bb561fe3983531f29a4a37eb4b/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
Jun 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/aec5f6d9327ca87529dab3b641615173efd3822c commit aec5f6d9327ca87529dab3b641615173efd3822c Author: Lukasz Anforowicz <lukasza@chromium.org> Date: Thu Jun 21 19:03:25 2018 Exclude plugins in NetworkService-version of Cross-Origin Read Blocking. This CL exclude requests made from plugins (e.g. from Flash) in the NetworkService implementation of Cross-Origin Read Blocking (CORB). Bug: 846339 Cq-Include-Trybots: luci.chromium.try:linux_mojo Change-Id: Iaa191f6805250fcc6f88e5042b5d149e7cfd216c Reviewed-on: https://chromium-review.googlesource.com/1101965 Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Cr-Commit-Position: refs/heads/master@{#569346} [modify] https://crrev.com/aec5f6d9327ca87529dab3b641615173efd3822c/components/mirroring/service/session.cc [modify] https://crrev.com/aec5f6d9327ca87529dab3b641615173efd3822c/content/browser/loader/cross_site_document_resource_handler.cc [modify] https://crrev.com/aec5f6d9327ca87529dab3b641615173efd3822c/content/public/browser/site_isolation_policy.cc [modify] https://crrev.com/aec5f6d9327ca87529dab3b641615173efd3822c/services/network/public/mojom/network_context.mojom [modify] https://crrev.com/aec5f6d9327ca87529dab3b641615173efd3822c/services/network/url_loader.cc [modify] https://crrev.com/aec5f6d9327ca87529dab3b641615173efd3822c/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
Jun 21 2018
At this point I think this issue should no longer block trials of the Network Service - let's mark it as fixed and track follow-up work in issue 855171 (Plugins should use a separate URLLoaderFactory).
,
Jun 21 2018
,
Jun 26 2018
,
Jun 26 2018
This is fixed, so is not really blocking trials of NetworkService, but let me associate this with issue 792546 for easier discoverability of this bug. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by lukasza@chromium.org
, May 24 2018