diff options
author | Ben Noordhuis <info@bnoordhuis.nl> | 2016-10-11 20:01:32 +0200 |
---|---|---|
committer | Ben Noordhuis <info@bnoordhuis.nl> | 2016-10-17 17:05:40 +0200 |
commit | 6ef6d42cce8535aa3f25b07ede5942f74959033a (patch) | |
tree | 96fd539dbc6c599c1acc833e01b90c80bbe75620 /test/parallel | |
parent | 1847670c88da5a10cdae6380b21af696b329471d (diff) | |
download | node-new-6ef6d42cce8535aa3f25b07ede5942f74959033a.tar.gz |
crypto: fix faulty logic in iv size check
Fix a regression introduced in commit 2996b5c ("crypto: Allow GCM
ciphers to have a longer IV length") from April 2016 where a misplaced
parenthesis in a 'is ECB cipher?' check made it possible to use empty
IVs with non-ECB ciphers.
Also fix some exit bugs in test/parallel/test-crypto-authenticated.js
that were introduced in commit 4a40832 ("test: cleanup IIFE tests")
where removing the IFFEs made the test exit prematurely instead of just
skipping subtests.
PR-URL: https://github.com/nodejs/node/pull/9032
Refs: https://github.com/nodejs/node/pull/6376
Refs: https://github.com/nodejs/node/issues/9024
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Diffstat (limited to 'test/parallel')
-rw-r--r-- | test/parallel/test-crypto-authenticated.js | 42 | ||||
-rw-r--r-- | test/parallel/test-crypto-cipheriv-decipheriv.js | 36 |
2 files changed, 52 insertions, 26 deletions
diff --git a/test/parallel/test-crypto-authenticated.js b/test/parallel/test-crypto-authenticated.js index 0e1c1298bf..8eab91e549 100644 --- a/test/parallel/test-crypto-authenticated.js +++ b/test/parallel/test-crypto-authenticated.js @@ -307,10 +307,10 @@ const TEST_CASES = [ tag: 'a44a8266ee1c8eb0c8b5d4cf5ae9f19a', tampered: false }, ]; -var ciphers = crypto.getCiphers(); +const ciphers = crypto.getCiphers(); -for (var i in TEST_CASES) { - var test = TEST_CASES[i]; +for (const i in TEST_CASES) { + const test = TEST_CASES[i]; if (ciphers.indexOf(test.algo) === -1) { common.skip('unsupported ' + test.algo + ' test'); @@ -359,8 +359,7 @@ for (var i in TEST_CASES) { } } - { - if (!test.password) return; + if (test.password) { if (common.hasFipsCrypto) { assert.throws(() => { crypto.createCipher(test.algo, test.password); }, /not supported in FIPS mode/); @@ -379,8 +378,7 @@ for (var i in TEST_CASES) { } } - { - if (!test.password) return; + if (test.password) { if (common.hasFipsCrypto) { assert.throws(() => { crypto.createDecipher(test.algo, test.password); }, /not supported in FIPS mode/); @@ -400,24 +398,6 @@ for (var i in TEST_CASES) { } } - // after normal operation, test some incorrect ways of calling the API: - // it's most certainly enough to run these tests with one algorithm only. - - if (i > 0) { - continue; - } - - { - // non-authenticating mode: - const encrypt = crypto.createCipheriv('aes-128-cbc', - 'ipxp9a6i1Mb4USb4', '6fKjEjR3Vl30EUYC'); - encrypt.update('blah', 'ascii'); - encrypt.final(); - assert.throws(() => { encrypt.getAuthTag(); }, / state/); - assert.throws(() => { encrypt.setAAD(Buffer.from('123', 'ascii')); }, - / state/); - } - { // trying to get tag before inputting all data: const encrypt = crypto.createCipheriv(test.algo, @@ -452,3 +432,15 @@ for (var i in TEST_CASES) { }, /Invalid IV length/); } } + +// Non-authenticating mode: +{ + const encrypt = + crypto.createCipheriv('aes-128-cbc', + 'ipxp9a6i1Mb4USb4', + '6fKjEjR3Vl30EUYC'); + encrypt.update('blah', 'ascii'); + encrypt.final(); + assert.throws(() => encrypt.getAuthTag(), / state/); + assert.throws(() => encrypt.setAAD(Buffer.from('123', 'ascii')), / state/); +} diff --git a/test/parallel/test-crypto-cipheriv-decipheriv.js b/test/parallel/test-crypto-cipheriv-decipheriv.js index 9c2091a836..a3a14738c4 100644 --- a/test/parallel/test-crypto-cipheriv-decipheriv.js +++ b/test/parallel/test-crypto-cipheriv-decipheriv.js @@ -61,5 +61,39 @@ testCipher1('0123456789abcd0123456789', '12345678'); testCipher1('0123456789abcd0123456789', Buffer.from('12345678')); testCipher1(Buffer.from('0123456789abcd0123456789'), '12345678'); testCipher1(Buffer.from('0123456789abcd0123456789'), Buffer.from('12345678')); - testCipher2(Buffer.from('0123456789abcd0123456789'), Buffer.from('12345678')); + +// Zero-sized IV should be accepted in ECB mode. +crypto.createCipheriv('aes-128-ecb', Buffer.alloc(16), Buffer.alloc(0)); + +// But non-empty IVs should be rejected. +for (let n = 1; n < 256; n += 1) { + assert.throws( + () => crypto.createCipheriv('aes-128-ecb', Buffer.alloc(16), + Buffer.alloc(n)), + /Invalid IV length/); +} + +// Correctly sized IV should be accepted in CBC mode. +crypto.createCipheriv('aes-128-cbc', Buffer.alloc(16), Buffer.alloc(16)); + +// But all other IV lengths should be rejected. +for (let n = 0; n < 256; n += 1) { + if (n === 16) continue; + assert.throws( + () => crypto.createCipheriv('aes-128-cbc', Buffer.alloc(16), + Buffer.alloc(n)), + /Invalid IV length/); +} + +// Zero-sized IV should be rejected in GCM mode. +assert.throws( + () => crypto.createCipheriv('aes-128-gcm', Buffer.alloc(16), + Buffer.alloc(0)), + /Invalid IV length/); + +// But all other IV lengths should be accepted. +for (let n = 1; n < 256; n += 1) { + if (common.hasFipsCrypto && n < 12) continue; + crypto.createCipheriv('aes-128-gcm', Buffer.alloc(16), Buffer.alloc(n)); +} |