Bug 1238160 - Add assertions in non-desktop code paths. r=bz,fabrice
authorJ. Ryan Stinnett <jryans@gmail.com>
Wed, 17 Feb 2016 22:57:41 -0600
changeset 322812 80f7b29a27f17f06c02e7b11b2507857332d56c3
parent 322811 76944f0d24abc3e3d968f324f18a8a3835e535dc
child 322813 14eddac51e70e35f839a5ffd6e7ec15f0d9fc032
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, fabrice
bugs1238160
milestone47.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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,20 @@ this.AppsUtils = {
       if (app.localId == aLocalId) {
         return app.manifestURL;
       }
     }
 
     return "";
   },
 
+  areAnyAppsInstalled: function(aApps) {
+    return Object.getOwnPropertyNames(aApps).length > 0;
+  },
+
   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
@@ -4794,16 +4794,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
@@ -1622,17 +1622,64 @@ 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) {
+    // 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
 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
@@ -86,17 +86,17 @@ AssertAppProcess(PBrowserParent* aActor,
   if (!aActor) {
     NS_WARNING("Testing process capability for null actor");
     return false;
   }
 
   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
 CheckAppStatusHelper(mozIApplication* aApp,
                      unsigned short aStatus)
 {
   bool valid = false;
 
@@ -168,17 +168,17 @@ AssertAppProcess(TabContext& aContext,
   // Do a origin-based permission check if the TabContext owns a signed package.
   if (!aContext.SignedPkgOriginNoSuffix().IsEmpty() &&
       (ASSERT_APP_HAS_PERMISSION == aType || ASSERT_APP_PROCESS_PERMISSION == aType)) {
     nsCString origin = aContext.SignedPkgOriginNoSuffix() + suffix;
     return CheckOriginPermission(origin, aCapability);
   }
 
   nsCOMPtr<mozIApplication> app = aContext.GetOwnOrContainingApp();
-  return CheckAppTypeHelper(app, aType, aCapability, aContext.IsBrowserElement());
+  return CheckAppTypeHelper(app, aType, aCapability, aContext.IsMozBrowserElement());
 }
 
 bool
 AssertAppStatus(TabContext& aContext,
                 unsigned short aStatus)
 {
 
   nsCOMPtr<mozIApplication> app = aContext.GetOwnOrContainingApp();
@@ -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: