diff --git a/playwright/e2e/crypto/backups.spec.ts b/playwright/e2e/crypto/backups.spec.ts index 95bf708122e..8209dedceeb 100644 --- a/playwright/e2e/crypto/backups.spec.ts +++ b/playwright/e2e/crypto/backups.spec.ts @@ -10,6 +10,7 @@ import { type Page } from "@playwright/test"; import { test, expect } from "../../element-web-test"; import { isDendrite } from "../../plugins/homeserver/dendrite"; +import { completeCreateSecretStorageDialog } from "./utils.ts"; async function expectBackupVersionToBe(page: Page, version: string) { await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(5) td")).toHaveText( @@ -35,19 +36,7 @@ test.describe("Backups", () => { await expect(securityTab.getByRole("heading", { name: "Secure Backup" })).toBeVisible(); await securityTab.getByRole("button", { name: "Set up", exact: true }).click(); - const currentDialogLocator = page.locator(".mx_Dialog"); - - // It's the first time and secure storage is not set up, so it will create one - await expect(currentDialogLocator.getByRole("heading", { name: "Set up Secure Backup" })).toBeVisible(); - await currentDialogLocator.getByRole("button", { name: "Continue", exact: true }).click(); - await expect(currentDialogLocator.getByRole("heading", { name: "Save your Security Key" })).toBeVisible(); - await currentDialogLocator.getByRole("button", { name: "Copy", exact: true }).click(); - // copy the recovery key to use it later - const securityKey = await app.getClipboard(); - await currentDialogLocator.getByRole("button", { name: "Continue", exact: true }).click(); - - await expect(currentDialogLocator.getByRole("heading", { name: "Secure Backup successful" })).toBeVisible(); - await currentDialogLocator.getByRole("button", { name: "Done", exact: true }).click(); + const securityKey = await completeCreateSecretStorageDialog(page); // Open the settings again await app.settings.openUserSettings("Security & Privacy"); @@ -62,6 +51,7 @@ test.describe("Backups", () => { await expectBackupVersionToBe(page, "1"); await securityTab.getByRole("button", { name: "Delete Backup", exact: true }).click(); + const currentDialogLocator = page.locator(".mx_Dialog"); await expect(currentDialogLocator.getByRole("heading", { name: "Delete Backup" })).toBeVisible(); // Delete it await currentDialogLocator.getByTestId("dialog-primary-button").click(); // Click "Delete Backup" diff --git a/playwright/e2e/crypto/crypto.spec.ts b/playwright/e2e/crypto/crypto.spec.ts index f99a7a64588..babee2aeeab 100644 --- a/playwright/e2e/crypto/crypto.spec.ts +++ b/playwright/e2e/crypto/crypto.spec.ts @@ -8,7 +8,14 @@ Please see LICENSE files in the repository root for full details. import type { Page } from "@playwright/test"; import { expect, test } from "../../element-web-test"; -import { autoJoin, copyAndContinue, createSharedRoomWithUser, enableKeyBackup, verify } from "./utils"; +import { + autoJoin, + completeCreateSecretStorageDialog, + copyAndContinue, + createSharedRoomWithUser, + enableKeyBackup, + verify, +} from "./utils"; import { Bot } from "../../pages/bot"; import { ElementAppPage } from "../../pages/ElementAppPage"; import { isDendrite } from "../../plugins/homeserver/dendrite"; @@ -111,18 +118,7 @@ test.describe("Cryptography", function () { await app.settings.openUserSettings("Security & Privacy"); await page.getByRole("button", { name: "Set up Secure Backup" }).click(); - const dialog = page.locator(".mx_Dialog"); - // Recovery key is selected by default - await dialog.getByRole("button", { name: "Continue" }).click(); - await copyAndContinue(page); - - // If the device is unverified, there should be a "Setting up keys" step; however, it - // can be quite quick, and playwright can miss it, so we can't test for it. - - // Either way, we end up at a success dialog: - await expect(dialog.getByText("Secure Backup successful")).toBeVisible(); - await dialog.getByRole("button", { name: "Done" }).click(); - await expect(dialog.getByText("Secure Backup successful")).not.toBeVisible(); + await completeCreateSecretStorageDialog(page); // Verify that the SSSS keys are in the account data stored in the server await verifyKey(app, "master"); diff --git a/playwright/e2e/crypto/dehydration.spec.ts b/playwright/e2e/crypto/dehydration.spec.ts index 9beb0539328..b3ac82a19da 100644 --- a/playwright/e2e/crypto/dehydration.spec.ts +++ b/playwright/e2e/crypto/dehydration.spec.ts @@ -11,6 +11,8 @@ import { Locator, type Page } from "@playwright/test"; import { test, expect } from "../../element-web-test"; import { viewRoomSummaryByName } from "../right-panel/utils"; import { isDendrite } from "../../plugins/homeserver/dendrite"; +import { completeCreateSecretStorageDialog, createBot, logIntoElement } from "./utils.ts"; +import { Client } from "../../pages/client.ts"; const ROOM_NAME = "Test room"; const NAME = "Alice"; @@ -44,7 +46,7 @@ test.use({ test.describe("Dehydration", () => { test.skip(isDendrite, "does not yet support dehydration v2"); - test("Create dehydrated device", async ({ page, user, app }, workerInfo) => { + test("'Set up secure backup' creates dehydrated device", async ({ page, user, app }, workerInfo) => { // Create a backup (which will create SSSS, and dehydrated device) const securityTab = await app.settings.openUserSettings("Security & Privacy"); @@ -53,17 +55,7 @@ test.describe("Dehydration", () => { await expect(securityTab.getByText("Offline device enabled")).not.toBeVisible(); await securityTab.getByRole("button", { name: "Set up", exact: true }).click(); - const currentDialogLocator = page.locator(".mx_Dialog"); - - // It's the first time and secure storage is not set up, so it will create one - await expect(currentDialogLocator.getByRole("heading", { name: "Set up Secure Backup" })).toBeVisible(); - await currentDialogLocator.getByRole("button", { name: "Continue", exact: true }).click(); - await expect(currentDialogLocator.getByRole("heading", { name: "Save your Security Key" })).toBeVisible(); - await currentDialogLocator.getByRole("button", { name: "Copy", exact: true }).click(); - await currentDialogLocator.getByRole("button", { name: "Continue", exact: true }).click(); - - await expect(currentDialogLocator.getByRole("heading", { name: "Secure Backup successful" })).toBeVisible(); - await currentDialogLocator.getByRole("button", { name: "Done", exact: true }).click(); + await completeCreateSecretStorageDialog(page); // Open the settings again await app.settings.openUserSettings("Security & Privacy"); @@ -96,4 +88,49 @@ test.describe("Dehydration", () => { await expect(page.locator(".mx_UserInfo_devices").getByText("Offline device enabled")).toBeVisible(); await expect(page.locator(".mx_UserInfo_devices").getByText("Dehydrated device")).not.toBeVisible(); }); + + test("Reset recovery key during login re-creates dehydrated device", async ({ + page, + homeserver, + app, + credentials, + }) => { + // Set up cross-signing and recovery + const { botClient } = await createBot(page, homeserver, credentials); + // ... and dehydration + await botClient.evaluate(async (client) => await client.getCrypto().startDehydration()); + + const initialDehydratedDeviceIds = await getDehydratedDeviceIds(botClient); + expect(initialDehydratedDeviceIds.length).toBe(1); + + await botClient.evaluate(async (client) => client.stopClient()); + + // Log in our client + await logIntoElement(page, credentials); + + // Oh no, we forgot our recovery key + await page.locator(".mx_AuthPage").getByRole("button", { name: "Reset all" }).click(); + await page.locator(".mx_AuthPage").getByRole("button", { name: "Proceed with reset" }).click(); + + await completeCreateSecretStorageDialog(page, { accountPassword: credentials.password }); + + // There should be a brand new dehydrated device + const dehydratedDeviceIds = await getDehydratedDeviceIds(app.client); + expect(dehydratedDeviceIds.length).toBe(1); + expect(dehydratedDeviceIds[0]).not.toEqual(initialDehydratedDeviceIds[0]); + }); }); + +async function getDehydratedDeviceIds(client: Client): Promise { + return await client.evaluate(async (client) => { + const userId = client.getUserId(); + const devices = await client.getCrypto().getUserDeviceInfo([userId]); + return Array.from( + devices + .get(userId) + .values() + .filter((d) => d.dehydrated) + .map((d) => d.deviceId), + ); + }); +} diff --git a/playwright/e2e/crypto/utils.ts b/playwright/e2e/crypto/utils.ts index 2c8fb7d3c1f..6753ae651c3 100644 --- a/playwright/e2e/crypto/utils.ts +++ b/playwright/e2e/crypto/utils.ts @@ -288,19 +288,52 @@ export async function doTwoWaySasVerification(page: Page, verifier: JSHandle { await app.settings.openUserSettings("Security & Privacy"); await app.page.getByRole("button", { name: "Set up Secure Backup" }).click(); - const dialog = app.page.locator(".mx_Dialog"); - // Recovery key is selected by default - await dialog.getByRole("button", { name: "Continue" }).click({ timeout: 60000 }); - // copy the text ourselves - const securityKey = await dialog.locator(".mx_CreateSecretStorageDialog_recoveryKey code").textContent(); - await copyAndContinue(app.page); + return await completeCreateSecretStorageDialog(app.page); +} + +/** + * Go through the "Set up Secure Backup" dialog (aka the `CreateSecretStorageDialog`). + * + * Assumes the dialog is already open for some reason (see also {@link enableKeyBackup}). + * + * @param page - The playwright `Page` fixture. + * @param opts - Options object + * @param opts.accountPassword - The user's account password. If we are also resetting cross-signing, then we will need + * to upload the public cross-signing keys, which will cause the app to prompt for the password. + * + * @returns the new recovery key. + */ +export async function completeCreateSecretStorageDialog( + page: Page, + opts?: { accountPassword?: string }, +): Promise { + const currentDialogLocator = page.locator(".mx_Dialog"); + + await expect(currentDialogLocator.getByRole("heading", { name: "Set up Secure Backup" })).toBeVisible(); + // "Generate a Security Key" is selected by default + await currentDialogLocator.getByRole("button", { name: "Continue", exact: true }).click(); + await expect(currentDialogLocator.getByRole("heading", { name: "Save your Security Key" })).toBeVisible(); + await currentDialogLocator.getByRole("button", { name: "Copy", exact: true }).click(); + // copy the recovery key to use it later + const recoveryKey = await page.evaluate(() => navigator.clipboard.readText()); + await currentDialogLocator.getByRole("button", { name: "Continue", exact: true }).click(); + + // If the device is unverified, there should be a "Setting up keys" step. + // If this is not the first time we are setting up cross-signing, the app will prompt for our password; otherwise + // the step is quite quick, and playwright can miss it, so we can't test for it. + if (opts && Object.hasOwn(opts, "accountPassword")) { + await expect(currentDialogLocator.getByRole("heading", { name: "Setting up keys" })).toBeVisible(); + await page.getByPlaceholder("Password").fill(opts!.accountPassword); + await currentDialogLocator.getByRole("button", { name: "Continue" }).click(); + } - await expect(dialog.getByText("Secure Backup successful")).toBeVisible(); - await dialog.getByRole("button", { name: "Done" }).click(); - await expect(dialog.getByText("Secure Backup successful")).not.toBeVisible(); + // Either way, we end up at a success dialog: + await expect(currentDialogLocator.getByRole("heading", { name: "Secure Backup successful" })).toBeVisible(); + await currentDialogLocator.getByRole("button", { name: "Done", exact: true }).click(); + await expect(currentDialogLocator.getByText("Secure Backup successful")).not.toBeVisible(); - return securityKey; + return recoveryKey; } /** diff --git a/src/SecurityManager.ts b/src/SecurityManager.ts index 6af8dd5f184..370d0e94532 100644 --- a/src/SecurityManager.ts +++ b/src/SecurityManager.ts @@ -9,7 +9,7 @@ Please see LICENSE files in the repository root for full details. import { lazy } from "react"; import { SecretStorage } from "matrix-js-sdk/src/matrix"; import { deriveRecoveryKeyFromPassphrase, decodeRecoveryKey, CryptoCallbacks } from "matrix-js-sdk/src/crypto-api"; -import { logger } from "matrix-js-sdk/src/logger"; +import { logger as rootLogger } from "matrix-js-sdk/src/logger"; import Modal from "./Modal"; import { MatrixClientPeg } from "./MatrixClientPeg"; @@ -29,6 +29,8 @@ let secretStorageKeys: Record = {}; let secretStorageKeyInfo: Record = {}; let secretStorageBeingAccessed = false; +const logger = rootLogger.getChild("SecurityManager:"); + /** * This can be used by other components to check if secret storage access is in * progress, so that we can e.g. avoid intermittently showing toasts during @@ -70,33 +72,34 @@ function makeInputToKey( }; } -async function getSecretStorageKey({ - keys: keyInfos, -}: { - keys: Record; -}): Promise<[string, Uint8Array]> { +async function getSecretStorageKey( + { + keys: keyInfos, + }: { + keys: Record; + }, + secretName: string, +): Promise<[string, Uint8Array]> { const cli = MatrixClientPeg.safeGet(); - let keyId = await cli.secretStorage.getDefaultKeyId(); - let keyInfo!: SecretStorage.SecretStorageKeyDescription; - if (keyId) { - // use the default SSSS key if set - keyInfo = keyInfos[keyId]; - if (!keyInfo) { - // if the default key is not available, pretend the default key - // isn't set - keyId = null; - } - } - if (!keyId) { - // if no default SSSS key is set, fall back to a heuristic of using the + const defaultKeyId = await cli.secretStorage.getDefaultKeyId(); + + let keyId: string; + // If the defaultKey is useful, use that + if (defaultKeyId && keyInfos[defaultKeyId]) { + keyId = defaultKeyId; + } else { + // Fall back to a heuristic of using the // only available key, if only one key is set - const keyInfoEntries = Object.entries(keyInfos); - if (keyInfoEntries.length > 1) { + const usefulKeys = Object.keys(keyInfos); + if (usefulKeys.length > 1) { throw new Error("Multiple storage key requests not implemented"); } - [keyId, keyInfo] = keyInfoEntries[0]; + keyId = usefulKeys[0]; } - logger.debug(`getSecretStorageKey: request for 4S keys [${Object.keys(keyInfos)}]: looking for key ${keyId}`); + const keyInfo = keyInfos[keyId]; + logger.debug( + `getSecretStorageKey: request for 4S keys [${Object.keys(keyInfos)}] for secret \`${secretName}\`: looking for key ${keyId}`, + ); // Check the in-memory cache if (secretStorageBeingAccessed && secretStorageKeys[keyId]) { @@ -106,12 +109,18 @@ async function getSecretStorageKey({ const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup.getSecretStorageKey(); if (keyFromCustomisations) { - logger.log("getSecretStorageKey: Using secret storage key from CryptoSetupExtension"); + logger.debug("getSecretStorageKey: Using secret storage key from CryptoSetupExtension"); cacheSecretStorageKey(keyId, keyInfo, keyFromCustomisations); return [keyId, keyFromCustomisations]; } - logger.debug("getSecretStorageKey: prompting user for key"); + // We only prompt the user for the default key + if (keyId !== defaultKeyId) { + logger.debug(`getSecretStorageKey: request for non-default key ${keyId}: not prompting user`); + throw new Error("Request for non-default 4S key"); + } + + logger.debug(`getSecretStorageKey: prompting user for key ${keyId}`); const inputToKey = makeInputToKey(keyInfo); const { finished } = Modal.createDialog( AccessSecretStorageDialog, @@ -139,7 +148,7 @@ async function getSecretStorageKey({ if (!keyParams) { throw new AccessCancelledError(); } - logger.debug("getSecretStorageKey: got key from user"); + logger.debug(`getSecretStorageKey: got key ${keyId} from user`); const key = await inputToKey(keyParams); // Save to cache to avoid future prompts in the current session @@ -154,6 +163,7 @@ function cacheSecretStorageKey( key: Uint8Array, ): void { if (secretStorageBeingAccessed) { + logger.debug(`Caching 4S key ${keyId}`); secretStorageKeys[keyId] = key; secretStorageKeyInfo[keyId] = keyInfo; } @@ -173,13 +183,13 @@ export const crossSigningCallbacks: CryptoCallbacks = { * @param func - The operation to be wrapped. */ export async function withSecretStorageKeyCache(func: () => Promise): Promise { - logger.debug("SecurityManager: enabling 4S key cache"); + logger.debug("enabling 4S key cache"); secretStorageBeingAccessed = true; try { return await func(); } finally { // Clear secret storage key cache now that work is complete - logger.debug("SecurityManager: disabling 4S key cache"); + logger.debug("disabling 4S key cache"); secretStorageBeingAccessed = false; secretStorageKeys = {}; secretStorageKeyInfo = {}; diff --git a/test/unit-tests/SecurityManager-test.ts b/test/unit-tests/SecurityManager-test.ts index 4575223a50c..f81e08ada24 100644 --- a/test/unit-tests/SecurityManager-test.ts +++ b/test/unit-tests/SecurityManager-test.ts @@ -7,11 +7,18 @@ Please see LICENSE files in the repository root for full details. */ import { mocked } from "jest-mock"; -import { CryptoApi } from "matrix-js-sdk/src/crypto-api"; +import { act } from "react"; +import { Crypto } from "@peculiar/webcrypto"; +import { CryptoApi, deriveRecoveryKeyFromPassphrase } from "matrix-js-sdk/src/crypto-api"; +import { SecretStorage } from "matrix-js-sdk/src/matrix"; -import { accessSecretStorage } from "../../src/SecurityManager"; +import { accessSecretStorage, crossSigningCallbacks } from "../../src/SecurityManager"; import { filterConsole, stubClient } from "../test-utils"; import Modal from "../../src/Modal.tsx"; +import { + default as AccessSecretStorageDialog, + KeyParams, +} from "../../src/components/views/dialogs/security/AccessSecretStorageDialog.tsx"; jest.mock("react", () => { const React = jest.requireActual("react"); @@ -19,6 +26,10 @@ jest.mock("react", () => { return React; }); +afterEach(() => { + jest.restoreAllMocks(); +}); + describe("SecurityManager", () => { describe("accessSecretStorage", () => { filterConsole("Not setting dehydration key: no SSSS key found"); @@ -74,4 +85,81 @@ describe("SecurityManager", () => { await expect(spy.mock.lastCall![0]).resolves.toEqual(expect.objectContaining({ __test: true })); }); }); + + describe("getSecretStorageKey", () => { + const { getSecretStorageKey } = crossSigningCallbacks; + + /** Polyfill crypto.subtle, which is unavailable in jsdom */ + function polyFillSubtleCrypto() { + Object.defineProperty(globalThis.crypto, "subtle", { value: new Crypto().subtle }); + } + + it("should prompt the user if the key is uncached", async () => { + polyFillSubtleCrypto(); + + const client = stubClient(); + mocked(client.secretStorage.getDefaultKeyId).mockResolvedValue("my_default_key"); + + const passphrase = "s3cret"; + const { recoveryKey, keyInfo } = await deriveKeyFromPassphrase(passphrase); + + jest.spyOn(Modal, "createDialog").mockImplementation((component) => { + expect(component).toBe(AccessSecretStorageDialog); + + const modalFunc = async () => [{ passphrase }] as [KeyParams]; + return { + finished: modalFunc(), + close: () => {}, + }; + }); + + const [keyId, key] = (await act(() => + getSecretStorageKey!({ keys: { my_default_key: keyInfo } }, "my_secret"), + ))!; + expect(keyId).toEqual("my_default_key"); + expect(key).toEqual(recoveryKey); + }); + + it("should not prompt the user if the requested key is not the default", async () => { + const client = stubClient(); + mocked(client.secretStorage.getDefaultKeyId).mockResolvedValue("my_default_key"); + const createDialogSpy = jest.spyOn(Modal, "createDialog"); + + await expect( + act(() => + getSecretStorageKey!( + { keys: { other_key: {} as SecretStorage.SecretStorageKeyDescription } }, + "my_secret", + ), + ), + ).rejects.toThrow("Request for non-default 4S key"); + expect(createDialogSpy).not.toHaveBeenCalled(); + }); + }); }); + +/** Derive a key from a passphrase, also returning the KeyInfo */ +async function deriveKeyFromPassphrase( + passphrase: string, +): Promise<{ recoveryKey: Uint8Array; keyInfo: SecretStorage.SecretStorageKeyDescription }> { + const salt = "SALTYGOODNESS"; + const iterations = 1000; + + const recoveryKey = await deriveRecoveryKeyFromPassphrase(passphrase, salt, iterations); + + const check = await SecretStorage.calculateKeyCheck(recoveryKey); + return { + recoveryKey, + keyInfo: { + iv: check.iv, + mac: check.mac, + algorithm: SecretStorage.SECRET_STORAGE_ALGORITHM_V1_AES, + name: "", + passphrase: { + algorithm: "m.pbkdf2", + iterations, + salt, + }, + }, + }; +}