From 1c2441bc76e46edda0a61eeb805f08b41aa500bb Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Thu, 5 Mar 2026 10:47:50 +0000 Subject: [PATCH] Reset key storage if restoring from Recovery encounters the wrong decryption key (#32668) * Set up the MatrixClient before each RecoveryPanelOutOfSync test Without this, we can't override MatrixClient methods until we've called `renderComponent`, which is awkward. * Actually test that we load the decryption key in RecoveryPanelOutOfSync It turns out the existing test didn't actually go down the expected code path and call loadSessionBackupPrivateKeyFromSecretStorage. * Reset key storage if restoring from Recovery encounters the wrong decryption key Fixes https://github.com/element-hq/element-web/issues/31793 Depends on https://github.com/matrix-org/matrix-js-sdk/pull/5202 When we try to load the key storage decryption key from Recovery, but we find that it does not match the public key of the current key storage backup, create a new key storage backup. --- .../encryption/RecoveryPanelOutOfSync.tsx | 14 +++- apps/web/src/toasts/SetupEncryptionToast.tsx | 14 +++- .../RecoveryPanelOutOfSync-test.tsx | 81 ++++++++++++++++++- .../toasts/SetupEncryptionToast-test.tsx | 79 +++++++++++++++++- 4 files changed, 183 insertions(+), 5 deletions(-) diff --git a/apps/web/src/components/views/settings/encryption/RecoveryPanelOutOfSync.tsx b/apps/web/src/components/views/settings/encryption/RecoveryPanelOutOfSync.tsx index dd06854bbc..3d8dcca428 100644 --- a/apps/web/src/components/views/settings/encryption/RecoveryPanelOutOfSync.tsx +++ b/apps/web/src/components/views/settings/encryption/RecoveryPanelOutOfSync.tsx @@ -9,6 +9,7 @@ import React, { type JSX } from "react"; import { Button } from "@vector-im/compound-web"; import KeyIcon from "@vector-im/compound-design-tokens/assets/web/icons/key"; import { logger } from "matrix-js-sdk/src/logger"; +import { DecryptionKeyDoesNotMatchError } from "matrix-js-sdk/src/crypto-api"; import { SettingsSection } from "../shared/SettingsSection"; import { _t } from "../../../../languageHandler"; @@ -92,7 +93,18 @@ export function RecoveryPanelOutOfSync({ if (needsBackupReset) { await resetKeyBackupAndWait(crypto); } else if (await matrixClient.isKeyBackupKeyStored()) { - await crypto.loadSessionBackupPrivateKeyFromSecretStorage(); + try { + await crypto.loadSessionBackupPrivateKeyFromSecretStorage(); + } catch (error: any) { + if (error instanceof DecryptionKeyDoesNotMatchError) { + logger.error( + "RecoveryPanelOutOfSync: decryption key does not match the public backup. Replacing.", + ); + await resetKeyBackupAndWait(crypto); + } else { + throw error; + } + } } }); }); diff --git a/apps/web/src/toasts/SetupEncryptionToast.tsx b/apps/web/src/toasts/SetupEncryptionToast.tsx index aaeb4b6cde..5e9c970c8d 100644 --- a/apps/web/src/toasts/SetupEncryptionToast.tsx +++ b/apps/web/src/toasts/SetupEncryptionToast.tsx @@ -12,6 +12,7 @@ import { KeyIcon, ErrorSolidIcon, SettingsSolidIcon } from "@vector-im/compound- import { type ComponentType } from "react"; import { type Interaction as InteractionEvent } from "@matrix-org/analytics-events/types/typescript/Interaction"; import { logger } from "matrix-js-sdk/src/logger"; +import { DecryptionKeyDoesNotMatchError } from "matrix-js-sdk/src/crypto-api"; import Modal from "../Modal"; import { _t } from "../languageHandler"; @@ -207,7 +208,18 @@ export const showToast = (state: DeviceStateForToast): void => { if (needsBackupReset) { await resetKeyBackupAndWait(crypto); } else if (await matrixClient.isKeyBackupKeyStored()) { - await crypto.loadSessionBackupPrivateKeyFromSecretStorage(); + try { + await crypto.loadSessionBackupPrivateKeyFromSecretStorage(); + } catch (error: any) { + if (error instanceof DecryptionKeyDoesNotMatchError) { + myLogger.error( + "SetupEncryptionToast: decryption key does not match the public backup. Replacing.", + ); + await resetKeyBackupAndWait(crypto); + } else { + throw error; + } + } } }); }); diff --git a/apps/web/test/unit-tests/components/views/settings/encryption/RecoveryPanelOutOfSync-test.tsx b/apps/web/test/unit-tests/components/views/settings/encryption/RecoveryPanelOutOfSync-test.tsx index 222df74ec1..8839382e8d 100644 --- a/apps/web/test/unit-tests/components/views/settings/encryption/RecoveryPanelOutOfSync-test.tsx +++ b/apps/web/test/unit-tests/components/views/settings/encryption/RecoveryPanelOutOfSync-test.tsx @@ -10,6 +10,8 @@ import { render, screen } from "jest-matrix-react"; import userEvent from "@testing-library/user-event"; import { mocked } from "jest-mock"; import { type MatrixClient } from "matrix-js-sdk/src/matrix"; +import { type SecretStorageKeyDescriptionAesV1 } from "matrix-js-sdk/src/secret-storage"; +import { DecryptionKeyDoesNotMatchError } from "matrix-js-sdk/src/crypto-api"; import { RecoveryPanelOutOfSync } from "../../../../../../src/components/views/settings/encryption/RecoveryPanelOutOfSync"; import { AccessCancelledError, accessSecretStorage } from "../../../../../../src/SecurityManager"; @@ -33,7 +35,6 @@ describe("", () => { onForgotRecoveryKey = jest.fn(), onAccessSecretStorageFailed = jest.fn(), ) { - matrixClient = createTestClient(); return render( ", () => { ); } + beforeEach(() => { + matrixClient = createTestClient(); + }); + afterEach(() => { jest.clearAllMocks(); }); @@ -63,7 +68,7 @@ describe("", () => { expect(onForgotRecoveryKey).toHaveBeenCalled(); }); - it("should access to 4S and call onFinish when 'Enter recovery key' is clicked", async () => { + it("should load backup decryption key and call onFinish when 'Enter recovery key' is clicked", async () => { jest.spyOn(DeviceListener.sharedInstance(), "keyStorageOutOfSyncNeedsBackupReset").mockResolvedValue(false); const user = userEvent.setup(); @@ -71,6 +76,8 @@ describe("", () => { return await func(); }); + mocked(matrixClient.isKeyBackupKeyStored).mockResolvedValue(fakeKeyBackupKey()); + const onFinish = jest.fn(); renderComponent(onFinish); @@ -78,7 +85,9 @@ describe("", () => { expect(accessSecretStorage).toHaveBeenCalled(); expect(onFinish).toHaveBeenCalled(); + expect(matrixClient.isKeyBackupKeyStored).toHaveBeenCalled(); expect(matrixClient.getCrypto()!.resetKeyBackup).not.toHaveBeenCalled(); + expect(matrixClient.getCrypto()!.loadSessionBackupPrivateKeyFromSecretStorage).toHaveBeenCalled(); }); it("should reset key backup if needed", async () => { @@ -99,6 +108,32 @@ describe("", () => { expect(matrixClient.getCrypto()!.resetKeyBackup).toHaveBeenCalled(); }); + it("should reset key backup if decryption key from secret storage does not match backup", async () => { + jest.spyOn(DeviceListener.sharedInstance(), "keyStorageOutOfSyncNeedsBackupReset").mockResolvedValue(false); + + const user = userEvent.setup(); + mocked(accessSecretStorage).mockImplementation(async (func = async (): Promise => {}) => func()); + mocked(matrixClient.isKeyBackupKeyStored).mockResolvedValue(fakeKeyBackupKey()); + + // Given we will fail to load a private key because it doesn't match the + // latest backup public key + mocked(matrixClient.getCrypto()!.loadSessionBackupPrivateKeyFromSecretStorage).mockRejectedValue( + new DecryptionKeyDoesNotMatchError("key no matchy"), + ); + + const onFinish = jest.fn(); + renderComponent(onFinish); + + // When we enter the recovery key + await user.click(screen.getByRole("button", { name: "Enter recovery key" })); + expect(accessSecretStorage).toHaveBeenCalled(); + expect(onFinish).toHaveBeenCalled(); + + // Then we reset backup after attempting to load the key + expect(matrixClient.getCrypto()!.loadSessionBackupPrivateKeyFromSecretStorage).toHaveBeenCalled(); + expect(matrixClient.getCrypto()!.resetKeyBackup).toHaveBeenCalled(); + }); + it("should call onAccessSecretStorageFailed on failure", async () => { jest.spyOn(DeviceListener.sharedInstance(), "keyStorageOutOfSyncNeedsBackupReset").mockResolvedValue(true); @@ -115,6 +150,28 @@ describe("", () => { expect(onAccessSecretStorageFailed).toHaveBeenCalled(); }); + it("should call onAccessSecretStorageFailed when loadSessionBackupPrivateKeyFromSecretStorage fails", async () => { + jest.spyOn(DeviceListener.sharedInstance(), "keyStorageOutOfSyncNeedsBackupReset").mockResolvedValue(false); + + const user = userEvent.setup(); + mocked(accessSecretStorage).mockImplementation(async (func = async (): Promise => {}) => func()); + + // Given we will fail to load a private key because of some unexpected error + mocked(matrixClient.getCrypto()!.loadSessionBackupPrivateKeyFromSecretStorage).mockRejectedValue( + new Error("Unexpected error"), + ); + mocked(matrixClient.isKeyBackupKeyStored).mockResolvedValue(fakeKeyBackupKey()); + + const onAccessSecretStorageFailed = jest.fn(); + renderComponent(jest.fn(), jest.fn(), onAccessSecretStorageFailed); + + // When we enter the recovery key + await user.click(screen.getByRole("button", { name: "Enter recovery key" })); + + // Then we handle the error in onAccessSecretStorageFailed + expect(onAccessSecretStorageFailed).toHaveBeenCalled(); + }); + it("should not call onAccessSecretStorageFailed when cancelled", async () => { jest.spyOn(DeviceListener.sharedInstance(), "keyStorageOutOfSyncNeedsBackupReset").mockResolvedValue(true); @@ -133,3 +190,23 @@ describe("", () => { expect(onAccessSecretStorageFailed).not.toHaveBeenCalled(); }); }); + +/** + * Just enough of a key backup key to persuade RecoveryPanelOutOfSync that we + * don't need to reset backup. + */ +function fakeKeyBackupKey(): Record { + return { + x: { + iv: "x", + mac: "y", + name: "n", + algorithm: "a", + passphrase: { + algorithm: "m.pbkdf2", + iterations: 1, + salt: "s", + }, + }, + }; +} diff --git a/apps/web/test/unit-tests/toasts/SetupEncryptionToast-test.tsx b/apps/web/test/unit-tests/toasts/SetupEncryptionToast-test.tsx index 8bae6689ec..6a589d32ef 100644 --- a/apps/web/test/unit-tests/toasts/SetupEncryptionToast-test.tsx +++ b/apps/web/test/unit-tests/toasts/SetupEncryptionToast-test.tsx @@ -11,7 +11,8 @@ import { act, render, screen } from "jest-matrix-react"; import { mocked, type Mocked } from "jest-mock"; import userEvent from "@testing-library/user-event"; import { type MatrixClient } from "matrix-js-sdk/src/matrix"; -import { type CryptoApi } from "matrix-js-sdk/src/crypto-api"; +import { type CryptoApi, DecryptionKeyDoesNotMatchError } from "matrix-js-sdk/src/crypto-api"; +import { type SecretStorageKeyDescriptionAesV1 } from "matrix-js-sdk/src/secret-storage"; import * as SecurityManager from "../../../src/SecurityManager"; import ToastContainer from "../../../src/components/structures/ToastContainer"; @@ -114,6 +115,30 @@ describe("SetupEncryptionToast", () => { expect(client.getCrypto()!.loadSessionBackupPrivateKeyFromSecretStorage).toHaveBeenCalled(); }); + it("should reset key backup if decryption key does not match backup", async () => { + showToast("key_storage_out_of_sync"); + + const crypto = client.getCrypto()!; + + jest.spyOn(SecurityManager, "accessSecretStorage").mockImplementation(async (func = async (): Promise => {}) => func()); + + // Given we throw when trying to load the backup decrption key + mocked(crypto.loadSessionBackupPrivateKeyFromSecretStorage).mockRejectedValue( + new DecryptionKeyDoesNotMatchError("it key no match"), + ); + + jest.spyOn(DeviceListener.sharedInstance(), "keyStorageOutOfSyncNeedsBackupReset").mockResolvedValue(false); + client.isKeyBackupKeyStored.mockResolvedValue({}); + + // When we enter our recovery key + const user = userEvent.setup(); + await user.click(await screen.findByText("Enter recovery key")); + + // Then we should reset the key backup because we caught the + // DecryptionKeyDoesNotMatchError. + expect(client.getCrypto()!.resetKeyBackup).toHaveBeenCalled(); + }); + it("should open settings to the reset flow when 'forgot recovery key' clicked and identity reset needed", async () => { act(() => showToast("key_storage_out_of_sync")); @@ -190,6 +215,38 @@ describe("SetupEncryptionToast", () => { }); }); + it("should go to change recovery key when recovering fails inside loadSessionBackup...", async () => { + jest.spyOn(SecurityManager, "accessSecretStorage").mockImplementation(async (func = async (): Promise => {}) => func()); + + jest.spyOn(DeviceListener.sharedInstance(), "keyStorageOutOfSyncNeedsCrossSigningReset").mockResolvedValue( + true, + ); + jest.spyOn(DeviceListener.sharedInstance(), "keyStorageOutOfSyncNeedsBackupReset").mockResolvedValue(false); + mocked(client.isKeyBackupKeyStored).mockResolvedValue(fakeKeyBackupKey()); + + // Given when we try to load from backup we get an unexpected error + mocked(client.getCrypto()!.loadSessionBackupPrivateKeyFromSecretStorage).mockRejectedValue( + new Error("Unexpected error"), + ); + + act(() => showToast("key_storage_out_of_sync")); + + // When we start to entry recovery key + const user = userEvent.setup(); + await user.click(await screen.findByText("Enter recovery key")); + + // Then we handle the error and jump to the Encryption Settings. + // + // Note: It seems reasonable to ask the user to reset their identity + // in this case, but we're not actually sure what happened, so it + // may not be the right response. + expect(dis.dispatch).toHaveBeenCalledWith({ + action: "view_user_settings", + initialTabId: "USER_ENCRYPTION_TAB", + props: { initialEncryptionState: "reset_identity_sync_failed" }, + }); + }); + it("should dismiss the toast when the close button is clicked", async () => { jest.spyOn(DeviceListener.sharedInstance(), "dismissEncryptionSetup"); @@ -200,6 +257,26 @@ describe("SetupEncryptionToast", () => { expect(DeviceListener.sharedInstance().dismissEncryptionSetup).toHaveBeenCalled(); }); + + /** + * Just enough of a key backup key to persuade SetupEncryptionToast that we + * don't need to reset backup. + */ + function fakeKeyBackupKey(): Record { + return { + x: { + iv: "x", + mac: "y", + name: "n", + algorithm: "a", + passphrase: { + algorithm: "m.pbkdf2", + iterations: 1, + salt: "s", + }, + }, + }; + } }); describe("Turn on key storage", () => {