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.

Sign up for GitHub

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

Do not rotate MatrixRTC media encryption key when a new member joins a session #4472

Merged
merged 2 commits into from
Oct 25, 2024
Merged

Do not rotate MatrixRTC media encryption key when a new member joins a session #4472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e69d91b
Do not rotate MatrixRTC media encryption key when a new member joins …
hughns Oct 25, 2024
fb4d315
Improve function name
hughns Oct 25, 2024
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
23 changes: 9 additions & 14 deletions spec/unit/matrixrtc/MatrixRTCSession.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,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 @@ -780,9 +780,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 @@ -804,14 +802,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 @@ -1099,8 +1092,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 @@ -1112,10 +1105,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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,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 @@ -548,10 +548,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 @@ -797,8 +797,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 @@ -810,7 +810,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
Loading