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 241999 cbe91037c39da7edff2d83093c538634ca05e52e
parent 241998 cb98d773829d0259bc4952d18b041a1a2c60ab95
child 242000 ca3c11c3b0bff67b226e126fc48b9412cf88bdc8
push id28673
push userkwierso@gmail.com
push dateSat, 02 May 2015 00:14:13 +0000
treeherdermozilla-central@47ea4e2d91ed [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdveditz
bugs1160340
milestone40.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 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);