Merge pull request #521 from matrix-org/rav/async_crypto/olmlib

Make bits of `olmlib` asynchronous

Author: richvdh

spec/unit/crypto/algorithms/megolm.spec.js

@@ -60,7 +60,8 @@ describe("MegolmDecryption", function() {
         // we stub out the olm encryption bits
         mockOlmLib = {};
         mockOlmLib.ensureOlmSessionsForDevices = expect.createSpy();
-        mockOlmLib.encryptMessageForDevice = expect.createSpy();
+        mockOlmLib.encryptMessageForDevice =
+            expect.createSpy().andReturn(Promise.resolve());
         megolmDecryption.olmlib = mockOlmLib;
     });
 

src/crypto/DeviceList.js

@@ -587,7 +587,7 @@ async function _storeDeviceKeys(_olmDevice, userStore, deviceResult) {
     const unsigned = deviceResult.unsigned || {};
 
     try {
-        olmlib.verifySignature(_olmDevice, deviceResult, userId, deviceId, signKey);
+        await olmlib.verifySignature(_olmDevice, deviceResult, userId, deviceId, signKey);
     } catch (e) {
         console.warn("Unable to verify signature on device " +
             userId + ":" + deviceId + ":" + e);

src/crypto/algorithms/megolm.js

@@ -288,7 +288,7 @@ MegolmEncryption.prototype._shareKeyWithDevices = function(session, devicesByUse
     return olmlib.ensureOlmSessionsForDevices(
         this._olmDevice, this._baseApis, devicesByUser,
     ).then(function(devicemap) {
-        let haveTargets = false;
+        const promises = [];
 
         for (const userId in devicesByUser) {
             if (!devicesByUser.hasOwnProperty(userId)) {
@@ -328,31 +328,35 @@ MegolmEncryption.prototype._shareKeyWithDevices = function(session, devicesByUse
                     ciphertext: {},
                 };
 
-                olmlib.encryptMessageForDevice(
-                    encryptedContent.ciphertext,
-                    self._userId,
-                    self._deviceId,
-                    self._olmDevice,
-                    userId,
-                    deviceInfo,
-                    payload,
-                );
-
                 if (!contentMap[userId]) {
                     contentMap[userId] = {};
                 }
 
                 contentMap[userId][deviceId] = encryptedContent;
-                haveTargets = true;
+
+                promises.push(
+                    olmlib.encryptMessageForDevice(
+                        encryptedContent.ciphertext,
+                        self._userId,
+                        self._deviceId,
+                        self._olmDevice,
+                        userId,
+                        deviceInfo,
+                        payload,
+                    ),
+                );
             }
         }
 
-        if (!haveTargets) {
+        if (promises.length === 0) {
+            // no devices to send to
             return Promise.resolve();
         }
 
-        // TODO: retries
-        return self._baseApis.sendToDevice("m.room.encrypted", contentMap);
+        return Promise.all(promises).then(() => {
+            // TODO: retries
+            return self._baseApis.sendToDevice("m.room.encrypted", contentMap);
+        });
     }).then(function() {
         console.log(`Completed megolm keyshare in ${self._roomId}`);
 
@@ -751,7 +755,7 @@ MegolmDecryption.prototype.shareKeysWithDevice = function(keyRequest) {
             //
             // ensureOlmSessionsForUsers has already done the logging,
             // so just skip it.
-            return;
+            return null;
         }
 
         console.log(
@@ -770,24 +774,24 @@ MegolmDecryption.prototype.shareKeysWithDevice = function(keyRequest) {
             ciphertext: {},
         };
 
-        this.olmlib.encryptMessageForDevice(
+        return this.olmlib.encryptMessageForDevice(
             encryptedContent.ciphertext,
             this._userId,
             this._deviceId,
             this._olmDevice,
             userId,
             deviceInfo,
             payload,
-        );
-
-        const contentMap = {
-            [userId]: {
-                [deviceId]: encryptedContent,
-            },
-        };
-
-        // TODO: retries
-        return this._baseApis.sendToDevice("m.room.encrypted", contentMap);
+        ).then(() => {
+            const contentMap = {
+                [userId]: {
+                    [deviceId]: encryptedContent,
+                },
+            };
+
+            // TODO: retries
+            return this._baseApis.sendToDevice("m.room.encrypted", contentMap);
+        });
     }).done();
 };
 

src/crypto/algorithms/olm.js

@@ -107,6 +107,8 @@ OlmEncryption.prototype.encryptMessage = function(room, eventType, content) {
             ciphertext: {},
         };
 
+        const promises = [];
+
         for (let i = 0; i < users.length; ++i) {
             const userId = users[i];
             const devices = self._crypto.getStoredDevicesForUser(userId);
@@ -123,15 +125,17 @@ OlmEncryption.prototype.encryptMessage = function(room, eventType, content) {
                     continue;
                 }
 
-                olmlib.encryptMessageForDevice(
-                    encryptedContent.ciphertext,
-                    self._userId, self._deviceId, self._olmDevice,
-                    userId, deviceInfo, payloadFields,
+                promises.push(
+                    olmlib.encryptMessageForDevice(
+                        encryptedContent.ciphertext,
+                        self._userId, self._deviceId, self._olmDevice,
+                        userId, deviceInfo, payloadFields,
+                    ),
                 );
             }
         }
 
-        return encryptedContent;
+        return Promise.all(promises).return(encryptedContent);
     });
 };
 

src/crypto/olmlib.js

@@ -48,8 +48,11 @@ module.exports.MEGOLM_ALGORITHM = "m.megolm.v1.aes-sha2";
  * @param {string} recipientUserId
  * @param {module:crypto/deviceinfo} recipientDevice
  * @param {object} payloadFields fields to include in the encrypted payload
+ *
+ * Returns a promise which resolves (to undefined) when the payload
+ *    has been encrypted into `resultsObject`
  */
-module.exports.encryptMessageForDevice = function(
+module.exports.encryptMessageForDevice = async function(
     resultsObject,
     ourUserId, ourDeviceId, olmDevice, recipientUserId, recipientDevice,
     payloadFields,
@@ -118,7 +121,7 @@ module.exports.encryptMessageForDevice = function(
  *    an Object mapping from userId to deviceId to
  *    {@link module:crypto~OlmSessionResult}
  */
-module.exports.ensureOlmSessionsForDevices = function(
+module.exports.ensureOlmSessionsForDevices = async function(
     olmDevice, baseApis, devicesByUser,
 ) {
     const devicesWithoutSession = [
@@ -148,7 +151,7 @@ module.exports.ensureOlmSessionsForDevices = function(
     }
 
     if (devicesWithoutSession.length === 0) {
-        return Promise.resolve(result);
+        return result;
     }
 
     // TODO: this has a race condition - if we try to send another message
@@ -158,55 +161,60 @@ module.exports.ensureOlmSessionsForDevices = function(
     // That should eventually resolve itself, but it's poor form.
 
     const oneTimeKeyAlgorithm = "signed_curve25519";
-    return baseApis.claimOneTimeKeys(
+    const res = await baseApis.claimOneTimeKeys(
         devicesWithoutSession, oneTimeKeyAlgorithm,
-    ).then(function(res) {
-        const otk_res = res.one_time_keys || {};
-        for (const userId in devicesByUser) {
-            if (!devicesByUser.hasOwnProperty(userId)) {
+    );
+
+    const otk_res = res.one_time_keys || {};
+    const promises = [];
+    for (const userId in devicesByUser) {
+        if (!devicesByUser.hasOwnProperty(userId)) {
+            continue;
+        }
+        const userRes = otk_res[userId] || {};
+        const devices = devicesByUser[userId];
+        for (let j = 0; j < devices.length; j++) {
+            const deviceInfo = devices[j];
+            const deviceId = deviceInfo.deviceId;
+            if (result[userId][deviceId].sessionId) {
+                // we already have a result for this device
                 continue;
             }
-            const userRes = otk_res[userId] || {};
-            const devices = devicesByUser[userId];
-            for (let j = 0; j < devices.length; j++) {
-                const deviceInfo = devices[j];
-                const deviceId = deviceInfo.deviceId;
-                if (result[userId][deviceId].sessionId) {
-                    // we already have a result for this device
-                    continue;
-                }
 
-                const deviceRes = userRes[deviceId] || {};
-                let oneTimeKey = null;
-                for (const keyId in deviceRes) {
-                    if (keyId.indexOf(oneTimeKeyAlgorithm + ":") === 0) {
-                        oneTimeKey = deviceRes[keyId];
-                    }
-                }
-
-                if (!oneTimeKey) {
-                    console.warn(
-                        "No one-time keys (alg=" + oneTimeKeyAlgorithm +
-                            ") for device " + userId + ":" + deviceId,
-                    );
-                    continue;
+            const deviceRes = userRes[deviceId] || {};
+            let oneTimeKey = null;
+            for (const keyId in deviceRes) {
+                if (keyId.indexOf(oneTimeKeyAlgorithm + ":") === 0) {
+                    oneTimeKey = deviceRes[keyId];
                 }
+            }
 
-                const sid = _verifyKeyAndStartSession(
-                    olmDevice, oneTimeKey, userId, deviceInfo,
+            if (!oneTimeKey) {
+                console.warn(
+                    "No one-time keys (alg=" + oneTimeKeyAlgorithm +
+                        ") for device " + userId + ":" + deviceId,
                 );
-                result[userId][deviceId].sessionId = sid;
+                continue;
             }
+
+            promises.push(
+                _verifyKeyAndStartSession(
+                    olmDevice, oneTimeKey, userId, deviceInfo,
+                ).then((sid) => {
+                    result[userId][deviceId].sessionId = sid;
+                }),
+            );
         }
-        return result;
-    });
-};
+    }
 
+    await Promise.all(promises);
+    return result;
+};
 
-function _verifyKeyAndStartSession(olmDevice, oneTimeKey, userId, deviceInfo) {
+async function _verifyKeyAndStartSession(olmDevice, oneTimeKey, userId, deviceInfo) {
     const deviceId = deviceInfo.deviceId;
     try {
-        _verifySignature(
+        await _verifySignature(
             olmDevice, oneTimeKey, userId, deviceId,
             deviceInfo.getFingerprint(),
         );
@@ -249,8 +257,11 @@ function _verifyKeyAndStartSession(olmDevice, oneTimeKey, userId, deviceInfo) {
  * @param {string} signingDeviceId  ID of the device whose signature should be checked
  *
  * @param {string} signingKey   base64-ed ed25519 public key
+ *
+ * Returns a promise which resolves (to undefined) if the the signature is good,
+ * or rejects with an Error if it is bad.
  */
-const _verifySignature = module.exports.verifySignature = function(
+const _verifySignature = module.exports.verifySignature = async function(
     olmDevice, obj, signingUserId, signingDeviceId, signingKey,
 ) {
     const signKeyId = "ed25519:" + signingDeviceId;