From 2c7247713a846258210f8904937cd78ce6d682a0 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 22 Oct 2024 14:58:20 +0100 Subject: [PATCH 1/8] Enable key backup by default When we set up cross signing, so the key backup key will be stored locally along with the cross signing keys until the user sets up recovery (4s). This will mean that a user can restore their backup if they log in on a new device as long as they verify with the one they registered on. --- src/components/structures/auth/E2eSetup.tsx | 4 ++-- ...gningDialog.tsx => InitialCryptoSetup.tsx} | 24 ++++++++++++------- 2 files changed, 17 insertions(+), 11 deletions(-) rename src/components/views/dialogs/security/{CreateCrossSigningDialog.tsx => InitialCryptoSetup.tsx} (79%) diff --git a/src/components/structures/auth/E2eSetup.tsx b/src/components/structures/auth/E2eSetup.tsx index 80a135fe19..d2fb046fe7 100644 --- a/src/components/structures/auth/E2eSetup.tsx +++ b/src/components/structures/auth/E2eSetup.tsx @@ -11,7 +11,7 @@ import { MatrixClient } from "matrix-js-sdk/src/matrix"; import AuthPage from "../../views/auth/AuthPage"; import CompleteSecurityBody from "../../views/auth/CompleteSecurityBody"; -import CreateCrossSigningDialog from "../../views/dialogs/security/CreateCrossSigningDialog"; +import InitialCryptoSetup from "../../views/dialogs/security/InitialCryptoSetup"; interface IProps { matrixClient: MatrixClient; @@ -25,7 +25,7 @@ export default class E2eSetup extends React.Component { return ( - = ({ matrixClient, accountPassword, tokenLogin, onFinished }) => { +const InitialCryptoSetup: React.FC = ({ matrixClient, accountPassword, tokenLogin, onFinished }) => { const [error, setError] = useState(false); - const bootstrapCrossSigning = useCallback(async () => { + const doSetup = useCallback(async () => { const cryptoApi = matrixClient.getCrypto(); if (!cryptoApi) return; @@ -40,6 +40,12 @@ const CreateCrossSigningDialog: React.FC = ({ matrixClient, accountPasswo try { await createCrossSigning(matrixClient, tokenLogin, accountPassword); + + const backupInfo = await matrixClient.getKeyBackupVersion(); + if (backupInfo === null) { + await cryptoApi.resetKeyBackup(); + } + onFinished(true); } catch (e) { if (tokenLogin) { @@ -58,8 +64,8 @@ const CreateCrossSigningDialog: React.FC = ({ matrixClient, accountPasswo }, [onFinished]); useEffect(() => { - bootstrapCrossSigning(); - }, [bootstrapCrossSigning]); + doSetup(); + }, [doSetup]); let content; if (error) { @@ -69,7 +75,7 @@ const CreateCrossSigningDialog: React.FC = ({ matrixClient, accountPasswo
@@ -96,4 +102,4 @@ const CreateCrossSigningDialog: React.FC = ({ matrixClient, accountPasswo ); }; -export default CreateCrossSigningDialog; +export default InitialCryptoSetup; From 7f10cd6a9e9ca71f371565496c1146ac4cdf560f Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 22 Oct 2024 15:36:04 +0100 Subject: [PATCH 2/8] Fix name & tests --- src/components/structures/auth/E2eSetup.tsx | 4 ++-- ...ryptoSetup.tsx => InitialCryptoSetupDialog.tsx} | 4 ++-- ...-test.tsx => InitialCryptoSetupDialog-test.tsx} | 14 +++++++------- 3 files changed, 11 insertions(+), 11 deletions(-) rename src/components/views/dialogs/security/{InitialCryptoSetup.tsx => InitialCryptoSetupDialog.tsx} (95%) rename test/components/views/dialogs/security/{CreateCrossSigningDialog-test.tsx => InitialCryptoSetupDialog-test.tsx} (91%) diff --git a/src/components/structures/auth/E2eSetup.tsx b/src/components/structures/auth/E2eSetup.tsx index d2fb046fe7..4ba7be6e5a 100644 --- a/src/components/structures/auth/E2eSetup.tsx +++ b/src/components/structures/auth/E2eSetup.tsx @@ -11,7 +11,7 @@ import { MatrixClient } from "matrix-js-sdk/src/matrix"; import AuthPage from "../../views/auth/AuthPage"; import CompleteSecurityBody from "../../views/auth/CompleteSecurityBody"; -import InitialCryptoSetup from "../../views/dialogs/security/InitialCryptoSetup"; +import InitialCryptoSetupDialog from "../../views/dialogs/security/InitialCryptoSetup"; interface IProps { matrixClient: MatrixClient; @@ -25,7 +25,7 @@ export default class E2eSetup extends React.Component { return ( - = ({ matrixClient, accountPassword, tokenLogin, onFinished }) => { +const InitialCryptoSetupDialog: React.FC = ({ matrixClient, accountPassword, tokenLogin, onFinished }) => { const [error, setError] = useState(false); const doSetup = useCallback(async () => { @@ -102,4 +102,4 @@ const InitialCryptoSetup: React.FC = ({ matrixClient, accountPassword, to ); }; -export default InitialCryptoSetup; +export default InitialCryptoSetupDialog; diff --git a/test/components/views/dialogs/security/CreateCrossSigningDialog-test.tsx b/test/components/views/dialogs/security/InitialCryptoSetupDialog-test.tsx similarity index 91% rename from test/components/views/dialogs/security/CreateCrossSigningDialog-test.tsx rename to test/components/views/dialogs/security/InitialCryptoSetupDialog-test.tsx index 3e5dc4eb94..199174a634 100644 --- a/test/components/views/dialogs/security/CreateCrossSigningDialog-test.tsx +++ b/test/components/views/dialogs/security/InitialCryptoSetupDialog-test.tsx @@ -12,14 +12,14 @@ import { mocked } from "jest-mock"; import { MatrixClient } from "matrix-js-sdk/src/matrix"; import { createCrossSigning } from "../../../../../src/CreateCrossSigning"; -import CreateCrossSigningDialog from "../../../../../src/components/views/dialogs/security/CreateCrossSigningDialog"; +import InitialCryptoSetupDialog from "../../../../../src/components/views/dialogs/security/InitialCryptoSetupDialog"; import { createTestClient } from "../../../../test-utils"; jest.mock("../../../../../src/CreateCrossSigning", () => ({ createCrossSigning: jest.fn(), })); -describe("CreateCrossSigningDialog", () => { +describe("InitialCryptoSetupDialog", () => { let client: MatrixClient; let createCrossSigningResolve: () => void; let createCrossSigningReject: (e: Error) => void; @@ -43,7 +43,7 @@ describe("CreateCrossSigningDialog", () => { const onFinished = jest.fn(); render( - { it("should display an error if createCrossSigning fails", async () => { render( - { const onFinished = jest.fn(); render( - { const onFinished = jest.fn(); render( - { it("should retry when the retry button is clicked", async () => { render( - Date: Tue, 22 Oct 2024 15:40:39 +0100 Subject: [PATCH 3/8] Fix import --- src/components/structures/auth/E2eSetup.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/auth/E2eSetup.tsx b/src/components/structures/auth/E2eSetup.tsx index 4ba7be6e5a..4063ae0252 100644 --- a/src/components/structures/auth/E2eSetup.tsx +++ b/src/components/structures/auth/E2eSetup.tsx @@ -11,7 +11,7 @@ import { MatrixClient } from "matrix-js-sdk/src/matrix"; import AuthPage from "../../views/auth/AuthPage"; import CompleteSecurityBody from "../../views/auth/CompleteSecurityBody"; -import InitialCryptoSetupDialog from "../../views/dialogs/security/InitialCryptoSetup"; +import InitialCryptoSetupDialog from "../../views/dialogs/security/InitialCryptoSetupDialog"; interface IProps { matrixClient: MatrixClient; From 6a912e28be76073661704a39dd911ed7ccdf2865 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 22 Oct 2024 16:03:26 +0100 Subject: [PATCH 4/8] Fix test --- test/unit-tests/components/structures/MatrixChat-test.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/unit-tests/components/structures/MatrixChat-test.tsx b/test/unit-tests/components/structures/MatrixChat-test.tsx index 7f565d682f..c042fd1b3b 100644 --- a/test/unit-tests/components/structures/MatrixChat-test.tsx +++ b/test/unit-tests/components/structures/MatrixChat-test.tsx @@ -148,6 +148,7 @@ describe("", () => { isRoomEncrypted: jest.fn(), logout: jest.fn(), getDeviceId: jest.fn(), + getKeyBackupVersion: jest.fn().mockResolvedValue(null), }); let mockClient: Mocked; const serverConfig = { @@ -1009,6 +1010,7 @@ describe("", () => { userHasCrossSigningKeys: jest.fn().mockResolvedValue(false), // This needs to not finish immediately because we need to test the screen appears bootstrapCrossSigning: jest.fn().mockImplementation(() => bootstrapDeferred.promise), + resetKeyBackup: jest.fn(), }; loginClient.getCrypto.mockReturnValue(mockCrypto as any); }); From b8f6bd1664d70e3722a42464faa31fa60622d5ac Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 2 Dec 2024 14:44:00 +0000 Subject: [PATCH 5/8] Secure backup prompt --- src/DeviceListener.ts | 28 ++++++++++++++-------- src/i18n/strings/en_EN.json | 3 +++ src/toasts/SetupEncryptionToast.ts | 38 +++++++++++++++++++++++++++--- 3 files changed, 56 insertions(+), 13 deletions(-) diff --git a/src/DeviceListener.ts b/src/DeviceListener.ts index 4f47cd7eac..e80c9402b2 100644 --- a/src/DeviceListener.ts +++ b/src/DeviceListener.ts @@ -292,21 +292,29 @@ export default class DeviceListener { await crypto.getUserDeviceInfo([cli.getSafeUserId()]); // cross signing isn't enabled - nag to enable it - // There are 2 different toasts for: + // There are 3 different toasts for: if (!(await crypto.getCrossSigningKeyId()) && (await crypto.userHasCrossSigningKeys())) { - // Cross-signing on account but this device doesn't trust the master key (verify this session) + // Toast 1. Cross-signing on account but this device doesn't trust the master key (verify this session) showSetupEncryptionToast(SetupKind.VERIFY_THIS_SESSION); this.checkKeyBackupStatus(); } else { - // No cross-signing or key backup on account (set up encryption) - await cli.waitForClientWellKnown(); - if (isSecureBackupRequired(cli) && isLoggedIn()) { - // If we're meant to set up, and Secure Backup is required, - // trigger the flow directly without a toast once logged in. - hideSetupEncryptionToast(); - accessSecretStorage(); + const backupInfo = await this.getKeyBackupInfo(); + if (backupInfo) { + // Toast 2: Key backup is enabled but recovery (4S) is not set up: prompt user to set up recovery. + // Since we now enable key backup at registration time, this will be the common case for + // new users. + showSetupEncryptionToast(SetupKind.SET_UP_RECOVERY); } else { - showSetupEncryptionToast(SetupKind.SET_UP_ENCRYPTION); + // Toast 3: No cross-signing or key backup on account (set up encryption) + await cli.waitForClientWellKnown(); + if (isSecureBackupRequired(cli) && isLoggedIn()) { + // If we're meant to set up, and Secure Backup is required, + // trigger the flow directly without a toast once logged in. + hideSetupEncryptionToast(); + accessSecretStorage(); + } else { + showSetupEncryptionToast(SetupKind.SET_UP_ENCRYPTION); + } } } } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 3b4765b0ad..807264c4d2 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -912,6 +912,9 @@ "warning": "If you didn't remove the recovery method, an attacker may be trying to access your account. Change your account password and set a new recovery method immediately in Settings." }, "reset_all_button": "Forgotten or lost all recovery methods? Reset all", + "set_up_recovery": "Set up recovery", + "set_up_recovery_later": "Not now", + "set_up_recovery_toast_title": "Set up recovery to protect your account", "set_up_toast_description": "Safeguard against losing access to encrypted messages & data", "set_up_toast_title": "Set up Secure Backup", "setup_secure_backup": { diff --git a/src/toasts/SetupEncryptionToast.ts b/src/toasts/SetupEncryptionToast.ts index 0dd54bb18f..8ae4830242 100644 --- a/src/toasts/SetupEncryptionToast.ts +++ b/src/toasts/SetupEncryptionToast.ts @@ -6,6 +6,9 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only Please see LICENSE files in the repository root for full details. */ +import KeyboardIcon from "@vector-im/compound-design-tokens/assets/web/icons/key"; +import { ComponentType } from "react"; + import Modal from "../Modal"; import { _t } from "../languageHandler"; import DeviceListener from "../DeviceListener"; @@ -23,33 +26,61 @@ const getTitle = (kind: Kind): string => { switch (kind) { case Kind.SET_UP_ENCRYPTION: return _t("encryption|set_up_toast_title"); + case Kind.SET_UP_RECOVERY: + return _t("encryption|set_up_recovery_toast_title"); case Kind.VERIFY_THIS_SESSION: return _t("encryption|verify_toast_title"); } }; -const getIcon = (kind: Kind): string => { +const getIcon = (kind: Kind): string | undefined => { switch (kind) { case Kind.SET_UP_ENCRYPTION: return "secure_backup"; + case Kind.SET_UP_RECOVERY: + return undefined; case Kind.VERIFY_THIS_SESSION: return "verification_warning"; } }; +// Gets the icon displayed on the prinary button +const getPrimaryIcon = (kind: Kind): ComponentType> | undefined => { + switch (kind) { + case Kind.SET_UP_RECOVERY: + return KeyboardIcon; + default: + return undefined; + } +}; + const getSetupCaption = (kind: Kind): string => { switch (kind) { case Kind.SET_UP_ENCRYPTION: return _t("action|continue"); + case Kind.SET_UP_RECOVERY: + return _t("encryption|set_up_recovery"); case Kind.VERIFY_THIS_SESSION: return _t("action|verify"); } }; +const getSecondaryButtonLabel = (kind: Kind): string => { + switch (kind) { + case Kind.SET_UP_RECOVERY: + return _t("encryption|set_up_recovery_later"); + case Kind.SET_UP_ENCRYPTION: + case Kind.VERIFY_THIS_SESSION: + return _t("encryption|verification|unverified_sessions_toast_reject"); + } +}; + const getDescription = (kind: Kind): string => { switch (kind) { case Kind.SET_UP_ENCRYPTION: return _t("encryption|set_up_toast_description"); + case Kind.SET_UP_RECOVERY: + return _t("encryption|set_up_recovery_toast_title"); case Kind.VERIFY_THIS_SESSION: return _t("encryption|verify_toast_description"); } @@ -57,6 +88,7 @@ const getDescription = (kind: Kind): string => { export enum Kind { SET_UP_ENCRYPTION = "set_up_encryption", + SET_UP_RECOVERY = "set_up_recovery", VERIFY_THIS_SESSION = "verify_this_session", } @@ -101,9 +133,9 @@ export const showToast = (kind: Kind): void => { description: getDescription(kind), primaryLabel: getSetupCaption(kind), onPrimaryClick: onAccept, - secondaryLabel: _t("encryption|verification|unverified_sessions_toast_reject"), + secondaryLabel: getSecondaryButtonLabel(kind), onSecondaryClick: onReject, - destructive: "secondary", + PrimaryIcon: getPrimaryIcon(kind), }, component: GenericToast, priority: kind === Kind.VERIFY_THIS_SESSION ? 95 : 40, From b5ae775d8f57bd70395c476f9034bff7d295d498 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 2 Dec 2024 15:08:35 +0000 Subject: [PATCH 6/8] Add mock so test passes again --- test/test-utils/test-utils.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index f9aee512a3..3879f8af81 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -281,6 +281,7 @@ export function createTestClient(): MatrixClient { getLocalAliases: jest.fn().mockReturnValue([]), uploadDeviceSigningKeys: jest.fn(), isKeyBackupKeyStored: jest.fn().mockResolvedValue(null), + getKeyBackupVersion: jest.fn(), } as unknown as MatrixClient; client.reEmitter = new ReEmitter(client); From 2a5af99ac2adc9b619fce3fa68d10b4438ce82e9 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 2 Dec 2024 15:25:51 +0000 Subject: [PATCH 7/8] Fix tests --- test/unit-tests/DeviceListener-test.ts | 4 ++-- test/unit-tests/components/structures/MatrixChat-test.tsx | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/test/unit-tests/DeviceListener-test.ts b/test/unit-tests/DeviceListener-test.ts index ad7f14e119..1c8fe1a1c7 100644 --- a/test/unit-tests/DeviceListener-test.ts +++ b/test/unit-tests/DeviceListener-test.ts @@ -352,13 +352,13 @@ describe("DeviceListener", () => { mockCrypto!.getCrossSigningKeyId.mockResolvedValue("abc"); }); - it("shows set up encryption toast when user has a key backup available", async () => { + it("shows set up recovery toast when user has a key backup available", async () => { // non falsy response mockCrypto.getKeyBackupInfo.mockResolvedValue({} as unknown as KeyBackupInfo); await createAndStart(); expect(SetupEncryptionToast.showToast).toHaveBeenCalledWith( - SetupEncryptionToast.Kind.SET_UP_ENCRYPTION, + SetupEncryptionToast.Kind.SET_UP_RECOVERY, ); }); }); diff --git a/test/unit-tests/components/structures/MatrixChat-test.tsx b/test/unit-tests/components/structures/MatrixChat-test.tsx index bc6bb0f9ff..7e3bd7d4f1 100644 --- a/test/unit-tests/components/structures/MatrixChat-test.tsx +++ b/test/unit-tests/components/structures/MatrixChat-test.tsx @@ -148,6 +148,7 @@ describe("", () => { whoami: jest.fn(), logout: jest.fn(), getDeviceId: jest.fn(), + getKeyBackupVersion: jest.fn(), }); let mockClient: Mocked; const serverConfig = { From 0a43803d469bce72760b64078417dc63789f6b3e Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 5 Dec 2024 14:10:16 +0000 Subject: [PATCH 8/8] Fix canUploadKeysWithPasswordOnly for MSC3967 Being able to upload keys without any auth works too, so return true. Otherwise we have to go the same route via the InteractiveAuthDialog which is way more complex. --- src/CreateCrossSigning.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/CreateCrossSigning.ts b/src/CreateCrossSigning.ts index e67e030f60..8c2d4488cf 100644 --- a/src/CreateCrossSigning.ts +++ b/src/CreateCrossSigning.ts @@ -23,11 +23,11 @@ import InteractiveAuthDialog from "./components/views/dialogs/InteractiveAuthDia async function canUploadKeysWithPasswordOnly(cli: MatrixClient): Promise { try { await cli.uploadDeviceSigningKeys(undefined, {} as CrossSigningKeys); - // We should never get here: the server should always require - // UI auth to upload device signing keys. If we do, we upload - // no keys which would be a no-op. + // If we get here, it's because the server is allowing us to upload keys without + // auth the first time due to MSC3967. Therefore, yes, we can upload keys + // (with or without password, technically, but that's fine). logger.log("uploadDeviceSigningKeys unexpectedly succeeded without UI auth!"); - return false; + return true; } catch (error) { if (!(error instanceof MatrixError) || !error.data || !error.data.flows) { logger.log("uploadDeviceSigningKeys advertised no flows!");