From 941ae18d74df97a42f959d5a37057f62a3c8b952 Mon Sep 17 00:00:00 2001 From: Zoe Date: Wed, 26 Feb 2020 15:28:13 +0000 Subject: [PATCH 1/7] Added tests for CrossSigningInfo.getCrossSigningKey --- spec/unit/crypto/CrossSigningInfo.spec.js | 68 +++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 spec/unit/crypto/CrossSigningInfo.spec.js diff --git a/spec/unit/crypto/CrossSigningInfo.spec.js b/spec/unit/crypto/CrossSigningInfo.spec.js new file mode 100644 index 000000000..797478387 --- /dev/null +++ b/spec/unit/crypto/CrossSigningInfo.spec.js @@ -0,0 +1,68 @@ +/* +Copyright 2020 New Vector Ltd +Copyright 2020 The Matrix.org Foundation C.I.C. + +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. +*/ + +import '../../olm-loader'; +import { CrossSigningInfo } from '../../../src/crypto/CrossSigning'; + +const userId = "@alice:example.com"; + +const masterKey = new Uint8Array([ + 0xda, 0x5a, 0x27, 0x60, 0xe3, 0x3a, 0xc5, 0x82, + 0x9d, 0x12, 0xc3, 0xbe, 0xe8, 0xaa, 0xc2, 0xef, + 0xae, 0xb1, 0x05, 0xc1, 0xe7, 0x62, 0x78, 0xa6, + 0xd7, 0x1f, 0xf8, 0x2c, 0x51, 0x85, 0xf0, 0x1d, +]); +const masterKeyPub = "nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk"; + +describe("CrossSigningInfo.getCrossSigningKey()", function() { + if (!global.Olm) { + console.warn('Not running megolm backup unit tests: libolm not present'); + return; + } + + beforeAll(function() { + return global.Olm.init(); + }); + + it("Throws if no callback is provided", async () => { + const info = new CrossSigningInfo(userId); + await expect(info.getCrossSigningKey("master")).rejects.toThrow(); + }); + + it("Throws if the callback returns falsey", async () => { + const info = new CrossSigningInfo(userId, { + getCrossSigningKey: () => false + }); + await expect(info.getCrossSigningKey("master")).rejects.toThrow("falsey"); + }); + + it("Throws if the expected key doesn't come back", async () => { + const info = new CrossSigningInfo(userId, { + getCrossSigningKey: () => masterKeyPub + }); + await expect(info.getCrossSigningKey("master", "")).rejects.toThrow(); + }); + + it("Returns a key from its callback", async () => { + const info = new CrossSigningInfo(userId, { + getCrossSigningKey: () => masterKey + }); + const [ pubKey, ab ] = await info.getCrossSigningKey("master", masterKeyPub); + expect(pubKey).toEqual(masterKeyPub); + expect(ab).toEqual({a: 106712, b: 106712}); + }); +}); From 02cbd33284eab34914ade1a81f04412069cad2e8 Mon Sep 17 00:00:00 2001 From: Zoe Date: Thu, 27 Feb 2020 11:55:07 +0000 Subject: [PATCH 2/7] Added cache callbacks to CrossSigningInfo --- spec/unit/crypto/CrossSigningInfo.spec.js | 112 +++++++++++++++++++++- src/crypto/CrossSigning.js | 59 +++++++++--- 2 files changed, 155 insertions(+), 16 deletions(-) diff --git a/spec/unit/crypto/CrossSigningInfo.spec.js b/spec/unit/crypto/CrossSigningInfo.spec.js index 797478387..d565b9652 100644 --- a/spec/unit/crypto/CrossSigningInfo.spec.js +++ b/spec/unit/crypto/CrossSigningInfo.spec.js @@ -26,6 +26,10 @@ const masterKey = new Uint8Array([ 0xae, 0xb1, 0x05, 0xc1, 0xe7, 0x62, 0x78, 0xa6, 0xd7, 0x1f, 0xf8, 0x2c, 0x51, 0x85, 0xf0, 0x1d, ]); + +const badKey = Uint8Array.from(masterKey); +badKey[0] ^= 1; + const masterKeyPub = "nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk"; describe("CrossSigningInfo.getCrossSigningKey()", function() { @@ -45,24 +49,124 @@ describe("CrossSigningInfo.getCrossSigningKey()", function() { it("Throws if the callback returns falsey", async () => { const info = new CrossSigningInfo(userId, { - getCrossSigningKey: () => false + getCrossSigningKey: () => false, }); await expect(info.getCrossSigningKey("master")).rejects.toThrow("falsey"); }); it("Throws if the expected key doesn't come back", async () => { const info = new CrossSigningInfo(userId, { - getCrossSigningKey: () => masterKeyPub + getCrossSigningKey: () => masterKeyPub, }); await expect(info.getCrossSigningKey("master", "")).rejects.toThrow(); }); it("Returns a key from its callback", async () => { const info = new CrossSigningInfo(userId, { - getCrossSigningKey: () => masterKey + getCrossSigningKey: () => masterKey, }); - const [ pubKey, ab ] = await info.getCrossSigningKey("master", masterKeyPub); + const [pubKey, ab] = await info.getCrossSigningKey("master", masterKeyPub); expect(pubKey).toEqual(masterKeyPub); expect(ab).toEqual({a: 106712, b: 106712}); }); + + it("Requests a key from the cache callback (if set) and does not call app" + + " if one is found", async () => { + const getCrossSigningKey = jest.fn().mockRejectedValue( + new Error("Regular callback called"), + ); + const getCrossSigningKeyCache = jest.fn().mockResolvedValue(masterKey); + const info = new CrossSigningInfo( + userId, + { getCrossSigningKey }, + { getCrossSigningKeyCache }, + ); + const [pubKey] = await info.getCrossSigningKey("master", masterKeyPub); + expect(pubKey).toEqual(masterKeyPub); + expect(getCrossSigningKeyCache.mock.calls.length).toBe(1); + expect(getCrossSigningKeyCache.mock. calls[0][0]).toBe("master"); + }); + + it("Stores a key with the cache callback (if set)", async () => { + const getCrossSigningKey = jest.fn().mockResolvedValue(masterKey); + const storeCrossSigningKeyCache = jest.fn().mockResolvedValue(undefined); + const info = new CrossSigningInfo( + userId, + { getCrossSigningKey }, + { storeCrossSigningKeyCache }, + ); + const [pubKey] = await info.getCrossSigningKey("master", masterKeyPub); + expect(pubKey).toEqual(masterKeyPub); + expect(storeCrossSigningKeyCache.mock.calls.length).toEqual(1); + expect(storeCrossSigningKeyCache.mock.calls[0][0]).toBe("master"); + expect(storeCrossSigningKeyCache.mock.calls[0][1]).toBe(masterKey); + }); + + it("Does not store a bad key to the cache", async () => { + const getCrossSigningKey = jest.fn().mockResolvedValue(badKey); + const storeCrossSigningKeyCache = jest.fn().mockResolvedValue(undefined); + const info = new CrossSigningInfo( + userId, + { getCrossSigningKey }, + { storeCrossSigningKeyCache }, + ); + await expect(info.getCrossSigningKey("master", masterKeyPub)).rejects.toThrow(); + expect(storeCrossSigningKeyCache.mock.calls.length).toEqual(0); + }); + + it("Does not store a value to the cache if it came from the cache", async () => { + const getCrossSigningKey = jest.fn().mockRejectedValue( + new Error("Regular callback called"), + ); + const getCrossSigningKeyCache = jest.fn().mockResolvedValue(masterKey); + const storeCrossSigningKeyCache = jest.fn().mockRejectedValue( + new Error("Tried to store a value from cache"), + ); + const info = new CrossSigningInfo( + userId, + { getCrossSigningKey }, + { getCrossSigningKeyCache, storeCrossSigningKeyCache }, + ); + expect(storeCrossSigningKeyCache.mock.calls.length).toBe(0); + const [pubKey] = await info.getCrossSigningKey("master", masterKeyPub); + expect(pubKey).toEqual(masterKeyPub); + }); + + it("Requests a key from the cache callback (if set) and then calls app " + + "if one is not found", async () => { + const getCrossSigningKey = jest.fn().mockResolvedValue(masterKey); + const getCrossSigningKeyCache = jest.fn().mockResolvedValue(undefined); + const storeCrossSigningKeyCache = jest.fn(); + const info = new CrossSigningInfo( + userId, + { getCrossSigningKey }, + { getCrossSigningKeyCache, storeCrossSigningKeyCache }, + ); + const [pubKey] = await info.getCrossSigningKey("master", masterKeyPub); + expect(pubKey).toEqual(masterKeyPub); + expect(getCrossSigningKey.mock.calls.length).toBe(1); + expect(getCrossSigningKeyCache.mock.calls.length).toBe(1); + + /* Also expect that the cache gets updated */ + expect(storeCrossSigningKeyCache.mock.calls.length).toBe(1); + }); + + it("Requests a key from the cache callback (if set) and then calls app if " + + "that key doesn't match", async () => { + const getCrossSigningKey = jest.fn().mockResolvedValue(masterKey); + const getCrossSigningKeyCache = jest.fn().mockResolvedValue(badKey); + const storeCrossSigningKeyCache = jest.fn(); + const info = new CrossSigningInfo( + userId, + { getCrossSigningKey }, + { getCrossSigningKeyCache, storeCrossSigningKeyCache }, + ); + const [pubKey] = await info.getCrossSigningKey("master", masterKeyPub); + expect(pubKey).toEqual(masterKeyPub); + expect(getCrossSigningKey.mock.calls.length).toBe(1); + expect(getCrossSigningKeyCache.mock.calls.length).toBe(1); + + /* Also expect that the cache gets updated */ + expect(storeCrossSigningKeyCache.mock.calls.length).toBe(1); + }); }); diff --git a/src/crypto/CrossSigning.js b/src/crypto/CrossSigning.js index 2bc9d0f3f..c9006972e 100644 --- a/src/crypto/CrossSigning.js +++ b/src/crypto/CrossSigning.js @@ -40,8 +40,9 @@ export class CrossSigningInfo extends EventEmitter { * @param {string} userId the user that the information is about * @param {object} callbacks Callbacks used to interact with the app * Requires getCrossSigningKey and saveCrossSigningKeys + * @param {object} cacheCallbacks Callbacks used to interact with the cache */ - constructor(userId, callbacks) { + constructor(userId, callbacks, cacheCallbacks) { super(); // you can't change the userId @@ -50,6 +51,7 @@ export class CrossSigningInfo extends EventEmitter { value: userId, }); this._callbacks = callbacks || {}; + this._cacheCallbacks = cacheCallbacks || {}; this.keys = {}; this.firstUse = true; } @@ -70,22 +72,55 @@ export class CrossSigningInfo extends EventEmitter { expectedPubkey = this.getId(type); } - const privkey = await this._callbacks.getCrossSigningKey(type, expectedPubkey); + function validateKey(key) { + if (!key) return; + const signing = new global.Olm.PkSigning(); + const gotPubkey = signing.init_with_seed(key); + if (gotPubkey === expectedPubkey) { + return [gotPubkey, signing]; + } + signing.free(); + } + + let privkey; + if (this._cacheCallbacks.getCrossSigningKeyCache) { + privkey = await this._cacheCallbacks.getCrossSigningKeyCache(type, expectedPubkey); + } + + if (privkey) { + const signing = new global.Olm.PkSigning(); + const gotPubkey = signing.init_with_seed(privkey); + if (gotPubkey === expectedPubkey) { + return [gotPubkey, signing]; + } + signing.free(); + } + + privkey = await this._callbacks.getCrossSigningKey(type, expectedPubkey); + + if (privkey) { + const signing = new global.Olm.PkSigning(); + const gotPubkey = signing.init_with_seed(privkey); + if (gotPubkey === expectedPubkey) { + if (this._cacheCallbacks.storeCrossSigningKeyCache) { + await this._cacheCallbacks.storeCrossSigningKeyCache(type, privkey); + } + return [gotPubkey, signing]; + } + signing.free(); + } + + /* No keysource even returned a key */ if (!privkey) { throw new Error( "getCrossSigningKey callback for " + type + " returned falsey", ); } - const signing = new global.Olm.PkSigning(); - const gotPubkey = signing.init_with_seed(privkey); - if (gotPubkey !== expectedPubkey) { - signing.free(); - throw new Error( - "Key type " + type + " from getCrossSigningKey callback did not match", - ); - } else { - return [gotPubkey, signing]; - } + + /* We got some keys from the keysource, but none of them were valid */ + throw new Error( + "Key type " + type + " from getCrossSigningKey callback did not match", + ); } static fromStorage(obj, userId) { From c2daf0d74e75b78ade1067f36b777d648d937fba Mon Sep 17 00:00:00 2001 From: Zoe Date: Thu, 27 Feb 2020 16:28:01 +0000 Subject: [PATCH 3/7] Store data in cryptostore --- package.json | 2 + spec/unit/crypto/CrossSigningInfo.spec.js | 42 ++++++++++- .../verification/verification_request.spec.js | 1 - src/crypto/CrossSigning.js | 54 +++++++++----- src/crypto/index.js | 7 +- .../store/indexeddb-crypto-store-backend.js | 17 +++++ src/crypto/store/indexeddb-crypto-store.js | 23 ++++++ src/crypto/store/localStorage-crypto-store.js | 11 +++ yarn.lock | 70 ++++++++++++++++--- 9 files changed, 197 insertions(+), 30 deletions(-) diff --git a/package.json b/package.json index fbf21bf63..e26664b6d 100644 --- a/package.json +++ b/package.json @@ -78,7 +78,9 @@ "eslint-plugin-babel": "^5.3.0", "eslint-plugin-jest": "^23.0.4", "exorcist": "^1.0.1", + "fake-indexeddb": "^3.0.0", "jest": "^24.9.0", + "jest-localstorage-mock": "^2.4.0", "jsdoc": "^3.5.5", "matrix-mock-request": "^1.2.3", "olm": "https://packages.matrix.org/npm/olm/olm-3.1.4.tgz", diff --git a/spec/unit/crypto/CrossSigningInfo.spec.js b/spec/unit/crypto/CrossSigningInfo.spec.js index d565b9652..13417f7eb 100644 --- a/spec/unit/crypto/CrossSigningInfo.spec.js +++ b/spec/unit/crypto/CrossSigningInfo.spec.js @@ -16,7 +16,15 @@ limitations under the License. */ import '../../olm-loader'; -import { CrossSigningInfo } from '../../../src/crypto/CrossSigning'; +import { + CrossSigningInfo, + createCryptoStoreCacheCallbacks, +} from '../../../src/crypto/CrossSigning'; +import { + IndexedDBCryptoStore, +} from '../../../src/crypto/store/indexeddb-crypto-store'; +import 'fake-indexeddb/auto'; +import 'jest-localstorage-mock'; const userId = "@alice:example.com"; @@ -170,3 +178,35 @@ describe("CrossSigningInfo.getCrossSigningKey()", function() { expect(storeCrossSigningKeyCache.mock.calls.length).toBe(1); }); }); + +/* XXX/TODO: MemoryStore isn't tested + * But that's because at time of writing, MemoryStore probably never gets used ever. + */ +describe.each([ + [global.indexedDB], + [undefined], +])("CrossSigning > createCryptoStoreCacheCallbacks", function(db) { + let store; + + beforeAll(() => { + store = new IndexedDBCryptoStore(db, "tests"); + }); + + beforeEach(async () => { + await store.deleteAllData(); + }); + + it("Caches data to the store and retrieves it", async () => { + const { getCrossSigningKeyCache, storeCrossSigningKeyCache } = + createCryptoStoreCacheCallbacks(store); + await storeCrossSigningKeyCache("master", masterKey); + + // If we've not saved anything, don't expect anything + // Definitely don't accidentally return the wrong key for the type + const nokey = await getCrossSigningKeyCache("self", ""); + expect(nokey).toBeNull(); + + const key = await getCrossSigningKeyCache("master", ""); + expect(key).toEqual(masterKey); + }); +}); diff --git a/spec/unit/crypto/verification/verification_request.spec.js b/spec/unit/crypto/verification/verification_request.spec.js index 544011c5a..ac9b0f8df 100644 --- a/spec/unit/crypto/verification/verification_request.spec.js +++ b/spec/unit/crypto/verification/verification_request.spec.js @@ -120,7 +120,6 @@ async function distributeEvent(ownRequest, theirRequest, event) { } describe("verification request unit tests", function() { - beforeAll(function() { setupWebcrypto(); }); diff --git a/src/crypto/CrossSigning.js b/src/crypto/CrossSigning.js index c9006972e..0a6a0d65b 100644 --- a/src/crypto/CrossSigning.js +++ b/src/crypto/CrossSigning.js @@ -23,6 +23,7 @@ limitations under the License. import {decodeBase64, encodeBase64, pkSign, pkVerify} from './olmlib'; import {EventEmitter} from 'events'; import {logger} from '../logger'; +import {IndexedDBCryptoStore} from '../crypto/store/indexeddb-crypto-store'; function publicKeyFromKeyInfo(keyInfo) { // `keys` is an object with { [`ed25519:${pubKey}`]: pubKey } @@ -84,30 +85,22 @@ export class CrossSigningInfo extends EventEmitter { let privkey; if (this._cacheCallbacks.getCrossSigningKeyCache) { - privkey = await this._cacheCallbacks.getCrossSigningKeyCache(type, expectedPubkey); + privkey = await this._cacheCallbacks + .getCrossSigningKeyCache(type, expectedPubkey); } - if (privkey) { - const signing = new global.Olm.PkSigning(); - const gotPubkey = signing.init_with_seed(privkey); - if (gotPubkey === expectedPubkey) { - return [gotPubkey, signing]; - } - signing.free(); + const cacheresult = validateKey(privkey); + if (cacheresult) { + return cacheresult; } privkey = await this._callbacks.getCrossSigningKey(type, expectedPubkey); - - if (privkey) { - const signing = new global.Olm.PkSigning(); - const gotPubkey = signing.init_with_seed(privkey); - if (gotPubkey === expectedPubkey) { - if (this._cacheCallbacks.storeCrossSigningKeyCache) { - await this._cacheCallbacks.storeCrossSigningKeyCache(type, privkey); - } - return [gotPubkey, signing]; + const result = validateKey(privkey); + if (result) { + if (this._cacheCallbacks.storeCrossSigningKeyCache) { + await this._cacheCallbacks.storeCrossSigningKeyCache(type, privkey); } - signing.free(); + return result; } /* No keysource even returned a key */ @@ -574,3 +567,28 @@ export class DeviceTrustLevel { return this._tofu; } } + +export function createCryptoStoreCacheCallbacks(store) { + return { + getCrossSigningKeyCache: function(type, _expectedPublicKey) { + return new Promise((resolve) => { + return store.doTxn( + 'readonly', + [IndexedDBCryptoStore.STORE_ACCOUNT], + (txn) => { + store.getCrossSigningPrivateKey(txn, resolve, type); + }, + ); + }); + }, + storeCrossSigningKeyCache: function(type, key) { + return store.doTxn( + 'readwrite', + [IndexedDBCryptoStore.STORE_ACCOUNT], + (txn) => { + store.storeCrossSigningPrivateKey(txn, type, key); + }, + ); + }, + }; +} diff --git a/src/crypto/index.js b/src/crypto/index.js index b59cdf895..b86f7ac3b 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -220,8 +220,13 @@ export function Crypto(baseApis, sessionStore, userId, deviceId, this._inRoomVerificationRequests = new InRoomRequests(); const cryptoCallbacks = this._baseApis._cryptoCallbacks || {}; + const cacheCallbacks = {}; - this._crossSigningInfo = new CrossSigningInfo(userId, cryptoCallbacks); + this._crossSigningInfo = new CrossSigningInfo( + userId, + cryptoCallbacks, + cacheCallbacks, + ); this._secretStorage = new SecretStorage( baseApis, cryptoCallbacks, this._crossSigningInfo, diff --git a/src/crypto/store/indexeddb-crypto-store-backend.js b/src/crypto/store/indexeddb-crypto-store-backend.js index f42874720..0951ecb75 100644 --- a/src/crypto/store/indexeddb-crypto-store-backend.js +++ b/src/crypto/store/indexeddb-crypto-store-backend.js @@ -341,11 +341,28 @@ export class Backend { }; } + getCrossSigningPrivateKey(txn, func, type) { + const objectStore = txn.objectStore("account"); + const getReq = objectStore.get(`ssss_cache:${type}`); + getReq.onsuccess = function() { + try { + func(getReq.result || null); + } catch (e) { + abortWithException(txn, e); + } + }; + } + storeCrossSigningKeys(txn, keys) { const objectStore = txn.objectStore("account"); objectStore.put(keys, "crossSigningKeys"); } + storeCrossSigningPrivateKey(txn, type, key) { + const objectStore = txn.objectStore("account"); + objectStore.put(key, `ssss_cache:${type}`); + } + // Olm Sessions countEndToEndSessions(txn, func) { diff --git a/src/crypto/store/indexeddb-crypto-store.js b/src/crypto/store/indexeddb-crypto-store.js index fac67baf6..66def5fc4 100644 --- a/src/crypto/store/indexeddb-crypto-store.js +++ b/src/crypto/store/indexeddb-crypto-store.js @@ -321,6 +321,16 @@ export class IndexedDBCryptoStore { }); } + /** + * @param {*} txn An active transaction. See doTxn(). + * @param {function(string)} func Called with the private key + * @param {string} type A key type + */ + async getCrossSigningPrivateKey(txn, func, type) { + const backend = await this._connect(); + return backend.getCrossSigningPrivateKey(txn, func, type); + } + /** * Write the cross-signing keys back to the store * @@ -333,6 +343,19 @@ export class IndexedDBCryptoStore { }); } + /** + * Write the cross-signing private keys back to the store + * + * @param {*} txn An active transaction. See doTxn(). + * @param {string} type The type of cross-signing private key to store + * @param {string} key keys object as getCrossSigningKeys() + */ + storeCrossSigningPrivateKey(txn, type, key) { + this._backendPromise.then(backend => { + backend.storeCrossSigningPrivateKey(txn, type, key); + }); + } + // Olm sessions /** diff --git a/src/crypto/store/localStorage-crypto-store.js b/src/crypto/store/localStorage-crypto-store.js index 2e7d3cbd3..3a871aab9 100644 --- a/src/crypto/store/localStorage-crypto-store.js +++ b/src/crypto/store/localStorage-crypto-store.js @@ -367,12 +367,23 @@ export class LocalStorageCryptoStore extends MemoryCryptoStore { func(keys); } + getCrossSigningPrivateKey(txn, func, type) { + const key = getJsonItem(this.store, E2E_PREFIX + `ssss_cache.${type}`); + func(key ? Uint8Array.from(key) : key); + } + storeCrossSigningKeys(txn, keys) { setJsonItem( this.store, KEY_CROSS_SIGNING_KEYS, keys, ); } + storeCrossSigningPrivateKey(txn, type, key) { + setJsonItem( + this.store, E2E_PREFIX + `ssss_cache.${type}`, Array.from(key), + ); + } + doTxn(mode, stores, func) { return Promise.resolve(func(null)); } diff --git a/yarn.lock b/yarn.lock index 1f176b304..86a11623e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1463,6 +1463,11 @@ base-x@^3.0.2: dependencies: safe-buffer "^5.0.1" +base64-arraybuffer-es6@0.5.0: + version "0.5.0" + resolved "https://registry.yarnpkg.com/base64-arraybuffer-es6/-/base64-arraybuffer-es6-0.5.0.tgz#27877d01148bcfb3919c17ecf64ea163d9bdba62" + integrity sha512-UCIPaDJrNNj5jG2ZL+nzJ7czvZV/ZYX6LaIRgfVU1k1edJOQg7dkbiSKzwHkNp6aHEHER/PhlFBrMYnlvJJQEw== + base64-js@^1.0.2: version "1.3.1" resolved "https://registry.yarnpkg.com/base64-js/-/base64-js-1.3.1.tgz#58ece8cb75dd07e71ed08c736abc5fac4dbf8df1" @@ -2059,7 +2064,7 @@ core-js-compat@^3.6.2: browserslist "^4.8.3" semver "7.0.0" -core-js@^2.4.0, core-js@^2.6.5: +core-js@^2.4.0, core-js@^2.5.3, core-js@^2.6.5: version "2.6.11" resolved "https://registry.yarnpkg.com/core-js/-/core-js-2.6.11.tgz#38831469f9922bded8ee21c9dc46985e0399308c" integrity sha512-5wjnpaT/3dV+XB4borEsnAYQchn00XSgTAWKDkEqv+K8KevjbzmofK6hfJ9TZIlpj2N0xQpazy7PiRQiWHqzWg== @@ -2728,6 +2733,19 @@ fast-deep-equal@^3.1.1: resolved "https://registry.yarnpkg.com/fast-deep-equal/-/fast-deep-equal-3.1.1.tgz#545145077c501491e33b15ec408c294376e94ae4" integrity sha512-8UEa58QDLauDNfpbrX55Q9jrGHThw2ZMdOky5Gl1CDtVeJDPVrG4Jxx1N8jw2gkWaff5UUuX1KJd+9zGe2B+ZA== +fake-indexeddb@^3.0.0: + version "3.0.0" + resolved "https://registry.yarnpkg.com/fake-indexeddb/-/fake-indexeddb-3.0.0.tgz#1bd0ffce41b0f433409df301d334d8fd7d77da27" + integrity sha512-VrnV9dJWlVWvd8hp9MMR+JS4RLC4ZmToSkuCg91ZwpYE5mSODb3n5VEaV62Hf3AusnbrPfwQhukU+rGZm5W8PQ== + dependencies: + realistic-structured-clone "^2.0.1" + setimmediate "^1.0.5" + +fast-deep-equal@^2.0.1: + version "2.0.1" + resolved "https://registry.yarnpkg.com/fast-deep-equal/-/fast-deep-equal-2.0.1.tgz#7b05218ddf9667bf7f370bf7fdb2cb15fdd0aa49" + integrity sha1-ewUhjd+WZ79/Nwv3/bLLFf3Qqkk= + fast-json-stable-stringify@^2.0.0: version "2.1.0" resolved "https://registry.yarnpkg.com/fast-json-stable-stringify/-/fast-json-stable-stringify-2.1.0.tgz#874bf69c6f404c2b5d99c481341399fd55892633" @@ -3725,6 +3743,11 @@ jest-leak-detector@^24.9.0: jest-get-type "^24.9.0" pretty-format "^24.9.0" +jest-localstorage-mock@^2.4.0: + version "2.4.0" + resolved "https://registry.yarnpkg.com/jest-localstorage-mock/-/jest-localstorage-mock-2.4.0.tgz#c6073810735dd3af74020ea6c3885ec1cc6d0d13" + integrity sha512-/mC1JxnMeuIlAaQBsDMilskC/x/BicsQ/BXQxEOw+5b1aGZkkOAqAF3nu8yq449CpzGtp5jJ5wCmDNxLgA2m6A== + jest-matcher-utils@^24.9.0: version "24.9.0" resolved "https://registry.yarnpkg.com/jest-matcher-utils/-/jest-matcher-utils-24.9.0.tgz#f5b3661d5e628dffe6dd65251dfdae0e87c3a073" @@ -5283,6 +5306,16 @@ readdirp@^2.2.1: micromatch "^3.1.10" readable-stream "^2.0.2" +realistic-structured-clone@^2.0.1: + version "2.0.2" + resolved "https://registry.yarnpkg.com/realistic-structured-clone/-/realistic-structured-clone-2.0.2.tgz#2f8ec225b1f9af20efc79ac96a09043704414959" + integrity sha512-5IEvyfuMJ4tjQOuKKTFNvd+H9GSbE87IcendSBannE28PTrbolgaVg5DdEApRKhtze794iXqVUFKV60GLCNKEg== + dependencies: + core-js "^2.5.3" + domexception "^1.0.1" + typeson "^5.8.2" + typeson-registry "^1.0.0-alpha.20" + realpath-native@^1.1.0: version "1.1.0" resolved "https://registry.yarnpkg.com/realpath-native/-/realpath-native-1.1.0.tgz#2003294fea23fb0672f2476ebe22fcf498a2d65c" @@ -5614,6 +5647,11 @@ set-value@^2.0.0, set-value@^2.0.1: is-plain-object "^2.0.3" split-string "^3.0.1" +setimmediate@^1.0.5: + version "1.0.5" + resolved "https://registry.yarnpkg.com/setimmediate/-/setimmediate-1.0.5.tgz#290cbb232e306942d7d7ea9b83732ab7856f8285" + integrity sha1-KQy7Iy4waULX1+qbg3Mqt4VvgoU= + sha.js@^2.4.0, sha.js@^2.4.8, sha.js@~2.4.4: version "2.4.11" resolved "https://registry.yarnpkg.com/sha.js/-/sha.js-2.4.11.tgz#37a5cf0b81ecbc6943de109ba2960d1b26584ae7" @@ -6255,6 +6293,20 @@ typescript@^3.2.2, typescript@^3.7.3: resolved "https://registry.yarnpkg.com/typescript/-/typescript-3.8.2.tgz#91d6868aaead7da74f493c553aeff76c0c0b1d5a" integrity sha512-EgOVgL/4xfVrCMbhYKUQTdF37SQn4Iw73H5BgCrF1Abdun7Kwy/QZsE/ssAy0y4LxBbvua3PIbFsbRczWWnDdQ== +typeson-registry@^1.0.0-alpha.20: + version "1.0.0-alpha.35" + resolved "https://registry.yarnpkg.com/typeson-registry/-/typeson-registry-1.0.0-alpha.35.tgz#b86abfe440e6ee69102eebb0e8c5a916dd182ff9" + integrity sha512-a/NffrpFswBTyU6w2d6vjk62K1TZ45H64af9AfRbn7LXqNEfL+h+gw3OV2IaG+enfwqgLB5WmbkrNBaQuc/97A== + dependencies: + base64-arraybuffer-es6 "0.5.0" + typeson "5.18.2" + whatwg-url "7.1.0" + +typeson@5.18.2, typeson@^5.8.2: + version "5.18.2" + resolved "https://registry.yarnpkg.com/typeson/-/typeson-5.18.2.tgz#0d217fc0e11184a66aa7ca0076d9aa7707eb7bc2" + integrity sha512-Vetd+OGX05P4qHyHiSLdHZ5Z5GuQDrHHwSdjkqho9NSCYVSLSfRMjklD/unpHH8tXBR9Z/R05rwJSuMpMFrdsw== + uc.micro@^1.0.1, uc.micro@^1.0.5: version "1.0.6" resolved "https://registry.yarnpkg.com/uc.micro/-/uc.micro-1.0.6.tgz#9c411a802a409a91fc6cf74081baba34b24499ac" @@ -6495,19 +6547,19 @@ whatwg-mimetype@^2.1.0, whatwg-mimetype@^2.2.0: resolved "https://registry.yarnpkg.com/whatwg-mimetype/-/whatwg-mimetype-2.3.0.tgz#3d4b1e0312d2079879f826aff18dbeeca5960fbf" integrity sha512-M4yMwr6mAnQz76TbJm914+gPpB/nCwvZbJU28cUD6dR004SAxDLOOSUaB1JDRqLtaOV/vi0IC5lEAGFgrjGv/g== -whatwg-url@^6.4.1: - version "6.5.0" - resolved "https://registry.yarnpkg.com/whatwg-url/-/whatwg-url-6.5.0.tgz#f2df02bff176fd65070df74ad5ccbb5a199965a8" - integrity sha512-rhRZRqx/TLJQWUpQ6bmrt2UV4f0HCQ463yQuONJqC6fO2VoEb1pTYddbe59SkYq87aoM5A3bdhMZiUiVws+fzQ== +whatwg-url@7.1.0, whatwg-url@^7.0.0: + version "7.1.0" + resolved "https://registry.yarnpkg.com/whatwg-url/-/whatwg-url-7.1.0.tgz#c2c492f1eca612988efd3d2266be1b9fc6170d06" + integrity sha512-WUu7Rg1DroM7oQvGWfOiAK21n74Gg+T4elXEQYkOhtyLeWiJFoOGLXPKI/9gzIie9CtwVLm8wtw6YJdKyxSjeg== dependencies: lodash.sortby "^4.7.0" tr46 "^1.0.1" webidl-conversions "^4.0.2" -whatwg-url@^7.0.0: - version "7.1.0" - resolved "https://registry.yarnpkg.com/whatwg-url/-/whatwg-url-7.1.0.tgz#c2c492f1eca612988efd3d2266be1b9fc6170d06" - integrity sha512-WUu7Rg1DroM7oQvGWfOiAK21n74Gg+T4elXEQYkOhtyLeWiJFoOGLXPKI/9gzIie9CtwVLm8wtw6YJdKyxSjeg== +whatwg-url@^6.4.1: + version "6.5.0" + resolved "https://registry.yarnpkg.com/whatwg-url/-/whatwg-url-6.5.0.tgz#f2df02bff176fd65070df74ad5ccbb5a199965a8" + integrity sha512-rhRZRqx/TLJQWUpQ6bmrt2UV4f0HCQ463yQuONJqC6fO2VoEb1pTYddbe59SkYq87aoM5A3bdhMZiUiVws+fzQ== dependencies: lodash.sortby "^4.7.0" tr46 "^1.0.1" From 25c467d6082316535d6c154dd072c87f89821cb8 Mon Sep 17 00:00:00 2001 From: Zoe Date: Thu, 27 Feb 2020 16:36:18 +0000 Subject: [PATCH 4/7] Wire cache through to matrix client --- src/crypto/index.js | 3 ++- src/crypto/store/memory-crypto-store.js | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/crypto/index.js b/src/crypto/index.js index b86f7ac3b..25706b88b 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -37,6 +37,7 @@ import { CrossSigningLevel, DeviceTrustLevel, UserTrustLevel, + createCryptoStoreCacheCallbacks, } from './CrossSigning'; import {SECRET_STORAGE_ALGORITHM_V1, SecretStorage} from './SecretStorage'; import {OutgoingRoomKeyRequestManager} from './OutgoingRoomKeyRequestManager'; @@ -220,7 +221,7 @@ export function Crypto(baseApis, sessionStore, userId, deviceId, this._inRoomVerificationRequests = new InRoomRequests(); const cryptoCallbacks = this._baseApis._cryptoCallbacks || {}; - const cacheCallbacks = {}; + const cacheCallbacks = createCryptoStoreCacheCallbacks(cryptoStore); this._crossSigningInfo = new CrossSigningInfo( userId, diff --git a/src/crypto/store/memory-crypto-store.js b/src/crypto/store/memory-crypto-store.js index a7ea243dd..89e2e499a 100644 --- a/src/crypto/store/memory-crypto-store.js +++ b/src/crypto/store/memory-crypto-store.js @@ -243,10 +243,22 @@ export class MemoryCryptoStore { func(this._crossSigningKeys); } + // XXX cache not implemented: I think this is only used in tests + // and I have IndexedDb tests for this. + getCrossSigningPrivateKey(txn, func) { + return func(null); + } + storeCrossSigningKeys(txn, keys) { this._crossSigningKeys = keys; } + // XXX cache not implemented: I think this is only used in tests + // and I have IndexedDb tests for this. + storeCrossSigningPrivateKey() { + return; + } + // Olm Sessions countEndToEndSessions(txn, func) { From a94503ad030462a3f2aa86165055f6817d149678 Mon Sep 17 00:00:00 2001 From: Zoe Date: Fri, 28 Feb 2020 10:43:57 +0000 Subject: [PATCH 5/7] address PR feedback --- spec/unit/crypto/CrossSigningInfo.spec.js | 73 +++++++++++++---------- src/crypto/store/memory-crypto-store.js | 14 ++--- 2 files changed, 47 insertions(+), 40 deletions(-) diff --git a/spec/unit/crypto/CrossSigningInfo.spec.js b/spec/unit/crypto/CrossSigningInfo.spec.js index 13417f7eb..687e44197 100644 --- a/spec/unit/crypto/CrossSigningInfo.spec.js +++ b/spec/unit/crypto/CrossSigningInfo.spec.js @@ -1,5 +1,4 @@ /* -Copyright 2020 New Vector Ltd Copyright 2020 The Matrix.org Foundation C.I.C. Licensed under the Apache License, Version 2.0 (the "License"); @@ -23,24 +22,26 @@ import { import { IndexedDBCryptoStore, } from '../../../src/crypto/store/indexeddb-crypto-store'; +import {MemoryCryptoStore} from '../../../src/crypto/store/memory-crypto-store'; import 'fake-indexeddb/auto'; import 'jest-localstorage-mock'; const userId = "@alice:example.com"; -const masterKey = new Uint8Array([ +// Private key for tests only +const testKey = new Uint8Array([ 0xda, 0x5a, 0x27, 0x60, 0xe3, 0x3a, 0xc5, 0x82, 0x9d, 0x12, 0xc3, 0xbe, 0xe8, 0xaa, 0xc2, 0xef, 0xae, 0xb1, 0x05, 0xc1, 0xe7, 0x62, 0x78, 0xa6, 0xd7, 0x1f, 0xf8, 0x2c, 0x51, 0x85, 0xf0, 0x1d, ]); -const badKey = Uint8Array.from(masterKey); +const badKey = Uint8Array.from(testKey); badKey[0] ^= 1; const masterKeyPub = "nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk"; -describe("CrossSigningInfo.getCrossSigningKey()", function() { +describe("CrossSigningInfo.getCrossSigningKey", function() { if (!global.Olm) { console.warn('Not running megolm backup unit tests: libolm not present'); return; @@ -50,40 +51,40 @@ describe("CrossSigningInfo.getCrossSigningKey()", function() { return global.Olm.init(); }); - it("Throws if no callback is provided", async () => { + it("should throw if no callback is provided", async () => { const info = new CrossSigningInfo(userId); await expect(info.getCrossSigningKey("master")).rejects.toThrow(); }); - it("Throws if the callback returns falsey", async () => { + it("should throw if the callback returns falsey", async () => { const info = new CrossSigningInfo(userId, { getCrossSigningKey: () => false, }); await expect(info.getCrossSigningKey("master")).rejects.toThrow("falsey"); }); - it("Throws if the expected key doesn't come back", async () => { + it("should throw if the expected key doesn't come back", async () => { const info = new CrossSigningInfo(userId, { getCrossSigningKey: () => masterKeyPub, }); await expect(info.getCrossSigningKey("master", "")).rejects.toThrow(); }); - it("Returns a key from its callback", async () => { + it("should return a key from its callback", async () => { const info = new CrossSigningInfo(userId, { - getCrossSigningKey: () => masterKey, + getCrossSigningKey: () => testKey, }); const [pubKey, ab] = await info.getCrossSigningKey("master", masterKeyPub); expect(pubKey).toEqual(masterKeyPub); expect(ab).toEqual({a: 106712, b: 106712}); }); - it("Requests a key from the cache callback (if set) and does not call app" + + it("should request a key from the cache callback (if set) and does not call app" + " if one is found", async () => { const getCrossSigningKey = jest.fn().mockRejectedValue( new Error("Regular callback called"), ); - const getCrossSigningKeyCache = jest.fn().mockResolvedValue(masterKey); + const getCrossSigningKeyCache = jest.fn().mockResolvedValue(testKey); const info = new CrossSigningInfo( userId, { getCrossSigningKey }, @@ -95,8 +96,8 @@ describe("CrossSigningInfo.getCrossSigningKey()", function() { expect(getCrossSigningKeyCache.mock. calls[0][0]).toBe("master"); }); - it("Stores a key with the cache callback (if set)", async () => { - const getCrossSigningKey = jest.fn().mockResolvedValue(masterKey); + it("should store a key with the cache callback (if set)", async () => { + const getCrossSigningKey = jest.fn().mockResolvedValue(testKey); const storeCrossSigningKeyCache = jest.fn().mockResolvedValue(undefined); const info = new CrossSigningInfo( userId, @@ -107,10 +108,10 @@ describe("CrossSigningInfo.getCrossSigningKey()", function() { expect(pubKey).toEqual(masterKeyPub); expect(storeCrossSigningKeyCache.mock.calls.length).toEqual(1); expect(storeCrossSigningKeyCache.mock.calls[0][0]).toBe("master"); - expect(storeCrossSigningKeyCache.mock.calls[0][1]).toBe(masterKey); + expect(storeCrossSigningKeyCache.mock.calls[0][1]).toBe(testKey); }); - it("Does not store a bad key to the cache", async () => { + it("does not store a bad key to the cache", async () => { const getCrossSigningKey = jest.fn().mockResolvedValue(badKey); const storeCrossSigningKeyCache = jest.fn().mockResolvedValue(undefined); const info = new CrossSigningInfo( @@ -122,11 +123,11 @@ describe("CrossSigningInfo.getCrossSigningKey()", function() { expect(storeCrossSigningKeyCache.mock.calls.length).toEqual(0); }); - it("Does not store a value to the cache if it came from the cache", async () => { + it("does not store a value to the cache if it came from the cache", async () => { const getCrossSigningKey = jest.fn().mockRejectedValue( new Error("Regular callback called"), ); - const getCrossSigningKeyCache = jest.fn().mockResolvedValue(masterKey); + const getCrossSigningKeyCache = jest.fn().mockResolvedValue(testKey); const storeCrossSigningKeyCache = jest.fn().mockRejectedValue( new Error("Tried to store a value from cache"), ); @@ -140,9 +141,9 @@ describe("CrossSigningInfo.getCrossSigningKey()", function() { expect(pubKey).toEqual(masterKeyPub); }); - it("Requests a key from the cache callback (if set) and then calls app " + - "if one is not found", async () => { - const getCrossSigningKey = jest.fn().mockResolvedValue(masterKey); + it("requests a key from the cache callback (if set) and then calls app " + + "if one is not found", async () => { + const getCrossSigningKey = jest.fn().mockResolvedValue(testKey); const getCrossSigningKeyCache = jest.fn().mockResolvedValue(undefined); const storeCrossSigningKeyCache = jest.fn(); const info = new CrossSigningInfo( @@ -159,9 +160,9 @@ describe("CrossSigningInfo.getCrossSigningKey()", function() { expect(storeCrossSigningKeyCache.mock.calls.length).toBe(1); }); - it("Requests a key from the cache callback (if set) and then calls app if " + - "that key doesn't match", async () => { - const getCrossSigningKey = jest.fn().mockResolvedValue(masterKey); + it("requests a key from the cache callback (if set) and then calls app if " + + "that key doesn't match", async () => { + const getCrossSigningKey = jest.fn().mockResolvedValue(testKey); const getCrossSigningKeyCache = jest.fn().mockResolvedValue(badKey); const storeCrossSigningKeyCache = jest.fn(); const info = new CrossSigningInfo( @@ -179,27 +180,35 @@ describe("CrossSigningInfo.getCrossSigningKey()", function() { }); }); -/* XXX/TODO: MemoryStore isn't tested - * But that's because at time of writing, MemoryStore probably never gets used ever. +/* + * Note that MemoryStore is weird. It's only used for testing - as far as I can tell, + * it's not possible to get one in normal execution unless you hack as we do here. */ describe.each([ - [global.indexedDB], - [undefined], -])("CrossSigning > createCryptoStoreCacheCallbacks", function(db) { + ["IndexedDBCryptoStore", + () => new IndexedDBCryptoStore(global.indexedDB, "tests")], + ["LocalStorageCryptoStore", + () => new IndexedDBCryptoStore(undefined, "tests")], + ["MemoryCryptoStore", () => { + const store = new IndexedDBCryptoStore(undefined, "tests"); + store._backendPromise = Promise.resolve(new MemoryCryptoStore()); + return store; + }], +])("CrossSigning > createCryptoStoreCacheCallbacks [%s]", function(name, dbFactory) { let store; beforeAll(() => { - store = new IndexedDBCryptoStore(db, "tests"); + store = dbFactory(); }); beforeEach(async () => { await store.deleteAllData(); }); - it("Caches data to the store and retrieves it", async () => { + it("should cache data to the store and retrieves it", async () => { const { getCrossSigningKeyCache, storeCrossSigningKeyCache } = createCryptoStoreCacheCallbacks(store); - await storeCrossSigningKeyCache("master", masterKey); + await storeCrossSigningKeyCache("master", testKey); // If we've not saved anything, don't expect anything // Definitely don't accidentally return the wrong key for the type @@ -207,6 +216,6 @@ describe.each([ expect(nokey).toBeNull(); const key = await getCrossSigningKeyCache("master", ""); - expect(key).toEqual(masterKey); + expect(key).toEqual(testKey); }); }); diff --git a/src/crypto/store/memory-crypto-store.js b/src/crypto/store/memory-crypto-store.js index 89e2e499a..ee9534339 100644 --- a/src/crypto/store/memory-crypto-store.js +++ b/src/crypto/store/memory-crypto-store.js @@ -33,6 +33,7 @@ export class MemoryCryptoStore { this._outgoingRoomKeyRequests = []; this._account = null; this._crossSigningKeys = null; + this._privateKeys = {}; // Map of {devicekey -> {sessionId -> session pickle}} this._sessions = {}; @@ -243,20 +244,17 @@ export class MemoryCryptoStore { func(this._crossSigningKeys); } - // XXX cache not implemented: I think this is only used in tests - // and I have IndexedDb tests for this. - getCrossSigningPrivateKey(txn, func) { - return func(null); + getCrossSigningPrivateKey(txn, func, type) { + const result = this._privateKeys[type]; + return func(result || null); } storeCrossSigningKeys(txn, keys) { this._crossSigningKeys = keys; } - // XXX cache not implemented: I think this is only used in tests - // and I have IndexedDb tests for this. - storeCrossSigningPrivateKey() { - return; + storeCrossSigningPrivateKey(txn, type, key) { + this._privateKeys[type] = key; } // Olm Sessions From 67fe4e1460eb57236ee8dfcfddc9e76d91c209b1 Mon Sep 17 00:00:00 2001 From: Zoe Date: Fri, 28 Feb 2020 11:04:28 +0000 Subject: [PATCH 6/7] lint & only cache valid keys --- spec/unit/crypto/CrossSigningInfo.spec.js | 91 +++++++++++++++-------- src/crypto/CrossSigning.js | 6 +- 2 files changed, 62 insertions(+), 35 deletions(-) diff --git a/spec/unit/crypto/CrossSigningInfo.spec.js b/spec/unit/crypto/CrossSigningInfo.spec.js index 687e44197..f956b5505 100644 --- a/spec/unit/crypto/CrossSigningInfo.spec.js +++ b/spec/unit/crypto/CrossSigningInfo.spec.js @@ -36,6 +36,13 @@ const testKey = new Uint8Array([ 0xd7, 0x1f, 0xf8, 0x2c, 0x51, 0x85, 0xf0, 0x1d, ]); +const types = [ + { type: "master", shouldCache: false }, + { type: "self_signing", shouldCache: true }, + { type: "user_signing", shouldCache: true }, + { type: "invalid", shouldCache: false }, +]; + const badKey = Uint8Array.from(testKey); badKey[0] ^= 1; @@ -56,11 +63,12 @@ describe("CrossSigningInfo.getCrossSigningKey", function() { await expect(info.getCrossSigningKey("master")).rejects.toThrow(); }); - it("should throw if the callback returns falsey", async () => { + it.each(types)("should throw if the callback returns falsey", + async ({type, shouldCache}) => { const info = new CrossSigningInfo(userId, { getCrossSigningKey: () => false, }); - await expect(info.getCrossSigningKey("master")).rejects.toThrow("falsey"); + await expect(info.getCrossSigningKey(type)).rejects.toThrow("falsey"); }); it("should throw if the expected key doesn't come back", async () => { @@ -79,24 +87,33 @@ describe("CrossSigningInfo.getCrossSigningKey", function() { expect(ab).toEqual({a: 106712, b: 106712}); }); - it("should request a key from the cache callback (if set) and does not call app" + - " if one is found", async () => { - const getCrossSigningKey = jest.fn().mockRejectedValue( - new Error("Regular callback called"), - ); + it.each(types)("should request a key from the cache callback (if set)" + + " and does not call app if one is found" + + `%o`, + async ({ type, shouldCache }) => { + const getCrossSigningKey = jest.fn().mockImplementation(() => { + if (shouldCache) { + return Promise.reject(new Error("Regular callback called")); + } else { + return Promise.resolve(testKey); + } + }); const getCrossSigningKeyCache = jest.fn().mockResolvedValue(testKey); const info = new CrossSigningInfo( userId, { getCrossSigningKey }, { getCrossSigningKeyCache }, ); - const [pubKey] = await info.getCrossSigningKey("master", masterKeyPub); + const [pubKey] = await info.getCrossSigningKey(type, masterKeyPub); expect(pubKey).toEqual(masterKeyPub); - expect(getCrossSigningKeyCache.mock.calls.length).toBe(1); - expect(getCrossSigningKeyCache.mock. calls[0][0]).toBe("master"); + expect(getCrossSigningKeyCache.mock.calls.length).toBe(shouldCache ? 1 : 0); + if (shouldCache) { + expect(getCrossSigningKeyCache.mock.calls[0][0]).toBe(type); + } }); - it("should store a key with the cache callback (if set)", async () => { + it.each(types)("should store a key with the cache callback (if set)", + async ({ type, shouldCache }) => { const getCrossSigningKey = jest.fn().mockResolvedValue(testKey); const storeCrossSigningKeyCache = jest.fn().mockResolvedValue(undefined); const info = new CrossSigningInfo( @@ -104,14 +121,17 @@ describe("CrossSigningInfo.getCrossSigningKey", function() { { getCrossSigningKey }, { storeCrossSigningKeyCache }, ); - const [pubKey] = await info.getCrossSigningKey("master", masterKeyPub); + const [pubKey] = await info.getCrossSigningKey(type, masterKeyPub); expect(pubKey).toEqual(masterKeyPub); - expect(storeCrossSigningKeyCache.mock.calls.length).toEqual(1); - expect(storeCrossSigningKeyCache.mock.calls[0][0]).toBe("master"); - expect(storeCrossSigningKeyCache.mock.calls[0][1]).toBe(testKey); + expect(storeCrossSigningKeyCache.mock.calls.length).toEqual(shouldCache ? 1 : 0); + if (shouldCache) { + expect(storeCrossSigningKeyCache.mock.calls[0][0]).toBe(type); + expect(storeCrossSigningKeyCache.mock.calls[0][1]).toBe(testKey); + } }); - it("does not store a bad key to the cache", async () => { + it.each(types)("does not store a bad key to the cache", + async ({ type, shouldCache }) => { const getCrossSigningKey = jest.fn().mockResolvedValue(badKey); const storeCrossSigningKeyCache = jest.fn().mockResolvedValue(undefined); const info = new CrossSigningInfo( @@ -119,14 +139,19 @@ describe("CrossSigningInfo.getCrossSigningKey", function() { { getCrossSigningKey }, { storeCrossSigningKeyCache }, ); - await expect(info.getCrossSigningKey("master", masterKeyPub)).rejects.toThrow(); + await expect(info.getCrossSigningKey(type, masterKeyPub)).rejects.toThrow(); expect(storeCrossSigningKeyCache.mock.calls.length).toEqual(0); }); - it("does not store a value to the cache if it came from the cache", async () => { - const getCrossSigningKey = jest.fn().mockRejectedValue( - new Error("Regular callback called"), - ); + it.each(types)("does not store a value to the cache if it came from the cache", + async ({ type, shouldCache }) => { + const getCrossSigningKey = jest.fn().mockImplementation(() => { + if (shouldCache) { + return Promise.reject(new Error("Regular callback called")); + } else { + return Promise.resolve(testKey); + } + }); const getCrossSigningKeyCache = jest.fn().mockResolvedValue(testKey); const storeCrossSigningKeyCache = jest.fn().mockRejectedValue( new Error("Tried to store a value from cache"), @@ -137,12 +162,12 @@ describe("CrossSigningInfo.getCrossSigningKey", function() { { getCrossSigningKeyCache, storeCrossSigningKeyCache }, ); expect(storeCrossSigningKeyCache.mock.calls.length).toBe(0); - const [pubKey] = await info.getCrossSigningKey("master", masterKeyPub); + const [pubKey] = await info.getCrossSigningKey(type, masterKeyPub); expect(pubKey).toEqual(masterKeyPub); }); - it("requests a key from the cache callback (if set) and then calls app " + - "if one is not found", async () => { + it.each(types)("requests a key from the cache callback (if set) and then calls app" + + " if one is not found", async ({ type, shouldCache }) => { const getCrossSigningKey = jest.fn().mockResolvedValue(testKey); const getCrossSigningKeyCache = jest.fn().mockResolvedValue(undefined); const storeCrossSigningKeyCache = jest.fn(); @@ -151,17 +176,17 @@ describe("CrossSigningInfo.getCrossSigningKey", function() { { getCrossSigningKey }, { getCrossSigningKeyCache, storeCrossSigningKeyCache }, ); - const [pubKey] = await info.getCrossSigningKey("master", masterKeyPub); + const [pubKey] = await info.getCrossSigningKey(type, masterKeyPub); expect(pubKey).toEqual(masterKeyPub); expect(getCrossSigningKey.mock.calls.length).toBe(1); - expect(getCrossSigningKeyCache.mock.calls.length).toBe(1); + expect(getCrossSigningKeyCache.mock.calls.length).toBe(shouldCache ? 1 : 0); /* Also expect that the cache gets updated */ - expect(storeCrossSigningKeyCache.mock.calls.length).toBe(1); + expect(storeCrossSigningKeyCache.mock.calls.length).toBe(shouldCache ? 1 : 0); }); - it("requests a key from the cache callback (if set) and then calls app if " + - "that key doesn't match", async () => { + it.each(types)("requests a key from the cache callback (if set) and then" + + " calls app if that key doesn't match", async ({ type, shouldCache }) => { const getCrossSigningKey = jest.fn().mockResolvedValue(testKey); const getCrossSigningKeyCache = jest.fn().mockResolvedValue(badKey); const storeCrossSigningKeyCache = jest.fn(); @@ -170,13 +195,13 @@ describe("CrossSigningInfo.getCrossSigningKey", function() { { getCrossSigningKey }, { getCrossSigningKeyCache, storeCrossSigningKeyCache }, ); - const [pubKey] = await info.getCrossSigningKey("master", masterKeyPub); + const [pubKey] = await info.getCrossSigningKey(type, masterKeyPub); expect(pubKey).toEqual(masterKeyPub); expect(getCrossSigningKey.mock.calls.length).toBe(1); - expect(getCrossSigningKeyCache.mock.calls.length).toBe(1); + expect(getCrossSigningKeyCache.mock.calls.length).toBe(shouldCache ? 1 : 0); /* Also expect that the cache gets updated */ - expect(storeCrossSigningKeyCache.mock.calls.length).toBe(1); + expect(storeCrossSigningKeyCache.mock.calls.length).toBe(shouldCache ? 1 : 0); }); }); @@ -187,7 +212,7 @@ describe("CrossSigningInfo.getCrossSigningKey", function() { describe.each([ ["IndexedDBCryptoStore", () => new IndexedDBCryptoStore(global.indexedDB, "tests")], - ["LocalStorageCryptoStore", + ["LocalStorageCryptoStore", () => new IndexedDBCryptoStore(undefined, "tests")], ["MemoryCryptoStore", () => { const store = new IndexedDBCryptoStore(undefined, "tests"); diff --git a/src/crypto/CrossSigning.js b/src/crypto/CrossSigning.js index 0a6a0d65b..dc69a31bc 100644 --- a/src/crypto/CrossSigning.js +++ b/src/crypto/CrossSigning.js @@ -65,6 +65,8 @@ export class CrossSigningInfo extends EventEmitter { * @returns {Array} An array with [ public key, Olm.PkSigning ] */ async getCrossSigningKey(type, expectedPubkey) { + const shouldCache = ["self_signing", "user_signing"].indexOf(type) >= 0; + if (!this._callbacks.getCrossSigningKey) { throw new Error("No getCrossSigningKey callback supplied"); } @@ -84,7 +86,7 @@ export class CrossSigningInfo extends EventEmitter { } let privkey; - if (this._cacheCallbacks.getCrossSigningKeyCache) { + if (this._cacheCallbacks.getCrossSigningKeyCache && shouldCache) { privkey = await this._cacheCallbacks .getCrossSigningKeyCache(type, expectedPubkey); } @@ -97,7 +99,7 @@ export class CrossSigningInfo extends EventEmitter { privkey = await this._callbacks.getCrossSigningKey(type, expectedPubkey); const result = validateKey(privkey); if (result) { - if (this._cacheCallbacks.storeCrossSigningKeyCache) { + if (this._cacheCallbacks.storeCrossSigningKeyCache && shouldCache) { await this._cacheCallbacks.storeCrossSigningKeyCache(type, privkey); } return result; From 656694ee009c7748a3f23c8d1e7f13cce6c961dc Mon Sep 17 00:00:00 2001 From: Zoe Date: Mon, 2 Mar 2020 09:45:55 +0000 Subject: [PATCH 7/7] proper spacing for test output text --- spec/unit/crypto/CrossSigningInfo.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/unit/crypto/CrossSigningInfo.spec.js b/spec/unit/crypto/CrossSigningInfo.spec.js index f956b5505..08572e846 100644 --- a/spec/unit/crypto/CrossSigningInfo.spec.js +++ b/spec/unit/crypto/CrossSigningInfo.spec.js @@ -89,7 +89,7 @@ describe("CrossSigningInfo.getCrossSigningKey", function() { it.each(types)("should request a key from the cache callback (if set)" + " and does not call app if one is found" + - `%o`, + " %o", async ({ type, shouldCache }) => { const getCrossSigningKey = jest.fn().mockImplementation(() => { if (shouldCache) {