IWA-gated features check the WebExposedIsolationLevel of their frame or
process, and enable themselves if the isolation level is
kMaybeIsolatedApplication. Currently WebExposedIsolationLevel (WEIL)
maps directly to WebExposedIsolationInfo (WEII), which results in
cross-origin IWA child frames claiming to have the
kMaybeIsolatedApplication level, which is incorrect.
This direct mapping is incorrect because WEII and WEIL are meant to
represent two different concepts: WEII represents the cross-origin
isolation (COI) mode of a browsing context group, while WEIL
represents the COI capability of a frame or worker, and should take
into account the 'cross-origin-isolated' permissions policy and the
frame/worker's origin.
All agents in a browsing context group will share a WEII, but the
WEIL of a kIsolated frame's child frame (same or cross-origin) could
be kNotIsolated if permissions policy did not delegate the
'cross-origin-isolated' permission. kMaybeIsolatedApplication
can only be delegated to same-origin pages in addition to being
controllable with permissions policy. This CL adds WEIL to
ProcessLock and lowers the isolation level based on origin if needed.
It also adds permissions policy checks to
RenderFrameHost::GetWebExposedIsolationLevel so that it will return
the correct isolation level if the permission policy wasn't
delegated to a child frame.
See go/content-isolation-levels for more background information.
Bug: 1467673, 1159832, b/280497768
Change-Id: I2a328185c9379a96d56f77178ba5e283148524b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4568370
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Daniel d'Andrada <dandrader@google.com>
Cr-Commit-Position: refs/heads/main@{#1182214}
Currently, third-party NTP processes are not locked, because they are
loaded from third-party sites (e.g., https://duckduckgo.com) and also
need to be able to embed most-visited tiles, which are represented by
chrome-search://most-visited iframes. This is not very good for
security, because it leaves the third-party NTP's web site without
site isolation. This also prevents us from turning on citadel
protections on desktop platforms, which also implies that the
third-party NTP site can access any other site's data.
Ideally, we'd refactor most-visited tile code so that the tiles can
become OOPIFs on the NTP, locked to their underlying
chrome-search://most-visited site. However, this appears to be
difficult to do and is a longer-term project (see
https://docs.google.com/document/d/1zc-g_AzuyxlzLOV-hZO0yIV_OkUI71dVR8X5Ee2PyWw/edit?usp=sharing&resourcekey=0-K034jFL-zTVXX8WrUewTtQ).
This CL improves on this situation by only skipping the process
locking requirement for the most visited tiles site
(chrome-search://most-visited), while requiring the actual third-party
NTP that embeds them to be locked. The third-party NTP is represented
by a SiteInfo with an effective site URL of
chrome-search://remote-ntp/ and a lock URL corresponding to the NTP's
underlying web site (e.g., https://duckduckgo.com), so we can simply
allow our existing effective URL logic to lock it to its lock URL of
https://duckduckgo.com. We already have a special case in
ShouldStayInParentProcessForNTP() which allows
chrome-search://most-visited to stay in the parent NTP process, and we
add another exception to CanAccessDataForOrigin() to allow these tiles
to actually commit in a locked process with a mismatched process lock.
To summarize, before this CL we'd have both the third-party NTP and
its most visited tiles in an unlocked process. After this CL, they
would both be in a process locked to the third party NTP's web site.
There's existing test coverage for this in
ThirdPartyNTPBrowserTest.EmbeddedMostVisitedIframe.
Change-Id: I18619781fd06537e3c7051863aa4ecd92584b751
Bug: 764958,566091
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4111688
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1088436}
This CL implements process isolation for fenced frames (isolating
fenced frames from their embedders) behind a flag. It adds an
'is_fenced' attribute to SiteInfo and IsolationContext to help enforce
this isolation. Having the is_fenced bit in IsolationContext is used to
maintain the same value of is_fenced for all related SiteInstances
created in a particular BrowsingInstance (e.g., for subframes of a
fenced frame).
The changes here currently only have an effect when
the flag is enabled and strict site isolation is enabled. Changes for
other site isolation modes will be in future CLs. See attached bug for
more details on design (design doc, explainer).
Bug: 1340662
Change-Id: I821943158286c9fa69639ad1156112bd74a4410e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3821704
Commit-Queue: Adithya Srinivasan <adithyas@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1041755}
This CL adds a "per-document" isolation grouping for isolation of
sandboxed iframes using the kIsolatedSandboxedIframes flag. This CL
parameterizes the flag to add "per-document", and when this new
parameter is present it creates a unique document identifier on every
SiteInfo associated with a sandboxed iframe in order to force each
document into its own process. This grouping is expected to be the most
granular option for isolated sandboxed iframes.
Bug: 510122
Change-Id: I217bf5c57f8e9badaa84c0fab9261c432e766fcb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3727308
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: James Maclean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1029273}
This is a reland of commit 81175d85f6
Original change's description:
> Do not allow ProcessLocks to be refined if process has been used.
>
> If ShouldAssignSiteForURL were to return false for URLs that commit
> actual content, then other sites that require dedicated processes
> might incorrectly share a process with them. Instead, prevent an
> allows_any_site() ProcessLock from being locked to a single site if
> that process has been used.
>
> Some of the test changes are adapted from ahemery's
> https://chromium-review.googlesource.com/c/chromium/src/+/3483667.
>
> Bug: 1324407
> Change-Id: I778d31c5e8b740179f0b6c9a40b010508ce7cfa2
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3640283
> Commit-Queue: Charlie Reis <creis@chromium.org>
> Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1004469}
Bug: 1324407
Change-Id: I9db2f24f66a112005664164ff19f33f1ebc098b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3654240
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1004979}
If ShouldAssignSiteForURL were to return false for URLs that commit
actual content, then other sites that require dedicated processes
might incorrectly share a process with them. Instead, prevent an
allows_any_site() ProcessLock from being locked to a single site if
that process has been used.
Some of the test changes are adapted from ahemery's
https://chromium-review.googlesource.com/c/chromium/src/+/3483667.
Bug: 1324407
Change-Id: I778d31c5e8b740179f0b6c9a40b010508ce7cfa2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3640283
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1004469}
This CL implements process isolation for origin-restricted sandboxed
iframes, i.e. those with the 'sandbox' attribute but no 'allow-origin'
modifier.
At present this CL implements a model where all the sandboxed iframes
for a site are placed in the same process, though this could be modified
to be (i) origin-specific, and/or (ii) place each sandbox into its own
process regardless of site-/origin-keying of the sandboxes.
Bug: 510122
Change-Id: If02a02d05a02552ef4b1a0a01145e7c26d9dc98e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3416475
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: James Maclean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/main@{#969578}
This CL adds initial support for enabling site isolation inside
<webview> guests. This mode is behind a new feature called
SiteIsolationForGuests.
The overall approach is to stop using special chrome-guest: site URLs
when creating SiteInfos for guests. Instead, site-isolated guests use
real site URLs that are computed with the existing process model code,
and rely on SiteInfo::storage_partition_config to keep navigations
within guests in the same StoragePartition and in guest SiteInstances.
This CL supports cross-process navigations in guests, OOPIFs, and
cross-BrowsingInstance navigations in guests. A particular guest will
no longer be restricted to a single SiteInstance and BrowsingInstance,
though it will always stay in the same StoragePartition, and for all
SiteInstances in a guest, SiteInstance::IsGuest() will be true.
In the current SiteInstance assignment paths, there are various early
returns that make guests always in the same SiteInstance. Those are
now only used when the new feature is off. Since guests will now
descend much deeper into the SiteInstance assignment paths, we needed
an additional signal as to whether the new SiteInstance/SiteInfo would
be for a guest. To do this, instead of plumbing an additional flag to
places like SiteInfo::CreateInternal, is_guest is added to
IsolationContext which is already plumbed into all the needed code
paths (see SiteInfo::CreateInternal for how it's used). Conceptually,
IsolationContext::is_guest just says whether a particular
BrowsingInstance is for a guest or not, so that any related
SiteInstances within it stay in the guest.
To handle cross-BrowsingInstance navigations in guests,
SiteInstanceImpl::CreateForUrlInfo (which is normally used to create
a new SiteInstance in a new BrowsingInstance) is updated to be able to
handle guests, in which case the StoragePartition info needs to be
transferred into the new SiteInstance/BrowsingInstance.
The following are some things that are not part of this CL and will be
addressed in future CLs:
- converting <webview> tests to run in both the legacy and site
isolation modes. (This CL does add standalone tests to validate
site isolation properties.)
- embedder navigating <webview> to about:blank and siteless
SiteInstances.
- error page navigations, WebViewRendererState, loadDataWithBaseURL.
Bug: 1267977
Change-Id: I3b747640c083a302dc07ee4106af4f6d33928165
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3380343
Reviewed-by: James Maclean <wjmaclean@chromium.org>
Reviewed-by: Charles Reis <creis@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#965525}
ProcessLocks are used to represent what level of isolation a given
SiteInfo principal requires (e.g., whether it needs a dedicated process
for its site or origin), as well as what sites or origins are allowed
into a given process.
Both SiteInstance and SiteInstanceGroup simply use their SiteInfo to
represent the required lock, and do not need dedicated APIs for it.
Instead, callers should use ProcessLock::FromSiteInfo to do ProcessLock
comparisons. A ProcessLock constructor that takes a SiteInfo exists,
but has been made private in favour of using the static function.
Internal doc with more background
https://docs.google.com/document/d/19H19czBu8_FjOT8Cy8KdEVXxN-gZ93uNVM5rx1Qu9kw/edit?usp=sharing&resourcekey=0-bf4Im7qJWJDYnImt6OsAzw .
Test: CQ (no behaviour change)
Bug: 1261963
Change-Id: Idd0aefea850a795450b62bfc5dcff0b770739e5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3284127
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Sharon Yang <yangsharon@chromium.org>
Cr-Commit-Position: refs/heads/main@{#946273}
There are some functions that are declared and defined in process_lock.h
that are more than simple getters or setters. Rename them accordingly
and move the definitions to process_lock.cc
Test: CQ (no behaviour change)
Change-Id: Ic600842d78a1d8a92b3538aa00db9dd5fabde113
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3276361
Commit-Queue: Sharon Yang <yangsharon@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#942284}
