Bug 1897278 - Add methods to BackupService to enable / disable encryption. r=backup-reviewers,kpatenio

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

browser/base/content/test/static/browser_all_files_referenced.js

@@ -287,10 +287,6 @@ var allowlist = [
 
   // Referenced programmatically
   { file: "chrome://browser/content/backup/BackupManifest.1.schema.json" },
-
-  // These will be referenced once bug 1897278 lands.
-  { file: "resource://app/modules/backup/ArchiveEncryptionState.sys.mjs" },
-  { file: "resource://app/modules/backup/ArchiveUtils.sys.mjs" },
 ];
 
 if (AppConstants.NIGHTLY_BUILD) {

browser/components/backup/BackupService.sys.mjs

@@ -25,6 +25,8 @@ ChromeUtils.defineLazyGetter(lazy, "fxAccounts", () => {
 });
 
 ChromeUtils.defineESModuleGetters(lazy, {
+  ArchiveEncryptionState:
+    "resource:///modules/backup/ArchiveEncryptionState.sys.mjs",
   ClientID: "resource://gre/modules/ClientID.sys.mjs",
   JsonSchemaValidator:
     "resource://gre/modules/components-utils/JsonSchemaValidator.sys.mjs",
@@ -113,6 +115,7 @@ export class BackupService extends EventTarget {
     backupFilePath: "Documents", // TODO: make save location configurable (bug 1895943)
     backupInProgress: false,
     scheduledBackupsEnabled: lazy.scheduledBackupsPref,
+    encryptionEnabled: false,
   };
 
   /**
@@ -132,6 +135,23 @@ export class BackupService extends EventTarget {
    */
   #postRecoveryResolver;
 
+  /**
+   * The currently used ArchiveEncryptionState. Callers should use
+   * loadEncryptionState() instead, to ensure that any pre-serialized
+   * encryption state has been read in and deserialized.
+   *
+   * This member can be in 3 states:
+   *
+   * 1. undefined - no attempt has been made to load encryption state from
+   *    disk yet.
+   * 2. null - encryption is not enabled.
+   * 3. ArchiveEncryptionState - encryption is enabled.
+   *
+   * @see BackupService.loadEncryptionState();
+   * @type {ArchiveEncryptionState|null|undefined}
+   */
+  #encState = undefined;
+
   /**
    * The name of the folder within the profile folder where this service reads
    * and writes state to.
@@ -196,6 +216,16 @@ export class BackupService extends EventTarget {
     return "post-recovery.json";
   }
 
+  /**
+   * The name of the serialized ArchiveEncryptionState that is written to disk
+   * if encryption is enabled.
+   *
+   * @type {string}
+   */
+  static get ARCHIVE_ENCRYPTION_STATE_FILE() {
+    return "enc-state.json";
+  }
+
   /**
    * Returns the schema for the backup manifest for a given version.
    *
@@ -264,7 +294,8 @@ export class BackupService extends EventTarget {
   /**
    * Create a BackupService instance.
    *
-   * @param {object} [backupResources=DefaultBackupResources] - Object containing BackupResource classes to associate with this service.
+   * @param {object} [backupResources=DefaultBackupResources]
+   *   Object containing BackupResource classes to associate with this service.
    */
   constructor(backupResources = DefaultBackupResources) {
     super();
@@ -355,6 +386,10 @@ export class BackupService extends EventTarget {
         }
       );
 
+      let encState = await this.loadEncryptionState(profilePath);
+      let encryptionEnabled = !!encState;
+      lazy.logConsole.debug("Encryption enabled: ", encryptionEnabled);
+
       // Perform the backup for each resource.
       for (let resourceClass of sortedResources) {
         try {
@@ -362,6 +397,14 @@ export class BackupService extends EventTarget {
             `Backing up resource with key ${resourceClass.key}. ` +
               `Requires encryption: ${resourceClass.requiresEncryption}`
           );
+
+          if (resourceClass.requiresEncryption && !encryptionEnabled) {
+            lazy.logConsole.debug(
+              "Encryption is not currently enabled. Skipping."
+            );
+            continue;
+          }
+
           let resourcePath = PathUtils.join(stagingPath, resourceClass.key);
           await IOUtils.makeDirectory(resourcePath);
 
@@ -584,10 +627,15 @@ export class BackupService extends EventTarget {
 
       /**
        * Bug 1892532: for now, we only support a single backup file.
-       * If there are other pre-existing backup folders, delete them.
+       * If there are other pre-existing backup folders, delete them - but don't
+       * delete anything that doesn't match the backup folder naming scheme.
        */
+      let expectedFormatRegex = /\d{4}(-\d{2}){2}T(\d{2}-){2}\d{2}Z/;
       for (let existingBackupPath of existingBackups) {
-        if (existingBackupPath !== renamedBackupPath) {
+        if (
+          existingBackupPath !== renamedBackupPath &&
+          existingBackupPath.match(expectedFormatRegex)
+        ) {
           await IOUtils.remove(existingBackupPath, {
             recursive: true,
           });
@@ -944,4 +992,153 @@ export class BackupService extends EventTarget {
       }
     }
   }
+
+  /**
+   * The internal promise that is created on the first call to
+   * loadEncryptionState.
+   *
+   * @type {Promise}
+   */
+  #loadEncryptionStatePromise = null;
+
+  /**
+   * Returns the current ArchiveEncryptionState. This method will only attempt
+   * to read the state from the disk the first time it is called.
+   *
+   * @param {string} [profilePath=PathUtils.profileDir]
+   *   The profile path where the encryption state might exist. This is only
+   *   used for testing.
+   * @returns {Promise<ArchiveEncryptionState>}
+   */
+  loadEncryptionState(profilePath = PathUtils.profileDir) {
+    if (this.#encState !== undefined) {
+      return Promise.resolve(this.#encState);
+    }
+
+    // This little dance makes it so that we only attempt to read the state off
+    // of the disk the first time `loadEncryptionState` is called. Any
+    // subsequent calls will await this same promise, OR, after the state has
+    // been read in, they'll just get the #encState which is set after the
+    // state has been read in.
+    if (!this.#loadEncryptionStatePromise) {
+      this.#loadEncryptionStatePromise = (async () => {
+        // Default this to null here - that way, if we fail to read it in,
+        // the null will indicate that we have at least _tried_ to load the
+        // state.
+        let encState = null;
+        let encStateFile = PathUtils.join(
+          profilePath,
+          BackupService.PROFILE_FOLDER_NAME,
+          BackupService.ARCHIVE_ENCRYPTION_STATE_FILE
+        );
+
+        // Try to read in any pre-existing encryption state. If that fails,
+        // we fallback to not encrypting, and only backing up non-sensitive data.
+        try {
+          if (await IOUtils.exists(encStateFile)) {
+            let stateObject = await IOUtils.readJSON(encStateFile);
+            ({ instance: encState } =
+              await lazy.ArchiveEncryptionState.initialize(stateObject));
+          }
+        } catch (e) {
+          lazy.logConsole.error(
+            "Failed to read / deserialize archive encryption state file: ",
+            e
+          );
+          // TODO: This kind of error might be worth collecting telemetry on.
+        }
+
+        this.#_state.encryptionEnabled = !!encState;
+        this.stateUpdate();
+
+        this.#encState = encState;
+        return encState;
+      })();
+    }
+
+    return this.#loadEncryptionStatePromise;
+  }
+
+  /**
+   * Enables encryption for backups, allowing sensitive data to be backed up.
+   * Throws if encryption is already enabled. After enabling encryption, that
+   * state is written to disk.
+   *
+   * @throws Exception
+   * @param {string} password
+   *   A non-blank password ("recovery code") that can be used to derive keys
+   *   for encrypting the backup.
+   * @param {string} [profilePath=PathUtils.profileDir]
+   *   The profile path where the encryption state will be written. This is only
+   *   used for testing.
+   */
+  async enableEncryption(password, profilePath = PathUtils.profileDir) {
+    lazy.logConsole.debug("Enabling encryption.");
+    let encState = await this.loadEncryptionState(profilePath);
+    if (encState) {
+      throw new Error("Encryption is already enabled.");
+    }
+
+    if (!password) {
+      throw new Error("Cannot supply a blank password.");
+    }
+
+    if (password.length < 8) {
+      throw new Error("Password must be at least 8 characters.");
+    }
+
+    // TODO: Enforce other password rules here, such as ensuring that the
+    // password is not considered common.
+    ({ instance: encState } = await lazy.ArchiveEncryptionState.initialize(
+      password
+    ));
+    if (!encState) {
+      throw new Error("Failed to construct ArchiveEncryptionState");
+    }
+
+    this.#encState = encState;
+
+    let encStateFile = PathUtils.join(
+      profilePath,
+      BackupService.PROFILE_FOLDER_NAME,
+      BackupService.ARCHIVE_ENCRYPTION_STATE_FILE
+    );
+
+    let stateObj = await encState.serialize();
+    await IOUtils.writeJSON(encStateFile, stateObj);
+
+    this.#_state.encryptionEnabled = true;
+    this.stateUpdate();
+  }
+
+  /**
+   * Disables encryption of backups. Throws is encryption is already disabled.
+   *
+   * @throws Exception
+   * @param {string} [profilePath=PathUtils.profileDir]
+   *   The profile path where the encryption state exists. This is only used for
+   *   testing.
+   * @returns {Promise<undefined>}
+   */
+  async disableEncryption(profilePath = PathUtils.profileDir) {
+    lazy.logConsole.debug("Disabling encryption.");
+    let encState = await this.loadEncryptionState(profilePath);
+    if (!encState) {
+      throw new Error("Encryption is already disabled.");
+    }
+
+    let encStateFile = PathUtils.join(
+      profilePath,
+      BackupService.PROFILE_FOLDER_NAME,
+      BackupService.ARCHIVE_ENCRYPTION_STATE_FILE
+    );
+    // It'd be pretty strange, but not impossible, for something else to have
+    // gotten rid of the encryption state file at this point. We'll ignore it
+    // if that's the case.
+    await IOUtils.remove(encStateFile, { ignoreAbsent: true });
+
+    this.#encState = null;
+    this.#_state.encryptionEnabled = false;
+    this.stateUpdate();
+  }
 }

browser/components/backup/content/debug.html

@@ -31,6 +31,9 @@
               preference="browser.backup.log"
             />BackupService debug logging enabled
           </li>
+          <li>
+            <input id="encryption-enabled" type="checkbox" />Encryption enabled
+          </li>
         </ol>
       </section>
       <section id="controls">

browser/components/backup/content/debug.js

@@ -17,12 +17,39 @@ let DebugUI = {
   init() {
     let controls = document.querySelector("#controls");
     controls.addEventListener("click", this);
+
+    let encryptionEnabled = document.querySelector("#encryption-enabled");
+    encryptionEnabled.addEventListener("click", this);
+
+    // We use `init` instead of `get` here, since this page might load before
+    // the BackupService has had a chance to initialize itself.
+    let service = BackupService.init();
+    service.addEventListener("BackupService:StateUpdate", this);
+    this.onStateUpdate();
+
+    // Kick-off reading any pre-existing encryption state off of the disk.
+    service.loadEncryptionState();
   },
 
   handleEvent(event) {
+    switch (event.type) {
+      case "BackupService:StateUpdate": {
+        this.onStateUpdate();
+        break;
+      }
+      case "click": {
         let target = event.target;
         if (HTMLButtonElement.isInstance(event.target)) {
           this.onButtonClick(target);
+        } else if (
+          HTMLInputElement.isInstance(event.target) &&
+          event.target.type == "checkbox"
+        ) {
+          event.preventDefault();
+          this.onCheckboxClick(target);
+        }
+        break;
+      }
     }
   },
 
@@ -111,9 +138,50 @@ let DebugUI = {
           );
           throw e;
         }
+        break;
       }
     }
   },
+
+  async onCheckboxClick(checkbox) {
+    if (checkbox.id == "encryption-enabled") {
+      let service = BackupService.get();
+      if (checkbox.checked) {
+        let password = prompt("What's the encryption password? (8 char min)");
+        if (password != null) {
+          try {
+            await service.enableEncryption(password);
+          } catch (e) {
+            console.error(e);
+          }
+        }
+      } else if (confirm("Disable encryption?")) {
+        try {
+          await service.disableEncryption();
+        } catch (e) {
+          console.error(e);
+        }
+      }
+    }
+  },
+
+  onStateUpdate() {
+    let service = BackupService.get();
+    let state = service.state;
+
+    let encryptionEnabled = document.querySelector("#encryption-enabled");
+    encryptionEnabled.checked = state.encryptionEnabled;
+  },
 };
 
-DebugUI.init();
+// Wait until the load event fires before setting up any listeners or updating
+// any of the state of the page. We do this in order to avoid having any of
+// our control states overwritten by SessionStore after a restoration, as
+// restoration of form state occurs _prior_ to the load event firing.
+addEventListener(
+  "load",
+  () => {
+    DebugUI.init();
+  },
+  { once: true }
+);

browser/components/backup/tests/marionette/test_backup.py

@@ -58,6 +58,9 @@ class BackupTest(MarionetteTestCase):
 
         originalStagingPath = self.marionette.execute_async_script(
             """
+          const { OSKeyStore } = ChromeUtils.importESModule(
+            "resource://gre/modules/OSKeyStore.sys.mjs"
+          );
           const { BackupService } = ChromeUtils.importESModule("resource:///modules/backup/BackupService.sys.mjs");
           let bs = BackupService.init();
           if (!bs) {
@@ -66,6 +69,18 @@ class BackupTest(MarionetteTestCase):
 
           let [outerResolve] = arguments;
           (async () => {
+            // This is some hackery to make it so that OSKeyStore doesn't kick
+            // off an OS authentication dialog in our test, and also to make
+            // sure we don't blow away the _real_ OSKeyStore key for the browser
+            // on the system that this test is running on. Normally, I'd use
+            // OSKeyStoreTestUtils.setup to do this, but apparently the
+            // testing-common modules aren't available in Marionette tests.
+            const ORIGINAL_STORE_LABEL = OSKeyStore.STORE_LABEL;
+            OSKeyStore.STORE_LABEL = "test-" + Math.random().toString(36).substr(2);
+            await bs.enableEncryption("This is a test password");
+            await OSKeyStore.cleanup();
+            OSKeyStore.STORE_LABEL = ORIGINAL_STORE_LABEL;
+
             let { stagingPath } = await bs.createBackup();
             if (!stagingPath) {
               throw new Error("Could not create backup.");

browser/components/backup/tests/xpcshell/head.js

@@ -48,7 +48,7 @@ class FakeBackupResource2 extends BackupResource {
     return "fake2";
   }
   static get requiresEncryption() {
-    return true;
+    return false;
   }
   static get priority() {
     return 1;

browser/components/backup/tests/xpcshell/test_BackupService_enable_disable_encryption.js

@@ -0,0 +1,290 @@
+/* Any copyright is dedicated to the Public Domain.
+https://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const { OSKeyStoreTestUtils } = ChromeUtils.importESModule(
+  "resource://testing-common/OSKeyStoreTestUtils.sys.mjs"
+);
+
+const TEST_PASSWORD = "This is some test password.";
+
+add_setup(async () => {
+  // Ensure that enabling encryption doesn't cause the native OSKeyStore to
+  // require authentication.
+  OSKeyStoreTestUtils.setup();
+  registerCleanupFunction(async () => {
+    await OSKeyStoreTestUtils.cleanup();
+  });
+});
+
+/**
+ * Tests that if encryption is disabled that only BackupResource's with
+ * requiresEncryption set to `false` will have `backup()` called on them.
+ */
+add_task(async function test_disabled_encryption() {
+  let sandbox = sinon.createSandbox();
+
+  let bs = new BackupService({
+    FakeBackupResource1,
+    FakeBackupResource2,
+    FakeBackupResource3,
+  });
+
+  Assert.ok(
+    !bs.state.encryptionEnabled,
+    "State should indicate that encryption is disabled."
+  );
+
+  let testProfilePath = await IOUtils.createUniqueDirectory(
+    PathUtils.tempDir,
+    "testDisabledEncryption"
+  );
+  let encState = await bs.loadEncryptionState(testProfilePath);
+  Assert.ok(!encState, "Should not find an ArchiveEncryptionState.");
+
+  // Override FakeBackupResource2 so that it requires encryption.
+  sandbox.stub(FakeBackupResource2, "requiresEncryption").get(() => {
+    return true;
+  });
+  Assert.ok(
+    FakeBackupResource2.requiresEncryption,
+    "FakeBackupResource2 requires encryption."
+  );
+
+  // This is how these FakeBackupResource's are defined in head.js
+  Assert.ok(
+    !FakeBackupResource1.requiresEncryption,
+    "FakeBackupResource1 does not require encryption."
+  );
+  Assert.ok(
+    !FakeBackupResource3.requiresEncryption,
+    "FakeBackupResource3 does not require encryption."
+  );
+
+  let resourceWithoutEncryptionStubs = [
+    sandbox.stub(FakeBackupResource1.prototype, "backup").resolves(null),
+    sandbox.stub(FakeBackupResource3.prototype, "backup").resolves(null),
+  ];
+
+  let resourceWithEncryptionStub = sandbox
+    .stub(FakeBackupResource2.prototype, "backup")
+    .resolves(null);
+
+  await bs.createBackup({ profilePath: testProfilePath });
+  Assert.ok(
+    resourceWithEncryptionStub.notCalled,
+    "FakeBackupResource2.backup should not have been called"
+  );
+
+  for (let resourceWithoutEncryptionStub of resourceWithoutEncryptionStubs) {
+    Assert.ok(
+      resourceWithoutEncryptionStub.calledOnce,
+      "backup called on resource that didn't require encryption"
+    );
+  }
+
+  await IOUtils.remove(testProfilePath, { recursive: true });
+  sandbox.restore();
+});
+
+/**
+ * Tests that encryption cannot be disabled if it's already disabled.
+ */
+add_task(async function test_already_disabled_encryption() {
+  let bs = new BackupService();
+
+  Assert.ok(
+    !bs.state.encryptionEnabled,
+    "State should indicate that encryption is disabled."
+  );
+
+  let encState = await bs.loadEncryptionState();
+  Assert.ok(!encState, "Should not find an ArchiveEncryptionState.");
+
+  await Assert.rejects(
+    bs.disableEncryption(),
+    /already disabled/,
+    "It should not be possible to disable encryption if it's already disabled"
+  );
+});
+
+/**
+ * Tests that if encryption is enabled from a non-enabled state, that an
+ * ArchiveEncryptionState is created, and state is written to the profile
+ * directory. Also tests that this allows BackupResource's with
+ * requiresEncryption set to `true` to have `backup()` called on them.
+ */
+add_task(async function test_enable_encryption() {
+  let sandbox = sinon.createSandbox();
+
+  let bs = new BackupService({
+    FakeBackupResource1,
+    FakeBackupResource2,
+    FakeBackupResource3,
+  });
+
+  Assert.ok(
+    !bs.state.encryptionEnabled,
+    "State should initially indicate that encryption is disabled."
+  );
+
+  let testProfilePath = await IOUtils.createUniqueDirectory(
+    PathUtils.tempDir,
+    "testEnableEncryption"
+  );
+  let encState = await bs.loadEncryptionState(testProfilePath);
+  Assert.ok(!encState, "Should not find an ArchiveEncryptionState.");
+
+  // Now enable encryption.
+  let stateUpdatePromise = new Promise(resolve => {
+    bs.addEventListener("BackupService:StateUpdate", resolve, { once: true });
+  });
+  await bs.enableEncryption(TEST_PASSWORD, testProfilePath);
+  await stateUpdatePromise;
+  Assert.ok(
+    bs.state.encryptionEnabled,
+    "State should indicate that encryption is enabled."
+  );
+
+  Assert.ok(
+    await IOUtils.exists(
+      PathUtils.join(
+        testProfilePath,
+        BackupService.PROFILE_FOLDER_NAME,
+        BackupService.ARCHIVE_ENCRYPTION_STATE_FILE
+      )
+    ),
+    "Encryption state file should exist."
+  );
+
+  // Override FakeBackupResource2 so that it requires encryption.
+  sandbox.stub(FakeBackupResource2, "requiresEncryption").get(() => {
+    return true;
+  });
+  Assert.ok(
+    FakeBackupResource2.requiresEncryption,
+    "FakeBackupResource2 requires encryption."
+  );
+
+  // This is how these FakeBackupResource's are defined in head.js
+  Assert.ok(
+    !FakeBackupResource1.requiresEncryption,
+    "FakeBackupResource1 does not require encryption."
+  );
+  Assert.ok(
+    !FakeBackupResource3.requiresEncryption,
+    "FakeBackupResource3 does not require encryption."
+  );
+
+  let allResourceBackupStubs = [
+    sandbox.stub(FakeBackupResource1.prototype, "backup").resolves(null),
+    sandbox.stub(FakeBackupResource3.prototype, "backup").resolves(null),
+    sandbox.stub(FakeBackupResource2.prototype, "backup").resolves(null),
+  ];
+
+  await bs.createBackup({
+    profilePath: testProfilePath,
+  });
+
+  for (let resourceBackupStub of allResourceBackupStubs) {
+    Assert.ok(resourceBackupStub.calledOnce, "backup called on resource");
+  }
+
+  Assert.ok(
+    await IOUtils.exists(
+      PathUtils.join(
+        testProfilePath,
+        BackupService.PROFILE_FOLDER_NAME,
+        BackupService.ARCHIVE_ENCRYPTION_STATE_FILE
+      )
+    ),
+    "Encryption state file should still exist."
+  );
+
+  await IOUtils.remove(testProfilePath, { recursive: true });
+  sandbox.restore();
+});
+
+/**
+ * Tests that encryption cannot be enabled if it's already enabled.
+ */
+add_task(async function test_already_enabled_encryption() {
+  let bs = new BackupService();
+
+  let testProfilePath = await IOUtils.createUniqueDirectory(
+    PathUtils.tempDir,
+    "testAlreadyEnabledEncryption"
+  );
+
+  // Enable encryption.
+  await bs.enableEncryption(TEST_PASSWORD, testProfilePath);
+
+  let encState = await bs.loadEncryptionState(testProfilePath);
+  Assert.ok(encState, "ArchiveEncryptionState is available.");
+
+  await Assert.rejects(
+    bs.enableEncryption(TEST_PASSWORD, testProfilePath),
+    /already enabled/,
+    "It should not be possible to enable encryption if it's already enabled"
+  );
+
+  await IOUtils.remove(testProfilePath, { recursive: true });
+});
+
+/**
+ * Tests that if encryption is enabled that it can be disabled.
+ */
+add_task(async function test_disabling_encryption() {
+  let bs = new BackupService();
+
+  let testProfilePath = await IOUtils.createUniqueDirectory(
+    PathUtils.tempDir,
+    "testDisableEncryption"
+  );
+
+  // Enable encryption.
+  await bs.enableEncryption(TEST_PASSWORD, testProfilePath);
+
+  let encState = await bs.loadEncryptionState(testProfilePath);
+  Assert.ok(encState, "ArchiveEncryptionState is available.");
+  Assert.ok(
+    bs.state.encryptionEnabled,
+    "State should indicate that encryption is enabled."
+  );
+
+  Assert.ok(
+    await IOUtils.exists(
+      PathUtils.join(
+        testProfilePath,
+        BackupService.PROFILE_FOLDER_NAME,
+        BackupService.ARCHIVE_ENCRYPTION_STATE_FILE
+      )
+    ),
+    "Encryption state file should exist."
+  );
+
+  // Now disable encryption.
+  let stateUpdatePromise = new Promise(resolve => {
+    bs.addEventListener("BackupService:StateUpdate", resolve, { once: true });
+  });
+  await bs.disableEncryption(testProfilePath);
+  await stateUpdatePromise;
+  Assert.ok(
+    !bs.state.encryptionEnabled,
+    "State should indicate that encryption is now disabled."
+  );
+
+  Assert.ok(
+    !(await IOUtils.exists(
+      PathUtils.join(
+        testProfilePath,
+        BackupService.PROFILE_FOLDER_NAME,
+        BackupService.ARCHIVE_ENCRYPTION_STATE_FILE
+      )
+    )),
+    "Encryption state file should have been removed."
+  );
+
+  await IOUtils.remove(testProfilePath, { recursive: true });
+});

browser/components/backup/tests/xpcshell/xpcshell.toml

@@ -16,6 +16,9 @@ support-files = ["data/test_xulstore.json"]
 
 ["test_BackupService.js"]
 
+["test_BackupService_enable_disable_encryption.js"]
+skip-if = ["apple_silicon && automation"] # bug 1729538
+
 ["test_BackupService_takeMeasurements.js"]
 
 ["test_CookiesBackupResource.js"]