From b9787fd5f314943ae66b43ca4af5c04867a233c8 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Thu, 7 Mar 2019 11:03:06 -0800 Subject: [PATCH] crypto: check for invalid chacha20-poly1305 IVs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IV lengths of 13, 14, 15, and 16 are invalid, but are not checked by OpenSSL. IV lengths of 17 or greater are also invalid, but they were already checked by OpenSSL. See: - https://github.com/openssl/openssl/commit/f426625b6a - https://www.openssl.org/news/secadv/20190306.txt PR-URL: https://github.com/nodejs/node/pull/26537 Reviewed-By: Ben Noordhuis Reviewed-By: Tobias Nießen Reviewed-By: Ruben Bridgewater Reviewed-By: James M Snell Reviewed-By: Michael Dawson --- src/node_crypto.cc | 10 +++++ test/parallel/test-crypto-authenticated.js | 48 ++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 8bf795ab4a88e1..c6de29693ffc53 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -3660,6 +3660,16 @@ void CipherBase::InitIv(const char* cipher_type, return env()->ThrowError("Invalid IV length"); } + if (EVP_CIPHER_nid(cipher) == NID_chacha20_poly1305) { + CHECK(has_iv); + // Check for invalid IV lengths, since OpenSSL does not under some + // conditions: + // https://www.openssl.org/news/secadv/20190306.txt. + if (iv_len > 12) { + return env()->ThrowError("Invalid IV length"); + } + } + CommonInit(cipher_type, cipher, key, key_len, iv, iv_len, auth_tag_len); } diff --git a/test/parallel/test-crypto-authenticated.js b/test/parallel/test-crypto-authenticated.js index c1abf12b8be3a3..8be7296c690769 100644 --- a/test/parallel/test-crypto-authenticated.js +++ b/test/parallel/test-crypto-authenticated.js @@ -616,3 +616,51 @@ for (const test of TEST_CASES) { assert(plain.equals(plaintext)); } } + + +// Test chacha20-poly1305 rejects invalid IV lengths of 13, 14, 15, and 16 (a +// length of 17 or greater was already rejected). +// - https://www.openssl.org/news/secadv/20190306.txt +{ + // Valid extracted from TEST_CASES, check that it detects IV tampering. + const valid = { + algo: 'chacha20-poly1305', + key: '808182838485868788898a8b8c8d8e8f909192939495969798999a9b9c9d9e9f', + iv: '070000004041424344454647', + plain: '4c616469657320616e642047656e746c656d656e206f662074686520636c6173' + + '73206f66202739393a204966204920636f756c64206f6666657220796f75206f' + + '6e6c79206f6e652074697020666f7220746865206675747572652c2073756e73' + + '637265656e20776f756c642062652069742e', + plainIsHex: true, + aad: '50515253c0c1c2c3c4c5c6c7', + ct: 'd31a8d34648e60db7b86afbc53ef7ec2a4aded51296e08fea9e2b5' + + 'a736ee62d63dbea45e8ca9671282fafb69da92728b1a71de0a9e06' + + '0b2905d6a5b67ecd3b3692ddbd7f2d778b8c9803aee328091b58fa' + + 'b324e4fad675945585808b4831d7bc3ff4def08e4b7a9de576d265' + + '86cec64b6116', + tag: '1ae10b594f09e26a7e902ecbd0600691', + tampered: false, + }; + + // Invalid IV lengths should be detected: + // - 12 and below are valid. + // - 13-16 are not detected as invalid by some OpenSSL versions. + check(13); + check(14); + check(15); + check(16); + // - 17 and above were always detected as invalid by OpenSSL. + check(17); + + function check(ivLength) { + const prefix = ivLength - valid.iv.length / 2; + assert.throws(() => crypto.createCipheriv( + valid.algo, + Buffer.from(valid.key, 'hex'), + Buffer.from(H(prefix) + valid.iv, 'hex'), + { authTagLength: valid.tag.length / 2 } + ), errMessages.length, `iv length ${ivLength} was not rejected`); + + function H(length) { return '00'.repeat(length); } + } +}