From b16e652acc1c9876606879458483c5bf9e9b75f2 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 8 Jun 2017 07:54:47 +0100 Subject: [PATCH 1/2] rewrite MegolmExportEncryption using async/await ... to make it easier to add exception handling --- src/utils/MegolmExportEncryption.js | 193 +++++++++++----------- test/utils/MegolmExportEncryption-test.js | 24 ++- 2 files changed, 112 insertions(+), 105 deletions(-) diff --git a/src/utils/MegolmExportEncryption.js b/src/utils/MegolmExportEncryption.js index de39ea71cc..e91dc06b04 100644 --- a/src/utils/MegolmExportEncryption.js +++ b/src/utils/MegolmExportEncryption.js @@ -36,7 +36,7 @@ const subtleCrypto = window.crypto.subtle || window.crypto.webkitSubtle; * @param {String} password * @return {Promise} promise for decrypted output */ -export function decryptMegolmKeyFile(data, password) { +export async function decryptMegolmKeyFile(data, password) { const body = unpackMegolmKeyFile(data); // check we have a version byte @@ -60,33 +60,30 @@ export function decryptMegolmKeyFile(data, password) { const ciphertext = body.subarray(37, 37+ciphertextLength); const hmac = body.subarray(-32); - return deriveKeys(salt, iterations, password).then((keys) => { - const [aesKey, hmacKey] = keys; + const [aesKey, hmacKey] = await deriveKeys(salt, iterations, password); - const toVerify = body.subarray(0, -32); - return subtleCrypto.verify( - {name: 'HMAC'}, - hmacKey, - hmac, - toVerify, - ).then((isValid) => { - if (!isValid) { - throw new Error('Authentication check failed: incorrect password?'); - } + const toVerify = body.subarray(0, -32); + const isValid = await subtleCrypto.verify( + {name: 'HMAC'}, + hmacKey, + hmac, + toVerify, + ); + if (!isValid) { + throw new Error('Authentication check failed: incorrect password?'); + } - return subtleCrypto.decrypt( - { - name: "AES-CTR", - counter: iv, - length: 64, - }, - aesKey, - ciphertext, - ); - }); - }).then((plaintext) => { - return new TextDecoder().decode(new Uint8Array(plaintext)); - }); + const plaintext = await subtleCrypto.decrypt( + { + name: "AES-CTR", + counter: iv, + length: 64, + }, + aesKey, + ciphertext, + ); + + return new TextDecoder().decode(new Uint8Array(plaintext)); } @@ -100,7 +97,7 @@ export function decryptMegolmKeyFile(data, password) { * key-derivation function. * @return {Promise} promise for encrypted output */ -export function encryptMegolmKeyFile(data, password, options) { +export async function encryptMegolmKeyFile(data, password, options) { options = options || {}; const kdfRounds = options.kdf_rounds || 500000; @@ -115,44 +112,42 @@ export function encryptMegolmKeyFile(data, password, options) { // of a single bit of iv is a price we have to pay. iv[9] &= 0x7f; - return deriveKeys(salt, kdfRounds, password).then((keys) => { - const [aesKey, hmacKey] = keys; + const [aesKey, hmacKey] = await deriveKeys(salt, kdfRounds, password); - return subtleCrypto.encrypt( - { - name: "AES-CTR", - counter: iv, - length: 64, - }, - aesKey, - new TextEncoder().encode(data), - ).then((ciphertext) => { - const cipherArray = new Uint8Array(ciphertext); - const bodyLength = (1+salt.length+iv.length+4+cipherArray.length+32); - const resultBuffer = new Uint8Array(bodyLength); - let idx = 0; - resultBuffer[idx++] = 1; // version - resultBuffer.set(salt, idx); idx += salt.length; - resultBuffer.set(iv, idx); idx += iv.length; - resultBuffer[idx++] = kdfRounds >> 24; - resultBuffer[idx++] = (kdfRounds >> 16) & 0xff; - resultBuffer[idx++] = (kdfRounds >> 8) & 0xff; - resultBuffer[idx++] = kdfRounds & 0xff; - resultBuffer.set(cipherArray, idx); idx += cipherArray.length; + const ciphertext = await subtleCrypto.encrypt( + { + name: "AES-CTR", + counter: iv, + length: 64, + }, + aesKey, + new TextEncoder().encode(data), + ); - const toSign = resultBuffer.subarray(0, idx); + const cipherArray = new Uint8Array(ciphertext); + const bodyLength = (1+salt.length+iv.length+4+cipherArray.length+32); + const resultBuffer = new Uint8Array(bodyLength); + let idx = 0; + resultBuffer[idx++] = 1; // version + resultBuffer.set(salt, idx); idx += salt.length; + resultBuffer.set(iv, idx); idx += iv.length; + resultBuffer[idx++] = kdfRounds >> 24; + resultBuffer[idx++] = (kdfRounds >> 16) & 0xff; + resultBuffer[idx++] = (kdfRounds >> 8) & 0xff; + resultBuffer[idx++] = kdfRounds & 0xff; + resultBuffer.set(cipherArray, idx); idx += cipherArray.length; - return subtleCrypto.sign( - {name: 'HMAC'}, - hmacKey, - toSign, - ).then((hmac) => { - hmac = new Uint8Array(hmac); - resultBuffer.set(hmac, idx); - return packMegolmKeyFile(resultBuffer); - }); - }); - }); + const toSign = resultBuffer.subarray(0, idx); + + const hmac = await subtleCrypto.sign( + {name: 'HMAC'}, + hmacKey, + toSign, + ); + + const hmacArray = new Uint8Array(hmac); + resultBuffer.set(hmacArray, idx); + return packMegolmKeyFile(resultBuffer); } /** @@ -163,51 +158,51 @@ export function encryptMegolmKeyFile(data, password, options) { * @param {String} password password * @return {Promise<[CryptoKey, CryptoKey]>} promise for [aes key, hmac key] */ -function deriveKeys(salt, iterations, password) { +async function deriveKeys(salt, iterations, password) { const start = new Date(); - return subtleCrypto.importKey( + const key = await subtleCrypto.importKey( 'raw', new TextEncoder().encode(password), {name: 'PBKDF2'}, false, ['deriveBits'], - ).then((key) => { - return subtleCrypto.deriveBits( - { - name: 'PBKDF2', - salt: salt, - iterations: iterations, - hash: 'SHA-512', - }, - key, - 512, - ); - }).then((keybits) => { - const now = new Date(); - console.log("E2e import/export: deriveKeys took " + (now - start) + "ms"); + ); - const aesKey = keybits.slice(0, 32); - const hmacKey = keybits.slice(32); + const keybits = await subtleCrypto.deriveBits( + { + name: 'PBKDF2', + salt: salt, + iterations: iterations, + hash: 'SHA-512', + }, + key, + 512, + ); - const aesProm = subtleCrypto.importKey( - 'raw', - aesKey, - {name: 'AES-CTR'}, - false, - ['encrypt', 'decrypt'], - ); - const hmacProm = subtleCrypto.importKey( - 'raw', - hmacKey, - { - name: 'HMAC', - hash: {name: 'SHA-256'}, - }, - false, - ['sign', 'verify'], - ); - return Promise.all([aesProm, hmacProm]); - }); + const now = new Date(); + console.log("E2e import/export: deriveKeys took " + (now - start) + "ms"); + + const aesKey = keybits.slice(0, 32); + const hmacKey = keybits.slice(32); + + const aesProm = subtleCrypto.importKey( + 'raw', + aesKey, + {name: 'AES-CTR'}, + false, + ['encrypt', 'decrypt'], + ); + const hmacProm = subtleCrypto.importKey( + 'raw', + hmacKey, + { + name: 'HMAC', + hash: {name: 'SHA-256'}, + }, + false, + ['sign', 'verify'], + ); + return await Promise.all([aesProm, hmacProm]); } const HEADER_LINE = '-----BEGIN MEGOLM SESSION DATA-----'; diff --git a/test/utils/MegolmExportEncryption-test.js b/test/utils/MegolmExportEncryption-test.js index 2637097837..fbd945ced6 100644 --- a/test/utils/MegolmExportEncryption-test.js +++ b/test/utils/MegolmExportEncryption-test.js @@ -81,15 +81,23 @@ describe('MegolmExportEncryption', function() { describe('decrypt', function() { it('should handle missing header', function() { const input=stringToArray(`-----`); - expect(()=>MegolmExportEncryption.decryptMegolmKeyFile(input, '')) - .toThrow('Header line not found'); + return MegolmExportEncryption.decryptMegolmKeyFile(input, '') + .then((res) => { + throw new Error('expected to throw'); + }, (error) => { + expect(error.message).toEqual('Header line not found'); + }); }); it('should handle missing trailer', function() { const input=stringToArray(`-----BEGIN MEGOLM SESSION DATA----- -----`); - expect(()=>MegolmExportEncryption.decryptMegolmKeyFile(input, '')) - .toThrow('Trailer line not found'); + return MegolmExportEncryption.decryptMegolmKeyFile(input, '') + .then((res) => { + throw new Error('expected to throw'); + }, (error) => { + expect(error.message).toEqual('Trailer line not found'); + }); }); it('should handle a too-short body', function() { @@ -98,8 +106,12 @@ AXNhbHRzYWx0c2FsdHNhbHSIiIiIiIiIiIiIiIiIiIiIAAAACmIRUW2OjZ3L2l6j9h0lHlV3M2dx cissyYBxjsfsAn -----END MEGOLM SESSION DATA----- `); - expect(()=>MegolmExportEncryption.decryptMegolmKeyFile(input, '')) - .toThrow('Invalid file: too short'); + return MegolmExportEncryption.decryptMegolmKeyFile(input, '') + .then((res) => { + throw new Error('expected to throw'); + }, (error) => { + expect(error.message).toEqual('Invalid file: too short'); + }); }); it('should decrypt a range of inputs', function(done) { From 175599bedaa4a30e79dd5e7f3d01297ed7303847 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 8 Jun 2017 14:21:25 +0100 Subject: [PATCH 2/2] Improve error logging/reporting in megolm import/export I saw a rageshake where somebody had apparently failed to import a key file. I have no idea why it happened. Also try to make the errors the users see useful. --- .../views/dialogs/ExportE2eKeysDialog.js | 4 +- .../views/dialogs/ImportE2eKeysDialog.js | 4 +- src/i18n/strings/en_EN.json | 5 +- src/utils/MegolmExportEncryption.js | 172 ++++++++++++------ 4 files changed, 129 insertions(+), 56 deletions(-) diff --git a/src/async-components/views/dialogs/ExportE2eKeysDialog.js b/src/async-components/views/dialogs/ExportE2eKeysDialog.js index 045ea63c34..8f113353d9 100644 --- a/src/async-components/views/dialogs/ExportE2eKeysDialog.js +++ b/src/async-components/views/dialogs/ExportE2eKeysDialog.js @@ -81,11 +81,13 @@ export default React.createClass({ FileSaver.saveAs(blob, 'riot-keys.txt'); this.props.onFinished(true); }).catch((e) => { + console.error("Error exporting e2e keys:", e); if (this._unmounted) { return; } + const msg = e.friendlyText || _t('Unknown error'); this.setState({ - errStr: e.message, + errStr: msg, phase: PHASE_EDIT, }); }); diff --git a/src/async-components/views/dialogs/ImportE2eKeysDialog.js b/src/async-components/views/dialogs/ImportE2eKeysDialog.js index 91010d33b9..9eac7f78b2 100644 --- a/src/async-components/views/dialogs/ImportE2eKeysDialog.js +++ b/src/async-components/views/dialogs/ImportE2eKeysDialog.js @@ -89,11 +89,13 @@ export default React.createClass({ // TODO: it would probably be nice to give some feedback about what we've imported here. this.props.onFinished(true); }).catch((e) => { + console.error("Error importing e2e keys:", e); if (this._unmounted) { return; } + const msg = e.friendlyText || _t('Unknown error'); this.setState({ - errStr: e.message, + errStr: msg, phase: PHASE_EDIT, }); }); diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 8dbbb98423..9199c3b103 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -860,5 +860,8 @@ "Username not available": "Username not available", "Something went wrong!": "Something went wrong!", "This will be your account name on the homeserver, or you can pick a different server.": "This will be your account name on the homeserver, or you can pick a different server.", - "If you already have a Matrix account you can log in instead.": "If you already have a Matrix account you can log in instead." + "If you already have a Matrix account you can log in instead.": "If you already have a Matrix account you can log in instead.", + "Your browser does not support the required cryptography extensions": "Your browser does not support the required cryptography extensions", + "Not a valid Riot keyfile": "Not a valid Riot keyfile", + "Authentication check failed: incorrect password?": "Authentication check failed: incorrect password?" } diff --git a/src/utils/MegolmExportEncryption.js b/src/utils/MegolmExportEncryption.js index e91dc06b04..11f9d86816 100644 --- a/src/utils/MegolmExportEncryption.js +++ b/src/utils/MegolmExportEncryption.js @@ -27,31 +27,57 @@ if (!TextDecoder) { TextDecoder = TextEncodingUtf8.TextDecoder; } +import { _t } from '../languageHandler'; + + const subtleCrypto = window.crypto.subtle || window.crypto.webkitSubtle; +/** + * Make an Error object which has a friendlyText property which is already + * translated and suitable for showing to the user. + * + * @param {string} msg message for the exception + * @param {string} friendlyText + * @returns {Error} + */ +function friendlyError(msg, friendlyText) { + const e = new Error(msg); + e.friendlyText = friendlyText; + return e; +} + +function cryptoFailMsg() { + return _t('Your browser does not support the required cryptography extensions'); +} + /** * Decrypt a megolm key file * * @param {ArrayBuffer} data file to decrypt * @param {String} password * @return {Promise} promise for decrypted output + * + * */ export async function decryptMegolmKeyFile(data, password) { const body = unpackMegolmKeyFile(data); // check we have a version byte if (body.length < 1) { - throw new Error('Invalid file: too short'); + throw friendlyError('Invalid file: too short', + _t('Not a valid Riot keyfile')); } const version = body[0]; if (version !== 1) { - throw new Error('Unsupported version'); + throw friendlyError('Unsupported version', + _t('Not a valid Riot keyfile')); } const ciphertextLength = body.length-(1+16+16+4+32); if (ciphertextLength < 0) { - throw new Error('Invalid file: too short'); + throw friendlyError('Invalid file: too short', + _t('Not a valid Riot keyfile')); } const salt = body.subarray(1, 1+16); @@ -61,27 +87,38 @@ export async function decryptMegolmKeyFile(data, password) { const hmac = body.subarray(-32); const [aesKey, hmacKey] = await deriveKeys(salt, iterations, password); - const toVerify = body.subarray(0, -32); - const isValid = await subtleCrypto.verify( - {name: 'HMAC'}, - hmacKey, - hmac, - toVerify, - ); + + let isValid; + try { + isValid = await subtleCrypto.verify( + {name: 'HMAC'}, + hmacKey, + hmac, + toVerify, + ); + } catch (e) { + throw friendlyError('subtleCrypto.verify failed: ' + e, cryptoFailMsg()); + } if (!isValid) { - throw new Error('Authentication check failed: incorrect password?'); + throw friendlyError('hmac mismatch', + _t('Authentication check failed: incorrect password?')); } - const plaintext = await subtleCrypto.decrypt( - { - name: "AES-CTR", - counter: iv, - length: 64, - }, - aesKey, - ciphertext, - ); + let plaintext; + try { + plaintext = await subtleCrypto.decrypt( + { + name: "AES-CTR", + counter: iv, + length: 64, + }, + aesKey, + ciphertext, + ); + } catch(e) { + throw friendlyError('subtleCrypto.decrypt failed: ' + e, cryptoFailMsg()); + } return new TextDecoder().decode(new Uint8Array(plaintext)); } @@ -113,16 +150,22 @@ export async function encryptMegolmKeyFile(data, password, options) { iv[9] &= 0x7f; const [aesKey, hmacKey] = await deriveKeys(salt, kdfRounds, password); + const encodedData = new TextEncoder().encode(data); - const ciphertext = await subtleCrypto.encrypt( - { - name: "AES-CTR", - counter: iv, - length: 64, - }, - aesKey, - new TextEncoder().encode(data), - ); + let ciphertext; + try { + ciphertext = await subtleCrypto.encrypt( + { + name: "AES-CTR", + counter: iv, + length: 64, + }, + aesKey, + encodedData, + ); + } catch (e) { + throw friendlyError('subtleCrypto.encrypt failed: ' + e, cryptoFailMsg()); + } const cipherArray = new Uint8Array(ciphertext); const bodyLength = (1+salt.length+iv.length+4+cipherArray.length+32); @@ -139,11 +182,17 @@ export async function encryptMegolmKeyFile(data, password, options) { const toSign = resultBuffer.subarray(0, idx); - const hmac = await subtleCrypto.sign( - {name: 'HMAC'}, - hmacKey, - toSign, - ); + let hmac; + try { + hmac = await subtleCrypto.sign( + {name: 'HMAC'}, + hmacKey, + toSign, + ); + } catch (e) { + throw friendlyError('subtleCrypto.sign failed: ' + e, cryptoFailMsg()); + } + const hmacArray = new Uint8Array(hmac); resultBuffer.set(hmacArray, idx); @@ -160,24 +209,35 @@ export async function encryptMegolmKeyFile(data, password, options) { */ async function deriveKeys(salt, iterations, password) { const start = new Date(); - const key = await subtleCrypto.importKey( - 'raw', - new TextEncoder().encode(password), - {name: 'PBKDF2'}, - false, - ['deriveBits'], - ); - const keybits = await subtleCrypto.deriveBits( - { - name: 'PBKDF2', - salt: salt, - iterations: iterations, - hash: 'SHA-512', - }, - key, - 512, - ); + let key; + try { + key = await subtleCrypto.importKey( + 'raw', + new TextEncoder().encode(password), + {name: 'PBKDF2'}, + false, + ['deriveBits'], + ); + } catch (e) { + throw friendlyError('subtleCrypto.importKey failed: ' + e, cryptoFailMsg()); + } + + let keybits; + try { + keybits = await subtleCrypto.deriveBits( + { + name: 'PBKDF2', + salt: salt, + iterations: iterations, + hash: 'SHA-512', + }, + key, + 512, + ); + } catch (e) { + throw friendlyError('subtleCrypto.deriveBits failed: ' + e, cryptoFailMsg()); + } const now = new Date(); console.log("E2e import/export: deriveKeys took " + (now - start) + "ms"); @@ -191,7 +251,10 @@ async function deriveKeys(salt, iterations, password) { {name: 'AES-CTR'}, false, ['encrypt', 'decrypt'], - ); + ).catch((e) => { + throw friendlyError('subtleCrypto.importKey failed for AES key: ' + e, cryptoFailMsg()); + }); + const hmacProm = subtleCrypto.importKey( 'raw', hmacKey, @@ -201,7 +264,10 @@ async function deriveKeys(salt, iterations, password) { }, false, ['sign', 'verify'], - ); + ).catch((e) => { + throw friendlyError('subtleCrypto.importKey failed for HMAC key: ' + e, cryptoFailMsg()); + }); + return await Promise.all([aesProm, hmacProm]); }