Skip to content

Commit

Permalink
crypto: Replace cryptoMode with DeviceIsolationMode concept (#4429)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
BillCarsonFr authored Sep 25, 2024
1 parent 1a8ea3d commit 538717c
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 70 deletions.
60 changes: 35 additions & 25 deletions spec/integ/crypto/crypto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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<MatrixEvent> {
// This tests that a message will not be decrypted if the sender
// is not sufficiently trusted according to the selected crypto
// mode.
Expand All @@ -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();

Expand Down Expand Up @@ -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: {} });
Expand Down
75 changes: 47 additions & 28 deletions src/crypto-api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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`.
*/
Expand Down
7 changes: 3 additions & 4 deletions src/crypto/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,9 @@ import {
BootstrapCrossSigningOpts,
CrossSigningKeyInfo,
CrossSigningStatus,
CryptoMode,
decodeRecoveryKey,
DecryptionFailureCode,
DeviceIsolationMode,
DeviceVerificationStatus,
encodeRecoveryKey,
EventEncryptionInfo,
Expand Down Expand Up @@ -650,12 +650,11 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
}

/**
* Implementation of {@link Crypto.CryptoApi#setCryptoMode}.
* Implementation of {@link Crypto.CryptoApi#setDeviceIsolationMode}.
*/
public setCryptoMode(cryptoMode: CryptoMode): void {
public setDeviceIsolationMode(isolationMode: DeviceIsolationMode): void {
throw new Error("Not supported");
}

/**
* Implementation of {@link Crypto.CryptoApi#getVersion}.
*/
Expand Down
29 changes: 16 additions & 13 deletions src/rust-crypto/rust-crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ import {
CrossSigningStatus,
CryptoApi,
CryptoCallbacks,
CryptoMode,
Curve25519AuthData,
DecryptionFailureCode,
DeviceVerificationStatus,
Expand All @@ -61,6 +60,9 @@ import {
VerificationRequest,
encodeRecoveryKey,
deriveRecoveryKeyFromPassphrase,
DeviceIsolationMode,
AllDevicesIsolationMode,
DeviceIsolationModeKind,
} from "../crypto-api/index.ts";
import { deviceKeysToDeviceMap, rustDeviceToJsDevice } from "./device-converter.ts";
import { IDownloadKeyResult, IQueryKeysRequest } from "../client.ts";
Expand Down Expand Up @@ -107,7 +109,7 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
private readonly RECOVERY_KEY_DERIVATION_ITERATIONS = 500000;

private _trustCrossSignedDevices = true;
private cryptoMode = CryptoMode.Legacy;
private deviceIsolationMode: DeviceIsolationMode = new AllDevicesIsolationMode(false);

/** whether {@link stop} has been called */
private stopped = false;
Expand Down Expand Up @@ -259,7 +261,7 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
// through decryptEvent and hence get rid of this case.
throw new Error("to-device event was not decrypted in preprocessToDeviceMessages");
}
return await this.eventDecryptor.attemptEventDecryption(event, this.cryptoMode);
return await this.eventDecryptor.attemptEventDecryption(event, this.deviceIsolationMode);
}

/**
Expand Down Expand Up @@ -370,10 +372,10 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
}

/**
* Implementation of {@link Crypto.CryptoApi#setCryptoMode}.
* Implementation of {@link CryptoApi#setDeviceIsolationMode}.
*/
public setCryptoMode(cryptoMode: CryptoMode): void {
this.cryptoMode = cryptoMode;
public setDeviceIsolationMode(isolationMode: DeviceIsolationMode): void {
this.deviceIsolationMode = isolationMode;
}

/**
Expand Down Expand Up @@ -1776,24 +1778,25 @@ class EventDecryptor {
private readonly perSessionBackupDownloader: PerSessionKeyBackupDownloader,
) {}

public async attemptEventDecryption(event: MatrixEvent, cryptoMode: CryptoMode): Promise<IEventDecryptionResult> {
public async attemptEventDecryption(
event: MatrixEvent,
isolationMode: DeviceIsolationMode,
): Promise<IEventDecryptionResult> {
// 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.
// (fixes https://github.com/vector-im/element-web/issues/5001)
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 {
Expand Down

0 comments on commit 538717c

Please sign in to comment.