From 538717c23eafd9acd68c7c10564687bc3ce5bcf1 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 25 Sep 2024 15:33:02 +0200 Subject: [PATCH] crypto: Replace cryptoMode with DeviceIsolationMode concept (#4429) * crypto: Replace cryptoMode with DeviceIsolationMode concept * use enum instead of string for the IsolationMode kind * Code review - Cleaning, renaming * review: unneeded @see in doc * review: Rename IsolationMode with better names * review: quick cleaning and doc --- spec/integ/crypto/crypto.spec.ts | 60 ++++++++++++++----------- src/crypto-api/index.ts | 75 ++++++++++++++++++++------------ src/crypto/index.ts | 7 ++- src/rust-crypto/rust-crypto.ts | 29 ++++++------ 4 files changed, 101 insertions(+), 70 deletions(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index e6b6951a4aa..e5d25537f10 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -82,11 +82,13 @@ import { SecretStorageKeyDescription } from "../../../src/secret-storage"; import { CrossSigningKey, CryptoCallbacks, - CryptoMode, DecryptionFailureCode, + DeviceIsolationMode, EventShieldColour, EventShieldReason, KeyBackupInfo, + AllDevicesIsolationMode, + OnlySignedDevicesIsolationMode, } from "../../../src/crypto-api"; import { E2EKeyResponder } from "../../test-utils/E2EKeyResponder"; import { IKeyBackup } from "../../../src/crypto/backup"; @@ -747,9 +749,34 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, ); }); - newBackendOnly( - "fails with an error when cross-signed sender is required but sender is not cross-signed", - async () => { + describe("IsolationMode decryption tests", () => { + newBackendOnly( + "OnlySigned mode - fails with an error when cross-signed sender is required but sender is not cross-signed", + async () => { + const decryptedEvent = await setUpTestAndDecrypt(new OnlySignedDevicesIsolationMode()); + + // It will error as an unknown device because we haven't fetched + // the sender's device keys. + expect(decryptedEvent.isDecryptionFailure()).toBe(true); + expect(decryptedEvent.decryptionFailureReason).toEqual(DecryptionFailureCode.UNKNOWN_SENDER_DEVICE); + }, + ); + + newBackendOnly( + "NoIsolation mode - Decrypts with warning when cross-signed sender is required but sender is not cross-signed", + async () => { + const decryptedEvent = await setUpTestAndDecrypt(new AllDevicesIsolationMode(false)); + + expect(decryptedEvent.isDecryptionFailure()).toBe(false); + + expect(await aliceClient.getCrypto()!.getEncryptionInfoForEvent(decryptedEvent)).toEqual({ + shieldColour: EventShieldColour.RED, + shieldReason: EventShieldReason.UNKNOWN_DEVICE, + }); + }, + ); + + async function setUpTestAndDecrypt(isolationMode: DeviceIsolationMode): Promise { // This tests that a message will not be decrypted if the sender // is not sufficiently trusted according to the selected crypto // mode. @@ -760,7 +787,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); // Start by using Invisible crypto mode - aliceClient.getCrypto()!.setCryptoMode(CryptoMode.Invisible); + aliceClient.getCrypto()!.setDeviceIsolationMode(isolationMode); await startClientAndAwaitFirstSync(); @@ -807,26 +834,9 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, expect(event.isEncrypted()).toBe(true); // it probably won't be decrypted yet, because it takes a while to process the olm keys - const decryptedEvent = await testUtils.awaitDecryption(event); - // It will error as an unknown device because we haven't fetched - // the sender's device keys. - expect(decryptedEvent.decryptionFailureReason).toEqual(DecryptionFailureCode.UNKNOWN_SENDER_DEVICE); - - // Next, try decrypting in transition mode, which should also - // fail for the same reason - aliceClient.getCrypto()!.setCryptoMode(CryptoMode.Transition); - - await event.attemptDecryption(aliceClient["cryptoBackend"]!); - expect(decryptedEvent.decryptionFailureReason).toEqual(DecryptionFailureCode.UNKNOWN_SENDER_DEVICE); - - // Decrypting in legacy mode should succeed since it doesn't - // care about device trust. - aliceClient.getCrypto()!.setCryptoMode(CryptoMode.Legacy); - - await event.attemptDecryption(aliceClient["cryptoBackend"]!); - expect(decryptedEvent.decryptionFailureReason).toEqual(null); - }, - ); + return await testUtils.awaitDecryption(event); + } + }); it("Decryption fails with Unable to decrypt for other errors", async () => { expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); diff --git a/src/crypto-api/index.ts b/src/crypto-api/index.ts index 39038290f34..4503a72cef6 100644 --- a/src/crypto-api/index.ts +++ b/src/crypto-api/index.ts @@ -41,11 +41,9 @@ export interface CryptoApi { globalBlacklistUnverifiedDevices: boolean; /** - * The cryptography mode to use. - * - * @see CryptoMode + * The {@link DeviceIsolationMode} mode to use. */ - setCryptoMode(cryptoMode: CryptoMode): void; + setDeviceIsolationMode(isolationMode: DeviceIsolationMode): void; /** * Return the current version of the crypto module. @@ -667,38 +665,59 @@ export enum DecryptionFailureCode { UNKNOWN_ENCRYPTION_ALGORITHM = "UNKNOWN_ENCRYPTION_ALGORITHM", } +/** Base {@link DeviceIsolationMode} kind. */ +export enum DeviceIsolationModeKind { + AllDevicesIsolationMode, + OnlySignedDevicesIsolationMode, +} + /** - * The cryptography mode. Affects how messages are encrypted and decrypted. - * Only supported by Rust crypto. + * A type of {@link DeviceIsolationMode}. + * + * Message encryption keys are shared with all devices in the room, except in case of + * verified user problems (see {@link errorOnVerifiedUserProblems}). + * + * Events from all senders are always decrypted (and should be decorated with message shields in case + * of authenticity warnings, see {@link EventEncryptionInfo}). */ -export enum CryptoMode { - /** - * Message encryption keys are shared with all devices in the room, except for - * blacklisted devices, or unverified devices if - * `globalBlacklistUnverifiedDevices` is set. Events from all senders are - * decrypted. - */ - Legacy, +export class AllDevicesIsolationMode { + public readonly kind = DeviceIsolationModeKind.AllDevicesIsolationMode; /** - * Events are encrypted as with `Legacy` mode, but encryption will throw an error if a - * verified user has an unsigned device, or if a verified user replaces - * their identity. Events are decrypted only if they come from cross-signed - * devices, or devices that existed before the Rust crypto SDK started - * tracking device trust: other events will result in a decryption failure. (To access the failure - * reason, see {@link MatrixEvent.decryptionFailureReason}.) + * + * @param errorOnVerifiedUserProblems - Behavior when sharing keys to remote devices. + * + * If set to `true`, sharing keys will fail (i.e. message sending will fail) with an error if: + * - The user was previously verified but is not anymore, or: + * - A verified user has some unverified devices (not cross-signed). + * + * If `false`, the keys will be distributed as usual. In this case, the client UX should display + * warnings to inform the user about problematic devices/users, and stop them hitting this case. */ - Transition, + public constructor(public readonly errorOnVerifiedUserProblems: boolean) {} +} - /** - * Message encryption keys are only shared with devices that have been cross-signed by their owner. - * Encryption will throw an error if a verified user replaces their identity. Events are - * decrypted only if they come from a cross-signed device other events will result in a decryption - * failure. (To access the failure reason, see {@link MatrixEvent.decryptionFailureReason}.) - */ - Invisible, +/** + * A type of {@link DeviceIsolationMode}. + * + * Message encryption keys are only shared with devices that have been cross-signed by their owner. + * Encryption will throw an error if a verified user replaces their identity. + * + * Events are decrypted only if they come from a cross-signed device. Other events will result in a decryption + * failure. (To access the failure reason, see {@link MatrixEvent.decryptionFailureReason}.) + */ +export class OnlySignedDevicesIsolationMode { + public readonly kind = DeviceIsolationModeKind.OnlySignedDevicesIsolationMode; } +/** + * DeviceIsolationMode represents the mode of device isolation used when encrypting or decrypting messages. + * It can be one of two types: {@link AllDevicesIsolationMode} or {@link OnlySignedDevicesIsolationMode}. + * + * Only supported by rust Crypto. + */ +export type DeviceIsolationMode = AllDevicesIsolationMode | OnlySignedDevicesIsolationMode; + /** * Options object for `CryptoApi.bootstrapCrossSigning`. */ diff --git a/src/crypto/index.ts b/src/crypto/index.ts index 38bf03c7fd8..4eed467758d 100644 --- a/src/crypto/index.ts +++ b/src/crypto/index.ts @@ -88,9 +88,9 @@ import { BootstrapCrossSigningOpts, CrossSigningKeyInfo, CrossSigningStatus, - CryptoMode, decodeRecoveryKey, DecryptionFailureCode, + DeviceIsolationMode, DeviceVerificationStatus, encodeRecoveryKey, EventEncryptionInfo, @@ -650,12 +650,11 @@ export class Crypto extends TypedEventEmitter { + public async attemptEventDecryption( + event: MatrixEvent, + isolationMode: DeviceIsolationMode, + ): Promise { // add the event to the pending list *before* attempting to decrypt. // then, if the key turns up while decryption is in progress (and // decryption fails), we will schedule a retry. @@ -1784,16 +1789,14 @@ class EventDecryptor { this.addEventToPendingList(event); let trustRequirement; - switch (cryptoMode) { - case CryptoMode.Legacy: + + switch (isolationMode.kind) { + case DeviceIsolationModeKind.AllDevicesIsolationMode: trustRequirement = RustSdkCryptoJs.TrustRequirement.Untrusted; break; - case CryptoMode.Transition: + case DeviceIsolationModeKind.OnlySignedDevicesIsolationMode: trustRequirement = RustSdkCryptoJs.TrustRequirement.CrossSignedOrLegacy; break; - case CryptoMode.Invisible: - trustRequirement = RustSdkCryptoJs.TrustRequirement.CrossSigned; - break; } try {