Merge pull request #376 from matrix-org/rav/delay_otk_generation

Upload one-time keys on /sync rather than a timer

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;
+ * If one-time keys have already been uploaded, return them. Otherwise,
-        return {one_time_key_counts: {
+ * set up an expectation that the keys will be uploaded, and wait for
-            signed_curve25519: Object.keys(self.oneTimeKeys).length,
+ * 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;
     });
 };
 

spec/integ/matrix-client-crypto.spec.js

@@ -44,15 +44,13 @@ const bobAccessToken = "fewgfkuesa";
 let aliMessages;
 let bobMessages;
 
-
+function bobUploadsDeviceKeys() {
-function bobUploadsKeys() {
+    bobTestClient.expectDeviceKeyUpload();
-    bobTestClient.expectKeyUpload();
     return q.all([
-        bobTestClient.client.uploadKeys(5),
+        bobTestClient.client.uploadKeys(),
         bobTestClient.httpBackend.flush(),
     ]).then(() => {
-        expect(Object.keys(bobTestClient.oneTimeKeys).length).toEqual(5);
+        expect(Object.keys(bobTestClient.deviceKeys).length).toNotEqual(0);
-        expect(bobTestClient.deviceKeys).toNotEqual({});
     });
 }
 
@@ -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
+    return bobTestClient.awaitOneTimeKeyUpload().then((keys) => {
-    expect(bobTestClient.oneTimeKeys).toNotEqual({});
+        aliTestClient.httpBackend.when(
-
+            "POST", "/keys/claim",
-    aliTestClient.httpBackend.when(
+        ).respond(200, function(path, content) {
-        "POST", "/keys/claim",
+            const claimType = content.one_time_keys[bobUserId][bobDeviceId];
-    ).respond(200, function(path, content) {
+            expect(claimType).toEqual("signed_curve25519");
-        const claimType = content.one_time_keys[bobUserId][bobDeviceId];
+            let keyId = null;
-        expect(claimType).toEqual("signed_curve25519");
+            for (keyId in keys) {
-        let keyId = null;
+                if (bobTestClient.oneTimeKeys.hasOwnProperty(keyId)) {
-        for (keyId in bobTestClient.oneTimeKeys) {
+                    if (keyId.indexOf(claimType + ":") === 0) {
-            if (bobTestClient.oneTimeKeys.hasOwnProperty(keyId)) {
+                        break;
-                if (keyId.indexOf(claimType + ":") === 0) {
+                    }
-                    break;
                 }
             }
-        }
+            const result = {};
-        const result = {};
+            result[bobUserId] = {};
-        result[bobUserId] = {};
+            result[bobUserId][bobDeviceId] = {};
-        result[bobUserId][bobDeviceId] = {};
+            result[bobUserId][bobDeviceId][keyId] = keys[keyId];
-        result[bobUserId][bobDeviceId][keyId] = bobTestClient.oneTimeKeys[keyId];
+            return {one_time_keys: result};
-        return {one_time_keys: result};
+        });
-    });
+    }).then(() => {
-
+        // it can take a while to process the key query, so give it some extra
-    // 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
-    // time, and make sure the claim actually happens rather than ploughing on
+        // confusingly.
-    // confusingly.
+        return aliTestClient.httpBackend.flush("/keys/claim", 1, 20).then((r) => {
-    return aliTestClient.httpBackend.flush("/keys/claim", 1, 20).then((r) => {
+            expect(r).toEqual(1);
-        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
+ * Send an initial sync response to the client (which just includes the member
- * start the client.
+ * 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) {
+    it("Bob uploads device keys", function() {
-        q()
+        return q()
-            .then(bobUploadsKeys)
+            .then(bobUploadsDeviceKeys);
-            .catch(testUtils.failTest).done(done);
     });
 
-    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()
+    it("Bob starts his client and uploads device keys and one-time keys", function() {
-            .then(bobUploadsKeys)
+        return q()
-            .then(() => startClient(aliTestClient))
+            .then(() => bobTestClient.start())
-            .then(aliEnablesEncryption)
+            .then(() => bobTestClient.awaitOneTimeKeyUpload())
-            .catch(testUtils.failTest).done(done);
+            .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(() => aliTestClient.start())
-            .then(() => startClient(aliTestClient))
+            .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(() => aliTestClient.start())
-            .then(() => startClient(aliTestClient))
+            .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(() => aliTestClient.start())
-            .then(() => startClient(aliTestClient))
+            .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(() => aliTestClient.start())
-            .then(() => startClient(aliTestClient))
+            .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(() => aliTestClient.start())
-            .then(() => startClient(aliTestClient))
+            .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) {
+    it("Bob replies to the message", function() {
-        q()
+        return q()
-            .then(() => startClient(aliTestClient))
+            .then(() => aliTestClient.start())
-            .then(() => startClient(bobTestClient))
+            .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)
+            }).then(aliRecvMessage);
-            .catch(testUtils.failTest).done(done);
     });
 
 
-    it("Ali does a key query when she gets a new_device event", function(done) {
+    it("Ali does a key query when she gets a new_device event", function() {
-        q()
+        return q()
-            .then(bobUploadsKeys)
+            .then(() => aliTestClient.start())
-            .then(() => startClient(aliTestClient))
+            .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)
+            }).then(() => {
-            .nodeify(done);
+                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()
+        return q()
-            .then(() => startClient(aliTestClient))
+            .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);
+            });
     });
 });

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];
+    return recipientTestClient.awaitOneTimeKeyUpload().then((keys) => {
-    const otk = recipientTestClient.oneTimeKeys[otkId];
+        const otkId = utils.keys(keys)[0];
+        const otk = keys[otkId];
 
-    const session = new Olm.Session();
+        const session = new Olm.Session();
-    session.create_outbound(
+        session.create_outbound(
-        olmAccount, recipientTestClient.getDeviceKey(), otk.key,
+            olmAccount, recipientTestClient.getDeviceKey(), otk.key,
-    );
+        );
-    return session;
+        return session;
+    });
 }
 
 /**
@@ -302,9 +304,9 @@ describe("megolm", function() {
     });
 
     it("Alice receives a megolm message", function(done) {
-        return aliceTestClient.start().then(function() {
+        return aliceTestClient.start().then(() => {
-            const p2pSession = createOlmSession(testOlmAccount, aliceTestClient);
+            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() {
+        return aliceTestClient.start().then(() => {
-            const p2pSession = createOlmSession(testOlmAccount, aliceTestClient);
+            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() {
+        return aliceTestClient.start().then(() => {
-            const p2pSession = createOlmSession(testOlmAccount, aliceTestClient);
+            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() {
+        return aliceTestClient.start().then(() => {
-            const syncResponse = getSyncResponse(['@bob:xyz']);
-
             // 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() {
+        return aliceTestClient.start().then(() => {
-            const syncResponse = getSyncResponse(['@bob:xyz']);
-
             // 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() {
+        return aliceTestClient.start().then(() => {
-            const syncResponse = getSyncResponse(['@bob:xyz']);
-
             // 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() {
+        return aliceTestClient.start().then(() => {
-            const syncResponse = getSyncResponse(['@bob:xyz']);
-
             // 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(
+            // establish an olm session with alice
-                testOlmAccount, aliceTestClient,
+            return createOlmSession(testOlmAccount, aliceTestClient);
-            );
+        }).then((p2pSession) => {
-
             const groupSession = new Olm.OutboundGroupSession();
             groupSession.create();
 

src/client.js

@@ -303,17 +303,15 @@ MatrixClient.prototype.getDeviceEd25519Key = function() {
 };
 
 /**
- * Upload the device keys to the homeserver and ensure that the
+ * Upload the device keys to the homeserver.
- * homeserver has enough one-time keys.
- * @param {number} maxKeys The maximum number of keys to generate
  * @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();
 };
 
 /**
@@ -2690,12 +2688,7 @@ MatrixClient.prototype.startClient = function(opts) {
     }
 
     if (this._crypto) {
-        this._crypto.uploadKeys(5).done();
+        this._crypto.uploadDeviceKeys().done();
-        const tenMinutes = 1000 * 60 * 10;
-        const self = this;
-        this._uploadIntervalID = global.setInterval(function() {
-            self._crypto.uploadKeys(5).done();
-        }, tenMinutes);
     }
 
     // periodically poll for turn servers if we support voip
@@ -2729,9 +2722,6 @@ MatrixClient.prototype.stopClient = function() {
         this._syncApi.stop();
         this._syncApi = null;
     }
-    if (this._crypto) {
-        global.clearInterval(this._uploadIntervalID);
-    }
     global.clearTimeout(this._checkTurnServersTimeoutID);
 };
 

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
+ * Upload the device keys to the homeserver.
- * homeserver has enough one-time keys.
- * @param {number} maxKeys The maximum number of keys to generate
  * @return {object} A promise that will resolve when the keys are uploaded.
  */
-Crypto.prototype.uploadKeys = function(maxKeys) {
+Crypto.prototype.uploadDeviceKeys = function() {
-    const self = this;
+    const crypto = 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) {
     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);
+    }
 };
 
 /**