Bug 1192930 - Require a special certificate for system add-ons. r=rhelmer

Makes sure that add-on objects always have the _installLocation property for
the location they will be installed into so that isUsableAddon can test for the
right signature.

toolkit/mozapps/extensions/AddonManager.jsm

@@ -2998,6 +2998,8 @@ this.AddonManager = {
   SIGNEDSTATE_PRELIMINARY: 1,
   // Add-on is fully reviewed.
   SIGNEDSTATE_SIGNED: 2,
+  // Add-on is system add-on.
+  SIGNEDSTATE_SYSTEM: 3,
 
   // Constants for the Addon.userDisabled property
   // Indicates that the userDisabled state of this add-on is currently

toolkit/mozapps/extensions/internal/XPIProvider.jsm

@@ -617,6 +617,12 @@ function isUsableAddon(aAddon) {
       return false;
     if (aAddon.foreignInstall && aAddon.signedState < AddonManager.SIGNEDSTATE_SIGNED)
       return false;
+
+    if (aAddon._installLocation.name == KEY_APP_SYSTEM_ADDONS ||
+        aAddon._installLocation.name == KEY_APP_SYSTEM_DEFAULTS) {
+      if (aAddon.signedState != AddonManager.SIGNEDSTATE_SYSTEM)
+        return false;
+    }
   }
 
   if (aAddon.blocklistState == Blocklist.STATE_BLOCKED)
@@ -1155,7 +1161,7 @@ function defineSyncGUID(aAddon) {
  * @return an AddonInternal object
  * @throws if the directory does not contain a valid install manifest
  */
-let loadManifestFromDir = Task.async(function* loadManifestFromDir(aDir) {
+let loadManifestFromDir = Task.async(function* loadManifestFromDir(aDir, aInstallLocation) {
   function getFileSize(aFile) {
     if (aFile.isSymlink())
       return 0;
@@ -1202,6 +1208,7 @@ let loadManifestFromDir = Task.async(function* loadManifestFromDir(aDir) {
                 loadFromRDF(file, bis);
 
     addon._sourceBundle = aDir.clone();
+    addon._installLocation = aInstallLocation;
     addon.size = getFileSize(aDir);
     addon.signedState = yield verifyDirSignedState(aDir, addon);
     addon.appDisabled = !isUsableAddon(addon);
@@ -1224,7 +1231,7 @@ let loadManifestFromDir = Task.async(function* loadManifestFromDir(aDir) {
  * @return an AddonInternal object
  * @throws if the XPI file does not contain a valid install manifest
  */
-let loadManifestFromZipReader = Task.async(function* loadManifestFromZipReader(aZipReader) {
+let loadManifestFromZipReader = Task.async(function* loadManifestFromZipReader(aZipReader, aInstallLocation) {
   function loadFromRDF(aStream) {
     let uri = buildJarURI(aZipReader.file, FILE_RDF_MANIFEST);
     let addon = loadManifestFromRDF(uri, aStream);
@@ -1259,6 +1266,7 @@ let loadManifestFromZipReader = Task.async(function* loadManifestFromZipReader(a
                 loadFromRDF(bis);
 
     addon._sourceBundle = aZipReader.file;
+    addon._installLocation = aInstallLocation;
 
     addon.size = 0;
     let entries = aZipReader.findEntries(null);
@@ -1286,7 +1294,7 @@ let loadManifestFromZipReader = Task.async(function* loadManifestFromZipReader(a
  * @return an AddonInternal object
  * @throws if the XPI file does not contain a valid install manifest
  */
-let loadManifestFromZipFile = Task.async(function* loadManifestFromZipFile(aXPIFile) {
+let loadManifestFromZipFile = Task.async(function* loadManifestFromZipFile(aXPIFile, aInstallLocation) {
   let zipReader = Cc["@mozilla.org/libjar/zip-reader;1"].
                   createInstance(Ci.nsIZipReader);
   try {
@@ -1295,7 +1303,7 @@ let loadManifestFromZipFile = Task.async(function* loadManifestFromZipFile(aXPIF
     // Can't return this promise because that will make us close the zip reader
     // before it has finished loading the manifest. Wait for the result and then
     // return.
-    let manifest = yield loadManifestFromZipReader(zipReader);
+    let manifest = yield loadManifestFromZipReader(zipReader, aInstallLocation);
     return manifest;
   }
   finally {
@@ -1303,22 +1311,22 @@ let loadManifestFromZipFile = Task.async(function* loadManifestFromZipFile(aXPIF
   }
 });
 
-function loadManifestFromFile(aFile) {
+function loadManifestFromFile(aFile, aInstallLocation) {
   if (aFile.isFile())
-    return loadManifestFromZipFile(aFile);
+    return loadManifestFromZipFile(aFile, aInstallLocation);
   else
-    return loadManifestFromDir(aFile);
+    return loadManifestFromDir(aFile, aInstallLocation);
 }
 
 /**
  * A synchronous method for loading an add-on's manifest. This should only ever
  * be used during startup or a sync load of the add-ons DB
  */
-function syncLoadManifestFromFile(aFile) {
+function syncLoadManifestFromFile(aFile, aInstallLocation) {
   let success = undefined;
   let result = null;
 
-  loadManifestFromFile(aFile).then(val => {
+  loadManifestFromFile(aFile, aInstallLocation).then(val => {
     success = true;
     result = val;
   }, val => {
@@ -1466,6 +1474,9 @@ function getSignedStatus(aRv, aCert, aExpectedID) {
         }
       }
 
+      if (aCert.organizationalUnit == "Mozilla Components")
+        return AddonManager.SIGNEDSTATE_SYSTEM;
+
       return /preliminary/i.test(aCert.organizationalUnit)
                ? AddonManager.SIGNEDSTATE_PRELIMINARY
                : AddonManager.SIGNEDSTATE_SIGNED;
@@ -2922,7 +2933,7 @@ this.XPIProvider = {
         let addon;
 
         try {
-          addon = syncLoadManifestFromFile(stageDirEntry);
+          addon = syncLoadManifestFromFile(stageDirEntry, aLocation);
         }
         catch (e) {
           logger.error("Unable to read add-on manifest from " + stageDirEntry.path, e);
@@ -3096,7 +3107,7 @@ this.XPIProvider = {
 
       let addon;
       try {
-        addon = syncLoadManifestFromFile(entry);
+        addon = syncLoadManifestFromFile(entry, profileLocation);
       }
       catch (e) {
         logger.warn("File entry " + entry.path + " contains an invalid add-on", e);
@@ -3119,7 +3130,7 @@ this.XPIProvider = {
       if (existingEntry) {
         let existingAddon;
         try {
-          existingAddon = syncLoadManifestFromFile(existingEntry);
+          existingAddon = syncLoadManifestFromFile(existingEntry, profileLocation);
 
           if (Services.vc.compare(addon.version, existingAddon.version) <= 0)
             continue;
@@ -4958,7 +4969,7 @@ AddonInstall.prototype = {
 
     try {
       // loadManifestFromZipReader performs the certificate verification for us
-      this.addon = yield loadManifestFromZipReader(zipreader);
+      this.addon = yield loadManifestFromZipReader(zipreader, this.installLocation);
     }
     catch (e) {
       zipreader.close();
@@ -5506,7 +5517,6 @@ AddonInstall.prototype = {
 
         // Update the metadata in the database
         this.addon._sourceBundle = file;
-        this.addon._installLocation = this.installLocation;
         this.addon.visible = true;
 
         if (isUpgrade) {

toolkit/mozapps/extensions/internal/XPIProviderUtils.js

@@ -1604,7 +1604,7 @@ this.XPIDatabaseReconcile = {
         // Load the manifest from the add-on.
         let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
         file.persistentDescriptor = aAddonState.descriptor;
-        aNewAddon = syncLoadManifestFromFile(file);
+        aNewAddon = syncLoadManifestFromFile(file, aInstallLocation);
       }
       // The add-on in the manifest should match the add-on ID.
       if (aNewAddon.id != aId) {
@@ -1625,7 +1625,6 @@ this.XPIDatabaseReconcile = {
     }
 
     // Update the AddonInternal properties.
-    aNewAddon._installLocation = aInstallLocation;
     aNewAddon.installDate = aAddonState.mtime;
     aNewAddon.updateDate = aAddonState.mtime;
 
@@ -1728,7 +1727,7 @@ this.XPIDatabaseReconcile = {
       if (!aNewAddon) {
         let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
         file.persistentDescriptor = aAddonState.descriptor;
-        aNewAddon = syncLoadManifestFromFile(file);
+        aNewAddon = syncLoadManifestFromFile(file, aInstallLocation);
         applyBlocklistChanges(aOldAddon, aNewAddon);
 
         // Carry over any pendingUninstall state to add-ons modified directly
@@ -1756,7 +1755,6 @@ this.XPIDatabaseReconcile = {
     }
 
     // Set the additional properties on the new AddonInternal
-    aNewAddon._installLocation = aInstallLocation;
     aNewAddon.updateDate = aAddonState.mtime;
 
     // Update the database
@@ -1814,7 +1812,7 @@ this.XPIDatabaseReconcile = {
         SIGNED_TYPES.has(aOldAddon.type)) {
       let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
       file.persistentDescriptor = aAddonState.descriptor;
-      let manifest = syncLoadManifestFromFile(file);
+      let manifest = syncLoadManifestFromFile(file, aInstallLocation);
       aOldAddon.signedState = manifest.signedState;
     }
     // This updates the addon's JSON cached data in place

toolkit/mozapps/extensions/test/xpcshell/test_system_reset.js

@@ -2,6 +2,9 @@
 // application versions
 const PREF_SYSTEM_ADDON_SET = "extensions.systemAddonSet";
 
+// Enable signature checks for these tests
+Services.prefs.setBoolPref(PREF_XPI_SIGNATURES_REQUIRED, true);
+
 const featureDir = gProfD.clone();
 featureDir.append("features");
 
@@ -47,13 +50,21 @@ function* check_installed(inProfile, ...versions) {
       do_check_true(uri instanceof AM_Ci.nsIFileURL);
       do_check_eq(uri.file.path, file.path);
 
+      do_check_eq(addon.signedState, AddonManager.SIGNEDSTATE_SYSTEM);
+
       // Verify the add-on actually started
       let installed = Services.prefs.getCharPref("bootstraptest." + id + ".active_version");
       do_check_eq(installed, versions[i]);
     }
     else {
-      // Add-on should not be installed
-      do_check_eq(addon, null);
+      if (inProfile) {
+        // Add-on should not be installed
+        do_check_eq(addon, null);
+      }
+      else {
+        // Either add-on should not be installed or it shouldn't be active
+        do_check_true(!addon || !addon.isActive);
+      }
 
       try {
         Services.prefs.getCharPref("bootstraptest." + id + ".active_version");
@@ -131,7 +142,7 @@ add_task(function* test_updated() {
   // Inject it into the system set
   let addonSet = {
     schema: 1,
-    directory: dirname,
+    directory: featureDir.leafName,
     addons: {
       "system2@tests.mozilla.org": {
         version: "1.0"
@@ -198,3 +209,50 @@ add_task(function* test_corrupt_pref() {
 
   yield promiseShutdownManager();
 });
+
+// An add-on with a bad certificate should cause us to use the default set
+add_task(function* test_bad_profile_cert() {
+  let file = do_get_file("data/system_addons/app3/features/system1@tests.mozilla.org.xpi");
+  file.copyTo(featureDir, file.leafName);
+
+  // Inject it into the system set
+  let addonSet = {
+    schema: 1,
+    directory: featureDir.leafName,
+    addons: {
+      "system1@tests.mozilla.org": {
+        version: "2.0"
+      },
+      "system2@tests.mozilla.org": {
+        version: "1.0"
+      },
+      "system3@tests.mozilla.org": {
+        version: "1.0"
+      },
+    }
+  };
+  Services.prefs.setCharPref(PREF_SYSTEM_ADDON_SET, JSON.stringify(addonSet));
+
+  startupManager(false);
+
+  yield check_installed(false, "1.0", "1.0", null);
+
+  yield promiseShutdownManager();
+});
+
+// Switching to app defaults that contain a bad certificate should ignore the
+// bad add-on
+add_task(function* test_bad_app_cert() {
+  gAppInfo.version = "3";
+  distroDir.leafName = "app3";
+  startupManager();
+
+  // Add-on will still be present just not active
+  let addon = yield promiseAddonByID("system1@tests.mozilla.org");
+  do_check_neq(addon, null);
+  do_check_eq(addon.signedState, AddonManager.SIGNEDSTATE_SIGNED);
+
+  yield check_installed(false, null, null, "1.0");
+
+  yield promiseShutdownManager();
+});