Skip to content

Commit

Permalink
crypto: configure key sharing strategy based on cryptoMode
Browse files Browse the repository at this point in the history
  • Loading branch information
@BillCarsonFr
BillCarsonFr committed Sep 23, 2024
1 parent dbd7d26 commit 522b369
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 14 deletions.
97 changes: 96 additions & 1 deletion spec/unit/rust-crypto/RoomEncryptor.spec.ts
View file
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 { CryptoMode } 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(undefined),
} as unknown as Mocked<Room>;

roomEncryptor = new RoomEncryptor(
Expand Down Expand Up @@ -181,5 +183,98 @@ describe("RoomEncryptor", () => {

expect(firstMessageFinished).toBe("hello");
});

describe("CryptoMode", () => {
type TestCase = [
string,
{ mode: CryptoMode; expectedStrategy: CollectStrategy; globalBlacklistUnverifiedDevices: boolean },
];

const testCases: TestCase[] = [
[
"Share Legacy CryptoMode",
{
mode: CryptoMode.Legacy,
expectedStrategy: CollectStrategy.deviceBasedStrategy(false, false),
globalBlacklistUnverifiedDevices: false,
},
],
[
"Share Legacy CryptoMode - blacklist true",
{
mode: CryptoMode.Legacy,
expectedStrategy: CollectStrategy.deviceBasedStrategy(true, false),
globalBlacklistUnverifiedDevices: true,
},
],
[
"Share Invisible CryptoMode - blacklist true",
{
mode: CryptoMode.Invisible,
expectedStrategy: CollectStrategy.identityBasedStrategy(),
globalBlacklistUnverifiedDevices: true,
},
],
[
"Share Invisible CryptoMode",
{
mode: CryptoMode.Invisible,
expectedStrategy: CollectStrategy.identityBasedStrategy(),
globalBlacklistUnverifiedDevices: false,
},
],
[
"Share Transition CryptoMode - blacklist true",
{
mode: CryptoMode.Transition,
expectedStrategy: CollectStrategy.deviceBasedStrategy(false, true),
globalBlacklistUnverifiedDevices: true,
},
],
[
"Share Transition CryptoMode",
{
mode: CryptoMode.Transition,
expectedStrategy: CollectStrategy.deviceBasedStrategy(false, true),
globalBlacklistUnverifiedDevices: false,
},
],
];

let capturedSettings: CollectStrategy | undefined = undefined;

beforeEach(() => {
capturedSettings = undefined;
mockOlmMachine.shareRoomKey.mockImplementationOnce(async (roomId, users, encryptionSettings) => {
capturedSettings = encryptionSettings.sharingStrategy;
});
});

it("should use Legacy as default mode", async () => {
await roomEncryptor.prepareForEncryption(false);
expect(mockOlmMachine.shareRoomKey).toHaveBeenCalled();
expect(capturedSettings?.eq(CollectStrategy.deviceBasedStrategy(false, false))).toBeTruthy();
});

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();
},
);
});
});
});
49 changes: 38 additions & 11 deletions src/rust-crypto/RoomEncryptor.ts
View file
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 { CryptoMode } from "../crypto-api";

Check failure on line 39 in src/rust-crypto/RoomEncryptor.ts

View workflow job for this annotation

GitHub Actions / ESLint 

require file extension '.ts'

/**
* RoomEncryptor: responsible for encrypting messages to a given room
Expand Down Expand Up @@ -121,16 +122,20 @@ export class RoomEncryptor {
* in the room.
*
* @param globalBlacklistUnverifiedDevices - When `true`, it will not send encrypted messages to unverified devices
* @param cryptoMode - The crypto mode see {@link CryptoMode}
*/
public async prepareForEncryption(globalBlacklistUnverifiedDevices: boolean): Promise<void> {
public async prepareForEncryption(
globalBlacklistUnverifiedDevices: boolean,
cryptoMode: CryptoMode = CryptoMode.Legacy,
): 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, cryptoMode);
}

/**
Expand All @@ -141,8 +146,13 @@ export class RoomEncryptor {
*
* @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 cryptoMode - The crypto mode see {@link CryptoMode}.
*/
public encryptEvent(event: MatrixEvent | null, globalBlacklistUnverifiedDevices: boolean): Promise<void> {
public encryptEvent(
event: MatrixEvent | null,
globalBlacklistUnverifiedDevices: boolean,
cryptoMode: CryptoMode = CryptoMode.Legacy,
): 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 +163,7 @@ export class RoomEncryptor {
})
.then(async () => {
await logDuration(logger, "ensureEncryptionSession", async () => {
await this.ensureEncryptionSession(logger, globalBlacklistUnverifiedDevices);
await this.ensureEncryptionSession(logger, globalBlacklistUnverifiedDevices, cryptoMode);
});
if (event) {
await logDuration(logger, "encryptEventInner", async () => {
Expand All @@ -174,8 +184,13 @@ export class RoomEncryptor {
*
* @param logger - a place to write diagnostics to
* @param globalBlacklistUnverifiedDevices - When `true`, it will not send encrypted messages to unverified devices
* @param cryptoMode - The crypto mode see {@link CryptoMode}.
*/
private async ensureEncryptionSession(logger: LogSpan, globalBlacklistUnverifiedDevices: boolean): Promise<void> {
private async ensureEncryptionSession(
logger: LogSpan,
globalBlacklistUnverifiedDevices: boolean,
cryptoMode: CryptoMode,
): 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 +266,24 @@ 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 (cryptoMode) {
case CryptoMode.Legacy:
// 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);
}
break;
case CryptoMode.Transition:
// Transition mode is still subject to changes, for now in order to not exclude users with no identity or
// unsigned devices we will use a device based strategy.
rustEncryptionSettings.sharingStrategy = CollectStrategy.deviceBasedStrategy(false, true);
break;
case CryptoMode.Invisible:
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,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.cryptoMode);
}

public async decryptEvent(event: MatrixEvent): Promise<IEventDecryptionResult> {
Expand Down Expand Up @@ -398,7 +398,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.cryptoMode);
}
}

Expand Down

0 comments on commit 522b369

Please sign in to comment.