This CL adds support to log UrlInfo in trace events. This also
includes WebExposedIsolationInfo which is contained within
UrlInfo. This will be handy for identifying non-standard navigations
in tracing, such as those involving sandboxed frames, IWAs, COOP/COEP,
PDF, etc.
This is then used to add trace events for
SiteInstanceImpl::IsSuitableForUrlInfo() and
SiteInstanceImpl::CreateForUrlInfo(), both core parts of
GetFrameHostForNavigation(). A couple of existing trace events are
converted to use UrlInfo as well.
Bug: 382303496
Change-Id: I85fe16c6b3400356e2d471cc43be3f2a98bfedbb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6073022
Reviewed-by: Sharon Yang <yangsharon@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1394016}
This CL implements process isolation of documents that set
Document-Isolation-Policy. It introduces an AgentClusterKey passed to SiteInfo,
which is used to isolate pages with DIP from pages without DIP. In this CL, the
AgentClusterKey is only computed for pages with DIP. Pages without DIP only get
an AgentClusterKey with an empty URL. Follow-up work will properly compute the
AgentClusterKey for all navigations.
Bug: 333047378
Change-Id: I86f1fa637f68dfe0932be7b2373323472c19ac7a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5588626
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1323878}
This further reduces the visibility that these pages can have into each
others' activity. Fortunately, we do BCG swaps in several cases so this
isn't too complicated.
This is guarded by a FeatureParam so that we can separately control this
behavior in the event it is suspected of causing surprising effects. It
defaults to off but will default to on shortly if all goes well.
Bug: 1439246
Change-Id: I80039754a10418708d575035368ad70f7f6deeba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5100677
Reviewed-by: Max Curran <curranmax@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1271723}
When origins are opaque, calls to
NavigationRequest::GetTentativeOriginAtRequestTime,
GetPossiblyOverriddenOriginFromUrl and similar create a new origin
with a new nonce each time. The origin for a navigation should be
stable across the navigation, so for data: URLs we cache the opaque
origin with its nonce and use that value when possible. In the future we
will expand this to all origins, but for now, it's only for data: URLs.
We do this because we need to make sure the speculative SiteInstance,
which is created very early in the navigation, uses the same nonce that
is computed at origin_to_commit time.
This value is stored in both NavigationRequest and UrlInfo. In UrlInfo,
we expand the use of the existing origin field to store this value.
Test: Added NavigationControllerBrowserTest
Bug: 1447896
Change-Id: I84e5e1233fe06baf6ffcec4684f7306251323c4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4902624
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Sharon Yang <yangsharon@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1224476}
This CL adds a new feature, kOriginKeyedProcessesByDefault, which
when enabled together with kOriginAgentClusterDefaultEnable allows
default-isolated origins to be placed in origin-keyed processes.
After this CL (and with the above modes enabled), origins with no
explicit Origin-Agent-Cluster headers (i.e., the majority of cases) will
temporarily trigger global walks and be tracked by
ChildProcessSecurityPolicy, because such tracking currently happens for
all origin-keyed processes.
Restricting the tracking to cases with explicit headers (non-default
isolation cases) will be addressed in
https://chromium-review.googlesource.com/c/chromium/src/+/4439195.
Bug: 1421329
Change-Id: Ic7341865279182c722644872b391735abeba4125
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4429100
Commit-Queue: James Maclean <wjmaclean@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1141628}
This CL renames kOriginAgentCluster, kRequiresOriginKeyedProcess and
related accessor functions on UrlInfo to clarify that they are specific
to the case where origin isolation requests derive from the explicit
presence of the Origin-Agent-Cluster header.
Bug: 1421329
Change-Id: I10ea6ec97cddd58edec3a164bb1a2890f4a332f2
Low-Coverage-Reason: refactor, no behavior change.
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4518660
Commit-Queue: James Maclean <wjmaclean@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1141585}
This CL is a refactor to store the default OriginAgentIsolationState
in the IsolationContext owned by a BrowsingInstance. By doing this at
the creation of the BrowsingInstance, we snapshot the default in case it
dynamically changes (e.g. via a change in the value of the enterprise
policy OriginAgentClusterDefaultEnabled).
This refactor also allows CanAccessDataForOrigin access to a
BrowsingInstance's default isolation state so that it can appropriately
construct the expected_process_lock. This is potentially a behavior
change, but only after we introduce process-isolated default OAC in a
follow-on CL.
Bug: 1421329
Change-Id: I7829c151365b685c724f79e8d40c56dd27ec5819
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4427512
Commit-Queue: James Maclean <wjmaclean@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1136085}
In preparation for supporting process-isolated origin isolation by
default, we would like to modify UrlInfo so that it always constructs
with the default origin isolation mode when no isolation mode is
explicitly indicated via the Origin-Agent-Cluster header.
This CL is a refactor to simplify the construction of a default
isolation request by separating out kCOOP from OriginIsolationRequest.
Since kCOOP is tracked in its own bool inside SiteInfo, this is easy
to do.
Bug: 1421329, 1018656
Change-Id: Ia537299f2da6504b0c0f1c090e991db3e09144bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4393673
Commit-Queue: James Maclean <wjmaclean@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1127144}
There are corner cases where about:blank can't stay in the source
SiteInstance, such as when the navigation to about:blank occurs from
an error page SiteInstance or if it requires a new BrowsingInstance.
Currently, in such cases, we place about:blank in an unassigned
SiteInstance and an unlocked process. However, this is problematic if
the navigation is renderer-initiated and carries an initiator origin,
since it might result in committing about:blank in an unlocked process
but with an initiator origin for a site that requires a dedicated
process. This fails Citadel enforcements that we're hoping to enable
for desktop platforms in issue 764958.
This situation happens in a couple of existing tests:
- UnassignedSiteInstanceBrowserTest.RendererInitiatedNavigationTo uses
a renderer-initiated navigation to about:blank where the old page
may get bfcached, and with bfcache the about:blank document must
commit in a new BrowsingInstance and new SiteInstance, which is
currently unassigned, yet about:blank still carries the initiator's
origin which might require a dedicated process.
- ChromeNavigationBrowserTest.RedirectErrorPageReloadToAboutBlank uses
an extension that redirects navigations to about:blank via
webRequest, starting from an error page which makes the source
SiteInstance incompatible. This CL adds a test for a similar
scenario that also involves navigating to about:blank from an error
page, but without an extension redirect. See the linked bug for
more details.
This CL modifies UrlInfo's origin to carry the initiator origin for
renderer-initiated about:blank navigations. This origin is not going
to be used in the common case where about:blank is left in the source
SiteInstance and an existing process. But if we end up needing to
create a new SiteInstance and compute its SiteInfo and ProcessLock for
an about:blank navigation, this will ensure that we would honor the
initiator origin, if it was present. As a result, about:blank in such
a case will now load in a SiteInstance corresponding to the
initiator's site, and its process will also be locked to that site (if
the site requires a dedicated process).
Bug: 1426928
Change-Id: I19d0265a4f0e6aae7ad732fe92b28c58bcb17c1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4362801
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1124841}
This is the core patch of the COOP: restrict-properties
implementation. It introduces an important abstraction, the
CoopRelatedGroup. It represents a grouping of BrowsingContext groups
(BrowsingInstance) that are capable of a limited set of asynchronous
interactions, namely postMessage and closed.
It is built as closely as possible to how BrowsingInstance is built
on SiteInstance:
- It is fully private, and interacted with through SiteInstance and
BrowsingInstance. SiteInstance exposes a single new method,
GetCoopRelatedSiteInstance(), that works similarly to the
GetRelatedSiteInstance() method, but at the CoopRelatedGroup level.
- It is collectively owned by the BrowsingInstances that are part of
it. A register/unregister mechanism is used to know about the
BrowsingInstances that are still part of the group.
- It might reuse an existing BrowsingInstance when necessary, similar
to how SiteInstance's are reused within a BrowsingInstance.
To support BrowsingInstance reuse, some information needs to be
available to the CoopRelatedGroup when doing the selection. In
particular it needs to know whether the BrowsingInstance
hosts COOP: restrict-properties pages, and from which origin. We
record that information from the NavigationRequest into UrlInfo.
The SiteInstance picking algorithm is updated to output a SiteInstance
created via GetCoopRelatedSiteInstance() when a
BrowsingContextGroupSwap::kCoopRelatedSwap is requested.
Added browser tests and unit tests cover the newly introduced code.
Bug: 1221127, 1385827
Change-Id: Ief8436b5d532b6aadc5d9a29d94346e3fae2641c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3948564
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1112861}
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 CL adds support for loadDataWithBaseUrl() <webview> API [1] when
<webview> is running in site isolation mode. The high-level idea is
to perform process selection and site URL computation using the base
URL's origin, rather than the data URL's opaque origin that would be
used normally. This matches up with the resulting document also
having the base URL's origin. For example,
webview.loadDataWithBaseUrl("data,foo", "https://www.example.com")
will load a document with contents "foo" with an origin of
https://www.example.com, using a guest process that's locked to
https://example.com.
To do this, this CL extends the mechanism originally introduced to
support an alternate origin for resources loaded from Web Bundles.
That mechanism used the UrlInfo::origin field, setting it to the
bundle's origin rather than the origin of UrlInfo::url, and then using
that origin for web bundle (urn:) URLs in
SiteInfo::GetSiteForURLInternal(). Now, UrlInfo::origin is also set
to the base URL origin when the navigation is done via
LoadDataWithBaseURL. UrlInfo::origin is converted to be an
absl::optional, and when it has a value,
SiteInfo::GetSiteForURLInternal() uses it in navigations to urn: and
data: URLs. Note that SiteInfo::GetSiteForURLInternal() does not know
precisely whether it's in the context of a LoadDataWithBaseURL
navigation, and adding a bit to convey that to UrlInfo seemed like
unneeded complexity, so we compromise by allowing UrlInfo::origin to
be used for all data: URLs, which is still safer than allowing origin
overrides for arbitrary navigations. NavigationRequest::GetUrlInfo()
is modified to populate the UrlInfo::origin for both Web Bundles and
LoadDataWithBaseURL.
Test coverage is provided through existing <webview>
LoadDataWithBaseURL tests, one of which is also slightly updated to
verify site isolation characteristics of the committed document. A
few tests are also updated to not pass .WithOrigin() unnecessarily
in cases where the origin is already the same as the URL.
One area of complexity here is the fact that UrlInfo is not only used
to describe information about the URL being navigated to, but it's
also used to implicitly pass around the renderer's *committed URL* and
*committed origin* from RenderFrameHostImpl::CanCommitOriginAndUrl()
(called as part of ValidateDidCommitParams()) into a few other places
that may end up referencing
RenderFrameHostImpl::CanCommitOriginAndUrl() later (e.g., to do a
ProcessLock comparison). This necessitated a few tweaks in
ChildProcessSecurityPolicyImpl::CanCommitOriginAndUrl() and
Navigator::CheckWebUIRendererDoesNotDisplayNormalURL() to maintain the
old assumptions, but this area of code should be investigated and
cleaned up further (separately from this CL). For more info, see
https://crbug.com/1320402.
Note that LoadDataWithBaseURL() has some special logic to skip
commit-time checks [2], but this is intended for Android Webview and
applies only when the process isn't locked. In the <webview> tag
case, the LoadDataWithBaseURL process will now be locked and subject
to regular checks. Android Webview's loadDataWithBaseURL behavior
remains unchanged by this CL, with no site isolation before or after.
[1] https://developer.chrome.com/docs/extensions/reference/webviewTag/#method-loadDataWithBaseUrl
[2] https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/render_frame_host_impl.cc;l=10728-10744;drc=edab26bf11e993e5069d433e8c2186ad2dee012e
Bug: 1267977
Change-Id: I9ebc0c5dc61d7b2dc8d12648b84e80f1423eab38
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3591660
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: James Maclean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/main@{#998992}
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}
When we take decisions in the process model about about isolation, we're
not only considering the raw values, but things like, "Are we doing a
speculative computation?", "Is this a navigation to about:blank?", etc.
These are grouped in the IsSiteInstanceCompatibleWithWebExposedIsolation
function. When we reach the lower levels of SiteInstances,
BrowsingInstances, etc, we don't have these information anymore, and
that leads us to doing weird WebExposedIsolationInfo (WEII) overrides.
See code with reference to https://crbug.com/1243449 for some examples.
Imagine we go from a COI page to another COI page. When we do the
speculative computation for a RenderFrameHost, since it's only
speculative, we've not yet received the headers that can determine
a final isolation state. It can live in any SiteInstance regarding
isolation. Later, down the line we check that we only put COI with COI,
and CHECK because we don't have the initial information anymore.
The alternative we propose is to make WEII an optional value. More
precisely, three different places hold WEII: UrlInfo, SiteInfo and
BrowsingInstance. We propose to make UrlInfo's WEII optional. The WEII
for a navigation is computed in the NavigationRequest and set in an
UrlInfo. If UrlInfo has an optional WEII, we can convey the information
that this is one of the special cases above, and that we shouldn't pay
attention to it.
The goal of this patch is to introduce the optional member itself, and
as well as the first use for it, a replacement for "is_speculative".
First we verified in this document that we have to keep this mechanism:
https://docs.google.com/document/d/135l25QN2MVjUAJCrvha2UQKqAq7QFp1sRRtDguHtEmA/edit#heading=h.ybelxngq8lbn
The current way of doing that is to infer a "is_speculative" boolean
based on the NavigationRequest state and carry it over in the
RenderFrameHostManager call sites. Instead, we use a nullopt value.
In terms of implementation:
- We introduce a set of static AreCompatible() funcctions that operate
between optional WEII and WEII, that returns true if comparing any WEII
with an empty WEII optional. The underlying meaning is that WEII should
not be a criteria for rejecting a SiteInstance if WEII is nullopt.
- In places that take an UrlInfo WEII and convert it to a SiteInfo /
BrowsingInstance WEII, we convert an empty WEII into a non isolated one.
While this is still technically incorrect, this replaces the previous
behavior that depended on the current frame isolation state. This has
the benefit of being consistent, but does not lead yet to the removal of
overrides mentioned above. This will come in following patches.
- We convert state >= REDIRECT to state >= RESPONSE. We do not need to
enforce WEII at redirect time, there was some confusion with COOP that
is applied even on redirects.
Bug: 1266819
Change-Id: I0176e2283f02929122c2da199737bccf2aea4c09
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3306588
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Cr-Commit-Position: refs/heads/main@{#953493}
This CL refactors the code that tracks OAC (OriginAgentCluster)
isolation opt-ins to allow for having both origin-keyed OAC processes
and non-origin-keyed OAC processes present at the same time.
The map in ChildProcessSecurityPolicyImpl that tracks OAC opt-ins is
|origin_isolation_by_browsing_instance_|. Prior to this CL it just
tracks a list of origins, with the assumption being that any origin
in the list is opted in for whatever OAC mechanism is currently being
used.
The two mechanisms are origin_keyed, in which each origin is assigned
its own process, and non-origin_keyed, in which each origin is logically
isolated in the renderer process, but may share a renderer process with
other origins. At present, only one of these mechanisms is active for
a given browser session.
In this CL we modify |origin_isolation_by_browsing_instance_| to track
which mechanism to use for each origin, thus allowing both mechanisms
to be active at once.
This CL also enhances the UrlInfo::OriginIsolationRequest flags to
allow us (in some future CL) to control which mechanism to register at
opt-in time.
Bug: 1259920
Change-Id: Id6a9c396f2cf94264aab171b80d72c7f4917a2f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3244802
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: W. James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/main@{#941698}
