New issue
Advanced search Search tips

Issue 918660 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac , Fuchsia
Pri: 3
Type: Bug

Blocking:
issue 846346



Sign in to add a comment

Content scripts should be able to perform same-origin fetches

Project Member Reported by lukasza@chromium.org, Jan 2

Issue description

Content scripts injected into foo.com page should be able to perform same-origin fetches.  This is about to be broken by https://crrev.com/c/1354260 (see also issue 846346).  This will be broken because today request_initiator will be set to chrome-extension:// origin, which is cross-origin from foo.com.

This might be fixable by changing request_initiator to be the same as the webpage origin, but this is difficult to do because:
1. Requests from allowlisted extensions need to use a separate URLLoaderFactory - the factory is today picked based on request_initiator
2. We need to audit what other places depend on the current value of request_initiator.
 
Cc: jam@chromium.org mmenke@chromium.org
+mmenke@ and jam@ in case their NetworkService perspective might offer some insights here
BTW: The problem here seems relatively limited / rare - only a few of the extensions on the current allowlist try to fetch same-origin resources (and they will continue to work because they are on the allowlist).

Maybe we can remove separate URLLoaderFactories once the allowlist shrinks down?(removing separate factories would also address the same-origin fetches problem).
RE: removing separate factories would also address the same-origin fetches problem

Actually, this is incorrect.  Even with a single, default factory, CORB would block same-origin requests from extensions (because request_initiator=chrome-extension:// would be different from the target URL's origin).
Status: Started (was: Assigned)
creis@ suggested that CORB can make exceptions based on the process lock.  I have a CL based on this idea and it seems to help (and make new tests pass).  I'll upload the CL and send it out for review as soon as its dependencies land (one is currently in CQ).
Blocking: 846346
WIP CL @ https://crrev.com/c/1394432 (some tryjobs are red though...)
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 8

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

commit 0decf43552c265a08a6a73c16bc9b7f0df800b78
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Tue Jan 08 20:47:42 2019

Allow same-origin responses in CORB (even initiated by content scripts).

Consider the following scenario: A content script from
chrome-extension://example-extension injected into
https://example.com/page.html performs an XHR/fetch of
https://example.com/resource.json.

The scenario above has been problematic for CORB, because
CrossOriginReadBlocking::ResponseAnalyzer::ShouldBlockBasedOnHeaders
sees that |initiator| (chrome-extension://example-extension) is
different from |target_origin| (https://example.com).  Before this
CL, such case would be considered cross-origin and blocked (this didn't
happen in practice only because URLLoaderFactories for extensions had
CORB turned off).  After this CL, such case would be considered
same-origin based on a matching |request_initiator_site_lock| (this
change enables turning CORB on for extensions in a separate, later CL).

Change-Id: I367a5452f7aa080d590ff46bf4a57d1403ae80dd
Bug:  918660 
Reviewed-on: https://chromium-review.googlesource.com/c/1394432
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620866}
[modify] https://crrev.com/0decf43552c265a08a6a73c16bc9b7f0df800b78/chrome/browser/extensions/cross_origin_read_blocking_browsertest.cc
[modify] https://crrev.com/0decf43552c265a08a6a73c16bc9b7f0df800b78/content/browser/loader/cross_site_document_resource_handler.cc
[modify] https://crrev.com/0decf43552c265a08a6a73c16bc9b7f0df800b78/extensions/browser/url_loader_factory_manager.cc
[modify] https://crrev.com/0decf43552c265a08a6a73c16bc9b7f0df800b78/services/network/cross_origin_read_blocking.cc
[modify] https://crrev.com/0decf43552c265a08a6a73c16bc9b7f0df800b78/services/network/cross_origin_read_blocking.h
[modify] https://crrev.com/0decf43552c265a08a6a73c16bc9b7f0df800b78/services/network/initiator_lock_compatibility.cc
[modify] https://crrev.com/0decf43552c265a08a6a73c16bc9b7f0df800b78/services/network/initiator_lock_compatibility.h
[modify] https://crrev.com/0decf43552c265a08a6a73c16bc9b7f0df800b78/services/network/url_loader.cc
[modify] https://crrev.com/0decf43552c265a08a6a73c16bc9b7f0df800b78/services/network/url_loader.h
[modify] https://crrev.com/0decf43552c265a08a6a73c16bc9b7f0df800b78/third_party/blink/web_tests/external/wpt/fetch/corb/img-mime-types-coverage.tentative.sub.html
[modify] https://crrev.com/0decf43552c265a08a6a73c16bc9b7f0df800b78/third_party/blink/web_tests/external/wpt/fetch/corb/img-png-mislabeled-as-html-nosniff.tentative.sub.html
[modify] https://crrev.com/0decf43552c265a08a6a73c16bc9b7f0df800b78/third_party/blink/web_tests/external/wpt/fetch/corb/preload-image-png-mislabeled-as-html-nosniff.tentative.sub.html
[modify] https://crrev.com/0decf43552c265a08a6a73c16bc9b7f0df800b78/third_party/blink/web_tests/external/wpt/fetch/corb/script-html-correctly-labeled.tentative.sub.html
[modify] https://crrev.com/0decf43552c265a08a6a73c16bc9b7f0df800b78/third_party/blink/web_tests/external/wpt/fetch/corb/script-resource-with-json-parser-breaker.tentative.sub.html
[modify] https://crrev.com/0decf43552c265a08a6a73c16bc9b7f0df800b78/third_party/blink/web_tests/external/wpt/html/semantics/document-metadata/the-link-element/resources/link-style-error.js

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 8

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

commit 763a3b389cf197921f1718bc7db7d6ede8e3024b
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Tue Jan 08 23:40:16 2019

Enable BypassCorbOnlyForExtensionsAllowlist feature.

Bug:  918660 
Change-Id: Ieb2c3434dc7ca8ec4d20f6e1daefa67e2202cd81
Reviewed-on: https://chromium-review.googlesource.com/c/1398803
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620935}
[modify] https://crrev.com/763a3b389cf197921f1718bc7db7d6ede8e3024b/extensions/common/extension_features.cc

Status: Fixed (was: Started)

Sign in to add a comment