From aff32afefa9aa6eab33a2272b428b11b1a87f740 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 26 Jul 2017 08:04:46 +0100 Subject: [PATCH 1/3] Make olmlib.encryptMessageForDevice async --- spec/unit/crypto/algorithms/megolm.spec.js | 3 +- src/crypto/algorithms/megolm.js | 56 ++++++++++++---------- src/crypto/algorithms/olm.js | 14 ++++-- src/crypto/olmlib.js | 5 +- 4 files changed, 45 insertions(+), 33 deletions(-) diff --git a/spec/unit/crypto/algorithms/megolm.spec.js b/spec/unit/crypto/algorithms/megolm.spec.js index ec7e65d41..f83307612 100644 --- a/spec/unit/crypto/algorithms/megolm.spec.js +++ b/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; }); diff --git a/src/crypto/algorithms/megolm.js b/src/crypto/algorithms/megolm.js index bb9bf4d25..9f630327f 100644 --- a/src/crypto/algorithms/megolm.js +++ b/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}`); @@ -753,7 +757,7 @@ MegolmDecryption.prototype.shareKeysWithDevice = function(keyRequest) { // // ensureOlmSessionsForUsers has already done the logging, // so just skip it. - return; + return null; } console.log( @@ -772,7 +776,7 @@ MegolmDecryption.prototype.shareKeysWithDevice = function(keyRequest) { ciphertext: {}, }; - this.olmlib.encryptMessageForDevice( + return this.olmlib.encryptMessageForDevice( encryptedContent.ciphertext, this._userId, this._deviceId, @@ -780,16 +784,16 @@ MegolmDecryption.prototype.shareKeysWithDevice = function(keyRequest) { userId, deviceInfo, payload, - ); + ).then(() => { + const contentMap = { + [userId]: { + [deviceId]: encryptedContent, + }, + }; - const contentMap = { - [userId]: { - [deviceId]: encryptedContent, - }, - }; - - // TODO: retries - return this._baseApis.sendToDevice("m.room.encrypted", contentMap); + // TODO: retries + return this._baseApis.sendToDevice("m.room.encrypted", contentMap); + }); }).done(); }; diff --git a/src/crypto/algorithms/olm.js b/src/crypto/algorithms/olm.js index 9cfe49a9b..26f2dd4b3 100644 --- a/src/crypto/algorithms/olm.js +++ b/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); }); }; diff --git a/src/crypto/olmlib.js b/src/crypto/olmlib.js index 476aef57d..5cc162bcd 100644 --- a/src/crypto/olmlib.js +++ b/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, From a2d7b221ee8a63893d82fc9268a702dc8ff40100 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 26 Jul 2017 08:04:46 +0100 Subject: [PATCH 2/3] Make olmlib.verifySignature async --- src/crypto/DeviceList.js | 2 +- src/crypto/olmlib.js | 22 ++++++++++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/crypto/DeviceList.js b/src/crypto/DeviceList.js index 034add04f..9a54b6346 100644 --- a/src/crypto/DeviceList.js +++ b/src/crypto/DeviceList.js @@ -587,7 +587,7 @@ function _updateStoredDeviceKeysForUser(_olmDevice, userId, userStore, 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); diff --git a/src/crypto/olmlib.js b/src/crypto/olmlib.js index 5cc162bcd..f2f019931 100644 --- a/src/crypto/olmlib.js +++ b/src/crypto/olmlib.js @@ -165,6 +165,7 @@ module.exports.ensureOlmSessionsForDevices = function( devicesWithoutSession, oneTimeKeyAlgorithm, ).then(function(res) { const otk_res = res.one_time_keys || {}; + const promises = []; for (const userId in devicesByUser) { if (!devicesByUser.hasOwnProperty(userId)) { continue; @@ -195,21 +196,23 @@ module.exports.ensureOlmSessionsForDevices = function( continue; } - const sid = _verifyKeyAndStartSession( - olmDevice, oneTimeKey, userId, deviceInfo, + promises.push( + _verifyKeyAndStartSession( + olmDevice, oneTimeKey, userId, deviceInfo, + ).then((sid) => { + result[userId][deviceId].sessionId = sid; + }), ); - result[userId][deviceId].sessionId = sid; } } - return result; + return 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(), ); @@ -252,8 +255,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; From ef889963d9d65b5a4f01c68d2f3644c28d08e56f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 26 Jul 2017 08:04:46 +0100 Subject: [PATCH 3/3] Rewrite olmlib.ensureOlmSessionsForDevices as async This is non-functional. It just looks a lot prettier. --- src/crypto/olmlib.js | 84 +++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 41 deletions(-) diff --git a/src/crypto/olmlib.js b/src/crypto/olmlib.js index f2f019931..e7fd76e3e 100644 --- a/src/crypto/olmlib.js +++ b/src/crypto/olmlib.js @@ -121,7 +121,7 @@ module.exports.encryptMessageForDevice = async 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 = [ @@ -151,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 @@ -161,52 +161,54 @@ 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 || {}; - const promises = []; - 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]; - } + 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; - } - - promises.push( - _verifyKeyAndStartSession( - olmDevice, oneTimeKey, userId, deviceInfo, - ).then((sid) => { - result[userId][deviceId].sessionId = sid; - }), - ); } + + if (!oneTimeKey) { + console.warn( + "No one-time keys (alg=" + oneTimeKeyAlgorithm + + ") for device " + userId + ":" + deviceId, + ); + continue; + } + + promises.push( + _verifyKeyAndStartSession( + olmDevice, oneTimeKey, userId, deviceInfo, + ).then((sid) => { + result[userId][deviceId].sessionId = sid; + }), + ); } - return Promise.all(promises).return(result); - }); + } + + await Promise.all(promises); + return result; }; async function _verifyKeyAndStartSession(olmDevice, oneTimeKey, userId, deviceInfo) {