From d1e64d0cfb043a24a558f2ab926fa37a30f18f1f Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Tue, 2 Apr 2019 23:22:36 -0400 Subject: [PATCH] support both the incorrect and correct MAC methods also do some refactoring to make it easier to support choices in the other methods in the future --- package.json | 2 +- spec/unit/crypto/verification/sas.spec.js | 218 +++++++++++++--------- src/crypto/verification/SAS.js | 117 +++++++----- yarn.lock | 6 +- 4 files changed, 203 insertions(+), 140 deletions(-) diff --git a/package.json b/package.json index 04e297877..b48cfd423 100644 --- a/package.json +++ b/package.json @@ -82,7 +82,7 @@ "matrix-mock-request": "^1.2.2", "mocha": "^5.2.0", "mocha-jenkins-reporter": "^0.4.0", - "olm": "https://matrix.org/packages/npm/olm/olm-3.1.0-pre1.tgz", + "olm": "https://matrix.org/packages/npm/olm/olm-3.1.0-pre3.tgz", "rimraf": "^2.5.4", "source-map-support": "^0.4.11", "sourceify": "^0.1.0", diff --git a/spec/unit/crypto/verification/sas.spec.js b/spec/unit/crypto/verification/sas.spec.js index c1c1d2a45..902d48083 100644 --- a/spec/unit/crypto/verification/sas.spec.js +++ b/spec/unit/crypto/verification/sas.spec.js @@ -58,105 +58,143 @@ describe("SAS verification", function() { expect(spy).toHaveBeenCalled(); }); - it("should verify a key", async function() { - const [alice, bob] = await makeTestClients( - [ - {userId: "@alice:example.com", deviceId: "Osborne2"}, - {userId: "@bob:example.com", deviceId: "Dynabook"}, - ], - { - verificationMethods: [verificationMethods.SAS], - }, - ); - - alice.setDeviceVerified = expect.createSpy(); - alice.getDeviceEd25519Key = () => { - return "alice+base64+ed25519+key"; - }; - alice.getStoredDevice = () => { - return DeviceInfo.fromStorage( - { - keys: { - "ed25519:Dynabook": "bob+base64+ed25519+key", - }, - }, - "Dynabook", - ); - }; - alice.downloadKeys = () => { - return Promise.resolve(); - }; - - bob.setDeviceVerified = expect.createSpy(); - bob.getStoredDevice = () => { - return DeviceInfo.fromStorage( - { - keys: { - "ed25519:Osborne2": "alice+base64+ed25519+key", - }, - }, - "Osborne2", - ); - }; - bob.getDeviceEd25519Key = () => { - return "bob+base64+ed25519+key"; - }; - bob.downloadKeys = () => { - return Promise.resolve(); - }; - + describe("verification", function() { + let alice; + let bob; let aliceSasEvent; let bobSasEvent; + let aliceVerifier; + let bobPromise; - const bobPromise = new Promise((resolve, reject) => { - bob.on("crypto.verification.start", (verifier) => { - verifier.on("show_sas", (e) => { - if (!e.sas.emoji || !e.sas.decimal) { - e.cancel(); - } else if (!aliceSasEvent) { - bobSasEvent = e; - } else { - try { - expect(e.sas).toEqual(aliceSasEvent.sas); - e.confirm(); - aliceSasEvent.confirm(); - } catch (error) { - e.mismatch(); - aliceSasEvent.mismatch(); + beforeEach(async function() { + [alice, bob] = await makeTestClients( + [ + {userId: "@alice:example.com", deviceId: "Osborne2"}, + {userId: "@bob:example.com", deviceId: "Dynabook"}, + ], + { + verificationMethods: [verificationMethods.SAS], + }, + ); + + alice.setDeviceVerified = expect.createSpy(); + alice.getDeviceEd25519Key = () => { + return "alice+base64+ed25519+key"; + }; + alice.getStoredDevice = () => { + return DeviceInfo.fromStorage( + { + keys: { + "ed25519:Dynabook": "bob+base64+ed25519+key", + }, + }, + "Dynabook", + ); + }; + alice.downloadKeys = () => { + return Promise.resolve(); + }; + + bob.setDeviceVerified = expect.createSpy(); + bob.getStoredDevice = () => { + return DeviceInfo.fromStorage( + { + keys: { + "ed25519:Osborne2": "alice+base64+ed25519+key", + }, + }, + "Osborne2", + ); + }; + bob.getDeviceEd25519Key = () => { + return "bob+base64+ed25519+key"; + }; + bob.downloadKeys = () => { + return Promise.resolve(); + }; + + aliceSasEvent = null; + bobSasEvent = null; + + bobPromise = new Promise((resolve, reject) => { + bob.on("crypto.verification.start", (verifier) => { + verifier.on("show_sas", (e) => { + if (!e.sas.emoji || !e.sas.decimal) { + e.cancel(); + } else if (!aliceSasEvent) { + bobSasEvent = e; + } else { + try { + expect(e.sas).toEqual(aliceSasEvent.sas); + e.confirm(); + aliceSasEvent.confirm(); + } catch (error) { + e.mismatch(); + aliceSasEvent.mismatch(); + } } - } + }); + resolve(verifier); }); - resolve(verifier); + }); + + aliceVerifier = alice.beginKeyVerification( + verificationMethods.SAS, bob.getUserId(), bob.deviceId, + ); + aliceVerifier.on("show_sas", (e) => { + if (!e.sas.emoji || !e.sas.decimal) { + e.cancel(); + } else if (!bobSasEvent) { + aliceSasEvent = e; + } else { + try { + expect(e.sas).toEqual(bobSasEvent.sas); + e.confirm(); + bobSasEvent.confirm(); + } catch (error) { + e.mismatch(); + bobSasEvent.mismatch(); + } + } }); }); - const aliceVerifier = alice.beginKeyVerification( - verificationMethods.SAS, bob.getUserId(), bob.deviceId, - ); - aliceVerifier.on("show_sas", (e) => { - if (!e.sas.emoji || !e.sas.decimal) { - e.cancel(); - } else if (!bobSasEvent) { - aliceSasEvent = e; - } else { - try { - expect(e.sas).toEqual(bobSasEvent.sas); - e.confirm(); - bobSasEvent.confirm(); - } catch (error) { - e.mismatch(); - bobSasEvent.mismatch(); - } - } + it("should verify a key", async function() { + await Promise.all([ + aliceVerifier.verify(), + bobPromise.then((verifier) => verifier.verify()), + ]); + expect(alice.setDeviceVerified) + .toHaveBeenCalledWith(bob.getUserId(), bob.deviceId); + expect(bob.setDeviceVerified) + .toHaveBeenCalledWith(alice.getUserId(), alice.deviceId); + }); + + it("should be able to verify using the old MAC", async function() { + // pretend that Alice can only understand the old (incorrect) MAC, + // and make sure that she can still verify with Bob + const origSendToDevice = alice.sendToDevice; + alice.sendToDevice = function(type, map) { + if (type === "m.key.verification.start") { + // Note: this modifies not only the message that Bob + // receives, but also the copy of the message that Alice + // has, since it is the same object. If this does not + // happen, the verification will fail due to a hash + // commitment mismatch. + map[bob.getUserId()][bob.deviceId] + .message_authentication_codes = ['hmac-sha256']; + } + return origSendToDevice.call(this, type, map); + }; + await Promise.all([ + aliceVerifier.verify(), + bobPromise.then((verifier) => verifier.verify()), + ]); + expect(alice.setDeviceVerified) + .toHaveBeenCalledWith(bob.getUserId(), bob.deviceId); + expect(bob.setDeviceVerified) + .toHaveBeenCalledWith(alice.getUserId(), alice.deviceId); }); - await Promise.all([ - aliceVerifier.verify(), - bobPromise.then((verifier) => verifier.verify()), - ]); - expect(alice.setDeviceVerified) - .toHaveBeenCalledWith(bob.getUserId(), bob.deviceId); - expect(bob.setDeviceVerified) - .toHaveBeenCalledWith(alice.getUserId(), alice.deviceId); }); it("should send a cancellation message on error", async function() { diff --git a/src/crypto/verification/SAS.js b/src/crypto/verification/SAS.js index 329d13fea..e17729b04 100644 --- a/src/crypto/verification/SAS.js +++ b/src/crypto/verification/SAS.js @@ -143,17 +143,44 @@ function generateEmojiSas(sasBytes) { return emojis.map((num) => emojiMapping[num]); } +const sasGenerators = { + decimal: generateDecimalSas, + emoji: generateEmojiSas, +}; + function generateSas(sasBytes, methods) { const sas = {}; - if (methods.includes("decimal")) { - sas["decimal"] = generateDecimalSas(sasBytes); - } - if (methods.includes("emoji")) { - sas["emoji"] = generateEmojiSas(sasBytes); + for (const method of methods) { + if (method in sasGenerators) { + sas[method] = sasGenerators[method](sasBytes); + } } return sas; } +const macMethods = { + "hmac-sha256-bits": "calculate_mac", + "hmac-sha256": "calculate_mac_long_kdf", +}; + +/* lists of algorithms/methods that are supported. The key agreement, hashes, + * and MAC lists should be sorted in order of preference (most preferred + * first). + */ +const KEY_AGREEMENT_LIST = ["curve25519"]; +const HASHES_LIST = ["sha256"]; +const MAC_LIST = ["hmac-sha256-bits", "hmac-sha256"]; +const SAS_LIST = Object.keys(sasGenerators); + +const KEY_AGREEMENT_SET = new Set(KEY_AGREEMENT_LIST); +const HASHES_SET = new Set(HASHES_LIST); +const MAC_SET = new Set(MAC_LIST); +const SAS_SET = new Set(SAS_LIST); + +function intersection(anArray, aSet) { + return anArray instanceof Array ? anArray.filter(x => aSet.has(x)) : []; +} + /** * @alias module:crypto/verification/SAS * @extends {module:crypto/verification/Base} @@ -181,11 +208,11 @@ export default class SAS extends Base { const initialMessage = { method: SAS.NAME, from_device: this._baseApis.deviceId, - key_agreement_protocols: ["curve25519"], - hashes: ["sha256"], - message_authentication_codes: ["hmac-sha256"], + key_agreement_protocols: KEY_AGREEMENT_LIST, + hashes: HASHES_LIST, + message_authentication_codes: MAC_LIST, // FIXME: allow app to specify what SAS methods can be used - short_authentication_string: ["decimal", "emoji"], + short_authentication_string: SAS_LIST, transaction_id: this.transactionId, }; this._sendToDevice("m.key.verification.start", initialMessage); @@ -193,19 +220,19 @@ export default class SAS extends Base { let e = await this._waitForEvent("m.key.verification.accept"); let content = e.getContent(); - if (!(content.key_agreement_protocol === "curve25519" - && content.hash === "sha256" - && content.message_authentication_code === "hmac-sha256" - && content.short_authentication_string instanceof Array - && (content.short_authentication_string.includes("decimal") - || content.short_authentication_string.includes("emoji")))) { + const sasMethods + = intersection(content.short_authentication_string, SAS_SET); + if (!(KEY_AGREEMENT_SET.has(content.key_agreement_protocol) + && HASHES_SET.has(content.hash) + && MAC_SET.has(content.message_authentication_code) + && sasMethods.length)) { throw newUnknownMethodError(); } if (typeof content.commitment !== "string") { throw newInvalidMessageError(); } + const macMethod = content.message_authentication_code; const hashCommitment = content.commitment; - const sasMethods = content.short_authentication_string; const olmSAS = new global.Olm.SAS(); try { this._sendToDevice("m.key.verification.key", { @@ -217,6 +244,7 @@ export default class SAS extends Base { // FIXME: make sure event is properly formed content = e.getContent(); const commitmentStr = content.key + anotherjson.stringify(initialMessage); + // TODO: use selected hash function (when we support multiple) if (olmutil.sha256(commitmentStr) !== hashCommitment) { throw newMismatchedCommitmentError(); } @@ -231,7 +259,7 @@ export default class SAS extends Base { this.emit("show_sas", { sas: generateSas(sasBytes, sasMethods), confirm: () => { - this._sendMAC(olmSAS); + this._sendMAC(olmSAS, macMethod); resolve(); }, cancel: () => reject(newUserCancelledError()), @@ -245,7 +273,7 @@ export default class SAS extends Base { verifySAS, ]); content = e.getContent(); - await this._checkMAC(olmSAS, content); + await this._checkMAC(olmSAS, content, macMethod); } finally { olmSAS.free(); } @@ -253,34 +281,31 @@ export default class SAS extends Base { async _doRespondVerification() { let content = this.startEvent.getContent(); - if (!(content.key_agreement_protocols instanceof Array - && content.key_agreement_protocols.includes("curve25519") - && content.hashes instanceof Array - && content.hashes.includes("sha256") - && content.message_authentication_codes instanceof Array - && content.message_authentication_codes.includes("hmac-sha256") - && content.short_authentication_string instanceof Array - && (content.short_authentication_string.includes("decimal") - || content.short_authentication_string.includes("emoji")))) { + const keyAgreement + = intersection(content.key_agreement_protocols, KEY_AGREEMENT_SET)[0]; + const hashMethod + = intersection(content.hashes, HASHES_SET)[0]; + const macMethod + = intersection(content.message_authentication_codes, MAC_SET)[0]; + // FIXME: allow app to specify what SAS methods can be used + const sasMethods + = intersection(content.short_authentication_string, SAS_SET); + if (!(keyAgreement !== undefined + && hashMethod !== undefined + && macMethod !== undefined + && sasMethods.length)) { throw newUnknownMethodError(); } const olmSAS = new global.Olm.SAS(); - const sasMethods = []; - // FIXME: allow app to specify what SAS methods can be used - if (content.short_authentication_string.includes("decimal")) { - sasMethods.push("decimal"); - } - if (content.short_authentication_string.includes("emoji")) { - sasMethods.push("emoji"); - } try { const commitmentStr = olmSAS.get_pubkey() + anotherjson.stringify(content); this._sendToDevice("m.key.verification.accept", { - key_agreement_protocol: "curve25519", - hash: "sha256", - message_authentication_code: "hmac-sha256", + key_agreement_protocol: keyAgreement, + hash: hashMethod, + message_authentication_code: macMethod, short_authentication_string: sasMethods, + // TODO: use selected hash function (when we support multiple) commitment: olmutil.sha256(commitmentStr), }); @@ -302,7 +327,7 @@ export default class SAS extends Base { this.emit("show_sas", { sas: generateSas(sasBytes, sasMethods), confirm: () => { - this._sendMAC(olmSAS); + this._sendMAC(olmSAS, macMethod); resolve(); }, cancel: () => reject(newUserCancelledError()), @@ -316,13 +341,13 @@ export default class SAS extends Base { verifySAS, ]); content = e.getContent(); - await this._checkMAC(olmSAS, content); + await this._checkMAC(olmSAS, content, macMethod); } finally { olmSAS.free(); } } - _sendMAC(olmSAS) { + _sendMAC(olmSAS, method) { const keyId = `ed25519:${this._baseApis.deviceId}`; const mac = {}; const baseInfo = "MATRIX_KEY_VERIFICATION_MAC" @@ -330,24 +355,24 @@ export default class SAS extends Base { + this.userId + this.deviceId + this.transactionId; - mac[keyId] = olmSAS.calculate_mac( + mac[keyId] = olmSAS[macMethods[method]]( this._baseApis.getDeviceEd25519Key(), baseInfo + keyId, ); - const keys = olmSAS.calculate_mac( + const keys = olmSAS[macMethods[method]]( keyId, baseInfo + "KEY_IDS", ); this._sendToDevice("m.key.verification.mac", { mac, keys }); } - async _checkMAC(olmSAS, content) { + async _checkMAC(olmSAS, content, method) { const baseInfo = "MATRIX_KEY_VERIFICATION_MAC" + this.userId + this.deviceId + this._baseApis.getUserId() + this._baseApis.deviceId + this.transactionId; - if (content.keys !== olmSAS.calculate_mac( + if (content.keys !== olmSAS[macMethods[method]]( Object.keys(content.mac).sort().join(","), baseInfo + "KEY_IDS", )) { @@ -355,7 +380,7 @@ export default class SAS extends Base { } await this._verifyKeys(this.userId, content.mac, (keyId, device, keyInfo) => { - if (keyInfo !== olmSAS.calculate_mac( + if (keyInfo !== olmSAS[macMethods[method]]( device.keys[keyId], baseInfo + keyId, )) { diff --git a/yarn.lock b/yarn.lock index 310dc6081..50e2c6d15 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3520,9 +3520,9 @@ object.pick@^1.3.0: dependencies: isobject "^3.0.1" -"olm@https://matrix.org/packages/npm/olm/olm-3.1.0-pre1.tgz": - version "3.1.0-pre1" - resolved "https://matrix.org/packages/npm/olm/olm-3.1.0-pre1.tgz#54f14fa901ff5c81db516b3b2adb294b91726eaa" +"olm@https://matrix.org/packages/npm/olm/olm-3.1.0-pre3.tgz": + version "3.1.0-pre3" + resolved "https://matrix.org/packages/npm/olm/olm-3.1.0-pre3.tgz#525aa8191b4b6fcb07a3aa6815687780b99be411" once@1.x, once@^1.3.0: version "1.4.0"