Skip to content

Commit

Permalink
crypto: fix faulty logic in iv size check
Browse files Browse the repository at this point in the history
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: #9032
Refs: #6376
Refs: #9024
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
  • Loading branch information
bnoordhuis authored and Myles Borins committed Nov 22, 2016
1 parent 62e83b3 commit 9389572
Showing 3 changed files with 58 additions and 15 deletions.
20 changes: 8 additions & 12 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
@@ -3068,25 +3068,21 @@ void CipherBase::InitIv(const char* cipher_type,
return env()->ThrowError("Unknown cipher");
}

/* OpenSSL versions up to 0.9.8l failed to return the correct
iv_length (0) for ECB ciphers */
if (EVP_CIPHER_iv_length(cipher_) != iv_len &&
!(EVP_CIPHER_mode(cipher_) == EVP_CIPH_ECB_MODE && iv_len == 0) &&
!(EVP_CIPHER_mode(cipher_) == EVP_CIPH_GCM_MODE) && iv_len > 0) {
const int expected_iv_len = EVP_CIPHER_iv_length(cipher_);
const bool is_gcm_mode = (EVP_CIPH_GCM_MODE == EVP_CIPHER_mode(cipher_));

if (is_gcm_mode == false && iv_len != expected_iv_len) {
return env()->ThrowError("Invalid IV length");
}

EVP_CIPHER_CTX_init(&ctx_);
const bool encrypt = (kind_ == kCipher);
EVP_CipherInit_ex(&ctx_, cipher_, nullptr, nullptr, nullptr, encrypt);

/* Set IV length. Only required if GCM cipher and IV is not default iv. */
if (EVP_CIPHER_mode(cipher_) == EVP_CIPH_GCM_MODE &&
iv_len != EVP_CIPHER_iv_length(cipher_)) {
if (!EVP_CIPHER_CTX_ctrl(&ctx_, EVP_CTRL_GCM_SET_IVLEN, iv_len, nullptr)) {
EVP_CIPHER_CTX_cleanup(&ctx_);
return env()->ThrowError("Invalid IV length");
}
if (is_gcm_mode &&
!EVP_CIPHER_CTX_ctrl(&ctx_, EVP_CTRL_GCM_SET_IVLEN, iv_len, nullptr)) {
EVP_CIPHER_CTX_cleanup(&ctx_);
return env()->ThrowError("Invalid IV length");
}

if (!EVP_CIPHER_CTX_set_key_length(&ctx_, key_len)) {
18 changes: 15 additions & 3 deletions test/parallel/test-crypto-authenticated.js
Original file line number Diff line number Diff line change
@@ -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');
@@ -452,3 +452,15 @@ for (var i in TEST_CASES) {
}, /Invalid IV length/);
})();
}

// Non-authenticating mode:
(function() {
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/);
})();
35 changes: 35 additions & 0 deletions test/parallel/test-crypto-cipheriv-decipheriv.js
Original file line number Diff line number Diff line change
@@ -63,3 +63,38 @@ testCipher1(new Buffer('0123456789abcd0123456789'), '12345678');
testCipher1(new Buffer('0123456789abcd0123456789'), new Buffer('12345678'));

testCipher2(new Buffer('0123456789abcd0123456789'), new Buffer('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));
}

0 comments on commit 9389572

Please sign in to comment.