From 9bcf33b6d3bcf6cb5f0bfc11fae182a96ed69638 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 17 Jun 2020 14:54:03 +0200 Subject: [PATCH 01/12] Add EncryptionSetupBuilder to capture bootstrapping side-effects --- src/crypto/EncryptionSetup.js | 133 ++++++++++++++++++++++++++++++++++ src/crypto/index.js | 9 +++ 2 files changed, 142 insertions(+) create mode 100644 src/crypto/EncryptionSetup.js diff --git a/src/crypto/EncryptionSetup.js b/src/crypto/EncryptionSetup.js new file mode 100644 index 000000000..16422eef7 --- /dev/null +++ b/src/crypto/EncryptionSetup.js @@ -0,0 +1,133 @@ +import {MatrixEvent} from "../models/event"; +import {EventEmitter} from "events"; + +/** + * Builds an EncryptionSetupOperation by calling any of the add.. methods. + * Once done, `buildOperation()` can be called which allows to apply to operation. + * + * This is used as a helper by Crypto to keep track of all the network requests + * and other side-effects of bootstrapping, so it can be applied in one go (and retried in the future) + * Also keeps track of all the private keys created during bootstrapping, so we don't need to prompt for them + * more than once. + */ +export class EncryptionSetupBuilder { + /** + * @param {Object.} accountData pre-existing account data, will only be read, not written. + */ + constructor(accountData) { + this.accountDataClientAdapter = new AccountDataClientAdapter(accountData); + this.crossSigningCallbacks = new CrossSigningCallbacks(); + this.ssssCryptoCallbacks = new SSSSCryptoCallbacks(); + } +} + + +/** + * Catches account data set by SecretStorage during bootstrapping by + * implementing the methods related to account data in MatrixClient + */ +class AccountDataClientAdapter extends EventEmitter { + /** + * @param {Object.} accountData existing account data + */ + constructor(accountData) { + super(); + this._existingValues = accountData; + this._values = new Map(); + } + + /** + * @param {String} type + * @return {Promise} the content of the account data + */ + getAccountDataFromServer(type) { + return Promise.resolve(this.getAccountData(type)); + } + + /** + * @param {String} type + * @return {Object} the content of the account data + */ + getAccountData(type) { + const modifiedValue = this._values.get(type); + if (modifiedValue) { + return modifiedValue; + } + const existingValue = this._existingValues[type]; + if (existingValue) { + return existingValue.getContent(); + } + return null; + } + + /** + * @param {String} type + * @param {Object} content + * @return {Promise} + */ + setAccountData(type, content) { + this._values.set(type, content); + // ensure accountData is emitted on the next tick, + // as SecretStorage listens for it while calling this method + // and it seems to rely on this. + return Promise.resolve().then(() => { + const event = new MatrixEvent({type, content}); + this.emit("accountData", event); + }); + } +} + +/** + * Catches the private cross-signing keys set during bootstrapping + * by both cache callbacks (see createCryptoStoreCacheCallbacks) as non-cache callbacks. + * See CrossSigningInfo constructor + */ +class CrossSigningCallbacks { + constructor() { + this.privateKeys = new Map(); + } + + // cache callbacks + getCrossSigningKeyCache(type, expectedPublicKey) { + return this.getCrossSigningKey(type, expectedPublicKey); + } + + storeCrossSigningKeyCache(type, key) { + this.privateKeys.set(type, key); + return Promise.resolve(); + } + + // non-cache callbacks + getCrossSigningKey(type, _expectedPubkey) { + return Promise.resolve(this.privateKeys.get(type)); + } + + saveCrossSigningKeys(privateKeys) { + for (const [type, privateKey] of Object.entries(privateKeys)) { + this.privateKeys.set(type, privateKey); + } + } +} + +/** + * Catches the 4S private key set during bootstrapping by implementing + * the SecretStorage crypto callbacks + */ +class SSSSCryptoCallbacks { + constructor() { + this._privateKeys = new Map(); + } + + getSecretStorageKey({ keys }, name) { + for (const keyId of Object.keys(keys)) { + const privateKey = this._privateKeys.get(keyId); + if (privateKey) { + return [keyId, privateKey]; + } + } + } + + addPrivateKey(keyId, privKey) { + this._privateKeys.set(keyId, privKey); + } +} diff --git a/src/crypto/index.js b/src/crypto/index.js index ef9f492de..035286fe7 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -39,6 +39,7 @@ import { UserTrustLevel, createCryptoStoreCacheCallbacks, } from './CrossSigning'; +import {EncryptionSetupBuilder} from "./EncryptionSetup"; import {SECRET_STORAGE_ALGORITHM_V1_AES, SecretStorage} from './SecretStorage'; import {OutgoingRoomKeyRequestManager} from './OutgoingRoomKeyRequestManager'; import {IndexedDBCryptoStore} from './store/indexeddb-crypto-store'; @@ -494,6 +495,14 @@ Crypto.prototype.bootstrapSecretStorage = async function({ // cross-signing private keys, but only for the scope of this method, so we // use temporary callbacks to weave them through the various APIs. const appCallbacks = Object.assign({}, this._baseApis._cryptoCallbacks); + const builder = new EncryptionSetupBuilder(this._baseApis.store.accountData); + const secretStorage = new SecretStorage( + builder.accountDataClientAdapter, + builder.ssssCryptoCallbacks); + const crossSigningInfo = new CrossSigningInfo( + this._userId, + builder.crossSigningCallbacks, + builder.crossSigningCallbacks); // the ID of the new SSSS key, if we create one let newKeyId = null; From d4b4bc5031654437a687a04014c0f78276c714bd Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 17 Jun 2020 15:06:41 +0200 Subject: [PATCH 02/12] move 4S creation to operation --- src/crypto/EncryptionSetup.js | 63 +++++++++++++++++++++++++++++++++++ src/crypto/index.js | 36 +++++--------------- 2 files changed, 72 insertions(+), 27 deletions(-) diff --git a/src/crypto/EncryptionSetup.js b/src/crypto/EncryptionSetup.js index 16422eef7..e706af556 100644 --- a/src/crypto/EncryptionSetup.js +++ b/src/crypto/EncryptionSetup.js @@ -18,6 +18,69 @@ export class EncryptionSetupBuilder { this.accountDataClientAdapter = new AccountDataClientAdapter(accountData); this.crossSigningCallbacks = new CrossSigningCallbacks(); this.ssssCryptoCallbacks = new SSSSCryptoCallbacks(); + + this._crossSigningKeys = null; + this._keySignatures = null; + this._keyBackupInfo = null; + } + + + /** + * @param {String} type + * @param {Object} content + * @return {Promise} + */ + setAccountData(type, content) { + return this.accountDataClientAdapter.setAccountData(type, content); + } + + /** + * builds the operation containing all the parts that have been added to the builder + * @return {EncryptionSetupOperation} + */ + buildOperation() { + const accountData = this.accountDataClientAdapter._values; + return new EncryptionSetupOperation( + accountData, + this._crossSigningKeys, + this._keyBackupInfo, + this._keySignatures, + ); + } + +/** + * Can be created from EncryptionSetupBuilder, or + * (in a follow-up PR, not implemented yet) restored from storage, to retry. + * + * It does not have knowledge of any private keys, unlike the builder. + */ +export class EncryptionSetupOperation { + /** + * @param {Map} accountData + * @param {Object} crossSigningKeys + * @param {Object} keyBackupInfo + * @param {Object} keySignatures + */ + constructor(accountData, crossSigningKeys, keyBackupInfo, keySignatures) { + this._accountData = accountData; + this._crossSigningKeys = crossSigningKeys; + this._keyBackupInfo = keyBackupInfo; + this._keySignatures = keySignatures; + } + + /** + * Runs the (remaining part of, in the future) operation by sending requests to the server. + * @param {Crypto} crypto + */ + async apply(crypto) { + const baseApis = crypto._baseApis; + // set account data + if (this._accountData) { + for (const [type, content] of this._accountData) { + await baseApis.setAccountData(type, content); + } + } + } } } diff --git a/src/crypto/index.js b/src/crypto/index.js index 035286fe7..261203b89 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -507,27 +507,6 @@ Crypto.prototype.bootstrapSecretStorage = async function({ // the ID of the new SSSS key, if we create one let newKeyId = null; - // cache SSSS keys so that we don't need to constantly pester the user about it - const ssssKeys = {}; - - this._baseApis._cryptoCallbacks.getSecretStorageKey = - async ({keys}, name) => { - // if we already have a key that works, return it - for (const keyId of Object.keys(keys)) { - if (ssssKeys[keyId]) { - return [keyId, ssssKeys[keyId]]; - } - } - - // otherwise, prompt the user and cache it - const key = await appCallbacks.getSecretStorageKey({keys}, name); - if (key) { - const [keyId, keyData] = key; - ssssKeys[keyId] = keyData; - } - return key; - }; - // create a new SSSS key and set it as default const createSSSS = async (opts, privateKey) => { opts = opts || {}; @@ -535,15 +514,16 @@ Crypto.prototype.bootstrapSecretStorage = async function({ opts.key = privateKey; } - const keyId = await this.addSecretStorageKey( + const keyId = await secretStorage.addKey( SECRET_STORAGE_ALGORITHM_V1_AES, opts, ); - await this.setDefaultSecretStorageKeyId(keyId); if (privateKey) { - // cache the private key so that we can access it again - ssssKeys[keyId] = privateKey; + // make the private key available to encrypt 4S secrets + builder.ssssCryptoCallbacks.addPrivateKey(keyId, privateKey); } + + await secretStorage.setDefaultKeyId(keyId); return keyId; }; @@ -566,12 +546,12 @@ Crypto.prototype.bootstrapSecretStorage = async function({ ); if (key) { const keyData = key[1]; - ssssKeys[keyId] = keyData; + builder.ssssCryptoCallbacks.addPrivateKey(keyId, keyData); const {iv, mac} = await SecretStorage._calculateKeyCheck(keyData); keyInfo.iv = iv; keyInfo.mac = mac; - await this._baseApis.setAccountData( + await builder.setAccountData( `m.secret_storage.key.${keyId}`, keyInfo, ); } @@ -746,6 +726,8 @@ Crypto.prototype.bootstrapSecretStorage = async function({ Object.assign(this._baseApis._cryptoCallbacks, appCallbacks); } + const operation = builder.buildOperation(); + await operation.apply(this); logger.log("Secure Secret Storage ready"); }; From f656698061f5575a66ba58fda99f8b379cb49d86 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 17 Jun 2020 15:14:26 +0200 Subject: [PATCH 03/12] fixup: secret storage indenting --- src/crypto/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/crypto/index.js b/src/crypto/index.js index 261203b89..16e6c4b5b 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -726,8 +726,8 @@ Crypto.prototype.bootstrapSecretStorage = async function({ Object.assign(this._baseApis._cryptoCallbacks, appCallbacks); } - const operation = builder.buildOperation(); - await operation.apply(this); + const operation = builder.buildOperation(); + await operation.apply(this); logger.log("Secure Secret Storage ready"); }; From 3c5b304b6b75f15aea2a0eabc23192b3cd515c6e Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 17 Jun 2020 15:22:24 +0200 Subject: [PATCH 04/12] move cross-signing bootstrapping over to builder --- src/crypto/CrossSigning.js | 6 +-- src/crypto/EncryptionSetup.js | 80 +++++++++++++++++++++++++++++++++++ src/crypto/index.js | 65 +++++++++++++--------------- 3 files changed, 113 insertions(+), 38 deletions(-) diff --git a/src/crypto/CrossSigning.js b/src/crypto/CrossSigning.js index ff5bc7080..ec82a6045 100644 --- a/src/crypto/CrossSigning.js +++ b/src/crypto/CrossSigning.js @@ -178,12 +178,12 @@ 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 {Map} keys The keys to store * @param {SecretStorage} secretStorage The secret store using account data */ static async storeInSecretStorage(keys, secretStorage) { - for (const type of Object.keys(keys)) { - const encodedKey = encodeBase64(keys[type]); + for (const [type, privateKey] of keys) { + const encodedKey = encodeBase64(privateKey); await secretStorage.store(`m.cross_signing.${type}`, encodedKey); } } diff --git a/src/crypto/EncryptionSetup.js b/src/crypto/EncryptionSetup.js index e706af556..9447fabdc 100644 --- a/src/crypto/EncryptionSetup.js +++ b/src/crypto/EncryptionSetup.js @@ -1,5 +1,10 @@ import {MatrixEvent} from "../models/event"; import {EventEmitter} from "events"; +import {createCryptoStoreCacheCallbacks} from "./CrossSigning"; +import {IndexedDBCryptoStore} from './store/indexeddb-crypto-store'; +import { + PREFIX_UNSTABLE, +} from "../http-api"; /** * Builds an EncryptionSetupOperation by calling any of the add.. methods. @@ -24,6 +29,32 @@ export class EncryptionSetupBuilder { this._keyBackupInfo = null; } + /** + * Adds new cross-signing public keys + * @param {Object} auth auth dictionary needed to upload the new keys + * @param {Object} keys the new keys + */ + addCrossSigningKeys(auth, keys) { + this._crossSigningKeys = {auth, keys}; + } + + /** + * Add signatures from a given user and device/x-sign key + * Used to sign the new cross-signing key with the device key + * + * @param {String} userId + * @param {String} deviceId + * @param {String} signature + */ + addKeySignature(userId, deviceId, signature) { + if (!this._keySignatures) { + this._keySignatures = {}; + } + const userSignatures = this._keySignatures[userId] || {}; + this._keySignatures[userId] = userSignatures; + userSignatures[deviceId] = signature; + } + /** * @param {String} type @@ -48,6 +79,37 @@ export class EncryptionSetupBuilder { ); } + /** + * Stores the created keys locally. + * + * This does not yet store the operation in a way that it can be restored, + * but that is the idea in the future. + * + * @param {Crypto} crypto + * @return {Promise} + */ + async persist(crypto) { + // store self_signing and user_signing private key in cache + if (this._crossSigningKeys) { + const cacheCallbacks = createCryptoStoreCacheCallbacks( + crypto._cryptoStore, crypto._olmDevice); + for (const type of ["self_signing", "user_signing"]) { + // logger.log(`Cache ${type} cross-signing private key locally`); + const privateKey = this.crossSigningCallbacks.privateKeys.get(type); + await cacheCallbacks.storeCrossSigningKeyCache(type, privateKey); + } + // store own cross-sign pubkeys as trusted + await crypto._cryptoStore.doTxn( + 'readwrite', [IndexedDBCryptoStore.STORE_ACCOUNT], + (txn) => { + crypto._cryptoStore.storeCrossSigningKeys( + txn, this._crossSigningKeys.keys); + }, + ); + } + } +} + /** * Can be created from EncryptionSetupBuilder, or * (in a follow-up PR, not implemented yet) restored from storage, to retry. @@ -80,6 +142,24 @@ export class EncryptionSetupOperation { await baseApis.setAccountData(type, content); } } + // upload cross-signing keys + if (this._crossSigningKeys) { + const keys = {}; + for (const [name, key] of Object.entries(this._crossSigningKeys.keys)) { + keys[name + "_key"] = key; + } + await baseApis.uploadDeviceSigningKeys( + this._crossSigningKeys.auth, + keys, + ); + // pass the new keys to the main instance of our own CrossSigningInfo. + crypto._crossSigningInfo.setKeys(this._crossSigningKeys.keys); + } + // upload first cross-signing signatures with the new key + // (e.g. signing our own device) + if (this._keySignatures) { + await baseApis.uploadKeySignatures(this._keySignatures); + } } } } diff --git a/src/crypto/index.js b/src/crypto/index.js index 16e6c4b5b..28865eb7a 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -476,7 +476,7 @@ Crypto.prototype.isCrossSigningReady = async function() { * SecretStorage#addKey: an object with `passphrase` and/or `pubkey` fields. */ Crypto.prototype.bootstrapSecretStorage = async function({ - authUploadDeviceSigningKeys, + authUploadDeviceSigningKeys = async func => await func(), createSecretStorageKey = async () => ({ }), keyBackupInfo, setupNewKeyBackup, @@ -484,17 +484,6 @@ Crypto.prototype.bootstrapSecretStorage = async function({ getKeyBackupPassphrase, } = {}) { logger.log("Bootstrapping Secure Secret Storage"); - - // Create cross-signing keys if they don't exist, as we want to sign the SSSS default - // 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. - const 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 them through the various APIs. - const appCallbacks = Object.assign({}, this._baseApis._cryptoCallbacks); const builder = new EncryptionSetupBuilder(this._baseApis.store.accountData); const secretStorage = new SecretStorage( builder.accountDataClientAdapter, @@ -529,14 +518,24 @@ Crypto.prototype.bootstrapSecretStorage = async function({ // reset the cross-signing keys const resetCrossSigning = async () => { - this._baseApis._cryptoCallbacks.saveCrossSigningKeys = - keys => Object.assign(crossSigningPrivateKeys, keys); - this._baseApis._cryptoCallbacks.getCrossSigningKey = - name => crossSigningPrivateKeys[name]; - await this.resetCrossSigningKeys( - CrossSigningLevel.MASTER, - { authUploadDeviceSigningKeys }, - ); + crossSigningInfo.resetKeys(); + // sign master key with device key + await this._signObject(crossSigningInfo.keys.master); + + await authUploadDeviceSigningKeys(authDict => { + builder.addCrossSigningKeys(authDict, crossSigningInfo.keys); + return Promise.resolve(); + }); + + // cross-sign own device + const device = this._deviceList.getStoredDevice(this._userId, this._deviceId); + const deviceSignature = await crossSigningInfo.signDevice(this._userId, device); + builder.addKeySignature(this._userId, this._deviceId, deviceSignature); + + if (keyBackupInfo) { + await crossSigningInfo.signObject(keyBackupInfo.auth_data, "master"); + builder.addSessionBackup(keyBackupInfo); + } }; const ensureCanCheckPassphrase = async (keyId, keyInfo) => { @@ -631,7 +630,7 @@ Crypto.prototype.bootstrapSecretStorage = async function({ // Sign the backup with the cross signing key so the key backup can // be trusted via cross-signing. logger.log("Adding cross signing signature to key backup"); - await this._crossSigningInfo.signObject( + await crossSigningInfo.signObject( keyBackupInfo.auth_data, "master", ); await this._baseApis._http.authedRequest( @@ -664,18 +663,16 @@ Crypto.prototype.bootstrapSecretStorage = async function({ } } - // 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 (Object.keys(crossSigningPrivateKeys).length) { + const crossSigningPrivateKeys = builder.crossSigningCallbacks.privateKeys; + if (crossSigningPrivateKeys.size) { logger.log("Storing cross-signing private keys in secret storage"); // Assuming no app-supplied callback, default to storing in SSSS. - if (!appCallbacks.saveCrossSigningKeys) { + if (!this._baseApis._cryptoCallbacks.saveCrossSigningKeys) { + // this is writing to in-memory account data in builder.accountDataClientAdapter + // so won't fail await CrossSigningInfo.storeInSecretStorage( crossSigningPrivateKeys, - this._secretStorage, + secretStorage, ); } } @@ -688,13 +685,7 @@ Crypto.prototype.bootstrapSecretStorage = async function({ await this._baseApis.createKeyBackupVersion(info); } - // Call `getCrossSigningKey` for side effect of caching private keys for - // future gossiping to other devices if enabled via app level callbacks. if (this._crossSigningInfo._cacheCallbacks) { - for (const type of ["self_signing", "user_signing"]) { - logger.log(`Cache ${type} cross-signing private key locally`); - await this._crossSigningInfo.getCrossSigningKey(type); - } } // and likewise for the session backup key @@ -728,6 +719,10 @@ Crypto.prototype.bootstrapSecretStorage = async function({ const operation = builder.buildOperation(); await operation.apply(this); + // this persists private keys and public keys as trusted, + // only do this if apply succeeded for now as retry isn't in place yet + await builder.persist(this); + } logger.log("Secure Secret Storage ready"); }; From b82870adb2e79748401f7268d1f99ff639c84737 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 17 Jun 2020 15:25:42 +0200 Subject: [PATCH 05/12] move key backup bootstrap/migration over to builder --- src/crypto/EncryptionSetup.js | 50 +++++++++++++++++++++++++++++++++++ src/crypto/index.js | 43 ++++++++++++++++++------------ 2 files changed, 76 insertions(+), 17 deletions(-) diff --git a/src/crypto/EncryptionSetup.js b/src/crypto/EncryptionSetup.js index 9447fabdc..549ec5e3d 100644 --- a/src/crypto/EncryptionSetup.js +++ b/src/crypto/EncryptionSetup.js @@ -38,6 +38,29 @@ export class EncryptionSetupBuilder { this._crossSigningKeys = {auth, keys}; } + /** + * Adds the key backup info to be updated on the server + * + * Used either to create a new key backup, or add signatures + * from the new MSK. + * + * @param {Object} keyBackupInfo as received from/sent to the server + */ + addSessionBackup(keyBackupInfo) { + this._keyBackupInfo = keyBackupInfo; + } + + /** + * Adds the session backup private key to be updated in the local cache + * + * Used after fixing the format of the key + * + * @param {Uint8Array} privateKey + */ + addSessionBackupPrivateKeyToCache(privateKey) { + this._sessionBackupPrivateKey = privateKey; + } + /** * Add signatures from a given user and device/x-sign key * Used to sign the new cross-signing key with the device key @@ -107,6 +130,10 @@ export class EncryptionSetupBuilder { }, ); } + // store session backup key in cache + if (this._sessionBackupPrivateKey) { + await crypto.storeSessionBackupPrivateKey(this._sessionBackupPrivateKey); + } } } @@ -160,6 +187,29 @@ export class EncryptionSetupOperation { if (this._keySignatures) { await baseApis.uploadKeySignatures(this._keySignatures); } + // need to create/update key backup info + if (this._keyBackupInfo) { + if (this._keyBackupInfo.version) { + // session backup signature + // The backup is trusted because the user provided the private key. + // Sign the backup with the cross signing key so the key backup can + // be trusted via cross-signing. + await baseApis._http.authedRequest( + undefined, "PUT", "/room_keys/version/" + this._keyBackupInfo.version, + undefined, { + algorithm: this._keyBackupInfo.algorithm, + auth_data: this._keyBackupInfo.auth_data, + }, + {prefix: PREFIX_UNSTABLE}, + ); + } else { + // add new key backup + await baseApis._http.authedRequest( + undefined, "POST", "/room_keys/version", + undefined, this._keyBackupInfo, + {prefix: PREFIX_UNSTABLE}, + ); + } } } } diff --git a/src/crypto/index.js b/src/crypto/index.js index 28865eb7a..5e4e7e9fa 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -34,7 +34,6 @@ import {DeviceInfo} from "./deviceinfo"; import * as algorithms from "./algorithms"; import { CrossSigningInfo, - CrossSigningLevel, DeviceTrustLevel, UserTrustLevel, createCryptoStoreCacheCallbacks, @@ -50,11 +49,10 @@ import { } from './verification/QRCode'; import {SAS} from './verification/SAS'; import {keyFromPassphrase} from './key_passphrase'; -import {encodeRecoveryKey} from './recoverykey'; +import {encodeRecoveryKey, decodeRecoveryKey} from './recoverykey'; import {VerificationRequest} from "./verification/request/VerificationRequest"; import {InRoomChannel, InRoomRequests} from "./verification/request/InRoomChannel"; import {ToDeviceChannel, ToDeviceRequests} from "./verification/request/ToDeviceChannel"; -import * as httpApi from "../http-api"; import {IllegalMethod} from "./verification/IllegalMethod"; import {KeySignatureUploadError} from "../errors"; import {decryptAES, encryptAES} from './aes'; @@ -622,7 +620,7 @@ Crypto.prototype.bootstrapSecretStorage = async function({ newKeyId = await createSSSS(opts, backupKey); // store the backup key in secret storage - await this.storeSecret( + await secretStorage.store( "m.megolm_backup.v1", olmlib.encodeBase64(backupKey), [newKeyId], ); @@ -633,11 +631,7 @@ Crypto.prototype.bootstrapSecretStorage = async function({ await crossSigningInfo.signObject( keyBackupInfo.auth_data, "master", ); - await this._baseApis._http.authedRequest( - undefined, "PUT", "/room_keys/version/" + keyBackupInfo.version, - undefined, keyBackupInfo, - {prefix: httpApi.PREFIX_UNSTABLE}, - ); + builder.addSessionBackup(keyBackupInfo); } else if (!this._crossSigningInfo.getId()) { // we have SSSS, but we don't know if the server's cross-signing // keys should be trusted @@ -680,30 +674,45 @@ Crypto.prototype.bootstrapSecretStorage = async function({ if (setupNewKeyBackup && !keyBackupInfo) { const info = await this._baseApis.prepareKeyBackupVersion( null /* random key */, - { secureSecretStorage: true }, + // don't write to secret storage, as it will write to this._secretStorage. + // Here, we want to capture all the side-effects of bootstrapping, + // and want to write to the local secretStorage object + { secureSecretStorage: false }, ); - await this._baseApis.createKeyBackupVersion(info); - } + // write the key ourselves to 4S + const privateKey = decodeRecoveryKey(info.recovery_key); + await secretStorage.store("m.megolm_backup.v1", olmlib.encodeBase64(privateKey)); - if (this._crossSigningInfo._cacheCallbacks) { + // create keyBackupInfo object to add to builder + const data = { + algorithm: info.algorithm, + auth_data: info.auth_data, + }; + // sign with cross-sign master key + await crossSigningInfo.signObject(data.auth_data, "master"); + // sign with the device fingerprint + await this._signObject(data.auth_data); + + + builder.addSessionBackup(data); } // and likewise for the session backup key - const sessionBackupKey = await this.getSecret('m.megolm_backup.v1'); + const sessionBackupKey = await secretStorage.get('m.megolm_backup.v1'); if (sessionBackupKey) { logger.info("Got session backup key from secret storage: caching"); // fix up the backup key if it's in the wrong format, and replace // in secret storage const fixedBackupKey = fixBackupKey(sessionBackupKey); if (fixedBackupKey) { - await this.storeSecret( - "m.megolm_backup.v1", fixedBackupKey, [newKeyId || oldKeyId], + await secretStorage.store("m.megolm_backup.v1", + fixedBackupKey, [newKeyId || oldKeyId], ); } const decodedBackupKey = new Uint8Array(olmlib.decodeBase64( fixedBackupKey || sessionBackupKey, )); - await this.storeSessionBackupPrivateKey(decodedBackupKey); + await builder.addSessionBackupPrivateKeyToCache(decodedBackupKey); } } finally { // Restore the original callbacks. NB. we must do this by manipulating From 8591815d663032708c46ecb0da39a7149d67c56a Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 17 Jun 2020 15:26:30 +0200 Subject: [PATCH 06/12] add comment --- src/crypto/index.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/crypto/index.js b/src/crypto/index.js index 5e4e7e9fa..4985ba242 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -637,8 +637,14 @@ Crypto.prototype.bootstrapSecretStorage = async function({ // keys should be trusted logger.log("Cross-signing private keys found in secret storage"); + // TODO: take this use case out of bootstrapping // fetch the private keys and set up our local copy of the keys for // use + // + // so if some other device resets the cross-signing keys, + // we mark them as untrusted from _onDeviceListUserCrossSigningUpdated + // you can either fix this by hitting the verify this session which (might?) call this method, + // or the reset button in the settings await this.checkOwnCrossSigningTrust(); if (oldKeyInfo && oldKeyInfo.algorithm === SECRET_STORAGE_ALGORITHM_V1_AES) { From c11796af4bb0849bf037caeafefc73630ff24576 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 17 Jun 2020 15:29:21 +0200 Subject: [PATCH 07/12] don't fix old key when we're force-creating a new one --- src/crypto/index.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/crypto/index.js b/src/crypto/index.js index 4985ba242..aab62d7ff 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -585,10 +585,6 @@ Crypto.prototype.bootstrapSecretStorage = async function({ const { keyInfo, privateKey } = await createSecretStorageKey(); newKeyId = await createSSSS(keyInfo, privateKey); } - - if (oldKeyInfo && oldKeyInfo.algorithm === SECRET_STORAGE_ALGORITHM_V1_AES) { - await ensureCanCheckPassphrase(oldKeyId, oldKeyInfo); - } } else if (!inStorage && keyBackupInfo) { // we have an existing backup, but no SSSS From d3b2c8246daa718d34212703ada41f316834333a Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 17 Jun 2020 15:34:02 +0200 Subject: [PATCH 08/12] remove try/finally as we don't monkey patch the callbacks anymore --- src/crypto/index.js | 316 +++++++++++++++++++++----------------------- 1 file changed, 152 insertions(+), 164 deletions(-) diff --git a/src/crypto/index.js b/src/crypto/index.js index aab62d7ff..fc2cbc0a8 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -555,185 +555,173 @@ Crypto.prototype.bootstrapSecretStorage = async function({ } }; - try { - const oldSSSSKey = await this.getSecretStorageKey(); - const [oldKeyId, oldKeyInfo] = oldSSSSKey || [null, null]; - const decryptionKeys = - await this._crossSigningInfo.isStoredInSecretStorage(this._secretStorage); - const inStorage = !setupNewSecretStorage && decryptionKeys; + const oldSSSSKey = await this.getSecretStorageKey(); + const [oldKeyId, oldKeyInfo] = oldSSSSKey || [null, null]; + const decryptionKeys = + await this._crossSigningInfo.isStoredInSecretStorage(this._secretStorage); + const inStorage = !setupNewSecretStorage && decryptionKeys; - if (!inStorage && !keyBackupInfo) { - // either we don't have anything, or we've been asked to restart - // from scratch - logger.log( - "Cross-signing private keys not found in secret storage, " + - "creating new keys", - ); + if (!inStorage && !keyBackupInfo) { + // either we don't have anything, or we've been asked to restart + // from scratch + logger.log( + "Cross-signing private keys not found in secret storage, " + + "creating new keys", + ); - await resetCrossSigning(); + await resetCrossSigning(); - if ( - setupNewSecretStorage || - !oldKeyInfo || - oldKeyInfo.algorithm !== SECRET_STORAGE_ALGORITHM_V1_AES - ) { - // if we already have a usable default SSSS key and aren't resetting SSSS just use it. - // otherwise, create a new one - // Note: we leave the old SSSS key in place: there could be other secrets using it, in theory. - // We could move them to the new key but a) that would mean we'd need to prompt for the old - // passphrase, and b) it's not clear that would be the right thing to do anyway. - const { keyInfo, privateKey } = await createSecretStorageKey(); - newKeyId = await createSSSS(keyInfo, privateKey); - } - } else if (!inStorage && keyBackupInfo) { - // we have an existing backup, but no SSSS - - logger.log("Secret storage default key not found, using key backup key"); - - // if we have the backup key already cached, use it; otherwise use the - // callback to prompt for the key - const backupKey = await this.getSessionBackupPrivateKey() || - await getKeyBackupPassphrase(); - - // create new cross-signing keys - await resetCrossSigning(); - - // create a new SSSS key and use the backup key as the new SSSS key - const opts = {}; - - if ( - keyBackupInfo.auth_data.private_key_salt && - keyBackupInfo.auth_data.private_key_iterations - ) { - opts.passphrase = { - algorithm: "m.pbkdf2", - iterations: keyBackupInfo.auth_data.private_key_iterations, - salt: keyBackupInfo.auth_data.private_key_salt, - bits: 256, - }; - } - - newKeyId = await createSSSS(opts, backupKey); - - // store the backup key in secret storage - await secretStorage.store( - "m.megolm_backup.v1", olmlib.encodeBase64(backupKey), [newKeyId], - ); - - // The backup is trusted because the user provided the private key. - // Sign the backup with the cross signing key so the key backup can - // be trusted via cross-signing. - logger.log("Adding cross signing signature to key backup"); - await crossSigningInfo.signObject( - keyBackupInfo.auth_data, "master", - ); - builder.addSessionBackup(keyBackupInfo); - } else if (!this._crossSigningInfo.getId()) { - // we have SSSS, but we don't know if the server's cross-signing - // keys should be trusted - logger.log("Cross-signing private keys found in secret storage"); - - // TODO: take this use case out of bootstrapping - // fetch the private keys and set up our local copy of the keys for - // use - // - // so if some other device resets the cross-signing keys, - // we mark them as untrusted from _onDeviceListUserCrossSigningUpdated - // you can either fix this by hitting the verify this session which (might?) call this method, - // or the reset button in the settings - await this.checkOwnCrossSigningTrust(); - - if (oldKeyInfo && oldKeyInfo.algorithm === SECRET_STORAGE_ALGORITHM_V1_AES) { - // make sure that the default key has the information needed to - // check the passphrase - await ensureCanCheckPassphrase(oldKeyId, oldKeyInfo); - } - } else { - // we have SSSS and we cross-signing is already set up - logger.log("Cross signing keys are present in secret storage"); - - if (oldKeyInfo && oldKeyInfo.algorithm === SECRET_STORAGE_ALGORITHM_V1_AES) { - // make sure that the default key has the information needed to - // check the passphrase - await ensureCanCheckPassphrase(oldKeyId, oldKeyInfo); - } + if ( + setupNewSecretStorage || + !oldKeyInfo || + oldKeyInfo.algorithm !== SECRET_STORAGE_ALGORITHM_V1_AES + ) { + // if we already have a usable default SSSS key and aren't resetting SSSS just use it. + // otherwise, create a new one + // Note: we leave the old SSSS key in place: there could be other secrets using it, in theory. + // We could move them to the new key but a) that would mean we'd need to prompt for the old + // passphrase, and b) it's not clear that would be the right thing to do anyway. + const { keyInfo, privateKey } = await createSecretStorageKey(); + newKeyId = await createSSSS(keyInfo, privateKey); } + } else if (!inStorage && keyBackupInfo) { + // we have an existing backup, but no SSSS - const crossSigningPrivateKeys = builder.crossSigningCallbacks.privateKeys; - if (crossSigningPrivateKeys.size) { - logger.log("Storing cross-signing private keys in secret storage"); - // Assuming no app-supplied callback, default to storing in SSSS. - if (!this._baseApis._cryptoCallbacks.saveCrossSigningKeys) { - // this is writing to in-memory account data in builder.accountDataClientAdapter - // so won't fail - await CrossSigningInfo.storeInSecretStorage( - crossSigningPrivateKeys, - secretStorage, - ); - } - } + logger.log("Secret storage default key not found, using key backup key"); - if (setupNewKeyBackup && !keyBackupInfo) { - const info = await this._baseApis.prepareKeyBackupVersion( - null /* random key */, - // don't write to secret storage, as it will write to this._secretStorage. - // Here, we want to capture all the side-effects of bootstrapping, - // and want to write to the local secretStorage object - { secureSecretStorage: false }, - ); - // write the key ourselves to 4S - const privateKey = decodeRecoveryKey(info.recovery_key); - await secretStorage.store("m.megolm_backup.v1", olmlib.encodeBase64(privateKey)); + // if we have the backup key already cached, use it; otherwise use the + // callback to prompt for the key + const backupKey = await this.getSessionBackupPrivateKey() || + await getKeyBackupPassphrase(); - // create keyBackupInfo object to add to builder - const data = { - algorithm: info.algorithm, - auth_data: info.auth_data, + // create new cross-signing keys + await resetCrossSigning(); + + // create a new SSSS key and use the backup key as the new SSSS key + const opts = {}; + + if ( + keyBackupInfo.auth_data.private_key_salt && + keyBackupInfo.auth_data.private_key_iterations + ) { + opts.passphrase = { + algorithm: "m.pbkdf2", + iterations: keyBackupInfo.auth_data.private_key_iterations, + salt: keyBackupInfo.auth_data.private_key_salt, + bits: 256, }; - // sign with cross-sign master key - await crossSigningInfo.signObject(data.auth_data, "master"); - // sign with the device fingerprint - await this._signObject(data.auth_data); - - - builder.addSessionBackup(data); } - // and likewise for the session backup key - const sessionBackupKey = await secretStorage.get('m.megolm_backup.v1'); - if (sessionBackupKey) { - logger.info("Got session backup key from secret storage: caching"); - // fix up the backup key if it's in the wrong format, and replace - // in secret storage - const fixedBackupKey = fixBackupKey(sessionBackupKey); - if (fixedBackupKey) { - await secretStorage.store("m.megolm_backup.v1", - fixedBackupKey, [newKeyId || oldKeyId], - ); - } - const decodedBackupKey = new Uint8Array(olmlib.decodeBase64( - fixedBackupKey || sessionBackupKey, - )); - await builder.addSessionBackupPrivateKeyToCache(decodedBackupKey); + newKeyId = await createSSSS(opts, backupKey); + + // store the backup key in secret storage + await secretStorage.store( + "m.megolm_backup.v1", olmlib.encodeBase64(backupKey), [newKeyId], + ); + + // The backup is trusted because the user provided the private key. + // Sign the backup with the cross signing key so the key backup can + // be trusted via cross-signing. + logger.log("Adding cross signing signature to key backup"); + await crossSigningInfo.signObject( + keyBackupInfo.auth_data, "master", + ); + builder.addSessionBackup(keyBackupInfo); + } else if (!this._crossSigningInfo.getId()) { + // we have SSSS, but we don't know if the server's cross-signing + // keys should be trusted + logger.log("Cross-signing private keys found in secret storage"); + + // TODO: take this use case out of bootstrapping + // fetch the private keys and set up our local copy of the keys for + // use + // + // so if some other device resets the cross-signing keys, + // we mark them as untrusted from _onDeviceListUserCrossSigningUpdated + // you can either fix this by hitting the verify this session which (might?) call this method, + // or the reset button in the settings + await this.checkOwnCrossSigningTrust(); + + if (oldKeyInfo && oldKeyInfo.algorithm === SECRET_STORAGE_ALGORITHM_V1_AES) { + // make sure that the default key has the information needed to + // check the passphrase + await ensureCanCheckPassphrase(oldKeyId, oldKeyInfo); } - } finally { - // Restore the original callbacks. NB. we must do this by manipulating - // the same object since the CrossSigning class has a reference to the - // object, so if we assign the object here then our callbacks will change - // but the instances of the CrossSigning class will be left with our - // random, otherwise dead closures. - for (const cb of Object.keys(this._baseApis._cryptoCallbacks)) { - delete this._baseApis._cryptoCallbacks[cb]; + } else { + // we have SSSS and we cross-signing is already set up + logger.log("Cross signing keys are present in secret storage"); + + if (oldKeyInfo && oldKeyInfo.algorithm === SECRET_STORAGE_ALGORITHM_V1_AES) { + // make sure that the default key has the information needed to + // check the passphrase + await ensureCanCheckPassphrase(oldKeyId, oldKeyInfo); } - Object.assign(this._baseApis._cryptoCallbacks, appCallbacks); } - const operation = builder.buildOperation(); - await operation.apply(this); - // this persists private keys and public keys as trusted, - // only do this if apply succeeded for now as retry isn't in place yet - await builder.persist(this); + const crossSigningPrivateKeys = builder.crossSigningCallbacks.privateKeys; + if (crossSigningPrivateKeys.size) { + logger.log("Storing cross-signing private keys in secret storage"); + // Assuming no app-supplied callback, default to storing in SSSS. + if (!this._baseApis._cryptoCallbacks.saveCrossSigningKeys) { + // this is writing to in-memory account data in builder.accountDataClientAdapter + // so won't fail + await CrossSigningInfo.storeInSecretStorage( + crossSigningPrivateKeys, + secretStorage, + ); + } } + + if (setupNewKeyBackup && !keyBackupInfo) { + const info = await this._baseApis.prepareKeyBackupVersion( + null /* random key */, + // don't write to secret storage, as it will write to this._secretStorage. + // Here, we want to capture all the side-effects of bootstrapping, + // and want to write to the local secretStorage object + { secureSecretStorage: false }, + ); + // write the key ourselves to 4S + const privateKey = decodeRecoveryKey(info.recovery_key); + await secretStorage.store("m.megolm_backup.v1", olmlib.encodeBase64(privateKey)); + + // create keyBackupInfo object to add to builder + const data = { + algorithm: info.algorithm, + auth_data: info.auth_data, + }; + // sign with cross-sign master key + await crossSigningInfo.signObject(data.auth_data, "master"); + // sign with the device fingerprint + await this._signObject(data.auth_data); + + + builder.addSessionBackup(data); + } + + // and likewise for the session backup key + const sessionBackupKey = await secretStorage.get('m.megolm_backup.v1'); + if (sessionBackupKey) { + logger.info("Got session backup key from secret storage: caching"); + // fix up the backup key if it's in the wrong format, and replace + // in secret storage + const fixedBackupKey = fixBackupKey(sessionBackupKey); + if (fixedBackupKey) { + await secretStorage.store("m.megolm_backup.v1", + fixedBackupKey, [newKeyId || oldKeyId], + ); + } + const decodedBackupKey = new Uint8Array(olmlib.decodeBase64( + fixedBackupKey || sessionBackupKey, + )); + await builder.addSessionBackupPrivateKeyToCache(decodedBackupKey); + } + + const operation = builder.buildOperation(); + await operation.apply(this); + // this persists private keys and public keys as trusted, + // only do this if apply succeeded for now as retry isn't in place yet + await builder.persist(this); + logger.log("Secure Secret Storage ready"); }; From c5eb290e66a6b0081f8a1b1a1dc8631ea25d17f1 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 17 Jun 2020 15:34:30 +0200 Subject: [PATCH 09/12] remove resetCrossSigningKeys it was only used by the bootstrap method in js-sdk, and was not used in react-sdk either. This is a breaking change, in case anything other than react-sdk was using this. --- src/client.js | 12 ----------- src/crypto/index.js | 52 +-------------------------------------------- 2 files changed, 1 insertion(+), 63 deletions(-) diff --git a/src/client.js b/src/client.js index fc73b41f0..345b979f2 100644 --- a/src/client.js +++ b/src/client.js @@ -1075,17 +1075,6 @@ function wrapCryptoFuncs(MatrixClient, names) { } } - /** - * Generate new cross-signing keys. - * The cross-signing API is currently UNSTABLE and may change without notice. - * - * @function module:client~MatrixClient#resetCrossSigningKeys - * @param {object} authDict Auth data to supply for User-Interactive auth. - * @param {CrossSigningLevel} [level] the level of cross-signing to reset. New - * keys will be created for the given level and below. Defaults to - * regenerating all keys. - */ - /** * Get the user's cross-signing key ID. * The cross-signing API is currently UNSTABLE and may change without notice. @@ -1155,7 +1144,6 @@ function wrapCryptoFuncs(MatrixClient, names) { * @param {module:models/room} room the room the event is in */ wrapCryptoFuncs(MatrixClient, [ - "resetCrossSigningKeys", "getCrossSigningId", "getStoredCrossSigningForUser", "checkUserTrust", diff --git a/src/crypto/index.js b/src/crypto/index.js index fc2cbc0a8..8a632f213 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -473,6 +473,7 @@ Crypto.prototype.isCrossSigningReady = async function() { * {Promise} A promise which resolves to key creation data for * SecretStorage#addKey: an object with `passphrase` and/or `pubkey` fields. */ + Crypto.prototype.bootstrapSecretStorage = async function({ authUploadDeviceSigningKeys = async func => await func(), createSecretStorageKey = async () => ({ }), @@ -882,57 +883,6 @@ Crypto.prototype.checkCrossSigningPrivateKey = function(privateKey, expectedPubl } }; -/** - * Generate new cross-signing keys. - * - * @param {CrossSigningLevel} [level] the level of cross-signing to reset. New - * keys will be created for the given level and below. Defaults to - * regenerating all keys. - * @param {function} [opts.authUploadDeviceSigningKeys] Optional. Function - * called to await an interactive auth flow when uploading device signing keys. - * Args: - * {function} A function that makes the request requiring auth. Receives the - * auth data as an object. - */ -Crypto.prototype.resetCrossSigningKeys = async function(level, { - authUploadDeviceSigningKeys = async func => await func(), -} = {}) { - logger.info(`Resetting cross-signing keys at level ${level}`); - // Copy old keys (usually empty) in case we need to revert - const oldKeys = Object.assign({}, this._crossSigningInfo.keys); - try { - await this._crossSigningInfo.resetKeys(level); - await this._signObject(this._crossSigningInfo.keys.master); - - // send keys to server first before storing as trusted locally - // to ensure upload succeeds - const keys = {}; - for (const [name, key] of Object.entries(this._crossSigningInfo.keys)) { - keys[name + "_key"] = key; - } - await authUploadDeviceSigningKeys(async authDict => { - await this._baseApis.uploadDeviceSigningKeys(authDict, keys); - }); - - // 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("Resetting cross-signing keys failed, revert to previous keys", e); - this._crossSigningInfo.keys = oldKeys; - throw e; - } - this._baseApis.emit("crossSigning.keysChanged", {}); - await this._afterCrossSigningLocalKeyChange(); - logger.info("Cross-signing key reset complete"); -}; - /** * Run various follow-up actions after cross-signing keys have changed locally * (either by resetting the keys for the account or by getting them from secret From 75fe596e248cb112100c368a9fd31e9949ed23dd Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 17 Jun 2020 15:36:24 +0200 Subject: [PATCH 10/12] fix tests --- spec/unit/crypto/backup.spec.js | 3 +- spec/unit/crypto/cross-signing.spec.js | 19 +++++----- spec/unit/crypto/crypto-utils.js | 44 +++++++++++++++++++++++ spec/unit/crypto/secrets.spec.js | 5 +-- spec/unit/crypto/verification/sas.spec.js | 5 +-- 5 files changed, 63 insertions(+), 13 deletions(-) create mode 100644 spec/unit/crypto/crypto-utils.js diff --git a/spec/unit/crypto/backup.spec.js b/spec/unit/crypto/backup.spec.js index e03460fe0..34766b871 100644 --- a/spec/unit/crypto/backup.spec.js +++ b/spec/unit/crypto/backup.spec.js @@ -27,6 +27,7 @@ import {MockStorageApi} from "../../MockStorageApi"; import * as testUtils from "../../test-utils"; import {OlmDevice} from "../../../src/crypto/OlmDevice"; import {Crypto} from "../../../src/crypto"; +import {resetCrossSigningKeys} from "./crypto-utils"; const Olm = global.Olm; @@ -332,7 +333,7 @@ describe("MegolmBackup", function() { client.on("crossSigning.getKey", function(e) { e.done(privateKeys[e.type]); }); - await client.resetCrossSigningKeys(); + await resetCrossSigningKeys(client); let numCalls = 0; await new Promise((resolve, reject) => { client._http.authedRequest = function( diff --git a/spec/unit/crypto/cross-signing.spec.js b/spec/unit/crypto/cross-signing.spec.js index ccdbef0bc..a15239a0c 100644 --- a/spec/unit/crypto/cross-signing.spec.js +++ b/spec/unit/crypto/cross-signing.spec.js @@ -20,6 +20,7 @@ import anotherjson from 'another-json'; import * as olmlib from "../../../src/crypto/olmlib"; import {TestClient} from '../../TestClient'; import {HttpResponse, setHttpResponses} from '../../test-utils'; +import {resetCrossSigningKeys, createSecretStorageKey} from "./crypto-utils"; async function makeTestClient(userInfo, options, keys) { if (!keys) keys = {}; @@ -66,8 +67,10 @@ describe("Cross Signing", function() { ); }); alice.uploadKeySignatures = async () => {}; + alice.setAccountData = async () => {}; + alice.getAccountDataFromServer = async () => {}; // set Alice's cross-signing key - await alice.resetCrossSigningKeys(); + await alice.bootstrapSecretStorage({createSecretStorageKey}); expect(alice.uploadDeviceSigningKeys).toHaveBeenCalled(); }); @@ -78,7 +81,7 @@ describe("Cross Signing", function() { alice.uploadDeviceSigningKeys = async () => {}; alice.uploadKeySignatures = async () => {}; // set Alice's cross-signing key - await alice.resetCrossSigningKeys(); + await resetCrossSigningKeys(alice); // Alice downloads Bob's device key alice._crypto._deviceList.storeCrossSigningForUser("@bob:example.com", { keys: { @@ -273,7 +276,7 @@ describe("Cross Signing", function() { alice.uploadDeviceSigningKeys = async () => {}; alice.uploadKeySignatures = async () => {}; // set Alice's cross-signing key - await alice.resetCrossSigningKeys(); + await resetCrossSigningKeys(alice); // Alice downloads Bob's ssk and device key const bobMasterSigning = new global.Olm.PkSigning(); const bobMasterPrivkey = bobMasterSigning.generate_seed(); @@ -363,7 +366,7 @@ describe("Cross Signing", function() { alice.uploadKeySignatures = async () => {}; // set Alice's cross-signing key - await alice.resetCrossSigningKeys(); + await resetCrossSigningKeys(alice); const selfSigningKey = new Uint8Array([ 0x1e, 0xf4, 0x01, 0x6d, 0x4f, 0xa1, 0x73, 0x66, @@ -520,7 +523,7 @@ describe("Cross Signing", function() { alice.uploadDeviceSigningKeys = async () => {}; alice.uploadKeySignatures = async () => {}; // set Alice's cross-signing key - await alice.resetCrossSigningKeys(); + await resetCrossSigningKeys(alice); // Alice downloads Bob's ssk and device key // (NOTE: device key is not signed by ssk) const bobMasterSigning = new global.Olm.PkSigning(); @@ -588,7 +591,7 @@ describe("Cross Signing", function() { ); alice.uploadDeviceSigningKeys = async () => {}; alice.uploadKeySignatures = async () => {}; - await alice.resetCrossSigningKeys(); + await resetCrossSigningKeys(alice); // Alice downloads Bob's keys const bobMasterSigning = new global.Olm.PkSigning(); const bobMasterPrivkey = bobMasterSigning.generate_seed(); @@ -740,7 +743,7 @@ describe("Cross Signing", function() { bob.uploadDeviceSigningKeys = async () => {}; bob.uploadKeySignatures = async () => {}; // set Bob's cross-signing key - await bob.resetCrossSigningKeys(); + await resetCrossSigningKeys(bob); alice._crypto._deviceList.storeDevicesForUser("@bob:example.com", { Dynabook: { algorithms: ["m.olm.curve25519-aes-sha256", "m.megolm.v1.aes-sha"], @@ -766,7 +769,7 @@ describe("Cross Signing", function() { let upgradePromise = new Promise((resolve) => { upgradeResolveFunc = resolve; }); - await alice.resetCrossSigningKeys(); + await resetCrossSigningKeys(alice); await upgradePromise; const bobTrust = alice.checkUserTrust("@bob:example.com"); diff --git a/spec/unit/crypto/crypto-utils.js b/spec/unit/crypto/crypto-utils.js new file mode 100644 index 000000000..b54802b05 --- /dev/null +++ b/spec/unit/crypto/crypto-utils.js @@ -0,0 +1,44 @@ +import {IndexedDBCryptoStore} from '../../../src/crypto/store/indexeddb-crypto-store'; + + +// needs to be phased out and replaced with bootstrapSecretStorage, +// but that is doing too much extra stuff for it to be an easy transition. +export async function resetCrossSigningKeys(client, { + level, + authUploadDeviceSigningKeys = async func => await func(), +} = {}) { + const crypto = client._crypto; + + const oldKeys = Object.assign({}, crypto._crossSigningInfo.keys); + try { + await crypto._crossSigningInfo.resetKeys(level); + await crypto._signObject(crypto._crossSigningInfo.keys.master); + // write a copy locally so we know these are trusted keys + await crypto._cryptoStore.doTxn( + 'readwrite', [IndexedDBCryptoStore.STORE_ACCOUNT], + (txn) => { + crypto._cryptoStore.storeCrossSigningKeys( + txn, crypto._crossSigningInfo.keys); + }, + ); + } catch (e) { + // If anything failed here, revert the keys so we know to try again from the start + // next time. + crypto._crossSigningInfo.keys = oldKeys; + throw e; + } + crypto._baseApis.emit("crossSigning.keysChanged", {}); + await crypto._afterCrossSigningLocalKeyChange(); +} + +export async function createSecretStorageKey() { + const decryption = new global.Olm.PkDecryption(); + const storagePublicKey = decryption.generate_key(); + const storagePrivateKey = decryption.get_private_key(); + decryption.free(); + return { + // `pubkey` not used anymore with symmetric 4S + keyInfo: { pubkey: storagePublicKey }, + privateKey: storagePrivateKey, + }; +} diff --git a/spec/unit/crypto/secrets.spec.js b/spec/unit/crypto/secrets.spec.js index d60ca1ba5..74748667f 100644 --- a/spec/unit/crypto/secrets.spec.js +++ b/spec/unit/crypto/secrets.spec.js @@ -21,6 +21,7 @@ import {MatrixEvent} from "../../../src/models/event"; import {TestClient} from '../../TestClient'; import {makeTestClients} from './verification/util'; import {encryptAES} from "../../../src/crypto/aes"; +import {resetCrossSigningKeys, createSecretStorageKey} from "./crypto-utils"; import * as utils from "../../../src/utils"; @@ -190,7 +191,7 @@ describe("Secrets", function() { }), ]); }; - alice.resetCrossSigningKeys(); + resetCrossSigningKeys(alice); const newKeyId = await alice.addSecretStorageKey( SECRET_STORAGE_ALGORITHM_V1_AES, @@ -325,7 +326,7 @@ describe("Secrets", function() { this.emit("accountData", event); }; - await bob.bootstrapSecretStorage(); + await bob.bootstrapSecretStorage({createSecretStorageKey}); const crossSigning = bob._crypto._crossSigningInfo; const secretStorage = bob._crypto._secretStorage; diff --git a/spec/unit/crypto/verification/sas.spec.js b/spec/unit/crypto/verification/sas.spec.js index 88f424fe8..607712318 100644 --- a/spec/unit/crypto/verification/sas.spec.js +++ b/spec/unit/crypto/verification/sas.spec.js @@ -22,6 +22,7 @@ import {DeviceInfo} from "../../../../src/crypto/deviceinfo"; import {verificationMethods} from "../../../../src/crypto"; import * as olmlib from "../../../../src/crypto/olmlib"; import {logger} from "../../../../src/logger"; +import {resetCrossSigningKeys} from "../crypto-utils"; const Olm = global.Olm; @@ -288,12 +289,12 @@ describe("SAS verification", function() { ); alice.httpBackend.when('POST', '/keys/signatures/upload').respond(200, {}); alice.httpBackend.flush(undefined, 2); - await alice.client.resetCrossSigningKeys(); + await resetCrossSigningKeys(alice.client); bob.httpBackend.when('POST', '/keys/device_signing/upload').respond(200, {}); bob.httpBackend.when('POST', '/keys/signatures/upload').respond(200, {}); bob.httpBackend.flush(undefined, 2); - await bob.client.resetCrossSigningKeys(); + await resetCrossSigningKeys(bob.client); bob.client._crypto._deviceList.storeCrossSigningForUser( "@alice:example.com", { From 1fe9dd03a3f45fe5655f9a647c3205d19b9cd936 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 18 Jun 2020 14:36:59 +0200 Subject: [PATCH 11/12] apparently we call the callback also to obtain flows, so handle that --- src/crypto/index.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/crypto/index.js b/src/crypto/index.js index 8a632f213..f2a372606 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -522,8 +522,15 @@ Crypto.prototype.bootstrapSecretStorage = async function({ await this._signObject(crossSigningInfo.keys.master); await authUploadDeviceSigningKeys(authDict => { - builder.addCrossSigningKeys(authDict, crossSigningInfo.keys); - return Promise.resolve(); + if (authDict) { + builder.addCrossSigningKeys(authDict, crossSigningInfo.keys); + return Promise.resolve(); + } else { + // This callback also gets called to obtain the IUA flows, + // so do a call to obtain those if we don't have the authDict yet + // We should get called again at a later point with the authDict. + return this._baseApis.uploadDeviceSigningKeys(null, {}); + } }); // cross-sign own device From f62049559cfa0320344c3236568f2b8c3c281751 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 18 Jun 2020 15:06:34 +0200 Subject: [PATCH 12/12] make authUploadDeviceSigningKeys required, as default is only used by tests and make tests pass {} instead of undefined for authDict as otherwise we'll enter in the "obtain flows" mode and not add the keys to the builder --- spec/unit/crypto/cross-signing.spec.js | 5 ++++- spec/unit/crypto/secrets.spec.js | 18 ++++++++++++++---- src/crypto/index.js | 6 +++--- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/spec/unit/crypto/cross-signing.spec.js b/spec/unit/crypto/cross-signing.spec.js index a15239a0c..7f24d1c7e 100644 --- a/spec/unit/crypto/cross-signing.spec.js +++ b/spec/unit/crypto/cross-signing.spec.js @@ -70,7 +70,10 @@ describe("Cross Signing", function() { alice.setAccountData = async () => {}; alice.getAccountDataFromServer = async () => {}; // set Alice's cross-signing key - await alice.bootstrapSecretStorage({createSecretStorageKey}); + await alice.bootstrapSecretStorage({ + createSecretStorageKey, + authUploadDeviceSigningKeys: async func => await func({}), + }); expect(alice.uploadDeviceSigningKeys).toHaveBeenCalled(); }); diff --git a/spec/unit/crypto/secrets.spec.js b/spec/unit/crypto/secrets.spec.js index 74748667f..ceb01e716 100644 --- a/spec/unit/crypto/secrets.spec.js +++ b/spec/unit/crypto/secrets.spec.js @@ -326,7 +326,10 @@ describe("Secrets", function() { this.emit("accountData", event); }; - await bob.bootstrapSecretStorage({createSecretStorageKey}); + await bob.bootstrapSecretStorage({ + createSecretStorageKey, + authUploadDeviceSigningKeys: async func => await func({}), + }); const crossSigning = bob._crypto._crossSigningInfo; const secretStorage = bob._crypto._secretStorage; @@ -382,6 +385,7 @@ describe("Secrets", function() { keyInfo: { pubkey: storagePublicKey }, privateKey: storagePrivateKey, }), + authUploadDeviceSigningKeys: async func => await func({}), }); // Clear local cross-signing keys and read from secret storage @@ -390,7 +394,9 @@ describe("Secrets", function() { crossSigning.toStorage(), ); crossSigning.keys = {}; - await bob.bootstrapSecretStorage(); + await bob.bootstrapSecretStorage({ + authUploadDeviceSigningKeys: async func => await func({}), + }); expect(crossSigning.getId()).toBeTruthy(); expect(await crossSigning.isStoredInSecretStorage(secretStorage)) @@ -511,7 +517,9 @@ describe("Secrets", function() { this.emit("accountData", event); }; - await alice.bootstrapSecretStorage(); + await alice.bootstrapSecretStorage({ + authUploadDeviceSigningKeys: async func => await func({}), + }); expect(alice.getAccountData("m.secret_storage.default_key").getContent()) .toEqual({key: "key_id"}); @@ -651,7 +659,9 @@ describe("Secrets", function() { this.emit("accountData", event); }; - await alice.bootstrapSecretStorage(); + await alice.bootstrapSecretStorage({ + authUploadDeviceSigningKeys: async func => await func({}), + }); const backupKey = alice.getAccountData("m.megolm_backup.v1") .getContent(); diff --git a/src/crypto/index.js b/src/crypto/index.js index f2a372606..370833024 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -449,11 +449,11 @@ Crypto.prototype.isCrossSigningReady = async function() { * - migrates Secure Secret Storage to use the latest algorithm, if an outdated * algorithm is found * - * @param {function} [opts.authUploadDeviceSigningKeys] Optional. Function + * @param {function} opts.authUploadDeviceSigningKeys Function * called to await an interactive auth flow when uploading device signing keys. * Args: * {function} A function that makes the request requiring auth. Receives the - * auth data as an object. + * auth data as an object. Can be called multiple times, first with an empty authDict, to obtain the flows. * @param {function} [opts.createSecretStorageKey] Optional. Function * called to await a secret storage key creation flow. * Returns: @@ -475,7 +475,7 @@ Crypto.prototype.isCrossSigningReady = async function() { */ Crypto.prototype.bootstrapSecretStorage = async function({ - authUploadDeviceSigningKeys = async func => await func(), + authUploadDeviceSigningKeys, createSecretStorageKey = async () => ({ }), keyBackupInfo, setupNewKeyBackup,