Merge pull request #3066 from matrix-org/dbkr/olm_savesession_undecryptable_todevice_debug

Add some debugging & a debug event for decryption

spec/unit/crypto.spec.ts

@@ -254,6 +254,7 @@ describe("Crypto", function () {
                 sendToDevice: jest.fn(),
                 getKeyBackupVersion: jest.fn(),
                 isGuest: jest.fn(),
+                emit: jest.fn(),
             } as unknown as MatrixClient;
             mockRoomList = {} as unknown as RoomList;
 

src/client.ts

@@ -855,6 +855,7 @@ export enum ClientEvent {
     SyncUnexpectedError = "sync.unexpectedError",
     ClientWellKnown = "WellKnown.client",
     ReceivedVoipEvent = "received_voip_event",
+    UndecryptableToDeviceEvent = "toDeviceEvent.undecryptable",
     TurnServers = "turnServers",
     TurnServersError = "turnServers.error",
 }
@@ -1063,6 +1064,18 @@ export type ClientEventHandlerMap = {
      * ```
      */
     [ClientEvent.ToDeviceEvent]: (event: MatrixEvent) => void;
+    /**
+     * Fires if a to-device event is received that cannot be decrypted.
+     * Encrypted to-device events will (generally) use plain Olm encryption,
+     * in which case decryption failures are fatal: the event will never be
+     * decryptable, unlike Megolm encrypted events where the key may simply
+     * arrive later.
+     *
+     * An undecryptable to-device event is therefore likley to indicate problems.
+     *
+     * @param event - The undecyptable to-device event
+     */
+    [ClientEvent.UndecryptableToDeviceEvent]: (event: MatrixEvent) => void;
     /**
      * Fires whenever new user-scoped account_data is added.
      * @param event - The event describing the account_data just added

src/crypto/OlmDevice.ts

@@ -368,6 +368,11 @@ export class OlmDevice {
      */
     private saveSession(deviceKey: string, sessionInfo: IUnpickledSessionInfo, txn: unknown): void {
         const sessionId = sessionInfo.session.session_id();
+        logger.debug(`Saving Olm session ${sessionId} with device ${deviceKey}: ${sessionInfo.session.describe()}`);
+
+        // Why do we re-use the input object for this, overwriting the same key with a different
+        // type? Is it because we want to erase the unpickled session to enforce that it's no longer
+        // used? A comment would be great.
         const pickledSessionInfo = Object.assign(sessionInfo, {
             session: sessionInfo.session.pickle(this.pickleKey),
         });

src/crypto/index.ts

@@ -3433,6 +3433,8 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
         const algorithm = content.algorithm;
         const deviceKey = content.sender_key;
 
+        this.baseApis.emit(ClientEvent.UndecryptableToDeviceEvent, event);
+
         // retry decryption for all events sent by the sender_key.  This will
         // update the events to show a message indicating that the olm session was
         // wedged.