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
--- a/caps/nsScriptSecurityManager.cpp
+++ b/caps/nsScriptSecurityManager.cpp
@@ -255,23 +255,35 @@ nsScriptSecurityManager::SecurityHashURI
{
return NS_SecurityHashURI(aURI);
}
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;
}
nsCOMPtr<nsIAppsService> appsService = do_GetService(APPS_SERVICE_CONTRACTID);
NS_ENSURE_TRUE(appsService, nsIPrincipal::APP_STATUS_NOT_INSTALLED);
nsCOMPtr<mozIApplication> app;
@@ -286,17 +298,17 @@ nsScriptSecurityManager::AppStatusForPri
NS_ENSURE_SUCCESS(app->GetOrigin(appOrigin),
nsIPrincipal::APP_STATUS_NOT_INSTALLED);
nsCOMPtr<nsIURI> appURI;
NS_ENSURE_SUCCESS(NS_NewURI(getter_AddRefs(appURI), appOrigin),
nsIPrincipal::APP_STATUS_NOT_INSTALLED);
// 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);
return aPrin->Equals(appPrin) ? status
: nsIPrincipal::APP_STATUS_NOT_INSTALLED;
}
/*
--- a/dom/apps/AppsService.js
+++ b/dom/apps/AppsService.js
@@ -101,16 +101,20 @@ AppsService.prototype = {
return DOMApplicationRegistry.getCoreAppsBasePath();
},
getWebAppsBasePath: function getWebAppsBasePath() {
debug("getWebAppsBasePath()");
return DOMApplicationRegistry.getWebAppsBasePath();
},
+ areAnyAppsInstalled: function() {
+ return DOMApplicationRegistry.areAnyAppsInstalled();
+ },
+
getAppInfo: function getAppInfo(aAppId) {
debug("getAppInfo()");
return DOMApplicationRegistry.getAppInfo(aAppId);
},
getRedirect: function getRedirect(aLocalId, aURI) {
debug("getRedirect for " + aLocalId + " " + aURI.spec);
if (this.isInvalidId(aLocalId)) {
--- a/dom/apps/AppsServiceChild.jsm
+++ b/dom/apps/AppsServiceChild.jsm
@@ -407,16 +407,20 @@ this.DOMApplicationRegistry = {
return null;
},
getWebAppsBasePath: function getWebAppsBasePath() {
debug("getWebAppsBasePath() not yet supported on child!");
return null;
},
+ areAnyAppsInstalled: function() {
+ return AppsUtils.areAnyAppsInstalled(this.webapps);
+ },
+
getAppInfo: function getAppInfo(aAppId) {
return AppsUtils.getAppInfo(this.webapps, aAppId);
},
updateDataStoreEntriesFromLocalId: function(aLocalId) {
debug("updateDataStoreEntriesFromLocalId() not yet supported on child!");
}
}
--- a/dom/apps/AppsUtils.jsm
+++ b/dom/apps/AppsUtils.jsm
@@ -340,16 +340,23 @@ this.AppsUtils = {
if (app.localId == aLocalId) {
return app.manifestURL;
}
}
return "";
},
+ areAnyAppsInstalled: function(aApps) {
+ for (let id in aApps) {
+ return true;
+ }
+ return false;
+ },
+
getCoreAppsBasePath: function getCoreAppsBasePath() {
debug("getCoreAppsBasePath()");
try {
return FileUtils.getDir("coreAppsDir", ["webapps"], false).path;
} catch(e) {
return null;
}
},
--- a/dom/apps/Webapps.jsm
+++ b/dom/apps/Webapps.jsm
@@ -4800,16 +4800,20 @@ this.DOMApplicationRegistry = {
getCoreAppsBasePath: function() {
return AppsUtils.getCoreAppsBasePath();
},
getWebAppsBasePath: function() {
return OS.Path.dirname(this.appsFile);
},
+ areAnyAppsInstalled: function() {
+ return AppsUtils.areAnyAppsInstalled(this.webapps);
+ },
+
updateDataStoreEntriesFromLocalId: function(aLocalId) {
let app = appsService.getAppByLocalId(aLocalId);
if (app) {
this.updateDataStoreForApp(app.id);
}
},
_isLaunchable: function(aApp) {
--- a/dom/base/nsFrameLoader.cpp
+++ b/dom/base/nsFrameLoader.cpp
@@ -1619,17 +1619,60 @@ nsFrameLoader::OwnerIsMozBrowserFrame()
bool
nsFrameLoader::OwnerIsIsolatedMozBrowserFrame()
{
nsCOMPtr<nsIMozBrowserFrame> browserFrame = do_QueryInterface(mOwnerContent);
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) {
+ return false;
+ }
+ bool appsInstalled;
+ nsresult rv = appsService->AreAnyAppsInstalled(&appsInstalled);
+ if (NS_WARN_IF(NS_FAILED(rv))) {
+ return true;
+ }
+#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
nsFrameLoader::GetOwnerAppManifestURL(nsAString& aOut)
{
aOut.Truncate();
nsCOMPtr<nsIMozBrowserFrame> browserFrame = do_QueryInterface(mOwnerContent);
if (browserFrame) {
--- a/dom/interfaces/apps/nsIAppsService.idl
+++ b/dom/interfaces/apps/nsIAppsService.idl
@@ -61,16 +61,21 @@ interface nsIAppsService : nsISupports
*/
DOMString getCoreAppsBasePath();
/**
* Returns the basepath for regular packaged apps
*/
DOMString getWebAppsBasePath();
+ /**
+ * Returns true if at least one app is in the registry.
+ */
+ boolean areAnyAppsInstalled();
+
jsval getAppInfo(in DOMString appId);
/**
* Returns a URI to redirect to when we get a redirection to 'uri'.
* Returns null if no redirection is declared for this uri.
*/
nsIURI getRedirect(in unsigned long localId, in nsIURI uri);
--- a/dom/ipc/AppProcessChecker.cpp
+++ b/dom/ipc/AppProcessChecker.cpp
@@ -244,26 +244,26 @@ AssertAppPrincipal(PContentParent* aActo
{
if (!aPrincipal) {
NS_WARNING("Principal is invalid, killing app process");
static_cast<ContentParent*>(aActor)->KillHard("AssertAppPrincipal");
return false;
}
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;
}
}
NS_WARNING("Principal is invalid, killing app process");
static_cast<ContentParent*>(aActor)->KillHard("AssertAppPrincipal");
@@ -314,18 +314,27 @@ CheckPermission(PContentParent* aActor,
NS_ENSURE_SUCCESS(rv, nsIPermissionManager::UNKNOWN_ACTION);
if (permission == nsIPermissionManager::UNKNOWN_ACTION ||
permission == nsIPermissionManager::DENY_ACTION) {
return permission;
}
// 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;
}
// Setting to "prompt" in the settings UI should prompt everywhere in
// non-browser content.
if (appPerm == nsIPermissionManager::PROMPT_ACTION ||
permission == nsIPermissionManager::PROMPT_ACTION) {
return nsIPermissionManager::PROMPT_ACTION;
--- a/dom/messages/SystemMessageManager.js
+++ b/dom/messages/SystemMessageManager.js
@@ -327,16 +327,26 @@ SystemMessageManager.prototype = {
// nsIDOMGlobalPropertyInitializer implementation.
init: function(aWindow) {
debug("init");
this.initDOMRequestHelper(aWindow,
["SystemMessageManager:Message",
"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;
let appsService = Cc["@mozilla.org/AppsService;1"]
.getService(Ci.nsIAppsService);
this._manifestURL = appsService.getManifestURLByLocalId(principal.appId);
// Two cases are valid to register the manifest URL for the current process: