Skip to content

Commit fdfddde

Browse files
authored
Import room key bundles received after invite. (#5080)
* feat: Import room key bundles when received after invite. * tests: Add spec test for room key bundle arriving after invite accepted. * chore: Fix code quality issue (unnecessary async function). * docs: Tidy up comments. * refactor: Simplify key bundle importing after invite to one entrypoint. - Remove `onReceiveToDeviceEvent` from `CryptoBackend`. - Copy old room key bundle importing logic to `preprocessToDeviceEvents`. * refactor: Move late bundle importing to main preprocess loop. * fix: Use `Map` over `Record` to prevent prototype pollution.
1 parent 0ecfef2 commit fdfddde

File tree

4 files changed

+187
-6
lines changed

4 files changed

+187
-6
lines changed

spec/integ/crypto/history-sharing.spec.ts

Lines changed: 102 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import {
3030
import { E2EKeyReceiver } from "../../test-utils/E2EKeyReceiver.ts";
3131
import { SyncResponder } from "../../test-utils/SyncResponder.ts";
3232
import { mockInitialApiRequests, mockSetupCrossSigningRequests } from "../../test-utils/mockEndpoints.ts";
33-
import { getSyncResponse, mkEventCustom, syncPromise } from "../../test-utils/test-utils.ts";
33+
import { getSyncResponse, mkEventCustom, syncPromise, waitFor } from "../../test-utils/test-utils.ts";
3434
import { E2EKeyResponder } from "../../test-utils/E2EKeyResponder.ts";
3535
import { flushPromises } from "../../test-utils/flushPromises.ts";
3636
import { E2EOTKClaimResponder } from "../../test-utils/E2EOTKClaimResponder.ts";
@@ -80,6 +80,9 @@ describe("History Sharing", () => {
8080
let bobSyncResponder: SyncResponder;
8181

8282
beforeEach(async () => {
83+
// Reset mocks.
84+
fetchMock.reset();
85+
8386
// anything that we don't have a specific matcher for silently returns a 404
8487
fetchMock.catch(404);
8588
fetchMock.config.warnOnFallback = false;
@@ -201,6 +204,104 @@ describe("History Sharing", () => {
201204
expect(event.getContent().body).toEqual("Hi!");
202205
});
203206

207+
test("Room keys are imported correctly if invite is accepted before the bundle arrives", async () => {
208+
// Alice is in an encrypted room
209+
const syncResponse = getSyncResponse([aliceClient.getSafeUserId()], ROOM_ID);
210+
aliceSyncResponder.sendOrQueueSyncResponse(syncResponse);
211+
await syncPromise(aliceClient);
212+
213+
// ... and she sends an event
214+
const msgProm = expectSendRoomEvent(ALICE_HOMESERVER_URL, "m.room.encrypted");
215+
await aliceClient.sendEvent(ROOM_ID, EventType.RoomMessage, { msgtype: MsgType.Text, body: "Hello!" });
216+
const sentMessage = await msgProm;
217+
debug(`Alice sent encrypted room event: ${JSON.stringify(sentMessage)}`);
218+
219+
// Now, Alice invites Bob
220+
const uploadProm = new Promise<Uint8Array>((resolve) => {
221+
fetchMock.postOnce(new URL("/_matrix/media/v3/upload", ALICE_HOMESERVER_URL).toString(), (url, request) => {
222+
const body = request.body as Uint8Array;
223+
debug(`Alice uploaded blob of length ${body.length}`);
224+
resolve(body);
225+
return { content_uri: "mxc://alice-server/here" };
226+
});
227+
});
228+
const toDeviceMessageProm = expectSendToDeviceMessage(ALICE_HOMESERVER_URL, "m.room.encrypted");
229+
// POST https://alice-server.com/_matrix/client/v3/rooms/!room%3Aexample.com/invite
230+
fetchMock.postOnce(`${ALICE_HOMESERVER_URL}/_matrix/client/v3/rooms/${encodeURIComponent(ROOM_ID)}/invite`, {});
231+
await aliceClient.invite(ROOM_ID, bobClient.getSafeUserId(), { shareEncryptedHistory: true });
232+
const uploadedBlob = await uploadProm;
233+
const sentToDeviceRequest = await toDeviceMessageProm;
234+
debug(`Alice sent encrypted to-device events: ${JSON.stringify(sentToDeviceRequest)}`);
235+
const bobToDeviceMessage = sentToDeviceRequest[bobClient.getSafeUserId()][bobClient.deviceId!];
236+
expect(bobToDeviceMessage).toBeDefined();
237+
238+
// Bob receives the room invite, but not the room key bundle
239+
const inviteEvent = mkEventCustom({
240+
type: "m.room.member",
241+
sender: aliceClient.getSafeUserId(),
242+
state_key: bobClient.getSafeUserId(),
243+
content: { membership: KnownMembership.Invite },
244+
});
245+
bobSyncResponder.sendOrQueueSyncResponse({
246+
rooms: { invite: { [ROOM_ID]: { invite_state: { events: [inviteEvent] } } } },
247+
});
248+
await syncPromise(bobClient);
249+
250+
const room = bobClient.getRoom(ROOM_ID);
251+
expect(room).toBeTruthy();
252+
expect(room?.getMyMembership()).toEqual(KnownMembership.Invite);
253+
254+
fetchMock.postOnce(`${BOB_HOMESERVER_URL}/_matrix/client/v3/join/${encodeURIComponent(ROOM_ID)}`, {
255+
room_id: ROOM_ID,
256+
});
257+
await bobClient.joinRoom(ROOM_ID, { acceptSharedHistory: true });
258+
259+
// Bob receives and attempts to decrypt the megolm message, but should not be able to (yet).
260+
const bobSyncResponse = getSyncResponse([aliceClient.getSafeUserId(), bobClient.getSafeUserId()], ROOM_ID);
261+
bobSyncResponse.rooms.join[ROOM_ID].timeline.events.push(
262+
mkEventCustom({
263+
type: "m.room.encrypted",
264+
sender: aliceClient.getSafeUserId(),
265+
content: sentMessage,
266+
event_id: "$event_id",
267+
}) as any,
268+
);
269+
bobSyncResponder.sendOrQueueSyncResponse(bobSyncResponse);
270+
await syncPromise(bobClient);
271+
const bobRoom = bobClient.getRoom(ROOM_ID);
272+
const event = bobRoom!.getLastLiveEvent()!;
273+
expect(event.getId()).toEqual("$event_id");
274+
await event.getDecryptionPromise();
275+
expect(event.isDecryptionFailure()).toBeTruthy();
276+
277+
// Now the room key bundle message arrives
278+
fetchMock.getOnce(
279+
`begin:${BOB_HOMESERVER_URL}/_matrix/client/v1/media/download/alice-server/here`,
280+
{ body: uploadedBlob },
281+
{ sendAsJson: false },
282+
);
283+
bobSyncResponder.sendOrQueueSyncResponse({
284+
to_device: {
285+
events: [
286+
{
287+
type: "m.room.encrypted",
288+
sender: aliceClient.getSafeUserId(),
289+
content: bobToDeviceMessage,
290+
},
291+
],
292+
},
293+
});
294+
await syncPromise(bobClient);
295+
296+
// Once the room key bundle finishes downloading, we should be able to decrypt the message.
297+
await waitFor(async () => {
298+
await event.getDecryptionPromise();
299+
expect(event.isDecryptionFailure()).toBeFalsy();
300+
expect(event.getType()).toEqual("m.room.message");
301+
expect(event.getContent().body).toEqual("Hello!");
302+
});
303+
});
304+
204305
afterEach(async () => {
205306
bobClient.stopClient();
206307
aliceClient.stopClient();

src/client.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2406,7 +2406,12 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
24062406

24072407
const roomId = res.room_id;
24082408
if (opts.acceptSharedHistory && inviter && this.cryptoBackend) {
2409-
await this.cryptoBackend.maybeAcceptKeyBundle(roomId, inviter);
2409+
// Try to accept the room key bundle specified in a `m.room_key_bundle` to-device message we (might have) already received.
2410+
const bundleDownloaded = await this.cryptoBackend.maybeAcceptKeyBundle(roomId, inviter);
2411+
// If this fails, i.e. we haven't received this message yet, we need to wait until the to-device message arrives.
2412+
if (!bundleDownloaded) {
2413+
this.cryptoBackend.markRoomAsPendingKeyBundle(roomId, inviter);
2414+
}
24102415
}
24112416

24122417
// In case we were originally given an alias, check the room cache again

src/common-crypto/CryptoBackend.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,20 @@ export interface CryptoBackend extends SyncCryptoCallbacks, CryptoApi {
9090
*
9191
* @param inviter - The user who invited us to the room and is expected to have
9292
* sent the room key bundle.
93+
*
94+
* @returns `true` if the key bundle was successfuly downloaded and imported.
95+
*/
96+
maybeAcceptKeyBundle(roomId: string, inviter: string): Promise<boolean>;
97+
98+
/**
99+
* Mark a room as pending a key bundle under MSC4268. The backend will listen for room key bundle messages, and if
100+
* it sees one matching the room specified, it will automatically import it as long as the message author's ID matches
101+
* the inviter's ID.
102+
*
103+
* @param roomId - The room we were invited to, for which we did not receive a key bundle before accepting the invite.
104+
* @param inviterId - The user who invited us to the room and is expected to send the room key bundle.
93105
*/
94-
maybeAcceptKeyBundle(roomId: string, inviter: string): Promise<void>;
106+
markRoomAsPendingKeyBundle(roomId: string, inviterId: string): void;
95107
}
96108

97109
/** The methods which crypto implementations should expose to the Sync api

src/rust-crypto/rust-crypto.ts

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,9 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, CryptoEventH
129129
/** mapping of roomId → encryptor class */
130130
private roomEncryptors: Record<string, RoomEncryptor> = {};
131131

132+
/** mapping of room ID -> inviter ID for rooms pending MSC4268 key bundles */
133+
private readonly roomsPendingKeyBundles: Map<string, string> = new Map();
134+
132135
private eventDecryptor: EventDecryptor;
133136
private keyClaimManager: KeyClaimManager;
134137
private outgoingRequestProcessor: OutgoingRequestProcessor;
@@ -329,10 +332,9 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, CryptoEventH
329332
/**
330333
* Implementation of {@link CryptoBackend.maybeAcceptKeyBundle}.
331334
*/
332-
public async maybeAcceptKeyBundle(roomId: string, inviter: string): Promise<void> {
335+
public async maybeAcceptKeyBundle(roomId: string, inviter: string): Promise<boolean> {
333336
// TODO: retry this if it gets interrupted or it fails. (https://github.com/matrix-org/matrix-rust-sdk/issues/5112)
334337
// TODO: do this in the background.
335-
// TODO: handle the bundle message arriving after the invite (https://github.com/element-hq/element-web/issues/30740)
336338

337339
const logger = new LogSpan(this.logger, `maybeAcceptKeyBundle(${roomId}, ${inviter})`);
338340

@@ -352,7 +354,7 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, CryptoEventH
352354
);
353355
if (!bundleData) {
354356
logger.info("No key bundle found for user");
355-
return;
357+
return false;
356358
}
357359

358360
logger.info(`Fetching key bundle ${bundleData.url}`);
@@ -391,7 +393,17 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, CryptoEventH
391393
logger.warn(`Error receiving encrypted bundle:`, err);
392394
throw err;
393395
}
396+
397+
return true;
398+
}
399+
400+
/**
401+
* Implementation of {@link CryptoBackend.markRoomAsPendingKeyBundle}.
402+
*/
403+
public markRoomAsPendingKeyBundle(roomId: string, inviter: string): void {
404+
this.roomsPendingKeyBundles.set(roomId, inviter);
394405
}
406+
395407
///////////////////////////////////////////////////////////////////////////////////////////////////////////////////
396408
//
397409
// CryptoApi implementation
@@ -1703,6 +1715,37 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, CryptoEventH
17031715
senderVerified: encryptionInfo.isSenderVerified(),
17041716
},
17051717
});
1718+
1719+
// If we have received a room key bundle message, and have previously marked the room
1720+
// IDs it references as pending key bundles, tell the Rust SDK to try and accept it,
1721+
// just in case it was received after invite.
1722+
//
1723+
// We don't actually need to validate the contents of the bundle message, or do
1724+
// anything with its contents at all. We simply want to inform the Rust SDK we have
1725+
// received a new room key bundle that we might be able to download.
1726+
if (
1727+
isRoomKeyBundleMessage(parsedMessage) &&
1728+
this.roomsPendingKeyBundles.has(parsedMessage.content.room_id)
1729+
) {
1730+
// No `await`-ing here, as this is called from inside the `/sync` loop.
1731+
this.maybeAcceptKeyBundle(
1732+
parsedMessage.content.room_id,
1733+
this.roomsPendingKeyBundles.get(parsedMessage.content.room_id)!,
1734+
).then(
1735+
(success) => {
1736+
if (success) {
1737+
this.roomsPendingKeyBundles.delete(parsedMessage.content.room_id);
1738+
}
1739+
},
1740+
(err) => {
1741+
this.logger.error(
1742+
`Error attempting to download key bundle for room ${parsedMessage.content.room_id}`,
1743+
);
1744+
this.logger.error(err);
1745+
},
1746+
);
1747+
}
1748+
17061749
break;
17071750
}
17081751
case RustSdkCryptoJs.ProcessedToDeviceEventType.PlainText: {
@@ -2468,5 +2511,25 @@ function rustEncryptionInfoToJsEncryptionInfo(
24682511
return { shieldColour, shieldReason };
24692512
}
24702513

2514+
interface RoomKeyBundleMessage {
2515+
type: "io.element.msc4268.room_key_bundle";
2516+
content: {
2517+
room_id: string;
2518+
};
2519+
}
2520+
2521+
/**
2522+
* Determines if the given payload is a RoomKeyBundleMessage.
2523+
*
2524+
* A RoomKeyBundleMessage is identified by having a specific message type
2525+
* ("io.element.msc4268.room_key_bundle") and a valid room_id in its content.
2526+
*
2527+
* @param message - The received to-device message to check.
2528+
* @returns True if the payload matches the RoomKeyBundleMessage structure, false otherwise.
2529+
*/
2530+
function isRoomKeyBundleMessage(message: IToDeviceEvent): message is IToDeviceEvent & RoomKeyBundleMessage {
2531+
return message.type === "io.element.msc4268.room_key_bundle" && typeof message.content.room_id === "string";
2532+
}
2533+
24712534
type CryptoEvents = (typeof CryptoEvent)[keyof typeof CryptoEvent];
24722535
type RustCryptoEvents = Exclude<CryptoEvents, CryptoEvent.LegacyCryptoStoreMigrationProgress>;

0 commit comments

Comments
 (0)