Bug 1238160 - Add assertions in non-desktop code paths. r=bz,fabrice draft
authorJ. Ryan Stinnett <jryans@gmail.com>
Wed, 17 Feb 2016 22:57:41 -0600
changeset 332007 81ac24e5839336d7975d9adb7c85a45a2bff84ed
parent 331763 ea89fa67bd580d4bf1c2a3fe358fe089846767de
child 332008 5cb3a81693ef0b410852d87449e72ed58e72565b
push id11136
push userbmo:jryans@gmail.com
push dateFri, 19 Feb 2016 00:39:07 +0000
reviewersbz, fabrice
bugs1238160
milestone47.0a1
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
caps/nsScriptSecurityManager.cpp
dom/apps/AppsService.js
dom/apps/AppsServiceChild.jsm
dom/apps/AppsUtils.jsm
dom/apps/Webapps.jsm
dom/base/nsFrameLoader.cpp
dom/interfaces/apps/nsIAppsService.idl
dom/ipc/AppProcessChecker.cpp
dom/messages/SystemMessageManager.js
--- 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: