OOR-CORS: Support Extensions' File URL allowlisting |
|||||||||||
Issue descriptionSimilar to crbug.com/870172 , we need to implement allowlisting for file URL. This is optional if NetworkService is disabled, but needs to work with NetworkService correctly.
,
Nov 28
,
Nov 29
See also: issue 895999
,
Nov 29
Hum...., the originally reported extension works with NetworkService, and does not with NetworkService and OOR-CORS. Access to XMLHttpRequest at '<url>' from origin 'chrome-extension://<id>' has been blocked by CORS policy: Cross origin requests are only supported for protocol schemes: http, data, chrome, chrome-extension, https. I assumed we allow all access via FileURLLoader in this mode, but it looks we have another point to reject it.
,
Nov 29
void FileURLLoaderFactory::CreateLoaderAndStart(
network::mojom::URLLoaderRequest loader,
int32_t routing_id,
int32_t request_id,
uint32_t options,
const network::ResourceRequest& request,
network::mojom::URLLoaderClientPtr client,
const net::MutableNetworkTrafficAnnotationTag& traffic_annotation) {
...
// FileURLLoader doesn't support CORS and it's not covered by CorsURLLoader,
// so we need to reject requests that need CORS manually.
if (ShouldFailRequestDueToCors(request)) {
client->OnComplete(
network::URLLoaderCompletionStatus(network::CorsErrorStatus(
network::mojom::CorsError::kCorsDisabledScheme)));
return;
}
...
ShouldFailRequestDueToCors returns false when OOR-CORS is disabled.
,
Nov 29
Ah, OK. yhirano@'s patch disabled its check only for non-OOR-CORS.
,
Nov 29
Fix is easy, but will depend on https://chromium-review.googlesource.com/c/chromium/src/+/1351208
,
Dec 3
,
Dec 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/deddf2bb8f2e519cf9f12d5f77139b3f14fb8236 commit deddf2bb8f2e519cf9f12d5f77139b3f14fb8236 Author: Takashi Toyoshima <toyoshim@chromium.org> Date: Tue Dec 04 14:51:28 2018 OOR-CORS: Origin access list support for FileURLLoader FileURLLoader does not know CORS access list for now, but it is needed to decide if the request is permitted by the access list beyond the CORS policy. Now, BrowserContext manages per-profile CORS access list. FileURLLoader also needs to access the list for this CORS exemption. Bug: 906536 , 895999 Change-Id: I3123de282d6eab5299388b902c19c96f434f4568 Reviewed-on: https://chromium-review.googlesource.com/c/1351321 Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#613547} [modify] https://crrev.com/deddf2bb8f2e519cf9f12d5f77139b3f14fb8236/content/browser/download/download_manager_impl.cc [modify] https://crrev.com/deddf2bb8f2e519cf9f12d5f77139b3f14fb8236/content/browser/download/file_download_url_loader_factory_getter.cc [modify] https://crrev.com/deddf2bb8f2e519cf9f12d5f77139b3f14fb8236/content/browser/download/file_download_url_loader_factory_getter.h [modify] https://crrev.com/deddf2bb8f2e519cf9f12d5f77139b3f14fb8236/content/browser/file_url_loader_factory.cc [modify] https://crrev.com/deddf2bb8f2e519cf9f12d5f77139b3f14fb8236/content/browser/file_url_loader_factory.h [modify] https://crrev.com/deddf2bb8f2e519cf9f12d5f77139b3f14fb8236/content/browser/frame_host/render_frame_host_impl.cc [modify] https://crrev.com/deddf2bb8f2e519cf9f12d5f77139b3f14fb8236/content/browser/loader/navigation_url_loader_impl.cc [modify] https://crrev.com/deddf2bb8f2e519cf9f12d5f77139b3f14fb8236/content/browser/worker_host/worker_script_fetch_initiator.cc [modify] https://crrev.com/deddf2bb8f2e519cf9f12d5f77139b3f14fb8236/content/public/browser/file_url_loader.h [modify] https://crrev.com/deddf2bb8f2e519cf9f12d5f77139b3f14fb8236/extensions/browser/updater/extension_downloader.cc
,
Dec 5
,
Dec 5
Pls apply appropriate OSs label. Thank you.
,
Dec 5
,
Dec 6
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 6
,
Dec 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b96c8462ac1f9070122df7597b665de78f31a544 commit b96c8462ac1f9070122df7597b665de78f31a544 Author: Takashi Toyoshima <toyoshim@chromium.org> Date: Thu Dec 06 08:23:01 2018 OOR-CORS: Origin access list support for FileURLLoader FileURLLoader does not know CORS access list for now, but it is needed to decide if the request is permitted by the access list beyond the CORS policy. Now, BrowserContext manages per-profile CORS access list. FileURLLoader also needs to access the list for this CORS exemption. Bug: 906536 , 895999 Change-Id: I3123de282d6eab5299388b902c19c96f434f4568 Reviewed-on: https://chromium-review.googlesource.com/c/1351321 Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#613547}(cherry picked from commit deddf2bb8f2e519cf9f12d5f77139b3f14fb8236) TBR=yhirano@chromium.org Change-Id: I3123de282d6eab5299388b902c19c96f434f4568 Reviewed-on: https://chromium-review.googlesource.com/c/1364973 Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#97} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/b96c8462ac1f9070122df7597b665de78f31a544/content/browser/download/download_manager_impl.cc [modify] https://crrev.com/b96c8462ac1f9070122df7597b665de78f31a544/content/browser/download/file_download_url_loader_factory_getter.cc [modify] https://crrev.com/b96c8462ac1f9070122df7597b665de78f31a544/content/browser/download/file_download_url_loader_factory_getter.h [modify] https://crrev.com/b96c8462ac1f9070122df7597b665de78f31a544/content/browser/file_url_loader_factory.cc [modify] https://crrev.com/b96c8462ac1f9070122df7597b665de78f31a544/content/browser/file_url_loader_factory.h [modify] https://crrev.com/b96c8462ac1f9070122df7597b665de78f31a544/content/browser/frame_host/render_frame_host_impl.cc [modify] https://crrev.com/b96c8462ac1f9070122df7597b665de78f31a544/content/browser/loader/navigation_url_loader_impl.cc [modify] https://crrev.com/b96c8462ac1f9070122df7597b665de78f31a544/content/browser/worker_host/worker_script_fetch_initiator.cc [modify] https://crrev.com/b96c8462ac1f9070122df7597b665de78f31a544/content/public/browser/file_url_loader.h [modify] https://crrev.com/b96c8462ac1f9070122df7597b665de78f31a544/extensions/browser/updater/extension_downloader.cc
,
Dec 6
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b96c8462ac1f9070122df7597b665de78f31a544 Commit: b96c8462ac1f9070122df7597b665de78f31a544 Author: toyoshim@chromium.org Commiter: toyoshim@chromium.org Date: 2018-12-06 08:23:01 +0000 UTC OOR-CORS: Origin access list support for FileURLLoader FileURLLoader does not know CORS access list for now, but it is needed to decide if the request is permitted by the access list beyond the CORS policy. Now, BrowserContext manages per-profile CORS access list. FileURLLoader also needs to access the list for this CORS exemption. Bug: 906536 , 895999 Change-Id: I3123de282d6eab5299388b902c19c96f434f4568 Reviewed-on: https://chromium-review.googlesource.com/c/1351321 Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#613547}(cherry picked from commit deddf2bb8f2e519cf9f12d5f77139b3f14fb8236) TBR=yhirano@chromium.org Change-Id: I3123de282d6eab5299388b902c19c96f434f4568 Reviewed-on: https://chromium-review.googlesource.com/c/1364973 Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#97} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by toyoshim@chromium.org
, Nov 19