From e686eb750ff41a2324a6884fdc1a2228d49d477a Mon Sep 17 00:00:00 2001 From: krombel Date: Wed, 19 Jul 2017 17:27:05 +0200 Subject: [PATCH] use device_one_time_keys_count transmitted by /sync (#493) Where it is available, use the one_time_keys_count returned by /sync instead of polling the server for it. This was added to synapse in matrix-org/synapse#2237. --- spec/integ/matrix-client-crypto.spec.js | 49 +++++++++++++ src/crypto/index.js | 96 ++++++++++++++++--------- src/sync.js | 7 ++ 3 files changed, 118 insertions(+), 34 deletions(-) diff --git a/spec/integ/matrix-client-crypto.spec.js b/spec/integ/matrix-client-crypto.spec.js index cafdfc78c25..bddff613254 100644 --- a/spec/integ/matrix-client-crypto.spec.js +++ b/spec/integ/matrix-client-crypto.spec.js @@ -689,4 +689,53 @@ describe("MatrixClient crypto", function() { return aliTestClient.httpBackend.flush('/keys/query', 1); }); }); + + it("Upload new oneTimeKeys based on a /sync request - no count-asking", function() { + // Send a response which causes a key upload + const httpBackend = aliTestClient.httpBackend; + const syncDataEmpty = { + next_batch: "a", + device_one_time_keys_count: { + signed_curve25519: 0, + }, + }; + + // enqueue expectations: + // * Sync with empty one_time_keys => upload keys + + return Promise.resolve() + .then(() => { + console.log(aliTestClient + ': starting'); + httpBackend.when("GET", "/pushrules").respond(200, {}); + httpBackend.when("POST", "/filter").respond(200, { filter_id: "fid" }); + aliTestClient.expectDeviceKeyUpload(); + + // we let the client do a very basic initial sync, which it needs before + // it will upload one-time keys. + httpBackend.when("GET", "/sync").respond(200, syncDataEmpty); + + aliTestClient.client.startClient({}); + + return httpBackend.flushAllExpected().then(() => { + console.log(aliTestClient + ': started'); + }); + }) + .then(() => httpBackend.when("POST", "/keys/upload") + .respond(200, (path, content) => { + expect(content.one_time_keys).toBeTruthy(); + expect(content.one_time_keys).toNotEqual({}); + expect(Object.keys(content.one_time_keys).length) + .toBeGreaterThanOrEqualTo(1); + console.log('received %i one-time keys', + Object.keys(content.one_time_keys).length); + // cancel futher calls by telling the client + // we have more than we need + return { + one_time_key_counts: { + signed_curve25519: 70, + }, + }; + })) + .then(() => httpBackend.flushAllExpected()); + }); }); diff --git a/src/crypto/index.js b/src/crypto/index.js index 8e71c4a7207..6c8b67e24b6 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -246,6 +246,20 @@ Crypto.prototype.uploadDeviceKeys = function() { }); }; +/** + * Stores the current one_time_key count which will be handled later (in a call of + * _onSyncCompleted). The count is e.g. coming from a /sync response. + * + * @param {Number} currentCount The current count of one_time_keys to be stored + */ +Crypto.prototype.updateOneTimeKeyCount = function(currentCount) { + if (isFinite(currentCount)) { + this._oneTimeKeyCount = currentCount; + } else { + throw new TypeError("Parameter for updateOneTimeKeyCount has to be a number"); + } +}; + // 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 @@ -271,61 +285,75 @@ function _maybeUploadOneTimeKeys(crypto) { crypto._lastOneTimeKeyCheck = now; - function uploadLoop(numberToGenerate) { - if (numberToGenerate <= 0) { + // 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. + + // 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); + + function uploadLoop(keyCount) { + if (keyLimit <= keyCount) { // If we don't need to generate any more keys then we are done. return; } - const keysThisLoop = Math.min(numberToGenerate, maxKeysPerCycle); + const keysThisLoop = Math.min(keyLimit - keyCount, 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); + return _uploadOneTimeKeys(crypto).then((res) => { + if (res.one_time_key_counts && res.one_time_key_counts.signed_curve25519) { + // if the response contains a more up to date value use this + // for the next loop + return uploadLoop(res.one_time_key_counts.signed_curve25519); + } else { + throw new Error("response for uploading keys does not contain " + + "one_time_key_counts.signed_curve25519"); + } }); } crypto._oneTimeKeyCheckInProgress = true; Promise.resolve().then(() => { + if (crypto._oneTimeKeyCount !== undefined) { + // We already have the current one_time_key count from a /sync response. + // Use this value instead of asking the server for the current key count. + return Promise.resolve(crypto._oneTimeKeyCount); + } // ask the server how many keys we have return crypto._baseApis.uploadKeysRequest({}, { device_id: crypto._deviceId, + }).then((res) => { + return res.one_time_key_counts.signed_curve25519 || 0; }); - }).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 + }).then((keyCount) => { + // Start the uploadLoop with the current keyCount. The function checks if + // we need to upload new keys or not. // 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); + return uploadLoop(keyCount); }).catch((e) => { console.error("Error uploading one-time keys", e.stack || e); }).finally(() => { + // reset _oneTimeKeyCount to prevent start uploading based on old data. + // it will be set again on the next /sync-response + crypto._oneTimeKeyCount = undefined; crypto._oneTimeKeyCheckInProgress = false; }).done(); } diff --git a/src/sync.js b/src/sync.js index 284b967efb1..ed53c6bb883 100644 --- a/src/sync.js +++ b/src/sync.js @@ -691,6 +691,7 @@ SyncApi.prototype._processSyncResponse = function(syncToken, data) { // account_data: { events: [] }, // device_lists: { changed: ["@user:server", ... ]}, // to_device: { events: [] }, + // device_one_time_keys_count: { signed_curve25519: 42 }, // rooms: { // invite: { // $roomid: { @@ -982,6 +983,12 @@ SyncApi.prototype._processSyncResponse = function(syncToken, data) { this.opts.crypto.userDeviceListChanged(u); }); } + + // Handle one_time_keys_count + if (this.opts.crypto && data.device_one_time_keys_count) { + const currentCount = data.device_one_time_keys_count.signed_curve25519 || 0; + this.opts.crypto.updateOneTimeKeyCount(currentCount); + } }; /**