Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-23 12:51:06 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1901387 - Part 3: Make nsIPrincipal::CheckMayLoad threadsafe, r=asuth,ckerschb,extension-reviewers,robwu

Browse Source 
This method was previously non-threadsafe due to it needing to access dynamic
URI flags. These flags were used to check the WEbExtensionPolicy to see if the
webextension resource being loaded should be accessible.

Making dynamic URI flags available off-main-thread in general would
unfortunately be quite difficult, due to some of them depending on things like
JS `nsIAboutModule` implementations, so that was not the approach taken.

Instead, all information required is already available in the threadsafe
WebExtensionPolicyCore, which is now directly queried, instead of being queried
indirectly through protocol flags.

Differential Revision: https://phabricator.services.mozilla.com/D215026
This commit is contained in:
Nika Layzell 2024-06-28 17:34:08 +00:00
parent e4913ea179
commit 5fb9592e42
4 changed files with 29 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
caps/BasePrincipal.cpp
View File

@ -610,7 +610,6 @@ BasePrincipal::SubsumesConsideringDomainIgnoringFPD(nsIPrincipal* aOther,
NS_IMETHODIMP
BasePrincipal::CheckMayLoad(nsIURI* aURI, bool aAllowIfInheritsPrincipal) {
AssertIsOnMainThread();
return CheckMayLoadHelper(aURI, aAllowIfInheritsPrincipal, false, 0);
}
@ -627,12 +626,11 @@ nsresult BasePrincipal::CheckMayLoadHelper(nsIURI* aURI,
bool aAllowIfInheritsPrincipal,
bool aReport,
uint64_t aInnerWindowID) {
AssertIsOnMainThread(); // Accesses non-threadsafe URI flags and the
// non-threadsafe ExtensionPolicyService
NS_ENSURE_ARG_POINTER(aURI);
MOZ_ASSERT(
aReport || aInnerWindowID == 0,
"Why do we have an inner window id if we're not supposed to report?");
MOZ_ASSERT(!aReport || NS_IsMainThread(), "Must be on main thread to report");
// Check the internal method first, which allows us to quickly approve loads
// for the System Principal.
@ -653,38 +651,33 @@ nsresult BasePrincipal::CheckMayLoadHelper(nsIURI* aURI,
}
}
// Web Accessible Resources in MV2 Extensions are marked with
// URI_FETCHABLE_BY_ANYONE
bool fetchableByAnyone;
rv = NS_URIChainHasFlags(aURI, nsIProtocolHandler::URI_FETCHABLE_BY_ANYONE,
&fetchableByAnyone);
if (NS_SUCCEEDED(rv) && fetchableByAnyone) {
return NS_OK;
}
// Get the principal uri for the last flag check or error.
// Get the principal uri for the WebExtension access check or error.
nsCOMPtr<nsIURI> prinURI;
rv = GetURI(getter_AddRefs(prinURI));
if (!(NS_SUCCEEDED(rv) && prinURI)) {
return NS_ERROR_DOM_BAD_URI;
}
// If MV3 Extension uris are web accessible by this principal it is allowed to
// load.
bool maybeWebAccessible = false;
NS_URIChainHasFlags(aURI, nsIProtocolHandler::WEBEXT_URI_WEB_ACCESSIBLE,
&maybeWebAccessible);
NS_ENSURE_SUCCESS(rv, rv);
if (maybeWebAccessible) {
bool isWebAccessible = false;
rv = ExtensionPolicyService::GetSingleton().SourceMayLoadExtensionURI(
prinURI, aURI, &isWebAccessible);
if (NS_SUCCEEDED(rv) && isWebAccessible) {
// If the URL being loaded corresponds to a WebExtension URL, ask the policy
// if the path should be accessible.
//
// This is done directly, rather than first checking `NS_URIChainHasFlags` for
// `WEBEXT_URI_WEB_ACCESSIBLE`, as `CheckMayLoadHelper` may be called
// off-main-thread, and it is not safe to check dynamic URI flags
// off-main-thread. MV2 web-accessible resources also currently do not set
// `WEBEXT_URI_WEB_ACCESSIBLE`, and need to be allowed in this code.
extensions::URLInfo urlInfo(aURI);
if (RefPtr<extensions::WebExtensionPolicyCore> urlPolicyCore =
ExtensionPolicyService::GetCoreByURL(urlInfo)) {
extensions::URLInfo prinUrlInfo(prinURI);
if (urlPolicyCore->SourceMayAccessPath(prinUrlInfo, urlInfo.FilePath())) {
return NS_OK;
}
}
if (aReport) {
// FIXME: Once bug 1900706 is complete, reporting can be updated to work
// off-main-thread.
nsScriptSecurityManager::ReportError("CheckSameOriginError", prinURI, aURI,
mOriginAttributes.IsPrivateBrowsing(),
aInnerWindowID);

3
caps/nsIPrincipal.idl
View File

@ -175,11 +175,12 @@ interface nsIPrincipal : nsISupports
* If the load is allowed this function does nothing. If the load is not
* allowed the function throws NS_ERROR_DOM_BAD_URI.
*
* May be called from any thread.
*
* NOTE: Other policies might override this, such as the Access-Control
* specification.
* NOTE: The 'domain' attribute has no effect on the behaviour of this
* function.
* NOTE: Main-Thread Only.
*
*
* @param uri The URI about to be loaded.

14
netwerk/base/nsIProtocolHandler.idl
View File

@ -251,14 +251,6 @@ interface nsIProtocolHandler : nsISupports
*/
const unsigned long URI_IS_POTENTIALLY_TRUSTWORTHY = (1<<17);
/**
* This URI may be fetched and the contents are visible to anyone. This is
* semantically equivalent to the resource being served with all-access CORS
* headers. This is only used in MV2 Extensions and should not otherwise
* be used.
*/
const unsigned long URI_FETCHABLE_BY_ANYONE = (1 << 18);
/**
* If this flag is set, then the origin for this protocol is the full URI
* spec, not just the scheme + host + port.
@ -311,7 +303,7 @@ interface nsIProtocolHandler : nsISupports
*/
const unsigned long DYNAMIC_URI_FLAGS =
URI_LOADABLE_BY_ANYONE | URI_DANGEROUS_TO_LOAD |
URI_IS_POTENTIALLY_TRUSTWORTHY | URI_FETCHABLE_BY_ANYONE |
URI_LOADABLE_BY_EXTENSIONS | URI_DISALLOW_IN_PRIVATE_CONTEXT |
WEBEXT_URI_WEB_ACCESSIBLE | URI_HAS_WEB_EXPOSED_ORIGIN;
URI_IS_POTENTIALLY_TRUSTWORTHY | URI_LOADABLE_BY_EXTENSIONS |
URI_DISALLOW_IN_PRIVATE_CONTEXT | WEBEXT_URI_WEB_ACCESSIBLE |
URI_HAS_WEB_EXPOSED_ORIGIN;
};

9
netwerk/protocol/res/ExtensionProtocolHandler.cpp
View File

@ -419,10 +419,15 @@ nsresult ExtensionProtocolHandler::GetFlagsForURI(nsIURI* aURI,
// In general a moz-extension URI is only loadable by chrome, but an
// allowlist subset are web-accessible (and cross-origin fetchable).
// The allowlist is checked using EPS.SourceMayLoadExtensionURI in
// BasePrincipal and nsScriptSecurityManager.
// nsScriptSecurityManager, and WEPC.SourceMayAccessPath in BasePrincipal.
if (policy->IsWebAccessiblePath(url.FilePath())) {
if (policy->ManifestVersion() < 3) {
flags |= URI_LOADABLE_BY_ANYONE | URI_FETCHABLE_BY_ANYONE;
// We could also be `WEBEXT_URI_WEB_ACCESSIBLE` here, but this would
// just check `IsWebAccessiblePath` for Manifest V2.
// nsIPrincipal::CheckMayLoad (which is used for resources) already
// directly queries the WebExtensionPolicy, so we only need to set
// URI_LOADABLE_BY_ANYONE to allow navigation loads.
flags |= URI_LOADABLE_BY_ANYONE;
} else {
flags |= WEBEXT_URI_WEB_ACCESSIBLE;
}