From 6942e3467b683beafd303a0b8082c619876c364c Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Tue, 10 Dec 2019 16:39:42 +0000 Subject: [PATCH] Rework to hold cross-signing keys in JS SDK as needed --- spec/unit/crypto/secrets.spec.js | 15 +-- src/client.js | 9 +- src/crypto/CrossSigning.js | 66 +++--------- src/crypto/SecretStorage.js | 2 - src/crypto/index.js | 174 +++++++++++++++---------------- 5 files changed, 110 insertions(+), 156 deletions(-) diff --git a/spec/unit/crypto/secrets.spec.js b/spec/unit/crypto/secrets.spec.js index bc4aee05c..7c1195aae 100644 --- a/spec/unit/crypto/secrets.spec.js +++ b/spec/unit/crypto/secrets.spec.js @@ -246,18 +246,11 @@ describe("Secrets", function() { }); it("bootstraps when no storage or cross-signing keys locally", async function() { - let keys = {}; const bob = await makeTestClient( { userId: "@bob:example.com", deviceId: "bob1", }, - { - cryptoCallbacks: { - getCrossSigningKey: t => keys[t], - saveCrossSigningKeys: k => keys = k, - }, - }, ); bob.uploadDeviceSigningKeys = async () => {}; bob.uploadKeySignatures = async () => {}; @@ -287,7 +280,6 @@ describe("Secrets", function() { const storagePublicKey = decryption.generate_key(); const storagePrivateKey = decryption.get_private_key(); - let crossSigningKeys = {}; const bob = await makeTestClient( { userId: "@bob:example.com", @@ -295,8 +287,6 @@ describe("Secrets", function() { }, { cryptoCallbacks: { - getCrossSigningKey: t => crossSigningKeys[t], - saveCrossSigningKeys: k => crossSigningKeys = k, getSecretStorageKey: request => { const defaultKeyId = bob.getDefaultSecretStorageKeyId(); expect(Object.keys(request.keys)).toEqual([defaultKeyId]); @@ -318,6 +308,7 @@ describe("Secrets", function() { ]); this.emit("accountData", event); }; + bob._crypto.checkKeyBackup = async () => {}; const crossSigning = bob._crypto._crossSigningInfo; const secretStorage = bob._crypto._secretStorage; @@ -328,6 +319,10 @@ describe("Secrets", function() { }); // Clear local cross-signing keys and read from secret storage + bob._crypto._deviceList.storeCrossSigningForUser( + "@bob:example.com", + crossSigning.toStorage(), + ); crossSigning.keys = {}; await bob.bootstrapSecretStorage(); diff --git a/src/client.js b/src/client.js index cf6579592..5ac34dc92 100644 --- a/src/client.js +++ b/src/client.js @@ -181,7 +181,8 @@ function keyFromRecoverySession(session, decryptionKey) { * The cross-signing API is currently UNSTABLE and may change without notice. * * @param {function} [opts.cryptoCallbacks.getCrossSigningKey] - * Optional (required for cross-signing). Function to call when a cross-signing private key is needed. + * Optional. Function to call when a cross-signing private key is needed. + * Secure Secret Storage will be used by default if this is unset. * Args: * {string} type The type of key needed. Will be one of "master", * "self_signing", or "user_signing" @@ -193,8 +194,8 @@ function keyFromRecoverySession(session, decryptionKey) { * UInt8Array or rejects with an error. * * @param {function} [opts.cryptoCallbacks.saveCrossSigningKeys] - * Optional (required for cross-signing). Called when new private keys - * for cross-signing need to be saved. + * Optional. Called when new private keys for cross-signing need to be saved. + * Secure Secret Storage will be used by default if this is unset. * Args: * {object} keys the private keys to save. Map of key name to private key * as a UInt8Array. The getPrivateKey callback above will be called @@ -298,7 +299,7 @@ function MatrixClient(opts) { this._cryptoStore = opts.cryptoStore; this._sessionStore = opts.sessionStore; this._verificationMethods = opts.verificationMethods; - this._cryptoCallbacks = opts.cryptoCallbacks; + this._cryptoCallbacks = opts.cryptoCallbacks || {}; this._forceTURN = opts.forceTURN || false; this._fallbackICEServerAllowed = opts.fallbackICEServerAllowed || false; diff --git a/src/crypto/CrossSigning.js b/src/crypto/CrossSigning.js index c4c4ab65a..764f2c21e 100644 --- a/src/crypto/CrossSigning.js +++ b/src/crypto/CrossSigning.js @@ -115,8 +115,8 @@ export class CrossSigningInfo extends EventEmitter { */ isStoredInSecretStorage(secretStorage) { let stored = true; - for (const name of ["master", "self_signing", "user_signing"]) { - stored &= secretStorage.isStored(`m.cross_signing.${name}`, false); + for (const type of ["master", "self_signing", "user_signing"]) { + stored &= secretStorage.isStored(`m.cross_signing.${type}`, false); } return stored; } @@ -126,13 +126,13 @@ export class CrossSigningInfo extends EventEmitter { * typically called in conjunction with the creation of new cross-signing * keys. * + * @param {object} keys The keys to store * @param {SecretStorage} secretStorage The secret store using account data */ - async storeInSecretStorage(secretStorage) { - const getKey = this._callbacks.getCrossSigningKey; - for (const name of ["master", "self_signing", "user_signing"]) { - const encodedKey = encodeBase64(await getKey(name)); - await secretStorage.store(`m.cross_signing.${name}`, encodedKey); + static async storeInSecretStorage(keys, secretStorage) { + for (const type of Object.keys(keys)) { + const encodedKey = encodeBase64(keys[type]); + await secretStorage.store(`m.cross_signing.${type}`, encodedKey); } } @@ -140,54 +140,14 @@ export class CrossSigningInfo extends EventEmitter { * Get private keys from secret storage created by some other device. This * also passes the private keys to the app-specific callback. * + * @param {string} type The type of key to get. One of "master", + * "self_signing", or "user_signing". * @param {SecretStorage} secretStorage The secret store using account data + * @return {Uint8Array} The private key */ - async getFromSecretStorage(secretStorage) { - if (!this._callbacks.saveCrossSigningKeys) { - throw new Error("No saveCrossSigningKeys callback supplied"); - } - - // Retrieve private keys from secret storage - const privateKeys = {}; - for (const name of ["master", "self_signing", "user_signing"]) { - const encodedKey = await secretStorage.get(`m.cross_signing.${name}`); - privateKeys[name] = decodeBase64(encodedKey); - } - - // Regenerate public keys from private keys - // XXX: Do we want to _also_ download public keys from the homeserver to - // verify they agree...? - // See also https://github.com/vector-im/riot-web/issues/11558 - const signings = {}; - const publicKeys = {}; - const keys = {}; - try { - for (const name of ["master", "self_signing", "user_signing"]) { - signings[name] = new global.Olm.PkSigning(); - publicKeys[name] = signings[name].init_with_seed(privateKeys[name]); - keys[name] = { - user_id: this.userId, - usage: [name], - keys: { - ['ed25519:' + publicKeys[name]]: publicKeys[name], - }, - }; - if (name !== "master") { - pkSign( - keys[name], signings["master"], - this.userId, publicKeys["master"], - ); - } - } - } finally { - for (const signing of Object.values(signings)) { - signing.free(); - } - } - - // Save public keys locally and private keys via app callback - Object.assign(this.keys, keys); - this._callbacks.saveCrossSigningKeys(privateKeys); + static async getFromSecretStorage(type, secretStorage) { + const encodedKey = await secretStorage.get(`m.cross_signing.${type}`); + return decodeBase64(encodedKey); } /** diff --git a/src/crypto/SecretStorage.js b/src/crypto/SecretStorage.js index 2c0e1884b..db984b950 100644 --- a/src/crypto/SecretStorage.js +++ b/src/crypto/SecretStorage.js @@ -167,8 +167,6 @@ export default class SecretStorage extends EventEmitter { return keyInfo && keyInfo.getContent(); } - // TODO: need a function to get all the secret keys - /** * Store an encrypted secret on the server * diff --git a/src/crypto/index.js b/src/crypto/index.js index 899f06172..0f4734fcc 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -245,13 +245,20 @@ export default function Crypto(baseApis, sessionStore, userId, deviceId, this._verificationTransactions = new Map(); - this._crossSigningInfo = new CrossSigningInfo( - userId, this._baseApis._cryptoCallbacks, - ); + const cryptoCallbacks = this._baseApis._cryptoCallbacks || {}; + + this._crossSigningInfo = new CrossSigningInfo(userId, cryptoCallbacks); this._secretStorage = new SecretStorage( - baseApis, this._baseApis._cryptoCallbacks, this._crossSigningInfo, + baseApis, cryptoCallbacks, this._crossSigningInfo, ); + + // Assuming no app-supplied callback, default to getting from SSSS. + if (!cryptoCallbacks.getCrossSigningKey) { + cryptoCallbacks.getCrossSigningKey = async (type) => { + return CrossSigningInfo.getFromSecretStorage(type, this._secretStorage); + }; + } } utils.inherits(Crypto, EventEmitter); @@ -377,55 +384,75 @@ Crypto.prototype.bootstrapSecretStorage = async function({ // key with the cross-signing master key. The cross-signing master key is also used // to verify the signature on the SSSS default key when adding secrets, so we // effectively need it for both reading and writing secrets. - let crossSigningKeysReset = false; - if ( - !this._crossSigningInfo.getId() || - !await this._baseApis._cryptoCallbacks.getCrossSigningKey("master") - ) { - logger.log( - "Cross-signing public and/or private keys not found on device, " + - "checking secret storage for private keys", - ); - if (this._crossSigningInfo.isStoredInSecretStorage(this._secretStorage)) { - logger.log("Cross-signing private keys found in secret storage"); - await this._getCrossSigningKeysFromSecretStorage(); - } else { + let crossSigningPrivateKeys = {}; + + // If we happen to reset cross-signing keys here, then we want access to the + // cross-signing private keys, but only for the scope of this method, so we + // use temporary callbacks to weave the them through the various APIs. + const appCallbacks = Object.assign({}, this._baseApis._cryptoCallbacks); + + try { + if ( + !this._crossSigningInfo.getId() || + !this._crossSigningInfo.isStoredInSecretStorage(this._secretStorage) + ) { logger.log( - "Cross-signing private keys not found in secret storage, " + - "creating new keys", + "Cross-signing public and/or private keys not found, " + + "checking secret storage for private keys", ); - await this.resetCrossSigningKeys( - CrossSigningLevel.MASTER, - { authUploadDeviceSigningKeys }, - ); - crossSigningKeysReset = true; + if (this._crossSigningInfo.isStoredInSecretStorage(this._secretStorage)) { + logger.log("Cross-signing private keys found in secret storage"); + await this.checkOwnCrossSigningTrust(); + } else { + logger.log( + "Cross-signing private keys not found in secret storage, " + + "creating new keys", + ); + this._baseApis._cryptoCallbacks.saveCrossSigningKeys = + keys => crossSigningPrivateKeys = keys; + this._baseApis._cryptoCallbacks.getCrossSigningKey = + name => crossSigningPrivateKeys[name]; + await this.resetCrossSigningKeys( + CrossSigningLevel.MASTER, + { authUploadDeviceSigningKeys }, + ); + } } - } - // Check if Secure Secret Storage has a default key. If we don't have one, create the - // default key (which will also be signed by the cross-signing master key). - if (!this.hasSecretStorageKey()) { - logger.log("Secret storage default key not found, creating new key"); - const keyOptions = await createSecretStorageKey(); - const newKeyId = await this.addSecretStorageKey( - SECRET_STORAGE_ALGORITHM_V1, - keyOptions, - ); - await this.setDefaultSecretStorageKeyId(newKeyId); - } + // Check if Secure Secret Storage has a default key. If we don't have one, create + // the default key (which will also be signed by the cross-signing master key). + if (!this.hasSecretStorageKey()) { + logger.log("Secret storage default key not found, creating new key"); + const keyOptions = await createSecretStorageKey(); + const newKeyId = await this.addSecretStorageKey( + SECRET_STORAGE_ALGORITHM_V1, + keyOptions, + ); + await this.setDefaultSecretStorageKeyId(newKeyId); + } - // If cross-signing keys were reset, store them in Secure Secret Storage. - // This is done in a separate step so we can ensure secret storage has its - // own key first. - // XXX: We need to think about how to re-do these steps if they fail. - if (crossSigningKeysReset) { - logger.log("Storing cross-signing private keys in secret storage"); - // SSSS expects its keys to be signed by cross-signing master key. - // Since we have just reset cross-signing keys, we need to re-sign the - // SSSS default key with the new cross-signing master key so that the - // following storage step can proceed. - await this._secretStorage.signKey(); - await this._crossSigningInfo.storeInSecretStorage(this._secretStorage); + // If cross-signing keys were reset, store them in Secure Secret Storage. + // This is done in a separate step so we can ensure secret storage has its + // own key first. + // XXX: We need to think about how to re-do these steps if they fail. + // See also https://github.com/vector-im/riot-web/issues/11635 + if (crossSigningPrivateKeys) { + logger.log("Storing cross-signing private keys in secret storage"); + // SSSS expects its keys to be signed by cross-signing master key. + // Since we have just reset cross-signing keys, we need to re-sign the + // SSSS default key with the new cross-signing master key so that the + // following storage step can proceed. + await this._secretStorage.signKey(); + // Assuming no app-supplied callback, default to storing in SSSS. + if (!appCallbacks.saveCrossSigningKeys) { + await CrossSigningInfo.storeInSecretStorage( + crossSigningPrivateKeys, + this._secretStorage, + ); + } + } + } finally { + this._baseApis._cryptoCallbacks = appCallbacks; } logger.log("Secure Secret Storage ready"); @@ -559,41 +586,6 @@ Crypto.prototype.resetCrossSigningKeys = async function(level, { logger.info("Cross-signing key reset complete"); }; -/** - * If cross-signing keys are known to exist in secret storage, this will get - * them and store them on this device as trusted. - */ -Crypto.prototype._getCrossSigningKeysFromSecretStorage = async function() { - logger.info("Getting cross-signing keys from secret storage"); - // Copy old keys (usually empty) in case we need to revert - const oldKeys = Object.assign({}, this._crossSigningInfo.keys); - try { - await this._crossSigningInfo.getFromSecretStorage(this._secretStorage); - // XXX: Do we also need to sign the cross-signing master key with the - // device key as in `resetCrossSigningKeys`? - - // write a copy locally so we know these are trusted keys - await this._cryptoStore.doTxn( - 'readwrite', [IndexedDBCryptoStore.STORE_ACCOUNT], - (txn) => { - this._cryptoStore.storeCrossSigningKeys(txn, this._crossSigningInfo.keys); - }, - ); - } catch (e) { - // If anything failed here, revert the keys so we know to try again from the start - // next time. - logger.error( - "Getting cross-signing keys from secret storage failed, " + - "revert to previous keys", e, - ); - this._crossSigningInfo.keys = oldKeys; - throw e; - } - this._baseApis.emit("crossSigning.keysChanged", {}); - await this._afterCrossSigningLocalKeyChange(); - logger.info("Cross-signing keys restored from secret storage"); -}; - /** * Run various follow-up actions after cross-signing keys have changed locally * (either by resetting the keys for the account or bye getting them from secret @@ -817,10 +809,10 @@ Crypto.prototype.checkOwnCrossSigningTrust = async function() { } const seenPubkey = newCrossSigning.getId(); - const changed = this._crossSigningInfo.getId() !== seenPubkey; - if (changed) { + const masterChanged = this._crossSigningInfo.getId() !== seenPubkey; + if (masterChanged) { // try to get the private key if the master key changed - logger.info("Got new master key", seenPubkey); + logger.info("Got new master public key", seenPubkey); let signing = null; try { @@ -828,6 +820,9 @@ Crypto.prototype.checkOwnCrossSigningTrust = async function() { 'master', seenPubkey, ); signing = ret[1]; + if (!signing) { + throw new Error("Cross-signing master private key not available"); + } } finally { signing.free(); } @@ -862,7 +857,7 @@ Crypto.prototype.checkOwnCrossSigningTrust = async function() { logger.info("Got new user-signing key", newCrossSigning.getId("user_signing")); } - if (changed) { + if (masterChanged) { await this._signObject(this._crossSigningInfo.keys.master); keySignatures[this._crossSigningInfo.getId()] = this._crossSigningInfo.keys.master; @@ -874,6 +869,11 @@ Crypto.prototype.checkOwnCrossSigningTrust = async function() { this.emit("userTrustStatusChanged", userId, this.checkUserTrust(userId)); + if (masterChanged) { + this._baseApis.emit("crossSigning.keysChanged", {}); + await this._afterCrossSigningLocalKeyChange(); + } + // Now we may be able to trust our key backup await this.checkKeyBackup(); // FIXME: if we previously trusted the backup, should we automatically sign