From c5caf8f8f41ebde92cb0527ca32ae770ef09ecec Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Wed, 3 Jul 2019 15:15:41 -0400 Subject: [PATCH] sign backups with master key --- src/client.js | 194 +++++-------------------------------- src/crypto/CrossSigning.js | 55 ++++------- src/crypto/index.js | 113 +++------------------ src/crypto/sskinfo.js | 80 --------------- 4 files changed, 58 insertions(+), 384 deletions(-) delete mode 100644 src/crypto/sskinfo.js diff --git a/src/client.js b/src/client.js index fa3543a52..8d9c8f404 100644 --- a/src/client.js +++ b/src/client.js @@ -1209,16 +1209,8 @@ MatrixClient.prototype.prepareKeyBackupVersion = async function(password) { throw new Error("End-to-end encryption disabled"); } - let decryption; - let encryption; - let signing; + const decryption = new global.Olm.PkDecryption(); try { - decryption = new global.Olm.PkDecryption(); - encryption = new global.Olm.PkEncryption(); - if (global.Olm.PkSigning) { - signing = new global.Olm.PkSigning(); - } - let publicKey; const authData = {}; if (password) { @@ -1231,64 +1223,15 @@ MatrixClient.prototype.prepareKeyBackupVersion = async function(password) { } authData.public_key = publicKey; - encryption.set_recipient_key(publicKey); - const returnInfo = { + return { algorithm: olmlib.MEGOLM_BACKUP_ALGORITHM, auth_data: authData, recovery_key: encodeRecoveryKey(decryption.get_private_key()), accountKeys: null, }; - - if (signing) { - await this._cryptoStore.doTxn( - 'readonly', [IndexedDBCryptoStore.STORE_ACCOUNT], - (txn) => { - this._cryptoStore.getAccountKeys(txn, (keys) => { - returnInfo.accountKeys = keys; - }); - }, - ); - - if (!returnInfo.accountKeys) { - const sskSeed = signing.generate_seed(); - const uskSeed = signing.generate_seed(); - - returnInfo.accountKeys = { - self_signing_key_seed: Buffer.from(sskSeed).toString('base64'), - user_signing_key_seed: Buffer.from(uskSeed).toString('base64'), - }; - } - - // put the encrypted version of the seed in the auth data to upload - // XXX: our encryption really should support encrypting binary data. - authData.self_signing_key_seed = encryption.encrypt( - returnInfo.accountKeys.self_signing_key_seed, - ); - // also keep the public part there - returnInfo.ssk_public = signing.init_with_seed( - Buffer.from(returnInfo.accountKeys.self_signing_key_seed, 'base64'), - ); - signing.free(); - - // same for the USK - authData.user_signing_key_seed = encryption.encrypt( - returnInfo.accountKeys.user_signing_key_seed, - ); - returnInfo.usk_public = signing.init_with_seed( - Buffer.from(returnInfo.accountKeys.user_signing_key_seed, 'base64'), - ); - signing.free(); - - // we don't save these keys back to the store yet: we'll do that when (if) we - // actually create the backup - } - - return returnInfo; } finally { - if (decryption) decryption.free(); - if (encryption) encryption.free(); - if (signing) signing.free(); + decryption.free(); } }; @@ -1297,11 +1240,9 @@ MatrixClient.prototype.prepareKeyBackupVersion = async function(password) { * from prepareKeyBackupVersion. * * @param {object} info Info object from prepareKeyBackupVersion - * @param {object} auth Auth object for UI auth - * @param {string} replacesSsk If the SSK is being replaced, the ID of the old key * @returns {Promise} Object with 'version' param indicating the version created */ -MatrixClient.prototype.createKeyBackupVersion = async function(info, auth, replacesSsk) { +MatrixClient.prototype.createKeyBackupVersion = async function(info) { if (this._crypto === null) { throw new Error("End-to-end encryption disabled"); } @@ -1311,73 +1252,25 @@ MatrixClient.prototype.createKeyBackupVersion = async function(info, auth, repla auth_data: info.auth_data, }; - const uskInfo = { - user_id: this.credentials.userId, - usage: ['user_signing'], - keys: { - ['ed25519:' + info.usk_public]: info.usk_public, - }, - }; - - // sign the USK with the SSK - pkSign( - uskInfo, - Buffer.from(info.accountKeys.self_signing_key_seed, 'base64'), - this.credentials.userId, - ); - // Now sig the backup auth data. Do it as this device first because crypto._signObject // is dumb and bluntly replaces the whole signatures block... // this can probably go away very soon in favour of just signing with the SSK. await this._crypto._signObject(data.auth_data); - // now also sign the auth data with the SSK - pkSign( - data.auth_data, - Buffer.from(info.accountKeys.self_signing_key_seed, 'base64'), - this.credentials.userId, + if (this._crypto._crossSigningInfo.getId()) { + // now also sign the auth data with the SSK + await this._crypto._crossSigningInfo.signObject(data.auth_data, "master"); + } + + const res = await this._http.authedRequest( + undefined, "POST", "/room_keys/version", undefined, data, ); - - const keys = { - self_signing_key: { - user_id: this.credentials.userId, - usage: ['self_signing'], - keys: { - ['ed25519:' + info.ssk_public]: info.ssk_public, - }, - replaces: replacesSsk, - }, - user_signing_key: uskInfo, - auth, - }; - - return this._cryptoStore.doTxn( - 'readwrite', [IndexedDBCryptoStore.STORE_ACCOUNT], - (txn) => { - // store the newly generated account keys - this._cryptoStore.storeAccountKeys(txn, info.accountKeys); - }, - ).then(() => { - // re-check the SSK in the device store if necessary - return this._crypto.checkOwnSskTrust(); - }).then(() => { - // upload the public part of the account keys - return this.uploadDeviceSigningKeys(keys); - }).then(() => { - return this._http.authedRequest( - undefined, "POST", "/room_keys/version", undefined, data, - ); - }).then((res) => { - this.enableKeyBackup({ - algorithm: info.algorithm, - auth_data: info.auth_data, - version: res.version, - }); - return res; - }).then(() => { - // upload signatures between the SSK & this device - return this._crypto.uploadDeviceKeySignatures(); + this.enableKeyBackup({ + algorithm: info.algorithm, + auth_data: info.auth_data, + version: res.version, }); + return res; }; MatrixClient.prototype.deleteKeyBackupVersion = function(version) { @@ -1483,7 +1376,7 @@ MatrixClient.prototype.restoreKeyBackupWithRecoveryKey = function( ); }; -MatrixClient.prototype._restoreKeyBackup = async function( +MatrixClient.prototype._restoreKeyBackup = function( privKey, targetRoomId, targetSessionId, backupInfo, ) { if (this._crypto === null) { @@ -1492,46 +1385,14 @@ MatrixClient.prototype._restoreKeyBackup = async function( let totalKeyCount = 0; let keys = []; + const path = this._makeKeyBackupPath( + targetRoomId, targetSessionId, backupInfo.version, + ); + const decryption = new global.Olm.PkDecryption(); let backupPubKey; try { backupPubKey = decryption.init_with_private_key(privKey); - - // decrypt the account keys from the backup info if there are any - // fetch the old ones first so we don't lose info if only one of them is in the backup - let accountKeys; - await this._cryptoStore.doTxn( - 'readonly', [IndexedDBCryptoStore.STORE_ACCOUNT], - (txn) => { - this._cryptoStore.getAccountKeys(txn, (keys) => { - accountKeys = keys || {}; - }); - }, - ); - - if (backupInfo.auth_data.self_signing_key_seed) { - accountKeys.self_signing_key_seed = decryption.decrypt( - backupInfo.auth_data.self_signing_key_seed.ephemeral, - backupInfo.auth_data.self_signing_key_seed.mac, - backupInfo.auth_data.self_signing_key_seed.ciphertext, - ); - } - if (backupInfo.auth_data.user_signing_key_seed) { - accountKeys.user_signing_key_seed = decryption.decrypt( - backupInfo.auth_data.user_signing_key_seed.ephemeral, - backupInfo.auth_data.user_signing_key_seed.mac, - backupInfo.auth_data.user_signing_key_seed.ciphertext, - ); - } - - await this._cryptoStore.doTxn( - 'readwrite', [IndexedDBCryptoStore.STORE_ACCOUNT], - (txn) => { - this._cryptoStore.storeAccountKeys(txn, accountKeys); - }, - ); - - await this._crypto.checkOwnSskTrust(); } catch(e) { decryption.free(); throw e; @@ -1544,16 +1405,9 @@ MatrixClient.prototype._restoreKeyBackup = async function( return Promise.reject({errcode: MatrixClient.RESTORE_BACKUP_ERROR_BAD_KEY}); } - // start by signing this device from the SSK now we have it - return this._crypto.uploadDeviceKeySignatures().then(() => { - // Now fetch the encrypted keys - const path = this._makeKeyBackupPath( - targetRoomId, targetSessionId, backupInfo.version, - ); - return this._http.authedRequest( - undefined, "GET", path.path, path.queryData, - ); - }).then((res) => { + return this._http.authedRequest( + undefined, "GET", path.path, path.queryData, + ).then((res) => { if (res.rooms) { for (const [roomId, roomData] of Object.entries(res.rooms)) { if (!roomData.sessions) continue; diff --git a/src/crypto/CrossSigning.js b/src/crypto/CrossSigning.js index bac387546..1652d5a79 100644 --- a/src/crypto/CrossSigning.js +++ b/src/crypto/CrossSigning.js @@ -40,13 +40,15 @@ async function getPrivateKey(self, type, check) { // FIXME: the key needs to be interpreted? const signing = new global.Olm.PkSigning(); const pubkey = signing.init_with_seed(key); - error = check(pubkey, signing); - if (error) { + // make sure it agrees with the pubkey that we have + if (pubkey !== getPublicKey(self.keys[type])[1]) { + error = "Key does not match"; logger.error(error); signing.free(); resolve([null, null]); + } else { + resolve([pubkey, signing]); } - resolve([pubkey, signing]); }, cancel: (error) => { reject(error || new Error("Cancelled")); @@ -131,14 +133,7 @@ export class CrossSigningInfo extends EventEmitter { }, }; } else { - [masterPub, masterSigning] = await getPrivateKey( - this, "master", (pubkey) => { - // make sure it agrees with the pubkey that we have - if (pubkey !== getPublicKey(this.keys.master)[1]) { - return "Key does not match"; - } - return; - }); + [masterPub, masterSigning] = await getPrivateKey(this, "master"); } if (level & CrossSigningLevel.SELF_SIGNING) { @@ -258,21 +253,21 @@ export class CrossSigningInfo extends EventEmitter { } } + async signObject(data, type) { + const [pubkey, signing] = await getPrivateKey(this, type); + try { + pkSign(data, signing, this.userId, pubkey); + return data; + } finally { + signing.free(); + } + } + async signUser(key) { if (!this.keys.user_signing) { return; } - const [pubkey, usk] = await getPrivateKey(this, "user_signing", (key) => { - // FIXME: - return; - }); - try { - const otherMaster = key.keys.master; - pkSign(otherMaster, usk, this.userId, pubkey); - return otherMaster; - } finally { - usk.free(); - } + return this.signObject(key.keys.master, "user_signing"); } async signDevice(userId, device) { @@ -284,22 +279,14 @@ export class CrossSigningInfo extends EventEmitter { if (!this.keys.self_signing) { return; } - const [pubkey, ssk] = await getPrivateKey(this, "self_signing", (key) => { - // FIXME: - return; - }); - try { - const keyObj = { + return this.signObject( + { algorithms: device.algorithms, keys: device.keys, device_id: device.deviceId, user_id: userId, - }; - pkSign(keyObj, ssk, this.userId, pubkey); - return keyObj; - } finally { - ssk.free(); - } + }, "self_signing", + ); } checkUserTrust(userCrossSigning) { diff --git a/src/crypto/index.js b/src/crypto/index.js index 845a7d1bc..f07e16242 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -513,52 +513,10 @@ Crypto.prototype.checkOwnCrossSigningTrust = async function() { this._baseApis.emit("cross-signing.keysChanged", {}); } - // FIXME: - // Now dig out the account keys and get the pubkey of the one in there - /* - let accountKeys = null; - await this._cryptoStore.doTxn( - 'readonly', [IndexedDBCryptoStore.STORE_ACCOUNT], - (txn) => { - this._cryptoStore.getAccountKeys(txn, keys => { - accountKeys = keys; - }); - }, - ); - if (!accountKeys || !accountKeys.self_signing_key_seed) { - logger.info( - "Ignoring new self-signing key for us because we have no private part stored", - ); - return; - } - let signing; - let localPubkey; - try { - signing = new global.Olm.PkSigning(); - localPubkey = signing.init_with_seed( - Buffer.from(accountKeys.self_signing_key_seed, 'base64'), - ); - } finally { - if (signing) signing.free(); - signing = null; - } - if (!localPubkey) { - logger.error("Unable to compute public key for stored SSK seed"); - } - - // Finally, are they the same? - if (seenPubkey === localPubkey) { - logger.info("Published self-signing key matches local copy: marking as verified"); - this.setSskVerification(userId, SskInfo.SskVerification.VERIFIED); - // Now we may be able to trust our key backup - await this.checkKeyBackup(); - } else { - logger.info( - "Published self-signing key DOES NOT match local copy! Local: " + - localPubkey + ", published: " + seenPubkey, - ); - } - */ + // Now we may be able to trust our key backup + await this.checkKeyBackup(); + // FIXME: if we previously trusted the backup, should we automatically sign + // the backup with the new key (if not already signed)? }; /** @@ -686,21 +644,23 @@ Crypto.prototype.isKeyBackupTrusted = async function(backupInfo) { // Could be an SSK but just say this is the device ID for backwards compat const sigInfo = { deviceId: keyIdParts[1] }; // XXX: is this how we're supposed to get the device ID? - // first check to see if it's from our SSK - const ssk = this._deviceList.getStoredSskForUser(this._userId); - if (ssk && ssk.getKeyId() === keyId) { - sigInfo.self_signing_key = ssk; + // first check to see if it's from our cross-signing key + const crossSigningId = this._crossSigningInfo.getId(); + if (crossSigningId === keyId) { + sigInfo.cross_signing_key = crossSigningId; try { await olmlib.verifySignature( this._olmDevice, backupInfo.auth_data, this._userId, sigInfo.deviceId, - ssk.getFingerprint(), + crossSigningId, ); sigInfo.valid = true; } catch (e) { - console.log("Bad signature from ssk " + ssk.getKeyId(), e); + logger.warning( + "Bad signature from cross signing key " + crossSigningId, e, + ); sigInfo.valid = false; } ret.sigs.push(sigInfo); @@ -745,7 +705,7 @@ Crypto.prototype.isKeyBackupTrusted = async function(backupInfo) { return ( s.valid && ( (s.device && s.device.isVerified()) || - (s.self_signing_key && s.self_signing_key.isVerified()) + (s.cross_signing_key) ) ); }); @@ -843,7 +803,6 @@ Crypto.prototype.uploadDeviceKeys = function() { user_id: userId, }; - let accountKeys; return crypto._signObject(deviceKeys).then(() => { return crypto._baseApis.uploadKeysRequest({ device_keys: deviceKeys, @@ -855,52 +814,6 @@ Crypto.prototype.uploadDeviceKeys = function() { }); }; -/** - * If a self-signing key is available, uploads the signature of this device from - * the self-signing key - * - * @return {bool} Promise: True if signatures were uploaded or otherwise false - * (eg. if no account keys were available) - */ -Crypto.prototype.uploadDeviceKeySignatures = async function() { - const crypto = this; - const userId = crypto._userId; - const deviceId = crypto._deviceId; - - const thisDeviceKey = { - algorithms: crypto._supportedAlgorithms, - device_id: deviceId, - keys: crypto._deviceKeys, - user_id: userId, - }; - let accountKeys; - await this._cryptoStore.doTxn( - 'readonly', [IndexedDBCryptoStore.STORE_ACCOUNT], - (txn) => { - this._cryptoStore.getAccountKeys(txn, keys => { - accountKeys = keys; - }, - ); - }); - if (!accountKeys || !accountKeys.self_signing_key_seed) return false; - - // Sign this device with the SSK - pkSign( - thisDeviceKey, - Buffer.from(accountKeys.self_signing_key_seed, 'base64'), - userId, - ); - - const content = { - [userId]: { - [deviceId]: thisDeviceKey, - }, - }; - - await crypto._baseApis.uploadKeySignatures(content); - return true; -}; - /** * Stores the current one_time_key count which will be handled later (in a call of * onSyncCompleted). The count is e.g. coming from a /sync response. diff --git a/src/crypto/sskinfo.js b/src/crypto/sskinfo.js deleted file mode 100644 index 1f578247f..000000000 --- a/src/crypto/sskinfo.js +++ /dev/null @@ -1,80 +0,0 @@ -/* -Copyright 2019 New Vector Ltd - -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. -*/ - - -/** - * @module crypto/sskinfo - */ - -/** - * Information about a user's self-signing key - * - * @constructor - * @alias module:crypto/sskinfo - * - * @property {Object.} keys a map from - * <key type>:<id> -> <base64-encoded key>> - * - * @property {module:crypto/sskinfo.SskVerification} verified - * whether the device has been verified/blocked by the user - * - * @property {boolean} known - * whether the user knows of this device's existence (useful when warning - * the user that a user has added new devices) - * - * @property {Object} unsigned additional data from the homeserver - */ -export default class SskInfo { - constructor() { - this.keys = {}; - this.verified = SskInfo.SskVerification.UNVERIFIED; - //this.known = false; // is this useful? - this.unsigned = {}; - } - - /** - * @enum - */ - static SskVerification = { - VERIFIED: 1, - UNVERIFIED: 0, - BLOCKED: -1, - }; - - static fromStorage(obj) { - const res = new SskInfo(); - for (const [prop, val] of Object.entries(obj)) { - res[prop] = val; - } - return res; - } - - getKeyId() { - return Object.keys(this.keys)[0]; - } - - getFingerprint() { - return Object.values(this.keys)[0]; - } - - isVerified() { - return this.verified == SskInfo.SskVerification.VERIFIED; - } - - isUnverified() { - return this.verified == SskInfo.SskVerification.UNVERIFIED; - } -}