Skip to content

MatrixRTC: Refactor | Introduce a new Encryption manager (used with experimental to device transport) #4799

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

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
404 changes: 404 additions & 0 deletions spec/unit/matrixrtc/BasicEncrytionManager.spec.ts

Large diffs are not rendered by default.

70 changes: 70 additions & 0 deletions spec/unit/matrixrtc/KeyBuffer.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
Copyright 2025 The Matrix.org Foundation C.I.C.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

import { KeyBuffer } from "../../../src/matrixrtc/utils.ts";
import { type InboundEncryptionSession } from "../../../src/matrixrtc";

describe("KeyBuffer Test", () => {
it("Should buffer and disambiguate keys by timestamp", () => {
jest.useFakeTimers();

const buffer = new KeyBuffer(1000);

const aKey = fakeInboundSessionWithTimestamp(1000);
const olderKey = fakeInboundSessionWithTimestamp(300);
// Simulate receiving out of order keys

const init = buffer.disambiguate(aKey.participantId, aKey);
expect(init).toEqual(aKey);
// Some time pass
jest.advanceTimersByTime(600);
// Then we receive the most recent key out of order

const key = buffer.disambiguate(aKey.participantId, olderKey);
// this key is older and should be ignored even if received after
expect(key).toBe(null);
});

it("Should clear buffer after ttl", () => {
jest.useFakeTimers();

const buffer = new KeyBuffer(1000);

const aKey = fakeInboundSessionWithTimestamp(1000);
const olderKey = fakeInboundSessionWithTimestamp(300);
// Simulate receiving out of order keys

const init = buffer.disambiguate(aKey.participantId, aKey);
expect(init).toEqual(aKey);

// Similar to previous test but there is too much delay
// We don't want to keep key material for too long
jest.advanceTimersByTime(1200);

const key = buffer.disambiguate(aKey.participantId, olderKey);
// The buffer is cleared so should return this key
expect(key).toBe(olderKey);
});

function fakeInboundSessionWithTimestamp(ts: number): InboundEncryptionSession {
return {
keyId: 0,
creationTS: ts,
participantId: "@alice:localhost|ABCDE",
key: new Uint8Array(16),
};
}
});
1 change: 1 addition & 0 deletions spec/unit/matrixrtc/ToDeviceKeyTransport.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ describe("ToDeviceKeyTransport", () => {
call_id: "",
scope: "m.room",
},
sent_ts: expect.any(Number),
},
);

Expand Down
237 changes: 237 additions & 0 deletions src/matrixrtc/BasicEncryptionManager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,237 @@
/*
Copyright 2025 The Matrix.org Foundation C.I.C.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

import { type IEncryptionManager } from "./EncryptionManager.ts";
import { type EncryptionConfig } from "./MatrixRTCSession.ts";
import { type CallMembership } from "./CallMembership.ts";
import { decodeBase64, encodeBase64 } from "../base64.ts";
import { type KeyTransportEventListener, KeyTransportEvents } from "./IKeyTransport.ts";
import { type Logger, logger } from "../logger.ts";
import { defer } from "../utils.ts";
import { type ToDeviceKeyTransport } from "./ToDeviceKeyTransport.ts";
import { type InboundEncryptionSession, type ParticipantId, type Statistics } from "./types.ts";
import { KeyBuffer } from "./utils.ts";

type DeviceInfo = {
userId: string;
deviceId: string;
};

type OutboundEncryptionSession = {
key: Uint8Array;
creationTS: number;
sharedWith: Array<DeviceInfo>;
// This is an index acting as the id of the key
keyId: number;
};

/**
* A simple encryption manager.
* This manager is basic becasue it will rotate the keys for any membership change.
* There is no ratchetting, or time based rotation.
* It works with to-device transport.
*/
export class BasicEncryptionManager implements IEncryptionManager {
// The current per-sender media key for this device
private outboundSession: OutboundEncryptionSession | null = null;

/**
* Ensures that there is only one distribute operation at a time for that call.
*/
private currentKeyDistributionPromise: Promise<void> | null = null;

/**
* There is a possibility that keys arrive in wrong order.
* For example after a quick join/leave/join, there will be 2 keys of index 0 distributed and
* it they are received in wrong order the stream won't be decryptable.
* For that reason we keep a small buffer of keys for a limited time to disambiguate.
* @private
*/
private keyBuffer = new KeyBuffer(1000 /** 1 second */);

private logger: Logger;

private needToRotateAgain = false;

public constructor(
private userId: string,
private deviceId: string,
private getMemberships: () => CallMembership[],
private transport: ToDeviceKeyTransport,
private statistics: Statistics,
private onEncryptionKeysChanged: (
keyBin: Uint8Array<ArrayBufferLike>,
encryptionKeyIndex: number,
participantId: ParticipantId,
) => void,
) {
this.logger = logger.getChild("BasicEncryptionManager");
}

public getEncryptionKeys(): Map<string, Array<{ key: Uint8Array; timestamp: number }>> {
// This is deprecated should be ignored. Only use by tests?
return new Map();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deprecation could already be added to IEncryptionManager (@deprecated)


public join(joinConfig: EncryptionConfig | undefined): void {
this.logger.info(`Joining room`);
this.transport.on(KeyTransportEvents.ReceivedKeys, this.onNewKeyReceived);
this.transport.start();

this.ensureMediaKey();
}

/**
* Will ensure that a new key is distributed and used to encrypt our media.
* If this function is called repeatidly, the calls will be buffered to a single key rotation.
*/
private ensureMediaKey(): void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private ensureMediaKey(): void {
private scheduleKeyRotation(): void {

We don't use media in most places. And ensure does not imply that calling this will result in sending keys over the chosen transport. I also think the name should make clear that we generate a new key (for a new index).
I think we should include the following words in the name:

  • rotation
  • sending (maybe implied by rotation?)
  • Key
  • schedule/ensure

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The manager has changed a bit so this call is really ensuring that everyone has the keys it needs. So it will not rotate key anymore.
it will be no op if no action needed.
I have no strong opinion on the name to use, maybe ensureOutboundKey ?

if (this.currentKeyDistributionPromise == null) {
this.logger.debug(`No active rollout, start a new one`);
// start a rollout
this.currentKeyDistributionPromise = this.rolloutOutboundKey().then(() => {
this.logger.debug(`Rollout completed`);
this.currentKeyDistributionPromise = null;
if (this.needToRotateAgain) {
this.logger.debug(`New Rollout needed`);
// rollout a new one
this.ensureMediaKey();
}
});
} else {
// There is a rollout in progress, but membership has changed and a new rollout is needed.
// Remember that a new rotation in needed after the current one.
this.logger.debug(`Rollout in progress, a new rollout will be started after the current one`);
this.needToRotateAgain = true;
}
}

public leave(): void {
this.keyBuffer.clear();
this.transport.off(KeyTransportEvents.ReceivedKeys, this.onNewKeyReceived);
this.transport.stop();
}

public onNewKeyReceived: KeyTransportEventListener = (userId, deviceId, keyBase64Encoded, index, timestamp) => {
this.logger.debug(`Received key over transport ${userId}:${deviceId} at index ${index}`);

// We have a new key notify the video layer of this new key so that it can decrypt the frames properly.
// We also store a copy of the key in the key store as we might need to re-emit them to the decoding layer.
const participantId = getParticipantId(userId, deviceId);
const keyBin = decodeBase64(keyBase64Encoded);
const newKey: InboundEncryptionSession = {
key: keyBin,
participantId,
keyId: index,
creationTS: timestamp,
};

const validKey = this.keyBuffer.disambiguate(participantId, newKey);
if (validKey) {
this.onEncryptionKeysChanged(validKey.key, index, validKey.participantId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this make me wonder if we are mixing event emission and "callbacks in constructors" too much in the matrixrtc part of the js-sdk.

If onEncryptionKeysChanged does nothing else but emitting the key when this is called, It might be nicer to use the reemitter pattern like we do for the transport change event.
IIRC the session will just emit the key change and the new key will be consumed by element-call/livekit.
So reemission (if there is not processing happening on the MatrixRTCSession level) sounds like it is easier to read. especially in line 144 this would make it super clear that this is where the key leaves the EncryptionManager.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can sanity check other locations where we do "callbacks in constructors" and see what makes the most sense to makt it consistent.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are other things that can be cleaned in the constructor. I don't think we also need the getMembership (if we could just pass it in the join)

this.statistics.counters.roomEventEncryptionKeysReceived += 1;
} else {
this.logger.info(`Received an out of order key for ${userId}:${deviceId}, dropping it`);
}
};

/**
* Called when the membership of the call changes.
* This encryption manager is very basic, it will rotate the key everytime this is called.
* @param oldMemberships
*/
public onMembershipsUpdate(oldMemberships: CallMembership[]): void {
this.logger.trace(`onMembershipsUpdate`);

// This encryption manager is very basic, it will rotate the key for any membership change
// Request rotation of the key
this.ensureMediaKey();
}

private async rolloutOutboundKey(): Promise<void> {
const hasExistingKey = this.outboundSession != null;

// Create a new key
const newOutboundKey: OutboundEncryptionSession = {
key: this.generateRandomKey(),
creationTS: Date.now(),
sharedWith: [],
keyId: this.nextKeyId(),
};

this.logger.info(`creating new outbound key index:${newOutboundKey.keyId}`);
// Set this new key has the current one
this.outboundSession = newOutboundKey;
this.needToRotateAgain = false;
const toShareWith = this.getMemberships();

try {
this.logger.trace(`Sending key...`);
await this.transport.sendKey(encodeBase64(newOutboundKey.key), newOutboundKey.keyId, toShareWith);
this.statistics.counters.roomEventEncryptionKeysSent += 1;
newOutboundKey.sharedWith = toShareWith.map((ms) => {
return {
userId: ms.sender ?? "",
deviceId: ms.deviceId ?? "",
};
});
this.logger.trace(
`key index:${newOutboundKey.keyId} sent to ${newOutboundKey.sharedWith.map((m) => `${m.userId}:${m.deviceId}`).join(",")}`,
);
if (!hasExistingKey) {
this.logger.trace(`Rollout immediately`);
// rollout immediately
this.onEncryptionKeysChanged(
newOutboundKey.key,
newOutboundKey.keyId,
getParticipantId(this.userId, this.deviceId),
);
} else {
// Delay a bit using this key
const rolledOut = defer<void>();
this.logger.trace(`Delay Rollout...`);
setTimeout(() => {
this.logger.trace(`...Delayed rollout of index:${newOutboundKey.keyId} `);
// Start encrypting with that key now that there was time to distibute it
this.onEncryptionKeysChanged(
newOutboundKey.key,
newOutboundKey.keyId,
getParticipantId(this.userId, this.deviceId),
);
rolledOut.resolve();
}, 1000);
return rolledOut.promise;
}
} catch (err) {
this.logger.error(`Failed to rollout key`, err);
}
}

private nextKeyId(): number {
if (this.outboundSession) {
return (this.outboundSession!.keyId + 1) % 256;
}
return 0;
}

private generateRandomKey(): Uint8Array {
const key = new Uint8Array(16);
globalThis.crypto.getRandomValues(key);
return key;
}
}

const getParticipantId = (userId: string, deviceId: string): ParticipantId => `${userId}:${deviceId}`;
31 changes: 31 additions & 0 deletions src/matrixrtc/MatrixRTCSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { ToDeviceKeyTransport } from "./ToDeviceKeyTransport.ts";
import { type Statistics } from "./types.ts";
import { RoomKeyTransport } from "./RoomKeyTransport.ts";
import type { IMembershipManager } from "./IMembershipManager.ts";
import { BasicEncryptionManager } from "./BasicEncryptionManager.ts";

export enum MatrixRTCSessionEvent {
// A member joined, left, or updated a property of their membership.
Expand Down Expand Up @@ -394,8 +395,38 @@ export class MatrixRTCSession extends TypedEventEmitter<MatrixRTCSessionEvent, M
this.statistics,
this.logger,
);
this.encryptionManager = new BasicEncryptionManager(
this.client.getUserId()!,
this.client.getDeviceId()!,
() => this.memberships,
transport,
this.statistics,
(keyBin: Uint8Array<ArrayBufferLike>, encryptionKeyIndex: number, participantId: string) => {
this.emit(
MatrixRTCSessionEvent.EncryptionKeyChanged,
keyBin,
encryptionKeyIndex,
participantId,
);
},
);
} else {
transport = new RoomKeyTransport(this.roomSubset, this.client, this.statistics);
this.encryptionManager = new EncryptionManager(
this.client.getUserId()!,
this.client.getDeviceId()!,
() => this.memberships,
transport,
this.statistics,
(keyBin: Uint8Array<ArrayBufferLike>, encryptionKeyIndex: number, participantId: string) => {
this.emit(
MatrixRTCSessionEvent.EncryptionKeyChanged,
keyBin,
encryptionKeyIndex,
participantId,
);
},
);
}
this.encryptionManager = new EncryptionManager(
this.client.getUserId()!,
Expand Down
1 change: 1 addition & 0 deletions src/matrixrtc/ToDeviceKeyTransport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export class ToDeviceKeyTransport
application: "m.call",
scope: "m.room",
},
sent_ts: Date.now(),
};

const targets = members
Expand Down
12 changes: 12 additions & 0 deletions src/matrixrtc/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,23 @@ limitations under the License.
import type { IMentions } from "../matrix.ts";
import type { CallMembership } from "./CallMembership.ts";

export type ParticipantId = string;

export interface EncryptionKeyEntry {
index: number;
key: string;
}

/**
* A type representing the information needed to decrypt video streams.
*/
export type InboundEncryptionSession = {
key: Uint8Array;
participantId: ParticipantId;
keyId: number;
creationTS: number;
};

export interface EncryptionKeysEventContent {
keys: EncryptionKeyEntry[];
device_id: string;
Expand Down
Loading