From bc97e7a5ea3011fcd2c60b273dfa679498dcd0d9 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 15 Jun 2020 17:47:25 -0400 Subject: [PATCH] don't trust keys megolm received from backup for verifying the sender --- spec/unit/crypto/backup.spec.js | 1 + src/client.js | 2 +- src/crypto/OlmDevice.js | 9 ++++++--- src/crypto/algorithms/megolm.js | 8 ++++++-- src/crypto/index.js | 9 +++++++-- src/models/event.js | 9 +++++++++ 6 files changed, 30 insertions(+), 8 deletions(-) diff --git a/spec/unit/crypto/backup.spec.js b/spec/unit/crypto/backup.spec.js index e03460fe0..edfbe6812 100644 --- a/spec/unit/crypto/backup.spec.js +++ b/spec/unit/crypto/backup.spec.js @@ -517,6 +517,7 @@ describe("MegolmBackup", function() { return megolmDecryption.decryptEvent(ENCRYPTED_EVENT); }).then((res) => { expect(res.clearEvent.content).toEqual('testytest'); + expect(res.untrusted).toBeTruthy(); // keys from backup are untrusted }); }); diff --git a/src/client.js b/src/client.js index fc73b41f0..5b716c283 100644 --- a/src/client.js +++ b/src/client.js @@ -1978,7 +1978,7 @@ MatrixClient.prototype._restoreKeyBackup = function( } } - return this.importRoomKeys(keys, { progressCallback }); + return this.importRoomKeys(keys, { progressCallback, untrusted: true, source: "backup" }); }).then(() => { return this._crypto.setTrustedBackupPubKey(backupPubKey); }).then(() => { diff --git a/src/crypto/OlmDevice.js b/src/crypto/OlmDevice.js index 6300609aa..38e2783ec 100644 --- a/src/crypto/OlmDevice.js +++ b/src/crypto/OlmDevice.js @@ -992,12 +992,14 @@ OlmDevice.prototype._getInboundGroupSession = function( * @param {Object} keysClaimed Other keys the sender claims. * @param {boolean} exportFormat true if the megolm keys are in export format * (ie, they lack an ed25519 signature) + * @param {Object} extraSessionData any other data to be include with the session */ OlmDevice.prototype.addInboundGroupSession = async function( roomId, senderKey, forwardingCurve25519KeyChain, sessionId, sessionKey, keysClaimed, - exportFormat, + exportFormat, extraSessionData, ) { + extraSessionData = extraSessionData || {}; await this._cryptoStore.doTxn( 'readwrite', [ IndexedDBCryptoStore.STORE_INBOUND_GROUP_SESSIONS, @@ -1043,12 +1045,12 @@ OlmDevice.prototype.addInboundGroupSession = async function( " with first index " + session.first_known_index(), ); - const sessionData = { + const sessionData = Object.assign({}, extraSessionData, { room_id: roomId, session: session.pickle(this._pickleKey), keysClaimed: keysClaimed, forwardingCurve25519KeyChain: forwardingCurve25519KeyChain, - }; + }); this._cryptoStore.storeEndToEndInboundGroupSession( senderKey, sessionId, sessionData, txn, @@ -1224,6 +1226,7 @@ OlmDevice.prototype.decryptGroupMessage = async function( forwardingCurve25519KeyChain: ( sessionData.forwardingCurve25519KeyChain || [] ), + untrusted: sessionData.untrusted, }; }, ); diff --git a/src/crypto/algorithms/megolm.js b/src/crypto/algorithms/megolm.js index 0976302a6..b247299d7 100644 --- a/src/crypto/algorithms/megolm.js +++ b/src/crypto/algorithms/megolm.js @@ -1201,6 +1201,7 @@ MegolmDecryption.prototype.decryptEvent = async function(event) { senderCurve25519Key: res.senderKey, claimedEd25519Key: res.keysClaimed.ed25519, forwardingCurve25519KeyChain: res.forwardingCurve25519KeyChain, + untrusted: res.untrusted, }; }; @@ -1548,8 +1549,10 @@ MegolmDecryption.prototype._buildKeyForwardingMessage = async function( * @inheritdoc * * @param {module:crypto/OlmDevice.MegolmSessionData} session + * @param {string} source where the key comes from */ -MegolmDecryption.prototype.importRoomKey = function(session) { +MegolmDecryption.prototype.importRoomKey = function(session, opts) { + opts = opts || {}; return this._olmDevice.addInboundGroupSession( session.room_id, session.sender_key, @@ -1558,8 +1561,9 @@ MegolmDecryption.prototype.importRoomKey = function(session) { session.session_key, session.sender_claimed_keys, true, + opts.untrusted ? { untrusted: opts.untrusted } : {}, ).then(() => { - if (this._crypto.backupInfo) { + if (this._crypto.backupInfo && opts.source !== "backup") { // don't wait for it to complete this._crypto.backupGroupSession( session.room_id, diff --git a/src/crypto/index.js b/src/crypto/index.js index ef9f492de..a0d943ae1 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -2238,11 +2238,16 @@ Crypto.prototype.getEventSenderDeviceInfo = function(event) { const forwardingChain = event.getForwardingCurve25519KeyChain(); if (forwardingChain.length > 0) { - // we got this event from somewhere else + // we got the key this event from somewhere else // TODO: check if we can trust the forwarders. return null; } + if (event.isUntrusted()) { + // we got the key for this event from a source that we consider untrusted + return null; + } + // senderKey is the Curve25519 identity key of the device which the event // was sent from. In the case of Megolm, it's actually the Curve25519 // identity key of the device which set up the Megolm session. @@ -2525,7 +2530,7 @@ Crypto.prototype.importRoomKeys = function(keys, opts = {}) { } const alg = this._getRoomDecryptor(key.room_id, key.algorithm); - return alg.importRoomKey(key).finally((r) => { + return alg.importRoomKey(key, opts).finally((r) => { successes++; if (opts.progressCallback) { updateProgress(); } }); diff --git a/src/models/event.js b/src/models/event.js index 90365432f..915b3fe8c 100644 --- a/src/models/event.js +++ b/src/models/event.js @@ -144,6 +144,10 @@ export const MatrixEvent = function( */ this._forwardingCurve25519KeyChain = []; + /* where the decryption key is untrusted + */ + this._untrusted = null; + /* if we have a process decrypting this event, a Promise which resolves * when it is finished. Normally null. */ @@ -599,6 +603,7 @@ utils.extend(MatrixEvent.prototype, { decryptionResult.claimedEd25519Key || null; this._forwardingCurve25519KeyChain = decryptionResult.forwardingCurve25519KeyChain || []; + this._untrusted = decryptionResult.untrusted || false; }, /** @@ -689,6 +694,10 @@ utils.extend(MatrixEvent.prototype, { return this._forwardingCurve25519KeyChain; }, + isUntrusted: function() { + return this._untrusted; + }, + getUnsigned: function() { return this.event.unsigned || {}; },