From 894c24880da0e1cc81818f51c0db80e3c9fb2be9 Mon Sep 17 00:00:00 2001 From: RiotRobot Date: Mon, 13 Sep 2021 12:34:48 +0100 Subject: [PATCH 1/3] Verify target device key on reshare --- src/crypto/algorithms/megolm.ts | 38 +++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/src/crypto/algorithms/megolm.ts b/src/crypto/algorithms/megolm.ts index 381f85188..55a766620 100644 --- a/src/crypto/algorithms/megolm.ts +++ b/src/crypto/algorithms/megolm.ts @@ -101,6 +101,13 @@ interface IPayload extends Partial { } /* eslint-enable camelcase */ +interface SharedWithData { + // The identity key of the device we shared with + deviceKey: string; + // The message index of the ratchet we shared with that device + messageIndex: number; +} + /** * @private * @constructor @@ -115,12 +122,12 @@ interface IPayload extends Partial { * * @property {object} sharedWithDevices * devices with which we have shared the session key - * userId -> {deviceId -> msgindex} + * userId -> {deviceId -> SharedWithData} */ class OutboundSessionInfo { public useCount = 0; public creationTime: number; - public sharedWithDevices: Record> = {}; + public sharedWithDevices: Record> = {}; public blockedDevicesNotified: Record> = {}; constructor(public readonly sessionId: string, public readonly sharedHistory = false) { @@ -150,11 +157,11 @@ class OutboundSessionInfo { return false; } - public markSharedWithDevice(userId: string, deviceId: string, chainIndex: number): void { + public markSharedWithDevice(userId: string, deviceId: string, deviceKey: string, chainIndex: number): void { if (!this.sharedWithDevices[userId]) { this.sharedWithDevices[userId] = {}; } - this.sharedWithDevices[userId][deviceId] = chainIndex; + this.sharedWithDevices[userId][deviceId] = { deviceKey, messageIndex: chainIndex }; } public markNotifiedBlockedDevice(userId: string, deviceId: string): void { @@ -572,6 +579,7 @@ class MegolmEncryption extends EncryptionAlgorithm { payload: IPayload, ): Promise { const contentMap = {}; + const deviceInfoByDeviceId = new Map(); const promises = []; for (let i = 0; i < userDeviceMap.length; i++) { @@ -584,6 +592,7 @@ class MegolmEncryption extends EncryptionAlgorithm { const userId = val.userId; const deviceInfo = val.deviceInfo; const deviceId = deviceInfo.deviceId; + deviceInfoByDeviceId.set(deviceId, deviceInfo); if (!contentMap[userId]) { contentMap[userId] = {}; @@ -636,7 +645,10 @@ class MegolmEncryption extends EncryptionAlgorithm { for (const userId of Object.keys(contentMap)) { for (const deviceId of Object.keys(contentMap[userId])) { session.markSharedWithDevice( - userId, deviceId, chainIndex, + userId, + deviceId, + deviceInfoByDeviceId.get(deviceId).getIdentityKey(), + chainIndex, ); } } @@ -719,8 +731,8 @@ class MegolmEncryption extends EncryptionAlgorithm { logger.debug(`megolm session ${sessionId} never shared with user ${userId}`); return; } - const sentChainIndex = obSessionInfo.sharedWithDevices[userId][device.deviceId]; - if (sentChainIndex === undefined) { + const sessionSharedData = obSessionInfo.sharedWithDevices[userId][device.deviceId]; + if (sessionSharedData === undefined) { logger.debug( "megolm session ID " + sessionId + " never shared with device " + userId + ":" + device.deviceId, @@ -728,10 +740,18 @@ class MegolmEncryption extends EncryptionAlgorithm { return; } + if (sessionSharedData.deviceKey !== device.getIdentityKey()) { + logger.warn( + `Session has been shared with device ${device.deviceId} but with identity ` + + `key ${sessionSharedData.deviceKey}. Key is now ${device.getIdentityKey()}!`, + ); + return; + } + // get the key from the inbound session: the outbound one will already // have been ratcheted to the next chain index. const key = await this.olmDevice.getInboundGroupSessionKey( - this.roomId, senderKey, sessionId, sentChainIndex, + this.roomId, senderKey, sessionId, sessionSharedData.messageIndex, ); if (!key) { @@ -882,7 +902,7 @@ class MegolmEncryption extends EncryptionAlgorithm { const deviceId = deviceInfo.deviceId; session.markSharedWithDevice( - userId, deviceId, key.chain_index, + userId, deviceId, deviceInfo.getIdentityKey(), key.chain_index, ); } From fac700bf4d3b113b898346d8eeeaabfb614f514d Mon Sep 17 00:00:00 2001 From: RiotRobot Date: Mon, 13 Sep 2021 12:43:13 +0100 Subject: [PATCH 2/3] Changelog for 12.4.1 --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5621e0f25..28a6141b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +Changes in [12.4.1](https://github.com/vector-im/element-desktop/releases/tag/v12.4.1) (2021-09-13) +=================================================================================================== + +## 🔒 SECURITY FIXES + * Fix a security issue with message key sharing. See https://matrix.org/blog/2021/09/13/vulnerability-disclosure-key-sharing + for details. + Changes in [12.4.0](https://github.com/vector-im/element-desktop/releases/tag/v12.4.0) (2021-08-31) =================================================================================================== From e878b7683b1d7b47eca10d8dfc4510f7826679b3 Mon Sep 17 00:00:00 2001 From: RiotRobot Date: Mon, 13 Sep 2021 12:47:04 +0100 Subject: [PATCH 3/3] v12.4.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 341cd0821..066e12b32 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "matrix-js-sdk", - "version": "12.4.0", + "version": "12.4.1", "description": "Matrix Client-Server SDK for Javascript", "scripts": { "prepublishOnly": "yarn build",