Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto: configure key sharing strategy based on DeviceIsolationMode #4425

Merged
merged 7 commits into from
Sep 30, 2024
121 changes: 112 additions & 9 deletions spec/unit/rust-crypto/RoomEncryptor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/

import {
CollectStrategy,
Curve25519PublicKey,
Ed25519PublicKey,
HistoryVisibility as RustHistoryVisibility,
Expand All @@ -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", () => {
Expand Down Expand Up @@ -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(
Expand All @@ -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>();
Expand All @@ -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.encryptEvent(createMockEvent("Hello"), false);
roomEncryptor.prepareForEncryption(false);
roomEncryptor.encryptEvent(createMockEvent("World"), false);
roomEncryptor.prepareForEncryption(false, defaultDevicesIsolationMode);
roomEncryptor.encryptEvent(createMockEvent("Hello"), false, defaultDevicesIsolationMode);
roomEncryptor.prepareForEncryption(false, defaultDevicesIsolationMode);
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);
Expand All @@ -158,8 +162,16 @@ describe("RoomEncryptor", () => {

let firstMessageFinished: string | null = null;

const firstRequest = roomEncryptor.encryptEvent(createMockEvent("Hello"), false);
const secondRequest = roomEncryptor.encryptEvent(createMockEvent("Edit of Hello"), false);
const firstRequest = roomEncryptor.encryptEvent(
createMockEvent("Hello"),
false,
defaultDevicesIsolationMode,
);
const secondRequest = roomEncryptor.encryptEvent(
createMockEvent("Edit of Hello"),
false,
defaultDevicesIsolationMode,
);

firstRequest.then(() => {
if (firstMessageFinished === null) {
Expand All @@ -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();
},
);
});
});
});
5 changes: 2 additions & 3 deletions src/crypto-api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
* `CryptoMode.Transition`.
* only be used if the device isolation mode is set to `OnlySignedDevicesIsolationMode`.
*/
UNKNOWN_SENDER_DEVICE = "UNKNOWN_SENDER_DEVICE",

Expand Down
60 changes: 45 additions & 15 deletions src/rust-crypto/RoomEncryptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -119,18 +120,23 @@ 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`, 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}
richvdh marked this conversation as resolved.
Show resolved Hide resolved
*/
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
// message is finally sent.
// 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);
}

/**
Expand All @@ -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.
Expand All @@ -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 () => {
Expand All @@ -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}'`,
Expand Down Expand Up @@ -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
// See Room#getBlacklistUnverifiedDevices
if (this.room.getBlacklistUnverifiedDevices() ?? globalBlacklistUnverifiedDevices) {
rustEncryptionSettings.sharingStrategy = CollectStrategy.deviceBasedStrategy(true, false);
} else {
rustEncryptionSettings.sharingStrategy = CollectStrategy.deviceBasedStrategy(false, false);
switch (deviceIsolationMode.kind) {
case DeviceIsolationModeKind.AllDevicesIsolationMode:
{
// When this.room.getBlacklistUnverifiedDevices() === null, the global settings should be used
// See Room#getBlacklistUnverifiedDevices
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 () => {
Expand Down
4 changes: 2 additions & 2 deletions src/rust-crypto/rust-crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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);
}
}

Expand Down