diff --git a/lib/crypto/OlmDevice.js b/lib/crypto/OlmDevice.js index f38016d88..2b65cd9a9 100644 --- a/lib/crypto/OlmDevice.js +++ b/lib/crypto/OlmDevice.js @@ -374,6 +374,10 @@ OlmDevice.prototype.encryptMessage = function( ) { var self = this; + if (payloadString === undefined) { + throw new Error("payloadString undefined"); + } + return this._getSession(theirDeviceIdentityKey, sessionId, function(session) { var res = session.encrypt(payloadString); self._saveSession(theirDeviceIdentityKey, session); diff --git a/lib/crypto/algorithms/base.js b/lib/crypto/algorithms/base.js index 2e406b459..2e1c8e72d 100644 --- a/lib/crypto/algorithms/base.js +++ b/lib/crypto/algorithms/base.js @@ -45,6 +45,7 @@ module.exports.DECRYPTION_CLASSES = {}; * @alias module:crypto/algorithms/base.EncryptionAlgorithm * * @param {object} params parameters + * @param {string} params.userId The UserID for the local user * @param {string} params.deviceId The identifier for this device. * @param {module:crypto} params.crypto crypto core * @param {module:crypto/OlmDevice} params.olmDevice olm.js wrapper @@ -52,6 +53,7 @@ module.exports.DECRYPTION_CLASSES = {}; * @param {string} params.roomId The ID of the room we will be sending to */ var EncryptionAlgorithm = function(params) { + this._userId = params.userId; this._deviceId = params.deviceId; this._crypto = params.crypto; this._olmDevice = params.olmDevice; @@ -101,9 +103,11 @@ EncryptionAlgorithm.prototype.onNewDevice = function(userId, deviceId) {}; * @alias module:crypto/algorithms/base.DecryptionAlgorithm * * @param {object} params parameters + * @param {string} params.userId The UserID for the local user * @param {module:crypto/OlmDevice} params.olmDevice olm.js wrapper */ var DecryptionAlgorithm = function(params) { + this._userId = params.userId; this._olmDevice = params.olmDevice; }; /** */ diff --git a/lib/crypto/algorithms/megolm.js b/lib/crypto/algorithms/megolm.js index cf9c09ab1..0452b0dc5 100644 --- a/lib/crypto/algorithms/megolm.js +++ b/lib/crypto/algorithms/megolm.js @@ -244,17 +244,27 @@ MegolmEncryption.prototype._shareKeyWithDevices = function(session_id, shareMap) var deviceInfo = sessionResult.device; + var encryptedContent = { + algorithm: olmlib.OLM_ALGORITHM, + sender_key: self._olmDevice.deviceCurve25519Key, + ciphertext: {}, + }; + + olmlib.encryptMessageForDevice( + encryptedContent.ciphertext, + self._userId, + self._deviceId, + self._olmDevice, + userId, + deviceInfo, + payload + ); + if (!contentMap[userId]) { contentMap[userId] = {}; } - contentMap[userId][deviceId] = - olmlib.encryptMessageForDevices( - self._deviceId, - self._olmDevice, - [deviceInfo.getIdentityKey()], - payload - ); + contentMap[userId][deviceId] = encryptedContent; haveTargets = true; } } @@ -413,21 +423,35 @@ MegolmDecryption.prototype.decryptEvent = function(event) { throw new base.DecryptionError("Missing fields in input"); } + var res; try { - var res = this._olmDevice.decryptGroupMessage( + res = this._olmDevice.decryptGroupMessage( event.room_id, content.sender_key, content.session_id, content.ciphertext ); - if (res === null) { - return null; - } - return { - payload: JSON.parse(res.result), - keysClaimed: res.keysClaimed, - keysProved: res.keysProved, - }; } catch (e) { throw new base.DecryptionError(e); } + + if (res === null) { + return null; + } + + var payload = JSON.parse(res.result); + + // belt-and-braces check that the room id matches that indicated by the HS + // (this is somewhat redundant, since the megolm session is scoped to the + // room, so neither the sender nor a MITM can lie about the room_id). + if (payload.room_id !== event.room_id) { + throw new base.DecryptionError( + "Message intended for room " + payload.room_id + ); + } + + return { + payload: payload, + keysClaimed: res.keysClaimed, + keysProved: res.keysProved, + }; }; /** diff --git a/lib/crypto/algorithms/olm.js b/lib/crypto/algorithms/olm.js index 55443f860..3b0b48bd8 100644 --- a/lib/crypto/algorithms/olm.js +++ b/lib/crypto/algorithms/olm.js @@ -95,32 +95,43 @@ OlmEncryption.prototype.encryptMessage = function(room, eventType, content) { var self = this; return this._ensureSession(users).then(function() { - var participantKeys = []; + var payloadFields = { + room_id: room.roomId, + type: eventType, + content: content, + }; + + var encryptedContent = { + algorithm: olmlib.OLM_ALGORITHM, + sender_key: self._olmDevice.deviceCurve25519Key, + ciphertext: {}, + }; + for (var i = 0; i < users.length; ++i) { var userId = users[i]; var devices = self._crypto.getStoredDevicesForUser(userId); + for (var j = 0; j < devices.length; ++j) { var deviceInfo = devices[j]; var key = deviceInfo.getIdentityKey(); if (key == self._olmDevice.deviceCurve25519Key) { - // don't bother setting up session to ourself + // don't bother sending to ourself continue; } if (deviceInfo.verified == DeviceVerification.BLOCKED) { // don't bother setting up sessions with blocked users continue; } - participantKeys.push(key); + + olmlib.encryptMessageForDevice( + encryptedContent.ciphertext, + self._userId, self._deviceId, self._olmDevice, + userId, deviceInfo, payloadFields + ); } } - return olmlib.encryptMessageForDevices( - self._deviceId, self._olmDevice, participantKeys, { - room_id: room.roomId, - type: eventType, - content: content, - } - ); + return encryptedContent; }); }; @@ -173,10 +184,70 @@ OlmDecryption.prototype.decryptEvent = function(event) { throw new base.DecryptionError("Bad Encrypted Message"); } - - // TODO: Check the sender user id matches the sender key. - // TODO: check the room_id and fingerprint var payload = JSON.parse(payloadString); + + // check that we were the intended recipient, to avoid unknown-key attack + // https://github.com/vector-im/vector-web/issues/2483 + if (payload.recipient === undefined) { + // older versions of riot did not set this field, so we cannot make + // this check. TODO: kill this off once our users have updated + console.warn( + "Olm event (id=" + event.event_id + ") contains no 'recipient' " + + "property; cannot prevent unknown-key attack"); + } else if (payload.recipient != this._userId) { + console.warn( + "Event " + event.event_id + ": Intended recipient " + + payload.recipient + " does not match our id " + this._userId + ); + throw new base.DecryptionError( + "Message was intented for " + payload.recipient + ); + } + + if (payload.recipient_keys === undefined) { + // ditto + console.warn( + "Olm event (id=" + event.event_id + ") contains no " + + "'recipient_keys' property; cannot prevent unknown-key attack"); + } else if (payload.recipient_keys.ed25519 != + this._olmDevice.deviceEd25519Key) { + console.warn( + "Event " + event.event_id + ": Intended recipient ed25519 key " + + payload.recipient_keys.ed25519 + " did not match ours" + ); + throw new base.DecryptionError("Message not intended for this device"); + } + + // check that the original sender matches what the homeserver told us, to + // avoid people masquerading as others. + // (this check is also provided via the sender's embedded ed25519 key, + // which is checked elsewhere). + if (payload.sender === undefined) { + // ditto + console.warn( + "Olm event (id=" + event.event_id + ") contains no " + + "'sender' property; cannot prevent unknown-key attack"); + } else if (payload.sender != event.sender) { + console.warn( + "Event " + event.event_id + ": original sender " + payload.sender + + " does not match reported sender " + event.sender + ); + throw new base.DecryptionError( + "Message forwarded from " + payload.sender + ); + } + + // Olm events intended for a room have a room_id. + if (payload.room_id !== event.room_id) { + console.warn( + "Event " + event.event_id + ": original room " + payload.room_id + + " does not match reported room " + event.room_id + ); + throw new base.DecryptionError( + "Message intended for room " + payload.room_id + ); + } + return { payload: payload, sessionExists: true, diff --git a/lib/crypto/index.js b/lib/crypto/index.js index 5a641c09d..9ce79b501 100644 --- a/lib/crypto/index.js +++ b/lib/crypto/index.js @@ -694,6 +694,7 @@ Crypto.prototype.setRoomEncryption = function(roomId, config) { this._sessionStore.storeEndToEndRoom(roomId, config); var alg = new AlgClass({ + userId: this._userId, deviceId: this._deviceId, crypto: this, olmDevice: this._olmDevice, @@ -901,6 +902,7 @@ Crypto.prototype.decryptEvent = function(event) { throw new algorithms.DecryptionError("Unable to decrypt " + content.algorithm); } var alg = new AlgClass({ + userId: this._userId, olmDevice: this._olmDevice, }); var r = alg.decryptEvent(event); @@ -1084,6 +1086,7 @@ Crypto.prototype._onRoomKeyEvent = function(event) { ); } var alg = new AlgClass({ + userId: this._userId, olmDevice: this._olmDevice, }); alg.onRoomKeyEvent(event); diff --git a/lib/crypto/olmlib.js b/lib/crypto/olmlib.js index a6db51f64..4152755fb 100644 --- a/lib/crypto/olmlib.js +++ b/lib/crypto/olmlib.js @@ -34,22 +34,38 @@ module.exports.MEGOLM_ALGORITHM = "m.megolm.v1.aes-sha2"; /** - * Encrypt an event payload for a list of devices + * Encrypt an event payload for an Olm device * + * @param {Object} resultsObject The `ciphertext` property + * of the m.room.encrypted event to which to add our result + * + * @param {string} ourUserId * @param {string} ourDeviceId * @param {module:crypto/OlmDevice} olmDevice olm.js wrapper - * @param {string[]} participantKeys list of curve25519 keys to encrypt for + * @param {string} recipientUserId + * @param {module:crypto/deviceinfo} recipientDevice * @param {object} payloadFields fields to include in the encrypted payload - * - * @return {object} content for an m.room.encrypted event */ -module.exports.encryptMessageForDevices = function( - ourDeviceId, olmDevice, participantKeys, payloadFields +module.exports.encryptMessageForDevice = function( + resultsObject, + ourUserId, ourDeviceId, olmDevice, recipientUserId, recipientDevice, + payloadFields ) { - participantKeys.sort(); - var participantHash = ""; // Olm.sha256(participantKeys.join()); - var payloadJson = { - fingerprint: participantHash, + var deviceKey = recipientDevice.getIdentityKey(); + var sessionId = olmDevice.getSessionIdForDevice(deviceKey); + if (sessionId === null) { + // If we don't have a session for a device then + // we can't encrypt a message for it. + return; + } + + console.log( + "Using sessionid " + sessionId + " for device " + + recipientUserId + ":" + recipientDevice.deviceId + ); + + var payload = { + sender: ourUserId, sender_device: ourDeviceId, // Include the Ed25519 key so that the recipient knows what @@ -63,28 +79,24 @@ module.exports.encryptMessageForDevices = function( keys: { "ed25519": olmDevice.deviceEd25519Key, }, - }; - utils.extend(payloadJson, payloadFields); - var ciphertext = {}; - var payloadString = JSON.stringify(payloadJson); - for (var i = 0; i < participantKeys.length; ++i) { - var deviceKey = participantKeys[i]; - var sessionId = olmDevice.getSessionIdForDevice(deviceKey); - if (sessionId === null) { - // If we don't have a session for a device then - // we can't encrypt a message for it. - continue; - } - console.log("Using sessionid " + sessionId + " for device " + deviceKey); - ciphertext[deviceKey] = olmDevice.encryptMessage( - deviceKey, sessionId, payloadString - ); - } - var encryptedContent = { - algorithm: module.exports.OLM_ALGORITHM, - sender_key: olmDevice.deviceCurve25519Key, - ciphertext: ciphertext + // include the recipient device details in the payload, + // to avoid unknown key attacks, per + // https://github.com/vector-im/vector-web/issues/2483 + recipient: recipientUserId, + recipient_keys: { + "ed25519": recipientDevice.getFingerprint(), + }, }; - return encryptedContent; + + // TODO: technically, a bunch of that stuff only needs to be included for + // pre-key messages: after that, both sides know exactly which devices are + // involved in the session. If we're looking to reduce data transfer in the + // future, we could elide them for subsequent messages. + + utils.extend(payload, payloadFields); + + resultsObject[deviceKey] = olmDevice.encryptMessage( + deviceKey, sessionId, JSON.stringify(payload) + ); }; diff --git a/spec/integ/matrix-client-crypto.spec.js b/spec/integ/matrix-client-crypto.spec.js index be121eaf4..f9ef98f68 100644 --- a/spec/integ/matrix-client-crypto.spec.js +++ b/spec/integ/matrix-client-crypto.spec.js @@ -338,15 +338,15 @@ function expectSendMessageRequest(httpBackend) { function aliRecvMessage() { var message = bobMessages.shift(); - return recvMessage(aliHttpBackend, aliClient, message); + return recvMessage(aliHttpBackend, aliClient, bobUserId, message); } function bobRecvMessage() { var message = aliMessages.shift(); - return recvMessage(bobHttpBackend, bobClient, message); + return recvMessage(bobHttpBackend, bobClient, aliUserId, message); } -function recvMessage(httpBackend, client, message) { +function recvMessage(httpBackend, client, sender, message) { var syncData = { next_batch: "x", rooms: { @@ -361,7 +361,8 @@ function recvMessage(httpBackend, client, message) { test_utils.mkEvent({ type: "m.room.encrypted", room: roomId, - content: message + content: message, + sender: sender, }) ] } @@ -557,6 +558,63 @@ describe("MatrixClient crypto", function() { .catch(test_utils.failTest).done(done); }); + it("Bob receives a message with a bogus sender", function(done) { + q() + .then(bobUploadsKeys) + .then(aliStartClient) + .then(aliEnablesEncryption) + .then(aliSendsFirstMessage) + .then(bobStartClient) + .then(function() { + var message = aliMessages.shift(); + var syncData = { + next_batch: "x", + rooms: { + join: { + + } + } + }; + syncData.rooms.join[roomId] = { + timeline: { + events: [ + test_utils.mkEvent({ + type: "m.room.encrypted", + room: roomId, + content: message, + sender: "@bogus:sender", + }) + ] + } + }; + bobHttpBackend.when("GET", "/sync").respond(200, syncData); + + var deferred = q.defer(); + var onEvent = function(event) { + console.log(bobClient.credentials.userId + " received event", + event); + + // ignore the m.room.member events + if (event.getType() == "m.room.member") { + return; + } + + expect(event.getType()).toEqual("m.room.message"); + expect(event.getContent().msgtype).toEqual("m.bad.encrypted"); + expect(event.isEncrypted()).toBeTruthy(); + + bobClient.removeListener("event", onEvent); + deferred.resolve(); + }; + + bobClient.on("event", onEvent); + + bobHttpBackend.flush(); + return deferred.promise; + }) + .catch(test_utils.failTest).done(done); + }); + it("Ali blocks Bob's device", function(done) { q() .then(bobUploadsKeys)