Skip to content

Commit 545eb16

Browse files
authored
Use a more structured algorithm for determining decryption client for appservice events (#41)
This PR changes the way we pick a client for decrypting an event, and caches the result so that it can be reused. This is intended to ensure that when the sender user isn't used for decryption, Appservice doesn't make repeated calls to get_joined_rooms, and when it picks a client it prefers one with encryption already enabled to save time. We also update to ES2022 so I can use Error.cause, one of my favourite new ES features. ## Checklist * [ ] Tests written for all new code * [ ] Linter has been satisfied * [ ] Sign-off given on the changes (see CONTRIBUTING.md)
2 parents a806668 + 9344147 commit 545eb16

File tree

4 files changed

+87
-36
lines changed

4 files changed

+87
-36
lines changed

src/appservice/Appservice.ts

Lines changed: 84 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,11 @@ export class Appservice extends EventEmitter {
238238
private eventProcessors: { [eventType: string]: IPreprocessor[] } = {};
239239
private pendingTransactions = new Map<string, Promise<void>>();
240240

241+
/**
242+
* A cache of intents for the purposes of decrypting rooms
243+
*/
244+
private cryptoClientForRoomId: LRU.LRUCache<string, MatrixClient>;
245+
241246
/**
242247
* Creates a new application service.
243248
* @param {IAppserviceOptions} options The options for the application service.
@@ -256,6 +261,11 @@ export class Appservice extends EventEmitter {
256261
ttl: options.intentOptions.maxAgeMs,
257262
});
258263

264+
this.cryptoClientForRoomId = new LRU.LRUCache({
265+
max: options.intentOptions.maxCached,
266+
ttl: options.intentOptions.maxAgeMs,
267+
});
268+
259269
this.registration = options.registration;
260270

261271
// If protocol is not defined, define an empty array.
@@ -658,6 +668,75 @@ export class Appservice extends EventEmitter {
658668
return providedToken === this.registration.hs_token;
659669
}
660670

671+
private async decryptAppserviceEvent(roomId: string, encrypted: EncryptedRoomEvent): ReturnType<Appservice["processEvent"]> {
672+
const existingClient = this.cryptoClientForRoomId.get(roomId);
673+
const decryptFn = async (client: MatrixClient) => {
674+
// Also fetches state in order to decrypt room. We should throw if the client is confused.
675+
if (!await client.crypto.isRoomEncrypted(roomId)) {
676+
throw new Error("Client detected that the room is not encrypted.");
677+
}
678+
let event = (await client.crypto.decryptRoomEvent(encrypted, roomId)).raw;
679+
event = await this.processEvent(event);
680+
this.cryptoClientForRoomId.set(roomId, client);
681+
// For logging purposes: show that the event was decrypted
682+
LogService.info("Appservice", `Processing decrypted event of type ${event["type"]}`);
683+
return event;
684+
};
685+
// 1. Try cached client
686+
if (existingClient) {
687+
try {
688+
return await decryptFn(existingClient);
689+
} catch (error) {
690+
LogService.debug("Appservice", `Failed to decrypt via cached client ${await existingClient.getUserId()}`, error);
691+
LogService.warn("Appservice", `Cached client was not able to decrypt ${roomId} ${encrypted.eventId} - trying other intents`);
692+
}
693+
}
694+
this.cryptoClientForRoomId.delete(roomId);
695+
// 2. Try the bot client
696+
if (this.botClient.crypto?.isReady) {
697+
try {
698+
return await decryptFn(this.botClient);
699+
} catch (error) {
700+
LogService.debug("Appservice", `Failed to decrypt via bot client`, error);
701+
LogService.warn("Appservice", `Bot client was not able to decrypt ${roomId} ${encrypted.eventId} - trying other intents`);
702+
}
703+
}
704+
705+
const userIdsInRoom = (await this.botClient.getJoinedRoomMembers(roomId)).filter(u => this.isNamespacedUser(u));
706+
// 3. Try existing clients with crypto enabled.
707+
for (const intentCacheEntry of this.intentsCache.entries()) {
708+
const [userId, intent] = intentCacheEntry as [string, Intent];
709+
if (!userIdsInRoom.includes(userId)) {
710+
// Not in this room.
711+
continue;
712+
}
713+
// Is this client crypto enabled?
714+
if (!intent.underlyingClient.crypto?.isReady) {
715+
continue;
716+
}
717+
try {
718+
return await decryptFn(intent.underlyingClient);
719+
} catch (error) {
720+
LogService.debug("Appservice", `Failed to decrypt via ${userId}`, error);
721+
LogService.warn("Appservice", `Existing encrypted client was not able to decrypt ${roomId} ${encrypted.eventId} - trying other intents`);
722+
}
723+
}
724+
725+
// 4. Try to enable crypto on any client to decrypt it.
726+
// We deliberately do not enable crypto on every client for performance reasons.
727+
const userInRoom = this.intentsCache.find((intent, userId) => !intent.underlyingClient.crypto?.isReady && userIdsInRoom.includes(userId));
728+
if (!userInRoom) {
729+
throw Error('No users in room, cannot decrypt');
730+
}
731+
try {
732+
await userInRoom.enableEncryption();
733+
return await decryptFn(userInRoom.underlyingClient);
734+
} catch (error) {
735+
LogService.debug("Appservice", `Failed to decrypt via random user ${userInRoom.userId}`, error);
736+
throw new Error("Unable to decrypt event", { cause: error });
737+
}
738+
}
739+
661740
private async handleTransaction(txnId: string, body: Record<string, unknown>) {
662741
// Process all the crypto stuff first to ensure that future transactions (if not this one)
663742
// will decrypt successfully. We start with EDUs because we need structures to put counts
@@ -804,39 +883,11 @@ export class Appservice extends EventEmitter {
804883
try {
805884
const encrypted = new EncryptedRoomEvent(event);
806885
const roomId = event['room_id'];
807-
try {
808-
event = (await this.botClient.crypto.decryptRoomEvent(encrypted, roomId)).raw;
809-
event = await this.processEvent(event);
810-
this.emit("room.decrypted_event", roomId, event);
811-
812-
// For logging purposes: show that the event was decrypted
813-
LogService.info("Appservice", `Processing decrypted event of type ${event["type"]}`);
814-
} catch (e1) {
815-
LogService.warn("Appservice", `Bot client was not able to decrypt ${roomId} ${event['event_id']} - trying other intents`);
816-
817-
let tryUserId: string;
818-
try {
819-
// TODO: This could be more efficient
820-
const userIdsInRoom = await this.botClient.getJoinedRoomMembers(roomId);
821-
tryUserId = userIdsInRoom.find(u => this.isNamespacedUser(u));
822-
} catch (e) {
823-
LogService.error("Appservice", "Failed to get members of room - cannot decrypt message");
824-
}
825-
826-
if (tryUserId) {
827-
const intent = this.getIntentForUserId(tryUserId);
828-
829-
event = (await intent.underlyingClient.crypto.decryptRoomEvent(encrypted, roomId)).raw;
830-
event = await this.processEvent(event);
831-
this.emit("room.decrypted_event", roomId, event);
832-
833-
// For logging purposes: show that the event was decrypted
834-
LogService.info("Appservice", `Processing decrypted event of type ${event["type"]}`);
835-
} else {
836-
// noinspection ExceptionCaughtLocallyJS
837-
throw e1;
838-
}
839-
}
886+
event = await this.decryptAppserviceEvent(roomId, encrypted);
887+
this.emit("room.decrypted_event", roomId, event);
888+
889+
// For logging purposes: show that the event was decrypted
890+
LogService.info("Appservice", `Processing decrypted event of type ${event["type"]}`);
840891
} catch (e) {
841892
LogService.error("Appservice", `Decryption error on ${event['room_id']} ${event['event_id']}`, e);
842893
this.emit("room.failed_decryption", event['room_id'], event, e);

tsconfig-examples.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"emitDecoratorMetadata": true,
55
"module": "commonjs",
66
"moduleResolution": "node",
7-
"target": "es2015",
7+
"target": "es2022",
88
"noImplicitAny": false,
99
"sourceMap": false,
1010
"outDir": "./lib",

tsconfig-release.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"emitDecoratorMetadata": true,
55
"module": "commonjs",
66
"moduleResolution": "node",
7-
"target": "es2020",
7+
"target": "es2022",
88
"noImplicitAny": false,
99
"sourceMap": true,
1010
"outDir": "./lib",

tsconfig.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"emitDecoratorMetadata": true,
55
"module": "commonjs",
66
"moduleResolution": "node",
7-
"target": "es2020",
7+
"target": "ES2022",
88
"noImplicitAny": false,
99
"sourceMap": true,
1010
"outDir": "./lib",

0 commit comments

Comments
 (0)