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

Jump to bottom

Use a more structured algorithm for determining decryption client for appservice events #41

Merged
merged 8 commits into from
Dec 4, 2023
Merged

Use a more structured algorithm for determining decryption client for appservice events #41

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e84fee9
Rewrite logic to determine correct client to decrypt event/.
Half-Shot Nov 28, 2023
42fc7e8
Use ES2022 for Error causes
Half-Shot Nov 28, 2023
ed4ba92
Fix release version
Half-Shot Nov 28, 2023
15643f8
And examples too
Half-Shot Nov 28, 2023
81e6824
Fix obvious bs
Half-Shot Nov 29, 2023
5a0dd1e
Ensure we check if the room is encrypted first.
Half-Shot Dec 4, 2023
c852f25
Tidy up
Half-Shot Dec 4, 2023
9344147
Lint
Half-Shot Dec 4, 2023
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
112 changes: 79 additions & 33 deletions src/appservice/Appservice.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,11 @@ export class Appservice extends EventEmitter {
private eventProcessors: { [eventType: string]: IPreprocessor[] } = {};
private pendingTransactions = new Map<string, Promise<void>>();

/**
* A cache of intents for the purposes of decrypting rooms
*/
private cryptoClientForRoomId: LRU.LRUCache<string, MatrixClient>;

/**
* Creates a new application service.
* @param {IAppserviceOptions} options The options for the application service.
Expand All @@ -256,6 +261,11 @@ export class Appservice extends EventEmitter {
ttl: options.intentOptions.maxAgeMs,
});

this.cryptoClientForRoomId = new LRU.LRUCache({
max: options.intentOptions.maxCached,
ttl: options.intentOptions.maxAgeMs,
});

this.registration = options.registration;

// If protocol is not defined, define an empty array.
Expand Down Expand Up @@ -658,6 +668,70 @@ export class Appservice extends EventEmitter {
return providedToken === this.registration.hs_token;
}

private async decryptAppserivceEvent(roomId: string, encrypted: EncryptedRoomEvent): ReturnType<Appservice["processEvent"]> {
Half-Shot marked this conversation as resolved.
Show resolved Hide resolved
const existingClient = this.cryptoClientForRoomId.get(roomId);
const decryptFn = async (client: MatrixClient) => {
let event = (await client.crypto.decryptRoomEvent(encrypted, roomId)).raw;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the rust-sdk's OlmMachine doesn't discard keys for rooms that a user has left, then this should first check if client is currently in the target room. Otherwise, this would decrypt an event that client shouldn't be able to access.

On the other hand, it might not matter in practice whether client shouldn't see the event, because as long as any of the appservice's users can see it, there's no difference in terms of privacy/security which of its clients the appservices uses to decrypt an event.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we materially care on the AS side. If we still have keys and can turn an encrypted body into unencrypted text then yay. It doesn't really matter which client did it.

event = await this.processEvent(event);
this.cryptoClientForRoomId.set(roomId, client);
// For logging purposes: show that the event was decrypted
LogService.info("Appservice", `Processing decrypted event of type ${event["type"]}`);
return event;
};
// 1. Try cached client
if (existingClient) {
try {
return await decryptFn(existingClient);
} catch (error) {
LogService.debug("Appservice", `Failed to decrypt via cached client ${await existingClient.getUserId()}`, error);
LogService.warn("Appservice", `Cached client was not able to decrypt ${roomId} ${encrypted.eventId} - trying other intents`);
}
}
this.cryptoClientForRoomId.delete(roomId);
// 2. Try the bot client
if (this.botClient.crypto?.isReady) {
try {
return await decryptFn(this.botClient);
} catch (error) {
LogService.debug("Appservice", `Failed to decrypt via bot client`, error);
LogService.warn("Appservice", `Bot client was not able to decrypt ${roomId} ${encrypted.eventId} - trying other intents`);
}
}

const userIdsInRoom = (await this.botClient.getJoinedRoomMembers(roomId)).filter(u => this.isNamespacedUser(u));
// 3. Try existing clients with crypto enabled.
for (const intentCacheEntry of this.intentsCache.entries()) {
const [userId, intent] = intentCacheEntry as [string, Intent];
if (!userIdsInRoom.includes(userId)) {
// Not in this room.
continue;
}
// Is this client crypto enabled?
if (!intent.underlyingClient.crypto?.isReady) {
continue;
}
try {
return await decryptFn(intent.underlyingClient);
} catch (error) {
LogService.debug("Appservice", `Failed to decrypt via ${userId}`, error);
LogService.warn("Appservice", `Existing encrypted client was not able to decrypt ${roomId} ${encrypted.eventId} - trying other intents`);
}
}

// 4. Try to enable crypto on any client to decrypt it.
const userInRoom = this.intentsCache.find((_intent, userId) => userIdsInRoom.includes(userId));
if (!userInRoom) {
throw Error('No users in room, cannot decrypt');
}
await userInRoom.enableEncryption();
try {
return await decryptFn(userInRoom.underlyingClient);
Half-Shot marked this conversation as resolved.
Show resolved Hide resolved
} catch (error) {
LogService.debug("Appservice", `Failed to decrypt via random user ${userInRoom.userId}`, error);
throw new Error("Unable to decrypt event", { cause: error });
}
}

private async handleTransaction(txnId: string, body: Record<string, unknown>) {
// Process all the crypto stuff first to ensure that future transactions (if not this one)
// will decrypt successfully. We start with EDUs because we need structures to put counts
Expand Down Expand Up @@ -804,39 +878,11 @@ export class Appservice extends EventEmitter {
try {
const encrypted = new EncryptedRoomEvent(event);
const roomId = event['room_id'];
try {
event = (await this.botClient.crypto.decryptRoomEvent(encrypted, roomId)).raw;
event = await this.processEvent(event);
this.emit("room.decrypted_event", roomId, event);

// For logging purposes: show that the event was decrypted
LogService.info("Appservice", `Processing decrypted event of type ${event["type"]}`);
} catch (e1) {
LogService.warn("Appservice", `Bot client was not able to decrypt ${roomId} ${event['event_id']} - trying other intents`);

let tryUserId: string;
try {
// TODO: This could be more efficient
const userIdsInRoom = await this.botClient.getJoinedRoomMembers(roomId);
tryUserId = userIdsInRoom.find(u => this.isNamespacedUser(u));
} catch (e) {
LogService.error("Appservice", "Failed to get members of room - cannot decrypt message");
}

if (tryUserId) {
const intent = this.getIntentForUserId(tryUserId);

event = (await intent.underlyingClient.crypto.decryptRoomEvent(encrypted, roomId)).raw;
event = await this.processEvent(event);
this.emit("room.decrypted_event", roomId, event);

// For logging purposes: show that the event was decrypted
LogService.info("Appservice", `Processing decrypted event of type ${event["type"]}`);
} else {
// noinspection ExceptionCaughtLocallyJS
throw e1;
}
}
event = await this.decryptAppserivceEvent(roomId, encrypted);
this.emit("room.decrypted_event", roomId, event);

// For logging purposes: show that the event was decrypted
LogService.info("Appservice", `Processing decrypted event of type ${event["type"]}`);
} catch (e) {
LogService.error("Appservice", `Decryption error on ${event['room_id']} ${event['event_id']}`, e);
this.emit("room.failed_decryption", event['room_id'], event, e);
Expand Down
2 changes: 1 addition & 1 deletion tsconfig-examples.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"emitDecoratorMetadata": true,
"module": "commonjs",
"moduleResolution": "node",
"target": "es2015",
"target": "es2022",
"noImplicitAny": false,
"sourceMap": false,
"outDir": "./lib",
Expand Down
2 changes: 1 addition & 1 deletion tsconfig-release.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"emitDecoratorMetadata": true,
"module": "commonjs",
"moduleResolution": "node",
"target": "es2020",
"target": "es2022",
"noImplicitAny": false,
"sourceMap": true,
"outDir": "./lib",
Expand Down
2 changes: 1 addition & 1 deletion tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"emitDecoratorMetadata": true,
"module": "commonjs",
"moduleResolution": "node",
"target": "es2020",
"target": "ES2022",
"noImplicitAny": false,
"sourceMap": true,
"outDir": "./lib",
Expand Down
Loading