Bug 1801211 - Add salt to site origin before hashing to compute SitePermsAddon ids. r=rpl.

We expose a function generating the salt so we can add a test case
in the xpcshell test.

Differential Revision: https://phabricator.services.mozilla.com/D162366

toolkit/mozapps/extensions/internal/SitePermsAddonProvider.sys.mjs

@@ -37,6 +37,33 @@ XPCOMUtils.defineLazyPreferenceGetter(
 const FIRST_CONTENT_PROCESS_TOPIC = "ipc:first-content-process-created";
 const SITEPERMS_ADDON_ID_SUFFIX = "@siteperms.mozilla.org";
 
+// Generate a per-session random salt, which is then used to generate
+// per-siteOrigin hashed strings used as the addon id in SitePermsAddonWrapper constructor
+// (expected to be matching new addon id generated for the same siteOrigin during
+// the same browsing session and different ones in new browsing sessions).
+//
+// NOTE: `generateSalt` is exported for testing purpose, should not be
+// used outside of tests.
+let SALT;
+export function generateSalt() {
+  //TODO: Use Services.env (See Bug 1541508).
+  let env = Cc["@mozilla.org/process/environment;1"].getService(
+    Ci.nsIEnvironment
+  );
+  // Throw if we're not in test and SALT is already defined
+  if (typeof SALT !== "undefined" && !env.exists("XPCSHELL_TEST_PROFILE_DIR")) {
+    throw new Error("This should only be called from XPCShell tests");
+  }
+  SALT = crypto.getRandomValues(new Uint8Array(12)).join("");
+}
+
+function getSalt() {
+  if (!SALT) {
+    generateSalt();
+  }
+  return SALT;
+}
+
 class SitePermsAddonWrapper {
   // An array of nsIPermission granted for the siteOrigin.
   // We can't use a Set as handlePermissionChange might be called with different
@@ -61,8 +88,10 @@ class SitePermsAddonWrapper {
     this.principal = Services.scriptSecurityManager.createContentPrincipalFromOrigin(
       this.siteOrigin
     );
+    // Use a template string for the concat in case `siteOrigin` isn't a string.
+    const saltedValue = `${this.siteOrigin}${getSalt()}`;
     this.id = `${computeSha256HashAsString(
-      this.siteOrigin
+      saltedValue
     )}${SITEPERMS_ADDON_ID_SUFFIX}`;
 
     for (const perm of permissions) {

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

@@ -825,5 +825,87 @@ add_task(
 
     addons = await promiseAddonsByTypes([SITEPERMS_ADDON_TYPE]);
     Assert.equal(addons.length, 1, "...and addon is uninstalled");
+
+    await addons[0]?.uninstall();
+  }
+);
+
+add_task(
+  {
+    pref_set: [[SITEPERMS_ADDON_PROVIDER_PREF, true]],
+  },
+  async function test_salted_hash_addon_id() {
+    // Make sure the test will also be able to run if it is the only one executed.
+    Services.obs.notifyObservers(null, "ipc:first-content-process-created");
+    ok(
+      AddonManager.hasProvider("SitePermsAddonProvider"),
+      "Expect SitePermsAddonProvider to be registered"
+    );
+    // Make sure no sitepermission addon is already installed.
+    let addons = await promiseAddonsByTypes([SITEPERMS_ADDON_TYPE]);
+    Assert.equal(addons.length, 0, "There's no addons");
+
+    expectAndHandleInstallPrompts();
+    await AddonManager.installSitePermsAddonFromWebpage(
+      null,
+      PRINCIPAL_COM,
+      GATED_SITE_PERM1
+    );
+    addons = await promiseAddonsByTypes([SITEPERMS_ADDON_TYPE]);
+    Assert.equal(
+      addons.length,
+      1,
+      "installSitePermsAddonFromWebpage should add the addon..."
+    );
+    Assert.equal(
+      PermissionTestUtils.testExactPermission(PRINCIPAL_COM, GATED_SITE_PERM1),
+      true,
+      "...and set the permission"
+    );
+
+    addons = await promiseAddonsByTypes([SITEPERMS_ADDON_TYPE]);
+    Assert.equal(addons.length, 1, "There is an addon installed");
+
+    const firstSaltedAddonId = addons[0].id;
+    ok(firstSaltedAddonId, "Got the first addon id");
+
+    info("Verify addon id after mocking new browsing session");
+
+    const { generateSalt } = ChromeUtils.importESModule(
+      "resource://gre/modules/addons/SitePermsAddonProvider.sys.mjs"
+    );
+    generateSalt();
+
+    await promiseRestartManager();
+
+    addons = await promiseAddonsByTypes([SITEPERMS_ADDON_TYPE]);
+    Assert.equal(addons.length, 1, "There is an addon installed");
+
+    const secondSaltedAddonId = addons[0].id;
+    ok(
+      secondSaltedAddonId,
+      "Got the second addon id after mocking new browsing session"
+    );
+
+    Assert.notEqual(
+      firstSaltedAddonId,
+      secondSaltedAddonId,
+      "The two addon ids are different"
+    );
+
+    // Confirm that new installs from the same siteOrigin will still
+    // belong to the existing addon entry while the salt isn't expected
+    // to have changed.
+    expectAndHandleInstallPrompts();
+    await AddonManager.installSitePermsAddonFromWebpage(
+      null,
+      PRINCIPAL_COM,
+      GATED_SITE_PERM1
+    );
+
+    addons = await promiseAddonsByTypes([SITEPERMS_ADDON_TYPE]);
+    Assert.equal(addons.length, 1, "There is still a single addon installed");
+
+    await addons[0]?.uninstall();
   }
 );