Skip to content

Commit b76e7ca

Browse files
committed
Reduce blocking while pre-fetching Megolm keys
Currently, calling `Client#prepareToEncrypt` in a megolm room has the potential to block for multiple seconds while it crunches numbers. Sleeping for 0 seconds (approximating `setImmediate`) allows the engine to process other events, updates, or re-renders in between checks. See - element-hq/element-web#21612 - element-hq/element-web#11836 Signed-off-by: Clark Fischer <[email protected]>
1 parent ddce1bc commit b76e7ca

File tree

2 files changed

+85
-8
lines changed

2 files changed

+85
-8
lines changed

spec/unit/crypto/algorithms/megolm.spec.ts

Lines changed: 79 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2022 The Matrix.org Foundation C.I.C.
2+
Copyright 2022 - 2023 The Matrix.org Foundation C.I.C.
33
44
Licensed under the Apache License, Version 2.0 (the "License");
55
you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@ limitations under the License.
1616

1717
import { mocked, MockedObject } from "jest-mock";
1818

19+
import type { DeviceInfoMap } from "../../../../src/crypto/DeviceList";
1920
import "../../../olm-loader";
2021
import type { OutboundGroupSession } from "@matrix-org/olm";
2122
import * as algorithms from "../../../../src/crypto/algorithms";
@@ -33,6 +34,7 @@ import { ClientEvent, MatrixClient, RoomMember } from "../../../../src";
3334
import { DeviceInfo, IDevice } from "../../../../src/crypto/deviceinfo";
3435
import { DeviceTrustLevel } from "../../../../src/crypto/CrossSigning";
3536
import { MegolmEncryption as MegolmEncryptionClass } from "../../../../src/crypto/algorithms/megolm";
37+
import { sleep } from "../../../../src/utils";
3638

3739
const MegolmDecryption = algorithms.DECRYPTION_CLASSES.get("m.megolm.v1.aes-sha2")!;
3840
const MegolmEncryption = algorithms.ENCRYPTION_CLASSES.get("m.megolm.v1.aes-sha2")!;
@@ -58,6 +60,12 @@ describe("MegolmDecryption", function () {
5860

5961
beforeEach(async function () {
6062
mockCrypto = testUtils.mock(Crypto, "Crypto") as MockedObject<Crypto>;
63+
64+
// @ts-ignore assigning to readonly prop
65+
mockCrypto.backupManager = {
66+
backupGroupSession: () => {},
67+
};
68+
6169
mockBaseApis = {
6270
claimOneTimeKeys: jest.fn(),
6371
sendToDevice: jest.fn(),
@@ -314,10 +322,6 @@ describe("MegolmDecryption", function () {
314322
let olmDevice: OlmDevice;
315323

316324
beforeEach(async () => {
317-
// @ts-ignore assigning to readonly prop
318-
mockCrypto.backupManager = {
319-
backupGroupSession: () => {},
320-
};
321325
const cryptoStore = new MemoryCryptoStore();
322326

323327
olmDevice = new OlmDevice(cryptoStore);
@@ -515,6 +519,76 @@ describe("MegolmDecryption", function () {
515519
});
516520
});
517521

522+
describe("prepareToEncrypt", () => {
523+
let megolm: MegolmEncryptionClass;
524+
let room: jest.Mocked<Room>;
525+
526+
const deviceMap: DeviceInfoMap = {
527+
"user-a": {
528+
"device-a": new DeviceInfo("device-a"),
529+
"device-b": new DeviceInfo("device-b"),
530+
"device-c": new DeviceInfo("device-c"),
531+
},
532+
"user-b": {
533+
"device-d": new DeviceInfo("device-d"),
534+
"device-e": new DeviceInfo("device-e"),
535+
"device-f": new DeviceInfo("device-f"),
536+
},
537+
"user-c": {
538+
"device-g": new DeviceInfo("device-g"),
539+
"device-h": new DeviceInfo("device-h"),
540+
"device-i": new DeviceInfo("device-i"),
541+
},
542+
};
543+
544+
beforeEach(() => {
545+
room = testUtils.mock(Room, "Room") as jest.Mocked<Room>;
546+
room.getEncryptionTargetMembers.mockImplementation(async () => [
547+
new RoomMember(room.roomId, "@user:example.org"),
548+
]);
549+
room.getBlacklistUnverifiedDevices.mockReturnValue(false);
550+
551+
mockCrypto.downloadKeys.mockImplementation(async () => deviceMap);
552+
553+
mockCrypto.checkDeviceTrust.mockImplementation(() => new DeviceTrustLevel(true, true, true, true));
554+
555+
const olmDevice = new OlmDevice(new MemoryCryptoStore());
556+
megolm = new MegolmEncryptionClass({
557+
userId: "@user:id",
558+
deviceId: "12345",
559+
crypto: mockCrypto,
560+
olmDevice,
561+
baseApis: mockBaseApis,
562+
roomId: room.roomId,
563+
config: {
564+
algorithm: "m.megolm.v1.aes-sha2",
565+
rotation_period_ms: 9_999_999,
566+
},
567+
});
568+
});
569+
570+
it("checks each device", async () => {
571+
megolm.prepareToEncrypt(room);
572+
//@ts-ignore private member access, gross
573+
await megolm.encryptionPreparation?.promise;
574+
575+
for (const userId in deviceMap) {
576+
for (const deviceId in deviceMap[userId]) {
577+
expect(mockCrypto.checkDeviceTrust).toHaveBeenCalledWith(userId, deviceId);
578+
}
579+
}
580+
});
581+
582+
it("defers before completing", async () => {
583+
megolm.prepareToEncrypt(room);
584+
// Ensure that `Crypto#checkDeviceTrust` has been called *fewer*
585+
// than the full nine times, after yielding once.
586+
await sleep(0);
587+
const callCount = mockCrypto.checkDeviceTrust.mock.calls.length;
588+
expect(callCount).toBeLessThan(9);
589+
});
590+
});
591+
518592
it("notifies devices that have been blocked", async function () {
519593
const aliceClient = new TestClient("@alice:example.com", "alicedevice").client;
520594
const bobClient1 = new TestClient("@bob:example.com", "bobdevice1").client;

src/crypto/algorithms/megolm.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2015 - 2021 The Matrix.org Foundation C.I.C.
2+
Copyright 2015 - 2021, 2023 The Matrix.org Foundation C.I.C.
33
44
Licensed under the Apache License, Version 2.0 (the "License");
55
you may not use this file except in compliance with the License.
@@ -43,6 +43,7 @@ import { IMegolmEncryptedContent, IncomingRoomKeyRequest, IEncryptedContent } fr
4343
import { RoomKeyRequestState } from "../OutgoingRoomKeyRequestManager";
4444
import { OlmGroupSessionExtraData } from "../../@types/crypto";
4545
import { MatrixError } from "../../http-api";
46+
import { immediate } from "../../utils";
4647

4748
// determine whether the key can be shared with invitees
4849
export function isRoomSharedHistory(room: Room): boolean {
@@ -73,7 +74,6 @@ export interface IOlmDevice<T = DeviceInfo> {
7374
deviceInfo: T;
7475
}
7576

76-
/* eslint-disable camelcase */
7777
export interface IOutboundGroupSessionKey {
7878
chain_index: number;
7979
key: string;
@@ -106,7 +106,6 @@ interface IPayload extends Partial<IMessage> {
106106
algorithm?: string;
107107
sender_key?: string;
108108
}
109-
/* eslint-enable camelcase */
110109

111110
interface SharedWithData {
112111
// The identity key of the device we shared with
@@ -1213,6 +1212,10 @@ export class MegolmEncryption extends EncryptionAlgorithm {
12131212
continue;
12141213
}
12151214

1215+
// Yield prior to checking each device so that we don't block
1216+
// updating/rendering for too long.
1217+
// See https://github.com/vector-im/element-web/issues/21612
1218+
await immediate();
12161219
const deviceTrust = this.crypto.checkDeviceTrust(userId, deviceId);
12171220

12181221
if (

0 commit comments

Comments
 (0)