Reset cross-signing before backup when resetting both (#28402)

* reset cross-signing before backup when resetting both

* add test for AccessSecretStorageDialog

* fix unit test

src/SecurityManager.ts

@@ -186,6 +186,15 @@ export async function withSecretStorageKeyCache<T>(func: () => Promise<T>): Prom
     }
 }
 
+export interface AccessSecretStorageOpts {
+    /** Reset secret storage even if it's already set up. */
+    forceReset?: boolean;
+    /** Create new cross-signing keys. Only applicable if `forceReset` is `true`. */
+    resetCrossSigning?: boolean;
+    /** The cached account password, if available. */
+    accountPassword?: string;
+}
+
 /**
  * This helper should be used whenever you need to access secret storage. It
  * ensures that secret storage (and also cross-signing since they each depend on
@@ -205,14 +214,17 @@ export async function withSecretStorageKeyCache<T>(func: () => Promise<T>): Prom
  *
  * @param {Function} [func] An operation to perform once secret storage has been
  * bootstrapped. Optional.
- * @param {bool} [forceReset] Reset secret storage even if it's already set up
+ * @param [opts] The options to use when accessing secret storage.
  */
-export async function accessSecretStorage(func = async (): Promise<void> => {}, forceReset = false): Promise<void> {
+export async function accessSecretStorage(
-    await withSecretStorageKeyCache(() => doAccessSecretStorage(func, forceReset));
+    func = async (): Promise<void> => {},
+    opts: AccessSecretStorageOpts = {},
+): Promise<void> {
+    await withSecretStorageKeyCache(() => doAccessSecretStorage(func, opts));
 }
 
 /** Helper for {@link #accessSecretStorage} */
-async function doAccessSecretStorage(func: () => Promise<void>, forceReset: boolean): Promise<void> {
+async function doAccessSecretStorage(func: () => Promise<void>, opts: AccessSecretStorageOpts): Promise<void> {
     try {
         const cli = MatrixClientPeg.safeGet();
         const crypto = cli.getCrypto();
@@ -221,7 +233,7 @@ async function doAccessSecretStorage(func: () => Promise<void>, forceReset: bool
         }
 
         let createNew = false;
-        if (forceReset) {
+        if (opts.forceReset) {
             logger.debug("accessSecretStorage: resetting 4S");
             createNew = true;
         } else if (!(await cli.secretStorage.hasKey())) {
@@ -234,9 +246,7 @@ async function doAccessSecretStorage(func: () => Promise<void>, forceReset: bool
             // passphrase creation.
             const { finished } = Modal.createDialog(
                 lazy(() => import("./async-components/views/dialogs/security/CreateSecretStorageDialog")),
-                {
+                opts,
-                    forceReset,
-                },
                 undefined,
                 /* priority = */ false,
                 /* static = */ true,

src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx

@@ -58,6 +58,7 @@ interface IProps {
     hasCancel?: boolean;
     accountPassword?: string;
     forceReset?: boolean;
+    resetCrossSigning?: boolean;
     onFinished(ok?: boolean): void;
 }
 
@@ -91,6 +92,7 @@ export default class CreateSecretStorageDialog extends React.PureComponent<IProp
     public static defaultProps: Partial<IProps> = {
         hasCancel: true,
         forceReset: false,
+        resetCrossSigning: false,
     };
     private recoveryKey?: GeneratedSecretStorageKey;
     private recoveryKeyNode = createRef<HTMLElement>();
@@ -270,7 +272,7 @@ export default class CreateSecretStorageDialog extends React.PureComponent<IProp
     private bootstrapSecretStorage = async (): Promise<void> => {
         const cli = MatrixClientPeg.safeGet();
         const crypto = cli.getCrypto()!;
-        const { forceReset } = this.props;
+        const { forceReset, resetCrossSigning } = this.props;
 
         let backupInfo;
         // First, unless we know we want to do a reset, we see if there is an existing key backup
@@ -292,12 +294,28 @@ export default class CreateSecretStorageDialog extends React.PureComponent<IProp
 
         try {
             if (forceReset) {
+                /* Resetting cross-signing requires secret storage to be reset
+                 * (otherwise it will try to store the cross-signing keys in the
+                 * old secret storage, and may prompt for the old key, which is
+                 * probably not available), and resetting key backup requires
+                 * cross-signing to be reset (so that the new backup can be
+                 * signed by the new cross-signing key).  So we reset secret
+                 * storage first, then cross-signing, then key backup.
+                 */
                 logger.log("Forcing secret storage reset");
                 await crypto.bootstrapSecretStorage({
                     createSecretStorageKey: async () => this.recoveryKey!,
-                    setupNewKeyBackup: true,
                     setupNewSecretStorage: true,
                 });
+                if (resetCrossSigning) {
+                    logger.log("Resetting cross signing");
+                    await crypto.bootstrapCrossSigning({
+                        authUploadDeviceSigningKeys: this.doBootstrapUIAuth,
+                        setupNewCrossSigning: true,
+                    });
+                }
+                logger.log("Resetting key backup");
+                await crypto.resetKeyBackup();
             } else {
                 // For password authentication users after 2020-09, this cross-signing
                 // step will be a no-op since it is now setup during registration or login

src/components/views/dialogs/security/AccessSecretStorageDialog.tsx

@@ -19,7 +19,6 @@ import AccessibleButton, { ButtonEvent } from "../../elements/AccessibleButton";
 import { _t } from "../../../../languageHandler";
 import { accessSecretStorage } from "../../../../SecurityManager";
 import Modal from "../../../../Modal";
-import InteractiveAuthDialog from "../InteractiveAuthDialog";
 import DialogButtons from "../../elements/DialogButtons";
 import BaseDialog from "../BaseDialog";
 import { chromeFileInputFix } from "../../../../utils/BrowserWorkarounds";
@@ -226,28 +225,14 @@ export default class AccessSecretStorageDialog extends React.PureComponent<IProp
 
         try {
             // Force reset secret storage (which resets the key backup)
-            await accessSecretStorage(async (): Promise<void> => {
+            await accessSecretStorage(
-                // Now reset cross-signing so everything Just Works™ again.
+                async (): Promise<void> => {
-                const cli = MatrixClientPeg.safeGet();
+                    // Now we can indicate that the user is done pressing buttons, finally.
-                await cli.getCrypto()?.bootstrapCrossSigning({
+                    // Upstream flows will detect the new secret storage, key backup, etc and use it.
-                    authUploadDeviceSigningKeys: async (makeRequest): Promise<void> => {
+                    this.props.onFinished({});
-                        const { finished } = Modal.createDialog(InteractiveAuthDialog, {
+                },
-                            title: _t("encryption|bootstrap_title"),
+                { forceReset: true, resetCrossSigning: true },
-                            matrixClient: cli,
+            );
-                            makeRequest,
-                        });
-                        const [confirmed] = await finished;
-                        if (!confirmed) {
-                            throw new Error("Cross-signing key upload auth canceled");
-                        }
-                    },
-                    setupNewCrossSigning: true,
-                });
-
-                // Now we can indicate that the user is done pressing buttons, finally.
-                // Upstream flows will detect the new secret storage, key backup, etc and use it.
-                this.props.onFinished({});
-            }, true);
         } catch (e) {
             logger.error(e);
             this.props.onFinished(false);

src/components/views/dialogs/security/RestoreKeyBackupDialog.tsx

@@ -109,7 +109,7 @@ export default class RestoreKeyBackupDialog extends React.PureComponent<IProps,
 
     private onResetRecoveryClick = (): void => {
         this.props.onFinished(false);
-        accessSecretStorage(async (): Promise<void> => {}, /* forceReset = */ true);
+        accessSecretStorage(async (): Promise<void> => {}, { forceReset: true });
     };
 
     /**

src/components/views/settings/SecureBackupPanel.tsx

@@ -209,7 +209,7 @@ export default class SecureBackupPanel extends React.PureComponent<{}, IState> {
     private resetSecretStorage = async (): Promise<void> => {
         this.setState({ error: false });
         try {
-            await accessSecretStorage(async (): Promise<void> => {}, /* forceReset = */ true);
+            await accessSecretStorage(async (): Promise<void> => {}, { forceReset: true });
         } catch (e) {
             logger.error("Error resetting secret storage", e);
             if (this.unmounted) return;

src/stores/SetupEncryptionStore.ts

@@ -19,9 +19,6 @@ import { Device, SecretStorage } from "matrix-js-sdk/src/matrix";
 
 import { MatrixClientPeg } from "../MatrixClientPeg";
 import { AccessCancelledError, accessSecretStorage } from "../SecurityManager";
-import Modal from "../Modal";
-import InteractiveAuthDialog from "../components/views/dialogs/InteractiveAuthDialog";
-import { _t } from "../languageHandler";
 import { SdkContextClass } from "../contexts/SDKContext";
 import { asyncSome } from "../utils/arrays";
 import { initialiseDehydration } from "../utils/device/dehydration";
@@ -230,42 +227,16 @@ export class SetupEncryptionStore extends EventEmitter {
             // secret storage key if they had one. Start by resetting
             // secret storage and setting up a new recovery key, then
             // create new cross-signing keys once that succeeds.
-            await accessSecretStorage(async (): Promise<void> => {
+            await accessSecretStorage(
-                const cli = MatrixClientPeg.safeGet();
+                async (): Promise<void> => {
-                await cli.getCrypto()?.bootstrapCrossSigning({
+                    this.phase = Phase.Finished;
-                    authUploadDeviceSigningKeys: async (makeRequest): Promise<void> => {
+                },
-                        const cachedPassword = SdkContextClass.instance.accountPasswordStore.getPassword();
+                {
-
+                    forceReset: true,
-                        if (cachedPassword) {
+                    resetCrossSigning: true,
-                            await makeRequest({
+                    accountPassword: SdkContextClass.instance.accountPasswordStore.getPassword(),
-                                type: "m.login.password",
+                },
-                                identifier: {
+            );
-                                    type: "m.id.user",
-                                    user: cli.getSafeUserId(),
-                                },
-                                user: cli.getSafeUserId(),
-                                password: cachedPassword,
-                            });
-                            return;
-                        }
-
-                        const { finished } = Modal.createDialog(InteractiveAuthDialog, {
-                            title: _t("encryption|bootstrap_title"),
-                            matrixClient: cli,
-                            makeRequest,
-                        });
-                        const [confirmed] = await finished;
-                        if (!confirmed) {
-                            throw new Error("Cross-signing key upload auth canceled");
-                        }
-                    },
-                    setupNewCrossSigning: true,
-                });
-
-                await initialiseDehydration(true);
-
-                this.phase = Phase.Finished;
-            }, true);
         } catch (e) {
             logger.error("Error resetting cross-signing", e);
             this.phase = Phase.Intro;

test/unit-tests/SecurityManager-test.ts

@@ -68,7 +68,7 @@ describe("SecurityManager", () => {
             stubClient();
 
             const func = jest.fn();
-            accessSecretStorage(func, true);
+            accessSecretStorage(func, { forceReset: true });
 
             expect(spy).toHaveBeenCalledTimes(1);
             await expect(spy.mock.lastCall![0]).resolves.toEqual(expect.objectContaining({ __test: true }));

test/unit-tests/components/views/dialogs/AccessSecretStorageDialog-test.tsx

@@ -122,4 +122,34 @@ describe("AccessSecretStorageDialog", () => {
 
         expect(screen.getByPlaceholderText("Security Phrase")).toHaveFocus();
     });
+
+    it("Can reset secret storage", async () => {
+        jest.spyOn(mockClient.secretStorage, "checkKey").mockResolvedValue(true);
+
+        const onFinished = jest.fn();
+        const checkPrivateKey = jest.fn().mockResolvedValue(true);
+        renderComponent({ onFinished, checkPrivateKey });
+
+        await userEvent.click(screen.getByText("Reset all"), { delay: null });
+
+        // It will prompt the user to confirm resetting
+        expect(screen.getByText("Reset everything")).toBeInTheDocument();
+        await userEvent.click(screen.getByText("Reset"), { delay: null });
+
+        // Then it will prompt the user to create a key/passphrase
+        await screen.findByText("Set up Secure Backup");
+        document.execCommand = jest.fn().mockReturnValue(true);
+        jest.spyOn(mockClient.getCrypto()!, "createRecoveryKeyFromPassphrase").mockResolvedValue({
+            privateKey: new Uint8Array(),
+            encodedPrivateKey: securityKey,
+        });
+        screen.getByRole("button", { name: "Continue" }).click();
+
+        await screen.findByText(/Save your Security Key/);
+        screen.getByRole("button", { name: "Copy" }).click();
+        await screen.findByText("Copied!");
+        screen.getByRole("button", { name: "Continue" }).click();
+
+        await screen.findByText("Secure Backup successful");
+    });
 });

test/unit-tests/components/views/dialogs/security/CreateSecretStorageDialog-test.tsx

@@ -97,4 +97,38 @@ describe("CreateSecretStorageDialog", () => {
             await screen.findByText("Your keys are now being backed up from this device.");
         });
     });
+
+    it("resets keys in the right order when resetting secret storage and cross-signing", async () => {
+        const result = renderComponent({ forceReset: true, resetCrossSigning: true });
+
+        await result.findByText(/Set up Secure Backup/);
+        jest.spyOn(mockClient.getCrypto()!, "createRecoveryKeyFromPassphrase").mockResolvedValue({
+            privateKey: new Uint8Array(),
+            encodedPrivateKey: "abcd efgh ijkl",
+        });
+        result.getByRole("button", { name: "Continue" }).click();
+
+        await result.findByText(/Save your Security Key/);
+        result.getByRole("button", { name: "Copy" }).click();
+
+        // Resetting should reset secret storage, cross signing, and key
+        // backup.  We make sure that all three are reset, and done in the
+        // right order.
+        const resetFunctionCallLog: string[] = [];
+        jest.spyOn(mockClient.getCrypto()!, "bootstrapSecretStorage").mockImplementation(async () => {
+            resetFunctionCallLog.push("bootstrapSecretStorage");
+        });
+        jest.spyOn(mockClient.getCrypto()!, "bootstrapCrossSigning").mockImplementation(async () => {
+            resetFunctionCallLog.push("bootstrapCrossSigning");
+        });
+        jest.spyOn(mockClient.getCrypto()!, "resetKeyBackup").mockImplementation(async () => {
+            resetFunctionCallLog.push("resetKeyBackup");
+        });
+
+        result.getByRole("button", { name: "Continue" }).click();
+
+        await result.findByText("Your keys are now being backed up from this device.");
+
+        expect(resetFunctionCallLog).toEqual(["bootstrapSecretStorage", "bootstrapCrossSigning", "resetKeyBackup"]);
+    });
 });

test/unit-tests/stores/SetupEncryptionStore-test.ts

@@ -170,15 +170,10 @@ describe("SetupEncryptionStore", () => {
 
         await setupEncryptionStore.resetConfirm();
 
-        expect(mocked(accessSecretStorage)).toHaveBeenCalledWith(expect.any(Function), true);
+        expect(mocked(accessSecretStorage)).toHaveBeenCalledWith(expect.any(Function), {
-        expect(makeRequest).toHaveBeenCalledWith({
+            accountPassword: cachedPassword,
-            identifier: {
+            forceReset: true,
-                type: "m.id.user",
+            resetCrossSigning: true,
-                user: "@userId:matrix.org",
-            },
-            password: cachedPassword,
-            type: "m.login.password",
-            user: "@userId:matrix.org",
         });
     });
 });