Bug 1160340: Themes should not be marked as unsigned in the add-ons manager. r=dveditz
authorDave Townsend <dtownsend@oxymoronical.com>
Thu, 30 Apr 2015 17:51:18 -0700
changeset 241869 cbe91037c39da7edff2d83093c538634ca05e52e
parent 241868 cb98d773829d0259bc4952d18b041a1a2c60ab95
child 241870 ca3c11c3b0bff67b226e126fc48b9412cf88bdc8
push id12686
push userdtownsend@mozilla.com
push dateFri, 01 May 2015 15:44:32 +0000
treeherderfx-team@cbe91037c39d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdveditz
bugs1160340
milestone40.0a1
Bug 1160340: Themes should not be marked as unsigned in the add-ons manager. r=dveditz
toolkit/mozapps/extensions/internal/XPIProvider.jsm
toolkit/mozapps/extensions/test/xpcshell/test_theme.js
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -1055,20 +1055,17 @@ let loadManifestFromDir = Task.async(fun
     addon.size = getFileSize(aDir);
 
     file = aDir.clone();
     file.append("chrome.manifest");
     let chromeManifest = ChromeManifestParser.parseSync(Services.io.newFileURI(file));
     addon.hasBinaryComponents = ChromeManifestParser.hasType(chromeManifest,
                                                              "binary-component");
 
-    if (SIGNED_TYPES.has(addon.type))
-      addon.signedState = yield verifyDirSignedState(aDir, addon.id);
-    else
-      addon.signedState = AddonManager.SIGNEDSTATE_MISSING;
+    addon.signedState = yield verifyDirSignedState(aDir, addon);
 
     addon.appDisabled = !isUsableAddon(addon);
     return addon;
   }
   finally {
     bis.close();
     fis.close();
   }
@@ -1103,20 +1100,17 @@ let loadManifestFromZipReader = Task.asy
       uri = buildJarURI(aZipReader.file, "chrome.manifest");
       let chromeManifest = ChromeManifestParser.parseSync(uri);
       addon.hasBinaryComponents = ChromeManifestParser.hasType(chromeManifest,
                                                                "binary-component");
     } else {
       addon.hasBinaryComponents = false;
     }
 
-    if (SIGNED_TYPES.has(addon.type))
-      addon.signedState = yield verifyZipSignedState(aZipReader.file, addon.id, addon.version);
-    else
-      addon.signedState = AddonManager.SIGNEDSTATE_MISSING;
+    addon.signedState = yield verifyZipSignedState(aZipReader.file, addon);
 
     addon.appDisabled = !isUsableAddon(addon);
     return addon;
   }
   finally {
     bis.close();
     zis.close();
   }
@@ -1320,66 +1314,72 @@ function getSignedStatus(aRv, aCert, aEx
 }
 
 /**
  * Verifies that a zip file's contents are all correctly signed by an
  * AMO-issued certificate
  *
  * @param  aFile
  *         the xpi file to check
- * @param  aExpectedID
- *         the expected ID of the signature
+ * @param  aAddon
+ *         the add-on object to verify
  * @return a Promise that resolves to an AddonManager.SIGNEDSTATE_* constant.
  */
-function verifyZipSignedState(aFile, aExpectedID, aVersion) {
+function verifyZipSignedState(aFile, aAddon) {
+  if (!SIGNED_TYPES.has(aAddon.type))
+    return Promise.resolve(undefined);
+
   let certDB = Cc["@mozilla.org/security/x509certdb;1"]
                .getService(Ci.nsIX509CertDB);
 
   let root = Ci.nsIX509CertDB.AddonsPublicRoot;
   if (!REQUIRE_SIGNING && Preferences.get(PREF_XPI_SIGNATURES_DEV_ROOT, false))
     root = Ci.nsIX509CertDB.AddonsStageRoot;
 
   return new Promise(resolve => {
     certDB.openSignedAppFileAsync(root, aFile, (aRv, aZipReader, aCert) => {
       if (aZipReader)
         aZipReader.close();
-      resolve(getSignedStatus(aRv, aCert, aExpectedID));
+      resolve(getSignedStatus(aRv, aCert, aAddon.id));
     });
   });
 }
 
 /**
  * Verifies that a directory's contents are all correctly signed by an
  * AMO-issued certificate
  *
  * @param  aDir
  *         the directory to check
- * @param  aExpectedID
- *         the expected ID of the signature
+ * @param  aAddon
+ *         the add-on object to verify
  * @return a Promise that resolves to an AddonManager.SIGNEDSTATE_* constant.
  */
-function verifyDirSignedState(aDir, aExpectedID) {
+function verifyDirSignedState(aDir, aAddon) {
+  if (!SIGNED_TYPES.has(aAddon.type))
+    return Promise.resolve(undefined);
+
   // TODO: Get the certificate for an unpacked add-on (bug 1038072)
   return Promise.resolve(AddonManager.SIGNEDSTATE_MISSING);
 }
 
 /**
  * Verifies that a bundle's contents are all correctly signed by an
  * AMO-issued certificate
  *
  * @param  aBundle
  *         the nsIFile for the bundle to check, either a directory or zip file
- * @param  aExpectedID
- *         the expected ID of the signature
+ * @param  aAddon
+ *         the add-on object to verify
  * @return a Promise that resolves to an AddonManager.SIGNEDSTATE_* constant.
  */
-function verifyBundleSignedState(aBundle, aExpectedID) {
+function verifyBundleSignedState(aBundle, aAddon) {
   if (aBundle.isFile())
-    return verifyZipSignedState(aBundle, aExpectedID);
-  return verifyDirSignedState(aBundle, aExpectedID);
+    return verifyZipSignedState(aBundle, aAddon);
+  return verifyDirSignedState(aBundle, aAddon);
 }
 
 /**
  * Replaces %...% strings in an addon url (update and updateInfo) with
  * appropriate values.
  *
  * @param  aAddon
  *         The AddonInternal representing the add-on
@@ -2517,29 +2517,29 @@ this.XPIProvider = {
     Services.prefs.setBoolPref(PREF_PENDING_OPERATIONS,
                                !XPIDatabase.writeAddonsList());
   },
 
   /**
    * Verifies that all installed add-ons are still correctly signed.
    */
   verifySignatures: function XPI_verifySignatures() {
-    XPIDatabase.getAddonList(addon => SIGNED_TYPES.has(addon.type), (addons) => {
+    XPIDatabase.getAddonList(a => true, (addons) => {
       Task.spawn(function*() {
         let changes = {
           enabled: [],
           disabled: []
         };
 
         for (let addon of addons) {
           // The add-on might have vanished, we'll catch that on the next startup
           if (!addon._sourceBundle.exists())
             continue;
 
-          let signedState = yield verifyBundleSignedState(addon._sourceBundle, addon.id);
+          let signedState = yield verifyBundleSignedState(addon._sourceBundle, addon);
           if (signedState == addon.signedState)
             continue;
 
           addon.signedState = signedState;
           AddonManagerPrivate.callAddonListeners("onPropertyChanged",
                                                  createWrapper(addon),
                                                  ["signedState"]);
 
@@ -3163,17 +3163,17 @@ this.XPIProvider = {
         let wasDisabled = aOldAddon.disabled;
         let wasAppDisabled = aOldAddon.appDisabled;
         let wasUserDisabled = aOldAddon.userDisabled;
         let wasSoftDisabled = aOldAddon.softDisabled;
         let updateDB = false;
 
         // If updating from a version of the app that didn't support signedState
         // then fetch that property now
-        if (aOldAddon.signedState === undefined) {
+        if (aOldAddon.signedState === undefined && SIGNED_TYPES.has(aOldAddon.type)) {
           let file = aInstallLocation.getLocationForID(aOldAddon.id);
           let manifest = syncLoadManifestFromFile(file);
           aOldAddon.signedState = manifest.signedState;
           updateDB = true;
         }
         // This updates the addon's JSON cached data in place
         applyBlocklistChanges(aOldAddon, aOldAddon, aOldAppVersion,
                               aOldPlatformVersion);
@@ -5413,17 +5413,18 @@ AddonInstall.prototype = {
         if (state == AddonManager.SIGNEDSTATE_MISSING)
           return Promise.reject([AddonManager.ERROR_SIGNEDSTATE_REQUIRED,
                                  "signature is required but missing"])
 
         return Promise.reject([AddonManager.ERROR_CORRUPT_FILE,
                                "signature verification failed"])
       }
     }
-    else if (this.addon.signedState == AddonManager.SIGNEDSTATE_UNKNOWN) {
+    else if (this.addon.signedState == AddonManager.SIGNEDSTATE_UNKNOWN ||
+             this.addon.signedState == undefined) {
       // Check object signing certificate, if any
       let x509 = zipreader.getSigningCert(null);
       if (x509) {
         logger.debug("Verifying XPI signature");
         if (verifyZipSigning(zipreader, x509)) {
           this.certificate = x509;
           if (this.certificate.commonName.length > 0) {
             this.certName = this.certificate.commonName;
--- a/toolkit/mozapps/extensions/test/xpcshell/test_theme.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_theme.js
@@ -90,33 +90,36 @@ function run_test() {
 
   AddonManager.getAddonsByIDs(["default@tests.mozilla.org",
                                "theme1@tests.mozilla.org",
                                "theme2@tests.mozilla.org"],
                                function([d, t1, t2]) {
     do_check_neq(d, null);
     do_check_false(d.skinnable);
     do_check_false(d.foreignInstall);
+    do_check_eq(d.signedState, undefined);
 
     do_check_neq(t1, null);
     do_check_false(t1.userDisabled);
     do_check_false(t1.appDisabled);
+    do_check_eq(t1.signedState, undefined);
     do_check_true(t1.isActive);
     do_check_true(t1.skinnable);
     do_check_true(t1.foreignInstall);
     do_check_eq(t1.screenshots, null);
     do_check_true(isThemeInAddonsList(profileDir, t1.id));
     do_check_false(hasFlag(t1.permissions, AddonManager.PERM_CAN_DISABLE));
     do_check_false(hasFlag(t1.permissions, AddonManager.PERM_CAN_ENABLE));
     do_check_eq(t1.operationsRequiringRestart, AddonManager.OP_NEEDS_RESTART_UNINSTALL |
                                                AddonManager.OP_NEEDS_RESTART_DISABLE);
 
     do_check_neq(t2, null);
     do_check_true(t2.userDisabled);
     do_check_false(t2.appDisabled);
+    do_check_eq(t2.signedState, undefined);
     do_check_false(t2.isActive);
     do_check_false(t2.skinnable);
     do_check_true(t2.foreignInstall);
     do_check_eq(t2.screenshots, null);
     do_check_false(isThemeInAddonsList(profileDir, t2.id));
     do_check_false(hasFlag(t2.permissions, AddonManager.PERM_CAN_DISABLE));
     do_check_true(hasFlag(t2.permissions, AddonManager.PERM_CAN_ENABLE));
     do_check_eq(t2.operationsRequiringRestart, AddonManager.OP_NEEDS_RESTART_ENABLE);