corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1834864 - Select BCG more consistently during COOP+COEP process switches, r=smaug,tabbrowser-reviewers,mak

Browse Source 
Previously it was possible to bypass specific BCG selection based on
cross-origin isolated status if the site was allowed to load file URIs
using enterprise policies, which could lead to a crash.

This patch changes the behaviour such that BCG selection now happens
correctly. The site will still not be cross-origin isolated due to being
loaded into a file content process.

Differential Revision: https://phabricator.services.mozilla.com/D217007
This commit is contained in:
Nika Layzell
2024-07-19 14:38:20 +00:00
parent 971159e0a1
commit 0c56f81d86
7 changed files with 93 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
browser/components/tabbrowser/test/browser/tabs/browser.toml
View File

@@ -202,6 +202,10 @@ skip-if = ["verify && os == 'mac'"]
["browser_new_file_whitelisted_http_tab.js"]
https_first_disabled = true
support-files = [
"file_coop_coep.html",
"file_coop_coep.html^headers^",
]
["browser_new_tab_bookmarks_toolbar_height.js"]
skip-if = ["!verify && os == 'mac'"] # Bug 1872477

62
browser/components/tabbrowser/test/browser/tabs/browser_new_file_whitelisted_http_tab.js
View File

@@ -1,8 +1,15 @@
/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
// eslint-disable-next-line @microsoft/sdl/no-insecure-url
const TEST_HTTP = "http://example.org/";
const TEST_HTTP_DOMAIN = "https://example.org/";
const TEST_ROOT = getRootDirectory(gTestPath).replace(
"chrome://mochitests/content",
"https://example.org"
);
const TEST_NORMAL = `${TEST_ROOT}dummy_page.html`;
const TEST_COOP_COEP = `${TEST_ROOT}file_coop_coep.html`;
// Test for bug 1378377.
add_task(async function () {
@@ -11,27 +18,72 @@ add_task(async function () {
set: [["browser.tabs.remote.separateFileUriProcess", true]],
});
await BrowserTestUtils.withNewTab(TEST_HTTP, async function (fileBrowser) {
await BrowserTestUtils.withNewTab(TEST_NORMAL, async function (fileBrowser) {
ok(
E10SUtils.isWebRemoteType(fileBrowser.remoteType),
"Check that tab normally has web remote type."
);
await SpecialPowers.spawn(fileBrowser, [], () => {
ok(
!content.crossOriginIsolated,
"Tab content is not cross-origin isolated"
);
});
});
await BrowserTestUtils.withNewTab(
TEST_COOP_COEP,
async function (fileBrowser) {
ok(
fileBrowser.remoteType.startsWith(
E10SUtils.WEB_REMOTE_COOP_COEP_TYPE_PREFIX
),
"Check that COOP+COEP tab normally has webCOOP+COEP remote type."
);
await SpecialPowers.spawn(fileBrowser, [], () => {
ok(content.crossOriginIsolated, "Tab content is cross-origin isolated");
});
}
);
// Set prefs to whitelist TEST_HTTP for file:// URI use.
await SpecialPowers.pushPrefEnv({
set: [
["capability.policy.policynames", "allowFileURI"],
["capability.policy.allowFileURI.sites", TEST_HTTP],
["capability.policy.allowFileURI.sites", TEST_HTTP_DOMAIN],
["capability.policy.allowFileURI.checkloaduri.enabled", "allAccess"],
],
});
await BrowserTestUtils.withNewTab(TEST_HTTP, async function (fileBrowser) {
await BrowserTestUtils.withNewTab(TEST_NORMAL, async function (fileBrowser) {
is(
fileBrowser.remoteType,
E10SUtils.FILE_REMOTE_TYPE,
"Check that tab now has file remote type."
);
await SpecialPowers.spawn(fileBrowser, [], () => {
ok(
!content.crossOriginIsolated,
"Tab content is not cross-origin isolated"
);
});
});
await BrowserTestUtils.withNewTab(
TEST_COOP_COEP,
async function (fileBrowser) {
is(
fileBrowser.remoteType,
E10SUtils.FILE_REMOTE_TYPE,
"Check that tab now has file remote type."
);
await SpecialPowers.spawn(fileBrowser, [], () => {
ok(
!content.crossOriginIsolated,
"Tab content is not cross-origin isolated"
);
});
}
);
});

1
browser/components/tabbrowser/test/browser/tabs/file_coop_coep.html Normal file
View File

@@ -0,0 +1 @@
<!DOCTYPE html>

2
browser/components/tabbrowser/test/browser/tabs/file_coop_coep.html^headers^ Normal file
View File

@@ -0,0 +1,2 @@
Cross-Origin-Opener-Policy: same-origin
Cross-Origin-Embedder-Policy: require-corp

10
docshell/base/CanonicalBrowsingContext.cpp
View File

@@ -2172,11 +2172,13 @@ CanonicalBrowsingContext::ChangeRemoteness(
new PendingRemotenessChange(this, promise, aPendingSwitchId, aOptions);
mPendingRemotenessChange = change;
// If a specific BrowsingContextGroup ID was specified for this load, make
// sure to keep it alive until the process switch is completed.
if (aOptions.mSpecificGroupId) {
// If we're replacing BrowsingContext, determine which BrowsingContextGroup
// we'll switch into, taking into account load options.
if (aOptions.mReplaceBrowsingContext) {
change->mSpecificGroup =
BrowsingContextGroup::GetOrCreate(aOptions.mSpecificGroupId);
aOptions.mSpecificGroupId
? BrowsingContextGroup::GetOrCreate(aOptions.mSpecificGroupId)
: BrowsingContextGroup::Create(aOptions.mShouldCrossOriginIsolate);
change->mSpecificGroup->AddKeepAlive();
}

43
dom/ipc/ProcessIsolation.cpp
View File

@@ -418,6 +418,24 @@ static already_AddRefed<BasePrincipal> GetAboutReaderURLPrincipal(
return nullptr;
}
/**
* Check the Cross-Origin-Opener-Policy of the given channel or ancestor
* BrowsingContext, checking if the response should be cross-origin isolated.
*/
static bool ShouldCrossOriginIsolate(nsIChannel* aChannel,
WindowGlobalParent* aParentWindow) {
nsILoadInfo::CrossOriginOpenerPolicy coop =
nsILoadInfo::OPENER_POLICY_UNSAFE_NONE;
if (aParentWindow) {
coop = aParentWindow->BrowsingContext()->Top()->GetOpenerPolicy();
} else if (nsCOMPtr<nsIHttpChannelInternal> httpChannel =
do_QueryInterface(aChannel)) {
MOZ_ALWAYS_SUCCEEDS(httpChannel->GetCrossOriginOpenerPolicy(&coop));
}
return coop ==
nsILoadInfo::OPENER_POLICY_SAME_ORIGIN_EMBEDDER_POLICY_REQUIRE_CORP;
}
/**
* Returns `true` if loads for this site should be isolated on a per-site basis.
* If `aTopBC` is nullptr, this is being called to check if a shared or service
@@ -580,6 +598,8 @@ Result<NavigationIsolationOptions, nsresult> IsolationOptionsForNavigation(
NavigationIsolationOptions options;
options.mReplaceBrowsingContext = aHasCOOPMismatch;
options.mShouldCrossOriginIsolate =
ShouldCrossOriginIsolate(aChannel, aParentWindow);
// Check if this load has an explicit remote type override. This is used to
// perform an about:blank load within a specific content process.
@@ -871,28 +891,9 @@ Result<NavigationIsolationOptions, nsresult> IsolationOptionsForNavigation(
webProcessType = WebProcessType::WebIsolated;
}
// Check if we should be loading in a webCOOP+COEP remote type due to our COOP
// status.
nsILoadInfo::CrossOriginOpenerPolicy coop =
nsILoadInfo::OPENER_POLICY_UNSAFE_NONE;
if (aParentWindow) {
coop = aTopBC->GetOpenerPolicy();
} else if (nsCOMPtr<nsIHttpChannelInternal> httpChannel =
do_QueryInterface(aChannel)) {
MOZ_ALWAYS_SUCCEEDS(httpChannel->GetCrossOriginOpenerPolicy(&coop));
}
if (coop ==
nsILoadInfo::OPENER_POLICY_SAME_ORIGIN_EMBEDDER_POLICY_REQUIRE_CORP) {
// Check if we should be cross-origin isolated.
if (options.mShouldCrossOriginIsolate) {
webProcessType = WebProcessType::WebCoopCoep;
// If we're changing BrowsingContext, and are going to end up within a
// webCOOP+COEP group, ensure we use a cross-origin isolated BCG ID.
if (options.mReplaceBrowsingContext) {
MOZ_ASSERT(!options.mSpecificGroupId,
"overriding previously-specified BCG ID");
options.mSpecificGroupId = BrowsingContextGroup::CreateId(
/* aPotentiallyCrossOriginIsolated */ true);
}
}
switch (webProcessType) {

1
dom/ipc/ProcessIsolation.h
View File

@@ -36,6 +36,7 @@ struct NavigationIsolationOptions {
nsCString mRemoteType;
bool mReplaceBrowsingContext = false;
uint64_t mSpecificGroupId = 0;
bool mShouldCrossOriginIsolate = false;
bool mTryUseBFCache = false;
RefPtr<SessionHistoryEntry> mActiveSessionHistoryEntry;
};