From 95bf5b84181c5de20b13ac674dce917e82b94e7d Mon Sep 17 00:00:00 2001 From: willclarktech Date: Tue, 10 Jul 2018 15:42:47 +0200 Subject: [PATCH] :seedling: Add better validation to decrypt passphrase Closes #688 --- packages/lisk-cryptography/src/convert.js | 12 +++--- packages/lisk-cryptography/src/encrypt.js | 29 ++++++++++---- packages/lisk-cryptography/test/convert.js | 20 +++++++++- packages/lisk-cryptography/test/encrypt.js | 46 +++++++++++++++++++++- 4 files changed, 92 insertions(+), 15 deletions(-) diff --git a/packages/lisk-cryptography/src/convert.js b/packages/lisk-cryptography/src/convert.js index e04be1879..a9282b73b 100644 --- a/packages/lisk-cryptography/src/convert.js +++ b/packages/lisk-cryptography/src/convert.js @@ -26,16 +26,18 @@ export const bufferToBigNumberString = bigNumberBuffer => export const bufferToHex = buffer => Buffer.from(buffer).toString('hex'); const hexRegex = /^[0-9a-f]+/i; -export const hexToBuffer = hex => { +export const hexToBuffer = (hex, argumentName = 'Argument') => { if (typeof hex !== 'string') { - throw new TypeError('Argument must be a string.'); + throw new TypeError(`${argumentName} must be a string.`); } const matchedHex = (hex.match(hexRegex) || [])[0]; if (!matchedHex || matchedHex.length !== hex.length) { - throw new TypeError('Argument must be a valid hex string.'); + throw new TypeError(`${argumentName} must be a valid hex string.`); } if (matchedHex.length % 2 !== 0) { - throw new TypeError('Argument must have a valid length of hex string.'); + throw new TypeError( + `${argumentName} must have a valid length of hex string.`, + ); } return Buffer.from(matchedHex, 'hex'); }; @@ -82,7 +84,7 @@ export const stringifyEncryptedPassphrase = encryptedPassphrase => { iv: encryptedPassphrase.iv, tag: encryptedPassphrase.tag, version: encryptedPassphrase.version, - }; + }; return querystring.stringify(objectToStringify); }; diff --git a/packages/lisk-cryptography/src/encrypt.js b/packages/lisk-cryptography/src/encrypt.js index f20dcdf33..16563bd28 100644 --- a/packages/lisk-cryptography/src/encrypt.js +++ b/packages/lisk-cryptography/src/encrypt.js @@ -126,23 +126,36 @@ const encryptAES256GCMWithPassword = ( }; const getTagBuffer = tag => { - const tagBuffer = hexToBuffer(tag); + const tagBuffer = hexToBuffer(tag, 'Tag'); if (tagBuffer.length !== 16) { throw new Error('Tag must be 16 bytes.'); } return tagBuffer; }; -const decryptAES256GCMWithPassword = ( - { iterations = PBKDF2_ITERATIONS, cipherText, iv, salt, tag }, - password, -) => { +const decryptAES256GCMWithPassword = (encryptedPassphrase, password) => { + const { + iterations = PBKDF2_ITERATIONS, + cipherText, + iv, + salt, + tag, + } = encryptedPassphrase; + const tagBuffer = getTagBuffer(tag); - const key = getKeyFromPassword(password, hexToBuffer(salt), iterations); + const key = getKeyFromPassword( + password, + hexToBuffer(salt, 'Salt'), + iterations, + ); - const decipher = crypto.createDecipheriv('aes-256-gcm', key, hexToBuffer(iv)); + const decipher = crypto.createDecipheriv( + 'aes-256-gcm', + key, + hexToBuffer(iv, 'IV'), + ); decipher.setAuthTag(tagBuffer); - const firstBlock = decipher.update(hexToBuffer(cipherText)); + const firstBlock = decipher.update(hexToBuffer(cipherText, 'Cipher text')); const decrypted = Buffer.concat([firstBlock, decipher.final()]); return decrypted.toString(); diff --git a/packages/lisk-cryptography/test/convert.js b/packages/lisk-cryptography/test/convert.js index a2035d16b..c72e69134 100644 --- a/packages/lisk-cryptography/test/convert.js +++ b/packages/lisk-cryptography/test/convert.js @@ -81,6 +81,12 @@ describe('convert', () => { ); }); + it('should throw an error for a non-string input with custom argument name', () => { + return expect(hexToBuffer.bind(null, {}, 'Custom')).to.throw( + 'Custom must be a string.', + ); + }); + it('should throw TypeError with non hex string', () => { return expect(hexToBuffer.bind(null, 'yKJj')).to.throw( TypeError, @@ -109,12 +115,24 @@ describe('convert', () => { ); }); - it('should throw TypeError with odd number of hex string', () => { + it('should throw an error for a non-hex string input with custom argument name', () => { + return expect(hexToBuffer.bind(null, 'yKJj', 'Custom')).to.throw( + 'Custom must be a valid hex string.', + ); + }); + + it('should throw TypeError with odd-length hex string', () => { return expect(hexToBuffer.bind(null, 'c3a5c3a4c3b6a')).to.throw( TypeError, 'Argument must have a valid length of hex string.', ); }); + + it('should throw an error for an odd-length hex string input with custom argument name', () => { + return expect(hexToBuffer.bind(null, 'c3a5c3a4c3b6a', 'Custom')).to.throw( + 'Custom must have a valid length of hex string.', + ); + }); }); describe('#getFirstEightBytesReversed', () => { diff --git a/packages/lisk-cryptography/test/encrypt.js b/packages/lisk-cryptography/test/encrypt.js index 4a0a13ddd..8807534ca 100644 --- a/packages/lisk-cryptography/test/encrypt.js +++ b/packages/lisk-cryptography/test/encrypt.js @@ -264,6 +264,50 @@ describe('encrypt', () => { return expect(decrypted).to.be.equal(defaultPassphrase); }); + it('should inform the user if cipherText is missing', () => { + delete encryptedPassphrase.cipherText; + return expect( + decryptPassphraseWithPassword.bind( + null, + encryptedPassphrase, + defaultPassword, + ), + ).to.throw('Cipher text must be a string.'); + }); + + it('should inform the user if iv is missing', () => { + delete encryptedPassphrase.iv; + return expect( + decryptPassphraseWithPassword.bind( + null, + encryptedPassphrase, + defaultPassword, + ), + ).to.throw('IV must be a string.'); + }); + + it('should inform the user if salt is missing', () => { + delete encryptedPassphrase.salt; + return expect( + decryptPassphraseWithPassword.bind( + null, + encryptedPassphrase, + defaultPassword, + ), + ).to.throw('Salt must be a string.'); + }); + + it('should inform the user if tag is missing', () => { + delete encryptedPassphrase.tag; + return expect( + decryptPassphraseWithPassword.bind( + null, + encryptedPassphrase, + defaultPassword, + ), + ).to.throw('Tag must be a string.'); + }); + it('should inform the user if the salt has been altered', () => { encryptedPassphrase.salt = `00${encryptedPassphrase.salt.slice(2)}`; return expect( @@ -294,7 +338,7 @@ describe('encrypt', () => { encryptedPassphrase, defaultPassword, ), - ).to.throw('Argument must be a valid hex string.'); + ).to.throw('Tag must be a valid hex string.'); }); it('should inform the user if the tag has been altered', () => {