New issue
Advanced search Search tips

Issue 779444 link

Starred by 5 users

Issue metadata

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


Sign in to add a comment

Remove or verify origins/URLs in IPCs sent from the renderer

Project Member Reported by sa...@chromium.org, Oct 30 2017

Issue description

Once the browser can be sure of the origin of a a document or web worker (when the blocking bugs are fixed), IPCs communicating that origin to the browser should be updated to omit the origin and the browser should instead use the origin it knows is the originator of the request. If the origin is not the same as the origin of the content, the browser should at least check the supplied origin is valid for the particular renderer process.

This includes migrating renderer-process-wide interfaces that should be origin-scoped to be per-ExecutionContext (frame or worker).

This is a tracking bug for updating all the IPCs.
 
Blockedon: 781922
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 10 2017

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

commit 8b4f74dad52dbe331553f1f733be51134df17b86
Author: Sam McNally <sammc@chromium.org>
Date: Fri Nov 10 00:07:56 2017

Expand WorkerInterfaceBinders to handle interface requests from frames.

Some //content interface binders are registered in both
RenderFrameHostImpl's BinderRegistry and the WorkerInterfaceBinders.

After trying to bind using the RenderFrameHostImpl's BinderRegistry
but before passing the request to the embedder, try binding frame
interface requests using the worker BinderRegistry. Interfaces with a
frame-specific implementation continue to be handled directly by the
RenderFrameHostImpl-registered binder; interfaces handled by the
embedder continue to be forwarded to
ContentBrowserClient::BindInterfaceRequestFromFrame().

Interface requests are still filtered by the per-context-type capability
specs in the content_browser manifest so this does not change which
interfaces are exposed to each context type.

Rename WorkerInterfaceBinders to RendererInterfaceBinders to reflect its
coverage of all renderer context types. Remove common interface binders
from RenderFrameHostImpl's BinderRegistry.

Bug: 779444
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I49e6c5658b9f545214eeaf1f13828bfddc2081f0
Reviewed-on: https://chromium-review.googlesource.com/758297
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515351}
[modify] https://crrev.com/8b4f74dad52dbe331553f1f733be51134df17b86/content/browser/BUILD.gn
[modify] https://crrev.com/8b4f74dad52dbe331553f1f733be51134df17b86/content/browser/dedicated_worker/dedicated_worker_host.cc
[modify] https://crrev.com/8b4f74dad52dbe331553f1f733be51134df17b86/content/browser/dedicated_worker/dedicated_worker_host.h
[modify] https://crrev.com/8b4f74dad52dbe331553f1f733be51134df17b86/content/browser/frame_host/render_frame_host_impl.cc
[rename] https://crrev.com/8b4f74dad52dbe331553f1f733be51134df17b86/content/browser/renderer_interface_binders.cc
[rename] https://crrev.com/8b4f74dad52dbe331553f1f733be51134df17b86/content/browser/renderer_interface_binders.h
[modify] https://crrev.com/8b4f74dad52dbe331553f1f733be51134df17b86/content/browser/service_worker/service_worker_provider_host.cc
[modify] https://crrev.com/8b4f74dad52dbe331553f1f733be51134df17b86/content/browser/shared_worker/shared_worker_host.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 13 2017

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

commit d80630e0dd04c3b1df49ba7afff1512b6e497852
Author: Mostyn Bramley-Moore <mostynb@vewd.com>
Date: Mon Nov 13 09:03:34 2017

[jumbo] avoid ForwardRequest symbol clash

content/browser/renderer_host/render_process_host_impl.cc also has a
ForwardRequest function in the anonymous namespace, which can collide
with the function in content/browser/renderer_interface_binders.cc

TBR=jam@chromium.org

Bug: 779444,746953
Change-Id: Iaa2c9f22ff0945b4a35fef689f2b60896d048fea
Reviewed-on: https://chromium-review.googlesource.com/765489
Commit-Queue: Mostyn Bramley-Moore <mostynb@vewd.com>
Reviewed-by: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515901}
[modify] https://crrev.com/d80630e0dd04c3b1df49ba7afff1512b6e497852/content/browser/renderer_interface_binders.cc

Comment 4 by creis@chromium.org, Nov 17 2017

Cc: creis@chromium.org
Components: Internals>Sandbox>SiteIsolation

Comment 5 by creis@chromium.org, Nov 18 2017

Blocking: 786673
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 11 2017

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

commit 2e5c71fb4ad134883e0a5d931aad77864625689c
Author: Sam McNally <sammc@chromium.org>
Date: Mon Dec 11 03:24:27 2017

Remove origins from the PermissionService interface messages.

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I392aa805b7d62816624cd944113c2132832dcf09
Bug: 779444
Reviewed-on: https://chromium-review.googlesource.com/706883
Commit-Queue: Sam McNally <sammc@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523033}
[modify] https://crrev.com/2e5c71fb4ad134883e0a5d931aad77864625689c/content/browser/permissions/permission_service_context.cc
[modify] https://crrev.com/2e5c71fb4ad134883e0a5d931aad77864625689c/content/browser/permissions/permission_service_context.h
[modify] https://crrev.com/2e5c71fb4ad134883e0a5d931aad77864625689c/content/browser/permissions/permission_service_impl.cc
[modify] https://crrev.com/2e5c71fb4ad134883e0a5d931aad77864625689c/content/browser/permissions/permission_service_impl.h
[modify] https://crrev.com/2e5c71fb4ad134883e0a5d931aad77864625689c/content/browser/permissions/permission_service_impl_unittest.cc
[modify] https://crrev.com/2e5c71fb4ad134883e0a5d931aad77864625689c/content/browser/renderer_interface_binders.cc
[modify] https://crrev.com/2e5c71fb4ad134883e0a5d931aad77864625689c/content/renderer/media/media_permission_dispatcher.cc
[modify] https://crrev.com/2e5c71fb4ad134883e0a5d931aad77864625689c/third_party/WebKit/Source/modules/accessibility/AXObjectCacheImpl.cpp
[modify] https://crrev.com/2e5c71fb4ad134883e0a5d931aad77864625689c/third_party/WebKit/Source/modules/notifications/NotificationManager.cpp
[modify] https://crrev.com/2e5c71fb4ad134883e0a5d931aad77864625689c/third_party/WebKit/Source/modules/permissions/PermissionStatus.cpp
[modify] https://crrev.com/2e5c71fb4ad134883e0a5d931aad77864625689c/third_party/WebKit/Source/modules/permissions/Permissions.cpp
[modify] https://crrev.com/2e5c71fb4ad134883e0a5d931aad77864625689c/third_party/WebKit/Source/modules/quota/StorageManager.cpp
[modify] https://crrev.com/2e5c71fb4ad134883e0a5d931aad77864625689c/third_party/WebKit/Source/modules/webmidi/MIDIAccessInitializer.cpp
[modify] https://crrev.com/2e5c71fb4ad134883e0a5d931aad77864625689c/third_party/WebKit/public/platform/modules/permissions/permission.mojom

Project Member

Comment 8 by bugdroid1@chromium.org, Dec 13 2017

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

commit 54bc028e532c4e8c8e72a8122b7f2987ae4801d4
Author: Sam McNally <sammc@chromium.org>
Date: Wed Dec 13 02:42:29 2017

Remove origins from the BackgroundFetchService interface messages.

Move registration of the BackgroundFetchService interface from
RenderProcessHost to RendererInterfaceBinders so its implementation can
receive a browser-tracked origin for its renderer-process client.

Add support for move-only types to base::AutoReset. Use it in the
BackgroundFetchServiceImpl test to swap in a service instance with a
different origin for the part of the test that previously passed a
different origin to the mojo interface.

Bug: 779444
Change-Id: I68737625cb5cd0a2843eeb46643aad3b4f852d8c
Reviewed-on: https://chromium-review.googlesource.com/788641
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523661}
[modify] https://crrev.com/54bc028e532c4e8c8e72a8122b7f2987ae4801d4/base/auto_reset.h
[modify] https://crrev.com/54bc028e532c4e8c8e72a8122b7f2987ae4801d4/content/browser/background_fetch/background_fetch_service_impl.cc
[modify] https://crrev.com/54bc028e532c4e8c8e72a8122b7f2987ae4801d4/content/browser/background_fetch/background_fetch_service_impl.h
[modify] https://crrev.com/54bc028e532c4e8c8e72a8122b7f2987ae4801d4/content/browser/background_fetch/background_fetch_service_unittest.cc
[modify] https://crrev.com/54bc028e532c4e8c8e72a8122b7f2987ae4801d4/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/54bc028e532c4e8c8e72a8122b7f2987ae4801d4/content/browser/renderer_interface_binders.cc
[modify] https://crrev.com/54bc028e532c4e8c8e72a8122b7f2987ae4801d4/content/public/app/mojo/content_browser_manifest.json
[modify] https://crrev.com/54bc028e532c4e8c8e72a8122b7f2987ae4801d4/third_party/WebKit/Source/modules/background_fetch/BackgroundFetchBridge.cpp
[modify] https://crrev.com/54bc028e532c4e8c8e72a8122b7f2987ae4801d4/third_party/WebKit/Source/modules/background_fetch/BackgroundFetchBridge.h
[modify] https://crrev.com/54bc028e532c4e8c8e72a8122b7f2987ae4801d4/third_party/WebKit/public/platform/modules/background_fetch/background_fetch.mojom

Comment 9 by jsb...@chromium.org, Dec 19 2017

Blocking: 467150
Project Member

Comment 11 by bugdroid1@chromium.org, Feb 16 2018

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

commit 064dbbfe9a8d232fa91c171c947da39670d3aa9c
Author: Noel Gordon <noel@chromium.org>
Date: Fri Feb 16 03:35:15 2018

Remove origin parameter from web socket mojo

IPCs communicating their Origin to the browser in requests should be updated to
remove their origin from such requests. The browser knows the Origin of the web
frame / worker making the request; it does not need the renderer to tell it the
Origin, nor should the browser believe them (renderers are untrusted, accepting
their idea of Origin is insecure). Update WebSockets, make them join the party,
remove Origin from their requests to the browser.

Bug: 779444
Change-Id: I000d23520f498f445b89f197d833fa335fa3de04
Reviewed-on: https://chromium-review.googlesource.com/788011
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537178}
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/content/browser/renderer_interface_binders.cc
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/content/browser/websockets/websocket_impl.cc
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/content/browser/websockets/websocket_impl.h
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/content/browser/websockets/websocket_manager.cc
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/content/browser/websockets/websocket_manager.h
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/content/browser/websockets/websocket_manager_unittest.cc
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannelTest.cpp
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/third_party/WebKit/Source/modules/websockets/WebSocketHandle.h
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/third_party/WebKit/Source/modules/websockets/WebSocketHandleImpl.cpp
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/third_party/WebKit/Source/modules/websockets/WebSocketHandleImpl.h
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/third_party/WebKit/public/platform/modules/websockets/websocket.mojom

Project Member

Comment 12 by bugdroid1@chromium.org, Feb 16 2018

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

commit 064dbbfe9a8d232fa91c171c947da39670d3aa9c
Author: Noel Gordon <noel@chromium.org>
Date: Fri Feb 16 03:35:15 2018

Remove origin parameter from web socket mojo

IPCs communicating their Origin to the browser in requests should be updated to
remove their origin from such requests. The browser knows the Origin of the web
frame / worker making the request; it does not need the renderer to tell it the
Origin, nor should the browser believe them (renderers are untrusted, accepting
their idea of Origin is insecure). Update WebSockets, make them join the party,
remove Origin from their requests to the browser.

Bug: 779444
Change-Id: I000d23520f498f445b89f197d833fa335fa3de04
Reviewed-on: https://chromium-review.googlesource.com/788011
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537178}
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/content/browser/renderer_interface_binders.cc
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/content/browser/websockets/websocket_impl.cc
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/content/browser/websockets/websocket_impl.h
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/content/browser/websockets/websocket_manager.cc
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/content/browser/websockets/websocket_manager.h
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/content/browser/websockets/websocket_manager_unittest.cc
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannelTest.cpp
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/third_party/WebKit/Source/modules/websockets/WebSocketHandle.h
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/third_party/WebKit/Source/modules/websockets/WebSocketHandleImpl.cpp
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/third_party/WebKit/Source/modules/websockets/WebSocketHandleImpl.h
[modify] https://crrev.com/064dbbfe9a8d232fa91c171c947da39670d3aa9c/third_party/WebKit/public/platform/modules/websockets/websocket.mojom

Comment 13 Deleted

issue 873661 covers doing this for the File System API

Sign in to add a comment