I have a WIP CL @ https://chromium-review.googlesource.com/c/chromium/src/+/666317 where I make ChildProcessSecurityProcessImpl track all origins ever committed to the given process (and then localStorage requests later consult this tracker when verifying the origin [of localStorage requested by a renderer]).

Avoiding the "citadel" or "jail" terms and just explaining in the title what goes wrong today.

Status update:

*) I have put together a design doc at https://docs.google.com/document/d/1Ncllh7QnqBW3eewJjUFhJqjmoA2u1Ijc8SuW9CxfOi4

*) the CL from #c1 is blocked on

    1) preventing script execution before commit (WebUI)

    2) no ordering guarantees between
       DidCommitProvisionalLoad and OpenLocalStorage

Expanding the subject of this bug to also cover WebUI, CWS and other isolation modes.  I hope that (for UI-thread checks) most isolation modes can covered by having ChildProcessSecurityPolicyImpl::CanAccessDataForOrigin consult RenderProcessHostImpl::IsSuitableHost (we might need to beef up ChromeContentBrowserClientExtensionsPart::IsSuitableHost to make sure it covers CWS).

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

commit 39603d88a9cb45351da34a9e15ae320780fffe2b
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Tue Dec 05 21:41:31 2017

Reject IPC requests for isolated origin, sent by non-isolated renderer.

The CL tweaks StoragePartitonInterceptor (in
isolated_origin_browsertest.cc) so that it can be configured to inject
different origins, depending on needs of individual tests.  This tweak
is supported by changes in base/lazy_instance.h (adding of inequality
operator implemented on top of the already existing equality operator),
and in render_process_host_impl.cc/.h (to support creating a test-only
StoragePartitionService via a base::Callback, rather than via a function
pointer).

Tweaks of StoragePartitonInterceptor allow forking of
IsolatedOriginTest.LocalStorageOriginEnforcement test into 2 separate
tests:
- LocalStorageOriginEnforcement_IsolatedAccessingNonIsolated
- LocalStorageOriginEnforcement_NonIsolatedAccessingIsolated
The second test is introduced by this CL and was failing before this CL.

Tweaks of ChildProcessSecurityPolicyImpl::CanAccessDataForOrigin
disallow requests from non-isolated renderers (in which case
CheckOriginLock will return NO_LOCK) if the request is for
an isolated origin.  This makes the new test pass.

Bug: 509125, 764958
Change-Id: Ibfff2c91cb2ac51966e1d89295f10592a3814c08
Reviewed-on: https://chromium-review.googlesource.com/775060
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521838}
[modify] https://crrev.com/39603d88a9cb45351da34a9e15ae320780fffe2b/content/browser/child_process_security_policy_impl.cc
[modify] https://crrev.com/39603d88a9cb45351da34a9e15ae320780fffe2b/content/browser/isolated_origin_browsertest.cc
[modify] https://crrev.com/39603d88a9cb45351da34a9e15ae320780fffe2b/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/39603d88a9cb45351da34a9e15ae320780fffe2b/content/browser/renderer_host/render_process_host_impl.h

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

commit b04d8e25de2f9a56bfe5df61e7cc4488193f66bd
Author: Łukasz Anforowicz <lukasza@chromium.org>
Date: Wed Dec 06 16:37:12 2017

Revert "Reject IPC requests for isolated origin, sent by non-isolated renderer."

This reverts commit 39603d88a9cb45351da34a9e15ae320780fffe2b.

Reason for revert: This CL has caused renderer kills during CrOS login - see https://crbug.com/792500

Original change's description:
> Reject IPC requests for isolated origin, sent by non-isolated renderer.
> 
> The CL tweaks StoragePartitonInterceptor (in
> isolated_origin_browsertest.cc) so that it can be configured to inject
> different origins, depending on needs of individual tests.  This tweak
> is supported by changes in base/lazy_instance.h (adding of inequality
> operator implemented on top of the already existing equality operator),
> and in render_process_host_impl.cc/.h (to support creating a test-only
> StoragePartitionService via a base::Callback, rather than via a function
> pointer).
> 
> Tweaks of StoragePartitonInterceptor allow forking of
> IsolatedOriginTest.LocalStorageOriginEnforcement test into 2 separate
> tests:
> - LocalStorageOriginEnforcement_IsolatedAccessingNonIsolated
> - LocalStorageOriginEnforcement_NonIsolatedAccessingIsolated
> The second test is introduced by this CL and was failing before this CL.
> 
> Tweaks of ChildProcessSecurityPolicyImpl::CanAccessDataForOrigin
> disallow requests from non-isolated renderers (in which case
> CheckOriginLock will return NO_LOCK) if the request is for
> an isolated origin.  This makes the new test pass.
> 
> Bug: 509125, 764958
> Change-Id: Ibfff2c91cb2ac51966e1d89295f10592a3814c08
> Reviewed-on: https://chromium-review.googlesource.com/775060
> Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
> Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#521838}

TBR=thestig@chromium.org,alexmos@chromium.org,lukasza@chromium.org

Change-Id: Id395a4be8f2127f2fbdc7e4f75e0fbd05272012f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 509125, 764958
Reviewed-on: https://chromium-review.googlesource.com/810986
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522105}
[modify] https://crrev.com/b04d8e25de2f9a56bfe5df61e7cc4488193f66bd/content/browser/child_process_security_policy_impl.cc
[modify] https://crrev.com/b04d8e25de2f9a56bfe5df61e7cc4488193f66bd/content/browser/isolated_origin_browsertest.cc
[modify] https://crrev.com/b04d8e25de2f9a56bfe5df61e7cc4488193f66bd/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/b04d8e25de2f9a56bfe5df61e7cc4488193f66bd/content/browser/renderer_host/render_process_host_impl.h

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

commit 3911f4cff960c7842fbfdcf61013093de6dd628b
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Wed Dec 06 18:35:26 2017

Revert "Reject IPC requests for isolated origin, sent by non-isolated renderer."

This reverts commit 39603d88a9cb45351da34a9e15ae320780fffe2b.

Reason for revert: This CL has caused renderer kills during CrOS login - see https://crbug.com/792500

Original change's description:
> Reject IPC requests for isolated origin, sent by non-isolated renderer.
>
> The CL tweaks StoragePartitonInterceptor (in
> isolated_origin_browsertest.cc) so that it can be configured to inject
> different origins, depending on needs of individual tests.  This tweak
> is supported by changes in base/lazy_instance.h (adding of inequality
> operator implemented on top of the already existing equality operator),
> and in render_process_host_impl.cc/.h (to support creating a test-only
> StoragePartitionService via a base::Callback, rather than via a function
> pointer).
>
> Tweaks of StoragePartitonInterceptor allow forking of
> IsolatedOriginTest.LocalStorageOriginEnforcement test into 2 separate
> tests:
> - LocalStorageOriginEnforcement_IsolatedAccessingNonIsolated
> - LocalStorageOriginEnforcement_NonIsolatedAccessingIsolated
> The second test is introduced by this CL and was failing before this CL.
>
> Tweaks of ChildProcessSecurityPolicyImpl::CanAccessDataForOrigin
> disallow requests from non-isolated renderers (in which case
> CheckOriginLock will return NO_LOCK) if the request is for
> an isolated origin.  This makes the new test pass.
>
> Bug: 509125, 764958
> Change-Id: Ibfff2c91cb2ac51966e1d89295f10592a3814c08
> Reviewed-on: https://chromium-review.googlesource.com/775060
> Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
> Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#521838}

TBR=alexmos@chromium.org, lukasza@chromium.org, thestig@chromium.org

(cherry picked from commit b04d8e25de2f9a56bfe5df61e7cc4488193f66bd)

Change-Id: Id395a4be8f2127f2fbdc7e4f75e0fbd05272012f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 509125, 764958, 792413
Reviewed-on: https://chromium-review.googlesource.com/810986
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#522105}
Reviewed-on: https://chromium-review.googlesource.com/811586
Cr-Commit-Position: refs/branch-heads/3286@{#3}
Cr-Branched-From: 7f87a643cf9d5de271f2b6adeb7e30d6a5f8eb5d-refs/heads/master@{#521956}
[modify] https://crrev.com/3911f4cff960c7842fbfdcf61013093de6dd628b/content/browser/child_process_security_policy_impl.cc
[modify] https://crrev.com/3911f4cff960c7842fbfdcf61013093de6dd628b/content/browser/isolated_origin_browsertest.cc
[modify] https://crrev.com/3911f4cff960c7842fbfdcf61013093de6dd628b/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/3911f4cff960c7842fbfdcf61013093de6dd628b/content/browser/renderer_host/render_process_host_impl.h

For some notes about scenarios that might have led to incorrect kills in the past (and make us revert the CLs above), please see another bug here: https://bugs.chromium.org/p/chromium/issues/detail?id=797968#c12

Citadel-style enforcement is not a problem when all sites require a process lock.  Therefore this bug doesn't matter on desktop platforms, where Strict Site Isolation (aka site-per-process) has shipped in M67.

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

commit c62eede3a3ee8bc68500ff68e502b014221edc97
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri Dec 28 21:52:01 2018

Browser test covering how CanAccessDataForOrigin handles opaque origins.

The CL refactors StoragePartitonInterceptor to allow the user to
provide an origin to spoof/inject (rather than hardcoding the origin
inside StoragePartitonInterceptor::OpenLocalStorage method).

The CL also renames and refactors
RenderProcessHostImpl::SetCreateStoragePartitionServiceFunction, so that
it allows specifying a base::Callback (instead of just a function
pointer).  This allows the base::Callback to "close over" some extra
data (like an origin to inject).

The CL then adds a browser test covering how CanAccessDataForOrigin
handles opaque origins.

Bug: 915396, 764958
Change-Id: I03aa05c1341f79d7b129ff83aee9b196301174c1
Reviewed-on: https://chromium-review.googlesource.com/c/1391743
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619199}
[modify] https://crrev.com/c62eede3a3ee8bc68500ff68e502b014221edc97/content/browser/isolated_origin_browsertest.cc
[modify] https://crrev.com/c62eede3a3ee8bc68500ff68e502b014221edc97/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/c62eede3a3ee8bc68500ff68e502b014221edc97/content/browser/renderer_host/render_process_host_impl.h