From 98d606fca483255b597b4c1cf9039cf88c64bb54 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 20 Feb 2017 16:26:24 +0000 Subject: [PATCH] Upload one-time keys on /sync rather than a timer Delay the upload of one-time keys until we have received a sync *without any to-device messages*. Doing so means that we can try to avoid throwing away our private keys just before we receive the to-device messages which use them. Once we've decided to go ahead and upload them, we keep uploading them in batches of 5 until we get to the desired 50 keys on the server. We then periodically check that there are still enough on the server. --- spec/TestClient.js | 71 ++++++++-- spec/integ/matrix-client-crypto.spec.js | 167 +++++++++++++----------- spec/integ/megolm.spec.js | 79 +++++------ src/client.js | 18 +-- src/crypto/index.js | 160 +++++++++++++++-------- 5 files changed, 300 insertions(+), 195 deletions(-) diff --git a/spec/TestClient.js b/spec/TestClient.js index f7a584eb4..ceee409fa 100644 --- a/spec/TestClient.js +++ b/spec/TestClient.js @@ -21,6 +21,7 @@ import sdk from '..'; import testUtils from './test-utils'; import MockHttpBackend from './mock-request'; import expect from 'expect'; +import q from 'q'; /** * Wrapper for a MockStorageApi, MockHttpBackend and MatrixClient @@ -49,6 +50,10 @@ export default function TestClient(userId, deviceId, accessToken) { this.oneTimeKeys = {}; } +TestClient.prototype.toString = function() { + return 'TestClient[' + this.userId + ']'; +}; + /** * start the client, and wait for it to initialise. * @@ -57,7 +62,11 @@ export default function TestClient(userId, deviceId, accessToken) { TestClient.prototype.start = function() { this.httpBackend.when("GET", "/pushrules").respond(200, {}); this.httpBackend.when("POST", "/filter").respond(200, { filter_id: "fid" }); - this.expectKeyUpload(); + this.expectDeviceKeyUpload(); + + // we let the client do a very basic initial sync, which it needs before + // it will upload one-time keys. + this.httpBackend.when("GET", "/sync").respond(200, { next_batch: 1 }); this.client.startClient({ // set this so that we can get hold of failed events @@ -65,7 +74,7 @@ TestClient.prototype.start = function() { }); return this.httpBackend.flush().then(() => { - console.log('TestClient[' + this.userId + ']: started'); + console.log(this + ': started'); }); }; @@ -77,24 +86,62 @@ TestClient.prototype.stop = function() { }; /** - * Set up expectations that the client will upload device and one-time keys. + * Set up expectations that the client will upload device keys. */ -TestClient.prototype.expectKeyUpload = function() { +TestClient.prototype.expectDeviceKeyUpload = function() { const self = this; this.httpBackend.when("POST", "/keys/upload").respond(200, function(path, content) { expect(content.one_time_keys).toBe(undefined); expect(content.device_keys).toBeTruthy(); + + console.log(self + ': received device keys'); + // we expect this to happen before any one-time keys are uploaded. + expect(Object.keys(self.oneTimeKeys).length).toEqual(0); + 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).toBe(undefined); - expect(content.one_time_keys).toBeTruthy(); - expect(content.one_time_keys).toNotEqual({}); - self.oneTimeKeys = content.one_time_keys; - return {one_time_key_counts: { - signed_curve25519: Object.keys(self.oneTimeKeys).length, - }}; +}; + + +/** + * If one-time keys have already been uploaded, return them. Otherwise, + * set up an expectation that the keys will be uploaded, and wait for + * that to happen. + * + * @returns {Promise} for the one-time keys + */ +TestClient.prototype.awaitOneTimeKeyUpload = function() { + if (Object.keys(this.oneTimeKeys).length != 0) { + // already got one-time keys + return q(this.oneTimeKeys); + } + + this.httpBackend.when("POST", "/keys/upload") + .respond(200, (path, content) => { + expect(content.device_keys).toBe(undefined); + expect(content.one_time_keys).toBe(undefined); + return {one_time_key_counts: { + signed_curve25519: Object.keys(this.oneTimeKeys).length, + }}; + }); + + this.httpBackend.when("POST", "/keys/upload") + .respond(200, (path, content) => { + expect(content.device_keys).toBe(undefined); + expect(content.one_time_keys).toBeTruthy(); + expect(content.one_time_keys).toNotEqual({}); + console.log('%s: received %i one-time keys', this, + Object.keys(content.one_time_keys).length); + this.oneTimeKeys = content.one_time_keys; + return {one_time_key_counts: { + signed_curve25519: Object.keys(this.oneTimeKeys).length, + }}; + }); + + return this.httpBackend.flush('/keys/upload', 2).then((flushed) => { + expect(flushed).toEqual(2); + return this.oneTimeKeys; }); }; diff --git a/spec/integ/matrix-client-crypto.spec.js b/spec/integ/matrix-client-crypto.spec.js index ed073a4af..d3e809348 100644 --- a/spec/integ/matrix-client-crypto.spec.js +++ b/spec/integ/matrix-client-crypto.spec.js @@ -44,15 +44,13 @@ const bobAccessToken = "fewgfkuesa"; let aliMessages; let bobMessages; - -function bobUploadsKeys() { - bobTestClient.expectKeyUpload(); +function bobUploadsDeviceKeys() { + bobTestClient.expectDeviceKeyUpload(); return q.all([ - bobTestClient.client.uploadKeys(5), + bobTestClient.client.uploadKeys(), bobTestClient.httpBackend.flush(), ]).then(() => { - expect(Object.keys(bobTestClient.oneTimeKeys).length).toEqual(5); - expect(bobTestClient.deviceKeys).toNotEqual({}); + expect(Object.keys(bobTestClient.deviceKeys).length).toNotEqual(0); }); } @@ -107,34 +105,33 @@ function expectBobQueryKeys() { * @return {promise} resolves once the http request has completed. */ function expectAliClaimKeys() { - // can't query keys before bob has uploaded them - expect(bobTestClient.oneTimeKeys).toNotEqual({}); - - aliTestClient.httpBackend.when( - "POST", "/keys/claim", - ).respond(200, function(path, content) { - const claimType = content.one_time_keys[bobUserId][bobDeviceId]; - expect(claimType).toEqual("signed_curve25519"); - let keyId = null; - for (keyId in bobTestClient.oneTimeKeys) { - if (bobTestClient.oneTimeKeys.hasOwnProperty(keyId)) { - if (keyId.indexOf(claimType + ":") === 0) { - break; + return bobTestClient.awaitOneTimeKeyUpload().then((keys) => { + aliTestClient.httpBackend.when( + "POST", "/keys/claim", + ).respond(200, function(path, content) { + const claimType = content.one_time_keys[bobUserId][bobDeviceId]; + expect(claimType).toEqual("signed_curve25519"); + let keyId = null; + for (keyId in keys) { + if (bobTestClient.oneTimeKeys.hasOwnProperty(keyId)) { + if (keyId.indexOf(claimType + ":") === 0) { + break; + } } } - } - const result = {}; - result[bobUserId] = {}; - result[bobUserId][bobDeviceId] = {}; - result[bobUserId][bobDeviceId][keyId] = bobTestClient.oneTimeKeys[keyId]; - return {one_time_keys: result}; - }); - - // it can take a while to process the key query, so give it some extra - // time, and make sure the claim actually happens rather than ploughing on - // confusingly. - return aliTestClient.httpBackend.flush("/keys/claim", 1, 20).then((r) => { - expect(r).toEqual(1); + const result = {}; + result[bobUserId] = {}; + result[bobUserId][bobDeviceId] = {}; + result[bobUserId][bobDeviceId][keyId] = keys[keyId]; + return {one_time_keys: result}; + }); + }).then(() => { + // it can take a while to process the key query, so give it some extra + // time, and make sure the claim actually happens rather than ploughing on + // confusingly. + return aliTestClient.httpBackend.flush("/keys/claim", 1, 20).then((r) => { + expect(r).toEqual(1); + }); }); } @@ -345,13 +342,13 @@ function recvMessage(httpBackend, client, sender, message) { /** - * Set http responses for the requests which are made when a client starts, and - * start the client. + * Send an initial sync response to the client (which just includes the member + * list for our test room). * * @param {TestClient} testClient - * @returns {Promise} which resolves when the client has done its initial requests + * @returns {Promise} which resolves when the sync has been flushed. */ -function startClient(testClient) { +function firstSync(testClient) { // send a sync response including our test room. const syncData = { next_batch: "x", @@ -377,7 +374,7 @@ function startClient(testClient) { }, }; testClient.httpBackend.when("GET", "/sync").respond(200, syncData); - return testClient.start(); + return testClient.httpBackend.flush("/sync", 1); } @@ -426,22 +423,21 @@ describe("MatrixClient crypto", function() { }, ); - it("Bob uploads without one-time keys and with one-time keys", function(done) { - q() - .then(bobUploadsKeys) - .catch(testUtils.failTest).done(done); + it("Bob uploads device keys", function() { + return q() + .then(bobUploadsDeviceKeys); }); - it("Ali downloads Bobs keys", function(done) { + it("Ali downloads Bobs device keys", function(done) { q() - .then(bobUploadsKeys) + .then(bobUploadsDeviceKeys) .then(aliDownloadsKeys) .catch(testUtils.failTest).done(done); }); it("Ali gets keys with an invalid signature", function(done) { q() - .then(bobUploadsKeys) + .then(bobUploadsDeviceKeys) .then(function() { // tamper bob's keys const bobDeviceKeys = bobTestClient.deviceKeys; @@ -533,18 +529,22 @@ describe("MatrixClient crypto", function() { }).catch(testUtils.failTest).done(done); }); - it("Ali enables encryption", function(done) { - q() - .then(bobUploadsKeys) - .then(() => startClient(aliTestClient)) - .then(aliEnablesEncryption) - .catch(testUtils.failTest).done(done); + + it("Bob starts his client and uploads device keys and one-time keys", function() { + return q() + .then(() => bobTestClient.start()) + .then(() => bobTestClient.awaitOneTimeKeyUpload()) + .then((keys) => { + expect(Object.keys(keys).length).toEqual(5); + expect(Object.keys(bobTestClient.deviceKeys).length).toNotEqual(0); + }); }); it("Ali sends a message", function(done) { q() - .then(bobUploadsKeys) - .then(() => startClient(aliTestClient)) + .then(() => aliTestClient.start()) + .then(() => bobTestClient.start()) + .then(() => firstSync(aliTestClient)) .then(aliEnablesEncryption) .then(aliSendsFirstMessage) .catch(testUtils.failTest).nodeify(done); @@ -552,22 +552,22 @@ describe("MatrixClient crypto", function() { it("Bob receives a message", function(done) { q() - .then(bobUploadsKeys) - .then(() => startClient(aliTestClient)) + .then(() => aliTestClient.start()) + .then(() => bobTestClient.start()) + .then(() => firstSync(aliTestClient)) .then(aliEnablesEncryption) .then(aliSendsFirstMessage) - .then(() => startClient(bobTestClient)) .then(bobRecvMessage) .catch(testUtils.failTest).done(done); }); it("Bob receives a message with a bogus sender", function(done) { q() - .then(bobUploadsKeys) - .then(() => startClient(aliTestClient)) + .then(() => aliTestClient.start()) + .then(() => bobTestClient.start()) + .then(() => firstSync(aliTestClient)) .then(aliEnablesEncryption) .then(aliSendsFirstMessage) - .then(() => startClient(bobTestClient)) .then(function() { const message = aliMessages.shift(); const syncData = { @@ -620,8 +620,9 @@ describe("MatrixClient crypto", function() { it("Ali blocks Bob's device", function(done) { q() - .then(bobUploadsKeys) - .then(() => startClient(aliTestClient)) + .then(() => aliTestClient.start()) + .then(() => bobTestClient.start()) + .then(() => firstSync(aliTestClient)) .then(aliEnablesEncryption) .then(aliDownloadsKeys) .then(function() { @@ -638,36 +639,37 @@ describe("MatrixClient crypto", function() { it("Bob receives two pre-key messages", function(done) { q() - .then(bobUploadsKeys) - .then(() => startClient(aliTestClient)) + .then(() => aliTestClient.start()) + .then(() => bobTestClient.start()) + .then(() => firstSync(aliTestClient)) .then(aliEnablesEncryption) .then(aliSendsFirstMessage) - .then(() => startClient(bobTestClient)) .then(bobRecvMessage) .then(aliSendsMessage) .then(bobRecvMessage) .catch(testUtils.failTest).done(done); }); - it("Bob replies to the message", function(done) { - q() - .then(() => startClient(aliTestClient)) - .then(() => startClient(bobTestClient)) + it("Bob replies to the message", function() { + return q() + .then(() => aliTestClient.start()) + .then(() => bobTestClient.start()) + .then(() => firstSync(aliTestClient)) + .then(() => firstSync(bobTestClient)) .then(aliEnablesEncryption) .then(aliSendsFirstMessage) .then(bobRecvMessage) .then(bobEnablesEncryption) .then(bobSendsReplyMessage).then(function(ciphertext) { expect(ciphertext.type).toEqual(1); - }).then(aliRecvMessage) - .catch(testUtils.failTest).done(done); + }).then(aliRecvMessage); }); - it("Ali does a key query when she gets a new_device event", function(done) { - q() - .then(bobUploadsKeys) - .then(() => startClient(aliTestClient)) + it("Ali does a key query when she gets a new_device event", function() { + return q() + .then(() => aliTestClient.start()) + .then(() => firstSync(aliTestClient)) .then(function() { const syncData = { next_batch: '2', @@ -686,15 +688,22 @@ describe("MatrixClient crypto", function() { }; aliTestClient.httpBackend.when('GET', '/sync').respond(200, syncData); return aliTestClient.httpBackend.flush('/sync', 1); - }).then(expectAliQueryKeys) - .nodeify(done); + }).then(() => { + aliTestClient.expectKeyQuery({ + device_keys: { + [bobUserId]: {}, + }, + }); + return aliTestClient.httpBackend.flush('/keys/query', 1); + }); }); - it("Ali does a key query when encryption is enabled", function(done) { + it("Ali does a key query when encryption is enabled", function() { // enabling encryption in the room should make alice download devices // for both members. - q() - .then(() => startClient(aliTestClient)) + return q() + .then(() => aliTestClient.start()) + .then(() => firstSync(aliTestClient)) .then(() => { const syncData = { next_batch: '2', @@ -727,6 +736,6 @@ describe("MatrixClient crypto", function() { }, }); return aliTestClient.httpBackend.flush('/keys/query', 1); - }).nodeify(done); + }); }); }); diff --git a/spec/integ/megolm.spec.js b/spec/integ/megolm.spec.js index 7943cae9b..67b66ba59 100644 --- a/spec/integ/megolm.spec.js +++ b/spec/integ/megolm.spec.js @@ -39,17 +39,19 @@ const ROOM_ID = "!room:id"; * * @param {Olm.Account} olmAccount * @param {TestClient} recipientTestClient - * @return {Olm.Session} + * @return {Promise} promise for Olm.Session */ function createOlmSession(olmAccount, recipientTestClient) { - const otkId = utils.keys(recipientTestClient.oneTimeKeys)[0]; - const otk = recipientTestClient.oneTimeKeys[otkId]; + return recipientTestClient.awaitOneTimeKeyUpload().then((keys) => { + const otkId = utils.keys(keys)[0]; + const otk = keys[otkId]; - const session = new Olm.Session(); - session.create_outbound( - olmAccount, recipientTestClient.getDeviceKey(), otk.key, - ); - return session; + const session = new Olm.Session(); + session.create_outbound( + olmAccount, recipientTestClient.getDeviceKey(), otk.key, + ); + return session; + }); } /** @@ -302,9 +304,9 @@ describe("megolm", function() { }); it("Alice receives a megolm message", function(done) { - return aliceTestClient.start().then(function() { - const p2pSession = createOlmSession(testOlmAccount, aliceTestClient); - + return aliceTestClient.start().then(() => { + return createOlmSession(testOlmAccount, aliceTestClient); + }).then((p2pSession) => { const groupSession = new Olm.OutboundGroupSession(); groupSession.create(); @@ -353,9 +355,9 @@ describe("megolm", function() { // https://github.com/vector-im/riot-web/issues/2273 let roomKeyEncrypted; - return aliceTestClient.start().then(function() { - const p2pSession = createOlmSession(testOlmAccount, aliceTestClient); - + return aliceTestClient.start().then(() => { + return createOlmSession(testOlmAccount, aliceTestClient); + }).then((p2pSession) => { const groupSession = new Olm.OutboundGroupSession(); groupSession.create(); @@ -413,9 +415,9 @@ describe("megolm", function() { }); it("Alice gets a second room_key message", function(done) { - return aliceTestClient.start().then(function() { - const p2pSession = createOlmSession(testOlmAccount, aliceTestClient); - + return aliceTestClient.start().then(() => { + return createOlmSession(testOlmAccount, aliceTestClient); + }).then((p2pSession) => { const groupSession = new Olm.OutboundGroupSession(); groupSession.create(); @@ -483,11 +485,13 @@ describe("megolm", function() { it('Alice sends a megolm message', function(done) { let p2pSession; - return aliceTestClient.start().then(function() { - const syncResponse = getSyncResponse(['@bob:xyz']); - + return aliceTestClient.start().then(() => { // establish an olm session with alice - p2pSession = createOlmSession(testOlmAccount, aliceTestClient); + return createOlmSession(testOlmAccount, aliceTestClient); + }).then((_p2pSession) => { + p2pSession = _p2pSession; + + const syncResponse = getSyncResponse(['@bob:xyz']); const olmEvent = encryptOlmEvent({ senderKey: testSenderKey, @@ -595,11 +599,11 @@ describe("megolm", function() { it("We shouldn't attempt to send to blocked devices", function(done) { - return aliceTestClient.start().then(function() { - const syncResponse = getSyncResponse(['@bob:xyz']); - + return aliceTestClient.start().then(() => { // establish an olm session with alice - const p2pSession = createOlmSession(testOlmAccount, aliceTestClient); + return createOlmSession(testOlmAccount, aliceTestClient); + }).then((p2pSession) => { + const syncResponse = getSyncResponse(['@bob:xyz']); const olmEvent = encryptOlmEvent({ senderKey: testSenderKey, @@ -644,11 +648,13 @@ describe("megolm", function() { let p2pSession; let megolmSessionId; - return aliceTestClient.start().then(function() { - const syncResponse = getSyncResponse(['@bob:xyz']); - + return aliceTestClient.start().then(() => { // establish an olm session with alice - p2pSession = createOlmSession(testOlmAccount, aliceTestClient); + return createOlmSession(testOlmAccount, aliceTestClient); + }).then((_p2pSession) => { + p2pSession = _p2pSession; + + const syncResponse = getSyncResponse(['@bob:xyz']); const olmEvent = encryptOlmEvent({ senderKey: testSenderKey, @@ -873,11 +879,11 @@ describe("megolm", function() { }; }); - return aliceTestClient.start().then(function() { - const syncResponse = getSyncResponse(['@bob:xyz']); - + return aliceTestClient.start().then(() => { // establish an olm session with alice - p2pSession = createOlmSession(testOlmAccount, aliceTestClient); + return createOlmSession(testOlmAccount, aliceTestClient); + }).then((p2pSession) => { + const syncResponse = getSyncResponse(['@bob:xyz']); const olmEvent = encryptOlmEvent({ senderKey: testSenderKey, @@ -1056,10 +1062,9 @@ describe("megolm", function() { let messageEncrypted; return aliceTestClient.start().then(() => { - const p2pSession = createOlmSession( - testOlmAccount, aliceTestClient, - ); - + // establish an olm session with alice + return createOlmSession(testOlmAccount, aliceTestClient); + }).then((p2pSession) => { const groupSession = new Olm.OutboundGroupSession(); groupSession.create(); diff --git a/src/client.js b/src/client.js index 2d02a4d7c..e690248d2 100644 --- a/src/client.js +++ b/src/client.js @@ -300,17 +300,15 @@ MatrixClient.prototype.getDeviceEd25519Key = function() { }; /** - * Upload the device keys to the homeserver and ensure that the - * homeserver has enough one-time keys. - * @param {number} maxKeys The maximum number of keys to generate + * Upload the device keys to the homeserver. * @return {object} A promise that will resolve when the keys are uploaded. */ -MatrixClient.prototype.uploadKeys = function(maxKeys) { +MatrixClient.prototype.uploadKeys = function() { if (this._crypto === null) { throw new Error("End-to-end encryption disabled"); } - return this._crypto.uploadKeys(maxKeys); + return this._crypto.uploadDeviceKeys(); }; /** @@ -2687,12 +2685,7 @@ MatrixClient.prototype.startClient = function(opts) { } if (this._crypto) { - this._crypto.uploadKeys(5).done(); - const tenMinutes = 1000 * 60 * 10; - const self = this; - this._uploadIntervalID = global.setInterval(function() { - self._crypto.uploadKeys(5).done(); - }, tenMinutes); + this._crypto.uploadDeviceKeys().done(); } // periodically poll for turn servers if we support voip @@ -2725,9 +2718,6 @@ MatrixClient.prototype.stopClient = function() { this._syncApi.stop(); this._syncApi = null; } - if (this._crypto) { - global.clearInterval(this._uploadIntervalID); - } global.clearTimeout(this._checkTurnServersTimeoutID); }; diff --git a/src/crypto/index.js b/src/crypto/index.js index cb2db1cdc..41e85b2ee 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -63,6 +63,13 @@ function Crypto(baseApis, eventEmitter, sessionStore, userId, deviceId, this._deviceList = new DeviceList(baseApis, sessionStore, this._olmDevice); this._initialDeviceListInvalidationDone = false; + this._clientRunning = false; + + // the last time we did a check for the number of one-time-keys on the + // server. + this._lastOneTimeKeyCheck = null; + this._oneTimeKeyCheckInProgress = false; + // EncryptionAlgorithm instance for each room this._roomEncryptors = {}; @@ -111,6 +118,11 @@ function Crypto(baseApis, eventEmitter, sessionStore, userId, deviceId, function _registerEventHandlers(crypto, eventEmitter) { eventEmitter.on("sync", function(syncState, oldState, data) { try { + if (syncState === "STOPPED") { + crypto._clientRunning = false; + } else if (syncState === "PREPARED") { + crypto._clientRunning = true; + } if (syncState === "SYNCING") { crypto._onSyncCompleted(data); } @@ -187,61 +199,11 @@ Crypto.prototype.getGlobalBlacklistUnverifiedDevices = function() { }; /** - * Upload the device keys to the homeserver and ensure that the - * homeserver has enough one-time keys. - * @param {number} maxKeys The maximum number of keys to generate + * Upload the device keys to the homeserver. * @return {object} A promise that will resolve when the keys are uploaded. */ -Crypto.prototype.uploadKeys = function(maxKeys) { - const self = this; - return _uploadDeviceKeys(this).then(function(res) { - // We need to keep a pool of one time public keys on the server so that - // other devices can start conversations with us. But we can only store - // a finite number of private keys in the olm Account object. - // To complicate things further then can be a delay between a device - // claiming a public one time key from the server and it sending us a - // message. We need to keep the corresponding private key locally until - // we receive the message. - // But that message might never arrive leaving us stuck with duff - // private keys clogging up our local storage. - // So we need some kind of enginering compromise to balance all of - // these factors. - - // We first find how many keys the server has for us. - const keyCount = res.one_time_key_counts.signed_curve25519 || 0; - // We then check how many keys we can store in the Account object. - const maxOneTimeKeys = self._olmDevice.maxNumberOfOneTimeKeys(); - // Try to keep at most half that number on the server. This leaves the - // rest of the slots free to hold keys that have been claimed from the - // server but we haven't recevied a message for. - // If we run out of slots when generating new keys then olm will - // discard the oldest private keys first. This will eventually clean - // out stale private keys that won't receive a message. - const keyLimit = Math.floor(maxOneTimeKeys / 2); - // We work out how many new keys we need to create to top up the server - // If there are too many keys on the server then we don't need to - // create any more keys. - let numberToGenerate = Math.max(keyLimit - keyCount, 0); - if (maxKeys !== undefined) { - // Creating keys can be an expensive operation so we limit the - // number we generate in one go to avoid blocking the application - // for too long. - numberToGenerate = Math.min(numberToGenerate, maxKeys); - } - - if (numberToGenerate <= 0) { - // If we don't need to generate any keys then we are done. - return; - } - - // Ask olm to generate new one time keys, then upload them to synapse. - self._olmDevice.generateOneTimeKeys(numberToGenerate); - return _uploadOneTimeKeys(self); - }); -}; - -// returns a promise which resolves to the response -function _uploadDeviceKeys(crypto) { +Crypto.prototype.uploadDeviceKeys = function() { + const crypto = this; const userId = crypto._userId; const deviceId = crypto._deviceId; @@ -260,6 +222,90 @@ function _uploadDeviceKeys(crypto) { // same one as used in login. device_id: deviceId, }); +}; + +// check if it's time to upload one-time keys, and do so if so. +function _maybeUploadOneTimeKeys(crypto) { + // frequency with which to check & upload one-time keys + const uploadPeriod = 1000 * 60; // one minute + + // max number of keys to upload at once + // Creating keys can be an expensive operation so we limit the + // number we generate in one go to avoid blocking the application + // for too long. + const maxKeysPerCycle = 5; + + if (crypto._oneTimeKeyCheckInProgress) { + return; + } + + const now = Date.now(); + if (crypto._lastOneTimeKeyCheck !== null && + now - crypto._lastOneTimeKeyCheck < uploadPeriod + ) { + // we've done a key upload recently. + return; + } + + crypto._lastOneTimeKeyCheck = now; + + function uploadLoop(numberToGenerate) { + if (numberToGenerate <= 0) { + // If we don't need to generate any more keys then we are done. + return; + } + + const keysThisLoop = Math.min(numberToGenerate, maxKeysPerCycle); + + // Ask olm to generate new one time keys, then upload them to synapse. + crypto._olmDevice.generateOneTimeKeys(keysThisLoop); + return _uploadOneTimeKeys(crypto).then(() => { + return uploadLoop(numberToGenerate - keysThisLoop); + }); + } + + crypto._oneTimeKeyCheckInProgress = true; + q().then(() => { + // ask the server how many keys we have + return crypto._baseApis.uploadKeysRequest({}, { + device_id: crypto._deviceId, + }); + }).then((res) => { + // We need to keep a pool of one time public keys on the server so that + // other devices can start conversations with us. But we can only store + // a finite number of private keys in the olm Account object. + // To complicate things further then can be a delay between a device + // claiming a public one time key from the server and it sending us a + // message. We need to keep the corresponding private key locally until + // we receive the message. + // But that message might never arrive leaving us stuck with duff + // private keys clogging up our local storage. + // So we need some kind of enginering compromise to balance all of + // these factors. + + // We first find how many keys the server has for us. + const keyCount = res.one_time_key_counts.signed_curve25519 || 0; + // We then check how many keys we can store in the Account object. + const maxOneTimeKeys = crypto._olmDevice.maxNumberOfOneTimeKeys(); + // Try to keep at most half that number on the server. This leaves the + // rest of the slots free to hold keys that have been claimed from the + // server but we haven't recevied a message for. + // If we run out of slots when generating new keys then olm will + // discard the oldest private keys first. This will eventually clean + // out stale private keys that won't receive a message. + const keyLimit = Math.floor(maxOneTimeKeys / 2); + + // We work out how many new keys we need to create to top up the server + // If there are too many keys on the server then we don't need to + // create any more keys. + const numberToGenerate = Math.max(keyLimit - keyCount, 0); + + return uploadLoop(numberToGenerate); + }).catch((e) => { + console.error("Error uploading one-time keys", e.stack || e); + }).finally(() => { + crypto._oneTimeKeyCheckInProgress = false; + }).done(); } // returns a promise which resolves to the response @@ -805,6 +851,14 @@ Crypto.prototype._onSyncCompleted = function(syncData) { // catch up on any new devices we got told about during the sync. this._deviceList.refreshOutdatedDeviceLists(); } + + // we don't start uploading one-time keys until we've caught up with + // to-device messages, to help us avoid throwing away one-time-keys that we + // are about to receive messages for + // (https://github.com/vector-im/riot-web/issues/2782). + if (!syncData.catchingUp) { + _maybeUploadOneTimeKeys(this); + } }; /**