crypto: configure key sharing strategy based on DeviceIsolationMode (#4425)

* crypto: configure key sharing strategy based on deviceIsolationMode

fix eslint import error

cryptoMode was renamed to deviceIsolationMode

post rebase fix: Device Isolation mode name changes

* Fix outdated docs referring to old cryptomode

* code review: better comment for globalBlacklistUnverifiedDevices option

* RoomEncryptor: Use appropriate default for getBlacklistUnverifiedDevices

* do not provide a default value for DeviceIsolationMode for encryption

* Update src/rust-crypto/RoomEncryptor.ts

---------

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

spec/unit/rust-crypto/RoomEncryptor.spec.ts

@@ -17,6 +17,7 @@
  */
 
 import {
+    CollectStrategy,
     Curve25519PublicKey,
     Ed25519PublicKey,
     HistoryVisibility as RustHistoryVisibility,
@@ -31,6 +32,7 @@ import { KeyClaimManager } from "../../../src/rust-crypto/KeyClaimManager";
 import { defer } from "../../../src/utils";
 import { OutgoingRequestsManager } from "../../../src/rust-crypto/OutgoingRequestsManager";
 import { KnownMembership } from "../../../src/@types/membership";
+import { DeviceIsolationMode, AllDevicesIsolationMode, OnlySignedDevicesIsolationMode } from "../../../src/crypto-api";
 
 describe("RoomEncryptor", () => {
     describe("History Visibility", () => {
@@ -99,7 +101,7 @@ describe("RoomEncryptor", () => {
                 getEncryptionTargetMembers: jest.fn().mockReturnValue([mockRoomMember]),
                 shouldEncryptForInvitedMembers: jest.fn().mockReturnValue(true),
                 getHistoryVisibility: jest.fn().mockReturnValue(HistoryVisibility.Invited),
-                getBlacklistUnverifiedDevices: jest.fn().mockReturnValue(false),
+                getBlacklistUnverifiedDevices: jest.fn().mockReturnValue(null),
             } as unknown as Mocked<Room>;
 
             roomEncryptor = new RoomEncryptor(
@@ -111,6 +113,8 @@ describe("RoomEncryptor", () => {
             );
         });
 
+        const defaultDevicesIsolationMode = new AllDevicesIsolationMode(false);
+
         it("should ensure that there is only one shareRoomKey at a time", async () => {
             const deferredShare = defer<void>();
             const insideOlmShareRoom = defer<void>();
@@ -120,19 +124,19 @@ describe("RoomEncryptor", () => {
                 await deferredShare.promise;
             });
 
-            roomEncryptor.prepareForEncryption(false);
+            roomEncryptor.prepareForEncryption(false, defaultDevicesIsolationMode);
             await insideOlmShareRoom.promise;
 
             // call several times more
-            roomEncryptor.prepareForEncryption(false);
+            roomEncryptor.prepareForEncryption(false, defaultDevicesIsolationMode);
-            roomEncryptor.encryptEvent(createMockEvent("Hello"), false);
+            roomEncryptor.encryptEvent(createMockEvent("Hello"), false, defaultDevicesIsolationMode);
-            roomEncryptor.prepareForEncryption(false);
+            roomEncryptor.prepareForEncryption(false, defaultDevicesIsolationMode);
-            roomEncryptor.encryptEvent(createMockEvent("World"), false);
+            roomEncryptor.encryptEvent(createMockEvent("World"), false, defaultDevicesIsolationMode);
 
             expect(mockOlmMachine.shareRoomKey).toHaveBeenCalledTimes(1);
 
             deferredShare.resolve();
-            await roomEncryptor.prepareForEncryption(false);
+            await roomEncryptor.prepareForEncryption(false, defaultDevicesIsolationMode);
 
             // should have been called again
             expect(mockOlmMachine.shareRoomKey).toHaveBeenCalledTimes(6);
@@ -158,8 +162,16 @@ describe("RoomEncryptor", () => {
 
             let firstMessageFinished: string | null = null;
 
-            const firstRequest = roomEncryptor.encryptEvent(createMockEvent("Hello"), false);
+            const firstRequest = roomEncryptor.encryptEvent(
-            const secondRequest = roomEncryptor.encryptEvent(createMockEvent("Edit of Hello"), false);
+                createMockEvent("Hello"),
+                false,
+                defaultDevicesIsolationMode,
+            );
+            const secondRequest = roomEncryptor.encryptEvent(
+                createMockEvent("Edit of Hello"),
+                false,
+                defaultDevicesIsolationMode,
+            );
 
             firstRequest.then(() => {
                 if (firstMessageFinished === null) {
@@ -181,5 +193,96 @@ describe("RoomEncryptor", () => {
 
             expect(firstMessageFinished).toBe("hello");
         });
+
+        describe("DeviceIsolationMode", () => {
+            type TestCase = [
+                string,
+                {
+                    mode: DeviceIsolationMode;
+                    expectedStrategy: CollectStrategy;
+                    globalBlacklistUnverifiedDevices: boolean;
+                },
+            ];
+
+            const testCases: TestCase[] = [
+                [
+                    "Share AllDevicesIsolationMode",
+                    {
+                        mode: new AllDevicesIsolationMode(false),
+                        expectedStrategy: CollectStrategy.deviceBasedStrategy(false, false),
+                        globalBlacklistUnverifiedDevices: false,
+                    },
+                ],
+                [
+                    "Share AllDevicesIsolationMode - with blacklist unverified",
+                    {
+                        mode: new AllDevicesIsolationMode(false),
+                        expectedStrategy: CollectStrategy.deviceBasedStrategy(true, false),
+                        globalBlacklistUnverifiedDevices: true,
+                    },
+                ],
+                [
+                    "Share OnlySigned - blacklist true",
+                    {
+                        mode: new OnlySignedDevicesIsolationMode(),
+                        expectedStrategy: CollectStrategy.identityBasedStrategy(),
+                        globalBlacklistUnverifiedDevices: true,
+                    },
+                ],
+                [
+                    "Share OnlySigned",
+                    {
+                        mode: new OnlySignedDevicesIsolationMode(),
+                        expectedStrategy: CollectStrategy.identityBasedStrategy(),
+                        globalBlacklistUnverifiedDevices: false,
+                    },
+                ],
+                [
+                    "Share AllDevicesIsolationMode - Verified user problems true",
+                    {
+                        mode: new AllDevicesIsolationMode(true),
+                        expectedStrategy: CollectStrategy.deviceBasedStrategy(false, true),
+                        globalBlacklistUnverifiedDevices: false,
+                    },
+                ],
+                [
+                    'Share AllDevicesIsolationMode - with blacklist unverified - Verified user problems true"',
+                    {
+                        mode: new AllDevicesIsolationMode(true),
+                        expectedStrategy: CollectStrategy.deviceBasedStrategy(true, true),
+                        globalBlacklistUnverifiedDevices: true,
+                    },
+                ],
+            ];
+
+            let capturedSettings: CollectStrategy | undefined = undefined;
+
+            beforeEach(() => {
+                capturedSettings = undefined;
+                mockOlmMachine.shareRoomKey.mockImplementationOnce(async (roomId, users, encryptionSettings) => {
+                    capturedSettings = encryptionSettings.sharingStrategy;
+                });
+            });
+
+            it.each(testCases)(
+                "prepareForEncryption should properly set sharing strategy based on crypto mode: %s",
+                async (_, { mode, expectedStrategy, globalBlacklistUnverifiedDevices }) => {
+                    await roomEncryptor.prepareForEncryption(globalBlacklistUnverifiedDevices, mode);
+                    expect(mockOlmMachine.shareRoomKey).toHaveBeenCalled();
+                    expect(capturedSettings).toBeDefined();
+                    expect(expectedStrategy.eq(capturedSettings!)).toBeTruthy();
+                },
+            );
+
+            it.each(testCases)(
+                "encryptEvent should properly set sharing strategy based on crypto mode: %s",
+                async (_, { mode, expectedStrategy, globalBlacklistUnverifiedDevices }) => {
+                    await roomEncryptor.encryptEvent(createMockEvent("Hello"), globalBlacklistUnverifiedDevices, mode);
+                    expect(mockOlmMachine.shareRoomKey).toHaveBeenCalled();
+                    expect(capturedSettings).toBeDefined();
+                    expect(expectedStrategy.eq(capturedSettings!)).toBeTruthy();
+                },
+            );
+        });
     });
 });

src/crypto-api/index.ts

@@ -611,14 +611,13 @@ export enum DecryptionFailureCode {
 
     /**
      * The sender device is not cross-signed.  This will only be used if the
-     * crypto mode is set to `CryptoMode.Invisible` or `CryptoMode.Transition`.
+     * device isolation mode is set to `OnlySignedDevicesIsolationMode`.
      */
     UNSIGNED_SENDER_DEVICE = "UNSIGNED_SENDER_DEVICE",
 
     /**
      * We weren't able to link the message back to any known device.  This will
-     * only be used if the crypto mode is set to `CryptoMode.Invisible` or
+     * only be used if the device isolation mode is set to `OnlySignedDevicesIsolationMode`.
-     * `CryptoMode.Transition`.
      */
     UNKNOWN_SENDER_DEVICE = "UNKNOWN_SENDER_DEVICE",
 

src/rust-crypto/RoomEncryptor.ts

@@ -36,6 +36,7 @@ import { HistoryVisibility } from "../@types/partials.ts";
 import { OutgoingRequestsManager } from "./OutgoingRequestsManager.ts";
 import { logDuration } from "../utils.ts";
 import { KnownMembership } from "../@types/membership.ts";
+import { DeviceIsolationMode, DeviceIsolationModeKind } from "../crypto-api/index.ts";
 
 /**
  * RoomEncryptor: responsible for encrypting messages to a given room
@@ -119,10 +120,15 @@ export class RoomEncryptor {
      *
      * This ensures that we have a megolm session ready to use and that we have shared its key with all the devices
      * in the room.
-     *
+     * @param globalBlacklistUnverifiedDevices - When `true`, and `deviceIsolationMode` is `AllDevicesIsolationMode`,
-     * @param globalBlacklistUnverifiedDevices - When `true`, it will not send encrypted messages to unverified devices
+     * will not send encrypted messages to unverified devices.
+     * Ignored when `deviceIsolationMode` is `OnlySignedDevicesIsolationMode`.
+     * @param deviceIsolationMode - The device isolation mode. See {@link DeviceIsolationMode}.
      */
-    public async prepareForEncryption(globalBlacklistUnverifiedDevices: boolean): Promise<void> {
+    public async prepareForEncryption(
+        globalBlacklistUnverifiedDevices: boolean,
+        deviceIsolationMode: DeviceIsolationMode,
+    ): Promise<void> {
         // We consider a prepareForEncryption as an encryption promise as it will potentially share keys
         // even if it doesn't send an event.
         // Usually this is called when the user starts typing, so we want to make sure we have keys ready when the
@@ -130,7 +136,7 @@ export class RoomEncryptor {
         // If `encryptEvent` is invoked before `prepareForEncryption` has completed, the `encryptEvent` call will wait for
         // `prepareForEncryption` to complete before executing.
         // The part where `encryptEvent` shares the room key will then usually be a no-op as it was already performed by `prepareForEncryption`.
-        await this.encryptEvent(null, globalBlacklistUnverifiedDevices);
+        await this.encryptEvent(null, globalBlacklistUnverifiedDevices, deviceIsolationMode);
     }
 
     /**
@@ -140,9 +146,16 @@ export class RoomEncryptor {
      * then, if an event is provided, encrypt it using the session.
      *
      * @param event - Event to be encrypted, or null if only preparing for encryption (in which case we will pre-share the room key).
-     * @param globalBlacklistUnverifiedDevices - When `true`, it will not send encrypted messages to unverified devices
+     * @param globalBlacklistUnverifiedDevices - When `true`, and `deviceIsolationMode` is `AllDevicesIsolationMode`,
+     * will not send encrypted messages to unverified devices.
+     * Ignored when `deviceIsolationMode` is `OnlySignedDevicesIsolationMode`.
+     * @param deviceIsolationMode - The device isolation mode. See {@link DeviceIsolationMode}.
      */
-    public encryptEvent(event: MatrixEvent | null, globalBlacklistUnverifiedDevices: boolean): Promise<void> {
+    public encryptEvent(
+        event: MatrixEvent | null,
+        globalBlacklistUnverifiedDevices: boolean,
+        deviceIsolationMode: DeviceIsolationMode,
+    ): Promise<void> {
         const logger = new LogSpan(this.prefixedLogger, event ? (event.getTxnId() ?? "") : "prepareForEncryption");
         // Ensure order of encryption to avoid message ordering issues, as the scheduler only ensures
         // events order after they have been encrypted.
@@ -153,7 +166,7 @@ export class RoomEncryptor {
             })
             .then(async () => {
                 await logDuration(logger, "ensureEncryptionSession", async () => {
-                    await this.ensureEncryptionSession(logger, globalBlacklistUnverifiedDevices);
+                    await this.ensureEncryptionSession(logger, globalBlacklistUnverifiedDevices, deviceIsolationMode);
                 });
                 if (event) {
                     await logDuration(logger, "encryptEventInner", async () => {
@@ -173,9 +186,16 @@ export class RoomEncryptor {
      * in the room.
      *
      * @param logger - a place to write diagnostics to
-     * @param globalBlacklistUnverifiedDevices - When `true`, it will not send encrypted messages to unverified devices
+     * @param globalBlacklistUnverifiedDevices - When `true`, and `deviceIsolationMode` is `AllDevicesIsolationMode`,
+     * will not send encrypted messages to unverified devices.
+     * Ignored when `deviceIsolationMode` is `OnlySignedDevicesIsolationMode`.
+     * @param deviceIsolationMode - The device isolation mode. See {@link DeviceIsolationMode}.
      */
-    private async ensureEncryptionSession(logger: LogSpan, globalBlacklistUnverifiedDevices: boolean): Promise<void> {
+    private async ensureEncryptionSession(
+        logger: LogSpan,
+        globalBlacklistUnverifiedDevices: boolean,
+        deviceIsolationMode: DeviceIsolationMode,
+    ): Promise<void> {
         if (this.encryptionSettings.algorithm !== "m.megolm.v1.aes-sha2") {
             throw new Error(
                 `Cannot encrypt in ${this.room.roomId} for unsupported algorithm '${this.encryptionSettings.algorithm}'`,
@@ -251,12 +271,22 @@ export class RoomEncryptor {
             rustEncryptionSettings.rotationPeriodMessages = BigInt(this.encryptionSettings.rotation_period_msgs);
         }
 
-        // When this.room.getBlacklistUnverifiedDevices() === null, the global settings should be used
+        switch (deviceIsolationMode.kind) {
-        // See Room#getBlacklistUnverifiedDevices
+            case DeviceIsolationModeKind.AllDevicesIsolationMode:
-        if (this.room.getBlacklistUnverifiedDevices() ?? globalBlacklistUnverifiedDevices) {
+                {
-            rustEncryptionSettings.sharingStrategy = CollectStrategy.deviceBasedStrategy(true, false);
+                    // When this.room.getBlacklistUnverifiedDevices() === null, the global settings should be used
-        } else {
+                    // See Room#getBlacklistUnverifiedDevices
-            rustEncryptionSettings.sharingStrategy = CollectStrategy.deviceBasedStrategy(false, false);
+                    const onlyAllowTrustedDevices =
+                        this.room.getBlacklistUnverifiedDevices() ?? globalBlacklistUnverifiedDevices;
+                    rustEncryptionSettings.sharingStrategy = CollectStrategy.deviceBasedStrategy(
+                        onlyAllowTrustedDevices,
+                        deviceIsolationMode.errorOnVerifiedUserProblems,
+                    );
+                }
+                break;
+            case DeviceIsolationModeKind.OnlySignedDevicesIsolationMode:
+                rustEncryptionSettings.sharingStrategy = CollectStrategy.identityBasedStrategy();
+                break;
         }
 
         await logDuration(this.prefixedLogger, "shareRoomKey", async () => {

src/rust-crypto/rust-crypto.ts

@@ -248,7 +248,7 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
             throw new Error(`Cannot encrypt event in unconfigured room ${roomId}`);
         }
 
-        await encryptor.encryptEvent(event, this.globalBlacklistUnverifiedDevices);
+        await encryptor.encryptEvent(event, this.globalBlacklistUnverifiedDevices, this.deviceIsolationMode);
     }
 
     public async decryptEvent(event: MatrixEvent): Promise<IEventDecryptionResult> {
@@ -400,7 +400,7 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
         const encryptor = this.roomEncryptors[room.roomId];
 
         if (encryptor) {
-            encryptor.prepareForEncryption(this.globalBlacklistUnverifiedDevices);
+            encryptor.prepareForEncryption(this.globalBlacklistUnverifiedDevices, this.deviceIsolationMode);
         }
     }