Add tests for recent work + fix related bugs

src/MatrixClient.ts

@@ -111,7 +111,7 @@ export class MatrixClient extends EventEmitter {
             this.crypto = new CryptoClient(this);
             LogService.debug("MatrixClientLite", "End-to-end encryption client created");
         } else {
-            LogService.trace("MatrixClientLite", "Not setting up encryption");
+            // LogService.trace("MatrixClientLite", "Not setting up encryption");
         }
 
         if (!this.storage) this.storage = new MemoryStorageProvider();

src/e2ee/CryptoClient.ts

@@ -203,16 +203,24 @@ export class CryptoClient {
         try {
             const sig = account.sign(anotherJson.stringify(obj));
             return {
-                ...existingSignatures,
                 [await this.client.getUserId()]: {
                     [`${DeviceKeyAlgorithm.Ed25119}:${this.deviceId}`]: sig,
                 },
+                ...existingSignatures,
             };
         } finally {
             account.free();
         }
     }
 
+    /**
+     * Verifies a signature on an object.
+     * @param {object} obj The signed object.
+     * @param {string} key The key which has supposedly signed the object.
+     * @param {string} signature The advertised signature.
+     * @returns {Promise<boolean>} Resolves to true if a valid signature, false otherwise.
+     */
+    @requiresReady()
     public async verifySignature(obj: object, key: string, signature: string): Promise<boolean> {
         obj = JSON.parse(JSON.stringify(obj));
 
@@ -237,10 +245,12 @@ export class CryptoClient {
      * Flags multiple user's device lists as outdated, optionally queuing an immediate update.
      * @param {string} userIds The user IDs to flag the device lists of.
      * @param {boolean} resync True (default) to queue an immediate update, false otherwise.
+     * @returns {Promise<void>} Resolves when the device lists have been flagged. Will also wait
+     * for the resync if one was requested.
      */
-    public flagUsersDeviceListsOutdated(userIds: string[], resync = true) {
-        // noinspection JSIgnoredPromiseFromCall
-        this.deviceTracker.flagUsersOutdated(userIds, resync);
+    @requiresReady()
+    public flagUsersDeviceListsOutdated(userIds: string[], resync = true): Promise<void> {
+        return this.deviceTracker.flagUsersOutdated(userIds, resync);
     }
 
     /**
@@ -251,6 +261,7 @@ export class CryptoClient {
      * ID to session. Users/devices which cannot have sessions made will not be included, thus the object
      * may be empty.
      */
+    @requiresReady()
     public async getOrCreateOlmSessions(userDeviceMap: Record<string, string[]>): Promise<Record<string, Record<string, IOlmSession>>> {
         const otkClaimRequest: Record<string, Record<string, OTKAlgorithm>> = {};
         const userDeviceSessionIds: Record<string, Record<string, IOlmSession>> = {};
@@ -275,71 +286,80 @@ export class CryptoClient {
             }
         }
 
-        const claimed = await this.client.claimOneTimeKeys(otkClaimRequest);
-        for (const userId of Object.keys(claimed.one_time_keys)) {
-            const storedDevices = await this.client.cryptoStore.getUserDevices(userId);
-            for (const deviceId of Object.keys(claimed.one_time_keys[userId])) {
-                try {
-                    const device = storedDevices.find(d => d.user_id === userId && d.device_id === deviceId);
-                    if (!device) {
-                        LogService.warn("CryptoClient", `Failed to handle claimed OTK: unable to locate stored device for user: ${userId} ${deviceId}`);
-                        continue;
-                    }
-
-                    const deviceKeyLabel = `${DeviceKeyAlgorithm.Ed25119}:${deviceId}`;
-
-                    const keyId = Object.keys(claimed.one_time_keys[userId][deviceId])[0];
-                    const signedKey = claimed.one_time_keys[userId][deviceId][keyId];
-                    const signature = signedKey?.signatures?.[userId]?.[deviceKeyLabel];
-                    if (!signature) {
-                        LogService.warn("CryptoClient", `Failed to find appropriate signature for claimed OTK ${userId} ${deviceId}`);
-                        continue;
-                    }
-
-                    const verified = await this.verifySignature(signedKey, device.keys[deviceKeyLabel], signature);
-                    if (!verified) {
-                        LogService.warn("CryptoClient", `Invalid signature for claimed OTK ${userId} ${deviceId}`);
-                        continue;
-                    }
-
-                    // TODO: Handle spec rate limiting
-                    // Clients should rate-limit the number of sessions it creates per device that it receives a message
-                    // from. Clients should not create a new session with another device if it has already created one
-                    // for that given device in the past 1 hour.
-
-                    // Finally, we can create a session. We do this on each loop just in case something goes wrong given
-                    // we don't have app-level transaction support here. We want to persist as many outbound sessions as
-                    // we can before exploding.
-                    const account = await this.getOlmAccount();
-                    const session = new Olm.Session();
+        if (Object.keys(otkClaimRequest).length > 0) {
+            const claimed = await this.client.claimOneTimeKeys(otkClaimRequest);
+            for (const userId of Object.keys(claimed.one_time_keys)) {
+                if (!otkClaimRequest[userId]) {
+                    LogService.warn("CryptoClient", `Server injected unexpected user: ${userId} - not claiming keys`);
+                    continue;
+                }
+                const storedDevices = await this.client.cryptoStore.getUserDevices(userId);
+                for (const deviceId of Object.keys(claimed.one_time_keys[userId])) {
                     try {
-                        const curveDeviceKey = device.keys[`${DeviceKeyAlgorithm.Curve25519}:${deviceId}`];
-                        session.create_outbound(account, curveDeviceKey, signedKey.key);
-                        const storedSession: IOlmSession = {
-                            sessionId: session.session_id(),
-                            lastDecryptionTs: Date.now(),
-                            pickled: session.pickle(this.pickleKey),
-                        };
-                        await this.client.cryptoStore.storeOlmSession(userId, deviceId, storedSession);
-
-                        if (!userDeviceSessionIds[userId]) userDeviceSessionIds[userId] = {};
-                        userDeviceSessionIds[userId][deviceId] = storedSession;
-
-                        // Send a dummy event so the device can prepare the session.
-                        // await this.encryptAndSendOlmMessage(device, storedSession, "m.dummy", {});
-                    } finally {
-                        session.free();
-                        await this.storeAndFreeOlmAccount(account);
+                        if (!otkClaimRequest[userId][deviceId]) {
+                            LogService.warn("CryptoClient", `Server provided an unexpected device in claim response (skipping): ${userId} ${deviceId}`);
+                            continue;
+                        }
+
+                        const device = storedDevices.find(d => d.user_id === userId && d.device_id === deviceId);
+                        if (!device) {
+                            LogService.warn("CryptoClient", `Failed to handle claimed OTK: unable to locate stored device for user: ${userId} ${deviceId}`);
+                            continue;
+                        }
+
+                        const deviceKeyLabel = `${DeviceKeyAlgorithm.Ed25119}:${deviceId}`;
+
+                        const keyId = Object.keys(claimed.one_time_keys[userId][deviceId])[0];
+                        const signedKey = claimed.one_time_keys[userId][deviceId][keyId];
+                        const signature = signedKey?.signatures?.[userId]?.[deviceKeyLabel];
+                        if (!signature) {
+                            LogService.warn("CryptoClient", `Failed to find appropriate signature for claimed OTK ${userId} ${deviceId}`);
+                            continue;
+                        }
+
+                        const verified = await this.verifySignature(signedKey, device.keys[deviceKeyLabel], signature);
+                        if (!verified) {
+                            LogService.warn("CryptoClient", `Invalid signature for claimed OTK ${userId} ${deviceId}`);
+                            continue;
+                        }
+
+                        // TODO: Handle spec rate limiting
+                        // Clients should rate-limit the number of sessions it creates per device that it receives a message
+                        // from. Clients should not create a new session with another device if it has already created one
+                        // for that given device in the past 1 hour.
+
+                        // Finally, we can create a session. We do this on each loop just in case something goes wrong given
+                        // we don't have app-level transaction support here. We want to persist as many outbound sessions as
+                        // we can before exploding.
+                        const account = await this.getOlmAccount();
+                        const session = new Olm.Session();
+                        try {
+                            const curveDeviceKey = device.keys[`${DeviceKeyAlgorithm.Curve25519}:${deviceId}`];
+                            session.create_outbound(account, curveDeviceKey, signedKey.key);
+                            const storedSession: IOlmSession = {
+                                sessionId: session.session_id(),
+                                lastDecryptionTs: Date.now(),
+                                pickled: session.pickle(this.pickleKey),
+                            };
+                            await this.client.cryptoStore.storeOlmSession(userId, deviceId, storedSession);
+
+                            if (!userDeviceSessionIds[userId]) userDeviceSessionIds[userId] = {};
+                            userDeviceSessionIds[userId][deviceId] = storedSession;
+                        } finally {
+                            session.free();
+                            await this.storeAndFreeOlmAccount(account);
+                        }
+                    } catch (e) {
+                        LogService.warn("CryptoClient", `Unable to verify signature of claimed OTK ${userId} ${deviceId}:`, e);
                     }
-                } catch (e) {
-                    LogService.warn("CryptoClient", `Unable to verify signature of claimed OTK ${userId} ${deviceId}:`, e);
                 }
             }
         }
 
         return userDeviceSessionIds;
     }
 
+    @requiresReady()
     private async encryptAndSendOlmMessage(device: UserDevice, session: IOlmSession, type: string, content: any): Promise<void> {
         const olmSession = new Olm.Session();
         try {
@@ -390,6 +410,7 @@ export class CryptoClient {
      * @param {any} content The event content being encrypted.
      * @returns {Promise<any>} Resolves to the encrypted content for an `m.room.encrypted` event.
      */
+    @requiresReady()
     public async encryptRoomEvent(roomId: string, eventType: string, content: any): Promise<any> {
         if (!(await this.isRoomEncrypted(roomId))) {
             throw new Error("Room is not encrypted");
@@ -475,7 +496,5 @@ export class CryptoClient {
         } finally {
             session.free();
         }
-
-
     }
 }

src/e2ee/DeviceTracker.ts

@@ -39,13 +39,13 @@ export class DeviceTracker {
      * Flags multiple user's device lists as outdated, optionally queuing an immediate update.
      * @param {string} userIds The user IDs to flag the device lists of.
      * @param {boolean} resync True (default) to queue an immediate update, false otherwise.
+     * @returns {Promise<void>} Resolves when the flagging has completed. Will wait for the resync
+     * if requested too.
      */
-    public async flagUsersOutdated(userIds: string[], resync = true) {
+    public async flagUsersOutdated(userIds: string[], resync = true): Promise<void> {
         await this.client.cryptoStore.flagUsersOutdated(userIds);
         if (resync) {
-            // We don't really want to wait around for this, so let it work in the background
-            // noinspection ES6MissingAwait
-            this.updateUsersDeviceLists(userIds);
+            await this.updateUsersDeviceLists(userIds);
         }
     }
 
@@ -63,52 +63,62 @@ export class DeviceTracker {
             await Promise.all(existingPromises);
         }
 
-        const promise = new Promise<void>(async resolve => {
-            const resp = await this.client.getUserDevices(userIds);
-            for (const userId of Object.keys(resp.device_keys)) {
-                const validated: UserDevice[] = [];
-                for (const deviceId of Object.keys(resp.device_keys[userId])) {
-                    const device = resp.device_keys[userId][deviceId];
-                    if (device.user_id !== userId || device.device_id !== deviceId) {
-                        LogService.warn("DeviceTracker", `Server appears to be lying about device lists: ${userId} ${deviceId} has unexpected device ${device.user_id} ${device.device_id} listed - ignoring device`);
+        const promise = new Promise<void>(async (resolve, reject) => {
+            try {
+                const resp = await this.client.getUserDevices(userIds);
+                for (const userId of Object.keys(resp.device_keys)) {
+                    if (!userIds.includes(userId)) {
+                        LogService.warn("DeviceTracker", `Server returned unexpected user ID: ${userId} - ignoring user`);
                         continue;
                     }
 
-                    const ed25519 = device.keys[`${DeviceKeyAlgorithm.Ed25119}:${deviceId}`];
-                    const curve25519 = device.keys[`${DeviceKeyAlgorithm.Curve25519}:${deviceId}`];
+                    const validated: UserDevice[] = [];
+                    for (const deviceId of Object.keys(resp.device_keys[userId])) {
+                        const device = resp.device_keys[userId][deviceId];
+                        if (device.user_id !== userId || device.device_id !== deviceId) {
+                            LogService.warn("DeviceTracker", `Server appears to be lying about device lists: ${userId} ${deviceId} has unexpected device ${device.user_id} ${device.device_id} listed - ignoring device`);
+                            continue;
+                        }
 
-                    if (!ed25519 || !curve25519) {
-                        LogService.warn("DeviceTracker", `Device ${userId} ${deviceId} is missing either an Ed25519 or Curve25519 key - ignoring device`);
-                        continue;
-                    }
+                        const ed25519 = device.keys[`${DeviceKeyAlgorithm.Ed25119}:${deviceId}`];
+                        const curve25519 = device.keys[`${DeviceKeyAlgorithm.Curve25519}:${deviceId}`];
+
+                        if (!ed25519 || !curve25519) {
+                            LogService.warn("DeviceTracker", `Device ${userId} ${deviceId} is missing either an Ed25519 or Curve25519 key - ignoring device`);
+                            continue;
+                        }
+
+                        const currentDevices = await this.client.cryptoStore.getUserDevices(userId);
+                        const existingDevice = currentDevices.find(d => d.device_id === deviceId);
 
-                    const currentDevices = await this.client.cryptoStore.getUserDevices(userId);
-                    const existingDevice = currentDevices.find(d => d.device_id === deviceId);
+                        if (existingDevice) {
+                            const existingEd25519 = existingDevice.keys[`${DeviceKeyAlgorithm.Ed25119}:${deviceId}`];
+                            if (existingEd25519 !== ed25519) {
+                                LogService.warn("DeviceTracker", `Device ${userId} ${deviceId} appears compromised: Ed25519 key changed - ignoring device`);
+                                continue;
+                            }
+                        }
 
-                    if (existingDevice) {
-                        const existingEd25519 = existingDevice.keys[`${DeviceKeyAlgorithm.Ed25119}:${deviceId}`];
-                        if (existingEd25519 !== ed25519) {
-                            LogService.warn("DeviceTracker", `Device ${userId} ${deviceId} appears compromised: Ed25519 key changed - ignoring device`);
+                        const signature = device.signatures?.[userId]?.[`${DeviceKeyAlgorithm.Ed25119}:${deviceId}`];
+                        if (!signature) {
+                            LogService.warn("DeviceTracker", `Device ${userId} ${deviceId} is missing a signature - ignoring device`);
                             continue;
                         }
-                    }
 
-                    const signature = device.signatures?.[userId]?.[`${DeviceKeyAlgorithm.Ed25119}:${deviceId}`];
-                    if (!signature) {
-                        LogService.warn("DeviceTracker", `Device ${userId} ${deviceId} is missing a signature - ignoring device`);
-                        continue;
-                    }
+                        const validSignature = await this.client.crypto.verifySignature(device, ed25519, signature);
+                        if (!validSignature) {
+                            LogService.warn("DeviceTracker", `Device ${userId} ${deviceId} has an invalid signature - ignoring device`);
+                            continue;
+                        }
 
-                    const validSignature = await this.client.crypto.verifySignature(device, ed25519, signature);
-                    if (!validSignature) {
-                        LogService.warn("DeviceTracker", `Device ${userId} ${deviceId} has an invalid signature - ignoring device`);
-                        continue;
+                        validated.push(device);
                     }
 
-                    validated.push(device);
+                    await this.client.cryptoStore.setUserDevices(userId, validated);
                 }
-
-                await this.client.cryptoStore.setUserDevices(userId, validated);
+            } catch (e) {
+                LogService.error("DeviceTracker", "Error updating device lists:", e);
+                // return reject(e);
             }
             resolve();
         });

src/storage/SqliteCryptoStorageProvider.ts

@@ -172,7 +172,7 @@ export class SqliteCryptoStorageProvider implements ICryptoStorageProvider {
                 usesLeft: session.usesLeft,
                 expiresTs: session.expiresTs,
             });
-        });
+        })();
     }
 
     public async getOutboundGroupSession(sessionId: string, roomId: string): Promise<IOutboundGroupSession> {
@@ -183,8 +183,8 @@ export class SqliteCryptoStorageProvider implements ICryptoStorageProvider {
                 roomId: result.room_id,
                 pickled: result.pickled,
                 isCurrent: result.current === 1,
-                usesLeft: result.usesLeft,
-                expiresTs: result.expiresTs,
+                usesLeft: result.uses_left,
+                expiresTs: result.expires_ts,
             };
         }
         return null;
@@ -198,8 +198,8 @@ export class SqliteCryptoStorageProvider implements ICryptoStorageProvider {
                 roomId: result.room_id,
                 pickled: result.pickled,
                 isCurrent: result.current === 1,
-                usesLeft: result.usesLeft,
-                expiresTs: result.expiresTs,
+                usesLeft: result.uses_left,
+                expiresTs: result.expires_ts,
             };
         }
         return null;