Improve error display for messages sent from insecure devices

playwright/e2e/crypto/event-shields.spec.ts

@@ -6,12 +6,16 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only
 Please see LICENSE files in the repository root for full details.
 */
 
-import { Page } from "@playwright/test";
-
 import { expect, test } from "../../element-web-test";
-import { autoJoin, createSharedRoomWithUser, enableKeyBackup, logIntoElement, logOutOfElement, verify } from "./utils";
-import { Bot } from "../../pages/bot";
-import { HomeserverInstance } from "../../plugins/homeserver";
+import {
+    autoJoin,
+    createSecondBotDevice,
+    createSharedRoomWithUser,
+    enableKeyBackup,
+    logIntoElement,
+    logOutOfElement,
+    verify,
+} from "./utils";
 
 test.describe("Cryptography", function () {
     test.use({
@@ -296,13 +300,3 @@ test.describe("Cryptography", function () {
         });
     });
 });
-
-async function createSecondBotDevice(page: Page, homeserver: HomeserverInstance, bob: Bot) {
-    const bobSecondDevice = new Bot(page, homeserver, {
-        bootstrapSecretStorage: false,
-        bootstrapCrossSigning: false,
-    });
-    bobSecondDevice.setCredentials(await homeserver.loginUser(bob.credentials.userId, bob.credentials.password));
-    await bobSecondDevice.prepareClient();
-    return bobSecondDevice;
-}

playwright/e2e/crypto/invisible-crypto.spec.ts

@@ -0,0 +1,56 @@
+/*
+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 { expect, test } from "../../element-web-test";
+import { autoJoin, createSecondBotDevice, createSharedRoomWithUser, verify } from "./utils";
+import { bootstrapCrossSigningForClient } from "../../pages/client.ts";
+
+/** Tests for the "invisible crypto" behaviour -- i.e., when the "exclude insecure devices" setting is enabled */
+test.describe("Invisible cryptography", () => {
+    test.use({
+        displayName: "Alice",
+        botCreateOpts: { displayName: "Bob" },
+        labsFlags: ["feature_exclude_insecure_devices"],
+    });
+
+    test("Messages fail to decrypt when sender is previously verified", async ({
+        page,
+        bot: bob,
+        user: aliceCredentials,
+        app,
+        homeserver,
+    }) => {
+        await app.client.bootstrapCrossSigning(aliceCredentials);
+        await autoJoin(bob);
+
+        // create an encrypted room
+        const testRoomId = await createSharedRoomWithUser(app, bob.credentials.userId, {
+            name: "TestRoom",
+            initial_state: [
+                {
+                    type: "m.room.encryption",
+                    state_key: "",
+                    content: {
+                        algorithm: "m.megolm.v1.aes-sha2",
+                    },
+                },
+            ],
+        });
+
+        // Verify Bob
+        await verify(app, bob);
+
+        // Bob logs in a new device and resets cross-signing
+        const bobSecondDevice = await createSecondBotDevice(page, homeserver, bob);
+        await bootstrapCrossSigningForClient(await bobSecondDevice.prepareClient(), bob.credentials, true);
+
+        /* should show an error for a message from a previously verified device */
+        await bobSecondDevice.sendMessage(testRoomId, "test encrypted from user that was previously verified");
+        const lastTile = page.locator(".mx_EventTile_last");
+        await expect(lastTile).toContainText("Verified identity has changed");
+    });
+});

playwright/e2e/crypto/utils.ts

@@ -377,3 +377,14 @@ export async function awaitVerifier(
         return verificationRequest.verifier;
     });
 }
+
+/** Log in a second device for the given bot user */
+export async function createSecondBotDevice(page: Page, homeserver: HomeserverInstance, bob: Bot) {
+    const bobSecondDevice = new Bot(page, homeserver, {
+        bootstrapSecretStorage: false,
+        bootstrapCrossSigning: false,
+    });
+    bobSecondDevice.setCredentials(await homeserver.loginUser(bob.credentials.userId, bob.credentials.password));
+    await bobSecondDevice.prepareClient();
+    return bobSecondDevice;
+}

res/css/views/messages/_DecryptionFailureBody.pcss

@@ -10,3 +10,24 @@ Please see LICENSE files in the repository root for full details.
     color: $secondary-content;
     font-style: italic;
 }
+
+// Formatting for the "Verified identity has changed" error
+.mx_DecryptionFailureVerifiedIdentityChanged > span {
+    // Show it in red
+    color: $e2e-warning-color;
+    // TODO: background colour?
+
+    // With a red border
+    border: 1px solid $e2e-warning-color;
+    border-radius: $font-16px;
+
+    // Some space inside the border
+    padding: 4px 12px 4px 8px;
+
+    // some space between the (!) icon and text
+    display: inline-flex;
+    gap: 8px;
+
+    // Center vertically
+    align-items: center;
+}

src/MatrixClientPeg.ts

@@ -42,6 +42,7 @@ import PlatformPeg from "./PlatformPeg";
 import { formatList } from "./utils/FormattingUtils";
 import SdkConfig from "./SdkConfig";
 import { Features } from "./settings/Settings";
+import { setDeviceIsolationMode } from "./settings/controllers/DeviceIsolationModeController.ts";
 
 export interface IMatrixClientCreds {
     homeserverUrl: string;
@@ -343,6 +344,9 @@ class MatrixClientPegClass implements IMatrixClientPeg {
         });
 
         StorageManager.setCryptoInitialised(true);
+
+        setDeviceIsolationMode(this.matrixClient, SettingsStore.getValue("feature_exclude_insecure_devices"));
+
         // TODO: device dehydration and whathaveyou
         return;
     }

src/components/views/messages/DecryptionFailureBody.tsx

@@ -6,15 +6,17 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only
 Please see LICENSE files in the repository root for full details.
 */
 
+import classNames from "classnames";
 import React, { forwardRef, ForwardRefExoticComponent, useContext } from "react";
 import { MatrixEvent } from "matrix-js-sdk/src/matrix";
 import { DecryptionFailureCode } from "matrix-js-sdk/src/crypto-api";
 
 import { _t } from "../../../languageHandler";
 import { IBodyProps } from "./IBodyProps";
 import { LocalDeviceVerificationStateContext } from "../../../contexts/LocalDeviceVerificationStateContext";
+import { Icon as WarningBadgeIcon } from "../../../../res/img/compound/error-16px.svg";
 
-function getErrorMessage(mxEvent: MatrixEvent, isVerified: boolean | undefined): string {
+function getErrorMessage(mxEvent: MatrixEvent, isVerified: boolean | undefined): string | React.JSX.Element {
     switch (mxEvent.decryptionFailureReason) {
         case DecryptionFailureCode.MEGOLM_KEY_WITHHELD_FOR_UNVERIFIED_DEVICE:
             return _t("timeline|decryption_failure|blocked");
@@ -32,16 +34,44 @@ function getErrorMessage(mxEvent: MatrixEvent, isVerified: boolean | undefined):
             break;
 
         case DecryptionFailureCode.HISTORICAL_MESSAGE_USER_NOT_JOINED:
+            // TODO: event should be hidden instead of showing this error.
+            //   To be revisited as part of https://github.com/element-hq/element-meta/issues/2449
             return _t("timeline|decryption_failure|historical_event_user_not_joined");
+
+        case DecryptionFailureCode.SENDER_IDENTITY_PREVIOUSLY_VERIFIED:
+            return (
+                <span>
+                    <WarningBadgeIcon className="mx_Icon mx_Icon_16" />
+                    {_t("timeline|decryption_failure|sender_identity_previously_verified")}
+                </span>
+            );
+
+        case DecryptionFailureCode.UNSIGNED_SENDER_DEVICE:
+            // TODO: event should be hidden instead of showing this error.
+            //   To be revisited as part of https://github.com/element-hq/element-meta/issues/2449
+            return _t("timeline|decryption_failure|sender_unsigned_device");
     }
     return _t("timeline|decryption_failure|unable_to_decrypt");
 }
 
+/** Get an extra CSS class, specific to the decryption failure reason */
+function errorClassName(mxEvent: MatrixEvent): string | null {
+    switch (mxEvent.decryptionFailureReason) {
+        case DecryptionFailureCode.SENDER_IDENTITY_PREVIOUSLY_VERIFIED:
+            return "mx_DecryptionFailureVerifiedIdentityChanged";
+
+        default:
+            return null;
+    }
+}
+
 // A placeholder element for messages that could not be decrypted
 export const DecryptionFailureBody = forwardRef<HTMLDivElement, IBodyProps>(({ mxEvent }, ref): React.JSX.Element => {
     const verificationState = useContext(LocalDeviceVerificationStateContext);
+    const classes = classNames("mx_DecryptionFailureBody", "mx_EventTile_content", errorClassName(mxEvent));
+
     return (
-        <div className="mx_DecryptionFailureBody mx_EventTile_content" ref={ref}>
+        <div className={classes} ref={ref}>
             {getErrorMessage(mxEvent, verificationState)}
         </div>
     );

src/i18n/strings/en_EN.json

@@ -1427,6 +1427,8 @@
         "dynamic_room_predecessors": "Dynamic room predecessors",
         "dynamic_room_predecessors_description": "Enable MSC3946 (to support late-arriving room archives)",
         "element_call_video_rooms": "Element Call video rooms",
+        "exclude_insecure_devices": "Exclude insecure devices when sending/receiving messages",
+        "exclude_insecure_devices_description": "When this mode is enabled, encrypted messages will not be shared with unverified devices, and messages from unverified devices will be shown as an error. Note that if you enable this mode, you may be unable to communicate with users who have not verified their devices.",
         "experimental_description": "Feeling experimental? Try out our latest ideas in development. These features are not finalised; they may be unstable, may change, or may be dropped altogether. <a>Learn more</a>.",
         "experimental_section": "Early previews",
         "extended_profiles_msc_support": "Requires your server to support MSC4133",
@@ -3301,6 +3303,8 @@
             "historical_event_no_key_backup": "Historical messages are not available on this device",
             "historical_event_unverified_device": "You need to verify this device for access to historical messages",
             "historical_event_user_not_joined": "You don't have access to this message",
+            "sender_identity_previously_verified": "Verified identity has changed",
+            "sender_unsigned_device": "Encrypted by a device not verified by its owner.",
             "unable_to_decrypt": "Unable to decrypt message"
         },
         "disambiguated_profile": "%(displayName)s (%(matrixId)s)",

src/settings/Settings.tsx

@@ -11,6 +11,7 @@ import React, { ReactNode } from "react";
 import { UNSTABLE_MSC4133_EXTENDED_PROFILES } from "matrix-js-sdk/src/matrix";
 
 import { _t, _td, TranslationKey } from "../languageHandler";
+import DeviceIsolationModeController from "./controllers/DeviceIsolationModeController.ts";
 import {
     NotificationBodyEnabledController,
     NotificationsEnabledController,
@@ -309,6 +310,16 @@ export const SETTINGS: { [setting: string]: ISetting } = {
         supportedLevelsAreOrdered: true,
         default: false,
     },
+    "feature_exclude_insecure_devices": {
+        isFeature: true,
+        labsGroup: LabGroup.Encryption,
+        controller: new DeviceIsolationModeController(),
+        displayName: _td("labs|exclude_insecure_devices"),
+        description: _td("labs|exclude_insecure_devices_description"),
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED,
+        supportedLevelsAreOrdered: true,
+        default: false,
+    },
     "useOnlyCurrentProfiles": {
         supportedLevels: LEVELS_ACCOUNT_SETTINGS,
         displayName: _td("settings|disable_historical_profile"),

src/settings/controllers/DeviceIsolationModeController.ts

@@ -0,0 +1,37 @@
+/*
+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 { AllDevicesIsolationMode, OnlySignedDevicesIsolationMode } from "matrix-js-sdk/src/crypto-api";
+import { MatrixClient } from "matrix-js-sdk/src/matrix";
+
+import SettingController from "./SettingController";
+import { MatrixClientPeg } from "../../MatrixClientPeg";
+import { SettingLevel } from "../SettingLevel";
+
+/**
+ * A controller for the "exclude_insecure_devices" setting, which will
+ * update the crypto stack's device isolation mode on change.
+ */
+export default class DeviceIsolationModeController extends SettingController {
+    public onChange(level: SettingLevel, roomId: string, newValue: any): void {
+        setDeviceIsolationMode(MatrixClientPeg.safeGet(), newValue);
+    }
+}
+
+/**
+ * Set the crypto stack's device isolation mode based on the current value of the
+ * "exclude_insecure_devices" setting.
+ *
+ * @param client - MatrixClient to update to the new setting.
+ * @param settingValue - value of the "exclude_insecure_devices" setting.
+ */
+export function setDeviceIsolationMode(client: MatrixClient, settingValue: boolean): void {
+    client
+        .getCrypto()
+        ?.setDeviceIsolationMode(
+            settingValue ? new OnlySignedDevicesIsolationMode() : new AllDevicesIsolationMode(true),
+        );
+}

test/components/structures/MatrixChat-test.tsx

@@ -181,7 +181,7 @@
         // without security setup we go to a loading page
         if (withoutSecuritySetup) {
             // we think we are logged in, but are still waiting for the /sync to complete
             await screen.findByText("Logout");
             // initial sync
             client.emit(ClientEvent.Sync, SyncState.Prepared, null);
             // wait for logged in view to load
@@ -190,7 +190,7 @@
             // otherwise we stay on login and load from there for longer
         } else {
             // we are logged in, but are still waiting for the /sync to complete
             await screen.findByText("Syncing…");
             // initial sync
             client.emit(ClientEvent.Sync, SyncState.Prepared, null);
         }
@@ -1002,6 +1002,7 @@
                     getUserVerificationStatus: jest
                         .fn()
                         .mockResolvedValue(new UserVerificationStatus(false, false, false)),
+                    setDeviceIsolationMode: jest.fn(),
                 };
                 loginClient.isCryptoEnabled.mockReturnValue(true);
                 loginClient.getCrypto.mockReturnValue(mockCrypto as any);

test/components/views/messages/DecryptionFailureBody-test.tsx

@@ -103,4 +103,32 @@ describe("DecryptionFailureBody", () => {
         // Then
         expect(container).toHaveTextContent("You don't have access to this message");
     });
+
+    it("should handle messages from users who change identities after verification", async () => {
+        // When
+        const event = await mkDecryptionFailureMatrixEvent({
+            code: DecryptionFailureCode.SENDER_IDENTITY_PREVIOUSLY_VERIFIED,
+            msg: "User previously verified",
+            roomId: "fakeroom",
+            sender: "fakesender",
+        });
+        const { container } = customRender(event);
+
+        // Then
+        expect(container).toMatchSnapshot();
+    });
+
+    it("should handle messages from unverified devices", async () => {
+        // When
+        const event = await mkDecryptionFailureMatrixEvent({
+            code: DecryptionFailureCode.UNSIGNED_SENDER_DEVICE,
+            msg: "Unsigned device",
+            roomId: "fakeroom",
+            sender: "fakesender",
+        });
+        const { container } = customRender(event);
+
+        // Then
+        expect(container).toHaveTextContent("Encrypted by a device not verified by its owner");
+    });
 });

test/components/views/messages/__snapshots__/DecryptionFailureBody-test.tsx.snap

@@ -19,3 +19,18 @@ exports[`DecryptionFailureBody Should display "Unable to decrypt message" 1`] =
   </div>
 </div>
 `;
+
+exports[`DecryptionFailureBody should handle messages from users who change identities after verification 1`] = `
+<div>
+  <div
+    class="mx_DecryptionFailureBody mx_EventTile_content mx_DecryptionFailureVerifiedIdentityChanged"
+  >
+    <span>
+      <div
+        class="mx_Icon mx_Icon_16"
+      />
+      Verified identity has changed
+    </span>
+  </div>
+</div>
+`;

test/settings/controllers/DeviceIsolationModeController-test.ts

@@ -0,0 +1,33 @@
+/*
+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 { AllDevicesIsolationMode, OnlySignedDevicesIsolationMode } from "matrix-js-sdk/src/crypto-api";
+
+import { stubClient } from "../../test-utils";
+import DeviceIsolationModeController from "../../../src/settings/controllers/DeviceIsolationModeController.ts";
+import { SettingLevel } from "../../../src/settings/SettingLevel";
+
+describe("DeviceIsolationModeController", () => {
+    afterEach(() => {
+        jest.resetAllMocks();
+    });
+
+    describe("tracks enabling and disabling", () => {
+        it("on sets signed device isolation mode", () => {
+            const cli = stubClient();
+            const controller = new DeviceIsolationModeController();
+            controller.onChange(SettingLevel.DEVICE, "", true);
+            expect(cli.getCrypto()?.setDeviceIsolationMode).toHaveBeenCalledWith(new OnlySignedDevicesIsolationMode());
+        });
+
+        it("off sets all device isolation mode", () => {
+            const cli = stubClient();
+            const controller = new DeviceIsolationModeController();
+            controller.onChange(SettingLevel.DEVICE, "", false);
+            expect(cli.getCrypto()?.setDeviceIsolationMode).toHaveBeenCalledWith(new AllDevicesIsolationMode(true));
+        });
+    });
+});

test/test-utils/test-utils.ts

@@ -131,6 +131,7 @@ export function createTestClient(): MatrixClient {
             resetKeyBackup: jest.fn(),
             isEncryptionEnabledInRoom: jest.fn(),
             getVerificationRequestsToDeviceInProgress: jest.fn().mockReturnValue([]),
+            setDeviceIsolationMode: jest.fn(),
         }),
 
         getPushActionsForEvent: jest.fn(),