From 0ae74a9e1f3f253228d095311498a9f69338df48 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 18 Nov 2024 22:17:24 -0500 Subject: [PATCH] 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 | 26 +++++++--- .../security/CreateSecretStorageDialog.tsx | 22 ++++++++- .../security/AccessSecretStorageDialog.tsx | 31 +++--------- .../security/RestoreKeyBackupDialog.tsx | 2 +- .../views/settings/SecureBackupPanel.tsx | 2 +- src/stores/SetupEncryptionStore.ts | 49 ++++--------------- test/unit-tests/SecurityManager-test.ts | 2 +- .../AccessSecretStorageDialog-test.tsx | 30 ++++++++++++ .../CreateSecretStorageDialog-test.tsx | 34 +++++++++++++ .../stores/SetupEncryptionStore-test.ts | 13 ++--- 10 files changed, 127 insertions(+), 84 deletions(-) diff --git a/src/SecurityManager.ts b/src/SecurityManager.ts index cf8d40acc8..f97dff786f 100644 --- a/src/SecurityManager.ts +++ b/src/SecurityManager.ts @@ -186,6 +186,15 @@ export async function withSecretStorageKeyCache(func: () => Promise): 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(func: () => Promise): 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 => {}, forceReset = false): Promise { - await withSecretStorageKeyCache(() => doAccessSecretStorage(func, forceReset)); +export async function accessSecretStorage( + func = async (): Promise => {}, + opts: AccessSecretStorageOpts = {}, +): Promise { + await withSecretStorageKeyCache(() => doAccessSecretStorage(func, opts)); } /** Helper for {@link #accessSecretStorage} */ -async function doAccessSecretStorage(func: () => Promise, forceReset: boolean): Promise { +async function doAccessSecretStorage(func: () => Promise, opts: AccessSecretStorageOpts): Promise { try { const cli = MatrixClientPeg.safeGet(); const crypto = cli.getCrypto(); @@ -221,7 +233,7 @@ async function doAccessSecretStorage(func: () => Promise, 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, forceReset: bool // passphrase creation. const { finished } = Modal.createDialog( lazy(() => import("./async-components/views/dialogs/security/CreateSecretStorageDialog")), - { - forceReset, - }, + opts, undefined, /* priority = */ false, /* static = */ true, diff --git a/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx b/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx index 1e87b5b826..1258bde2ca 100644 --- a/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx +++ b/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 = { hasCancel: true, forceReset: false, + resetCrossSigning: false, }; private recoveryKey?: GeneratedSecretStorageKey; private recoveryKeyNode = createRef(); @@ -270,7 +272,7 @@ export default class CreateSecretStorageDialog extends React.PureComponent => { 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 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 diff --git a/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx b/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx index 7361e3982d..d9c97261dd 100644 --- a/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx +++ b/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 => { - // Now reset cross-signing so everything Just Works™ again. - const cli = MatrixClientPeg.safeGet(); - await cli.getCrypto()?.bootstrapCrossSigning({ - authUploadDeviceSigningKeys: async (makeRequest): Promise => { - 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, - }); - - // 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); + await accessSecretStorage( + async (): Promise => { + // 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({}); + }, + { forceReset: true, resetCrossSigning: true }, + ); } catch (e) { logger.error(e); this.props.onFinished(false); diff --git a/src/components/views/dialogs/security/RestoreKeyBackupDialog.tsx b/src/components/views/dialogs/security/RestoreKeyBackupDialog.tsx index af84feb848..ec85e72ac9 100644 --- a/src/components/views/dialogs/security/RestoreKeyBackupDialog.tsx +++ b/src/components/views/dialogs/security/RestoreKeyBackupDialog.tsx @@ -109,7 +109,7 @@ export default class RestoreKeyBackupDialog extends React.PureComponent { this.props.onFinished(false); - accessSecretStorage(async (): Promise => {}, /* forceReset = */ true); + accessSecretStorage(async (): Promise => {}, { forceReset: true }); }; /** diff --git a/src/components/views/settings/SecureBackupPanel.tsx b/src/components/views/settings/SecureBackupPanel.tsx index db165eb115..06c67c7d0b 100644 --- a/src/components/views/settings/SecureBackupPanel.tsx +++ b/src/components/views/settings/SecureBackupPanel.tsx @@ -209,7 +209,7 @@ export default class SecureBackupPanel extends React.PureComponent<{}, IState> { private resetSecretStorage = async (): Promise => { this.setState({ error: false }); try { - await accessSecretStorage(async (): Promise => {}, /* forceReset = */ true); + await accessSecretStorage(async (): Promise => {}, { forceReset: true }); } catch (e) { logger.error("Error resetting secret storage", e); if (this.unmounted) return; diff --git a/src/stores/SetupEncryptionStore.ts b/src/stores/SetupEncryptionStore.ts index 5422f68d7b..2fb9c6a9ca 100644 --- a/src/stores/SetupEncryptionStore.ts +++ b/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 => { - const cli = MatrixClientPeg.safeGet(); - await cli.getCrypto()?.bootstrapCrossSigning({ - authUploadDeviceSigningKeys: async (makeRequest): Promise => { - const cachedPassword = SdkContextClass.instance.accountPasswordStore.getPassword(); - - if (cachedPassword) { - await makeRequest({ - 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); + await accessSecretStorage( + async (): Promise => { + this.phase = Phase.Finished; + }, + { + forceReset: true, + resetCrossSigning: true, + accountPassword: SdkContextClass.instance.accountPasswordStore.getPassword(), + }, + ); } catch (e) { logger.error("Error resetting cross-signing", e); this.phase = Phase.Intro; diff --git a/test/unit-tests/SecurityManager-test.ts b/test/unit-tests/SecurityManager-test.ts index 63143d4644..574549d8b2 100644 --- a/test/unit-tests/SecurityManager-test.ts +++ b/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 })); diff --git a/test/unit-tests/components/views/dialogs/AccessSecretStorageDialog-test.tsx b/test/unit-tests/components/views/dialogs/AccessSecretStorageDialog-test.tsx index 30e1151d53..f5b0b1e074 100644 --- a/test/unit-tests/components/views/dialogs/AccessSecretStorageDialog-test.tsx +++ b/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"); + }); }); diff --git a/test/unit-tests/components/views/dialogs/security/CreateSecretStorageDialog-test.tsx b/test/unit-tests/components/views/dialogs/security/CreateSecretStorageDialog-test.tsx index b9d0514148..fa1d74955d 100644 --- a/test/unit-tests/components/views/dialogs/security/CreateSecretStorageDialog-test.tsx +++ b/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"]); + }); }); diff --git a/test/unit-tests/stores/SetupEncryptionStore-test.ts b/test/unit-tests/stores/SetupEncryptionStore-test.ts index 388f1965d7..b9ab29b94b 100644 --- a/test/unit-tests/stores/SetupEncryptionStore-test.ts +++ b/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(makeRequest).toHaveBeenCalledWith({ - identifier: { - type: "m.id.user", - user: "@userId:matrix.org", - }, - password: cachedPassword, - type: "m.login.password", - user: "@userId:matrix.org", + expect(mocked(accessSecretStorage)).toHaveBeenCalledWith(expect.any(Function), { + accountPassword: cachedPassword, + forceReset: true, + resetCrossSigning: true, }); }); });