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

When we receive an `m.unavailable` notification, do not show it as "Decryption
key withheld".

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");
+        });
+    });
 });

src/crypto/algorithms/megolm.ts

@@ -1613,67 +1613,10 @@ export class MegolmDecryption extends DecryptionAlgorithm {
         const senderKey = content.sender_key;
 
         if (content.code === "m.no_olm") {
-            const sender = event.getSender()!;
+            await this.onNoOlmWithheldEvent(event);
-            this.prefixedLogger.warn(`${sender}:${senderKey} was unable to establish an olm session with us`);
+        } else if (content.code === "m.unavailable") {
-            // if the sender says that they haven't been able to establish an olm
+            // this simply means that the other device didn't have the key, which isn't very useful information. Don't
-            // session, let's proactively establish one
+            // record it in the storage
-
-            // Note: after we record that the olm session has had a problem, 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.
-
-            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);
-            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;
-                }
-            }
-
-            // 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 this.olmDevice.recordSessionProblem(senderKey, "no_olm", true);
-            this.retryDecryptionFromSender(senderKey);
-
-            await this.baseApis.sendToDevice("m.room.encrypted", {
-                [sender]: {
-                    [device.deviceId]: encryptedContent,
-                },
-            });
         } else {
             await this.olmDevice.addInboundGroupSessionWithheld(
                 content.room_id,
@@ -1683,6 +1626,78 @@ export class MegolmDecryption extends DecryptionAlgorithm {
                 content.reason,
             );
         }
+
+        // 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);
+        }
+    }
+
+    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) {
+                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?
+
+        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);
+
+        await this.baseApis.sendToDevice("m.room.encrypted", {
+            [sender]: {
+                [device.deviceId]: encryptedContent,
+            },
+        });
     }
 
     public hasKeysForKeyRequest(keyRequest: IncomingRoomKeyRequest): Promise<boolean> {