From d24be7ee552195721d694fed67d5d379a3252a72 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 28 Aug 2020 13:51:58 +0200 Subject: [PATCH 1/6] extract constants out --- src/matrix/e2ee/Account.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/matrix/e2ee/Account.js b/src/matrix/e2ee/Account.js index ef342d49..829e7595 100644 --- a/src/matrix/e2ee/Account.js +++ b/src/matrix/e2ee/Account.js @@ -18,6 +18,8 @@ import anotherjson from "../../../lib/another-json/index.js"; const ACCOUNT_SESSION_KEY = "olmAccount"; const DEVICE_KEY_FLAG_SESSION_KEY = "areDeviceKeysUploaded"; +const OLM_ALGORITHM = "m.olm.v1.curve25519-aes-sha2"; +const MEGOLM_ALGORITHM = "m.megolm.v1.aes-sha2"; export class Account { static async load({olm, pickleKey, hsApi, userId, deviceId, txn}) { @@ -84,10 +86,7 @@ export class Account { const obj = { user_id: this._userId, device_id: this._deviceId, - algorithms: [ - "m.olm.v1.curve25519-aes-sha2", - "m.megolm.v1.aes-sha2" - ], + algorithms: [OLM_ALGORITHM, MEGOLM_ALGORITHM], keys: {} }; for (const [algorithm, pubKey] of Object.entries(identityKeys)) { From 3ab5a7222160551e9143a7182f374d462633677f Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 28 Aug 2020 13:52:27 +0200 Subject: [PATCH 2/6] give e2ee account values a prefix so we can prevent from clearing them --- src/matrix/e2ee/Account.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/matrix/e2ee/Account.js b/src/matrix/e2ee/Account.js index 829e7595..f53532f1 100644 --- a/src/matrix/e2ee/Account.js +++ b/src/matrix/e2ee/Account.js @@ -16,8 +16,10 @@ limitations under the License. import anotherjson from "../../../lib/another-json/index.js"; -const ACCOUNT_SESSION_KEY = "olmAccount"; -const DEVICE_KEY_FLAG_SESSION_KEY = "areDeviceKeysUploaded"; +// use common prefix so it's easy to clear properties that are not e2ee related during session clear +export const SESSION_KEY_PREFIX = "e2ee:"; +const ACCOUNT_SESSION_KEY = SESSION_KEY_PREFIX + "olmAccount"; +const DEVICE_KEY_FLAG_SESSION_KEY = SESSION_KEY_PREFIX + "areDeviceKeysUploaded"; const OLM_ALGORITHM = "m.olm.v1.curve25519-aes-sha2"; const MEGOLM_ALGORITHM = "m.megolm.v1.aes-sha2"; From d64db185bd1a32726af2e05edd19f9f0a3363a46 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 28 Aug 2020 13:54:42 +0200 Subject: [PATCH 3/6] await callback in case we need to read, then write from it --- src/matrix/e2ee/Account.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/matrix/e2ee/Account.js b/src/matrix/e2ee/Account.js index f53532f1..92478156 100644 --- a/src/matrix/e2ee/Account.js +++ b/src/matrix/e2ee/Account.js @@ -115,7 +115,7 @@ export class Account { storage.storeNames.session ]); try { - callback(txn.session); + await callback(txn.session); } catch (err) { txn.abort(); throw err; From 681dfdf62ba39c955bfdf3e4305932917067e070 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 28 Aug 2020 13:56:44 +0200 Subject: [PATCH 4/6] sync otk count to e2ee account --- src/matrix/Session.js | 17 ++++++++++++++--- src/matrix/Sync.js | 2 +- src/matrix/e2ee/Account.js | 37 ++++++++++++++++++++++++++++++++----- 3 files changed, 47 insertions(+), 9 deletions(-) diff --git a/src/matrix/Session.js b/src/matrix/Session.js index f5d9021c..90d29b61 100644 --- a/src/matrix/Session.js +++ b/src/matrix/Session.js @@ -151,20 +151,31 @@ export class Session { return room; } - writeSync(syncToken, syncFilterId, accountData, txn) { + writeSync(syncResponse, syncFilterId, txn) { + const changes = {}; + const syncToken = syncResponse.next_batch; + const deviceOneTimeKeysCount = syncResponse.device_one_time_keys_count; + + if (this._e2eeAccount && deviceOneTimeKeysCount) { + changes.e2eeAccountChanges = this._e2eeAccount.writeSync(deviceOneTimeKeysCount, txn); + } if (syncToken !== this.syncToken) { const syncInfo = {token: syncToken, filterId: syncFilterId}; // don't modify `this` because transaction might still fail txn.session.set("sync", syncInfo); - return syncInfo; + changes.syncInfo = syncInfo; } + return changes; } - afterSync(syncInfo) { + afterSync({syncInfo, e2eeAccountChanges}) { if (syncInfo) { // sync transaction succeeded, modify object state now this._syncInfo = syncInfo; } + if (this._e2eeAccount && e2eeAccountChanges) { + this._e2eeAccount.afterSync(e2eeAccountChanges); + } } get syncToken() { diff --git a/src/matrix/Sync.js b/src/matrix/Sync.js index e6de7146..e14451a9 100644 --- a/src/matrix/Sync.js +++ b/src/matrix/Sync.js @@ -127,7 +127,7 @@ export class Sync { const roomChanges = []; let sessionChanges; try { - sessionChanges = this._session.writeSync(syncToken, syncFilterId, response.account_data, syncTxn); + sessionChanges = this._session.writeSync(response, syncFilterId, syncTxn); // to_device // presence if (response.rooms) { diff --git a/src/matrix/e2ee/Account.js b/src/matrix/e2ee/Account.js index 92478156..90fabc72 100644 --- a/src/matrix/e2ee/Account.js +++ b/src/matrix/e2ee/Account.js @@ -20,6 +20,7 @@ import anotherjson from "../../../lib/another-json/index.js"; export const SESSION_KEY_PREFIX = "e2ee:"; const ACCOUNT_SESSION_KEY = SESSION_KEY_PREFIX + "olmAccount"; const DEVICE_KEY_FLAG_SESSION_KEY = SESSION_KEY_PREFIX + "areDeviceKeysUploaded"; +const SERVER_OTK_COUNT_SESSION_KEY = SESSION_KEY_PREFIX + "serverOTKCount"; const OLM_ALGORITHM = "m.olm.v1.curve25519-aes-sha2"; const MEGOLM_ALGORITHM = "m.megolm.v1.aes-sha2"; @@ -30,7 +31,9 @@ export class Account { const account = new olm.Account(); const areDeviceKeysUploaded = await txn.session.get(DEVICE_KEY_FLAG_SESSION_KEY); account.unpickle(pickleKey, pickledAccount); - return new Account({pickleKey, hsApi, account, userId, deviceId, areDeviceKeysUploaded}); + const serverOTKCount = await txn.session.get(SERVER_OTK_COUNT_SESSION_KEY); + return new Account({pickleKey, hsApi, account, userId, + deviceId, areDeviceKeysUploaded, serverOTKCount}); } } @@ -44,16 +47,19 @@ export class Account { const areDeviceKeysUploaded = false; await txn.session.add(ACCOUNT_SESSION_KEY, pickledAccount); await txn.session.add(DEVICE_KEY_FLAG_SESSION_KEY, areDeviceKeysUploaded); - return new Account({pickleKey, hsApi, account, userId, deviceId, areDeviceKeysUploaded}); + await txn.session.add(SERVER_OTK_COUNT_SESSION_KEY, 0); + return new Account({pickleKey, hsApi, account, userId, + deviceId, areDeviceKeysUploaded, serverOTKCount: 0}); } - constructor({pickleKey, hsApi, account, userId, deviceId, areDeviceKeysUploaded}) { + constructor({pickleKey, hsApi, account, userId, deviceId, areDeviceKeysUploaded, serverOTKCount}) { this._pickleKey = pickleKey; this._hsApi = hsApi; this._account = account; this._userId = userId; this._deviceId = deviceId; this._areDeviceKeysUploaded = areDeviceKeysUploaded; + this._serverOTKCount = serverOTKCount; } async uploadKeys(storage) { @@ -69,12 +75,17 @@ export class Account { if (oneTimeKeysEntries.length) { payload.one_time_keys = this._oneTimeKeysPayload(oneTimeKeysEntries); } - await this._hsApi.uploadKeys(payload); - + const response = await this._hsApi.uploadKeys(payload).response(); + this._serverOTKCount = response?.one_time_key_counts?.signed_curve25519; + // TODO: should we not modify this in the txn like we do elsewhere? + // we'd have to pickle and unpickle the account to clone it though ... + // and the upload has succeed at this point, so in-memory would be correct + // but in-storage not if the txn fails. await this._updateSessionStorage(storage, sessionStore => { if (oneTimeKeysEntries.length) { this._account.mark_keys_as_published(); sessionStore.set(ACCOUNT_SESSION_KEY, this._account.pickle(this._pickleKey)); + sessionStore.set(SERVER_OTK_COUNT_SESSION_KEY, this._serverOTKCount); } if (!this._areDeviceKeysUploaded) { this._areDeviceKeysUploaded = true; @@ -84,6 +95,22 @@ export class Account { } } + writeSync(deviceOneTimeKeysCount, txn) { + // we only upload signed_curve25519 otks + const otkCount = deviceOneTimeKeysCount.signed_curve25519; + if (Number.isSafeInteger(otkCount) && otkCount !== this._serverOTKCount) { + txn.session.set(SERVER_OTK_COUNT_SESSION_KEY, otkCount); + return otkCount; + } + } + + afterSync(otkCount) { + // could also be undefined + if (Number.isSafeInteger(otkCount)) { + this._serverOTKCount = otkCount; + } + } + _deviceKeysPayload(identityKeys) { const obj = { user_id: this._userId, From a1ba5d7dba9ef7e3db4bc24cafe450c6d513fe34 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 28 Aug 2020 13:58:17 +0200 Subject: [PATCH 5/6] between syncs, generate more otks if needed, and upload them --- src/matrix/Session.js | 10 ++++++++++ src/matrix/Sync.js | 6 ++++++ src/matrix/e2ee/Account.js | 25 +++++++++++++++++++++++++ 3 files changed, 41 insertions(+) diff --git a/src/matrix/Session.js b/src/matrix/Session.js index 90d29b61..bc52227a 100644 --- a/src/matrix/Session.js +++ b/src/matrix/Session.js @@ -60,6 +60,7 @@ export class Session { } await txn.complete(); } + await this._e2eeAccount.generateOTKsIfNeeded(this._storage); await this._e2eeAccount.uploadKeys(this._storage); } } @@ -178,6 +179,15 @@ export class Session { } } + async afterSyncCompleted() { + const needsToUploadOTKs = await this._e2eeAccount.generateOTKsIfNeeded(this._storage); + if (needsToUploadOTKs) { + // TODO: we could do this in parallel with sync if it proves to be too slow + // but I'm not sure how to not swallow errors in that case + await this._e2eeAccount.uploadKeys(this._storage); + } + } + get syncToken() { return this._syncInfo?.token; } diff --git a/src/matrix/Sync.js b/src/matrix/Sync.js index e14451a9..c7aaaa99 100644 --- a/src/matrix/Sync.js +++ b/src/matrix/Sync.js @@ -100,6 +100,12 @@ export class Sync { this._status.set(SyncStatus.Stopped); } } + try { + await this._session.afterSyncCompleted(); + } catch (err) { + console.err("error during after sync completed, continuing to sync.", err.stack); + // swallowing error here apart from logging + } } } diff --git a/src/matrix/e2ee/Account.js b/src/matrix/e2ee/Account.js index 90fabc72..b8c39826 100644 --- a/src/matrix/e2ee/Account.js +++ b/src/matrix/e2ee/Account.js @@ -95,6 +95,31 @@ export class Account { } } + async generateOTKsIfNeeded(storage) { + const maxOTKs = this._account.max_number_of_one_time_keys(); + const limit = maxOTKs / 2; + if (this._serverOTKCount < limit) { + // TODO: cache unpublishedOTKCount, so we don't have to parse this JSON on every sync iteration + // for now, we only determine it when serverOTKCount is sufficiently low, which is should rarely be, + // and recheck + const oneTimeKeys = JSON.parse(this._account.one_time_keys()); + const oneTimeKeysEntries = Object.entries(oneTimeKeys.curve25519); + const unpublishedOTKCount = oneTimeKeysEntries.length; + const totalOTKCount = this._serverOTKCount + unpublishedOTKCount; + if (totalOTKCount < limit) { + // we could in theory also generated the keys and store them in + // writeSync, but then we would have to clone the account to avoid side-effects. + await this._updateSessionStorage(storage, sessionStore => { + const newKeyCount = maxOTKs - totalOTKCount; + this._account.generate_one_time_keys(newKeyCount); + sessionStore.set(ACCOUNT_SESSION_KEY, this._account.pickle(this._pickleKey)); + }); + return true; + } + } + return false; + } + writeSync(deviceOneTimeKeysCount, txn) { // we only upload signed_curve25519 otks const otkCount = deviceOneTimeKeysCount.signed_curve25519; From e751333bbdf2978af811b1c4876eb05b95d277fd Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 28 Aug 2020 13:58:42 +0200 Subject: [PATCH 6/6] don't assume setting up a session went all the way through when stopping --- src/matrix/SessionContainer.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/matrix/SessionContainer.js b/src/matrix/SessionContainer.js index ae9572a9..1b6e21d8 100644 --- a/src/matrix/SessionContainer.js +++ b/src/matrix/SessionContainer.js @@ -237,10 +237,16 @@ export class SessionContainer { } stop() { - this._reconnectSubscription(); - this._reconnectSubscription = null; - this._sync.stop(); - this._session.stop(); + if (this._reconnectSubscription) { + this._reconnectSubscription(); + this._reconnectSubscription = null; + } + if (this._sync) { + this._sync.stop(); + } + if (this._session) { + this._session.stop(); + } if (this._waitForFirstSyncHandle) { this._waitForFirstSyncHandle.dispose(); this._waitForFirstSyncHandle = null;