Skip to content

Commit

Permalink
Do not rotate MatrixRTC media encryption key when a new member joins …
Browse files Browse the repository at this point in the history
…a session (#4472)

* Do not rotate MatrixRTC media encryption key when a new member joins a call

This change reverts #4422.

Instead, the rotation when a new member joins will be reintroduced as part of supporting to-device based MatrixRTC encryption key distribution.

* Improve function name
  • Loading branch information
hughns authored Oct 25, 2024
1 parent 3cc3bd0 commit 0a29063
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 20 deletions.
23 changes: 9 additions & 14 deletions spec/unit/matrixrtc/MatrixRTCSession.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,7 @@ describe("MatrixRTCSession", () => {
expect(client.cancelPendingEvent).toHaveBeenCalledWith(eventSentinel);
});

it("rotates key if a new member joins", async () => {
it("Re-sends key if a new member joins", async () => {
jest.useFakeTimers();
try {
const mockRoom = makeMockRoom([membershipTemplate]);
Expand All @@ -784,9 +784,7 @@ describe("MatrixRTCSession", () => {
});

sess.joinRoomSession([mockFocus], mockFocus, { manageMediaKeys: true });
const firstKeysPayload = await keysSentPromise1;
expect(firstKeysPayload.keys).toHaveLength(1);
expect(firstKeysPayload.keys[0].index).toEqual(0);
await keysSentPromise1;
expect(sess!.statistics.counters.roomEventEncryptionKeysSent).toEqual(1);

sendEventMock.mockClear();
Expand All @@ -808,14 +806,9 @@ describe("MatrixRTCSession", () => {
.mockReturnValue(makeMockRoomState([membershipTemplate, member2], mockRoom.roomId));
sess.onMembershipUpdate();

jest.advanceTimersByTime(10000);

const secondKeysPayload = await keysSentPromise2;
await keysSentPromise2;

expect(sendEventMock).toHaveBeenCalled();
expect(secondKeysPayload.keys).toHaveLength(1);
expect(secondKeysPayload.keys[0].index).toEqual(1);
expect(secondKeysPayload.keys[0].key).not.toEqual(firstKeysPayload.keys[0].key);
expect(sess!.statistics.counters.roomEventEncryptionKeysSent).toEqual(2);
} finally {
jest.useRealTimers();
Expand Down Expand Up @@ -1103,8 +1096,8 @@ describe("MatrixRTCSession", () => {
}
jest.useFakeTimers();
try {
// start with a single member
const mockRoom = makeMockRoom(members.slice(0, 1));
// start with all members
const mockRoom = makeMockRoom(members);

for (let i = 0; i < membersToTest; i++) {
const keysSentPromise = new Promise<EncryptionKeysEventContent>((resolve) => {
Expand All @@ -1116,10 +1109,12 @@ describe("MatrixRTCSession", () => {
sess = MatrixRTCSession.roomSessionForRoom(client, mockRoom);
sess.joinRoomSession([mockFocus], mockFocus, { manageMediaKeys: true });
} else {
// otherwise update the state
// otherwise update the state reducing the membership each time in order to trigger key rotation
mockRoom.getLiveTimeline().getState = jest
.fn()
.mockReturnValue(makeMockRoomState(members.slice(0, i + 1), mockRoom.roomId));
.mockReturnValue(
makeMockRoomState(members.slice(0, membersToTest - i), mockRoom.roomId),
);
}

sess!.onMembershipUpdate();
Expand Down
12 changes: 6 additions & 6 deletions src/matrixrtc/MatrixRTCSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ export class MatrixRTCSession extends TypedEventEmitter<MatrixRTCSessionEvent, M
logger.info(`Joining call session in room ${this.room.roomId} with manageMediaKeys=${this.manageMediaKeys}`);
if (joinConfig?.manageMediaKeys) {
this.makeNewSenderKey();
this.requestKeyEventSend();
this.requestSendCurrentKey();
}
// We don't wait for this, mostly because it may fail and schedule a retry, so this
// function returning doesn't really mean anything at all.
Expand Down Expand Up @@ -546,10 +546,10 @@ export class MatrixRTCSession extends TypedEventEmitter<MatrixRTCSessionEvent, M
}

/**
* Requests that we resend our keys to the room. May send a keys event immediately
* Requests that we resend our current keys to the room. May send a keys event immediately
* or queue for alter if one has already been sent recently.
*/
private requestKeyEventSend(): void {
private requestSendCurrentKey(): void {
if (!this.manageMediaKeys) return;

if (
Expand Down Expand Up @@ -795,8 +795,8 @@ export class MatrixRTCSession extends TypedEventEmitter<MatrixRTCSessionEvent, M
logger.debug(`Member(s) have left: queueing sender key rotation`);
this.makeNewKeyTimeout = setTimeout(this.onRotateKeyTimeout, MAKE_KEY_DELAY);
} else if (anyJoined) {
logger.debug(`New member(s) have joined: queueing sender key rotation`);
this.makeNewKeyTimeout = setTimeout(this.onRotateKeyTimeout, MAKE_KEY_DELAY);
logger.debug(`New member(s) have joined: re-sending keys`);
this.requestSendCurrentKey();
} else if (oldFingerprints) {
// does it look like any of the members have updated their memberships?
const newFingerprints = this.lastMembershipFingerprints!;
Expand All @@ -808,7 +808,7 @@ export class MatrixRTCSession extends TypedEventEmitter<MatrixRTCSessionEvent, M
Array.from(newFingerprints).some((x) => !oldFingerprints.has(x));
if (candidateUpdates) {
logger.debug(`Member(s) have updated/reconnected: re-sending keys to everyone`);
this.requestKeyEventSend();
this.requestSendCurrentKey();
}
}
}
Expand Down

0 comments on commit 0a29063

Please sign in to comment.