Bug 1238160 - Add assertions in non-desktop code paths. r=bz,fabrice

Several code paths try to ask the principal if it's in a browser element, but
the principal now only knows about *isolated* browser elements.  All such code
paths are currently unused on desktop.  The frame loader now asserts that
isolation remains enabled for cases where apps are used.

MozReview-Commit-ID: 775DZecc35t
This commit is contained in:
J. Ryan Stinnett 2016-02-17 22:57:41 -06:00
parent 3872b5e0b0
commit 7e2f42d388
9 changed files with 110 additions and 11 deletions

View File

@ -260,13 +260,25 @@ uint16_t
nsScriptSecurityManager::AppStatusForPrincipal(nsIPrincipal *aPrin)
{
uint32_t appId = aPrin->GetAppId();
bool inMozBrowser = aPrin->GetIsInIsolatedMozBrowserElement();
// After bug 1238160, the principal no longer knows how to answer "is this a
// browser element", which is really what this code path wants. Currently,
// desktop is the only platform where we intend to disable isolation on a
// browser frame, so non-desktop should be able to assume that
// inIsolatedMozBrowser is true for all mozbrowser frames. Additionally,
// apps are no longer used on desktop, so appId is always NO_APP_ID. We use
// a release assertion in nsFrameLoader::OwnerIsIsolatedMozBrowserFrame so
// that platforms with apps can assume inIsolatedMozBrowser is true for all
// mozbrowser frames.
bool inIsolatedMozBrowser = aPrin->GetIsInIsolatedMozBrowserElement();
NS_WARN_IF_FALSE(appId != nsIScriptSecurityManager::UNKNOWN_APP_ID,
"Asking for app status on a principal with an unknown app id");
// Installed apps have a valid app id (not NO_APP_ID or UNKNOWN_APP_ID)
// and they are not inside a mozbrowser.
if (appId == nsIScriptSecurityManager::NO_APP_ID ||
appId == nsIScriptSecurityManager::UNKNOWN_APP_ID || inMozBrowser)
appId == nsIScriptSecurityManager::UNKNOWN_APP_ID ||
inIsolatedMozBrowser)
{
return nsIPrincipal::APP_STATUS_NOT_INSTALLED;
}
@ -291,7 +303,7 @@ nsScriptSecurityManager::AppStatusForPrincipal(nsIPrincipal *aPrin)
// The app could contain a cross-origin iframe - make sure that the content
// is actually same-origin with the app.
MOZ_ASSERT(inMozBrowser == false, "Checked this above");
MOZ_ASSERT(inIsolatedMozBrowser == false, "Checked this above");
PrincipalOriginAttributes attrs(appId, false);
nsCOMPtr<nsIPrincipal> appPrin = BasePrincipal::CreateCodebasePrincipal(appURI, attrs);
NS_ENSURE_TRUE(appPrin, nsIPrincipal::APP_STATUS_NOT_INSTALLED);

View File

@ -106,6 +106,10 @@ AppsService.prototype = {
return DOMApplicationRegistry.getWebAppsBasePath();
},
areAnyAppsInstalled: function() {
return DOMApplicationRegistry.areAnyAppsInstalled();
},
getAppInfo: function getAppInfo(aAppId) {
debug("getAppInfo()");
return DOMApplicationRegistry.getAppInfo(aAppId);

View File

@ -412,6 +412,10 @@ this.DOMApplicationRegistry = {
return null;
},
areAnyAppsInstalled: function() {
return AppsUtils.areAnyAppsInstalled(this.webapps);
},
getAppInfo: function getAppInfo(aAppId) {
return AppsUtils.getAppInfo(this.webapps, aAppId);
},

View File

@ -345,6 +345,10 @@ this.AppsUtils = {
return "";
},
areAnyAppsInstalled: function(aApps) {
return Object.getOwnPropertyNames(aApps).length > 0;
},
getCoreAppsBasePath: function getCoreAppsBasePath() {
debug("getCoreAppsBasePath()");
try {

View File

@ -4799,6 +4799,10 @@ this.DOMApplicationRegistry = {
return OS.Path.dirname(this.appsFile);
},
areAnyAppsInstalled: function() {
return AppsUtils.areAnyAppsInstalled(this.webapps);
},
updateDataStoreEntriesFromLocalId: function(aLocalId) {
let app = appsService.getAppByLocalId(aLocalId);
if (app) {

View File

@ -1627,7 +1627,54 @@ nsFrameLoader::OwnerIsIsolatedMozBrowserFrame()
if (!browserFrame) {
return false;
}
return OwnerIsMozBrowserFrame() && browserFrame->GetIsolated();
if (!OwnerIsMozBrowserFrame()) {
return false;
}
bool isolated = browserFrame->GetIsolated();
if (isolated) {
return true;
}
// After bug 1238160, which allows isolation to be disabled on mozbrowser
// frames, we no longer have a way to tell from the principal alone if
// something "is a mozbrowser". Instead, we now only know "is an isolated
// mozbrowser". The following code paths would return invalid results if it
// were possible to have apps *and* isolation could be disabled:
// * CheckPermission in AppProcessChecker.cpp
// * nsScriptSecurityManager::AppStatusForPrincipal
// * init() in SystemMessageManager.js
// Currently, desktop is the only platform where we intend to disable
// isolation on a browser frame, so non-desktop should be able to assume that
// inIsolatedMozBrowser is true for all mozbrowser frames. To enforce these
// assumptions, we assert that there are no apps installed if we have tried
// to disable isolation.
nsCOMPtr<nsIAppsService> appsService = do_GetService(APPS_SERVICE_CONTRACTID);
if (!appsService) {
// If the apps service is not present, we assume this means there can't be
// any apps at all, so there is no problem.
return false;
}
bool appsInstalled;
nsresult rv = appsService->AreAnyAppsInstalled(&appsInstalled);
if (NS_WARN_IF(NS_FAILED(rv))) {
// The apps service exists, but it threw an error when checking if there are
// any apps, so we don't know if we have them or not.
return false;
}
#ifdef MOZ_B2G
MOZ_RELEASE_ASSERT(!appsInstalled,
"Disabling mozbrowser isolation is not currently "
"allowed when apps are installed.");
#else
if (appsInstalled) {
NS_WARNING("Disabling mozbrowser isolation is not currently allowed when "
"apps are installed.");
}
#endif
return false;
}
void

View File

@ -66,6 +66,11 @@ interface nsIAppsService : nsISupports
*/
DOMString getWebAppsBasePath();
/**
* Returns true if at least one app is in the registry.
*/
boolean areAnyAppsInstalled();
jsval getAppInfo(in DOMString appId);
/**

View File

@ -91,7 +91,7 @@ AssertAppProcess(PBrowserParent* aActor,
TabParent* tab = TabParent::GetFrom(aActor);
nsCOMPtr<mozIApplication> app = tab->GetOwnOrContainingApp();
return CheckAppTypeHelper(app, aType, aCapability, tab->IsBrowserElement());
return CheckAppTypeHelper(app, aType, aCapability, tab->IsMozBrowserElement());
}
static bool
@ -173,7 +173,7 @@ AssertAppProcess(TabContext& aContext,
}
nsCOMPtr<mozIApplication> app = aContext.GetOwnOrContainingApp();
return CheckAppTypeHelper(app, aType, aCapability, aContext.IsBrowserElement());
return CheckAppTypeHelper(app, aType, aCapability, aContext.IsMozBrowserElement());
}
bool
@ -249,16 +249,16 @@ AssertAppPrincipal(PContentParent* aActor,
}
uint32_t principalAppId = aPrincipal->GetAppId();
bool inBrowserElement = aPrincipal->GetIsInBrowserElement();
bool inIsolatedBrowser = aPrincipal->GetIsInIsolatedMozBrowserElement();
// Check if the permission's appId matches a child we manage.
nsTArray<TabContext> contextArray =
static_cast<ContentParent*>(aActor)->GetManagedTabContext();
for (uint32_t i = 0; i < contextArray.Length(); ++i) {
if (contextArray[i].OwnOrContainingAppId() == principalAppId) {
// If the child only runs inBrowserElement content and the principal claims
// it's not in a browser element, it's lying.
if (!contextArray[i].IsBrowserElement() || inBrowserElement) {
// If the child only runs isolated browser content and the principal
// claims it's not in an isolated browser element, it's lying.
if (!contextArray[i].IsIsolatedMozBrowserElement() || inIsolatedBrowser) {
return true;
}
break;
@ -319,8 +319,17 @@ CheckPermission(PContentParent* aActor,
// For browser content (and if the app hasn't explicitly denied this),
// consider the requesting origin, not the app.
// After bug 1238160, the principal no longer knows how to answer "is this a
// browser element", which is really what this code path wants. Currently,
// desktop is the only platform where we intend to disable isolation on a
// browser frame, so non-desktop should be able to assume that
// inIsolatedMozBrowser is true for all mozbrowser frames. This code path is
// currently unused on desktop, since MOZ_CHILD_PERMISSIONS is only set for
// MOZ_B2G. We use a release assertion in
// nsFrameLoader::OwnerIsIsolatedMozBrowserFrame so that platforms with apps
// can assume inIsolatedMozBrowser is true for all mozbrowser frames.
if (appPerm == nsIPermissionManager::PROMPT_ACTION &&
aPrincipal->GetIsInBrowserElement()) {
aPrincipal->GetIsInIsolatedMozBrowserElement()) {
return permission;
}

View File

@ -332,6 +332,16 @@ SystemMessageManager.prototype = {
"SystemMessageManager:GetPendingMessages:Return"]);
let principal = aWindow.document.nodePrincipal;
// After bug 1238160, the principal no longer knows how to answer "is this a
// browser element", which is really what this code path wants. Currently,
// desktop is the only platform where we intend to disable isolation on a
// browser frame, so non-desktop should be able to assume that
// inIsolatedMozBrowser is true for all mozbrowser frames. Additionally,
// this system message API is disabled on desktop behind the pref
// "dom.sysmsg.enabled". We use a release assertion in
// nsFrameLoader::OwnerIsIsolatedMozBrowserFrame so that platforms with apps
// can assume inIsolatedMozBrowser is true for all mozbrowser frames.
this._isInBrowserElement = principal.isInIsolatedMozBrowserElement;
this._pageURL = principal.URI.spec;