From d2acce1221f65c4b4fc285f4b6f7fa69d7c2ae9c Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Tue, 3 Dec 2024 16:26:37 +0100 Subject: [PATCH] Remove manual device verification which is not supported by the new cryptography stack (#28588) * Remove call of `MatrixClient.setDeviceVerified` * Replace usage of deprecated crypto events * Replace deprecated imports * Remove legacy button in `UntrustedDeviceDialog` * Review fixes * Add tests * Fix doc --- src/SecurityManager.ts | 6 +- src/SlashCommands.tsx | 64 ----- .../ManualDeviceKeyVerificationDialog.tsx | 90 ------- .../views/dialogs/UntrustedDeviceDialog.tsx | 18 +- src/hooks/useEncryptionStatus.ts | 3 +- src/i18n/strings/en_EN.json | 14 -- src/rageshake/submit-rageshake.ts | 7 +- src/utils/device/dehydration.ts | 4 +- src/verification.ts | 6 - ...ManualDeviceKeyVerificationDialog-test.tsx | 104 -------- .../dialogs/UntrustedDeviceDialog-test.tsx | 59 +++++ ...lDeviceKeyVerificationDialog-test.tsx.snap | 231 ------------------ .../UntrustedDeviceDialog-test.tsx.snap | 149 +++++++++++ .../security/ExportE2eKeysDialog-test.tsx | 5 +- .../security/ImportE2eKeysDialog-test.tsx | 4 +- 15 files changed, 237 insertions(+), 527 deletions(-) delete mode 100644 src/components/views/dialogs/ManualDeviceKeyVerificationDialog.tsx delete mode 100644 test/unit-tests/components/views/dialogs/ManualDeviceKeyVerificationDialog-test.tsx create mode 100644 test/unit-tests/components/views/dialogs/UntrustedDeviceDialog-test.tsx delete mode 100644 test/unit-tests/components/views/dialogs/__snapshots__/ManualDeviceKeyVerificationDialog-test.tsx.snap create mode 100644 test/unit-tests/components/views/dialogs/__snapshots__/UntrustedDeviceDialog-test.tsx.snap diff --git a/src/SecurityManager.ts b/src/SecurityManager.ts index f97dff786fa..e8122b2dbf2 100644 --- a/src/SecurityManager.ts +++ b/src/SecurityManager.ts @@ -7,8 +7,8 @@ Please see LICENSE files in the repository root for full details. */ import { lazy } from "react"; -import { ICryptoCallbacks, SecretStorage } from "matrix-js-sdk/src/matrix"; -import { deriveRecoveryKeyFromPassphrase, decodeRecoveryKey } from "matrix-js-sdk/src/crypto-api"; +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 Modal from "./Modal"; @@ -159,7 +159,7 @@ function cacheSecretStorageKey( } } -export const crossSigningCallbacks: ICryptoCallbacks = { +export const crossSigningCallbacks: CryptoCallbacks = { getSecretStorageKey, cacheSecretStorageKey, }; diff --git a/src/SlashCommands.tsx b/src/SlashCommands.tsx index 5fc4d8c3576..b739cec12fc 100644 --- a/src/SlashCommands.tsx +++ b/src/SlashCommands.tsx @@ -49,7 +49,6 @@ import VoipUserMapper from "./VoipUserMapper"; import { htmlSerializeFromMdIfNeeded } from "./editor/serialize"; import { leaveRoomBehaviour } from "./utils/leave-behaviour"; import { MatrixClientPeg } from "./MatrixClientPeg"; -import { getDeviceCryptoInfo } from "./utils/crypto/deviceInfo"; import { isCurrentLocalRoom, reject, singleMxcUpload, success, successSync } from "./slash-commands/utils"; import { deop, op } from "./slash-commands/op"; import { CommandCategories } from "./slash-commands/interface"; @@ -658,69 +657,6 @@ export const Commands = [ category: CommandCategories.admin, renderingTypes: [TimelineRenderingType.Room], }), - new Command({ - command: "verify", - args: " ", - description: _td("slash_command|verify"), - runFn: function (cli, roomId, threadId, args) { - if (args) { - const matches = args.match(/^(\S+) +(\S+) +(\S+)$/); - if (matches) { - const userId = matches[1]; - const deviceId = matches[2]; - const fingerprint = matches[3]; - - return success( - (async (): Promise => { - const device = await getDeviceCryptoInfo(cli, userId, deviceId); - if (!device) { - throw new UserFriendlyError("slash_command|verify_unknown_pair", { - userId, - deviceId, - cause: undefined, - }); - } - const deviceTrust = await cli.getCrypto()?.getDeviceVerificationStatus(userId, deviceId); - - if (deviceTrust?.isVerified()) { - if (device.getFingerprint() === fingerprint) { - throw new UserFriendlyError("slash_command|verify_nop"); - } else { - throw new UserFriendlyError("slash_command|verify_nop_warning_mismatch"); - } - } - - if (device.getFingerprint() !== fingerprint) { - const fprint = device.getFingerprint(); - throw new UserFriendlyError("slash_command|verify_mismatch", { - fprint, - userId, - deviceId, - fingerprint, - cause: undefined, - }); - } - - await cli.setDeviceVerified(userId, deviceId, true); - - // Tell the user we verified everything - Modal.createDialog(InfoDialog, { - title: _t("slash_command|verify_success_title"), - description: ( -
-

{_t("slash_command|verify_success_description", { userId, deviceId })}

-
- ), - }); - })(), - ); - } - } - return reject(this.getUsage()); - }, - category: CommandCategories.advanced, - renderingTypes: [TimelineRenderingType.Room], - }), new Command({ command: "discardsession", description: _td("slash_command|discardsession"), diff --git a/src/components/views/dialogs/ManualDeviceKeyVerificationDialog.tsx b/src/components/views/dialogs/ManualDeviceKeyVerificationDialog.tsx deleted file mode 100644 index 76e7580e018..00000000000 --- a/src/components/views/dialogs/ManualDeviceKeyVerificationDialog.tsx +++ /dev/null @@ -1,90 +0,0 @@ -/* -Copyright 2024 New Vector Ltd. -Copyright 2020 The Matrix.org Foundation C.I.C. -Copyright 2019 New Vector Ltd -Copyright 2019 Michael Telatynski <7t3chguy@gmail.com> -Copyright 2017 Vector Creations Ltd -Copyright 2016 OpenMarket Ltd - -SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only -Please see LICENSE files in the repository root for full details. -*/ - -import React, { useCallback } from "react"; -import { Device } from "matrix-js-sdk/src/matrix"; - -import * as FormattingUtils from "../../../utils/FormattingUtils"; -import { _t } from "../../../languageHandler"; -import QuestionDialog from "./QuestionDialog"; -import { MatrixClientPeg } from "../../../MatrixClientPeg"; - -interface IManualDeviceKeyVerificationDialogProps { - userId: string; - device: Device; - onFinished(confirm?: boolean): void; -} - -export function ManualDeviceKeyVerificationDialog({ - userId, - device, - onFinished, -}: IManualDeviceKeyVerificationDialogProps): JSX.Element { - const mxClient = MatrixClientPeg.safeGet(); - - const onLegacyFinished = useCallback( - (confirm: boolean) => { - if (confirm) { - mxClient.setDeviceVerified(userId, device.deviceId, true); - } - onFinished(confirm); - }, - [mxClient, userId, device, onFinished], - ); - - let text; - if (mxClient?.getUserId() === userId) { - text = _t("encryption|verification|manual_device_verification_self_text"); - } else { - text = _t("encryption|verification|manual_device_verification_user_text"); - } - - const fingerprint = device.getFingerprint(); - const key = fingerprint && FormattingUtils.formatCryptoKey(fingerprint); - const body = ( -
-

{text}

-
-
    -
  • - {" "} - {device.displayName} -
  • -
  • - {" "} - - {device.deviceId} - -
  • -
  • - {" "} - - - {key} - - -
  • -
-
-

{_t("encryption|verification|manual_device_verification_footer")}

-
- ); - - return ( - - ); -} diff --git a/src/components/views/dialogs/UntrustedDeviceDialog.tsx b/src/components/views/dialogs/UntrustedDeviceDialog.tsx index c2bc92e7510..4b37032207b 100644 --- a/src/components/views/dialogs/UntrustedDeviceDialog.tsx +++ b/src/components/views/dialogs/UntrustedDeviceDialog.tsx @@ -17,9 +17,20 @@ import BaseDialog from "./BaseDialog"; import { IDevice } from "../right_panel/UserInfo"; interface IProps { + /** + * The user whose device is untrusted. + */ user: User; + /** + * The device that is untrusted. + */ device: IDevice; - onFinished(mode?: "legacy" | "sas" | false): void; + /** + * Callback for when the dialog is dismissed. + * If mode is "sas", the user wants to verify the device with SAS. Otherwise, the dialog was dismissed normally. + * @param mode The mode of dismissal. + */ + onFinished(mode?: "sas"): void; } const UntrustedDeviceDialog: React.FC = ({ device, user, onFinished }) => { @@ -56,13 +67,10 @@ const UntrustedDeviceDialog: React.FC = ({ device, user, onFinished }) =

{askToVerifyText}

- onFinished("legacy")}> - {_t("encryption|udd|manual_verification_button")} - onFinished("sas")}> {_t("encryption|udd|interactive_verification_button")} - onFinished(false)}> + onFinished()}> {_t("action|done")}
diff --git a/src/hooks/useEncryptionStatus.ts b/src/hooks/useEncryptionStatus.ts index 686f68f25e2..ed8cceb9f81 100644 --- a/src/hooks/useEncryptionStatus.ts +++ b/src/hooks/useEncryptionStatus.ts @@ -6,9 +6,10 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only Please see LICENSE files in the repository root for full details. */ -import { CryptoEvent, MatrixClient, Room, RoomStateEvent } from "matrix-js-sdk/src/matrix"; +import { MatrixClient, Room, RoomStateEvent } from "matrix-js-sdk/src/matrix"; import { useEffect, useMemo, useState } from "react"; import { throttle } from "lodash"; +import { CryptoEvent } from "matrix-js-sdk/src/crypto-api"; import { E2EStatus, shieldStatusForRoom } from "../utils/ShieldUtils"; import { useTypedEventEmitter } from "./useEventEmitter"; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index e601a7ecd53..fe5e54e5596 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -922,7 +922,6 @@ }, "udd": { "interactive_verification_button": "Interactively verify by emoji", - "manual_verification_button": "Manually verify by text", "other_ask_verify_text": "Ask this user to verify their session, or manually verify it below.", "other_new_session_text": "%(name)s (%(userId)s) signed in to a new session without verifying it:", "own_ask_verify_text": "Verify your other session using one of the options below.", @@ -957,12 +956,6 @@ "incoming_sas_dialog_waiting": "Waiting for partner to confirm…", "incoming_sas_user_dialog_text_1": "Verify this user to mark them as trusted. Trusting users gives you extra peace of mind when using end-to-end encrypted messages.", "incoming_sas_user_dialog_text_2": "Verifying this user will mark their session as trusted, and also mark your session as trusted to them.", - "manual_device_verification_device_id_label": "Session ID", - "manual_device_verification_device_key_label": "Session key", - "manual_device_verification_device_name_label": "Session name", - "manual_device_verification_footer": "If they don't match, the security of your communication may be compromised.", - "manual_device_verification_self_text": "Confirm by comparing the following with the User Settings in your other session:", - "manual_device_verification_user_text": "Confirm this user's session by comparing the following with their User Settings:", "no_key_or_device": "It looks like you don't have a Security Key or any other devices you can verify against. This device will not be able to access old encrypted messages. In order to verify your identity on this device, you'll need to reset your verification keys.", "no_support_qr_emoji": "The device you are trying to verify doesn't support scanning a QR code or emoji verification, which is what %(brand)s supports. Try with a different client.", "other_party_cancelled": "The other party cancelled the verification.", @@ -3036,13 +3029,6 @@ "upgraderoom": "Upgrades a room to a new version", "upgraderoom_permission_error": "You do not have the required permissions to use this command.", "usage": "Usage", - "verify": "Verifies a user, session, and pubkey tuple", - "verify_mismatch": "WARNING: KEY VERIFICATION FAILED! The signing key for %(userId)s and session %(deviceId)s is \"%(fprint)s\" which does not match the provided key \"%(fingerprint)s\". This could mean your communications are being intercepted!", - "verify_nop": "Session already verified!", - "verify_nop_warning_mismatch": "WARNING: session already verified, but keys do NOT MATCH!", - "verify_success_description": "The signing key you provided matches the signing key you received from %(userId)s's session %(deviceId)s. Session marked as verified.", - "verify_success_title": "Verified key", - "verify_unknown_pair": "Unknown (user, session) pair: (%(userId)s, %(deviceId)s)", "view": "Views room with given address", "whois": "Displays information about a user" }, diff --git a/src/rageshake/submit-rageshake.ts b/src/rageshake/submit-rageshake.ts index ec529336d1e..0c00cc777a2 100644 --- a/src/rageshake/submit-rageshake.ts +++ b/src/rageshake/submit-rageshake.ts @@ -9,7 +9,8 @@ Please see LICENSE files in the repository root for full details. */ import { logger } from "matrix-js-sdk/src/logger"; -import { Method, MatrixClient, Crypto } from "matrix-js-sdk/src/matrix"; +import { Method, MatrixClient } from "matrix-js-sdk/src/matrix"; +import { CryptoApi } from "matrix-js-sdk/src/crypto-api"; import type * as Pako from "pako"; import { MatrixClientPeg } from "../MatrixClientPeg"; @@ -169,7 +170,7 @@ async function collectSynapseSpecific(client: MatrixClient, body: FormData): Pro /** * Collects crypto related information. */ -async function collectCryptoInfo(cryptoApi: Crypto.CryptoApi, body: FormData): Promise { +async function collectCryptoInfo(cryptoApi: CryptoApi, body: FormData): Promise { body.append("crypto_version", cryptoApi.getVersion()); const ownDeviceKeys = await cryptoApi.getOwnDeviceKeys(); @@ -198,7 +199,7 @@ async function collectCryptoInfo(cryptoApi: Crypto.CryptoApi, body: FormData): P /** * Collects information about secret storage and backup. */ -async function collectRecoveryInfo(client: MatrixClient, cryptoApi: Crypto.CryptoApi, body: FormData): Promise { +async function collectRecoveryInfo(client: MatrixClient, cryptoApi: CryptoApi, body: FormData): Promise { const secretStorage = client.secretStorage; body.append("secret_storage_ready", String(await cryptoApi.isSecretStorageReady())); body.append("secret_storage_key_in_account", String(await secretStorage.hasKey())); diff --git a/src/utils/device/dehydration.ts b/src/utils/device/dehydration.ts index f18c4c5c7cd..b27b3c54c26 100644 --- a/src/utils/device/dehydration.ts +++ b/src/utils/device/dehydration.ts @@ -7,7 +7,7 @@ Please see LICENSE files in the repository root for full details. */ import { logger } from "matrix-js-sdk/src/logger"; -import { Crypto } from "matrix-js-sdk/src/matrix"; +import { CryptoApi } from "matrix-js-sdk/src/crypto-api"; import { MatrixClientPeg } from "../../MatrixClientPeg"; @@ -21,7 +21,7 @@ import { MatrixClientPeg } from "../../MatrixClientPeg"; * * Dehydration can currently only be enabled by setting a flag in the .well-known file. */ -async function deviceDehydrationEnabled(crypto: Crypto.CryptoApi | undefined): Promise { +async function deviceDehydrationEnabled(crypto: CryptoApi | undefined): Promise { if (!crypto) { return false; } diff --git a/src/verification.ts b/src/verification.ts index 9c14a64c515..9f774964998 100644 --- a/src/verification.ts +++ b/src/verification.ts @@ -15,7 +15,6 @@ import { RightPanelPhases } from "./stores/right-panel/RightPanelStorePhases"; import { accessSecretStorage } from "./SecurityManager"; import UntrustedDeviceDialog from "./components/views/dialogs/UntrustedDeviceDialog"; import { IDevice } from "./components/views/right_panel/UserInfo"; -import { ManualDeviceKeyVerificationDialog } from "./components/views/dialogs/ManualDeviceKeyVerificationDialog"; import RightPanelStore from "./stores/right-panel/RightPanelStore"; import { IRightPanelCardState } from "./stores/right-panel/RightPanelStoreIPanelState"; import { findDMForUser } from "./utils/dm/findDMForUser"; @@ -53,11 +52,6 @@ export async function verifyDevice(matrixClient: MatrixClient, user: User, devic .getCrypto() ?.requestDeviceVerification(user.userId, device.deviceId); setRightPanel({ member: user, verificationRequestPromise }); - } else if (action === "legacy") { - Modal.createDialog(ManualDeviceKeyVerificationDialog, { - userId: user.userId, - device, - }); } }, }); diff --git a/test/unit-tests/components/views/dialogs/ManualDeviceKeyVerificationDialog-test.tsx b/test/unit-tests/components/views/dialogs/ManualDeviceKeyVerificationDialog-test.tsx deleted file mode 100644 index cb01b346dee..00000000000 --- a/test/unit-tests/components/views/dialogs/ManualDeviceKeyVerificationDialog-test.tsx +++ /dev/null @@ -1,104 +0,0 @@ -/* - * Copyright 2024 New Vector Ltd. - * Copyright 2023 The Matrix.org Foundation C.I.C. - * - * SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only - * Please see LICENSE files in the repository root for full details. - */ - -import React from "react"; -import { render, screen } from "jest-matrix-react"; -import { Device, MatrixClient } from "matrix-js-sdk/src/matrix"; - -import { stubClient } from "../../../../test-utils"; -import { ManualDeviceKeyVerificationDialog } from "../../../../../src/components/views/dialogs/ManualDeviceKeyVerificationDialog"; -import MatrixClientContext from "../../../../../src/contexts/MatrixClientContext"; - -describe("ManualDeviceKeyVerificationDialog", () => { - let mockClient: MatrixClient; - - function renderDialog(userId: string, device: Device, onLegacyFinished: (confirm: boolean) => void) { - return render( - - - , - ); - } - - beforeEach(() => { - mockClient = stubClient(); - }); - - it("should display the device", () => { - // When - const deviceId = "XYZ"; - const device = new Device({ - userId: mockClient.getUserId()!, - deviceId, - displayName: "my device", - algorithms: [], - keys: new Map([[`ed25519:${deviceId}`, "ABCDEFGH"]]), - }); - const { container } = renderDialog(mockClient.getUserId()!, device, jest.fn()); - - // Then - expect(container).toMatchSnapshot(); - }); - - it("should display the device of another user", () => { - // When - const userId = "@alice:example.com"; - const deviceId = "XYZ"; - const device = new Device({ - userId, - deviceId, - displayName: "my device", - algorithms: [], - keys: new Map([[`ed25519:${deviceId}`, "ABCDEFGH"]]), - }); - const { container } = renderDialog(userId, device, jest.fn()); - - // Then - expect(container).toMatchSnapshot(); - }); - - it("should call onFinished and matrixClient.setDeviceVerified", () => { - // When - const deviceId = "XYZ"; - const device = new Device({ - userId: mockClient.getUserId()!, - deviceId, - displayName: "my device", - algorithms: [], - keys: new Map([[`ed25519:${deviceId}`, "ABCDEFGH"]]), - }); - const onFinished = jest.fn(); - renderDialog(mockClient.getUserId()!, device, onFinished); - - screen.getByRole("button", { name: "Verify session" }).click(); - - // Then - expect(onFinished).toHaveBeenCalledWith(true); - expect(mockClient.setDeviceVerified).toHaveBeenCalledWith(mockClient.getUserId(), deviceId, true); - }); - - it("should call onFinished and not matrixClient.setDeviceVerified", () => { - // When - const deviceId = "XYZ"; - const device = new Device({ - userId: mockClient.getUserId()!, - deviceId, - displayName: "my device", - algorithms: [], - keys: new Map([[`ed25519:${deviceId}`, "ABCDEFGH"]]), - }); - const onFinished = jest.fn(); - renderDialog(mockClient.getUserId()!, device, onFinished); - - screen.getByRole("button", { name: "Cancel" }).click(); - - // Then - expect(onFinished).toHaveBeenCalledWith(false); - expect(mockClient.setDeviceVerified).not.toHaveBeenCalled(); - }); -}); diff --git a/test/unit-tests/components/views/dialogs/UntrustedDeviceDialog-test.tsx b/test/unit-tests/components/views/dialogs/UntrustedDeviceDialog-test.tsx new file mode 100644 index 00000000000..3bfafa394bf --- /dev/null +++ b/test/unit-tests/components/views/dialogs/UntrustedDeviceDialog-test.tsx @@ -0,0 +1,59 @@ +/* + * Copyright 2024 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only + * Please see LICENSE files in the repository root for full details. + */ + +import React from "react"; +import { Device, MatrixClient, User } from "matrix-js-sdk/src/matrix"; +import { render, screen } from "jest-matrix-react"; + +import { stubClient } from "../../../../test-utils"; +import UntrustedDeviceDialog from "../../../../../src/components/views/dialogs/UntrustedDeviceDialog.tsx"; + +describe("", () => { + let client: MatrixClient; + let user: User; + let device: Device; + const onFinished = jest.fn(); + + beforeEach(() => { + client = stubClient(); + user = User.createUser("@alice:example.org", client); + user.setDisplayName("Alice"); + device = new Device({ deviceId: "device_id", userId: user.userId, algorithms: [], keys: new Map() }); + }); + + afterEach(() => { + onFinished.mockReset(); + }); + + function renderComponent() { + return render(); + } + + it("should display the dialog for the device of another user", () => { + const { asFragment } = renderComponent(); + expect(asFragment()).toMatchSnapshot(); + }); + + it("should display the dialog for the device of the current user", () => { + jest.spyOn(client, "getUserId").mockReturnValue(user.userId); + + const { asFragment } = renderComponent(); + expect(asFragment()).toMatchSnapshot(); + }); + + it("should call onFinished without parameter when Done is clicked", () => { + renderComponent(); + screen.getByRole("button", { name: "Done" }).click(); + expect(onFinished).toHaveBeenCalledWith(); + }); + + it("should call onFinished with sas when Interactively verify by emoji is clicked", () => { + renderComponent(); + screen.getByRole("button", { name: "Interactively verify by emoji" }).click(); + expect(onFinished).toHaveBeenCalledWith("sas"); + }); +}); diff --git a/test/unit-tests/components/views/dialogs/__snapshots__/ManualDeviceKeyVerificationDialog-test.tsx.snap b/test/unit-tests/components/views/dialogs/__snapshots__/ManualDeviceKeyVerificationDialog-test.tsx.snap deleted file mode 100644 index 1186f06f386..00000000000 --- a/test/unit-tests/components/views/dialogs/__snapshots__/ManualDeviceKeyVerificationDialog-test.tsx.snap +++ /dev/null @@ -1,231 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`ManualDeviceKeyVerificationDialog should display the device 1`] = ` -
-
-