Merge pull request #1061 from matrix-org/rav/megolm_key_encryption_errors

Improve error logging/reporting in megolm import/export

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,
             });
         });

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,
             });
         });

src/i18n/strings/en_EN.json

@@ -908,5 +908,8 @@
   "Username not available": "Username not available",
   "Something went wrong!": "Something went wrong!",
   "This will be your account name on the <span></span> homeserver, or you can pick a <a>different server</a>.": "This will be your account name on the <span></span> homeserver, or you can pick a <a>different server</a>.",
-  "If you already have a Matrix account you can <a>log in</a> instead.": "If you already have a Matrix account you can <a>log in</a> instead."
+  "If you already have a Matrix account you can <a>log in</a> instead.": "If you already have a Matrix account you can <a>log in</a> 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?"
 }

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<String>} 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
     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);
@@ -60,33 +86,41 @@ 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] = await deriveKeys(salt, iterations, password);
-        const [aesKey, hmacKey] = keys;
+    const toVerify = body.subarray(0, -32);
 
-        const toVerify = body.subarray(0, -32);
+    let isValid;
-        return subtleCrypto.verify(
+    try {
+        isValid = await subtleCrypto.verify(
             {name: 'HMAC'},
             hmacKey,
             hmac,
             toVerify,
-        ).then((isValid) => {
+        );
-            if (!isValid) {
+    } catch (e) {
-                throw new Error('Authentication check failed: incorrect password?');
+        throw friendlyError('subtleCrypto.verify failed: ' + e, cryptoFailMsg());
-            }
+    }
+    if (!isValid) {
+        throw friendlyError('hmac mismatch',
+            _t('Authentication check failed: incorrect password?'));
+    }
 
-            return subtleCrypto.decrypt(
+    let plaintext;
-                {
+    try {
-                    name: "AES-CTR",
+        plaintext = await subtleCrypto.decrypt(
-                    counter: iv,
+            {
-                    length: 64,
+                name: "AES-CTR",
-                },
+                counter: iv,
-                aesKey,
+                length: 64,
-                ciphertext,
+            },
-            );
+            aesKey,
-        });
+            ciphertext,
-    }).then((plaintext) => {
+        );
-        return new TextDecoder().decode(new Uint8Array(plaintext));
+    } catch(e) {
-    });
+        throw friendlyError('subtleCrypto.decrypt failed: ' + e, cryptoFailMsg());
+    }
+
+    return new TextDecoder().decode(new Uint8Array(plaintext));
 }
 
 
@@ -100,7 +134,7 @@ export function decryptMegolmKeyFile(data, password) {
  *    key-derivation function.
  * @return {Promise<ArrayBuffer>} 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 +149,54 @@ 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] = await deriveKeys(salt, kdfRounds, password);
-        const [aesKey, hmacKey] = keys;
+    const encodedData = new TextEncoder().encode(data);
 
-        return subtleCrypto.encrypt(
+    let ciphertext;
+    try {
+        ciphertext = await subtleCrypto.encrypt(
             {
                 name: "AES-CTR",
                 counter: iv,
                 length: 64,
             },
             aesKey,
-            new TextEncoder().encode(data),
+            encodedData,
-        ).then((ciphertext) => {
+        );
-            const cipherArray = new Uint8Array(ciphertext);
+    } catch (e) {
-            const bodyLength = (1+salt.length+iv.length+4+cipherArray.length+32);
+        throw friendlyError('subtleCrypto.encrypt failed: ' + e, cryptoFailMsg());
-            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 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(
+    const toSign = resultBuffer.subarray(0, idx);
-                {name: 'HMAC'},
+
-                hmacKey,
+    let hmac;
-                toSign,
+    try {
-            ).then((hmac) => {
+        hmac = await subtleCrypto.sign(
-                hmac = new Uint8Array(hmac);
+            {name: 'HMAC'},
-                resultBuffer.set(hmac, idx);
+            hmacKey,
-                return packMegolmKeyFile(resultBuffer);
+            toSign,
-            });
+        );
-        });
+    } catch (e) {
-    });
+        throw friendlyError('subtleCrypto.sign failed: ' + e, cryptoFailMsg());
+    }
+
+
+    const hmacArray = new Uint8Array(hmac);
+    resultBuffer.set(hmacArray, idx);
+    return packMegolmKeyFile(resultBuffer);
 }
 
 /**
@@ -163,16 +207,25 @@ 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(
+
-        'raw',
+    let key;
-        new TextEncoder().encode(password),
+    try {
-        {name: 'PBKDF2'},
+        key = await subtleCrypto.importKey(
-        false,
+            'raw',
-        ['deriveBits'],
+            new TextEncoder().encode(password),
-    ).then((key) => {
+            {name: 'PBKDF2'},
-        return subtleCrypto.deriveBits(
+            false,
+            ['deriveBits'],
+        );
+    } catch (e) {
+        throw friendlyError('subtleCrypto.importKey failed: ' + e, cryptoFailMsg());
+    }
+
+    let keybits;
+    try {
+        keybits = await subtleCrypto.deriveBits(
             {
                 name: 'PBKDF2',
                 salt: salt,
@@ -182,32 +235,40 @@ function deriveKeys(salt, iterations, password) {
             key,
             512,
         );
-    }).then((keybits) => {
+    } catch (e) {
-        const now = new Date();
+        throw friendlyError('subtleCrypto.deriveBits failed: ' + e, cryptoFailMsg());
-        console.log("E2e import/export: deriveKeys took " + (now - start) + "ms");
+    }
 
-        const aesKey = keybits.slice(0, 32);
+    const now = new Date();
-        const hmacKey = keybits.slice(32);
+    console.log("E2e import/export: deriveKeys took " + (now - start) + "ms");
 
-        const aesProm = subtleCrypto.importKey(
+    const aesKey = keybits.slice(0, 32);
-            'raw',
+    const hmacKey = keybits.slice(32);
-            aesKey,
+
-            {name: 'AES-CTR'},
+    const aesProm = subtleCrypto.importKey(
-            false,
+        'raw',
-            ['encrypt', 'decrypt'],
+        aesKey,
-        );
+        {name: 'AES-CTR'},
-        const hmacProm = subtleCrypto.importKey(
+        false,
-            'raw',
+        ['encrypt', 'decrypt'],
-            hmacKey,
+    ).catch((e) => {
-            {
+        throw friendlyError('subtleCrypto.importKey failed for AES key: ' + e, cryptoFailMsg());
-                name: 'HMAC',
-                hash: {name: 'SHA-256'},
-            },
-            false,
-            ['sign', 'verify'],
-        );
-        return Promise.all([aesProm, hmacProm]);
     });
+
+    const hmacProm = subtleCrypto.importKey(
+        'raw',
+        hmacKey,
+        {
+            name: 'HMAC',
+            hash: {name: 'SHA-256'},
+        },
+        false,
+        ['sign', 'verify'],
+    ).catch((e) => {
+        throw friendlyError('subtleCrypto.importKey failed for HMAC key: ' + e, cryptoFailMsg());
+    });
+
+    return await Promise.all([aesProm, hmacProm]);
 }
 
 const HEADER_LINE = '-----BEGIN MEGOLM SESSION DATA-----';

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, ''))
+            return MegolmExportEncryption.decryptMegolmKeyFile(input, '')
-                .toThrow('Header line not found');
+            .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, ''))
+            return MegolmExportEncryption.decryptMegolmKeyFile(input, '')
-                .toThrow('Trailer line not found');
+            .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, ''))
+            return MegolmExportEncryption.decryptMegolmKeyFile(input, '')
-                .toThrow('Invalid file: too short');
+            .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) {