Bug 1294483: Centralise where we check for correct signing state and make the checks more obvious. r=aswan, a=sylvestre
authorDave Townsend <dtownsend@oxymoronical.com>
Thu, 11 Aug 2016 11:45:06 -0700
changeset 407639 e33c4c0efaaf4df186fb5e11f24cc9d7f5df2ba8
parent 407638 0253fd92571ac6842143496156ad5c8a8fd1c89a
child 407640 a86f28044dd4e943bae3961442d7abdb7c3a3880
push id28002
push userfelipc@gmail.com
push dateTue, 30 Aug 2016 18:14:28 +0000
reviewersaswan, sylvestre
bugs1294483
milestone48.0.1
Bug 1294483: Centralise where we check for correct signing state and make the checks more obvious. r=aswan, a=sylvestre MozReview-Commit-ID: JN5KMu1zjIm
toolkit/mozapps/extensions/content/extensions.js
toolkit/mozapps/extensions/internal/XPIProvider.jsm
toolkit/mozapps/extensions/test/browser/head.js
--- a/toolkit/mozapps/extensions/content/extensions.js
+++ b/toolkit/mozapps/extensions/content/extensions.js
@@ -195,28 +195,19 @@ function loadView(aViewId) {
 
     gViewController.loadInitialView(aViewId);
   } else {
     gViewController.loadView(aViewId);
   }
 }
 
 function isCorrectlySigned(aAddon) {
-  // Temporary add-ons do not require signing.
-  if (aAddon.scope == AddonManager.SCOPE_TEMPORARY)
-      return true;
-  // On UNIX platforms except OSX, an additional location for system add-ons
-  // exists in /usr/{lib,share}/mozilla/extensions. Add-ons installed there
-  // do not require signing either.
-  if (aAddon.scope == AddonManager.SCOPE_SYSTEM &&
-      Services.appinfo.OS != "Darwin")
-    return true;
-  if (aAddon.signedState <= AddonManager.SIGNEDSTATE_MISSING)
-    return false;
-  return true;
+  // Add-ons without an "isCorrectlySigned" property are correctly signed as
+  // they aren't the correct type for signing.
+  return aAddon.isCorrectlySigned !== false;
 }
 
 function isDiscoverEnabled() {
   if (Services.prefs.getPrefType(PREF_DISCOVERURL) == Services.prefs.PREF_INVALID)
     return false;
 
   try {
     if (!Services.prefs.getBoolPref(PREF_DISCOVER_ENABLED))
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -686,32 +686,20 @@ function canRunInSafeMode(aAddon) {
  *         The add-on to check
  * @return true if the add-on should not be appDisabled
  */
 function isUsableAddon(aAddon) {
   // Hack to ensure the default theme is always usable
   if (aAddon.type == "theme" && aAddon.internalName == XPIProvider.defaultSkin)
     return true;
 
-  if (aAddon._installLocation.name == KEY_APP_SYSTEM_ADDONS &&
-      aAddon.signedState != AddonManager.SIGNEDSTATE_SYSTEM) {
+  if (mustSign(aAddon.type) && !aAddon.isCorrectlySigned) {
+    logger.warn(`Add-on ${aAddon.id} is not correctly signed.`);
     return false;
   }
-  // Temporary and system add-ons do not require signing.
-  // On UNIX platforms except OSX, an additional location for system add-ons
-  // exists in /usr/{lib,share}/mozilla/extensions. Add-ons installed there
-  // do not require signing either.
-  if (((aAddon._installLocation.scope != AddonManager.SCOPE_SYSTEM ||
-        Services.appinfo.OS == "Darwin") &&
-       aAddon._installLocation.name != KEY_APP_SYSTEM_DEFAULTS &&
-       aAddon._installLocation.name != KEY_APP_TEMPORARY) &&
-       mustSign(aAddon.type)) {
-    if (aAddon.signedState <= AddonManager.SIGNEDSTATE_MISSING)
-      return false;
-  }
 
   if (aAddon.blocklistState == Blocklist.STATE_BLOCKED)
     return false;
 
   // Experiments are installed through an external mechanism that
   // limits target audience to compatible clients. We trust it knows what
   // it's doing and skip compatibility checks.
   //
@@ -6705,16 +6693,42 @@ AddonInternal.prototype = {
     return this._selectedLocale;
   },
 
   get providesUpdatesSecurely() {
     return !!(this.updateKey || !this.updateURL ||
               this.updateURL.substring(0, 6) == "https:");
   },
 
+  get isCorrectlySigned() {
+    switch (this._installLocation.name) {
+      case KEY_APP_SYSTEM_ADDONS:
+        // System add-ons must be signed by the system key.
+        return this.signedState == AddonManager.SIGNEDSTATE_SYSTEM
+
+      case KEY_APP_SYSTEM_DEFAULTS:
+      case KEY_APP_TEMPORARY:
+        // Temporary and built-in system add-ons do not require signing.
+        return true;
+
+      case KEY_APP_SYSTEM_SHARE:
+      case KEY_APP_SYSTEM_LOCAL:
+        // On UNIX platforms except OSX, an additional location for system
+        // add-ons exists in /usr/{lib,share}/mozilla/extensions. Add-ons
+        // installed there do not require signing.
+        if (Services.appinfo.OS != "Darwin")
+          return true;
+        break;
+    }
+
+    if (this.signedState === AddonManager.SIGNEDSTATE_NOT_REQUIRED)
+      return true;
+    return this.signedState > AddonManager.SIGNEDSTATE_MISSING;
+  },
+
   get isCompatible() {
     return this.isCompatibleWith();
   },
 
   get disabled() {
     return (this.userDisabled || this.appDisabled || this.softDisabled);
   },
 
@@ -7454,17 +7468,18 @@ function defineAddonWrapperProperty(name
     enumerable: true,
   });
 }
 
 ["id", "syncGUID", "version", "isCompatible", "isPlatformCompatible",
  "providesUpdatesSecurely", "blocklistState", "blocklistURL", "appDisabled",
  "softDisabled", "skinnable", "size", "foreignInstall", "hasBinaryComponents",
  "strictCompatibility", "compatibilityOverrides", "updateURL",
- "getDataDirectory", "multiprocessCompatible", "signedState"].forEach(function(aProp) {
+ "getDataDirectory", "multiprocessCompatible", "signedState",
+ "isCorrectlySigned"].forEach(function(aProp) {
    defineAddonWrapperProperty(aProp, function() {
      let addon = addonFor(this);
      return (aProp in addon) ? addon[aProp] : undefined;
    });
 });
 
 ["fullDescription", "developerComments", "eula", "supportURL",
  "contributionURL", "contributionAmount", "averageRating", "reviewCount",
--- a/toolkit/mozapps/extensions/test/browser/head.js
+++ b/toolkit/mozapps/extensions/test/browser/head.js
@@ -1148,16 +1148,22 @@ function MockAddon(aId, aName, aType, aO
     aOperationsRequiringRestart :
     (AddonManager.OP_NEEDS_RESTART_INSTALL |
      AddonManager.OP_NEEDS_RESTART_UNINSTALL |
      AddonManager.OP_NEEDS_RESTART_ENABLE |
      AddonManager.OP_NEEDS_RESTART_DISABLE);
 }
 
 MockAddon.prototype = {
+  get isCorrectlySigned() {
+    if (this.signedState === AddonManager.SIGNEDSTATE_NOT_REQUIRED)
+      return true;
+    return this.signedState > AddonManager.SIGNEDSTATE_MISSING;
+  },
+
   get shouldBeActive() {
     return !this.appDisabled && !this._userDisabled &&
            !(this.pendingOperations & AddonManager.PENDING_UNINSTALL);
   },
 
   get appDisabled() {
     return this._appDisabled;
   },