From 6769c96942dc5d1edff937c66b9b702935fe23be Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 2 Nov 2016 17:55:23 +0000 Subject: [PATCH 01/29] Add a method for querying the js-sdk's current 'request' function in case people want to wrap it --- lib/matrix.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/matrix.js b/lib/matrix.js index 1a4e8ee4e..4edc6c326 100644 --- a/lib/matrix.js +++ b/lib/matrix.js @@ -82,6 +82,13 @@ module.exports.request = function(r) { request = r; }; +/** + * Return the currently-set request function. + */ +module.exports.getRequest = function() { + return request; +}; + /** * Construct a Matrix Client. Similar to {@link module:client~MatrixClient} * except that the 'request', 'store' and 'scheduler' dependencies are satisfied. From 4529578cd6e7e4b350d7aa16c24475886370b535 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 2 Nov 2016 18:02:02 +0000 Subject: [PATCH 02/29] Make a handy shortcut for SDK users to provide request wrapping functions in a neat stack --- lib/matrix.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/lib/matrix.js b/lib/matrix.js index 4edc6c326..56d38d520 100644 --- a/lib/matrix.js +++ b/lib/matrix.js @@ -89,6 +89,19 @@ module.exports.getRequest = function() { return request; }; +/** + * Apply wrapping code around the request function. The wrapper function is + * installed as the new request handler, and when invoked it is passed the + * previous value, along with the options and callback arguments. + * @param {requestWrapperFunction} wrapper The wrapping function. + */ +module.exports.wrapRequest = function(wrapper) { + var origRequest = request; + request = function(options, callback) { + return wrapper(origRequest, options, callback); + }; +}; + /** * Construct a Matrix Client. Similar to {@link module:client~MatrixClient} * except that the 'request', 'store' and 'scheduler' dependencies are satisfied. @@ -136,6 +149,16 @@ module.exports.createClient = function(opts) { * @param {requestCallback} callback The request callback. */ +/** + * A wrapper for the request function interface. + * @callback requestWrapperFunction + * @param {requestFunction} origRequest The underlying request function being + * wrapped + * @param {Object} opts The options for this HTTP request, given in the same + * form as {@link requestFunction}. + * @param {requestCallback} callback The request callback. + */ + /** * The request callback interface for performing HTTP requests. This matches the * API for the {@link https://github.com/request/request#requestoptions-callback| From 65f1b3c9760caa1753f4d6bf78aec0f250217c76 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 2 Nov 2016 18:04:00 +0000 Subject: [PATCH 03/29] Document the return type of getRequest() --- lib/matrix.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/matrix.js b/lib/matrix.js index 56d38d520..260f1d23f 100644 --- a/lib/matrix.js +++ b/lib/matrix.js @@ -84,6 +84,7 @@ module.exports.request = function(r) { /** * Return the currently-set request function. + * @return {requestFunction} The current request function. */ module.exports.getRequest = function() { return request; From e173d822e81cead3f152a59412b6a1e8ce73f6cd Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 7 Nov 2016 18:57:09 +0000 Subject: [PATCH 04/29] Log to the console on unknown session This might help diagnose Erik/Matthew's comms breakdown. --- lib/crypto/index.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/crypto/index.js b/lib/crypto/index.js index 0e7dafefa..a0487b70d 100644 --- a/lib/crypto/index.js +++ b/lib/crypto/index.js @@ -958,6 +958,10 @@ Crypto.prototype.decryptEvent = function(event) { // back to all devices. device_id = null; } + console.log( + "Unknown session decrypting event id " + event.event_id + + ": sending m.new_device event" + ); this._sendPingToDevice(event.sender, device_id, event.room_id); } From 77508f38bbf8028e3efaf436f95bf623953de897 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 8 Nov 2016 16:52:12 +0000 Subject: [PATCH 05/29] event jsdoc Add a comment on the event event --- lib/client.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/client.js b/lib/client.js index 3c02e4807..77b477d3e 100644 --- a/lib/client.js +++ b/lib/client.js @@ -2872,6 +2872,10 @@ module.exports.CRYPTO_ENABLED = CRYPTO_ENABLED; /** * Fires whenever the SDK receives a new event. + *

+ * This is only fired for live events received via /sync - it is not fired for + * events received over context, search, or pagination APIs. + * * @event module:client~MatrixClient#"event" * @param {MatrixEvent} event The matrix event which caused this event to fire. * @example From 2113c83679c469042edeb30292138d4626d5cd4a Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 10 Nov 2016 19:28:08 +0000 Subject: [PATCH 06/29] Ignore reshares of known megolm sessions If we get a second key for a known megolm session, ignore it. Fixes https://github.com/vector-im/vector-web/issues/2326, one hopes. --- lib/crypto/OlmDevice.js | 18 ++ spec/integ/matrix-client-crypto.spec.js | 21 +- spec/integ/megolm.spec.js | 366 ++++++++++++++++++++++++ spec/test-utils.js | 48 +++- 4 files changed, 431 insertions(+), 22 deletions(-) create mode 100644 spec/integ/megolm.spec.js diff --git a/lib/crypto/OlmDevice.js b/lib/crypto/OlmDevice.js index 1d7e8598c..07335e548 100644 --- a/lib/crypto/OlmDevice.js +++ b/lib/crypto/OlmDevice.js @@ -627,6 +627,24 @@ OlmDevice.prototype.addInboundGroupSession = function( roomId, senderKey, sessionId, sessionKey, keysClaimed ) { var self = this; + + /* if we already have this session, consider updating it */ + function updateSession(session) { + console.log("Update for megolm session " + senderKey + "/" + sessionId); + // for now we just ignore updates. TODO: implement something here + + return true; + } + + var r = this._getInboundGroupSession( + roomId, senderKey, sessionId, updateSession + ); + + if (r !== null) { + return; + } + + // new session. var session = new Olm.InboundGroupSession(); try { session.create(sessionKey); diff --git a/spec/integ/matrix-client-crypto.spec.js b/spec/integ/matrix-client-crypto.spec.js index 14207eafe..5066304a0 100644 --- a/spec/integ/matrix-client-crypto.spec.js +++ b/spec/integ/matrix-client-crypto.spec.js @@ -5,21 +5,6 @@ var HttpBackend = require("../mock-request"); var utils = require("../../lib/utils"); var test_utils = require("../test-utils"); -function MockStorageApi() { - this.data = {}; -} -MockStorageApi.prototype = { - setItem: function(k, v) { - this.data[k] = v; - }, - getItem: function(k) { - return this.data[k] || null; - }, - removeItem: function(k) { - delete this.data[k]; - } -}; - var aliHttpBackend; var bobHttpBackend; var aliClient; @@ -36,7 +21,6 @@ var aliDeviceKeys; var bobDeviceKeys; var bobDeviceCurve25519Key; var bobDeviceEd25519Key; -var aliLocalStore; var aliStorage; var bobStorage; var aliMessages; @@ -461,11 +445,9 @@ describe("MatrixClient crypto", function() { } beforeEach(function() { - aliLocalStore = new MockStorageApi(); - aliStorage = new sdk.WebStorageSessionStore(aliLocalStore); - bobStorage = new sdk.WebStorageSessionStore(new MockStorageApi()); test_utils.beforeEach(this); + aliStorage = new sdk.WebStorageSessionStore(new test_utils.MockStorageApi()); aliHttpBackend = new HttpBackend(); aliClient = sdk.createClient({ baseUrl: "http://alis.server", @@ -476,6 +458,7 @@ describe("MatrixClient crypto", function() { request: aliHttpBackend.requestFn, }); + bobStorage = new sdk.WebStorageSessionStore(new test_utils.MockStorageApi()); bobHttpBackend = new HttpBackend(); bobClient = sdk.createClient({ baseUrl: "http://bobs.server", diff --git a/spec/integ/megolm.spec.js b/spec/integ/megolm.spec.js new file mode 100644 index 000000000..f30aa428e --- /dev/null +++ b/spec/integ/megolm.spec.js @@ -0,0 +1,366 @@ +/* +Copyright 2016 OpenMarket Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +"use strict"; + + +try { + var Olm = require('olm'); +} catch (e) {} + +var sdk = require('../..'); +var utils = require('../../lib/utils'); +var test_utils = require('../test-utils'); +var MockHttpBackend = require('../mock-request'); + +/** + * Wrapper for a MockStorageApi, MockHttpBackend and MatrixClient + * + * @constructor + * @param {string} userId + * @param {string} deviceId + * @param {string} accessToken + */ +function TestClient(userId, deviceId, accessToken) { + this.userId = userId; + this.deviceId = deviceId; + + this.storage = new sdk.WebStorageSessionStore(new test_utils.MockStorageApi()); + this.httpBackend = new MockHttpBackend(); + this.client = sdk.createClient({ + baseUrl: "http://test.server", + userId: userId, + accessToken: accessToken, + deviceId: deviceId, + sessionStore: this.storage, + request: this.httpBackend.requestFn, + }); + + this.deviceKeys = null; + this.oneTimeKeys = []; +} + +/** + * start the client, and wait for it to initialise. + * + * @return {Promise} + */ +TestClient.prototype.start = function() { + var self = this; + this.httpBackend.when("GET", "/pushrules").respond(200, {}); + this.httpBackend.when("POST", "/filter").respond(200, { filter_id: "fid" }); + this.httpBackend.when("POST", "/keys/upload").respond(200, function(path, content) { + expect(content.one_time_keys).not.toBeDefined(); + expect(content.device_keys).toBeDefined(); + self.deviceKeys = content.device_keys; + return {one_time_key_counts: {signed_curve25519: 0}}; + }); + this.httpBackend.when("POST", "/keys/upload").respond(200, function(path, content) { + expect(content.device_keys).not.toBeDefined(); + expect(content.one_time_keys).toBeDefined(); + expect(content.one_time_keys).not.toEqual({}); + self.oneTimeKeys = content.one_time_keys; + return {one_time_key_counts: { + signed_curve25519: utils.keys(self.oneTimeKeys).length + }}; + }); + + this.client.startClient(); + + return this.httpBackend.flush(); +}; + +/** + * stop the client + */ +TestClient.prototype.stop = function() { + this.client.stopClient(); +}; + +/** + * get the uploaded curve25519 device key + * + * @return {string} base64 device key + */ +TestClient.prototype.getDeviceKey = function() { + var key_id = 'curve25519:' + this.deviceId; + return this.deviceKeys.keys[key_id]; +}; + + +/** + * start an Olm session with a given recipient + * + * @param {Olm.Account} olmAccount + * @param {TestClient} recipientTestClient + * @return {Olm.Session} + */ +function createOlmSession(olmAccount, recipientTestClient) { + var otk_id = utils.keys(recipientTestClient.oneTimeKeys)[0]; + var otk = recipientTestClient.oneTimeKeys[otk_id]; + + var session = new Olm.Session(); + session.create_outbound( + olmAccount, recipientTestClient.getDeviceKey(), otk.key + ); + return session; +} + +/** + * encrypt an event with olm + * + * @param {object} opts + * @param {string=} opts.sender + * @param {string} opts.senderKey + * @param {string} opts.recipientKey + * @param {Olm.Session} opts.p2pSession + * @param {object} opts.plaintext + * + * @return {object} event + */ +function encryptOlmEvent(opts) { + expect(opts.senderKey).toBeDefined(); + expect(opts.p2pSession).toBeDefined(); + expect(opts.plaintext).toBeDefined(); + expect(opts.recipientKey).toBeDefined(); + + var event = { + content: { + algorithm: "m.olm.v1.curve25519-aes-sha2", + ciphertext: {}, + sender_key: opts.senderKey, + }, + sender: opts.sender || "@bob:xyz", + type: "m.room.encrypted", + }; + event.content.ciphertext[opts.recipientKey] = + opts.p2pSession.encrypt(JSON.stringify(opts.plaintext)); + return event; +} + +/** + * encrypt an event with megolm + * + * @param {object} opts + * @param {string} opts.senderKey + * @param {Olm.OutboundGroupSession} opts.groupSession + * @param {object=} opts.plaintext + * @param {string=} opts.room_id + * + * @return {object} event + */ +function encryptMegolmEvent(opts) { + expect(opts.senderKey).toBeDefined(); + expect(opts.groupSession).toBeDefined(); + + var plaintext = opts.plaintext || {}; + if (!plaintext.content) { + plaintext.content = { + body: '42', + msgtype: "m.text", + }; + } + if (!plaintext.type) { + plaintext.type = "m.room.message"; + } + if (!plaintext.room_id) { + expect(opts.room_id).toBeDefined(); + plaintext.room_id = opts.room_id; + } + + return { + content: { + algorithm: "m.megolm.v1.aes-sha2", + ciphertext: opts.groupSession.encrypt(JSON.stringify(plaintext)), + device_id: "testDevice", + sender_key: opts.senderKey, + session_id: opts.groupSession.session_id(), + }, + type: "m.room.encrypted", + }; +} + +/** + * build an encrypted room_key event to share a group session + * + * @param {object} opts + * @param {string} opts.senderKey + * @param {string} opts.recipientKey + * @param {Olm.Session} opts.p2pSession + * @param {Olm.OutboundGroupSession} opts.groupSession + * @param {string=} opts.room_id + * + * @return {object} event + */ +function encryptGroupSessionKey(opts) { + return encryptOlmEvent({ + senderKey: opts.senderKey, + recipientKey: opts.recipientKey, + p2pSession: opts.p2pSession, + plaintext: { + content: { + algorithm: "m.megolm.v1.aes-sha2", + room_id: opts.room_id, + session_id: opts.groupSession.session_id(), + session_key: opts.groupSession.session_key(), + }, + type: "m.room_key", + }, + }); +} + +describe("megolm", function() { + if (!sdk.CRYPTO_ENABLED) { + return; + } + + var ROOM_ID = "!room:id"; + + var testOlmAccount; + var testSenderKey; + var aliceTestClient; + + beforeEach(test_utils.asyncTest(function() { + test_utils.beforeEach(this); + + aliceTestClient = new TestClient( + "@alice:localhost", "xzcvb", "akjgkrgjs" + ); + + testOlmAccount = new Olm.Account(); + testOlmAccount.create(); + var testE2eKeys = JSON.parse(testOlmAccount.identity_keys()); + testSenderKey = testE2eKeys.curve25519; + + return aliceTestClient.start(); + })); + + afterEach(function() { + aliceTestClient.stop(); + }); + + it("Alice receives a megolm message", test_utils.asyncTest(function() { + var p2pSession = createOlmSession(testOlmAccount, aliceTestClient); + + var groupSession = new Olm.OutboundGroupSession(); + groupSession.create(); + + // make the room_key event + var roomKeyEncrypted = encryptGroupSessionKey({ + senderKey: testSenderKey, + recipientKey: aliceTestClient.getDeviceKey(), + p2pSession: p2pSession, + groupSession: groupSession, + room_id: ROOM_ID, + }); + + // encrypt a message with the group session + var messageEncrypted = encryptMegolmEvent({ + senderKey: testSenderKey, + groupSession: groupSession, + room_id: ROOM_ID, + }); + + // Alice gets both the events in a single sync + var syncResponse = { + next_batch: 1, + to_device: { + events: [roomKeyEncrypted], + }, + rooms: { + join: {}, + }, + }; + syncResponse.rooms.join[ROOM_ID] = { + timeline: { + events: [messageEncrypted], + }, + }; + aliceTestClient.httpBackend.when("GET", "/sync").respond(200, syncResponse); + return aliceTestClient.httpBackend.flush("/sync", 1).then(function() { + var room = aliceTestClient.client.getRoom(ROOM_ID); + var event = room.getLiveTimeline().getEvents()[0]; + expect(event.getContent().body).toEqual('42'); + }); + })); + + it("Alice gets a second room_key message", test_utils.asyncTest(function() { + var p2pSession = createOlmSession(testOlmAccount, aliceTestClient); + + var groupSession = new Olm.OutboundGroupSession(); + groupSession.create(); + + // make the room_key event + var roomKeyEncrypted1 = encryptGroupSessionKey({ + senderKey: testSenderKey, + recipientKey: aliceTestClient.getDeviceKey(), + p2pSession: p2pSession, + groupSession: groupSession, + room_id: ROOM_ID, + }); + + // encrypt a message with the group session + var messageEncrypted = encryptMegolmEvent({ + senderKey: testSenderKey, + groupSession: groupSession, + room_id: ROOM_ID, + }); + + // make a second room_key event now that we have advanced the group + // session. + var roomKeyEncrypted2 = encryptGroupSessionKey({ + senderKey: testSenderKey, + recipientKey: aliceTestClient.getDeviceKey(), + p2pSession: p2pSession, + groupSession: groupSession, + room_id: ROOM_ID, + }); + + // on the first sync, send the best room key + aliceTestClient.httpBackend.when("GET", "/sync").respond(200, { + next_batch: 1, + to_device: { + events: [roomKeyEncrypted1], + }, + }); + + // on the second sync, send the advanced room key, along with the + // message. This simulates the situation where Alice has been sent a + // later copy of the room key and is reloading the client. + var syncResponse2 = { + next_batch: 2, + to_device: { + events: [roomKeyEncrypted2], + }, + rooms: { + join: {}, + }, + }; + syncResponse2.rooms.join[ROOM_ID] = { + timeline: { + events: [messageEncrypted], + }, + }; + aliceTestClient.httpBackend.when("GET", "/sync").respond(200, syncResponse2); + + return aliceTestClient.httpBackend.flush("/sync", 2).then(function() { + var room = aliceTestClient.client.getRoom(ROOM_ID); + var event = room.getLiveTimeline().getEvents()[0]; + expect(event.getContent().body).toEqual('42'); + }); + + })); +}); diff --git a/spec/test-utils.js b/spec/test-utils.js index ecee27331..109301e14 100644 --- a/spec/test-utils.js +++ b/spec/test-utils.js @@ -159,7 +159,7 @@ module.exports.mkMessage = function(opts) { *

This is useful for use with integration tests which use asyncronous * methods: it can be added as a 'catch' handler in a promise chain. * - * @param {Error} error exception to be reported + * @param {Error} err exception to be reported * * @example * it("should not throw", function(done) { @@ -168,6 +168,48 @@ module.exports.mkMessage = function(opts) { * }).catch(utils.failTest).done(done); * }); */ -module.exports.failTest = function(error) { - expect(error.stack).toBe(null); +module.exports.failTest = function(err) { + expect(true).toBe(false, "Testfunc threw: " + err.stack); +}; + +/** + * Wrap a test function which returns a promise into a format which + * jasmine will understand. + * + * @param {Function} testfunc test function, which should return a promise + * + * @return {Function} + * + * @example + * it("should not throw", asyncTest(function() { + * return asynchronousMethod().then(function() { + * // some tests + * }); + * })); + */ +module.exports.asyncTest = function(testfunc) { + return function(done) { + testfunc.call(this).catch(module.exports.failTest).done(done); + }; +}; + + +/** + * A mock implementation of webstorage + * + * @constructor + */ +module.exports.MockStorageApi = function() { + this.data = {}; +}; +module.exports.MockStorageApi.prototype = { + setItem: function(k, v) { + this.data[k] = v; + }, + getItem: function(k) { + return this.data[k] || null; + }, + removeItem: function(k) { + delete this.data[k]; + } }; From 24283dcbd5adea6dbb94491151c9e2d1c3fc3923 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Thu, 10 Nov 2016 19:42:16 +0000 Subject: [PATCH 07/29] Encrypt all events, including 'm.call.*' --- lib/crypto/index.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/crypto/index.js b/lib/crypto/index.js index 0e7dafefa..e9e22e4c8 100644 --- a/lib/crypto/index.js +++ b/lib/crypto/index.js @@ -842,7 +842,6 @@ Crypto.prototype.isRoomEncrypted = function(roomId) { return Boolean(this._roomAlgorithms[roomId]); }; - /** * Encrypt an event according to the configuration of the room, if necessary. * @@ -862,11 +861,6 @@ Crypto.prototype.encryptEventIfNeeded = function(event, room) { return null; } - if (event.getType() !== "m.room.message") { - // we only encrypt m.room.message - return null; - } - if (!room) { throw new Error("Cannot send encrypted messages in unknown rooms"); } From f8b1c124df34403c33c12c91aef051945183920a Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Fri, 11 Nov 2016 10:04:48 +0000 Subject: [PATCH 08/29] Add a travis.yml --- .travis.yml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 000000000..9d6a11439 --- /dev/null +++ b/.travis.yml @@ -0,0 +1,3 @@ +language: node_js +node_js: + - node # Latest stable version of nodejs. From a5f0ec7c7da0398d03a7ebd2af6bedbac5e573db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Wang?= Date: Sat, 12 Nov 2016 17:39:48 +0100 Subject: [PATCH 09/29] Fix examples. --- examples/browser/lib/matrix.js | 2 +- examples/node/app.js | 14 +++++++++----- examples/voip/browserTest.js | 12 ++++++++++-- examples/voip/lib/matrix.js | 2 +- 4 files changed, 21 insertions(+), 9 deletions(-) diff --git a/examples/browser/lib/matrix.js b/examples/browser/lib/matrix.js index e4b1b6ecb..518d47ddb 120000 --- a/examples/browser/lib/matrix.js +++ b/examples/browser/lib/matrix.js @@ -1 +1 @@ -../../../dist/browser-matrix-dev.js \ No newline at end of file +../../../dist/browser-matrix.js \ No newline at end of file diff --git a/examples/node/app.js b/examples/node/app.js index 65bed3ba2..dcff22aac 100644 --- a/examples/node/app.js +++ b/examples/node/app.js @@ -135,11 +135,15 @@ rl.on('line', function(line) { // ==== END User input // show the room list after syncing. -matrixClient.on("syncComplete", function() { - setRoomList(); - printRoomList(); - printHelp(); - rl.prompt(); +matrixClient.on("sync", function(state, prevState, data) { + switch (state) { + case "PREPARED": + setRoomList(); + printRoomList(); + printHelp(); + rl.prompt(); + break; + } }); matrixClient.on("Room", function() { diff --git a/examples/voip/browserTest.js b/examples/voip/browserTest.js index 53b19920c..29ede49a8 100644 --- a/examples/voip/browserTest.js +++ b/examples/voip/browserTest.js @@ -44,7 +44,15 @@ window.onload = function() { disableButtons(true, true, true); }; -client.on("syncComplete", function () { +matrixClient.on("sync", function(state, prevState, data) { + switch (state) { + case "PREPARED": + syncComplete(); + break; + } +}); + +function syncComplete() { document.getElementById("result").innerHTML = "

Ready for calls.

"; disableButtons(false, true, true); @@ -85,5 +93,5 @@ client.on("syncComplete", function () { call = c; addListeners(call); }); -}); +} client.startClient(); diff --git a/examples/voip/lib/matrix.js b/examples/voip/lib/matrix.js index e4b1b6ecb..518d47ddb 120000 --- a/examples/voip/lib/matrix.js +++ b/examples/voip/lib/matrix.js @@ -1 +1 @@ -../../../dist/browser-matrix-dev.js \ No newline at end of file +../../../dist/browser-matrix.js \ No newline at end of file From 7029083266ad2710c604b3a2fb3275d628f85ab9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Wang?= Date: Sat, 12 Nov 2016 21:58:58 +0100 Subject: [PATCH 10/29] Send a STOPPED sync updated after call to stopClient --- lib/sync.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/sync.js b/lib/sync.js index 554007aba..80bf36aa3 100644 --- a/lib/sync.js +++ b/lib/sync.js @@ -542,6 +542,7 @@ SyncApi.prototype._sync = function(syncOptions) { self._connectionReturnedDefer.reject(); self._connectionReturnedDefer = null; } + self._updateSyncState("STOPPED"); return; } console.error("/sync error %s", err); From 04093692c9a071b1768da7ca3a229edcc638d64a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Wang?= Date: Sun, 13 Nov 2016 13:47:31 +0100 Subject: [PATCH 11/29] Use native Array.isArray when available. --- lib/utils.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/utils.js b/lib/utils.js index 523cde013..8a1c09404 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -204,7 +204,8 @@ module.exports.isFunction = function(value) { * @return {boolean} True if it is an array. */ module.exports.isArray = function(value) { - return Boolean(value && value.constructor === Array); + return Array.isArray ? Array.isArray(value) : + Boolean(value && value.constructor === Array); }; /** From 1532188d951c356542c7a4914e0b488c98747e2e Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 13 Nov 2016 13:24:51 +0000 Subject: [PATCH 12/29] fix typo --- lib/http-api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/http-api.js b/lib/http-api.js index d1faf6f45..8eeefd349 100644 --- a/lib/http-api.js +++ b/lib/http-api.js @@ -104,7 +104,7 @@ module.exports.MatrixHttpApi.prototype = { * * @param {object} file The object to upload. On a browser, something that * can be sent to XMLHttpRequest.send (typically a File). Under node.js, - * a a Buffer, String or ReadStream. + * a Buffer, String or ReadStream. * * @param {object} opts options object * From e623b539c4872a6d32ced94a483417224470ddb0 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 1 Nov 2016 10:46:51 +0000 Subject: [PATCH 13/29] persist DecryptionAlgorithm instances It's useful to be able to keep state between events in the DecryptionAlgorithm, so store them in a map. --- lib/crypto/algorithms/base.js | 5 ++ lib/crypto/index.js | 93 ++++++++++++++++++++++++++--------- 2 files changed, 74 insertions(+), 24 deletions(-) diff --git a/lib/crypto/algorithms/base.js b/lib/crypto/algorithms/base.js index 6d25bc2bd..387ab448b 100644 --- a/lib/crypto/algorithms/base.js +++ b/lib/crypto/algorithms/base.js @@ -105,11 +105,16 @@ EncryptionAlgorithm.prototype.onNewDevice = function(userId, deviceId) {}; * * @param {object} params parameters * @param {string} params.userId The UserID for the local user + * @param {module:crypto} params.crypto crypto core * @param {module:crypto/OlmDevice} params.olmDevice olm.js wrapper + * @param {string=} params.roomId The ID of the room we will be receiving + * from. Null for to-device events. */ var DecryptionAlgorithm = function(params) { this._userId = params.userId; + this._crypto = params.crypto; this._olmDevice = params.olmDevice; + this._roomId = params.roomId; }; /** */ module.exports.DecryptionAlgorithm = DecryptionAlgorithm; diff --git a/lib/crypto/index.js b/lib/crypto/index.js index 2f0c15cc8..c444e4246 100644 --- a/lib/crypto/index.js +++ b/lib/crypto/index.js @@ -57,7 +57,10 @@ function Crypto(baseApis, eventEmitter, sessionStore, userId, deviceId) { this._olmDevice = new OlmDevice(sessionStore); // EncryptionAlgorithm instance for each room - this._roomAlgorithms = {}; + this._roomEncryptors = {}; + + // map from algorithm to DecryptionAlgorithm instance, for each room + this._roomDecryptors = {}; this._supportedAlgorithms = utils.keys( algorithms.DECRYPTION_CLASSES @@ -705,7 +708,7 @@ Crypto.prototype.setRoomEncryption = function(roomId, config) { roomId: roomId, config: config, }); - this._roomAlgorithms[roomId] = alg; + this._roomEncryptors[roomId] = alg; }; @@ -839,7 +842,7 @@ Crypto.prototype.ensureOlmSessionsForUsers = function(users) { * @return {bool} whether encryption is enabled. */ Crypto.prototype.isRoomEncrypted = function(roomId) { - return Boolean(this._roomAlgorithms[roomId]); + return Boolean(this._roomEncryptors[roomId]); }; /** @@ -867,7 +870,7 @@ Crypto.prototype.encryptEventIfNeeded = function(event, room) { var roomId = event.getRoomId(); - var alg = this._roomAlgorithms[roomId]; + var alg = this._roomEncryptors[roomId]; if (!alg) { // not encrypting messages in this room @@ -922,14 +925,7 @@ Crypto.prototype.encryptEventIfNeeded = function(event, room) { */ Crypto.prototype.decryptEvent = function(event) { var content = event.content; - var AlgClass = algorithms.DECRYPTION_CLASSES[content.algorithm]; - if (!AlgClass) { - throw new algorithms.DecryptionError("Unable to decrypt " + content.algorithm); - } - var alg = new AlgClass({ - userId: this._userId, - olmDevice: this._olmDevice, - }); + var alg = this._getRoomDecryptor(event.room_id, content.algorithm); var r = alg.decryptEvent(event); if (r !== null) { @@ -1054,7 +1050,7 @@ Crypto.prototype._onInitialSyncCompleted = function(rooms) { var room = rooms[i]; // check for rooms with encryption enabled - var alg = this._roomAlgorithms[room.roomId]; + var alg = this._roomEncryptors[room.roomId]; if (!alg) { continue; } @@ -1108,16 +1104,13 @@ Crypto.prototype._onInitialSyncCompleted = function(rooms) { */ Crypto.prototype._onRoomKeyEvent = function(event) { var content = event.getContent(); - var AlgClass = algorithms.DECRYPTION_CLASSES[content.algorithm]; - if (!AlgClass) { - throw new algorithms.DecryptionError( - "Unable to handle keys for " + content.algorithm - ); + + if (!content.room_id || !content.algorithm) { + console.error("key event is missing fields"); + return; } - var alg = new AlgClass({ - userId: this._userId, - olmDevice: this._olmDevice, - }); + + var alg = this._getRoomDecryptor(content.room_id, content.algorithm); alg.onRoomKeyEvent(event); }; @@ -1141,7 +1134,7 @@ Crypto.prototype._onRoomMembership = function(event, member, oldMembership) { var roomId = member.roomId; - var alg = this._roomAlgorithms[roomId]; + var alg = this._roomEncryptors[roomId]; if (!alg) { // not encrypting in this room return; @@ -1177,7 +1170,7 @@ Crypto.prototype._onNewDeviceEvent = function(event) { ).then(function() { for (var i = 0; i < rooms.length; i++) { var roomId = rooms[i]; - var alg = self._roomAlgorithms[roomId]; + var alg = self._roomEncryptors[roomId]; if (!alg) { // not encrypting in this room continue; @@ -1193,6 +1186,58 @@ Crypto.prototype._onNewDeviceEvent = function(event) { }).done(); }; +/** + * Get a decryptor for a given room and algorithm. + * + * If we already have a decryptor for the given room and algorithm, return + * it. Otherwise try to instantiate it. + * + * @private + * + * @param {string?} roomId room id for decryptor. If undefined, a temporary + * decryptor is instantiated. + * + * @param {string} algorithm crypto algorithm + * + * @return {module:crypto.algorithms.base.DecryptionAlgorithm} + * + * @raises {module:crypto.algorithms.DecryptionError} if the algorithm is + * unknown + */ +Crypto.prototype._getRoomDecryptor = function(roomId, algorithm) { + var decryptors; + var alg; + + roomId = roomId || null; + if (roomId) { + decryptors = this._roomDecryptors[roomId]; + if (!decryptors) { + this._roomDecryptors[roomId] = decryptors = {}; + } + + alg = decryptors[algorithm]; + if (alg) { + return alg; + } + } + + var AlgClass = algorithms.DECRYPTION_CLASSES[algorithm]; + if (!AlgClass) { + throw new algorithms.DecryptionError("Unable to decrypt " + algorithm); + } + alg = new AlgClass({ + userId: this._userId, + crypto: this, + olmDevice: this._olmDevice, + roomId: roomId, + }); + + if (decryptors) { + decryptors[algorithm] = alg; + } + return alg; +}; + /** * sign the given object with our ed25519 key From 1a03e534bdafad1ce6f13ddcdb4064c31c5dde9e Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 11 Nov 2016 16:56:22 +0000 Subject: [PATCH 14/29] Refactor decryption Create the MatrixEvent wrapper before decryption, and then pass that into the decryptors, which should update it. Also remove the workaround that sends m.new_device messages when we get an unknown session; it's just a bandaid which is obscuring more meaningful problems. --- lib/client.js | 28 +++++++---------- lib/crypto/algorithms/megolm.js | 17 +++++------ lib/crypto/algorithms/olm.js | 35 +++++++++------------ lib/crypto/index.js | 54 +++------------------------------ lib/models/event.js | 44 ++++++++++++++++++--------- 5 files changed, 65 insertions(+), 113 deletions(-) diff --git a/lib/client.js b/lib/client.js index 77b477d3e..878f93b74 100644 --- a/lib/client.js +++ b/lib/client.js @@ -463,38 +463,31 @@ MatrixClient.prototype.isRoomEncrypted = function(roomId) { * Decrypt a received event according to the algorithm specified in the event. * * @param {MatrixClient} client - * @param {object} raw event - * - * @return {MatrixEvent} + * @param {MatrixEvent} event */ function _decryptEvent(client, event) { if (!client._crypto) { - return _badEncryptedMessage(event, "**Encryption not enabled**"); + _badEncryptedMessage(event, "**Encryption not enabled**"); + return; } - var decryptionResult; try { - decryptionResult = client._crypto.decryptEvent(event); + client._crypto.decryptEvent(event); } catch (e) { if (!(e instanceof Crypto.DecryptionError)) { throw e; } - return _badEncryptedMessage(event, "**" + e.message + "**"); + _badEncryptedMessage(event, "**" + e.message + "**"); + return; } - return new MatrixEvent( - event, decryptionResult.payload, - decryptionResult.keysProved, - decryptionResult.keysClaimed - ); } function _badEncryptedMessage(event, reason) { - return new MatrixEvent(event, { + event.setClearData({ type: "m.room.message", content: { msgtype: "m.bad.encrypted", body: reason, - content: event.content, }, }); } @@ -2829,10 +2822,11 @@ function _resolve(callback, defer, res) { function _PojoToMatrixEventMapper(client) { function mapper(plainOldJsObject) { - if (plainOldJsObject.type === "m.room.encrypted") { - return _decryptEvent(client, plainOldJsObject); + var event = new MatrixEvent(plainOldJsObject); + if (event.isEncrypted()) { + _decryptEvent(client, event); } - return new MatrixEvent(plainOldJsObject); + return event; } return mapper; } diff --git a/lib/crypto/algorithms/megolm.js b/lib/crypto/algorithms/megolm.js index 076158012..d750845ff 100644 --- a/lib/crypto/algorithms/megolm.js +++ b/lib/crypto/algorithms/megolm.js @@ -451,7 +451,7 @@ utils.inherits(MegolmDecryption, base.DecryptionAlgorithm); /** * @inheritdoc * - * @param {object} event raw event + * @param {MatrixEvent} event * * @return {null} The event referred to an unknown megolm session * @return {module:crypto.DecryptionResult} decryption result @@ -460,7 +460,7 @@ utils.inherits(MegolmDecryption, base.DecryptionAlgorithm); * problem decrypting the event */ MegolmDecryption.prototype.decryptEvent = function(event) { - var content = event.content; + var content = event.getWireContent(); if (!content.sender_key || !content.session_id || !content.ciphertext @@ -471,14 +471,15 @@ MegolmDecryption.prototype.decryptEvent = function(event) { var res; try { res = this._olmDevice.decryptGroupMessage( - event.room_id, content.sender_key, content.session_id, content.ciphertext + event.getRoomId(), content.sender_key, content.session_id, content.ciphertext ); } catch (e) { throw new base.DecryptionError(e); } if (res === null) { - return null; + // We've got a message for a session we don't have. + throw new base.DecryptionError("Unknown inbound session id"); } var payload = JSON.parse(res.result); @@ -486,17 +487,13 @@ MegolmDecryption.prototype.decryptEvent = function(event) { // 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) { + if (payload.room_id !== event.getRoomId()) { throw new base.DecryptionError( "Message intended for room " + payload.room_id ); } - return { - payload: payload, - keysClaimed: res.keysClaimed, - keysProved: res.keysProved, - }; + event.setClearData(payload, res.keysClaimed, res.keysProved); }; /** diff --git a/lib/crypto/algorithms/olm.js b/lib/crypto/algorithms/olm.js index 3b0b48bd8..84b64900d 100644 --- a/lib/crypto/algorithms/olm.js +++ b/lib/crypto/algorithms/olm.js @@ -151,15 +151,13 @@ utils.inherits(OlmDecryption, base.DecryptionAlgorithm); /** * @inheritdoc * - * @param {object} event raw event - * - * @return {module:crypto.DecryptionResult} decryption result + * @param {MatrixEvent} event * * @throws {module:crypto/algorithms/base.DecryptionError} if there is a * problem decrypting the event */ OlmDecryption.prototype.decryptEvent = function(event) { - var content = event.content; + var content = event.getWireContent(); var deviceKey = content.sender_key; var ciphertext = content.ciphertext; @@ -178,7 +176,7 @@ OlmDecryption.prototype.decryptEvent = function(event) { } catch (e) { console.warn( "Failed to decrypt Olm event (id=" + - event.event_id + ") from " + deviceKey + + event.getId() + ") from " + deviceKey + ": " + e.message ); throw new base.DecryptionError("Bad Encrypted Message"); @@ -192,11 +190,11 @@ OlmDecryption.prototype.decryptEvent = function(event) { // 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' " + + "Olm event (id=" + event.getId() + ") contains no 'recipient' " + "property; cannot prevent unknown-key attack"); } else if (payload.recipient != this._userId) { console.warn( - "Event " + event.event_id + ": Intended recipient " + + "Event " + event.getId() + ": Intended recipient " + payload.recipient + " does not match our id " + this._userId ); throw new base.DecryptionError( @@ -207,12 +205,12 @@ OlmDecryption.prototype.decryptEvent = function(event) { if (payload.recipient_keys === undefined) { // ditto console.warn( - "Olm event (id=" + event.event_id + ") contains no " + + "Olm event (id=" + event.getId() + ") 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 " + + "Event " + event.getId() + ": Intended recipient ed25519 key " + payload.recipient_keys.ed25519 + " did not match ours" ); throw new base.DecryptionError("Message not intended for this device"); @@ -225,12 +223,12 @@ OlmDecryption.prototype.decryptEvent = function(event) { if (payload.sender === undefined) { // ditto console.warn( - "Olm event (id=" + event.event_id + ") contains no " + + "Olm event (id=" + event.getId() + ") contains no " + "'sender' property; cannot prevent unknown-key attack"); - } else if (payload.sender != event.sender) { + } else if (payload.sender != event.getSender()) { console.warn( - "Event " + event.event_id + ": original sender " + payload.sender + - " does not match reported sender " + event.sender + "Event " + event.getId() + ": original sender " + payload.sender + + " does not match reported sender " + event.getSender() ); throw new base.DecryptionError( "Message forwarded from " + payload.sender @@ -238,9 +236,9 @@ OlmDecryption.prototype.decryptEvent = function(event) { } // Olm events intended for a room have a room_id. - if (payload.room_id !== event.room_id) { + if (payload.room_id !== event.getRoomId()) { console.warn( - "Event " + event.event_id + ": original room " + payload.room_id + + "Event " + event.getId() + ": original room " + payload.room_id + " does not match reported room " + event.room_id ); throw new base.DecryptionError( @@ -248,12 +246,7 @@ OlmDecryption.prototype.decryptEvent = function(event) { ); } - return { - payload: payload, - sessionExists: true, - keysProved: {curve25519: deviceKey}, - keysClaimed: payload.keys || {} - }; + event.setClearData(payload, {curve25519: deviceKey}, payload.keys || {}); }; diff --git a/lib/crypto/index.js b/lib/crypto/index.js index c444e4246..3d5ef4346 100644 --- a/lib/crypto/index.js +++ b/lib/crypto/index.js @@ -900,63 +900,17 @@ Crypto.prototype.encryptEventIfNeeded = function(event, room) { }); }; -/** - * @typedef {Object} module:crypto.DecryptionResult - * - * @property {Object} payload decrypted payload (with properties 'type', - * 'content'). - * - * @property {Object} keysClaimed keys that the sender of the - * event claims ownership of: map from key type to base64-encoded key - * - * @property {Object} keysProved keys that the sender of the - * event is known to have ownership of: map from key type to base64-encoded - * key - */ - /** * Decrypt a received event * - * @param {object} event raw event - * - * @return {module:crypto.DecryptionResult} decryption result + * @param {MatrixEvent} event * * @raises {algorithms.DecryptionError} if there is a problem decrypting the event */ Crypto.prototype.decryptEvent = function(event) { - var content = event.content; - var alg = this._getRoomDecryptor(event.room_id, content.algorithm); - var r = alg.decryptEvent(event); - - if (r !== null) { - return r; - } else { - // We've got a message for a session we don't have. Maybe the sender - // forgot to tell us about the session. Remind the sender that we - // exist so that they might tell us about the session on their next - // send. - // - // (Alternatively, it might be that we are just looking at - // scrollback... at least we rate-limit the m.new_device events :/) - // - // XXX: this is a band-aid which masks symptoms of other bugs. It would - // be nice to get rid of it. - if (event.room_id !== undefined && event.sender !== undefined) { - var device_id = event.content.device_id; - if (device_id === undefined) { - // if the sending device didn't tell us its device_id, fall - // back to all devices. - device_id = null; - } - console.log( - "Unknown session decrypting event id " + event.event_id + - ": sending m.new_device event" - ); - this._sendPingToDevice(event.sender, device_id, event.room_id); - } - - throw new algorithms.DecryptionError("Unknown inbound session id"); - } + var content = event.getWireContent(); + var alg = this._getRoomDecryptor(event.getRoomId(), content.algorithm); + alg.decryptEvent(event); }; /** diff --git a/lib/models/event.js b/lib/models/event.js index 31da3d72d..79eca1ad5 100644 --- a/lib/models/event.js +++ b/lib/models/event.js @@ -51,15 +51,6 @@ module.exports.EventStatus = { * * @param {Object} event The raw event to be wrapped in this DAO * - * @param {Object=} clearEvent For encrypted events, the plaintext payload for - * the event (typically containing type and content fields). - * - * @param {Object=} keysProved Keys owned by the sender of this event. - * See {@link module:models/event.MatrixEvent#getKeysProved}. - * - * @param {Object=} keysClaimed Keys the sender of this event claims. - * See {@link module:models/event.MatrixEvent#getKeysClaimed}. - * * @prop {Object} event The raw (possibly encrypted) event. Do not access * this property directly unless you absolutely have to. Prefer the getter * methods defined on this class. Using the getter methods shields your app @@ -75,19 +66,18 @@ module.exports.EventStatus = { * Default: true. This property is experimental and may change. */ module.exports.MatrixEvent = function MatrixEvent( - event, clearEvent, keysProved, keysClaimed + event ) { this.event = event || {}; this.sender = null; this.target = null; this.status = null; this.forwardLooking = true; - - this._clearEvent = clearEvent || {}; this._pushActions = null; - this._keysProved = keysProved || {}; - this._keysClaimed = keysClaimed || {}; + this._clearEvent = {}; + this._keysProved = {}; + this._keysClaimed = {}; }; module.exports.MatrixEvent.prototype = { @@ -239,12 +229,36 @@ module.exports.MatrixEvent.prototype = { this._keysClaimed = keys; }, + /** + * Update the cleartext data on this event. + * + * (This is used after decrypting an event; it should not be used by applications). + * + * @internal + * + * @fires module:models/event.MatrixEvent#"Event.decrypted" + * + * @param {Object} clearEvent The plaintext payload for the event + * (typically containing type and content fields). + * + * @param {Object=} keysProved Keys owned by the sender of this event. + * See {@link module:models/event.MatrixEvent#getKeysProved}. + * + * @param {Object=} keysClaimed Keys the sender of this event claims. + * See {@link module:models/event.MatrixEvent#getKeysClaimed}. + */ + setClearData: function(clearEvent, keysProved, keysClaimed) { + this._clearEvent = clearEvent; + this._keysProved = keysProved || {}; + this._keysClaimed = keysClaimed || {}; + }, + /** * Check if the event is encrypted. * @return {boolean} True if this event is encrypted. */ isEncrypted: function() { - return Boolean(this._clearEvent.type); + return this.event.type === "m.room.encrypted"; }, /** From a5d857945a9300c74e4397ee0fdbcc5770cd3b4b Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 14 Nov 2016 15:05:49 +0000 Subject: [PATCH 15/29] Retry decryption after receiving keys m.room_keys may arrive after the messages themselves, so allow events to be decrypted after the event (haha). --- lib/crypto/algorithms/megolm.js | 42 +++++++++++++++++++++++++++++++++ lib/models/event.js | 23 ++++++++++++++++-- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/lib/crypto/algorithms/megolm.js b/lib/crypto/algorithms/megolm.js index d750845ff..7e87ad37c 100644 --- a/lib/crypto/algorithms/megolm.js +++ b/lib/crypto/algorithms/megolm.js @@ -445,6 +445,10 @@ MegolmEncryption.prototype.onNewDevice = function(userId, deviceId) { */ function MegolmDecryption(params) { base.DecryptionAlgorithm.call(this, params); + + // events which we couldn't decrypt due to unknown sessions / indexes: map from + // senderKey|sessionId to list of MatrixEvents + this._pendingEvents = {}; } utils.inherits(MegolmDecryption, base.DecryptionAlgorithm); @@ -474,11 +478,15 @@ MegolmDecryption.prototype.decryptEvent = function(event) { event.getRoomId(), content.sender_key, content.session_id, content.ciphertext ); } catch (e) { + if (e.message === 'OLM.UNKNOWN_MESSAGE_INDEX') { + this._addEventToPendingList(event); + } throw new base.DecryptionError(e); } if (res === null) { // We've got a message for a session we don't have. + this._addEventToPendingList(event); throw new base.DecryptionError("Unknown inbound session id"); } @@ -496,6 +504,24 @@ MegolmDecryption.prototype.decryptEvent = function(event) { event.setClearData(payload, res.keysClaimed, res.keysProved); }; + +/** + * Add an event to the list of those we couldn't decrypt the first time we + * saw them. + * + * @private + * + * @param {module:models/event.MatrixEvent} event + */ +MegolmDecryption.prototype._addEventToPendingList = function(event) { + var content = event.getWireContent(); + var k = content.sender_key + "|" + content.session_id; + if (!this._pendingEvents[k]) { + this._pendingEvents[k] = []; + } + this._pendingEvents[k].push(event); +}; + /** * @inheritdoc * @@ -517,6 +543,22 @@ MegolmDecryption.prototype.onRoomKeyEvent = function(event) { content.room_id, event.getSenderKey(), content.session_id, content.session_key, event.getKeysClaimed() ); + + var k = event.getSenderKey() + "|" + content.session_id; + var pending = this._pendingEvents[k]; + if (pending) { + // have another go at decrypting events sent with this session. + delete this._pendingEvents[k]; + + for (var i = 0; i < pending.length; i++) { + try { + this.decryptEvent(pending[i]); + console.log("successful re-decryption of", pending[i]); + } catch (e) { + console.log("Still can't decrypt", pending[i], e.stack || e); + } + } + } }; base.registerAlgorithm( diff --git a/lib/models/event.js b/lib/models/event.js index 79eca1ad5..42d1b1299 100644 --- a/lib/models/event.js +++ b/lib/models/event.js @@ -21,6 +21,10 @@ limitations under the License. * @module models/event */ +var EventEmitter = require("events").EventEmitter; + +var utils = require('../utils.js'); + /** * Enum for event statuses. * @readonly @@ -79,8 +83,10 @@ module.exports.MatrixEvent = function MatrixEvent( this._keysProved = {}; this._keysClaimed = {}; }; +utils.inherits(module.exports.MatrixEvent, EventEmitter); -module.exports.MatrixEvent.prototype = { + +utils.extend(module.exports.MatrixEvent.prototype, { /** * Get the event_id for this event. @@ -251,6 +257,7 @@ module.exports.MatrixEvent.prototype = { this._clearEvent = clearEvent; this._keysProved = keysProved || {}; this._keysClaimed = keysClaimed || {}; + this.emit("Event.decrypted", this); }, /** @@ -367,7 +374,7 @@ module.exports.MatrixEvent.prototype = { setPushActions: function(pushActions) { this._pushActions = pushActions; }, -}; +}); /* http://matrix.org/docs/spec/r0.0.1/client_server.html#redactions says: @@ -408,3 +415,15 @@ var _REDACT_KEEP_CONTENT_MAP = { }, 'm.room.aliases': {'aliases': 1}, }; + + + + +/** + * Fires when an event is decrypted + * + * @event module:models/event.MatrixEvent#"Event.decrypted" + * + * @param {module:models/event.MatrixEvent} event + * The matrix event which has been decrypted + */ From d8c0b16d7ec5cffe17ad14e2197ce5aa91268e2c Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 15 Nov 2016 13:27:42 +0000 Subject: [PATCH 16/29] Make timeline-window _unpaginate public and remove _ --- lib/timeline-window.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/timeline-window.js b/lib/timeline-window.js index 1889631ae..68d50388b 100644 --- a/lib/timeline-window.js +++ b/lib/timeline-window.js @@ -234,7 +234,7 @@ TimelineWindow.prototype.paginate = function(direction, size, makeRequest, // remove some events from the other end, if necessary var excess = this._eventCount - this._windowLimit; if (excess > 0) { - this._unpaginate(excess, direction != EventTimeline.BACKWARDS); + this.unpaginate(excess, direction != EventTimeline.BACKWARDS); } return q(true); } @@ -292,10 +292,8 @@ TimelineWindow.prototype.paginate = function(direction, size, makeRequest, * @param {number} delta number of events to remove from the timeline * @param {boolean} startOfTimeline if events should be removed from the start * of the timeline. - * - * @private */ -TimelineWindow.prototype._unpaginate = function(delta, startOfTimeline) { +TimelineWindow.prototype.unpaginate = function(delta, startOfTimeline) { var tl = startOfTimeline ? this._start : this._end; // sanity-check the delta From 5b4aedd4be38d450fa321d93ca9f38664109d5a1 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 16 Nov 2016 09:22:31 +0000 Subject: [PATCH 17/29] Fix bug in verifying megolm event senders 1a03e534bda introduced a bug which mixed up the keys_proved and the keys_claimed. Switch them around again so that megolm messages are correctly tied back to the sending device. --- lib/crypto/algorithms/megolm.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/crypto/algorithms/megolm.js b/lib/crypto/algorithms/megolm.js index 7e87ad37c..1444f3b5e 100644 --- a/lib/crypto/algorithms/megolm.js +++ b/lib/crypto/algorithms/megolm.js @@ -501,7 +501,7 @@ MegolmDecryption.prototype.decryptEvent = function(event) { ); } - event.setClearData(payload, res.keysClaimed, res.keysProved); + event.setClearData(payload, res.keysProved, res.keysClaimed); }; From c0d862c9f0741fd8aa8fa27330d410eacb8a7d52 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Wed, 16 Nov 2016 11:06:56 +0000 Subject: [PATCH 18/29] Correct jsdoc for unpaginate --- lib/timeline-window.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/timeline-window.js b/lib/timeline-window.js index 68d50388b..742634313 100644 --- a/lib/timeline-window.js +++ b/lib/timeline-window.js @@ -287,7 +287,7 @@ TimelineWindow.prototype.paginate = function(direction, size, makeRequest, /** - * Trim the window to the windowlimit + * Remove `delta` events from the start or end of the timeline. * * @param {number} delta number of events to remove from the timeline * @param {boolean} startOfTimeline if events should be removed from the start From a0fd87c0320380de2cc11f6ab49b18d3da5e35b5 Mon Sep 17 00:00:00 2001 From: Alexander Date: Wed, 16 Nov 2016 09:26:16 -0500 Subject: [PATCH 19/29] Allows to start client with initialSyncLimit = 0 (see http://bit.ly/2f49Kbs) --- lib/sync.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/sync.js b/lib/sync.js index 80bf36aa3..7d4ab4d5a 100644 --- a/lib/sync.js +++ b/lib/sync.js @@ -60,7 +60,7 @@ function debuglog() { function SyncApi(client, opts) { this.client = client; opts = opts || {}; - opts.initialSyncLimit = opts.initialSyncLimit || 8; + opts.initialSyncLimit = (opts.initialSyncLimit === undefined ? 8 : opts.initialSyncLimit); opts.resolveInvitesToProfiles = opts.resolveInvitesToProfiles || false; opts.pollTimeout = opts.pollTimeout || (30 * 1000); opts.pendingEventOrdering = opts.pendingEventOrdering || "chronological"; From fc958a3922c529deed642713a6c7d50fea129eb2 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 16 Nov 2016 16:33:08 +0000 Subject: [PATCH 20/29] lint --- lib/sync.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/sync.js b/lib/sync.js index 7d4ab4d5a..160a8b26d 100644 --- a/lib/sync.js +++ b/lib/sync.js @@ -60,7 +60,9 @@ function debuglog() { function SyncApi(client, opts) { this.client = client; opts = opts || {}; - opts.initialSyncLimit = (opts.initialSyncLimit === undefined ? 8 : opts.initialSyncLimit); + opts.initialSyncLimit = ( + opts.initialSyncLimit === undefined ? 8 : opts.initialSyncLimit + ); opts.resolveInvitesToProfiles = opts.resolveInvitesToProfiles || false; opts.pollTimeout = opts.pollTimeout || (30 * 1000); opts.pendingEventOrdering = opts.pendingEventOrdering || "chronological"; From 851b33aac2d2fa265da4ff57171c989eba8de5a9 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 16 Nov 2016 18:05:41 +0000 Subject: [PATCH 21/29] distinguish unknown users from deviceless users Fixes https://github.com/vector-im/vector-web/issues/2275 --- lib/client.js | 2 +- lib/crypto/index.js | 44 +++-- spec/integ/matrix-client-crypto.spec.js | 21 +++ spec/integ/megolm.spec.js | 238 +++++++++++++++++++++--- spec/test-utils.js | 32 +--- 5 files changed, 265 insertions(+), 72 deletions(-) diff --git a/lib/client.js b/lib/client.js index 878f93b74..8ea538d69 100644 --- a/lib/client.js +++ b/lib/client.js @@ -351,7 +351,7 @@ MatrixClient.prototype.getStoredDevicesForUser = function(userId) { if (this._crypto === null) { throw new Error("End-to-end encryption disabled"); } - return this._crypto.getStoredDevicesForUser(userId); + return this._crypto.getStoredDevicesForUser(userId) || []; }; diff --git a/lib/crypto/index.js b/lib/crypto/index.js index 3d5ef4346..d8c52206f 100644 --- a/lib/crypto/index.js +++ b/lib/crypto/index.js @@ -281,13 +281,16 @@ Crypto.prototype.downloadKeys = function(userIds, forceDownload) { stored[userId] = {}; var devices = this.getStoredDevicesForUser(userId); - for (var j = 0; j < devices.length; ++j) { - var dev = devices[j]; - stored[userId][dev.deviceId] = dev; + + if (!devices || forceDownload) { + downloadUsers.push(userId); } - if (devices.length === 0 || forceDownload) { - downloadUsers.push(userId); + if (devices) { + for (var j = 0; j < devices.length; ++j) { + var dev = devices[j]; + stored[userId][dev.deviceId] = dev; + } } } @@ -298,22 +301,24 @@ Crypto.prototype.downloadKeys = function(userIds, forceDownload) { return this._baseApis.downloadKeysForUsers( downloadUsers ).then(function(res) { - for (var userId in res.device_keys) { - if (!stored.hasOwnProperty(userId)) { - // spurious result + var dk = res.device_keys || {}; + + for (var i = 0; i < downloadUsers.length; ++i) { + var userId = downloadUsers[i]; + // console.log('keys for ' + userId + ':', dk[userId]); + + if (!dk[userId]) { + // no result for this user + // TODO: do something with failures continue; } // map from deviceid -> deviceinfo for this user var userStore = stored[userId]; - var updated = _updateStoredDeviceKeysForUser( - self._olmDevice, userId, userStore, res.device_keys[userId] + _updateStoredDeviceKeysForUser( + self._olmDevice, userId, userStore, dk[userId] ); - if (!updated) { - continue; - } - // update the session store var storage = {}; for (var deviceId in userStore) { @@ -440,12 +445,13 @@ function _storeDeviceKeys(_olmDevice, userStore, deviceResult) { * * @param {string} userId the user to list keys for. * - * @return {module:crypto/deviceinfo[]} list of devices + * @return {module:crypto/deviceinfo[]?} list of devices, or null if we haven't + * managed to get a list of devices for this user yet. */ Crypto.prototype.getStoredDevicesForUser = function(userId) { var devs = this._sessionStore.getEndToEndDevicesForUser(userId); if (!devs) { - return []; + return null; } var res = []; for (var deviceId in devs) { @@ -468,7 +474,7 @@ Crypto.prototype.getStoredDevicesForUser = function(userId) { * "key", and "display_name" parameters. */ Crypto.prototype.listDeviceKeys = function(userId) { - var devices = this.getStoredDevicesForUser(userId); + var devices = this.getStoredDevicesForUser(userId) || []; var result = []; @@ -600,7 +606,7 @@ Crypto.prototype.setDeviceVerification = function(userId, deviceId, verified, bl * @return {Object.} */ Crypto.prototype.getOlmSessionsForUser = function(userId) { - var devices = this.getStoredDevicesForUser(userId); + var devices = this.getStoredDevicesForUser(userId) || []; var result = {}; for (var j = 0; j < devices.length; ++j) { var device = devices[j]; @@ -738,7 +744,7 @@ Crypto.prototype.ensureOlmSessionsForUsers = function(users) { var userId = users[i]; result[userId] = {}; - var devices = this.getStoredDevicesForUser(userId); + var devices = this.getStoredDevicesForUser(userId) || []; for (var j = 0; j < devices.length; ++j) { var deviceInfo = devices[j]; var deviceId = deviceInfo.deviceId; diff --git a/spec/integ/matrix-client-crypto.spec.js b/spec/integ/matrix-client-crypto.spec.js index 5066304a0..f326b52bf 100644 --- a/spec/integ/matrix-client-crypto.spec.js +++ b/spec/integ/matrix-client-crypto.spec.js @@ -483,6 +483,27 @@ describe("MatrixClient crypto", function() { bobClient.stopClient(); }); + it('Ali knows the difference between a new user and one with no devices', + function(done) { + aliHttpBackend.when('POST', '/keys/query').respond(200, { + device_keys: { + '@bob:id': {}, + } + }); + + var p1 = aliClient.downloadKeys(['@bob:id']); + var p2 = aliHttpBackend.flush('/keys/query', 1); + + q.all([p1, p2]).then(function() { + var devices = aliStorage.getEndToEndDevicesForUser('@bob:id'); + expect(utils.keys(devices).length).toEqual(0); + + // request again: should be no more requests + return aliClient.downloadKeys(['@bob:id']); + }).nodeify(done); + } + ); + it("Bob uploads without one-time keys and with one-time keys", function(done) { q() .then(bobUploadsKeys) diff --git a/spec/integ/megolm.spec.js b/spec/integ/megolm.spec.js index f30aa428e..6779ae675 100644 --- a/spec/integ/megolm.spec.js +++ b/spec/integ/megolm.spec.js @@ -21,6 +21,9 @@ try { var Olm = require('olm'); } catch (e) {} +var anotherjson = require('another-json'); +var q = require('q'); + var sdk = require('../..'); var utils = require('../../lib/utils'); var test_utils = require('../test-utils'); @@ -101,6 +104,16 @@ TestClient.prototype.getDeviceKey = function() { }; +/** + * get the uploaded ed25519 device key + * + * @return {string} base64 device key + */ +TestClient.prototype.getSigningKey = function() { + var key_id = 'ed25519:' + this.deviceId; + return this.deviceKeys.keys[key_id]; +}; + /** * start an Olm session with a given recipient * @@ -125,29 +138,39 @@ function createOlmSession(olmAccount, recipientTestClient) { * @param {object} opts * @param {string=} opts.sender * @param {string} opts.senderKey - * @param {string} opts.recipientKey * @param {Olm.Session} opts.p2pSession - * @param {object} opts.plaintext + * @param {TestClient} opts.recipient + * @param {object=} opts.plaincontent + * @param {string=} opts.plaintype * * @return {object} event */ function encryptOlmEvent(opts) { expect(opts.senderKey).toBeDefined(); expect(opts.p2pSession).toBeDefined(); - expect(opts.plaintext).toBeDefined(); - expect(opts.recipientKey).toBeDefined(); + expect(opts.recipient).toBeDefined(); + + var plaintext = { + content: opts.plaincontent || {}, + recipient: opts.recipient.userId, + recipient_keys: { + ed25519: opts.recipient.getSigningKey(), + }, + sender: opts.sender || '@bob:xyz', + type: opts.plaintype || 'm.test', + }; var event = { content: { - algorithm: "m.olm.v1.curve25519-aes-sha2", + algorithm: 'm.olm.v1.curve25519-aes-sha2', ciphertext: {}, sender_key: opts.senderKey, }, - sender: opts.sender || "@bob:xyz", - type: "m.room.encrypted", + sender: opts.sender || '@bob:xyz', + type: 'm.room.encrypted', }; - event.content.ciphertext[opts.recipientKey] = - opts.p2pSession.encrypt(JSON.stringify(opts.plaintext)); + event.content.ciphertext[opts.recipient.getDeviceKey()] = + opts.p2pSession.encrypt(JSON.stringify(plaintext)); return event; } @@ -198,7 +221,7 @@ function encryptMegolmEvent(opts) { * * @param {object} opts * @param {string} opts.senderKey - * @param {string} opts.recipientKey + * @param {TestClient} opts.recipient * @param {Olm.Session} opts.p2pSession * @param {Olm.OutboundGroupSession} opts.groupSession * @param {string=} opts.room_id @@ -208,17 +231,15 @@ function encryptMegolmEvent(opts) { function encryptGroupSessionKey(opts) { return encryptOlmEvent({ senderKey: opts.senderKey, - recipientKey: opts.recipientKey, + recipient: opts.recipient, p2pSession: opts.p2pSession, - plaintext: { - content: { - algorithm: "m.megolm.v1.aes-sha2", - room_id: opts.room_id, - session_id: opts.groupSession.session_id(), - session_key: opts.groupSession.session_key(), - }, - type: "m.room_key", + plaincontent: { + algorithm: 'm.megolm.v1.aes-sha2', + room_id: opts.room_id, + session_id: opts.groupSession.session_id(), + session_key: opts.groupSession.session_key(), }, + plaintype: 'm.room_key', }); } @@ -232,8 +253,9 @@ describe("megolm", function() { var testOlmAccount; var testSenderKey; var aliceTestClient; + var testDeviceKeys; - beforeEach(test_utils.asyncTest(function() { + beforeEach(function(done) { test_utils.beforeEach(this); aliceTestClient = new TestClient( @@ -245,14 +267,31 @@ describe("megolm", function() { var testE2eKeys = JSON.parse(testOlmAccount.identity_keys()); testSenderKey = testE2eKeys.curve25519; - return aliceTestClient.start(); - })); + testDeviceKeys = { + algorithms: ['m.olm.v1.curve25519-aes-sha2', 'm.megolm.v1.aes-sha2'], + device_id: 'DEVICE_ID', + keys: { + 'curve25519:DEVICE_ID': testE2eKeys.curve25519, + 'ed25519:DEVICE_ID': testE2eKeys.ed25519, + }, + user_id: '@bob:xyz', + }; + var j = anotherjson.stringify(testDeviceKeys); + var sig = testOlmAccount.sign(j); + testDeviceKeys.signatures = { + '@bob:xyz': { + 'ed25519:DEVICE_ID': sig, + }, + }; + + return aliceTestClient.start().nodeify(done); + }); afterEach(function() { aliceTestClient.stop(); }); - it("Alice receives a megolm message", test_utils.asyncTest(function() { + it("Alice receives a megolm message", function(done) { var p2pSession = createOlmSession(testOlmAccount, aliceTestClient); var groupSession = new Olm.OutboundGroupSession(); @@ -261,7 +300,7 @@ describe("megolm", function() { // make the room_key event var roomKeyEncrypted = encryptGroupSessionKey({ senderKey: testSenderKey, - recipientKey: aliceTestClient.getDeviceKey(), + recipient: aliceTestClient, p2pSession: p2pSession, groupSession: groupSession, room_id: ROOM_ID, @@ -294,10 +333,10 @@ describe("megolm", function() { var room = aliceTestClient.client.getRoom(ROOM_ID); var event = room.getLiveTimeline().getEvents()[0]; expect(event.getContent().body).toEqual('42'); - }); - })); + }).nodeify(done); + }); - it("Alice gets a second room_key message", test_utils.asyncTest(function() { + it("Alice gets a second room_key message", function(done) { var p2pSession = createOlmSession(testOlmAccount, aliceTestClient); var groupSession = new Olm.OutboundGroupSession(); @@ -306,7 +345,7 @@ describe("megolm", function() { // make the room_key event var roomKeyEncrypted1 = encryptGroupSessionKey({ senderKey: testSenderKey, - recipientKey: aliceTestClient.getDeviceKey(), + recipient: aliceTestClient, p2pSession: p2pSession, groupSession: groupSession, room_id: ROOM_ID, @@ -323,7 +362,7 @@ describe("megolm", function() { // session. var roomKeyEncrypted2 = encryptGroupSessionKey({ senderKey: testSenderKey, - recipientKey: aliceTestClient.getDeviceKey(), + recipient: aliceTestClient, p2pSession: p2pSession, groupSession: groupSession, room_id: ROOM_ID, @@ -360,7 +399,146 @@ describe("megolm", function() { var room = aliceTestClient.client.getRoom(ROOM_ID); var event = room.getLiveTimeline().getEvents()[0]; expect(event.getContent().body).toEqual('42'); + }).nodeify(done); + }); + + it('Alice sends a megolm message', function(done) { + // establish an olm session with alice + var p2pSession = createOlmSession(testOlmAccount, aliceTestClient); + + var olmEvent = encryptOlmEvent({ + senderKey: testSenderKey, + recipient: aliceTestClient, + p2pSession: p2pSession, }); - })); + var syncResponse = { + next_batch: 1, + to_device: { + events: [olmEvent], + }, + rooms: { + join: {}, + }, + }; + syncResponse.rooms.join[ROOM_ID] = { + state: { + events: [ + test_utils.mkEvent({ + type: 'm.room.encryption', + skey: '', + content: { + algorithm: 'm.megolm.v1.aes-sha2', + }, + }), + test_utils.mkMembership({ + mship: 'join', + sender: '@bob:xyz', + }), + ], + }, + }; + aliceTestClient.httpBackend.when('GET', '/sync').respond(200, syncResponse); + + var inboundGroupSession; + + return aliceTestClient.httpBackend.flush('/sync', 1).then(function() { + aliceTestClient.httpBackend.when('POST', '/keys/query').respond(200, { + device_keys: { + '@bob:xyz': { + 'DEVICE_ID': testDeviceKeys, + }, + } + }); + + aliceTestClient.httpBackend.when( + 'PUT', '/sendToDevice/m.room.encrypted/' + ).respond(200, function(path, content) { + var m = content.messages['@bob:xyz'].DEVICE_ID; + var ct = m.ciphertext[testSenderKey]; + var decrypted = JSON.parse(p2pSession.decrypt(ct.type, ct.body)); + + expect(decrypted.type).toEqual('m.room_key'); + inboundGroupSession = new Olm.InboundGroupSession(); + inboundGroupSession.create(decrypted.content.session_key); + return {}; + }); + + aliceTestClient.httpBackend.when( + 'PUT', '/send/' + ).respond(200, function(path, content) { + var ct = content.ciphertext; + var decrypted = JSON.parse(inboundGroupSession.decrypt(ct)); + + console.log('Decrypted received megolm message', decrypted); + expect(decrypted.type).toEqual('m.room.message'); + expect(decrypted.content.body).toEqual('test'); + + return { + event_id: '$event_id', + }; + }); + + return q.all([ + aliceTestClient.client.sendTextMessage(ROOM_ID, 'test'), + aliceTestClient.httpBackend.flush(), + ]); + }).nodeify(done); + }); + + it("Alice shouldn't do a second /query for non-e2e-capable devices", function(done) { + var syncResponse = { + next_batch: 1, + rooms: { + join: {}, + }, + }; + syncResponse.rooms.join[ROOM_ID] = { + state: { + events: [ + test_utils.mkEvent({ + type: 'm.room.encryption', + skey: '', + content: { + algorithm: 'm.megolm.v1.aes-sha2', + }, + }), + test_utils.mkMembership({ + mship: 'join', + sender: '@bob:xyz', + }), + ], + }, + }; + aliceTestClient.httpBackend.when('GET', '/sync').respond(200, syncResponse); + + return aliceTestClient.httpBackend.flush('/sync', 1).then(function() { + console.log("Forcing alice to download our device keys"); + + aliceTestClient.httpBackend.when('POST', '/keys/query').respond(200, { + device_keys: { + '@bob:xyz': {}, + } + }); + + return q.all([ + aliceTestClient.client.downloadKeys(['@bob:xyz']), + aliceTestClient.httpBackend.flush('/keys/query', 1), + ]); + }).then(function() { + console.log("Telling alice to send a megolm message"); + + aliceTestClient.httpBackend.when( + 'PUT', '/send/' + ).respond(200, { + event_id: '$event_id', + }); + + return q.all([ + aliceTestClient.client.sendTextMessage(ROOM_ID, 'test'), + aliceTestClient.httpBackend.flush(), + ]); + }).nodeify(done); + }); + }); diff --git a/spec/test-utils.js b/spec/test-utils.js index 109301e14..e03cf66c4 100644 --- a/spec/test-utils.js +++ b/spec/test-utils.js @@ -65,7 +65,7 @@ module.exports.mkEvent = function(opts) { content: opts.content, event_id: "$" + Math.random() + "-" + Math.random() }; - if (opts.skey) { + if (opts.skey !== undefined) { event.state_key = opts.skey; } else if (["m.room.name", "m.room.topic", "m.room.create", "m.room.join_rules", @@ -161,6 +161,15 @@ module.exports.mkMessage = function(opts) { * * @param {Error} err exception to be reported * + * @deprecated + * It turns out there are easier ways of doing this. Just use nodeify(): + * + * it("should not throw", function(done) { + * asynchronousMethod().then(function() { + * // some tests + * }).nodeify(done); + * }); + * * @example * it("should not throw", function(done) { * asynchronousMethod().then(function() { @@ -172,27 +181,6 @@ module.exports.failTest = function(err) { expect(true).toBe(false, "Testfunc threw: " + err.stack); }; -/** - * Wrap a test function which returns a promise into a format which - * jasmine will understand. - * - * @param {Function} testfunc test function, which should return a promise - * - * @return {Function} - * - * @example - * it("should not throw", asyncTest(function() { - * return asynchronousMethod().then(function() { - * // some tests - * }); - * })); - */ -module.exports.asyncTest = function(testfunc) { - return function(done) { - testfunc.call(this).catch(module.exports.failTest).done(done); - }; -}; - /** * A mock implementation of webstorage From 766e8377750fc563b0eb4ae2f216d7f6e3597d7c Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 10 Nov 2016 07:43:59 +0000 Subject: [PATCH 22/29] Factor out ensureOlmSessionsForDevices ... and move to olmlib --- lib/crypto/index.js | 129 +++------------------------------ lib/crypto/olmlib.js | 166 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 175 insertions(+), 120 deletions(-) diff --git a/lib/crypto/index.js b/lib/crypto/index.js index d8c52206f..4aa45217d 100644 --- a/lib/crypto/index.js +++ b/lib/crypto/index.js @@ -407,7 +407,7 @@ function _storeDeviceKeys(_olmDevice, userStore, deviceResult) { var unsigned = deviceResult.unsigned || {}; try { - _verifySignature(_olmDevice, deviceResult, userId, deviceId, signKey); + olmlib.verifySignature(_olmDevice, deviceResult, userId, deviceId, signKey); } catch (e) { console.log("Unable to verify signature on device " + userId + ":" + deviceId + ":", e); @@ -726,7 +726,8 @@ Crypto.prototype.setRoomEncryption = function(roomId, config) { */ /** - * Try to make sure we have established olm sessions for the given users. + * Try to make sure we have established olm sessions for all known devices for + * the given users. * * @param {string[]} users list of user ids * @@ -735,19 +736,15 @@ Crypto.prototype.setRoomEncryption = function(roomId, config) { * {@link module:crypto~OlmSessionResult} */ Crypto.prototype.ensureOlmSessionsForUsers = function(users) { - var devicesWithoutSession = [ - // [userId, deviceId, deviceInfo], ... - ]; - var result = {}; + var devicesByUser = {}; for (var i = 0; i < users.length; ++i) { var userId = users[i]; - result[userId] = {}; + devicesByUser[userId] = []; var devices = this.getStoredDevicesForUser(userId) || []; for (var j = 0; j < devices.length; ++j) { var deviceInfo = devices[j]; - var deviceId = deviceInfo.deviceId; var key = deviceInfo.getIdentityKey(); if (key == this._olmDevice.deviceCurve25519Key) { @@ -759,87 +756,13 @@ Crypto.prototype.ensureOlmSessionsForUsers = function(users) { continue; } - var sessionId = this._olmDevice.getSessionIdForDevice(key); - if (sessionId === null) { - devicesWithoutSession.push([userId, deviceId, deviceInfo]); - } - result[userId][deviceId] = { - device: deviceInfo, - sessionId: sessionId, - }; + devicesByUser[userId].push(deviceInfo); } } - if (devicesWithoutSession.length === 0) { - return q(result); - } - - // TODO: this has a race condition - if we try to send another message - // while we are claiming a key, we will end up claiming two and setting up - // two sessions. - // - // That should eventually resolve itself, but it's poor form. - - var self = this; - var oneTimeKeyAlgorithm = "signed_curve25519"; - return this._baseApis.claimOneTimeKeys( - devicesWithoutSession, oneTimeKeyAlgorithm - ).then(function(res) { - for (var i = 0; i < devicesWithoutSession.length; ++i) { - var device = devicesWithoutSession[i]; - var userId = device[0]; - var deviceId = device[1]; - var deviceInfo = device[2]; - - var userRes = res.one_time_keys[userId] || {}; - var deviceRes = userRes[deviceId]; - var oneTimeKey = null; - for (var 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; - } - - try { - _verifySignature( - self._olmDevice, oneTimeKey, userId, deviceId, - deviceInfo.getFingerprint() - ); - } catch (e) { - console.log( - "Unable to verify signature on one-time key for device " + - userId + ":" + deviceId + ":", e - ); - continue; - } - - var sid; - try { - sid = self._olmDevice.createOutboundSession( - deviceInfo.getIdentityKey(), oneTimeKey.key - ); - } catch (e) { - // possibly a bad key - console.error("Error starting session with device " + - userId + ":" + deviceId + ": " + e); - continue; - } - - console.log("Started new sessionid " + sid + - " for device " + userId + ":" + deviceId); - - result[userId][deviceId].sessionId = sid; - } - return result; - }); + return olmlib.ensureOlmSessionsForDevices( + this._olmDevice, this._baseApis, devicesByUser + ); }; /** @@ -1212,40 +1135,6 @@ Crypto.prototype._signObject = function(obj) { obj.signatures = sigs; }; -/** - * Verify the signature on an object - * - * @param {module:crypto/OlmDevice} olmDevice olm wrapper to use for verify op - * - * @param {Object} obj object to check signature on. Note that this will be - * stripped of its 'signatures' and 'unsigned' properties. - * - * @param {string} signingUserId ID of the user whose signature should be checked - * - * @param {string} signingDeviceId ID of the device whose signature should be checked - * - * @param {string} signingKey base64-ed ed25519 public key - */ -function _verifySignature(olmDevice, obj, signingUserId, signingDeviceId, signingKey) { - var signKeyId = "ed25519:" + signingDeviceId; - var signatures = obj.signatures || {}; - var userSigs = signatures[signingUserId] || {}; - var signature = userSigs[signKeyId]; - if (!signature) { - throw Error("No signature"); - } - - // prepare the canonical json: remove unsigned and signatures, and stringify with - // anotherjson - delete obj.unsigned; - delete obj.signatures; - var json = anotherjson.stringify(obj); - - olmDevice.verifySignature( - signingKey, json, signature - ); -} - /** * @see module:crypto/algorithms/base.DecryptionError */ diff --git a/lib/crypto/olmlib.js b/lib/crypto/olmlib.js index 4152755fb..341e0ecd7 100644 --- a/lib/crypto/olmlib.js +++ b/lib/crypto/olmlib.js @@ -20,6 +20,9 @@ limitations under the License. * Utilities common to olm encryption algorithms */ +var q = require('q'); +var anotherjson = require('another-json'); + var utils = require("../utils"); /** @@ -100,3 +103,166 @@ module.exports.encryptMessageForDevice = function( deviceKey, sessionId, JSON.stringify(payload) ); }; + +/** + * Try to make sure we have established olm sessions for the given devices. + * + * @param {module:crypto/OlmDevice} olmDevice + * + * @param {module:base-apis~MatrixBaseApis} baseApis + * + * @param {object} devicesByUser + * map from userid to list of devices + * + * @return {module:client.Promise} resolves once the sessions are complete, to + * an Object mapping from userId to deviceId to + * {@link module:crypto~OlmSessionResult} + */ +module.exports.ensureOlmSessionsForDevices = function( + olmDevice, baseApis, devicesByUser +) { + var devicesWithoutSession = [ + // [userId, deviceId], ... + ]; + var result = {}; + + for (var userId in devicesByUser) { + if (!devicesByUser.hasOwnProperty(userId)) { continue; } + result[userId] = {}; + var devices = devicesByUser[userId]; + for (var j = 0; j < devices.length; j++) { + var deviceInfo = devices[j]; + var deviceId = deviceInfo.deviceId; + var key = deviceInfo.getIdentityKey(); + var sessionId = olmDevice.getSessionIdForDevice(key); + if (sessionId === null) { + devicesWithoutSession.push([userId, deviceId]); + } + result[userId][deviceId] = { + device: deviceInfo, + sessionId: sessionId, + }; + } + } + + if (devicesWithoutSession.length === 0) { + return q(result); + } + + // TODO: this has a race condition - if we try to send another message + // while we are claiming a key, we will end up claiming two and setting up + // two sessions. + // + // That should eventually resolve itself, but it's poor form. + + var oneTimeKeyAlgorithm = "signed_curve25519"; + return baseApis.claimOneTimeKeys( + devicesWithoutSession, oneTimeKeyAlgorithm + ).then(function(res) { + for (var userId in devicesByUser) { + if (!devicesByUser.hasOwnProperty(userId)) { continue; } + var userRes = res.one_time_keys[userId] || {}; + var devices = devicesByUser[userId]; + for (var j = 0; j < devices.length; j++) { + var deviceInfo = devices[j]; + var deviceId = deviceInfo.deviceId; + if (result[userId][deviceId].sessionId) { + // we already have a result for this device + continue; + } + + var deviceRes = userRes[deviceId] || {}; + var oneTimeKey = null; + for (var 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; + } + + var sid = _verifyKeyAndStartSession( + olmDevice, oneTimeKey, userId, deviceInfo + ); + result[userId][deviceId].sessionId = sid; + } + } + return result; + }); +}; + + +function _verifyKeyAndStartSession(olmDevice, oneTimeKey, userId, deviceInfo) { + var deviceId = deviceInfo.deviceId; + try { + _verifySignature( + olmDevice, oneTimeKey, userId, deviceId, + deviceInfo.getFingerprint() + ); + } catch (e) { + console.error( + "Unable to verify signature on one-time key for device " + + userId + ":" + deviceId + ":", e + ); + return null; + } + + var sid; + try { + sid = olmDevice.createOutboundSession( + deviceInfo.getIdentityKey(), oneTimeKey.key + ); + } catch (e) { + // possibly a bad key + console.error("Error starting session with device " + + userId + ":" + deviceId + ": " + e); + return null; + } + + console.log("Started new sessionid " + sid + + " for device " + userId + ":" + deviceId); + return sid; +} + + +/** + * Verify the signature on an object + * + * @param {module:crypto/OlmDevice} olmDevice olm wrapper to use for verify op + * + * @param {Object} obj object to check signature on. Note that this will be + * stripped of its 'signatures' and 'unsigned' properties. + * + * @param {string} signingUserId ID of the user whose signature should be checked + * + * @param {string} signingDeviceId ID of the device whose signature should be checked + * + * @param {string} signingKey base64-ed ed25519 public key + */ +var _verifySignature = module.exports.verifySignature = function( + olmDevice, obj, signingUserId, signingDeviceId, signingKey +) { + var signKeyId = "ed25519:" + signingDeviceId; + var signatures = obj.signatures || {}; + var userSigs = signatures[signingUserId] || {}; + var signature = userSigs[signKeyId]; + if (!signature) { + throw Error("No signature"); + } + + // prepare the canonical json: remove unsigned and signatures, and stringify with + // anotherjson + delete obj.unsigned; + delete obj.signatures; + var json = anotherjson.stringify(obj); + + olmDevice.verifySignature( + signingKey, json, signature + ); +}; From 769a0cb76f93655ab2a5ea980cf62101fe8c682b Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 15 Nov 2016 21:50:25 +0000 Subject: [PATCH 23/29] Check devices to share keys with on each send Instead of trying to maintain a list of devices we need to share with, just check all the devices for all the users on each send. This should fix https://github.com/vector-im/vector-web/issues/2568, and generally mean we're less likely to get out of sync. --- lib/crypto/algorithms/megolm.js | 225 ++++++++++++++------------------ spec/integ/megolm.spec.js | 73 +++++++++++ 2 files changed, 171 insertions(+), 127 deletions(-) diff --git a/lib/crypto/algorithms/megolm.js b/lib/crypto/algorithms/megolm.js index 1444f3b5e..da4ee7c8b 100644 --- a/lib/crypto/algorithms/megolm.js +++ b/lib/crypto/algorithms/megolm.js @@ -38,12 +38,17 @@ var base = require("./base"); * @property {Number} creationTime when the session was created (ms since the epoch) * @property {module:client.Promise?} sharePromise If a share operation is in progress, * a promise which resolves when it is complete. + * + * @property {object} sharedWithDevices + * devices with which we have shared the session key + * userId -> {deviceId -> msgindex} */ function OutboundSessionInfo(sessionId) { this.sessionId = sessionId; this.useCount = 0; this.creationTime = new Date().getTime(); this.sharePromise = null; + this.sharedWithDevices = {}; } @@ -90,11 +95,6 @@ function MegolmEncryption(params) { // case _outboundSession.sharePromise will be non-null.) this._outboundSession = null; - // devices which have joined since we last sent a message. - // userId -> {deviceId -> true}, or - // userId -> true - this._devicesPendingKeyShare = {}; - // default rotation periods this._sessionRotationPeriodMsgs = 100; this._sessionRotationPeriodMs = 7 * 24 * 3600 * 1000; @@ -134,32 +134,55 @@ MegolmEncryption.prototype._ensureOutboundSession = function(room) { return session.sharePromise; } - // no share in progress: check for new devices - var shareMap = this._devicesPendingKeyShare; - this._devicesPendingKeyShare = {}; + // no share in progress: check if we need to share with any devices + var prom = this._getDevicesInRoom(room).then(function(devicesInRoom) { + var shareMap = {}; - // check each user is (still) a member of the room - for (var userId in shareMap) { - if (!shareMap.hasOwnProperty(userId)) { - continue; + for (var userId in devicesInRoom) { + if (!devicesInRoom.hasOwnProperty(userId)) { + continue; + } + + var userDevices = devicesInRoom[userId]; + + for (var deviceId in userDevices) { + if (!userDevices.hasOwnProperty(deviceId)) { + continue; + } + + var deviceInfo = userDevices[deviceId]; + + if (deviceInfo.isBlocked()) { + continue; + } + + var key = deviceInfo.getIdentityKey(); + if (key == self._olmDevice.deviceCurve25519Key) { + // don't bother sending to ourself + continue; + } + + if ( + !session.sharedWithDevices[userId] || + session.sharedWithDevices[userId][deviceId] === undefined + ) { + shareMap[userId] = shareMap[userId] || []; + shareMap[userId].push(deviceInfo); + } + } } - // XXX what about rooms where invitees can see the content? - var member = room.getMember(userId); - if (member.membership !== "join") { - delete shareMap[userId]; - } - } - - session.sharePromise = this._shareKeyWithDevices( - session.sessionId, shareMap - ).finally(function() { + return self._shareKeyWithDevices( + session, shareMap + ); + }).finally(function() { session.sharePromise = null; }).then(function() { return session; }); - return session.sharePromise; + session.sharePromise = prom; + return prom; }; /** @@ -178,95 +201,53 @@ MegolmEncryption.prototype._prepareNewSession = function(room) { key.key, {ed25519: this._olmDevice.deviceEd25519Key} ); - // we're going to share the key with all current members of the room, - // so we can reset this. - this._devicesPendingKeyShare = {}; - - var session = new OutboundSessionInfo(session_id); - - var roomMembers = utils.map(room.getJoinedMembers(), function(u) { - return u.userId; - }); - - var shareMap = {}; - for (var i = 0; i < roomMembers.length; i++) { - var userId = roomMembers[i]; - shareMap[userId] = true; - } - - var self = this; - - // TODO: we need to give the user a chance to block any devices or users - // before we send them the keys; it's too late to download them here. - session.sharePromise = this._crypto.downloadKeys( - roomMembers, false - ).then(function(res) { - return self._shareKeyWithDevices(session_id, shareMap); - }).then(function() { - return session; - }).finally(function() { - session.sharePromise = null; - }); - - return session; + return new OutboundSessionInfo(session_id); }; /** * @private * - * @param {string} session_id + * @param {module:crypto/algorithms/megolm.OutboundSessionInfo} session * - * @param {Object|boolean>} shareMap - * Map from userid to either: true (meaning this is a new user in the room, - * so all of his devices need the keys); or a map from deviceid to true - * (meaning this user has one or more new devices, which need the keys). + * @param {object} devicesByUser + * map from userid to list of devices * * @return {module:client.Promise} Promise which resolves once the key sharing * message has been sent. */ -MegolmEncryption.prototype._shareKeyWithDevices = function(session_id, shareMap) { +MegolmEncryption.prototype._shareKeyWithDevices = function(session, devicesByUser) { var self = this; - var key = this._olmDevice.getOutboundGroupSessionKey(session_id); + var key = this._olmDevice.getOutboundGroupSessionKey(session.sessionId); var payload = { type: "m.room_key", content: { algorithm: olmlib.MEGOLM_ALGORITHM, room_id: this._roomId, - session_id: session_id, + session_id: session.sessionId, session_key: key.key, chain_index: key.chain_index, } }; - // we downloaded the user's device list when they joined the room, or when - // the new device announced itself, so there is no need to do so now. + var contentMap = {}; - return self._crypto.ensureOlmSessionsForUsers( - utils.keys(shareMap) + return olmlib.ensureOlmSessionsForDevices( + this._olmDevice, this._baseApis, devicesByUser ).then(function(devicemap) { - var contentMap = {}; var haveTargets = false; - for (var userId in devicemap) { - if (!devicemap.hasOwnProperty(userId)) { + for (var userId in devicesByUser) { + if (!devicesByUser.hasOwnProperty(userId)) { continue; } - var devicesToShareWith = shareMap[userId]; + var devicesToShareWith = devicesByUser[userId]; var sessionResults = devicemap[userId]; - for (var deviceId in sessionResults) { - if (!sessionResults.hasOwnProperty(deviceId)) { - continue; - } - - if (devicesToShareWith === true) { - // all devices - } else if (!devicesToShareWith[deviceId]) { - // not a new device - continue; - } + for (var i = 0; i < devicesToShareWith.length; i++) { + var deviceInfo = devicesToShareWith[i]; + var deviceId = deviceInfo.deviceId; var sessionResult = sessionResults[deviceId]; if (!sessionResult.sessionId) { @@ -288,8 +269,6 @@ MegolmEncryption.prototype._shareKeyWithDevices = function(session_id, shareMap) "sharing keys with device " + userId + ":" + deviceId ); - var deviceInfo = sessionResult.device; - var encryptedContent = { algorithm: olmlib.OLM_ALGORITHM, sender_key: self._olmDevice.deviceCurve25519Key, @@ -321,6 +300,27 @@ MegolmEncryption.prototype._shareKeyWithDevices = function(session_id, shareMap) // TODO: retries return self._baseApis.sendToDevice("m.room.encrypted", contentMap); + }).then(function() { + // Add the devices we have shared with to session.sharedWithDevices. + // + // we deliberately iterate over devicesByUser (ie, the devices we + // attempted to share with) rather than the contentMap (those we did + // share with), because we don't want to try to claim a one-time-key + // for dead devices on every message. + for (var userId in devicesByUser) { + if (!devicesByUser.hasOwnProperty(userId)) { + continue; + } + if (!session.sharedWithDevices[userId]) { + session.sharedWithDevices[userId] = {}; + } + var devicesToShareWith = devicesByUser[userId]; + for (var i = 0; i < devicesToShareWith.length; i++) { + var deviceInfo = devicesToShareWith[i]; + session.sharedWithDevices[userId][deviceInfo.deviceId] = + key.chain_index; + } + } }); }; @@ -369,20 +369,9 @@ MegolmEncryption.prototype.encryptMessage = function(room, eventType, content) { * @param {string=} oldMembership previous membership */ MegolmEncryption.prototype.onRoomMembership = function(event, member, oldMembership) { - // if we haven't yet made a session, there's nothing to do here. - if (!this._outboundSession) { - return; - } - var newMembership = member.membership; - if (newMembership === 'join') { - this._onNewRoomMember(member.userId); - return; - } - - if (newMembership === 'invite' && oldMembership !== 'join') { - // we don't (yet) share keys with invited members, so nothing to do yet + if (newMembership === 'join' || newMembership === 'invite') { return; } @@ -396,44 +385,26 @@ MegolmEncryption.prototype.onRoomMembership = function(event, member, oldMembers }; /** - * handle a new user joining a room + * Get the list of devices for all users in the room * - * @param {string} userId new member - */ -MegolmEncryption.prototype._onNewRoomMember = function(userId) { - // make sure we have a list of this user's devices. We are happy to use a - // cached version here: we assume that if we already have a list of the - // user's devices, then we already share an e2e room with them, which means - // that they will have announced any new devices via an m.new_device. - this._crypto.downloadKeys([userId], false).done(); - - // also flag this user up for needing a keyshare. - this._devicesPendingKeyShare[userId] = true; -}; - - -/** - * @inheritdoc + * @param {module:models/room} room * - * @param {string} userId owner of the device - * @param {string} deviceId deviceId of the device + * @return {module:client.Promise} Promise which resolves to a map + * from userId to deviceId to deviceInfo */ -MegolmEncryption.prototype.onNewDevice = function(userId, deviceId) { - var d = this._devicesPendingKeyShare[userId]; +MegolmEncryption.prototype._getDevicesInRoom = function(room) { + // XXX what about rooms where invitees can see the content? + var roomMembers = utils.map(room.getJoinedMembers(), function(u) { + return u.userId; + }); - if (d === true) { - // we already want to share keys with all devices for this user - return; - } - - if (!d) { - this._devicesPendingKeyShare[userId] = d = {}; - } - - d[deviceId] = true; + // We are happy to use a cached version here: we assume that if we already + // have a list of the user's devices, then we already share an e2e room + // with them, which means that they will have announced any new devices via + // an m.new_device. + return this._crypto.downloadKeys(roomMembers, false); }; - /** * Megolm decryption implementation * diff --git a/spec/integ/megolm.spec.js b/spec/integ/megolm.spec.js index 6779ae675..086180fe7 100644 --- a/spec/integ/megolm.spec.js +++ b/spec/integ/megolm.spec.js @@ -541,4 +541,77 @@ describe("megolm", function() { }).nodeify(done); }); + + it("We shouldn't attempt to send to blocked devices", function(done) { + // establish an olm session with alice + var p2pSession = createOlmSession(testOlmAccount, aliceTestClient); + + var olmEvent = encryptOlmEvent({ + senderKey: testSenderKey, + recipient: aliceTestClient, + p2pSession: p2pSession, + }); + + var syncResponse = { + next_batch: 1, + to_device: { + events: [olmEvent], + }, + rooms: { + join: {}, + }, + }; + + syncResponse.rooms.join[ROOM_ID] = { + state: { + events: [ + test_utils.mkEvent({ + type: 'm.room.encryption', + skey: '', + content: { + algorithm: 'm.megolm.v1.aes-sha2', + }, + }), + test_utils.mkMembership({ + mship: 'join', + sender: '@bob:xyz', + }), + ], + }, + }; + aliceTestClient.httpBackend.when('GET', '/sync').respond(200, syncResponse); + + return aliceTestClient.httpBackend.flush('/sync', 1).then(function() { + console.log('Forcing alice to download our device keys'); + + aliceTestClient.httpBackend.when('POST', '/keys/query').respond(200, { + device_keys: { + '@bob:xyz': { + 'DEVICE_ID': testDeviceKeys, + }, + } + }); + + return q.all([ + aliceTestClient.client.downloadKeys(['@bob:xyz']), + aliceTestClient.httpBackend.flush('/keys/query', 1), + ]); + }).then(function() { + console.log('Telling alice to block our device'); + aliceTestClient.client.setDeviceBlocked('@bob:xyz', 'DEVICE_ID'); + + console.log('Telling alice to send a megolm message'); + aliceTestClient.httpBackend.when( + 'PUT', '/send/' + ).respond(200, { + event_id: '$event_id', + }); + + return q.all([ + aliceTestClient.client.sendTextMessage(ROOM_ID, 'test'), + aliceTestClient.httpBackend.flush(), + ]); + }).nodeify(done); + }); + }); From f6830992ea14115eb853724a7d192498a3c598e8 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 16 Nov 2016 22:39:08 +0000 Subject: [PATCH 24/29] Apply unknown-keyshare mitigations Now that the mobile clients have been updated to send the right fields, enforce their correctness on the recipient side. --- lib/crypto/algorithms/olm.js | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/lib/crypto/algorithms/olm.js b/lib/crypto/algorithms/olm.js index 84b64900d..5852aa82c 100644 --- a/lib/crypto/algorithms/olm.js +++ b/lib/crypto/algorithms/olm.js @@ -186,13 +186,7 @@ OlmDecryption.prototype.decryptEvent = function(event) { // 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.getId() + ") contains no 'recipient' " + - "property; cannot prevent unknown-key attack"); - } else if (payload.recipient != this._userId) { + if (payload.recipient != this._userId) { console.warn( "Event " + event.getId() + ": Intended recipient " + payload.recipient + " does not match our id " + this._userId @@ -202,12 +196,7 @@ OlmDecryption.prototype.decryptEvent = function(event) { ); } - if (payload.recipient_keys === undefined) { - // ditto - console.warn( - "Olm event (id=" + event.getId() + ") contains no " + - "'recipient_keys' property; cannot prevent unknown-key attack"); - } else if (payload.recipient_keys.ed25519 != + if (payload.recipient_keys.ed25519 != this._olmDevice.deviceEd25519Key) { console.warn( "Event " + event.getId() + ": Intended recipient ed25519 key " + @@ -220,12 +209,7 @@ OlmDecryption.prototype.decryptEvent = function(event) { // 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.getId() + ") contains no " + - "'sender' property; cannot prevent unknown-key attack"); - } else if (payload.sender != event.getSender()) { + if (payload.sender != event.getSender()) { console.warn( "Event " + event.getId() + ": original sender " + payload.sender + " does not match reported sender " + event.getSender() From 036d1da013447e67ae1415630c7ecb0221467c36 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 17 Nov 2016 15:31:23 +0000 Subject: [PATCH 25/29] Avoid a packetstorm of device queries on startup Two main changes here: * when we get an m.new_device event for a device we know about, ignore it * Batch up the m.new_device events received during initialsync and spam out all the queries at once. --- lib/crypto/algorithms/base.js | 9 -- lib/crypto/index.js | 192 ++++++++++++++++++----- spec/integ/matrix-client-crypto.spec.js | 27 ++++ spec/integ/matrix-client-methods.spec.js | 15 -- 4 files changed, 180 insertions(+), 63 deletions(-) diff --git a/lib/crypto/algorithms/base.js b/lib/crypto/algorithms/base.js index 387ab448b..afdc555b8 100644 --- a/lib/crypto/algorithms/base.js +++ b/lib/crypto/algorithms/base.js @@ -88,15 +88,6 @@ EncryptionAlgorithm.prototype.onRoomMembership = function( event, member, oldMembership ) {}; -/** - * Called when a new device announces itself in the room - * - * @param {string} userId owner of the device - * @param {string} deviceId deviceId of the device - */ -EncryptionAlgorithm.prototype.onNewDevice = function(userId, deviceId) {}; - - /** * base type for decryption implementations * diff --git a/lib/crypto/index.js b/lib/crypto/index.js index d8c52206f..0e769825e 100644 --- a/lib/crypto/index.js +++ b/lib/crypto/index.js @@ -54,6 +54,10 @@ function Crypto(baseApis, eventEmitter, sessionStore, userId, deviceId) { this._userId = userId; this._deviceId = deviceId; + this._initialSyncCompleted = false; + // userId -> deviceId -> true + this._pendingNewDevices = {}; + this._olmDevice = new OlmDevice(sessionStore); // EncryptionAlgorithm instance for each room @@ -272,24 +276,25 @@ Crypto.prototype.downloadKeys = function(userIds, forceDownload) { // map from userid -> deviceid -> DeviceInfo var stored = {}; + function storeDev(userId, dev) { + stored[userId][dev.deviceId] = dev; + } // list of userids we need to download keys for var downloadUsers = []; - for (var i = 0; i < userIds.length; ++i) { - var userId = userIds[i]; - stored[userId] = {}; + if (forceDownload) { + downloadUsers = userIds; + } else { + for (var i = 0; i < userIds.length; ++i) { + var userId = userIds[i]; + var devices = this.getStoredDevicesForUser(userId); - var devices = this.getStoredDevicesForUser(userId); - - if (!devices || forceDownload) { - downloadUsers.push(userId); - } - - if (devices) { - for (var j = 0; j < devices.length; ++j) { - var dev = devices[j]; - stored[userId][dev.deviceId] = dev; + if (!devices) { + downloadUsers.push(userId); + } else { + stored[userId] = {}; + devices.map(storeDev.bind(null, userId)); } } } @@ -298,30 +303,79 @@ Crypto.prototype.downloadKeys = function(userIds, forceDownload) { return q(stored); } - return this._baseApis.downloadKeysForUsers( + var r = this._doKeyDownloadForUsers(downloadUsers); + var promises = []; + downloadUsers.map(function(u) { + promises.push(r[u].catch(function(e) { + console.warn('Error downloading keys for user ' + u + ':', e); + }).then(function() { + stored[u] = {}; + var devices = self.getStoredDevicesForUser(u) || []; + devices.map(storeDev.bind(null, u)); + })); + }); + + return q.all(promises).then(function() { + return stored; + }); +}; + +/** + * @param {string[]} downloadUsers list of userIds + * + * @return {Object a map from userId to a promise for a result for that user + */ +Crypto.prototype._doKeyDownloadForUsers = function(downloadUsers) { + var self = this; + + console.log('Starting key download for ' + downloadUsers); + + var deferMap = {}; + var promiseMap = {}; + + downloadUsers.map(function(u) { + deferMap[u] = q.defer(); + promiseMap[u] = deferMap[u].promise; + }); + + this._baseApis.downloadKeysForUsers( downloadUsers - ).then(function(res) { + ).done(function(res) { var dk = res.device_keys || {}; for (var i = 0; i < downloadUsers.length; ++i) { var userId = downloadUsers[i]; - // console.log('keys for ' + userId + ':', dk[userId]); + var deviceId; + + console.log('got keys for ' + userId + ':', dk[userId]); if (!dk[userId]) { // no result for this user - // TODO: do something with failures + var err = 'Unknown'; + // TODO: do something with res.failures + deferMap[userId].reject(err); continue; } // map from deviceid -> deviceinfo for this user - var userStore = stored[userId]; + var userStore = {}; + var devs = self._sessionStore.getEndToEndDevicesForUser(userId); + if (devs) { + for (deviceId in devs) { + if (devs.hasOwnProperty(deviceId)) { + var d = DeviceInfo.fromStorage(devs[deviceId], deviceId); + userStore[deviceId] = d; + } + } + } + _updateStoredDeviceKeysForUser( self._olmDevice, userId, userStore, dk[userId] ); // update the session store var storage = {}; - for (var deviceId in userStore) { + for (deviceId in userStore) { if (!userStore.hasOwnProperty(deviceId)) { continue; } @@ -331,9 +385,16 @@ Crypto.prototype.downloadKeys = function(userIds, forceDownload) { self._sessionStore.storeEndToEndDevicesForUser( userId, storage ); + + deferMap[userId].resolve(); } - return stored; + }, function(err) { + downloadUsers.map(function(u) { + deferMap[u].reject(err); + }); }); + + return promiseMap; }; function _updateStoredDeviceKeysForUser(_olmDevice, userId, userStore, @@ -462,6 +523,22 @@ Crypto.prototype.getStoredDevicesForUser = function(userId) { return res; }; +/** + * Get the stored keys for a single device + * + * @param {string} userId + * @param {string} deviceId + * + * @return {module:crypto/deviceinfo?} list of devices, or undefined + * if we don't know about this device + */ +Crypto.prototype.getStoredDevice = function(userId, deviceId) { + var devs = this._sessionStore.getEndToEndDevicesForUser(userId); + if (!devs || !devs[deviceId]) { + return undefined; + } + return DeviceInfo.fromStorage(devs[deviceId], deviceId); +}; /** * List the stored device keys for a user id @@ -998,6 +1075,11 @@ Crypto.prototype._onCryptoEvent = function(event) { * @param {module:models/room[]} rooms list of rooms the client knows about */ Crypto.prototype._onInitialSyncCompleted = function(rooms) { + this._initialSyncCompleted = true; + + // catch up on any m.new_device events which arrived during the initial sync. + this._flushNewDeviceRequests(); + if (this._sessionStore.getDeviceAnnounced()) { return; } @@ -1124,26 +1206,58 @@ Crypto.prototype._onNewDeviceEvent = function(event) { console.log("m.new_device event from " + userId + ":" + deviceId + " for rooms " + rooms); + if (this.getStoredDevice(userId, deviceId)) { + console.log("Known device; ignoring"); + return; + } + + this._pendingNewDevices[userId] = this._pendingNewDevices[userId] || {}; + this._pendingNewDevices[userId][deviceId] = true; + + // we delay handling these until the intialsync has completed, so that we + // can do all of them together. + if (this._initialSyncCompleted) { + this._flushNewDeviceRequests(); + } +}; + +/** + * Start device queries for any users who sent us an m.new_device recently + */ +Crypto.prototype._flushNewDeviceRequests = function() { var self = this; - this.downloadKeys( - [userId], true - ).then(function() { - for (var i = 0; i < rooms.length; i++) { - var roomId = rooms[i]; - var alg = self._roomEncryptors[roomId]; - if (!alg) { - // not encrypting in this room - continue; - } - alg.onNewDevice(userId, deviceId); - } - }).catch(function(e) { - console.error( - "Error updating device keys for new device " + userId + ":" + - deviceId, - e - ); - }).done(); + + var pending = this._pendingNewDevices; + var users = utils.keys(pending).filter(function(u) { + return utils.keys(pending[u]).length > 0; + }); + + if (users.length === 0) { + return; + } + + var r = this._doKeyDownloadForUsers(users); + + // we've kicked off requests to these users: remove their + // pending flag for now. + this._pendingNewDevices = {}; + + users.map(function(u) { + r[u] = r[u].catch(function(e) { + console.error( + 'Error updating device keys for user ' + u + ':', e + ); + + // reinstate the pending flags on any users which failed; this will + // mean that we will do another download in the future, but won't + // tight-loop. + // + self._pendingNewDevices[u] = self._pendingNewDevices[u] || {}; + utils.update(self._pendingNewDevices[u], pending[u]); + }); + }); + + q.all(utils.values(r)).done(); }; /** diff --git a/spec/integ/matrix-client-crypto.spec.js b/spec/integ/matrix-client-crypto.spec.js index f326b52bf..b563a08c8 100644 --- a/spec/integ/matrix-client-crypto.spec.js +++ b/spec/integ/matrix-client-crypto.spec.js @@ -739,4 +739,31 @@ describe("MatrixClient crypto", function() { }).then(aliRecvMessage) .catch(test_utils.failTest).done(done); }); + + + it("Ali does a key query when she gets a new_device event", function(done) { + q() + .then(bobUploadsKeys) + .then(aliStartClient) + .then(function() { + var syncData = { + next_batch: '2', + to_device: { + events: [ + test_utils.mkEvent({ + content: { + device_id: 'TEST_DEVICE', + rooms: [], + }, + sender: bobUserId, + type: 'm.new_device', + }), + ], + }, + }; + aliHttpBackend.when('GET', '/sync').respond(200, syncData); + return aliHttpBackend.flush('/sync', 1); + }).then(expectAliQueryKeys) + .nodeify(done); + }); }); diff --git a/spec/integ/matrix-client-methods.spec.js b/spec/integ/matrix-client-methods.spec.js index 7a853f4e7..68d23ccd3 100644 --- a/spec/integ/matrix-client-methods.spec.js +++ b/spec/integ/matrix-client-methods.spec.js @@ -371,21 +371,6 @@ describe("MatrixClient", function() { httpBackend.flush(); }); - - it("should return a rejected promise if the request fails", function(done) { - httpBackend.when("POST", "/keys/query").respond(400); - - var exceptionThrown; - client.downloadKeys(["bottom"]).then(function() { - fail("download didn't fail"); - }, function(err) { - exceptionThrown = err; - }).then(function() { - expect(exceptionThrown).toBeTruthy(); - }).catch(utils.failTest).done(done); - - httpBackend.flush(); - }); }); describe("deleteDevice", function() { From 579218aafcca97ae9522253494dc0711981397d2 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sat, 19 Nov 2016 01:25:21 +0200 Subject: [PATCH 26/29] correctly crash out if jq or hub are missing --- release.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/release.sh b/release.sh index 1a56c853f..d24574702 100755 --- a/release.sh +++ b/release.sh @@ -10,8 +10,8 @@ set -e -jq --version > /dev/null || (echo "jq is required: please install it"; exit) -hub --version > /dev/null || (echo "hub is required: please install it"; exit) +jq --version > /dev/null || (echo "jq is required: please install it"; kill $$) +hub --version > /dev/null || (echo "hub is required: please install it"; kill $$) USAGE="$0 [-xz] [-c changelog_file] vX.Y.Z" From d8de23228fcc28588b8ea872e21af026bfc98f52 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sat, 19 Nov 2016 01:51:10 +0200 Subject: [PATCH 27/29] actually speak valid git in release.sh --- release.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release.sh b/release.sh index d24574702..a8f7a818b 100755 --- a/release.sh +++ b/release.sh @@ -140,7 +140,7 @@ if [ $dodist -eq 0 ]; then echo "Building distribution copy in $builddir" pushd "$builddir" git clone "$projdir" . - git co "$rel_branch" + git checkout "$rel_branch" npm install # We haven't tagged yet, so tell the dist script what version # it's building From 1f33d76e875dca357fa22928492a96efb192853e Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sat, 19 Nov 2016 01:52:51 +0200 Subject: [PATCH 28/29] Prepare changelog for v0.7.0 --- CHANGELOG.md | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 831a73f1c..c76d92a38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,38 @@ +Changes in [0.7.0](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v0.7.0) (2016-11-18) +================================================================================================ +[Full Changelog](https://github.com/matrix-org/matrix-js-sdk/compare/v0.6.4...v0.7.0) + + * Avoid a packetstorm of device queries on startup + [\#297](https://github.com/matrix-org/matrix-js-sdk/pull/297) + * E2E: Check devices to share keys with on each send + [\#295](https://github.com/matrix-org/matrix-js-sdk/pull/295) + * Apply unknown-keyshare mitigations + [\#296](https://github.com/matrix-org/matrix-js-sdk/pull/296) + * distinguish unknown users from deviceless users + [\#294](https://github.com/matrix-org/matrix-js-sdk/pull/294) + * Allow starting client with initialSyncLimit = 0 + [\#293](https://github.com/matrix-org/matrix-js-sdk/pull/293) + * Make timeline-window _unpaginate public and rename to unpaginate + [\#289](https://github.com/matrix-org/matrix-js-sdk/pull/289) + * Send a STOPPED sync updated after call to stopClient + [\#286](https://github.com/matrix-org/matrix-js-sdk/pull/286) + * Fix bug in verifying megolm event senders + [\#292](https://github.com/matrix-org/matrix-js-sdk/pull/292) + * Handle decryption of events after they arrive + [\#288](https://github.com/matrix-org/matrix-js-sdk/pull/288) + * Fix examples. + [\#287](https://github.com/matrix-org/matrix-js-sdk/pull/287) + * Add a travis.yml + [\#278](https://github.com/matrix-org/matrix-js-sdk/pull/278) + * Encrypt all events, including 'm.call.*' + [\#277](https://github.com/matrix-org/matrix-js-sdk/pull/277) + * Ignore reshares of known megolm sessions + [\#276](https://github.com/matrix-org/matrix-js-sdk/pull/276) + * Log to the console on unknown session + [\#274](https://github.com/matrix-org/matrix-js-sdk/pull/274) + * Make it easier for SDK users to wrap prevailing the 'request' function + [\#273](https://github.com/matrix-org/matrix-js-sdk/pull/273) + Changes in [0.6.4](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v0.6.4) (2016-11-04) ================================================================================================ [Full Changelog](https://github.com/matrix-org/matrix-js-sdk/compare/v0.6.4-rc.2...v0.6.4) From 889bfce65d1cf84dfc55b93188ce3c7fa17ca214 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sat, 19 Nov 2016 01:52:52 +0200 Subject: [PATCH 29/29] v0.7.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 162e43d00..c526c95a6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "matrix-js-sdk", - "version": "0.6.4", + "version": "0.7.0", "description": "Matrix Client-Server SDK for Javascript", "main": "index.js", "scripts": {