Element-R: detect "withheld key" UTD errors, and mark them as such (#4302)

Partial fix to element-hq/element-web#27653

Author: richvdh

spec/integ/crypto/crypto.spec.ts

@@ -2333,8 +2333,82 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
     });
 
     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.
+        describe.each([
+            ["m.blacklisted", "The sender has blocked you.", DecryptionFailureCode.MEGOLM_KEY_WITHHELD],
+            [
+                "m.unverified",
+                "The sender has disabled encrypting to unverified devices.",
+                DecryptionFailureCode.MEGOLM_KEY_WITHHELD_FOR_UNVERIFIED_DEVICE,
+            ],
+        ])(
+            "Decryption fails with withheld error if a withheld notice with code '%s' is received",
+            (withheldCode, expectedMessage, expectedErrorCode) => {
+                // TODO: test arrival after the event too.
+                it.each(["before"])("%s the event", async (when) => {
+                    expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} });
+                    await startClientAndAwaitFirstSync();
+
+                    // A promise which resolves, with the MatrixEvent which wraps the event, once the decryption fails.
+                    const awaitDecryption = emitPromise(aliceClient, MatrixEventEvent.Decrypted);
+
+                    // Send Alice an encrypted room event which looks like it was encrypted with a megolm session
+                    async function sendEncryptedEvent() {
+                        const event = {
+                            ...testData.ENCRYPTED_EVENT,
+                            origin_server_ts: Date.now(),
+                        };
+                        const syncResponse = {
+                            next_batch: 1,
+                            rooms: { join: { [ROOM_ID]: { timeline: { events: [event] } } } },
+                        };
+
+                        syncResponder.sendOrQueueSyncResponse(syncResponse);
+                        await syncPromise(aliceClient);
+                    }
+
+                    // Send Alice a withheld notice
+                    async function sendWithheldMessage() {
+                        const withheldMessage = {
+                            type: "m.room_key.withheld",
+                            sender: "@bob:example.com",
+                            content: {
+                                algorithm: "m.megolm.v1.aes-sha2",
+                                room_id: ROOM_ID,
+                                sender_key: testData.ENCRYPTED_EVENT.content!.sender_key,
+                                session_id: testData.ENCRYPTED_EVENT.content!.session_id,
+                                code: withheldCode,
+                                reason: "zzz",
+                            },
+                        };
+
+                        syncResponder.sendOrQueueSyncResponse({
+                            next_batch: 1,
+                            to_device: { events: [withheldMessage] },
+                        });
+                        await syncPromise(aliceClient);
+                    }
+
+                    if (when === "before") {
+                        await sendWithheldMessage();
+                        await sendEncryptedEvent();
+                    } else {
+                        await sendEncryptedEvent();
+                        await sendWithheldMessage();
+                    }
+
+                    const ev = await awaitDecryption;
+                    expect(ev.getContent()).toEqual({
+                        body: `** Unable to decrypt: DecryptionError: ${expectedMessage} **`,
+                        msgtype: "m.bad.encrypted",
+                    });
+
+                    expect(ev.decryptionFailureReason).toEqual(expectedErrorCode);
+
+                    // `isEncryptedDisabledForUnverifiedDevices` should be true for `m.unverified` and false for other errors.
+                    expect(ev.isEncryptedDisabledForUnverifiedDevices).toEqual(withheldCode === "m.unverified");
+                });
+            },
+        );
 
         oldBackendOnly("does not block decryption on an 'm.unavailable' report", async function () {
             // there may be a key downloads for alice

spec/unit/models/event.spec.ts

@@ -412,7 +412,12 @@ describe("MatrixEvent", () => {
             const crypto = {
                 decryptEvent: jest
                     .fn()
-                    .mockRejectedValue("DecryptionError: The sender has disabled encrypting to unverified devices."),
+                    .mockRejectedValue(
+                        new DecryptionError(
+                            DecryptionFailureCode.MEGOLM_KEY_WITHHELD_FOR_UNVERIFIED_DEVICE,
+                            "The sender has disabled encrypting to unverified devices.",
+                        ),
+                    ),
             } as unknown as Crypto;
 
             await encryptedEvent.attemptDecryption(crypto);

src/crypto-api/index.ts

@@ -557,6 +557,12 @@ export enum DecryptionFailureCode {
     /** Message was encrypted with a Megolm session whose keys have not been shared with us. */
     MEGOLM_UNKNOWN_INBOUND_SESSION_ID = "MEGOLM_UNKNOWN_INBOUND_SESSION_ID",
 
+    /** A special case of {@link MEGOLM_UNKNOWN_INBOUND_SESSION_ID}: the sender has told us it is withholding the key. */
+    MEGOLM_KEY_WITHHELD = "MEGOLM_KEY_WITHHELD",
+
+    /** A special case of {@link MEGOLM_KEY_WITHHELD}: the sender has told us it is withholding the key, because the current device is unverified. */
+    MEGOLM_KEY_WITHHELD_FOR_UNVERIFIED_DEVICE = "MEGOLM_KEY_WITHHELD_FOR_UNVERIFIED_DEVICE",
+
     /** Message was encrypted with a Megolm session which has been shared with us, but in a later ratchet state. */
     OLM_UNKNOWN_MESSAGE_INDEX = "OLM_UNKNOWN_MESSAGE_INDEX",
 

src/crypto/OlmDevice.ts

@@ -1221,13 +1221,13 @@ export class OlmDevice {
                 this.getInboundGroupSession(roomId, senderKey, sessionId, txn, (session, sessionData, withheld) => {
                     if (session === null || sessionData === null) {
                         if (withheld) {
-                            error = new DecryptionError(
-                                DecryptionFailureCode.MEGOLM_UNKNOWN_INBOUND_SESSION_ID,
-                                calculateWithheldMessage(withheld),
-                                {
-                                    session: senderKey + "|" + sessionId,
-                                },
-                            );
+                            const failureCode =
+                                withheld.code === "m.unverified"
+                                    ? DecryptionFailureCode.MEGOLM_KEY_WITHHELD_FOR_UNVERIFIED_DEVICE
+                                    : DecryptionFailureCode.MEGOLM_KEY_WITHHELD;
+                            error = new DecryptionError(failureCode, calculateWithheldMessage(withheld), {
+                                session: senderKey + "|" + sessionId,
+                            });
                         }
                         result = null;
                         return;
@@ -1237,13 +1237,13 @@ export class OlmDevice {
                         res = session.decrypt(body);
                     } catch (e) {
                         if ((<Error>e)?.message === "OLM.UNKNOWN_MESSAGE_INDEX" && withheld) {
-                            error = new DecryptionError(
-                                DecryptionFailureCode.MEGOLM_UNKNOWN_INBOUND_SESSION_ID,
-                                calculateWithheldMessage(withheld),
-                                {
-                                    session: senderKey + "|" + sessionId,
-                                },
-                            );
+                            const failureCode =
+                                withheld.code === "m.unverified"
+                                    ? DecryptionFailureCode.MEGOLM_KEY_WITHHELD_FOR_UNVERIFIED_DEVICE
+                                    : DecryptionFailureCode.MEGOLM_KEY_WITHHELD;
+                            error = new DecryptionError(failureCode, calculateWithheldMessage(withheld), {
+                                session: senderKey + "|" + sessionId,
+                            });
                         } else {
                             error = <Error>e;
                         }

src/models/event.ts

@@ -43,7 +43,6 @@ import { MatrixError } from "../http-api";
 import { TypedEventEmitter } from "./typed-event-emitter";
 import { EventStatus } from "./event-status";
 import { CryptoBackend, DecryptionError } from "../common-crypto/CryptoBackend";
-import { WITHHELD_MESSAGES } from "../crypto/OlmDevice";
 import { IAnnotatedPushRule } from "../@types/PushRules";
 import { Room } from "./room";
 import { EventTimeline } from "./event-timeline";
@@ -312,12 +311,6 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
     private thread?: Thread;
     private threadId?: string;
 
-    /*
-     * True if this event is an encrypted event which we failed to decrypt, the receiver's device is unverified and
-     * the sender has disabled encrypting to unverified devices.
-     */
-    private encryptedDisabledForUnverifiedDevices = false;
-
     /* Set an approximate timestamp for the event relative the local clock.
      * This will inherently be approximate because it doesn't take into account
      * the time between the server putting the 'age' field on the event as it sent
@@ -787,12 +780,14 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
         return this._decryptionFailureReason;
     }
 
-    /*
+    /**
      * True if this event is an encrypted event which we failed to decrypt, the receiver's device is unverified and
      * the sender has disabled encrypting to unverified devices.
+     *
+     * @deprecated: Prefer `event.decryptionFailureReason === DecryptionFailureCode.MEGOLM_KEY_WITHHELD_FOR_UNVERIFIED_DEVICE`.
      */
     public get isEncryptedDisabledForUnverifiedDevices(): boolean {
-        return this.isDecryptionFailure() && this.encryptedDisabledForUnverifiedDevices;
+        return this.decryptionFailureReason === DecryptionFailureCode.MEGOLM_KEY_WITHHELD_FOR_UNVERIFIED_DEVICE;
     }
 
     public shouldAttemptDecryption(): boolean {
@@ -982,7 +977,6 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
         this.claimedEd25519Key = decryptionResult.claimedEd25519Key ?? null;
         this.forwardingCurve25519KeyChain = decryptionResult.forwardingCurve25519KeyChain || [];
         this.untrusted = decryptionResult.untrusted || false;
-        this.encryptedDisabledForUnverifiedDevices = false;
         this.invalidateExtensibleEvent();
     }
 
@@ -1003,7 +997,6 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
         this.claimedEd25519Key = null;
         this.forwardingCurve25519KeyChain = [];
         this.untrusted = false;
-        this.encryptedDisabledForUnverifiedDevices = reason === `DecryptionError: ${WITHHELD_MESSAGES["m.unverified"]}`;
         this.invalidateExtensibleEvent();
     }
 

src/rust-crypto/rust-crypto.ts

@@ -1787,6 +1787,17 @@ class EventDecryptor {
             }
         }
 
+        // If we got a withheld code, expose that.
+        if (err.maybe_withheld) {
+            // Unfortunately the Rust SDK API doesn't let us distinguish between different withheld cases, other than
+            // by string-matching.
+            const failureCode =
+                err.maybe_withheld === "The sender has disabled encrypting to unverified devices."
+                    ? DecryptionFailureCode.MEGOLM_KEY_WITHHELD_FOR_UNVERIFIED_DEVICE
+                    : DecryptionFailureCode.MEGOLM_KEY_WITHHELD;
+            throw new DecryptionError(failureCode, err.maybe_withheld, errorDetails);
+        }
+
         switch (err.code) {
             case RustSdkCryptoJs.DecryptionErrorCode.MissingRoomKey:
                 throw new DecryptionError(