Bug 1225629: Always verify signatures for hotfixes and system add-on updates. r=rhelmer a=lizzard
☠☠ backed out by 64f08f88a6ab ☠ ☠
authorDave Townsend <dtownsend@oxymoronical.com>
Tue, 17 Nov 2015 14:05:04 -0800
changeset 298491 ca8e41ac5c2b019c6ac9d4ebd200d67ad5af25f2
parent 298490 dddd1cb91dd6381fd3d1e582b5e589299483130f
child 298492 64f08f88a6abccee76ae35e175a8792156472e09
push id962
push userjlund@mozilla.com
push dateFri, 04 Dec 2015 23:28:54 +0000
treeherdermozilla-release@23a2d286e80f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrhelmer, lizzard
bugs1225629
milestone43.0
Bug 1225629: Always verify signatures for hotfixes and system add-on updates. r=rhelmer a=lizzard
toolkit/mozapps/extensions/internal/XPIProvider.jsm
toolkit/mozapps/extensions/test/xpcshell/test_system_reset.js
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -1519,28 +1519,47 @@ function getSignedStatus(aRv, aCert, aAd
     default:
       // Any other error indicates that either the add-on isn't signed or it
       // is signed by a signature that doesn't chain to the trusted root.
       return AddonManager.SIGNEDSTATE_UNKNOWN;
       break;
   }
 }
 
+function shouldVerifySignedState(aAddon) {
+  // Updated system add-ons should always have their signature checked
+  if (aAddon._installLocation.name == KEY_APP_SYSTEM_ADDONS)
+    return true;
+
+  // We don't care about signatures for default system add-ons
+  if (aAddon._installLocation.name == KEY_APP_SYSTEM_DEFAULTS)
+    return false;
+
+  // Hotfixes should always have their signature checked
+  let hotfixID = Preferences.get(PREF_EM_HOTFIX_ID, undefined);
+  if (hotfixID && aAddon.id == hotfixID)
+    return true;
+
+  // Otherwise only check signatures if signing is enabled and the add-on is one
+  // of the signed types.
+  return ADDON_SIGNING && SIGNED_TYPES.has(aAddon.type);
+}
+
 /**
  * Verifies that a zip file's contents are all correctly signed by an
  * AMO-issued certificate
  *
  * @param  aFile
  *         the xpi file to check
  * @param  aAddon
  *         the add-on object to verify
  * @return a Promise that resolves to an AddonManager.SIGNEDSTATE_* constant.
  */
 function verifyZipSignedState(aFile, aAddon) {
-  if (!ADDON_SIGNING || !SIGNED_TYPES.has(aAddon.type))
+  if (!shouldVerifySignedState(aAddon))
     return Promise.resolve(AddonManager.SIGNEDSTATE_NOT_REQUIRED);
 
   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;
@@ -1560,17 +1579,17 @@ function verifyZipSignedState(aFile, aAd
  *
  * @param  aDir
  *         the directory to check
  * @param  aAddon
  *         the add-on object to verify
  * @return a Promise that resolves to an AddonManager.SIGNEDSTATE_* constant.
  */
 function verifyDirSignedState(aDir, aAddon) {
-  if (!ADDON_SIGNING || !SIGNED_TYPES.has(aAddon.type))
+  if (!shouldVerifySignedState(aAddon))
     return Promise.resolve(AddonManager.SIGNEDSTATE_NOT_REQUIRED);
 
   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;
--- a/toolkit/mozapps/extensions/test/xpcshell/test_system_reset.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_system_reset.js
@@ -247,14 +247,14 @@ add_task(function* test_bad_profile_cert
 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);
+  do_check_eq(addon.signedState, AddonManager.SIGNEDSTATE_NOT_REQUIRED);
 
   yield check_installed(false, null, null, "1.0");
 
   yield promiseShutdownManager();
 });