New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 829412 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug


Sign in to add a comment

WebUI contexts should not make network requests

Project Member Reported by roc...@chromium.org, Apr 5 2018

Issue description

As we transition to the Network Service we want to phase out any support for WebUI resources making direct network requests (e.g. XHRs, subresource loads)

Discussion here: https://groups.google.com/a/chromium.org/forum/#!searchin/network-service-dev/WebUI%7Csort:date/network-service-dev/R0BQIXtAOhQ/m-AEYp2dAAAJ

It is reasonable to work around this constraint by embedding an iframe navigated to a network resource and thus isolated from the WebUI's render process.
 
Blocking: 721414
Blockedon: 829414
Components: UI>Browser>WebUI

Comment 4 by nasko@chromium.org, Apr 5 2018

Actually, embedding iframes with web content into WebUI is something we explicitly don't want and are about to commit code to enforce ( issue 683418 ). 
Ah hmm. So what's the alternative? I thought we already had uses of web content in WebUI iframes e.g. I believe Chrome OS OOBE loads terms of service in an iframe.

Comment 6 by nasko@chromium.org, Apr 5 2018

We have been moving all of them to use <webview>, which creates a fully isolated container that can't communicate with the parent page by default, but could if the parent opts into it. Example is issue 774683.
Ah, right. webview, not iframe. Thanks.
So what can one do when DCHECK fails? Should we whitelist for now?
It is the case for chrome://sync-confirmation:

[9544:9553:0409/100937.352862:FATAL:web_request_permissions.cc(109)] Check failed: IsWebUIAllowedToMakeNetworkRequests(*initiator). Unsupported network request from chrome://sync-confirmation/ for https://lh3.googleusercontent.com/-XdUIqdMkCWA/AAAAAAAAAAI/AAAAAAAAAAA/4252rscbv5M/s128-c/photo.jpg

The stacktrace is 

#0 0x7f8300d2a6fd base::debug::StackTrace::StackTrace()
#1 0x7f8300d274bc base::debug::StackTrace::StackTrace()
#2 0x7f8300dada9a logging::LogMessage::~LogMessage()
#3 0x55ef0f7ab0f6 IsSensitiveURL()
#4 0x55ef0f7ab903 WebRequestPermissions::HideRequest()
#5 0x55ef0f74d4f1 extensions::(anonymous namespace)::ShouldHideEvent()
#6 0x55ef0f74cb8e extensions::ExtensionWebRequestEventRouter::OnBeforeRequest()
#7 0x55ef107c93b6 (anonymous namespace)::ChromeExtensionsNetworkDelegateImpl::OnBeforeURLRequest()
#8 0x55ef107d1e25 ChromeNetworkDelegate::OnBeforeURLRequest()
#9 0x7f82fe40dbba net::NetworkDelegate::NotifyBeforeURLRequest()
#10 0x7f82fe3e6eb7 net::LayeredNetworkDelegate::OnBeforeURLRequest()
#11 0x7f82fe40dbba net::NetworkDelegate::NotifyBeforeURLRequest()
#12 0x7f82fe3e6eb7 net::LayeredNetworkDelegate::OnBeforeURLRequest()
#13 0x7f82fe40dbba net::NetworkDelegate::NotifyBeforeURLRequest()
#14 0x7f82feca676d net::URLRequest::Start()
#15 0x7f82fb156cd4 content::ResourceLoader::StartRequestInternal()
#16 0x7f82fb15639b content::ResourceLoader::Resume()
#17 0x7f82fb160369 content::ResourceLoader::ScopedDeferral::~ScopedDeferral()
#18 0x7f82fb14fd4e content::ResourceLoader::StartRequest()
#19 0x7f82fb11c2ee content::ResourceDispatcherHostImpl::StartLoading()
#20 0x7f82fb113df0 content::ResourceDispatcherHostImpl::BeginRequestInternal()
#21 0x7f82fb110627 content::ResourceDispatcherHostImpl::ContinuePendingBeginRequest()
#22 0x7f82fb10ae36 content::ResourceDispatcherHostImpl::BeginRequest()
#23 0x7f82fb109a9e content::ResourceDispatcherHostImpl::OnRequestResourceInternal()
#24 0x7f82fb11b9dd content::ResourceDispatcherHostImpl::OnRequestResourceWithMojo()
#25 0x7f82fb16ae76 content::URLLoaderFactoryImpl::CreateLoaderAndStart()
#26 0x7f82fb15a376 content::ResourceMessageFilter::CreateLoaderAndStart()
#27 0x7f82fb15a410 content::ResourceMessageFilter::CreateLoaderAndStart()
#28 0x7f82f99bb846 network::mojom::URLLoaderFactoryStubDispatch::Accept()
#29 0x7f82fa8ef333 network::mojom::URLLoaderFactoryStub<>::Accept()
...

Yes, we should update the whitelist as new failures appear.
Actually droger@ has whitelisted sync-confirmation in https://chromium-review.googlesource.com/c/chromium/src/+/1005035. I've verified and the DCHECK does not fail anymore.
It looks like chrome://inspect fails the DCHECK too (similar stack trace as c#8):

[228471:228490:0411/104501.622481:FATAL:web_request_permissions.cc(109)] Check failed: IsWebUIAllowedToMakeNetworkRequests(*initiator). Unsupported network request from chrome://inspect/ for https://www.google.com/favicon.ico
#0 0x7f0e3e9cc55c base::debug::StackTrace::StackTrace()
#1 0x7f0e3e9f6b9b logging::LogMessage::~LogMessage()
#2 0x55721bbd4050 IsSensitiveURL()
#3 0x55721bbd4433 WebRequestPermissions::HideRequest()
#4 0x55721bbbe946 extensions::ExtensionWebRequestEventRouter::OnBeforeRequest()
#5 0x55721bee40ac ChromeNetworkDelegate::OnBeforeURLRequest()
...
Thanks for the report. I'll whitelist it.
Blockedon: 831812
Blockedon: 831813
Project Member

Comment 15 by bugdroid1@chromium.org, Apr 12 2018

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

commit b4c44707e3c4d7a26bec2ca70c54a0c13968cd57
Author: Ken Rockot <rockot@chromium.org>
Date: Thu Apr 12 15:16:21 2018

Whitelist chrome://inspect for network subresource requests

Bug: 829412
Change-Id: Ia42efe7c749e0eaa44ea9c45a1a4be1a7d43580f
Reviewed-on: https://chromium-review.googlesource.com/1007980
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550206}
[modify] https://crrev.com/b4c44707e3c4d7a26bec2ca70c54a0c13968cd57/extensions/browser/api/web_request/web_request_permissions.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b4c44707e3c4d7a26bec2ca70c54a0c13968cd57

commit b4c44707e3c4d7a26bec2ca70c54a0c13968cd57
Author: Ken Rockot <rockot@chromium.org>
Date: Thu Apr 12 15:16:21 2018

Whitelist chrome://inspect for network subresource requests

Bug: 829412
Change-Id: Ia42efe7c749e0eaa44ea9c45a1a4be1a7d43580f
Reviewed-on: https://chromium-review.googlesource.com/1007980
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550206}
[modify] https://crrev.com/b4c44707e3c4d7a26bec2ca70c54a0c13968cd57/extensions/browser/api/web_request/web_request_permissions.cc

Blockedon: 837420
Blockedon: 859345
Owner: ----
Status: Available (was: Assigned)
Unassigning Ken since this needs to be fixed by individual owners of each webui (thanks Ken for putting in the code to track these exceptions).
Project Member

Comment 20 by bugdroid1@chromium.org, Jul 12

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

commit 04bfa853ac261c316e0ef799e7c10ae05dece8c1
Author: John Abd-El-Malek <jam@chromium.org>
Date: Thu Jul 12 05:28:22 2018

Add workaround for WebUIs that make network requests when running with the network service.

Currently with the network service, renderers that have WebUI bindings are not allowed to make requests over the internet. This is to improve security in case the response hijacks the renderer. However there are a couple of WebUIs that are doing this now, so temporarily allow them.

Bug:  848987 , 829412
Change-Id: I34c1b66e63fd1b7c2960cc867709c065329cf489
Reviewed-on: https://chromium-review.googlesource.com/1134412
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574490}
[modify] https://crrev.com/04bfa853ac261c316e0ef799e7c10ae05dece8c1/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/04bfa853ac261c316e0ef799e7c10ae05dece8c1/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/04bfa853ac261c316e0ef799e7c10ae05dece8c1/chrome/browser/extensions/chrome_extensions_browser_client.cc
[modify] https://crrev.com/04bfa853ac261c316e0ef799e7c10ae05dece8c1/chrome/browser/extensions/chrome_extensions_browser_client.h
[modify] https://crrev.com/04bfa853ac261c316e0ef799e7c10ae05dece8c1/chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc
[modify] https://crrev.com/04bfa853ac261c316e0ef799e7c10ae05dece8c1/chrome/browser/ui/webui/chrome_web_ui_controller_factory.h
[modify] https://crrev.com/04bfa853ac261c316e0ef799e7c10ae05dece8c1/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/04bfa853ac261c316e0ef799e7c10ae05dece8c1/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/04bfa853ac261c316e0ef799e7c10ae05dece8c1/content/public/browser/content_browser_client.h
[modify] https://crrev.com/04bfa853ac261c316e0ef799e7c10ae05dece8c1/extensions/browser/api/web_request/web_request_permissions.cc
[modify] https://crrev.com/04bfa853ac261c316e0ef799e7c10ae05dece8c1/extensions/browser/extensions_browser_client.cc
[modify] https://crrev.com/04bfa853ac261c316e0ef799e7c10ae05dece8c1/extensions/browser/extensions_browser_client.h

Sign in to add a comment