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>
This commit is contained in:
parent
1847670c88
commit
6ef6d42cce
@ -3258,11 +3258,10 @@ void CipherBase::InitIv(const char* cipher_type,
|
|||||||
return env()->ThrowError("Unknown cipher");
|
return env()->ThrowError("Unknown cipher");
|
||||||
}
|
}
|
||||||
|
|
||||||
/* OpenSSL versions up to 0.9.8l failed to return the correct
|
const int expected_iv_len = EVP_CIPHER_iv_length(cipher_);
|
||||||
iv_length (0) for ECB ciphers */
|
const bool is_gcm_mode = (EVP_CIPH_GCM_MODE == EVP_CIPHER_mode(cipher_));
|
||||||
if (EVP_CIPHER_iv_length(cipher_) != iv_len &&
|
|
||||||
!(EVP_CIPHER_mode(cipher_) == EVP_CIPH_ECB_MODE && iv_len == 0) &&
|
if (is_gcm_mode == false && iv_len != expected_iv_len) {
|
||||||
!(EVP_CIPHER_mode(cipher_) == EVP_CIPH_GCM_MODE) && iv_len > 0) {
|
|
||||||
return env()->ThrowError("Invalid IV length");
|
return env()->ThrowError("Invalid IV length");
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -3270,13 +3269,10 @@ void CipherBase::InitIv(const char* cipher_type,
|
|||||||
const bool encrypt = (kind_ == kCipher);
|
const bool encrypt = (kind_ == kCipher);
|
||||||
EVP_CipherInit_ex(&ctx_, cipher_, nullptr, nullptr, nullptr, encrypt);
|
EVP_CipherInit_ex(&ctx_, cipher_, nullptr, nullptr, nullptr, encrypt);
|
||||||
|
|
||||||
/* Set IV length. Only required if GCM cipher and IV is not default iv. */
|
if (is_gcm_mode &&
|
||||||
if (EVP_CIPHER_mode(cipher_) == EVP_CIPH_GCM_MODE &&
|
!EVP_CIPHER_CTX_ctrl(&ctx_, EVP_CTRL_GCM_SET_IVLEN, iv_len, nullptr)) {
|
||||||
iv_len != EVP_CIPHER_iv_length(cipher_)) {
|
EVP_CIPHER_CTX_cleanup(&ctx_);
|
||||||
if (!EVP_CIPHER_CTX_ctrl(&ctx_, EVP_CTRL_GCM_SET_IVLEN, iv_len, nullptr)) {
|
return env()->ThrowError("Invalid IV length");
|
||||||
EVP_CIPHER_CTX_cleanup(&ctx_);
|
|
||||||
return env()->ThrowError("Invalid IV length");
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!EVP_CIPHER_CTX_set_key_length(&ctx_, key_len)) {
|
if (!EVP_CIPHER_CTX_set_key_length(&ctx_, key_len)) {
|
||||||
|
@ -307,10 +307,10 @@ const TEST_CASES = [
|
|||||||
tag: 'a44a8266ee1c8eb0c8b5d4cf5ae9f19a', tampered: false },
|
tag: 'a44a8266ee1c8eb0c8b5d4cf5ae9f19a', tampered: false },
|
||||||
];
|
];
|
||||||
|
|
||||||
var ciphers = crypto.getCiphers();
|
const ciphers = crypto.getCiphers();
|
||||||
|
|
||||||
for (var i in TEST_CASES) {
|
for (const i in TEST_CASES) {
|
||||||
var test = TEST_CASES[i];
|
const test = TEST_CASES[i];
|
||||||
|
|
||||||
if (ciphers.indexOf(test.algo) === -1) {
|
if (ciphers.indexOf(test.algo) === -1) {
|
||||||
common.skip('unsupported ' + test.algo + ' test');
|
common.skip('unsupported ' + test.algo + ' test');
|
||||||
@ -359,8 +359,7 @@ for (var i in TEST_CASES) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
{
|
if (test.password) {
|
||||||
if (!test.password) return;
|
|
||||||
if (common.hasFipsCrypto) {
|
if (common.hasFipsCrypto) {
|
||||||
assert.throws(() => { crypto.createCipher(test.algo, test.password); },
|
assert.throws(() => { crypto.createCipher(test.algo, test.password); },
|
||||||
/not supported in FIPS mode/);
|
/not supported in FIPS mode/);
|
||||||
@ -379,8 +378,7 @@ for (var i in TEST_CASES) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
{
|
if (test.password) {
|
||||||
if (!test.password) return;
|
|
||||||
if (common.hasFipsCrypto) {
|
if (common.hasFipsCrypto) {
|
||||||
assert.throws(() => { crypto.createDecipher(test.algo, test.password); },
|
assert.throws(() => { crypto.createDecipher(test.algo, test.password); },
|
||||||
/not supported in FIPS mode/);
|
/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:
|
// trying to get tag before inputting all data:
|
||||||
const encrypt = crypto.createCipheriv(test.algo,
|
const encrypt = crypto.createCipheriv(test.algo,
|
||||||
@ -452,3 +432,15 @@ for (var i in TEST_CASES) {
|
|||||||
}, /Invalid IV length/);
|
}, /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/);
|
||||||
|
}
|
||||||
|
@ -61,5 +61,39 @@ testCipher1('0123456789abcd0123456789', '12345678');
|
|||||||
testCipher1('0123456789abcd0123456789', Buffer.from('12345678'));
|
testCipher1('0123456789abcd0123456789', Buffer.from('12345678'));
|
||||||
testCipher1(Buffer.from('0123456789abcd0123456789'), '12345678');
|
testCipher1(Buffer.from('0123456789abcd0123456789'), '12345678');
|
||||||
testCipher1(Buffer.from('0123456789abcd0123456789'), Buffer.from('12345678'));
|
testCipher1(Buffer.from('0123456789abcd0123456789'), Buffer.from('12345678'));
|
||||||
|
|
||||||
testCipher2(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));
|
||||||
|
}
|
||||||
|
Loading…
x
Reference in New Issue
Block a user