Clean up code for handling decryption failures (#4126)

Various improvements, including:

* Defining an enum for decryption failure reasons
* Exposing the reason code as a property on Event

spec/integ/crypto/crypto.spec.ts

@@ -80,12 +80,12 @@ import { SecretStorageKeyDescription } from "../../../src/secret-storage";
 import {
     CrossSigningKey,
     CryptoCallbacks,
+    DecryptionFailureCode,
     EventShieldColour,
     EventShieldReason,
     KeyBackupInfo,
 } from "../../../src/crypto-api";
 import { E2EKeyResponder } from "../../test-utils/E2EKeyResponder";
-import { DecryptionError } from "../../../src/crypto/algorithms";
 import { IKeyBackup } from "../../../src/crypto/backup";
 import {
     createOlmAccount,
@@ -470,9 +470,8 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
             await startClientAndAwaitFirstSync();
 
             const awaitUISI = new Promise<void>((resolve) => {
-                aliceClient.on(MatrixEventEvent.Decrypted, (ev, err) => {
-                    const error = err as DecryptionError;
-                    if (error.code == "MEGOLM_UNKNOWN_INBOUND_SESSION_ID") {
+                aliceClient.on(MatrixEventEvent.Decrypted, (ev) => {
+                    if (ev.decryptionFailureReason === DecryptionFailureCode.MEGOLM_UNKNOWN_INBOUND_SESSION_ID) {
                         resolve();
                     }
                 });
@@ -499,9 +498,8 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
             await startClientAndAwaitFirstSync();
 
             const awaitUnknownIndex = new Promise<void>((resolve) => {
-                aliceClient.on(MatrixEventEvent.Decrypted, (ev, err) => {
-                    const error = err as DecryptionError;
-                    if (error.code == "OLM_UNKNOWN_MESSAGE_INDEX") {
+                aliceClient.on(MatrixEventEvent.Decrypted, (ev) => {
+                    if (ev.decryptionFailureReason === DecryptionFailureCode.OLM_UNKNOWN_MESSAGE_INDEX) {
                         resolve();
                     }
                 });
@@ -532,13 +530,12 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
             await aliceClient.getCrypto()!.importRoomKeys([testData.MEGOLM_SESSION_DATA]);
 
             const awaitDecryptionError = new Promise<void>((resolve) => {
-                aliceClient.on(MatrixEventEvent.Decrypted, (ev, err) => {
-                    const error = err as DecryptionError;
+                aliceClient.on(MatrixEventEvent.Decrypted, (ev) => {
                     // rust and libolm can't have an exact 1:1 mapping for all errors,
                     // but some errors are part of API and should match
                     if (
-                        error.code != "MEGOLM_UNKNOWN_INBOUND_SESSION_ID" &&
-                        error.code != "OLM_UNKNOWN_MESSAGE_INDEX"
+                        ev.decryptionFailureReason !== DecryptionFailureCode.MEGOLM_UNKNOWN_INBOUND_SESSION_ID &&
+                        ev.decryptionFailureReason !== DecryptionFailureCode.OLM_UNKNOWN_MESSAGE_INDEX
                     ) {
                         resolve();
                     }

spec/unit/models/event.spec.ts

@@ -27,6 +27,8 @@ import {
     THREAD_RELATION_TYPE,
     TweakName,
 } from "../../../src";
+import { DecryptionFailureCode } from "../../../src/crypto-api";
+import { DecryptionError } from "../../../src/common-crypto/CryptoBackend";
 
 describe("MatrixEvent", () => {
     it("should create copies of itself", () => {
@@ -360,20 +362,50 @@ describe("MatrixEvent", () => {
             });
         });
 
-        it("should report decryption errors", async () => {
+        it("should report unknown decryption errors", async () => {
+            const decryptionListener = jest.fn();
+            encryptedEvent.addListener(MatrixEventEvent.Decrypted, decryptionListener);
+
+            const testError = new Error("test error");
             const crypto = {
-                decryptEvent: jest.fn().mockRejectedValue(new Error("test error")),
+                decryptEvent: jest.fn().mockRejectedValue(testError),
             } as unknown as Crypto;
 
             await encryptedEvent.attemptDecryption(crypto);
             expect(encryptedEvent.isEncrypted()).toBeTruthy();
             expect(encryptedEvent.isBeingDecrypted()).toBeFalsy();
             expect(encryptedEvent.isDecryptionFailure()).toBeTruthy();
+            expect(encryptedEvent.decryptionFailureReason).toEqual(DecryptionFailureCode.UNKNOWN_ERROR);
             expect(encryptedEvent.isEncryptedDisabledForUnverifiedDevices).toBeFalsy();
             expect(encryptedEvent.getContent()).toEqual({
                 msgtype: "m.bad.encrypted",
                 body: "** Unable to decrypt: Error: test error **",
             });
+            expect(decryptionListener).toHaveBeenCalledWith(encryptedEvent, testError);
+        });
+
+        it("should report known decryption errors", async () => {
+            const decryptionListener = jest.fn();
+            encryptedEvent.addListener(MatrixEventEvent.Decrypted, decryptionListener);
+
+            const testError = new DecryptionError(DecryptionFailureCode.MEGOLM_UNKNOWN_INBOUND_SESSION_ID, "uisi");
+            const crypto = {
+                decryptEvent: jest.fn().mockRejectedValue(testError),
+            } as unknown as Crypto;
+
+            await encryptedEvent.attemptDecryption(crypto);
+            expect(encryptedEvent.isEncrypted()).toBeTruthy();
+            expect(encryptedEvent.isBeingDecrypted()).toBeFalsy();
+            expect(encryptedEvent.isDecryptionFailure()).toBeTruthy();
+            expect(encryptedEvent.decryptionFailureReason).toEqual(
+                DecryptionFailureCode.MEGOLM_UNKNOWN_INBOUND_SESSION_ID,
+            );
+            expect(encryptedEvent.isEncryptedDisabledForUnverifiedDevices).toBeFalsy();
+            expect(encryptedEvent.getContent()).toEqual({
+                msgtype: "m.bad.encrypted",
+                body: "** Unable to decrypt: DecryptionError: uisi **",
+            });
+            expect(decryptionListener).toHaveBeenCalledWith(encryptedEvent, testError);
         });
 
         it(`should report "DecryptionError: The sender has disabled encrypting to unverified devices."`, async () => {
@@ -423,6 +455,8 @@ describe("MatrixEvent", () => {
             expect(eventAttemptDecryptionSpy).toHaveBeenCalledTimes(2);
             expect(crypto.decryptEvent).toHaveBeenCalledTimes(2);
             expect(encryptedEvent.getType()).toEqual("m.room.message");
+            expect(encryptedEvent.isDecryptionFailure()).toBe(false);
+            expect(encryptedEvent.decryptionFailureReason).toBe(null);
         });
     });
 

spec/unit/room-state.spec.ts

@@ -25,10 +25,11 @@ import { EventType, RelationType, UNSTABLE_MSC2716_MARKER } from "../../src/@typ
 import { MatrixEvent, MatrixEventEvent } from "../../src/models/event";
 import { M_BEACON } from "../../src/@types/beacon";
 import { MatrixClient } from "../../src/client";
-import { DecryptionError } from "../../src/crypto/algorithms";
 import { defer } from "../../src/utils";
 import { Room } from "../../src/models/room";
 import { KnownMembership } from "../../src/@types/membership";
+import { DecryptionFailureCode } from "../../src/crypto-api";
+import { DecryptionError } from "../../src/common-crypto/CryptoBackend";
 
 describe("RoomState", function () {
     const roomId = "!foo:bar";
@@ -1040,7 +1041,9 @@ describe("RoomState", function () {
                     content: beacon1RelationContent,
                 });
                 jest.spyOn(failedDecryptionRelatedEvent, "isDecryptionFailure").mockReturnValue(true);
-                mockClient.decryptEventIfNeeded.mockRejectedValue(new DecryptionError("ERR", "msg"));
+                mockClient.decryptEventIfNeeded.mockRejectedValue(
+                    new DecryptionError(DecryptionFailureCode.UNKNOWN_ERROR, "msg"),
+                );
                 // spy on event.once
                 const eventOnceSpy = jest.spyOn(failedDecryptionRelatedEvent, "once");
 

spec/unit/testing.spec.ts

@@ -16,6 +16,7 @@ limitations under the License.
 
 import { mkDecryptionFailureMatrixEvent, mkEncryptedMatrixEvent, mkMatrixEvent } from "../../src/testing";
 import { EventType } from "../../src";
+import { DecryptionFailureCode } from "../../src/crypto-api";
 
 describe("testing", () => {
     describe("mkMatrixEvent", () => {
@@ -60,6 +61,7 @@ describe("testing", () => {
             expect(event.sender?.userId).toEqual("@alice:test");
             expect(event.isEncrypted()).toBe(true);
             expect(event.isDecryptionFailure()).toBe(false);
+            expect(event.decryptionFailureReason).toBe(null);
             expect(event.getContent()).toEqual({ body: "blah" });
             expect(event.getType()).toEqual("m.room.message");
         });
@@ -70,13 +72,14 @@ describe("testing", () => {
             const event = await mkDecryptionFailureMatrixEvent({
                 sender: "@alice:test",
                 roomId: "!test:room",
-                code: "UNKNOWN",
+                code: DecryptionFailureCode.UNKNOWN_ERROR,
                 msg: "blah",
             });
 
             expect(event.sender?.userId).toEqual("@alice:test");
             expect(event.isEncrypted()).toBe(true);
             expect(event.isDecryptionFailure()).toBe(true);
+            expect(event.decryptionFailureReason).toEqual(DecryptionFailureCode.UNKNOWN_ERROR);
             expect(event.getContent()).toEqual({
                 body: "** Unable to decrypt: DecryptionError: blah **",
                 msgtype: "m.bad.encrypted",

src/common-crypto/CryptoBackend.ts

@@ -17,7 +17,7 @@ limitations under the License.
 import type { IDeviceLists, IToDeviceEvent } from "../sync-accumulator";
 import { IClearEvent, MatrixEvent } from "../models/event";
 import { Room } from "../models/room";
-import { CryptoApi, ImportRoomKeysOpts } from "../crypto-api";
+import { CryptoApi, DecryptionFailureCode, ImportRoomKeysOpts } from "../crypto-api";
 import { CrossSigningInfo, UserTrustLevel } from "../crypto/CrossSigning";
 import { IEncryptedEventInfo } from "../crypto/api";
 import { KeyBackupInfo, KeyBackupSession } from "../crypto-api/keybackup";
@@ -226,10 +226,6 @@ export interface EventDecryptionResult {
      * restored from backup)
      */
     untrusted?: boolean;
-    /**
-     * The sender doesn't authorize the unverified devices to decrypt his messages
-     */
-    encryptedDisabledForUnverifiedDevices?: boolean;
 }
 
 /**
@@ -263,3 +259,43 @@ export interface BackupDecryptor {
      */
     free(): void;
 }
+
+/**
+ * Exception thrown when decryption fails
+ *
+ * @param code - Reason code for the failure.
+ *
+ * @param msg - user-visible message describing the problem
+ *
+ * @param details - key/value pairs reported in the logs but not shown
+ *   to the user.
+ */
+export class DecryptionError extends Error {
+    public readonly detailedString: string;
+
+    public constructor(
+        public readonly code: DecryptionFailureCode,
+        msg: string,
+        details?: Record<string, string | Error>,
+    ) {
+        super(msg);
+        this.name = "DecryptionError";
+        this.detailedString = detailedStringForDecryptionError(this, details);
+    }
+}
+
+function detailedStringForDecryptionError(err: DecryptionError, details?: Record<string, string | Error>): string {
+    let result = err.name + "[msg: " + err.message;
+
+    if (details) {
+        result +=
+            ", " +
+            Object.keys(details)
+                .map((k) => k + ": " + details[k])
+                .join(", ");
+    }
+
+    result += "]";
+
+    return result;
+}

src/crypto-api.ts

@@ -498,6 +498,57 @@ export interface CryptoApi {
     deleteKeyBackupVersion(version: string): Promise<void>;
 }
 
+/** A reason code for a failure to decrypt an event. */
+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",
+
+    /** 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",
+
+    /** Unknown or unclassified error. */
+    UNKNOWN_ERROR = "UNKNOWN_ERROR",
+
+    /** @deprecated only used in legacy crypto */
+    MEGOLM_BAD_ROOM = "MEGOLM_BAD_ROOM",
+
+    /** @deprecated only used in legacy crypto */
+    MEGOLM_MISSING_FIELDS = "MEGOLM_MISSING_FIELDS",
+
+    /** @deprecated only used in legacy crypto */
+    OLM_DECRYPT_GROUP_MESSAGE_ERROR = "OLM_DECRYPT_GROUP_MESSAGE_ERROR",
+
+    /** @deprecated only used in legacy crypto */
+    OLM_BAD_ENCRYPTED_MESSAGE = "OLM_BAD_ENCRYPTED_MESSAGE",
+
+    /** @deprecated only used in legacy crypto */
+    OLM_BAD_RECIPIENT = "OLM_BAD_RECIPIENT",
+
+    /** @deprecated only used in legacy crypto */
+    OLM_BAD_RECIPIENT_KEY = "OLM_BAD_RECIPIENT_KEY",
+
+    /** @deprecated only used in legacy crypto */
+    OLM_BAD_ROOM = "OLM_BAD_ROOM",
+
+    /** @deprecated only used in legacy crypto */
+    OLM_BAD_SENDER_CHECK_FAILED = "OLM_BAD_SENDER_CHECK_FAILED",
+
+    /** @deprecated only used in legacy crypto */
+    OLM_BAD_SENDER = "OLM_BAD_SENDER",
+
+    /** @deprecated only used in legacy crypto */
+    OLM_FORWARDED_MESSAGE = "OLM_FORWARDED_MESSAGE",
+
+    /** @deprecated only used in legacy crypto */
+    OLM_MISSING_CIPHERTEXT = "OLM_MISSING_CIPHERTEXT",
+
+    /** @deprecated only used in legacy crypto */
+    OLM_NOT_INCLUDED_IN_RECIPIENTS = "OLM_NOT_INCLUDED_IN_RECIPIENTS",
+
+    /** @deprecated only used in legacy crypto */
+    UNKNOWN_ENCRYPTION_ALGORITHM = "UNKNOWN_ENCRYPTION_ALGORITHM",
+}
+
 /**
  * Options object for `CryptoApi.bootstrapCrossSigning`.
  */

src/crypto/OlmDevice.ts

@@ -18,11 +18,12 @@ import { Account, InboundGroupSession, OutboundGroupSession, Session, Utility }
 
 import { logger, Logger } from "../logger";
 import { IndexedDBCryptoStore } from "./store/indexeddb-crypto-store";
-import * as algorithms from "./algorithms";
 import { CryptoStore, IProblem, ISessionInfo, IWithheld } from "./store/base";
 import { IOlmDevice, IOutboundGroupSessionKey } from "./algorithms/megolm";
 import { IMegolmSessionData, OlmGroupSessionExtraData } from "../@types/crypto";
 import { IMessage } from "./algorithms/olm";
+import { DecryptionFailureCode } from "../crypto-api";
+import { DecryptionError } from "../common-crypto/CryptoBackend";
 
 // The maximum size of an event is 65K, and we base64 the content, so this is a
 // reasonable approximation to the biggest plaintext we can encrypt.
@@ -1220,8 +1221,8 @@ export class OlmDevice {
                 this.getInboundGroupSession(roomId, senderKey, sessionId, txn, (session, sessionData, withheld) => {
                     if (session === null || sessionData === null) {
                         if (withheld) {
-                            error = new algorithms.DecryptionError(
-                                "MEGOLM_UNKNOWN_INBOUND_SESSION_ID",
+                            error = new DecryptionError(
+                                DecryptionFailureCode.MEGOLM_UNKNOWN_INBOUND_SESSION_ID,
                                 calculateWithheldMessage(withheld),
                                 {
                                     session: senderKey + "|" + sessionId,
@@ -1236,8 +1237,8 @@ export class OlmDevice {
                         res = session.decrypt(body);
                     } catch (e) {
                         if ((<Error>e)?.message === "OLM.UNKNOWN_MESSAGE_INDEX" && withheld) {
-                            error = new algorithms.DecryptionError(
-                                "MEGOLM_UNKNOWN_INBOUND_SESSION_ID",
+                            error = new DecryptionError(
+                                DecryptionFailureCode.MEGOLM_UNKNOWN_INBOUND_SESSION_ID,
                                 calculateWithheldMessage(withheld),
                                 {
                                     session: senderKey + "|" + sessionId,

src/crypto/algorithms/base.ts

@@ -199,45 +199,6 @@ export abstract class DecryptionAlgorithm {
     public sendSharedHistoryInboundSessions?(devicesByUser: Map<string, DeviceInfo[]>): Promise<void>;
 }
 
-/**
- * Exception thrown when decryption fails
- *
- * @param msg - user-visible message describing the problem
- *
- * @param details - key/value pairs reported in the logs but not shown
- *   to the user.
- */
-export class DecryptionError extends Error {
-    public readonly detailedString: string;
-
-    public constructor(
-        public readonly code: string,
-        msg: string,
-        details?: Record<string, string | Error>,
-    ) {
-        super(msg);
-        this.code = code;
-        this.name = "DecryptionError";
-        this.detailedString = detailedStringForDecryptionError(this, details);
-    }
-}
-
-function detailedStringForDecryptionError(err: DecryptionError, details?: Record<string, string | Error>): string {
-    let result = err.name + "[msg: " + err.message;
-
-    if (details) {
-        result +=
-            ", " +
-            Object.keys(details)
-                .map((k) => k + ": " + details[k])
-                .join(", ");
-    }
-
-    result += "]";
-
-    return result;
-}
-
 export class UnknownDeviceError extends Error {
     /**
      * Exception thrown specifically when we want to warn the user to consider
@@ -274,3 +235,6 @@ export function registerAlgorithm<P extends IParams = IParams>(
     ENCRYPTION_CLASSES.set(algorithm, encryptor as new (params: IParams) => EncryptionAlgorithm);
     DECRYPTION_CLASSES.set(algorithm, decryptor as new (params: DecryptionClassParams) => DecryptionAlgorithm);
 }
+
+/* Re-export for backwards compatibility. Deprecated: this is an internal class. */
+export { DecryptionError } from "../../common-crypto/CryptoBackend";