Implement dynamic isolated origins |
|||||
Issue descriptionIsolated origins are currently stored in a global list in ChildProcessSecurityPolicy and initialized only at startup. To support selective site isolation on Android, as well as opt-in isolation headers, we want to be able to add isolated origins dynamically at runtime. For example, we might use heuristics such as password entry to start isolating certain sites dynamically. This bug will track the implementation work needed to support this. Design doc: https://docs.google.com/document/d/10P1qV2X-gNoDoqgRVya4iXo7PDtGphytgle08o1ccr8/edit?usp=sharing Some of the planned major changes include (see doc for details): (1) Dynamic isolated origins will need to apply to future BrowsingInstances only. (2) Dynamic isolated origins will likely be per-profile rather than global. (3) At least some dynamic isolated origins will need to be persisted to disk.
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/25754cd4d3832e74532b6e67eea0eae260869900 commit 25754cd4d3832e74532b6e67eea0eae260869900 Author: Alex Moshchuk <alexmos@chromium.org> Date: Wed Dec 05 22:43:08 2018 Remove isolated origin workarounds from CCBCEP::GetEffectiveURL(). Previously, if a hosted app's web extent matched one or more isolated origins, we used complicated logic in ChromeContentBrowserClientExtensionsPart::GetEffectiveURL to determine when a URL was allowed to go into a hosted app process (by utilizing an effective site URL) without compromising the security of isolated origins. Specifically, if a hosted app was contained entirely within an isolated origin, then we decided it was safe to place any app URL into an app process. This is because we used to have an issue, https://crbug.com/791796 , where all origins covered by the app would end up in the *same* app process, which could break process isolation for isolated origins. This logic is no longer needed after we started locking hosted app processes to origin, which fixed https://crbug.com/791796 and allowed a hosted app to span multiple processes. Given that work, if a hosted app corresponds to different isolated origins, it's safe to simply load those origins in different app processes. Therefore, this CL removes that logic and allows any URL covered by an app to load in an app process. If any of those URLs match isolated origins, they will simply swap to a different app process -- no additional changes were needed for this. For example, suppose there's one isolated origin, http://isolated.origin.com, and a hosted app's extent covers foo.com and isolated.origin.com. Suppose a main frame at foo.com (in app process) embeds an iframe for http://isolated.origin.com. Previously, we'd kick the iframe into an OOPIF, with site URL of http://isolated.origin.com, locked to http://isolated.origin.com, and in a non-app process. After this CL, we'd still kick the iframe into an OOPIF, locked to https://isolated.origin.com, but it will have a site URL of chrome-extension://appid/#https://isolated.origin.com, and it will actually be in its own app process. This is arguably more correct, and should make hosted apps more compatible with isolated origin uses. Another motivation for this change is that it removes the only use of ChildProcessSecurityPolicy::GetMatchingIsolatedOrigin outside of content/. That API will need to be updated to support dynamic isolated origins (see issue 905513), and this will be easier to do if this use case is removed. Bug: 879722 , 718516, 905513 Change-Id: Ia60e1ad435d58c66a02730a1459ab0684fad89d1 Reviewed-on: https://chromium-review.googlesource.com/c/1343516 Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Reviewed-by: Charlie Reis <creis@chromium.org> Commit-Queue: Alex Moshchuk <alexmos@chromium.org> Cr-Commit-Position: refs/heads/master@{#614144} [modify] https://crrev.com/25754cd4d3832e74532b6e67eea0eae260869900/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc [modify] https://crrev.com/25754cd4d3832e74532b6e67eea0eae260869900/chrome/browser/extensions/chrome_content_browser_client_extensions_part.h [modify] https://crrev.com/25754cd4d3832e74532b6e67eea0eae260869900/chrome/browser/extensions/chrome_content_browser_client_extensions_part_unittest.cc [modify] https://crrev.com/25754cd4d3832e74532b6e67eea0eae260869900/chrome/browser/ui/extensions/hosted_app_browsertest.cc [modify] https://crrev.com/25754cd4d3832e74532b6e67eea0eae260869900/content/browser/child_process_security_policy_impl.h [modify] https://crrev.com/25754cd4d3832e74532b6e67eea0eae260869900/content/public/browser/child_process_security_policy.h
,
Jan 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/916408b7872b5784a60b93eb9617823064b3b678 commit 916408b7872b5784a60b93eb9617823064b3b678 Author: Alex Moshchuk <alexmos@chromium.org> Date: Sat Jan 05 00:07:21 2019 Avoid calling ShouldLockToOrigin from GetRequestInitiatorSiteLock. GetRequestInitiatorSiteLock currently calls ShouldLockToOrigin, but that check is redundant for one of its call sites from RenderProcessHostImpl, which only calls GetRequestInitiatorSiteLock for valid, non-empty process locks. This CL moves the ShouldLockToOrigin call to the other call site in RenderFrameHostImpl. This removes GetRequestInitiatorSiteLock's dependency on ShouldLockToOrigin params, which helps prepare for the dynamic isolated origin plumbing to be introduced in https://chromium-review.googlesource.com/c/chromium/src/+/1377616. Bug: 905513 Change-Id: I4f64bd71370f67536c37156274ea578745c56e0e Reviewed-on: https://chromium-review.googlesource.com/c/1396450 Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org> Commit-Queue: Alex Moshchuk <alexmos@chromium.org> Cr-Commit-Position: refs/heads/master@{#620117} [modify] https://crrev.com/916408b7872b5784a60b93eb9617823064b3b678/content/browser/frame_host/render_frame_host_impl.cc [modify] https://crrev.com/916408b7872b5784a60b93eb9617823064b3b678/content/browser/renderer_host/render_process_host_impl.cc [modify] https://crrev.com/916408b7872b5784a60b93eb9617823064b3b678/content/browser/site_instance_impl.cc [modify] https://crrev.com/916408b7872b5784a60b93eb9617823064b3b678/content/browser/site_instance_impl.h
,
Jan 8
This work will be essential for the form of site isolation that we plan to ship on Android, so adding the launch blocker label.
,
Jan 11
,
Jan 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8e5c195150aa944906770e605d22e5dc09831f51 commit 8e5c195150aa944906770e605d22e5dc09831f51 Author: Alex Moshchuk <alexmos@chromium.org> Date: Tue Jan 15 03:39:50 2019 Implement support for dynamically added isolated origins. This CL introduces an ability to add isolated origins at any time, rather than only at browser startup. Isolated origins added dynamically will apply only to future BrowsingInstances and processes. To do this, the calls involved in making process model decisions and looking up isolated origins, such as DoesSiteRequireDedicatedProcess, need to be aware of which BrowsingInstance is asking. This CL adds the required plumbing in the form of a new IsolationContext object. For now, IsolationContext only contains the BrowsingInstance ID, but in the future it will be extended to include BrowserContext info as well, allowing isolated origins to also be scoped to particular profiles. Calls that currently take both BrowserContext and IsolationContext will be able to simply pass an IsolationContext. Design doc: https://goo.gl/4xVPKW Bug: 905513 Change-Id: I5d6fb7724524e85efe492da26077209fa90be1bf Reviewed-on: https://chromium-review.googlesource.com/c/1377616 Commit-Queue: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Charlie Reis <creis@chromium.org> Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org> Cr-Commit-Position: refs/heads/master@{#622715} [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/BUILD.gn [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/browsing_instance.cc [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/browsing_instance.h [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/child_process_security_policy_impl.cc [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/child_process_security_policy_impl.h [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/child_process_security_policy_unittest.cc [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/dom_storage/session_storage_namespace_impl_mojo_unittest.cc [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/frame_host/navigation_handle_impl.cc [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/frame_host/navigation_handle_impl.h [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/frame_host/navigation_request.cc [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/frame_host/render_frame_host_impl.cc [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/frame_host/render_frame_host_manager.cc [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/frame_host/webui_navigation_browsertest.cc [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/isolated_origin_browsertest.cc [add] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/isolation_context.cc [add] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/isolation_context.h [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/loader/cross_site_document_blocking_browsertest.cc [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/loader/resource_dispatcher_host_impl.cc [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/renderer_host/render_process_host_impl.cc [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/renderer_host/render_process_host_impl.h [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/renderer_host/render_process_host_unittest.cc [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/renderer_host/web_database_host_impl_unittest.cc [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/site_instance_impl.cc [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/site_instance_impl.h [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/browser/site_instance_impl_unittest.cc [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/public/browser/render_process_host.h [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/public/browser/site_instance.h [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/public/test/mock_render_process_host.cc [modify] https://crrev.com/8e5c195150aa944906770e605d22e5dc09831f51/content/public/test/mock_render_process_host.h
,
Jan 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f01172ea04d6dc7a4d50975a10e442fe16749d4a commit f01172ea04d6dc7a4d50975a10e442fe16749d4a Author: Alex Moshchuk <alexmos@chromium.org> Date: Wed Jan 16 00:54:17 2019 Use a finer-grained lock for isolated_origins_ in CPSPI. This is a followup on a suggestion from https://chromium-review.googlesource.com/c/chromium/src/+/1208942 to make a separate lock for isolated_origins_. This reduces contention when looking up isolated origins, and it simplifies lock acquisitions in CanAccessDataForOrigin, which needs to access both security state and isolated origins. Bug: 877653, 905513 Change-Id: Idb0126abc156315939141f6aa2b917db8b02141e Reviewed-on: https://chromium-review.googlesource.com/c/1214102 Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org> Reviewed-by: Charlie Reis <creis@chromium.org> Commit-Queue: Alex Moshchuk <alexmos@chromium.org> Cr-Commit-Position: refs/heads/master@{#622950} [modify] https://crrev.com/f01172ea04d6dc7a4d50975a10e442fe16749d4a/content/browser/child_process_security_policy_impl.cc [modify] https://crrev.com/f01172ea04d6dc7a4d50975a10e442fe16749d4a/content/browser/child_process_security_policy_impl.h [modify] https://crrev.com/f01172ea04d6dc7a4d50975a10e442fe16749d4a/content/browser/child_process_security_policy_unittest.cc
,
Jan 17
(6 days ago)
,
Jan 17
(5 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7476f384c7011d2309a7a97520c8ac1195a5f65f commit 7476f384c7011d2309a7a97520c8ac1195a5f65f Author: Alex Moshchuk <alexmos@chromium.org> Date: Thu Jan 17 19:05:41 2019 Add support for operator <= to IdType. This is a followup to r622715 to simplify some of the comparisons done in ChildProcessSecurityPolicyImpl on BrowsingInstanceId, which is an IdType. Bug: 905513 Change-Id: I7c0fc4c3f649d6cf851188b0a6560489067806b9 Reviewed-on: https://chromium-review.googlesource.com/c/1413811 Reviewed-by: Zhenyao Mo <zmo@chromium.org> Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org> Commit-Queue: Alex Moshchuk <alexmos@chromium.org> Cr-Commit-Position: refs/heads/master@{#623782} [modify] https://crrev.com/7476f384c7011d2309a7a97520c8ac1195a5f65f/content/browser/child_process_security_policy_impl.cc [modify] https://crrev.com/7476f384c7011d2309a7a97520c8ac1195a5f65f/gpu/command_buffer/common/id_type.h
,
Jan 17
(5 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/47dbb2329b54771e4957ec240c8ebe0532f429a0 commit 47dbb2329b54771e4957ec240c8ebe0532f429a0 Author: Alex Moshchuk <alexmos@chromium.org> Date: Thu Jan 17 23:48:29 2019 Remove outdated isolation checks for report_raw_headers in RDHI. When deciding whether raw headers should be reported to the renderer process, ResourceDispatcherHostImpl::ContinuePendingBeginRequest checks both CanAccessDataForOrigin and whether site isolation is enabled via UseDedicatedProcessesForAllSites and IsIsolatedOrigin. Since site isolation has shipped by default on desktop, this means that |is_isolated| will always be true, and |report_raw_headers| will always be determined by the CanAccessDataForOrigin check on desktop. This CL simplifies this check to only check CanAccessDataForOrigin. This should still keep web sites from being able to look at raw headers for other sites; i.e., it shouldn't change any behavior on desktop. Note that for processes that aren't locked to anything (such as web sites for site isolation opt-out users), CanAccessDataForOrigin currently returns true. That means that |report_raw_headers| wouldn't have been set to false in those cases, both before and after this CL. I.e., this CL doesn't regress citadel-style checking, where a process that's not locked to anything is permitted to see raw headers for a URL that requires isolation -- such checking doesn't happen both before and after this CL. This should eventually be fixed under the general umbrella of citadel-style protections (issue 764958), by having CanAccessDataForOrigin return false in those cases. This change is also desirable to simplify plumbing for dynamic isolated origins, which introduced another parameter to IsIsolatedOrigin() in https://crrev.com/c/1377616/, which isn't easy to get here. This CL follows up on discussion in https://chromium-review.googlesource.com/c/chromium/src/+/1377616/10/content/browser/loader/resource_dispatcher_host_impl.cc#936 Bug: 905513 Change-Id: Idc5bf95eefa0a41ec2870a9906613fa1415d3860 Reviewed-on: https://chromium-review.googlesource.com/c/1395834 Reviewed-by: Charlie Reis <creis@chromium.org> Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org> Commit-Queue: Alex Moshchuk <alexmos@chromium.org> Cr-Commit-Position: refs/heads/master@{#623913} [modify] https://crrev.com/47dbb2329b54771e4957ec240c8ebe0532f429a0/content/browser/loader/resource_dispatcher_host_impl.cc
,
Today
(12 hours ago)
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by bugdroid1@chromium.org
, Nov 30