From 16c08c5baee61b803aaa8c9f63891f5a439b6315 Mon Sep 17 00:00:00 2001 From: Shigeki Ohtsu Date: Tue, 20 Jun 2017 23:44:53 +0900 Subject: [PATCH 1/3] crypto: warn if counter mode used in createCipher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `crypto.createCipher()` sets the fixed IV derived from password and it leads to a security risk of nonce reuse when counter mode is used. A warning is emitted when CTR, GCM or CCM is used in `crypto.createCipher()` to notify users to avoid nonce reuse. Fixes: https://github.com/nodejs/node/issues/13801 PR-URL: https://github.com/nodejs/node/pull/13821 Reviewed-By: Ben Noordhuis Reviewed-By: Fedor Indutny Reviewed-By: James M Snell Reviewed-By: Tobias Nießen --- doc/api/crypto.md | 7 ++++++- src/node_crypto.cc | 8 ++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/doc/api/crypto.md b/doc/api/crypto.md index e8d9a94462b5e0..bad128c9ec0ba8 100644 --- a/doc/api/crypto.md +++ b/doc/api/crypto.md @@ -1102,7 +1102,11 @@ rapidly. In line with OpenSSL's recommendation to use pbkdf2 instead of [`EVP_BytesToKey`][] it is recommended that developers derive a key and IV on their own using [`crypto.pbkdf2()`][] and to use [`crypto.createCipheriv()`][] -to create the `Cipher` object. +to create the `Cipher` object. Users should not use ciphers with counter mode +(e.g. CTR, GCM or CCM) in `crypto.createCipher()`. A warning is emitted when +they are used in order to avoid the risk of IV reuse that causes +vulnerabilities. For the case when IV is reused in GCM, see [Nonce-Disrespecting +Adversaries][] for details. ### crypto.createCipheriv(algorithm, key, iv) @@ -2023,6 +2027,7 @@ the `crypto`, `tls`, and `https` modules and are generally specific to OpenSSL. [NIST SP 800-131A]: http://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar1.pdf [NIST SP 800-132]: http://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-132.pdf [OpenSSL cipher list format]: https://www.openssl.org/docs/man1.0.2/apps/ciphers.html#CIPHER-LIST-FORMAT +[Nonce-Disrespecting Adversaries]: https://github.com/nonce-disrespect/nonce-disrespect [OpenSSL's SPKAC implementation]: https://www.openssl.org/docs/man1.0.2/apps/spkac.html [publicly trusted list of CAs]: https://mxr.mozilla.org/mozilla/source/security/nss/lib/ckfw/builtins/certdata.txt [RFC 2412]: https://www.rfc-editor.org/rfc/rfc2412.txt diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 99ed0ddf0808bb..c14ae987803f6f 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -3352,6 +3352,14 @@ void CipherBase::Init(const char* cipher_type, EVP_CIPHER_CTX_init(&ctx_); const bool encrypt = (kind_ == kCipher); EVP_CipherInit_ex(&ctx_, cipher_, nullptr, nullptr, nullptr, encrypt); + + int mode = EVP_CIPHER_CTX_mode(&ctx_); + if (encrypt && (mode == EVP_CIPH_CTR_MODE || mode == EVP_CIPH_GCM_MODE || + mode == EVP_CIPH_CCM_MODE)) { + ProcessEmitWarning(env(), "Use Cipheriv for counter mode of %s", + cipher_type); + } + if (!EVP_CIPHER_CTX_set_key_length(&ctx_, key_len)) { EVP_CIPHER_CTX_cleanup(&ctx_); return env()->ThrowError("Invalid key length"); From 0be25bba09c97688b5852b87b24da3cb22fedf66 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sun, 29 Oct 2017 11:27:55 +0100 Subject: [PATCH 2/3] test: add regression test for counter mode warning The previous commit is a back-port of pull request #13821 to v6.x. Its regression test does not apply to the v6.x branch (depends on semver-major pull request #9405) so this commit adds a new test. Refs: https://github.com/nodejs/node/pull/13821 Refs: https://github.com/nodejs/node/pull/9405 --- test/parallel/test-crypto-cipher-decipher.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/parallel/test-crypto-cipher-decipher.js b/test/parallel/test-crypto-cipher-decipher.js index 8b1b0051d34962..6b2df3d81fd893 100644 --- a/test/parallel/test-crypto-cipher-decipher.js +++ b/test/parallel/test-crypto-cipher-decipher.js @@ -148,3 +148,7 @@ testCipher2(Buffer.from('0123456789abcdef')); assert.strictEqual(decipher.setAuthTag(tagbuf), decipher); assert.strictEqual(decipher.setAAD(aadbuf), decipher); } + +// https://github.com/nodejs/node/issues/13801 +common.expectWarning('Warning', 'Use Cipheriv for counter mode of aes-256-gcm'); +crypto.createCipher('aes-256-gcm', '0123456789'); From 07e141c8285299ea1259186ee5f2900de98fc8cf Mon Sep 17 00:00:00 2001 From: Shigeki Ohtsu Date: Fri, 25 Aug 2017 01:42:55 +0900 Subject: [PATCH 3/3] crypto: fix error of createCipher in wrap mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EVP_CIPHER_CTX_FLAG_WRAP_ALLOW flag needs to be set in using wrap mode ciphers. In `crypto.createCipher()`, AES key wrap mode does not use a default IV defined in RFC3394 but a generated IV with `EVP_BytesToKey()` to be consistent API behaviors with other ciphers. The built-in AES wrap mode in OpenSSL is not supported in FIPS mode as http://openssl.6102.n7.nabble.com/AES-Key-Wrap-in-FIPS-Mode-td50238.html so its tests in FIPS mode are skipped. Fixes: https://github.com/nodejs/node/issues/15009 PR-URL: https://github.com/nodejs/node/pull/15037 Reviewed-By: Fedor Indutny Reviewed-By: Ben Noordhuis Reviewed-By: Tobias Nießen Reviewed-By: James M Snell --- src/node_crypto.cc | 10 +++++++- test/parallel/test-crypto-binary-default.js | 21 ++++++++++++++++ .../test-crypto-cipheriv-decipheriv.js | 24 +++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index c14ae987803f6f..345bf078a1fec7 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -3360,6 +3360,9 @@ void CipherBase::Init(const char* cipher_type, cipher_type); } + if (mode == EVP_CIPH_WRAP_MODE) + EVP_CIPHER_CTX_set_flags(&ctx_, EVP_CIPHER_CTX_FLAG_WRAP_ALLOW); + if (!EVP_CIPHER_CTX_set_key_length(&ctx_, key_len)) { EVP_CIPHER_CTX_cleanup(&ctx_); return env()->ThrowError("Invalid key length"); @@ -3407,13 +3410,18 @@ void CipherBase::InitIv(const char* cipher_type, } const int expected_iv_len = EVP_CIPHER_iv_length(cipher_); - const bool is_gcm_mode = (EVP_CIPH_GCM_MODE == EVP_CIPHER_mode(cipher_)); + const int mode = EVP_CIPHER_mode(cipher_); + const bool is_gcm_mode = (EVP_CIPH_GCM_MODE == mode); if (is_gcm_mode == false && iv_len != expected_iv_len) { return env()->ThrowError("Invalid IV length"); } EVP_CIPHER_CTX_init(&ctx_); + + if (mode == EVP_CIPH_WRAP_MODE) + EVP_CIPHER_CTX_set_flags(&ctx_, EVP_CIPHER_CTX_FLAG_WRAP_ALLOW); + const bool encrypt = (kind_ == kCipher); EVP_CipherInit_ex(&ctx_, cipher_, nullptr, nullptr, nullptr, encrypt); diff --git a/test/parallel/test-crypto-binary-default.js b/test/parallel/test-crypto-binary-default.js index 2343360df9fadf..c477301256d919 100644 --- a/test/parallel/test-crypto-binary-default.js +++ b/test/parallel/test-crypto-binary-default.js @@ -509,12 +509,33 @@ function testCipher4(key, iv) { 'encryption and decryption with key and iv'); } + +function testCipher5(key, iv) { + // Test encryption and decryption with explicit key with aes128-wrap + const plaintext = + '32|RmVZZkFUVmpRRkp0TmJaUm56ZU9qcnJkaXNNWVNpTTU*|iXmckfRWZBGWWELw' + + 'eCBsThSsfUHLeRe0KCsK8ooHgxie0zOINpXxfZi/oNG7uq9JWFVCk70gfzQH8ZUJ' + + 'jAfaFg**'; + const cipher = crypto.createCipher('id-aes128-wrap', key); + let ciph = cipher.update(plaintext, 'utf8', 'buffer'); + ciph = Buffer.concat([ciph, cipher.final('buffer')]); + + const decipher = crypto.createDecipher('id-aes128-wrap', key); + let txt = decipher.update(ciph, 'buffer', 'utf8'); + txt += decipher.final('utf8'); + + assert.strictEqual(txt, plaintext, + 'encryption and decryption with key'); +} + if (!common.hasFipsCrypto) { testCipher1('MySecretKey123'); testCipher1(Buffer.from('MySecretKey123')); testCipher2('0123456789abcdef'); testCipher2(Buffer.from('0123456789abcdef')); + + testCipher5(Buffer.from('0123456789abcd0123456789')); } testCipher3('0123456789abcd0123456789', '12345678'); diff --git a/test/parallel/test-crypto-cipheriv-decipheriv.js b/test/parallel/test-crypto-cipheriv-decipheriv.js index 2ee8e813233e77..285440643c42b8 100644 --- a/test/parallel/test-crypto-cipheriv-decipheriv.js +++ b/test/parallel/test-crypto-cipheriv-decipheriv.js @@ -55,12 +55,36 @@ function testCipher2(key, iv) { assert.strictEqual(txt, plaintext, 'encryption/decryption with key and iv'); } + +function testCipher3(key, iv) { + // Test encryption and decryption with explicit key and iv. + // AES Key Wrap test vector comes from RFC3394 + const plaintext = Buffer.from('00112233445566778899AABBCCDDEEFF', 'hex'); + + const cipher = crypto.createCipheriv('id-aes128-wrap', key, iv); + let ciph = cipher.update(plaintext, 'utf8', 'buffer'); + ciph = Buffer.concat([ciph, cipher.final('buffer')]); + const ciph2 = Buffer.from('1FA68B0A8112B447AEF34BD8FB5A7B829D3E862371D2CFE5', + 'hex'); + assert(ciph.equals(ciph2)); + const decipher = crypto.createDecipheriv('id-aes128-wrap', key, iv); + let deciph = decipher.update(ciph, 'buffer'); + deciph = Buffer.concat([deciph, decipher.final()]); + + assert(deciph.equals(plaintext), 'encryption/decryption with key and iv'); +} + 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')); +if (!common.hasFipsCrypto) { + testCipher3(Buffer.from('000102030405060708090A0B0C0D0E0F', 'hex'), + Buffer.from('A6A6A6A6A6A6A6A6', 'hex')); +} + // Zero-sized IV should be accepted in ECB mode. crypto.createCipheriv('aes-128-ecb', Buffer.alloc(16), Buffer.alloc(0));