Fix spurious "Decryption key withheld" messages (#3061)

richvdh · web-flow · commit eb058edb1b0e · 2023-01-13T13:17:48.000Z
When we receive an `m.unavailable` notification, do not show it as "Decryption
key withheld".
diff --git a/spec/integ/megolm-integ.spec.ts b/spec/integ/megolm-integ.spec.ts
@@ -1702,4 +1702,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
@@ -1613,76 +1613,91 @@ 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 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,
+                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> {
