From 53de9f529d1a22f1acdab3a64910aa37c22a6454 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Thu, 12 Jan 2023 20:08:01 +0000
Subject: [PATCH 1/2] Retry decryption after room_key_withheld events
... to update the error message
---
src/crypto/algorithms/megolm.ts | 132 +++++++++++++++++---------------
1 file changed, 72 insertions(+), 60 deletions(-)
diff --git a/src/crypto/algorithms/megolm.ts b/src/crypto/algorithms/megolm.ts
index 76ad7eab0be..c49fd5b2da2 100644
--- a/src/crypto/algorithms/megolm.ts
+++ b/src/crypto/algorithms/megolm.ts
@@ -1613,76 +1613,88 @@ export class MegolmDecryption extends DecryptionAlgorithm {
const senderKey = content.sender_key;
if (content.code === "m.no_olm") {
- const sender = event.getSender()!;
- this.prefixedLogger.warn(`${sender}:${senderKey} was unable to establish an olm session with us`);
- // if the sender says that they haven't been able to establish an olm
- // session, let's proactively establish one
+ await this.onNoOlmWithheldEvent(event);
+ } else {
+ await this.olmDevice.addInboundGroupSessionWithheld(
+ content.room_id,
+ senderKey,
+ content.session_id,
+ content.code,
+ content.reason,
+ );
+ }
- // Note: after we record that the olm session has had a problem, we
- // trigger retrying decryption for all the messages from the sender's
+ // Having recorded the problem, retry decryption on any affected messages.
+ // It's unlikely we'll be able to decrypt sucessfully now, but this will
+ // update the error message.
+ //
+ if (content.session_id) {
+ await this.retryDecryption(senderKey, content.session_id);
+ } else {
+ // no_olm messages aren't specific to a given megolm session, so
+ // we trigger retrying decryption for all the messages from the sender's
// key, so that we can update the error message to indicate the olm
// session problem.
+ await this.retryDecryptionFromSender(senderKey);
+ }
+ }
- if (await this.olmDevice.getSessionIdForDevice(senderKey)) {
- // a session has already been established, so we don't need to
- // create a new one.
- this.prefixedLogger.debug("New session already created. Not creating a new one.");
- await this.olmDevice.recordSessionProblem(senderKey, "no_olm", true);
- this.retryDecryptionFromSender(senderKey);
- return;
- }
- let device = this.crypto.deviceList.getDeviceByIdentityKey(content.algorithm, senderKey);
+ private async onNoOlmWithheldEvent(event: MatrixEvent): Promise<void> {
+ const content = event.getContent();
+ const senderKey = content.sender_key;
+ const sender = event.getSender()!;
+ this.prefixedLogger.warn(`${sender}:${senderKey} was unable to establish an olm session with us`);
+ // if the sender says that they haven't been able to establish an olm
+ // session, let's proactively establish one
+
+ if (await this.olmDevice.getSessionIdForDevice(senderKey)) {
+ // a session has already been established, so we don't need to
+ // create a new one.
+ this.prefixedLogger.debug("New session already created. Not creating a new one.");
+ await this.olmDevice.recordSessionProblem(senderKey, "no_olm", true);
+ return;
+ }
+ let device = this.crypto.deviceList.getDeviceByIdentityKey(content.algorithm, senderKey);
+ if (!device) {
+ // if we don't know about the device, fetch the user's devices again
+ // and retry before giving up
+ await this.crypto.downloadKeys([sender], false);
+ device = this.crypto.deviceList.getDeviceByIdentityKey(content.algorithm, senderKey);
if (!device) {
- // if we don't know about the device, fetch the user's devices again
- // and retry before giving up
- await this.crypto.downloadKeys([sender], false);
- device = this.crypto.deviceList.getDeviceByIdentityKey(content.algorithm, senderKey);
- if (!device) {
- this.prefixedLogger.info(
- "Couldn't find device for identity key " + senderKey + ": not establishing session",
- );
- await this.olmDevice.recordSessionProblem(senderKey, "no_olm", false);
- this.retryDecryptionFromSender(senderKey);
- return;
- }
+ this.prefixedLogger.info(
+ "Couldn't find device for identity key " + senderKey + ": not establishing session",
+ );
+ await this.olmDevice.recordSessionProblem(senderKey, "no_olm", false);
+ return;
}
+ }
- // XXX: switch this to use encryptAndSendToDevices() rather than duplicating it?
+ // XXX: switch this to use encryptAndSendToDevices() rather than duplicating it?
- await olmlib.ensureOlmSessionsForDevices(this.olmDevice, this.baseApis, { [sender]: [device] }, false);
- const encryptedContent: IEncryptedContent = {
- algorithm: olmlib.OLM_ALGORITHM,
- sender_key: this.olmDevice.deviceCurve25519Key!,
- ciphertext: {},
- [ToDeviceMessageId]: uuidv4(),
- };
- await olmlib.encryptMessageForDevice(
- encryptedContent.ciphertext,
- this.userId,
- undefined,
- this.olmDevice,
- sender,
- device,
- { type: "m.dummy" },
- );
+ await olmlib.ensureOlmSessionsForDevices(this.olmDevice, this.baseApis, { [sender]: [device] }, false);
+ const encryptedContent: IEncryptedContent = {
+ algorithm: olmlib.OLM_ALGORITHM,
+ sender_key: this.olmDevice.deviceCurve25519Key!,
+ ciphertext: {},
+ [ToDeviceMessageId]: uuidv4(),
+ };
+ await olmlib.encryptMessageForDevice(
+ encryptedContent.ciphertext,
+ this.userId,
+ undefined,
+ this.olmDevice,
+ sender,
+ device,
+ { type: "m.dummy" },
+ );
- await this.olmDevice.recordSessionProblem(senderKey, "no_olm", true);
- this.retryDecryptionFromSender(senderKey);
+ await this.olmDevice.recordSessionProblem(senderKey, "no_olm", true);
- await this.baseApis.sendToDevice("m.room.encrypted", {
- [sender]: {
- [device.deviceId]: encryptedContent,
- },
- });
- } else {
- await this.olmDevice.addInboundGroupSessionWithheld(
- content.room_id,
- senderKey,
- content.session_id,
- content.code,
- content.reason,
- );
- }
+ await this.baseApis.sendToDevice("m.room.encrypted", {
+ [sender]: {
+ [device.deviceId]: encryptedContent,
+ },
+ });
}
public hasKeysForKeyRequest(keyRequest: IncomingRoomKeyRequest): Promise<boolean> {
From 22a0997dcabba6daeca03142b8125a047c6793e8 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Thu, 12 Jan 2023 20:14:54 +0000
Subject: [PATCH 2/2] Fix spurious "Decryption key withheld" messages
When we receive an `m.unavailable` notification, do not show it as "Decryption
key withheld".
---
spec/integ/megolm-integ.spec.ts | 75 +++++++++++++++++++++++++++++++++
src/crypto/algorithms/megolm.ts | 3 ++
2 files changed, 78 insertions(+)
diff --git a/spec/integ/megolm-integ.spec.ts b/spec/integ/megolm-integ.spec.ts
index 1bdf3a09f47..88e438e7510 100644
--- a/spec/integ/megolm-integ.spec.ts
+++ b/spec/integ/megolm-integ.spec.ts
@@ -1678,4 +1678,79 @@ describe("megolm", () => {
await Promise.all([sendPromise, megolmMessagePromise, aliceTestClient.httpBackend.flush("/keys/query", 1)]);
});
});
+
+ describe("m.room_key.withheld handling", () => {
+ // TODO: there are a bunch more tests for this sort of thing in spec/unit/crypto/algorithms/megolm.spec.ts.
+ // They should be converted to integ tests and moved.
+
+ it("does not block decryption on an 'm.unavailable' report", async function () {
+ await aliceTestClient.start();
+
+ // there may be a key downloads for alice
+ aliceTestClient.httpBackend.when("POST", "/keys/query").respond(200, {});
+ aliceTestClient.httpBackend.flush("/keys/query", 1, 5000);
+
+ // encrypt a message with a group session.
+ const groupSession = new Olm.OutboundGroupSession();
+ groupSession.create();
+ const messageEncryptedEvent = encryptMegolmEvent({
+ senderKey: testSenderKey,
+ groupSession: groupSession,
+ room_id: ROOM_ID,
+ });
+
+ // Alice gets the room message, but not the key
+ aliceTestClient.httpBackend.when("GET", "/sync").respond(200, {
+ next_batch: 1,
+ rooms: {
+ join: { [ROOM_ID]: { timeline: { events: [messageEncryptedEvent] } } },
+ },
+ });
+ await aliceTestClient.flushSync();
+
+ // alice will (eventually) send a room-key request
+ aliceTestClient.httpBackend.when("PUT", "/sendToDevice/m.room_key_request/").respond(200, {});
+ await aliceTestClient.httpBackend.flush("/sendToDevice/m.room_key_request/", 1, 1000);
+
+ // at this point, the message should be a decryption failure
+ const room = aliceTestClient.client.getRoom(ROOM_ID)!;
+ const event = room.getLiveTimeline().getEvents()[0];
+ expect(event.isDecryptionFailure()).toBeTruthy();
+
+ // we want to wait for the message to be updated, so create a promise for it
+ const retryPromise = new Promise((resolve) => {
+ event.once(MatrixEventEvent.Decrypted, (ev) => {
+ resolve(ev);
+ });
+ });
+
+ // alice gets back a room-key-withheld notification
+ aliceTestClient.httpBackend.when("GET", "/sync").respond(200, {
+ next_batch: 2,
+ to_device: {
+ events: [
+ {
+ type: "m.room_key.withheld",
+ sender: "@bob:example.com",
+ content: {
+ algorithm: "m.megolm.v1.aes-sha2",
+ room_id: ROOM_ID,
+ session_id: groupSession.session_id(),
+ sender_key: testSenderKey,
+ code: "m.unavailable",
+ reason: "",
+ },
+ },
+ ],
+ },
+ });
+ await aliceTestClient.flushSync();
+
+ // the withheld notification should trigger a retry; wait for it
+ await retryPromise;
+
+ // finally: the message should still be a regular decryption failure, not a withheld notification.
+ expect(event.getContent().body).not.toContain("withheld");
+ });
+ });
});
diff --git a/src/crypto/algorithms/megolm.ts b/src/crypto/algorithms/megolm.ts
index c49fd5b2da2..163d3953d45 100644
--- a/src/crypto/algorithms/megolm.ts
+++ b/src/crypto/algorithms/megolm.ts
@@ -1614,6 +1614,9 @@ export class MegolmDecryption extends DecryptionAlgorithm {
if (content.code === "m.no_olm") {
await this.onNoOlmWithheldEvent(event);
+ } else if (content.code === "m.unavailable") {
+ // this simply means that the other device didn't have the key, which isn't very useful information. Don't
+ // record it in the storage
} else {
await this.olmDevice.addInboundGroupSessionWithheld(
content.room_id,