Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 9389572

Browse filesBrowse files
bnoordhuisMyles Borins
authored andcommitted
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: #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>
1 parent 62e83b3 commit 9389572
Copy full SHA for 9389572

File tree

Expand file treeCollapse file tree

3 files changed

+58
-15
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+58
-15
lines changed
Open diff view settings
Collapse file

‎src/node_crypto.cc‎

Copy file name to clipboardExpand all lines: src/node_crypto.cc
+8-12Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3068,25 +3068,21 @@ void CipherBase::InitIv(const char* cipher_type,
30683068
return env()->ThrowError("Unknown cipher");
30693069
}
30703070

3071-
/* OpenSSL versions up to 0.9.8l failed to return the correct
3072-
iv_length (0) for ECB ciphers */
3073-
if (EVP_CIPHER_iv_length(cipher_) != iv_len &&
3074-
!(EVP_CIPHER_mode(cipher_) == EVP_CIPH_ECB_MODE && iv_len == 0) &&
3075-
!(EVP_CIPHER_mode(cipher_) == EVP_CIPH_GCM_MODE) && iv_len > 0) {
3071+
const int expected_iv_len = EVP_CIPHER_iv_length(cipher_);
3072+
const bool is_gcm_mode = (EVP_CIPH_GCM_MODE == EVP_CIPHER_mode(cipher_));
3073+
3074+
if (is_gcm_mode == false && iv_len != expected_iv_len) {
30763075
return env()->ThrowError("Invalid IV length");
30773076
}
30783077

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

3083-
/* Set IV length. Only required if GCM cipher and IV is not default iv. */
3084-
if (EVP_CIPHER_mode(cipher_) == EVP_CIPH_GCM_MODE &&
3085-
iv_len != EVP_CIPHER_iv_length(cipher_)) {
3086-
if (!EVP_CIPHER_CTX_ctrl(&ctx_, EVP_CTRL_GCM_SET_IVLEN, iv_len, nullptr)) {
3087-
EVP_CIPHER_CTX_cleanup(&ctx_);
3088-
return env()->ThrowError("Invalid IV length");
3089-
}
3082+
if (is_gcm_mode &&
3083+
!EVP_CIPHER_CTX_ctrl(&ctx_, EVP_CTRL_GCM_SET_IVLEN, iv_len, nullptr)) {
3084+
EVP_CIPHER_CTX_cleanup(&ctx_);
3085+
return env()->ThrowError("Invalid IV length");
30903086
}
30913087

30923088
if (!EVP_CIPHER_CTX_set_key_length(&ctx_, key_len)) {
Collapse file

‎test/parallel/test-crypto-authenticated.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-crypto-authenticated.js
+15-3Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -307,10 +307,10 @@ const TEST_CASES = [
307307
tag: 'a44a8266ee1c8eb0c8b5d4cf5ae9f19a', tampered: false },
308308
];
309309

310-
var ciphers = crypto.getCiphers();
310+
const ciphers = crypto.getCiphers();
311311

312-
for (var i in TEST_CASES) {
313-
var test = TEST_CASES[i];
312+
for (const i in TEST_CASES) {
313+
const test = TEST_CASES[i];
314314

315315
if (ciphers.indexOf(test.algo) === -1) {
316316
common.skip('unsupported ' + test.algo + ' test');
@@ -452,3 +452,15 @@ for (var i in TEST_CASES) {
452452
}, /Invalid IV length/);
453453
})();
454454
}
455+
456+
// Non-authenticating mode:
457+
(function() {
458+
const encrypt =
459+
crypto.createCipheriv('aes-128-cbc',
460+
'ipxp9a6i1Mb4USb4',
461+
'6fKjEjR3Vl30EUYC');
462+
encrypt.update('blah', 'ascii');
463+
encrypt.final();
464+
assert.throws(() => encrypt.getAuthTag(), / state/);
465+
assert.throws(() => encrypt.setAAD(Buffer.from('123', 'ascii')), / state/);
466+
})();
Collapse file

‎test/parallel/test-crypto-cipheriv-decipheriv.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-crypto-cipheriv-decipheriv.js
+35Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,3 +63,38 @@ testCipher1(new Buffer('0123456789abcd0123456789'), '12345678');
6363
testCipher1(new Buffer('0123456789abcd0123456789'), new Buffer('12345678'));
6464

6565
testCipher2(new Buffer('0123456789abcd0123456789'), new Buffer('12345678'));
66+
67+
// Zero-sized IV should be accepted in ECB mode.
68+
crypto.createCipheriv('aes-128-ecb', Buffer.alloc(16), Buffer.alloc(0));
69+
70+
// But non-empty IVs should be rejected.
71+
for (let n = 1; n < 256; n += 1) {
72+
assert.throws(
73+
() => crypto.createCipheriv('aes-128-ecb', Buffer.alloc(16),
74+
Buffer.alloc(n)),
75+
/Invalid IV length/);
76+
}
77+
78+
// Correctly sized IV should be accepted in CBC mode.
79+
crypto.createCipheriv('aes-128-cbc', Buffer.alloc(16), Buffer.alloc(16));
80+
81+
// But all other IV lengths should be rejected.
82+
for (let n = 0; n < 256; n += 1) {
83+
if (n === 16) continue;
84+
assert.throws(
85+
() => crypto.createCipheriv('aes-128-cbc', Buffer.alloc(16),
86+
Buffer.alloc(n)),
87+
/Invalid IV length/);
88+
}
89+
90+
// Zero-sized IV should be rejected in GCM mode.
91+
assert.throws(
92+
() => crypto.createCipheriv('aes-128-gcm', Buffer.alloc(16),
93+
Buffer.alloc(0)),
94+
/Invalid IV length/);
95+
96+
// But all other IV lengths should be accepted.
97+
for (let n = 1; n < 256; n += 1) {
98+
if (common.hasFipsCrypto && n < 12) continue;
99+
crypto.createCipheriv('aes-128-gcm', Buffer.alloc(16), Buffer.alloc(n));
100+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.