From cc8976b7a1f513041863eb510783764c6c86def8 Mon Sep 17 00:00:00 2001 From: Ian Carroll Date: Mon, 26 Jun 2017 23:06:18 -0400 Subject: [PATCH 01/13] crypto: Deprecate createCipher for createCipheriv --- src/node_crypto.cc | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 9d196c10cc8c3f..ece2e96c47ed5f 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -3340,7 +3340,6 @@ void CipherBase::Init(const char* cipher_type, } unsigned char key[EVP_MAX_KEY_LENGTH]; - unsigned char iv[EVP_MAX_IV_LENGTH]; int key_len = EVP_BytesToKey(cipher_, EVP_md5(), @@ -3349,11 +3348,20 @@ void CipherBase::Init(const char* cipher_type, key_buf_len, 1, key, - iv); + nullptr); EVP_CIPHER_CTX_init(&ctx_); const bool encrypt = (kind_ == kCipher); + EVP_CipherInit_ex(&ctx_, cipher_, nullptr, nullptr, nullptr, encrypt); + + const int mode = EVP_CIPHER_CTX_mode(&ctx_); + const int iv_len = EVP_CIPHER_iv_length(cipher_); + + if (encrypt && iv_len != 0) { + return env()->ThrowError("crypto.createCipher() is no longer supported with ciphers that require initialization vectors. Generate a random IV and pass it to crypto.createCipheriv()."); + } + if (!EVP_CIPHER_CTX_set_key_length(&ctx_, key_len)) { EVP_CIPHER_CTX_cleanup(&ctx_); return env()->ThrowError("Invalid key length"); @@ -3363,7 +3371,7 @@ void CipherBase::Init(const char* cipher_type, nullptr, nullptr, reinterpret_cast(key), - reinterpret_cast(iv), + nullptr, kind_ == kCipher); initialised_ = true; } @@ -3443,14 +3451,22 @@ void CipherBase::InitIv(const FunctionCallbackInfo& args) { THROW_AND_RETURN_IF_NOT_STRING(args[0], "Cipher type"); THROW_AND_RETURN_IF_NOT_BUFFER(args[1], "Key"); - THROW_AND_RETURN_IF_NOT_BUFFER(args[2], "IV"); + + if (!Buffer::HasInstance(args[2]) && !args[2]->IsNull()) { + return env->ThrowTypeError("IV must be a buffer or null"); + } const node::Utf8Value cipher_type(env->isolate(), args[0]); ssize_t key_len = Buffer::Length(args[1]); const char* key_buf = Buffer::Data(args[1]); - ssize_t iv_len = Buffer::Length(args[2]); - const char* iv_buf = Buffer::Data(args[2]); - cipher->InitIv(*cipher_type, key_buf, key_len, iv_buf, iv_len); + + if (args[2]->IsNull()) { + cipher->InitIv(*cipher_type, key_buf, key_len, nullptr, 0); + } else { + ssize_t iv_len = Buffer::Length(args[2]); + const char* iv_buf = Buffer::Data(args[2]); + cipher->InitIv(*cipher_type, key_buf, key_len, iv_buf, iv_len); + } } From 3d0b5a59649e76e61c1ec24045d602a097a3a05f Mon Sep 17 00:00:00 2001 From: Ian Carroll Date: Mon, 26 Jun 2017 23:23:02 -0400 Subject: [PATCH 02/13] crypto: remove unused mode variable --- src/node_crypto.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index ece2e96c47ed5f..c1855cc9204fab 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -3355,7 +3355,6 @@ void CipherBase::Init(const char* cipher_type, EVP_CipherInit_ex(&ctx_, cipher_, nullptr, nullptr, nullptr, encrypt); - const int mode = EVP_CIPHER_CTX_mode(&ctx_); const int iv_len = EVP_CIPHER_iv_length(cipher_); if (encrypt && iv_len != 0) { From 1e810d0bb50f0286e0c975e601799f8a3bc06049 Mon Sep 17 00:00:00 2001 From: Ian Carroll Date: Tue, 4 Jul 2017 01:57:32 -0400 Subject: [PATCH 03/13] crypto: mark createCipher as deprecated --- doc/api/deprecations.md | 15 +++++++++++++++ lib/crypto.js | 21 +++++++++++++++++++-- src/node_crypto.cc | 2 +- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 5fb3974bb1a67b..08888a7e43bb68 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -634,6 +634,19 @@ Type: Runtime *Note*: change was made while `async_hooks` was an experimental API. + +### DEP00XX: crypto.createCipher() + +Type: End-of-Life + +[`crypto.createCipher()`][] generates keys from strings in an insecure manner, and, when used with a cipher that utilizes an initialization vector, will dangerously re-use initialization vectors. As such, it is immediately marked as End-of-Life when used with ciphers that require initialization vectors, and will be fully removed in a later version. + +[`crypto.createCipheriv()`][] should be used in place of [`crypto.createCipher()`][]. Since [`crypto.createCipheriv()`][] will no longer attempt to derive a proper encryption key from a string, you must use a key-derivation function such as [`crypto.pbkdf2()`][] to obtain a valid key if you normally supply a string to [`crypto.createCipher()`][]. + +Additionally, for ciphers that require an initialization vector, you must generate an initialization vector with the same length as the cipher's key length to pass to [`crypto.createCipheriv()`][]. Initialization vectors must never be re-used, especially in modes such as AES-CTR, where encryption is effectively removed upon reuse. Your application will need to store this initialization vector along with the encrypted data, as it is required for decryption. + +If an initialization vector is not needed by the cipher, you may pass `null` or omit the argument. + [`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size [`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array [`Buffer.from(buffer)`]: buffer.html#buffer_class_method_buffer_from_buffer @@ -647,6 +660,8 @@ Type: Runtime [`child_process`]: child_process.html [`console.error()`]: console.html#console_console_error_data_args [`console.log()`]: console.html#console_console_log_data_args +[`crypto.createCipher()`]: crypto.html#crypto_crypto_createcipher_algorithm_password +[`crypto.createCipheriv()`]: crypto.html#crypto_crypto_createcipheriv_algorithm_key_iv [`crypto.createCredentials()`]: crypto.html#crypto_crypto_createcredentials_details [`crypto.pbkdf2()`]: crypto.html#crypto_crypto_pbkdf2_password_salt_iterations_keylen_digest_callback [`domain`]: domain.html diff --git a/lib/crypto.js b/lib/crypto.js index 8893aa742519ea..098a7fbb491efc 100644 --- a/lib/crypto.js +++ b/lib/crypto.js @@ -131,11 +131,28 @@ function getDecoder(decoder, encoding) { return decoder; } +Object.defineProperty(exports, 'createCipher', { + configurable: true, + enumerable: true, + get: internalUtil.deprecate(function() { + return Cipher; + }, 'crypto.createCipher is deprecated. ' + + 'Use crypto.createCipheriv instead.', 'DEP00XX') +}); + +Object.defineProperty(exports, 'Cipher', { + configurable: true, + enumerable: true, + get: internalUtil.deprecate(function() { + return Cipher; + }, 'crypto.Cipher is deprecated. ' + + 'Use crypto.createCipheriv instead.', 'DEP00XX') +}); -exports.createCipher = exports.Cipher = Cipher; function Cipher(cipher, password, options) { if (!(this instanceof Cipher)) return new Cipher(cipher, password, options); + this._handle = new binding.CipherBase(true); this._handle.init(cipher, toBuf(password)); @@ -214,7 +231,7 @@ function Cipheriv(cipher, key, iv, options) { if (!(this instanceof Cipheriv)) return new Cipheriv(cipher, key, iv, options); this._handle = new binding.CipherBase(true); - this._handle.initiv(cipher, toBuf(key), toBuf(iv)); + this._handle.initiv(cipher, toBuf(key), iv ? toBuf(iv) : null); this._decoder = null; LazyTransform.call(this, options); diff --git a/src/node_crypto.cc b/src/node_crypto.cc index c1855cc9204fab..e322a2ace6e9cc 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -3459,7 +3459,7 @@ void CipherBase::InitIv(const FunctionCallbackInfo& args) { ssize_t key_len = Buffer::Length(args[1]); const char* key_buf = Buffer::Data(args[1]); - if (args[2]->IsNull()) { + if (args.Length() < 3 || args[2]->IsNull()) { cipher->InitIv(*cipher_type, key_buf, key_len, nullptr, 0); } else { ssize_t iv_len = Buffer::Length(args[2]); From 8f2b81ff776cae1894af5fb4ba399acfcae5886c Mon Sep 17 00:00:00 2001 From: Ian Carroll Date: Tue, 4 Jul 2017 16:39:55 -0400 Subject: [PATCH 04/13] crypto: add legacy KDF methods --- doc/api/deprecations.md | 2 +- lib/crypto.js | 12 +++++++ src/node_crypto.cc | 78 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 1 deletion(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 08888a7e43bb68..0cc96261541c62 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -643,7 +643,7 @@ Type: End-of-Life [`crypto.createCipheriv()`][] should be used in place of [`crypto.createCipher()`][]. Since [`crypto.createCipheriv()`][] will no longer attempt to derive a proper encryption key from a string, you must use a key-derivation function such as [`crypto.pbkdf2()`][] to obtain a valid key if you normally supply a string to [`crypto.createCipher()`][]. -Additionally, for ciphers that require an initialization vector, you must generate an initialization vector with the same length as the cipher's key length to pass to [`crypto.createCipheriv()`][]. Initialization vectors must never be re-used, especially in modes such as AES-CTR, where encryption is effectively removed upon reuse. Your application will need to store this initialization vector along with the encrypted data, as it is required for decryption. +Additionally, for ciphers that require an initialization vector, you must generate a proper-length initialization vector to pass to [`crypto.createCipheriv()`][]. Initialization vectors must never be re-used, especially in modes such as AES-CTR, where encryption is effectively removed upon reuse. Your application will need to store this initialization vector along with the encrypted data, as it is required for decryption. If an initialization vector is not needed by the cipher, you may pass `null` or omit the argument. diff --git a/lib/crypto.js b/lib/crypto.js index 098a7fbb491efc..bc0317c96e8660 100644 --- a/lib/crypto.js +++ b/lib/crypto.js @@ -131,6 +131,18 @@ function getDecoder(decoder, encoding) { return decoder; } +exports.generateLegacyKey = generateLegacyKey; + +function generateLegacyKey(cipher, key) { + return binding.generateLegacyKey(cipher, key); +} + +exports.generateLegacyIV = generateLegacyIV; + +function generateLegacyIV(cipher, key) { + return binding.generateLegacyIV(cipher, key); +} + Object.defineProperty(exports, 'createCipher', { configurable: true, enumerable: true, diff --git a/src/node_crypto.cc b/src/node_crypto.cc index e322a2ace6e9cc..35f5abafa7ccef 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -3293,6 +3293,82 @@ void Connection::SetSNICallback(const FunctionCallbackInfo& args) { } #endif +void GenerateLegacyKey(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + + if (args.Length() != 2) { + return env->ThrowError("Cipher and key must be supplied."); + } + + THROW_AND_RETURN_IF_NOT_STRING(args[0], "Cipher"); + THROW_AND_RETURN_IF_NOT_STRING(args[1], "Key"); + + const node::Utf8Value cipher_type(env->isolate(), args[0]); + const node::Utf8Value key_value(env->isolate(), args[1]); + + const EVP_CIPHER * cipher = EVP_get_cipherbyname(*cipher_type); + if (cipher == nullptr) { + return env->ThrowError("Unknown cipher"); + } + + const char* key_buf = *key_value; + ssize_t key_buf_len = key_value.length(); + + unsigned char * key = (unsigned char *) node::Malloc(EVP_MAX_KEY_LENGTH); + + int key_len = EVP_BytesToKey(cipher, + EVP_md5(), + nullptr, + reinterpret_cast(key_buf), + key_buf_len, + 1, + key, + nullptr); + + Local buf = Buffer::New(env, (char *) key, (unsigned) key_len).ToLocalChecked(); + args.GetReturnValue().Set(buf); +} + +void GenerateLegacyIV(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + + if (args.Length() != 2) { + return env->ThrowError("Cipher and key must be supplied."); + } + + THROW_AND_RETURN_IF_NOT_STRING(args[0], "Cipher"); + THROW_AND_RETURN_IF_NOT_STRING(args[1], "Key"); + + const node::Utf8Value cipher_type(env->isolate(), args[0]); + const node::Utf8Value key_value(env->isolate(), args[1]); + + const EVP_CIPHER * cipher = EVP_get_cipherbyname(*cipher_type); + if (cipher == nullptr) { + return env->ThrowError("Unknown cipher"); + } + + const int expected_iv_len = EVP_CIPHER_iv_length(cipher); + if (expected_iv_len == 0) { + return env->ThrowError("Cipher has no IV"); + } + + const char* key_buf = *key_value; + ssize_t key_buf_len = key_value.length(); + + unsigned char * iv = (unsigned char *) node::Malloc(EVP_MAX_IV_LENGTH); + + EVP_BytesToKey(cipher, + EVP_md5(), + nullptr, + reinterpret_cast(key_buf), + key_buf_len, + 1, + nullptr, + iv); + + Local buf = Buffer::New(env, (char *) iv, (unsigned) expected_iv_len).ToLocalChecked(); + args.GetReturnValue().Set(buf); +} void CipherBase::Initialize(Environment* env, Local target) { Local t = env->NewFunctionTemplate(New); @@ -6246,6 +6322,8 @@ void InitCrypto(Local target, env->SetMethod(target, "certVerifySpkac", VerifySpkac); env->SetMethod(target, "certExportPublicKey", ExportPublicKey); env->SetMethod(target, "certExportChallenge", ExportChallenge); + env->SetMethod(target, "generateLegacyKey", GenerateLegacyKey); + env->SetMethod(target, "generateLegacyIV", GenerateLegacyIV); #ifndef OPENSSL_NO_ENGINE env->SetMethod(target, "setEngine", SetEngine); #endif // !OPENSSL_NO_ENGINE From 7f913ec6506aa7dbbff6d074b26fa76292ce07cd Mon Sep 17 00:00:00 2001 From: Ian Carroll Date: Sun, 9 Jul 2017 18:28:45 -0400 Subject: [PATCH 05/13] crypto: update tests for createCipher --- benchmark/crypto/cipher-stream.js | 7 ++- lib/crypto.js | 2 +- src/node_crypto.cc | 27 +++++--- test/parallel/test-crypto-authenticated.js | 5 +- test/parallel/test-crypto-binary-default.js | 16 +++-- test/parallel/test-crypto-cipher-decipher.js | 65 +++++++++++++------- test/parallel/test-crypto-deprecated.js | 16 ++++- test/parallel/test-crypto.js | 2 +- 8 files changed, 100 insertions(+), 40 deletions(-) diff --git a/benchmark/crypto/cipher-stream.js b/benchmark/crypto/cipher-stream.js index 9fd88f1d864dff..a51943912184c0 100644 --- a/benchmark/crypto/cipher-stream.js +++ b/benchmark/crypto/cipher-stream.js @@ -33,8 +33,11 @@ function main(conf) { // alice_secret and bob_secret should be the same assert(alice_secret === bob_secret); - var alice_cipher = crypto.createCipher(conf.cipher, alice_secret); - var bob_cipher = crypto.createDecipher(conf.cipher, bob_secret); + const key = crypto.generateLegacyKey(conf.cipher, alice_secret); + const iv = crypto.generateLegacyIV(conf.cipher, alice_secret); + + var alice_cipher = crypto.createCipheriv(conf.cipher, key, iv); + var bob_cipher = crypto.createDecipheriv(conf.cipher, key, iv); var message; var encoding; diff --git a/lib/crypto.js b/lib/crypto.js index bc0317c96e8660..e11be2ed6a9b00 100644 --- a/lib/crypto.js +++ b/lib/crypto.js @@ -164,7 +164,7 @@ Object.defineProperty(exports, 'Cipher', { function Cipher(cipher, password, options) { if (!(this instanceof Cipher)) return new Cipher(cipher, password, options); - + this._handle = new binding.CipherBase(true); this._handle.init(cipher, toBuf(password)); diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 35f5abafa7ccef..dd2a788423b530 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -3301,7 +3301,7 @@ void GenerateLegacyKey(const FunctionCallbackInfo& args) { } THROW_AND_RETURN_IF_NOT_STRING(args[0], "Cipher"); - THROW_AND_RETURN_IF_NOT_STRING(args[1], "Key"); + THROW_AND_RETURN_IF_NOT_STRING_OR_BUFFER(args[1], "Key"); const node::Utf8Value cipher_type(env->isolate(), args[0]); const node::Utf8Value key_value(env->isolate(), args[1]); @@ -3324,8 +3324,12 @@ void GenerateLegacyKey(const FunctionCallbackInfo& args) { 1, key, nullptr); - - Local buf = Buffer::New(env, (char *) key, (unsigned) key_len).ToLocalChecked(); + + Local buf = Buffer::New(env, + reinterpret_cast(key), + (unsigned) key_len) + .ToLocalChecked(); + args.GetReturnValue().Set(buf); } @@ -3337,7 +3341,7 @@ void GenerateLegacyIV(const FunctionCallbackInfo& args) { } THROW_AND_RETURN_IF_NOT_STRING(args[0], "Cipher"); - THROW_AND_RETURN_IF_NOT_STRING(args[1], "Key"); + THROW_AND_RETURN_IF_NOT_STRING_OR_BUFFER(args[1], "Key"); const node::Utf8Value cipher_type(env->isolate(), args[0]); const node::Utf8Value key_value(env->isolate(), args[1]); @@ -3366,7 +3370,11 @@ void GenerateLegacyIV(const FunctionCallbackInfo& args) { nullptr, iv); - Local buf = Buffer::New(env, (char *) iv, (unsigned) expected_iv_len).ToLocalChecked(); + Local buf = Buffer::New(env, + reinterpret_cast(iv), + (unsigned) expected_iv_len) + .ToLocalChecked(); + args.GetReturnValue().Set(buf); } @@ -3416,6 +3424,7 @@ void CipherBase::Init(const char* cipher_type, } unsigned char key[EVP_MAX_KEY_LENGTH]; + unsigned char iv[EVP_MAX_KEY_LENGTH]; int key_len = EVP_BytesToKey(cipher_, EVP_md5(), @@ -3424,7 +3433,7 @@ void CipherBase::Init(const char* cipher_type, key_buf_len, 1, key, - nullptr); + iv); EVP_CIPHER_CTX_init(&ctx_); const bool encrypt = (kind_ == kCipher); @@ -3434,7 +3443,9 @@ void CipherBase::Init(const char* cipher_type, const int iv_len = EVP_CIPHER_iv_length(cipher_); if (encrypt && iv_len != 0) { - return env()->ThrowError("crypto.createCipher() is no longer supported with ciphers that require initialization vectors. Generate a random IV and pass it to crypto.createCipheriv()."); + return env()->ThrowError("crypto.createCipher() is no longer supported" + " with ciphers that require initialization vectors. " + " Generate a random IV and pass it to crypto.createCipheriv()."); } if (!EVP_CIPHER_CTX_set_key_length(&ctx_, key_len)) { @@ -3446,7 +3457,7 @@ void CipherBase::Init(const char* cipher_type, nullptr, nullptr, reinterpret_cast(key), - nullptr, + reinterpret_cast(iv), kind_ == kCipher); initialised_ = true; } diff --git a/test/parallel/test-crypto-authenticated.js b/test/parallel/test-crypto-authenticated.js index 2d5370ba3eac2b..8948203f9f0ce0 100644 --- a/test/parallel/test-crypto-authenticated.js +++ b/test/parallel/test-crypto-authenticated.js @@ -332,6 +332,7 @@ const errMessages = { auth: / auth/, state: / state/, FIPS: /not supported in FIPS mode/, + IV: /no longer supported with ciphers that require initialization vectors/, length: /Invalid IV length/, }; @@ -390,9 +391,9 @@ for (const i in TEST_CASES) { } if (test.password) { - if (common.hasFipsCrypto) { + if (!test.algo.endsWith('ecb')) { assert.throws(() => { crypto.createCipher(test.algo, test.password); }, - errMessages.FIPS); + errMessages.IV); } else { const encrypt = crypto.createCipher(test.algo, test.password); if (test.aad) diff --git a/test/parallel/test-crypto-binary-default.js b/test/parallel/test-crypto-binary-default.js index e92f70035bde78..6c98daebbd1c20 100644 --- a/test/parallel/test-crypto-binary-default.js +++ b/test/parallel/test-crypto-binary-default.js @@ -456,9 +456,12 @@ assert.strictEqual(s3Verified, true, 'sign and verify (buffer)'); function testCipher1(key) { + const derived_key = crypto.generateLegacyKey('aes-192-cbc', key); + const iv = crypto.generateLegacyIV('aes-192-cbc', key); + // Test encryption and decryption const plaintext = 'Keep this a secret? No! Tell everyone about node.js!'; - const cipher = crypto.createCipher('aes192', key); + const cipher = crypto.createCipheriv('aes-192-cbc', derived_key, iv); // encrypt plaintext which is in utf8 format // to a ciphertext which will be in hex @@ -466,7 +469,8 @@ function testCipher1(key) { // Only use binary or hex, not base64. ciph += cipher.final('hex'); - const decipher = crypto.createDecipher('aes192', key); + const decipher = crypto.createDecipheriv('aes-192-cbc', derived_key, iv); + let txt = decipher.update(ciph, 'hex', 'utf8'); txt += decipher.final('utf8'); @@ -481,14 +485,18 @@ function testCipher2(key) { '32|RmVZZkFUVmpRRkp0TmJaUm56ZU9qcnJkaXNNWVNpTTU*|iXmckfRWZBGWWELw' + 'eCBsThSsfUHLeRe0KCsK8ooHgxie0zOINpXxfZi/oNG7uq9JWFVCk70gfzQH8ZUJ' + 'jAfaFg**'; - const cipher = crypto.createCipher('aes256', key); + + const derived_key = crypto.generateLegacyKey('aes-256-cbc', key); + const iv = crypto.generateLegacyIV('aes-256-cbc', key); + + const cipher = crypto.createCipheriv('aes-256-cbc', derived_key, iv); // encrypt plaintext which is in utf8 format // to a ciphertext which will be in Base64 let ciph = cipher.update(plaintext, 'utf8', 'base64'); ciph += cipher.final('base64'); - const decipher = crypto.createDecipher('aes256', key); + const decipher = crypto.createDecipheriv('aes-256-cbc', derived_key, iv); let txt = decipher.update(ciph, 'base64', 'utf8'); txt += decipher.final('utf8'); diff --git a/test/parallel/test-crypto-cipher-decipher.js b/test/parallel/test-crypto-cipher-decipher.js index dfb68a7f920844..5c9faa6b398bc0 100644 --- a/test/parallel/test-crypto-cipher-decipher.js +++ b/test/parallel/test-crypto-cipher-decipher.js @@ -13,9 +13,13 @@ const crypto = require('crypto'); const assert = require('assert'); function testCipher1(key) { + key = crypto.generateLegacyKey('aes-192-cbc', key); + const iv = crypto.generateLegacyIV('aes-192-cbc', key); + // Test encryption and decryption const plaintext = 'Keep this a secret? No! Tell everyone about node.js!'; - const cipher = crypto.createCipher('aes192', key); + + const cipher = crypto.createCipheriv('aes-192-cbc', key, iv); // encrypt plaintext which is in utf8 format // to a ciphertext which will be in hex @@ -23,7 +27,7 @@ function testCipher1(key) { // Only use binary or hex, not base64. ciph += cipher.final('hex'); - const decipher = crypto.createDecipher('aes192', key); + const decipher = crypto.createDecipheriv('aes-192-cbc', key, iv); let txt = decipher.update(ciph, 'hex', 'utf8'); txt += decipher.final('utf8'); @@ -33,11 +37,11 @@ function testCipher1(key) { // NB: In real life, it's not guaranteed that you can get all of it // in a single read() like this. But in this case, we know it's // quite small, so there's no harm. - const cStream = crypto.createCipher('aes192', key); + const cStream = crypto.createCipheriv('aes-192-cbc', key, iv); cStream.end(plaintext); ciph = cStream.read(); - const dStream = crypto.createDecipher('aes192', key); + const dStream = crypto.createDecipheriv('aes-192-cbc', key, iv); dStream.end(ciph); txt = dStream.read().toString('utf8'); @@ -48,18 +52,21 @@ function testCipher1(key) { function testCipher2(key) { // encryption and decryption with Base64 // reported in https://github.com/joyent/node/issues/738 + key = crypto.generateLegacyKey('aes-256-cbc', key); + const iv = crypto.generateLegacyIV('aes-256-cbc', key); + const plaintext = '32|RmVZZkFUVmpRRkp0TmJaUm56ZU9qcnJkaXNNWVNpTTU*|iXmckfRWZBGWWELw' + 'eCBsThSsfUHLeRe0KCsK8ooHgxie0zOINpXxfZi/oNG7uq9JWFVCk70gfzQH8ZUJ' + 'jAfaFg**'; - const cipher = crypto.createCipher('aes256', key); + const cipher = crypto.createCipheriv('aes-256-cbc', key, iv); // encrypt plaintext which is in utf8 format // to a ciphertext which will be in Base64 let ciph = cipher.update(plaintext, 'utf8', 'base64'); ciph += cipher.final('base64'); - const decipher = crypto.createDecipher('aes256', key); + const decipher = crypto.createDecipheriv('aes-256-cbc', key, iv); let txt = decipher.update(ciph, 'base64', 'utf8'); txt += decipher.final('utf8'); @@ -74,7 +81,10 @@ testCipher2(Buffer.from('0123456789abcdef')); // Base64 padding regression test, see #4837. { - const c = crypto.createCipher('aes-256-cbc', 'secret'); + const key = crypto.generateLegacyKey('aes-256-cbc', 'secret'); + const iv = crypto.generateLegacyIV('aes-256-cbc', 'secret'); + + const c = crypto.createCipheriv('aes-256-cbc', key, iv); const s = c.update('test', 'utf8', 'base64') + c.final('base64'); assert.strictEqual(s, '375oxUQCIocvxmC5At+rvA=='); } @@ -82,11 +92,14 @@ testCipher2(Buffer.from('0123456789abcdef')); // Calling Cipher.final() or Decipher.final() twice should error but // not assert. See #4886. { - const c = crypto.createCipher('aes-256-cbc', 'secret'); + const key = crypto.generateLegacyKey('aes-256-cbc', 'secret'); + const iv = crypto.generateLegacyIV('aes-256-cbc', 'secret'); + + const c = crypto.createCipheriv('aes-256-cbc', key, iv); try { c.final('xxx'); } catch (e) { /* Ignore. */ } try { c.final('xxx'); } catch (e) { /* Ignore. */ } try { c.final('xxx'); } catch (e) { /* Ignore. */ } - const d = crypto.createDecipher('aes-256-cbc', 'secret'); + const d = crypto.createDecipheriv('aes-256-cbc', key, iv); try { d.final('xxx'); } catch (e) { /* Ignore. */ } try { d.final('xxx'); } catch (e) { /* Ignore. */ } try { d.final('xxx'); } catch (e) { /* Ignore. */ } @@ -94,47 +107,55 @@ testCipher2(Buffer.from('0123456789abcdef')); // Regression test for #5482: string to Cipher#update() should not assert. { - const c = crypto.createCipher('aes192', '0123456789abcdef'); + const key = crypto.generateLegacyKey('aes-192-cbc', '0123456789abcdef'); + const iv = crypto.generateLegacyIV('aes-192-cbc', '0123456789abcdef'); + + const c = crypto.createCipheriv('aes-192-cbc', key, iv); c.update('update'); c.final(); } // #5655 regression tests, 'utf-8' and 'utf8' are identical. { - let c = crypto.createCipher('aes192', '0123456789abcdef'); + const key = crypto.generateLegacyKey('aes-192-cbc', '0123456789abcdef'); + const iv = crypto.generateLegacyIV('aes-192-cbc', '0123456789abcdef'); + + let c = crypto.createCipheriv('aes-192-cbc', key, iv); c.update('update', ''); // Defaults to "utf8". c.final('utf-8'); // Should not throw. - c = crypto.createCipher('aes192', '0123456789abcdef'); + c = crypto.createCipheriv('aes-192-cbc', key, iv); c.update('update', 'utf8'); c.final('utf-8'); // Should not throw. - c = crypto.createCipher('aes192', '0123456789abcdef'); + c = crypto.createCipheriv('aes-192-cbc', key, iv); c.update('update', 'utf-8'); c.final('utf8'); // Should not throw. } // Regression tests for https://github.com/nodejs/node/issues/8236. { - const key = '0123456789abcdef'; + const key = crypto.generateLegacyKey('aes-192-cbc', '0123456789abcdef'); + const iv = crypto.generateLegacyIV('aes-192-cbc', '0123456789abcdef'); + const plaintext = 'Top secret!!!'; - const c = crypto.createCipher('aes192', key); + const c = crypto.createCipheriv('aes-192-cbc', key, iv); let ciph = c.update(plaintext, 'utf16le', 'base64'); ciph += c.final('base64'); - let decipher = crypto.createDecipher('aes192', key); + let decipher = crypto.createDecipheriv('aes-192-cbc', key, iv); let txt; assert.doesNotThrow(() => txt = decipher.update(ciph, 'base64', 'ucs2')); assert.doesNotThrow(() => txt += decipher.final('ucs2')); assert.strictEqual(txt, plaintext, 'decrypted result in ucs2'); - decipher = crypto.createDecipher('aes192', key); + decipher = crypto.createDecipheriv('aes-192-cbc', key, iv); assert.doesNotThrow(() => txt = decipher.update(ciph, 'base64', 'ucs-2')); assert.doesNotThrow(() => txt += decipher.final('ucs-2')); assert.strictEqual(txt, plaintext, 'decrypted result in ucs-2'); - decipher = crypto.createDecipher('aes192', key); + decipher = crypto.createDecipheriv('aes-192-cbc', key, iv); assert.doesNotThrow(() => txt = decipher.update(ciph, 'base64', 'utf-16le')); assert.doesNotThrow(() => txt += decipher.final('utf-16le')); assert.strictEqual(txt, plaintext, 'decrypted result in utf-16le'); @@ -153,11 +174,13 @@ testCipher2(Buffer.from('0123456789abcdef')); // error throwing in setAAD/setAuthTag/getAuthTag/setAutoPadding { - const key = '0123456789'; + const key = crypto.generateLegacyKey('aes-256-gcm', '0123456789'); + const iv = crypto.generateLegacyIV('aes-256-gcm', '0123456789'); + const aadbuf = Buffer.from('aadbuf'); const data = Buffer.from('test-crypto-cipher-decipher'); - const cipher = crypto.createCipher('aes-256-gcm', key); + const cipher = crypto.createCipheriv('aes-256-gcm', key, iv); cipher.setAAD(aadbuf); cipher.setAutoPadding(); @@ -167,7 +190,7 @@ testCipher2(Buffer.from('0123456789abcdef')); const encrypted = Buffer.concat([cipher.update(data), cipher.final()]); - const decipher = crypto.createDecipher('aes-256-gcm', key); + const decipher = crypto.createDecipheriv('aes-256-gcm', key, iv); decipher.setAAD(aadbuf); decipher.setAuthTag(cipher.getAuthTag()); decipher.setAutoPadding(); diff --git a/test/parallel/test-crypto-deprecated.js b/test/parallel/test-crypto-deprecated.js index 903862d6c8ed27..f3ae3d87dd9ec8 100644 --- a/test/parallel/test-crypto-deprecated.js +++ b/test/parallel/test-crypto-deprecated.js @@ -11,7 +11,10 @@ const tls = require('tls'); common.expectWarning('DeprecationWarning', [ 'crypto.Credentials is deprecated. Use tls.SecureContext instead.', - 'crypto.createCredentials is deprecated. Use tls.createSecureContext instead.' + 'crypto.createCredentials is deprecated. ' + + 'Use tls.createSecureContext instead.', + 'crypto.createCipher is deprecated. Use crypto.createCipheriv instead.', + 'crypto.Cipher is deprecated. Use crypto.createCipheriv instead.', ]); // Accessing the deprecated function is enough to trigger the warning event. @@ -20,3 +23,14 @@ common.expectWarning('DeprecationWarning', [ // mapped to the correct non-deprecated function. assert.strictEqual(crypto.Credentials, tls.SecureContext); assert.strictEqual(crypto.createCredentials, tls.createSecureContext); + +crypto.createCipher; +crypto.Cipher; + +assert.throws(() => { + crypto.createCipher('aes-128-cbc', 'this-should-throw'); +}, /no longer supported with ciphers that require initialization vectors/); + +assert.doesNotThrow(() => { + crypto.createCipher('aes-128-ecb', 'this-should-warn'); +}); diff --git a/test/parallel/test-crypto.js b/test/parallel/test-crypto.js index a7638d3cd899a1..e094d7475ea7a3 100644 --- a/test/parallel/test-crypto.js +++ b/test/parallel/test-crypto.js @@ -137,7 +137,7 @@ testImmutability(crypto.getCurves); // throw, not assert in C++ land. assert.throws(function() { crypto.createCipher('aes192', 'test').update('0', 'hex'); -}, common.hasFipsCrypto ? /not supported in FIPS mode/ : /Bad input string/); +}, /no longer supported with ciphers that require initialization vectors/); assert.throws(function() { crypto.createDecipher('aes192', 'test').update('0', 'hex'); From b7abe64573c2240ed1155318f23137343871d367 Mon Sep 17 00:00:00 2001 From: Ian Carroll Date: Sun, 9 Jul 2017 18:32:30 -0400 Subject: [PATCH 06/13] crypto: fix IV buffer length --- src/node_crypto.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index dd2a788423b530..97296d1c44419b 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -3424,7 +3424,7 @@ void CipherBase::Init(const char* cipher_type, } unsigned char key[EVP_MAX_KEY_LENGTH]; - unsigned char iv[EVP_MAX_KEY_LENGTH]; + unsigned char iv[EVP_MAX_IV_LENGTH]; int key_len = EVP_BytesToKey(cipher_, EVP_md5(), From b16018ac4b339122155bdf951600e4e491640655 Mon Sep 17 00:00:00 2001 From: Ian Carroll Date: Sun, 9 Jul 2017 18:33:20 -0400 Subject: [PATCH 07/13] crypto: fix removal error --- src/node_crypto.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 97296d1c44419b..dcd4300b61a684 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -3445,7 +3445,7 @@ void CipherBase::Init(const char* cipher_type, if (encrypt && iv_len != 0) { return env()->ThrowError("crypto.createCipher() is no longer supported" " with ciphers that require initialization vectors. " - " Generate a random IV and pass it to crypto.createCipheriv()."); + "Generate a random IV and pass it to crypto.createCipheriv()."); } if (!EVP_CIPHER_CTX_set_key_length(&ctx_, key_len)) { From 39a2112a40eb259f904566ac952a73c72d01bea8 Mon Sep 17 00:00:00 2001 From: Ian Carroll Date: Fri, 29 Sep 2017 09:15:28 -0400 Subject: [PATCH 08/13] crypto: Fix createCipheriv with null IVs and FIPS mode. --- lib/crypto.js | 4 ++-- src/node_crypto.cc | 5 +---- test/parallel/test-crypto-authenticated.js | 2 +- test/parallel/test-crypto-deprecated.js | 11 +++++++---- test/parallel/test-crypto-ecb.js | 8 +++----- test/parallel/test-crypto.js | 6 +++++- 6 files changed, 19 insertions(+), 17 deletions(-) diff --git a/lib/crypto.js b/lib/crypto.js index e11be2ed6a9b00..a4499249094e7b 100644 --- a/lib/crypto.js +++ b/lib/crypto.js @@ -243,7 +243,7 @@ function Cipheriv(cipher, key, iv, options) { if (!(this instanceof Cipheriv)) return new Cipheriv(cipher, key, iv, options); this._handle = new binding.CipherBase(true); - this._handle.initiv(cipher, toBuf(key), iv ? toBuf(iv) : null); + this._handle.initiv(cipher, toBuf(key), iv ? toBuf(iv) : Buffer([])); this._decoder = null; LazyTransform.call(this, options); @@ -291,7 +291,7 @@ function Decipheriv(cipher, key, iv, options) { return new Decipheriv(cipher, key, iv, options); this._handle = new binding.CipherBase(false); - this._handle.initiv(cipher, toBuf(key), toBuf(iv)); + this._handle.initiv(cipher, toBuf(key), iv ? toBuf(iv) : Buffer([])); this._decoder = null; LazyTransform.call(this, options); diff --git a/src/node_crypto.cc b/src/node_crypto.cc index dcd4300b61a684..d3d4d5ce41e206 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -3537,10 +3537,7 @@ void CipherBase::InitIv(const FunctionCallbackInfo& args) { THROW_AND_RETURN_IF_NOT_STRING(args[0], "Cipher type"); THROW_AND_RETURN_IF_NOT_BUFFER(args[1], "Key"); - - if (!Buffer::HasInstance(args[2]) && !args[2]->IsNull()) { - return env->ThrowTypeError("IV must be a buffer or null"); - } + THROW_AND_RETURN_IF_NOT_BUFFER(args[2], "IV"); const node::Utf8Value cipher_type(env->isolate(), args[0]); ssize_t key_len = Buffer::Length(args[1]); diff --git a/test/parallel/test-crypto-authenticated.js b/test/parallel/test-crypto-authenticated.js index 8948203f9f0ce0..68b2a26525b1ad 100644 --- a/test/parallel/test-crypto-authenticated.js +++ b/test/parallel/test-crypto-authenticated.js @@ -393,7 +393,7 @@ for (const i in TEST_CASES) { if (test.password) { if (!test.algo.endsWith('ecb')) { assert.throws(() => { crypto.createCipher(test.algo, test.password); }, - errMessages.IV); + common.hasFipsCrypto ? errMessages.FIPS : errMessages.IV); } else { const encrypt = crypto.createCipher(test.algo, test.password); if (test.aad) diff --git a/test/parallel/test-crypto-deprecated.js b/test/parallel/test-crypto-deprecated.js index f3ae3d87dd9ec8..4c23639b249d89 100644 --- a/test/parallel/test-crypto-deprecated.js +++ b/test/parallel/test-crypto-deprecated.js @@ -29,8 +29,11 @@ crypto.Cipher; assert.throws(() => { crypto.createCipher('aes-128-cbc', 'this-should-throw'); -}, /no longer supported with ciphers that require initialization vectors/); +}, common.hasFipsCrypto ? /not supported in FIPS mode/ : /no longer supported with ciphers that require initialization vectors/); -assert.doesNotThrow(() => { - crypto.createCipher('aes-128-ecb', 'this-should-warn'); -}); + +if (!common.hasFipsCrypto) { + assert.doesNotThrow(() => { + crypto.createCipher('aes-128-ecb', 'this-should-warn'); + }); +} \ No newline at end of file diff --git a/test/parallel/test-crypto-ecb.js b/test/parallel/test-crypto-ecb.js index d69cb9ea0d2754..db35bf81c0bed6 100644 --- a/test/parallel/test-crypto-ecb.js +++ b/test/parallel/test-crypto-ecb.js @@ -39,16 +39,14 @@ crypto.DEFAULT_ENCODING = 'buffer'; // Reference: bug#1997 { - const encrypt = - crypto.createCipheriv('BF-ECB', 'SomeRandomBlahz0c5GZVnR', ''); + const encrypt = crypto.createCipheriv('BF-ECB', 'SomeRandomBlahz0c5GZVnR'); let hex = encrypt.update('Hello World!', 'ascii', 'hex'); hex += encrypt.final('hex'); assert.strictEqual(hex.toUpperCase(), '6D385F424AAB0CFBF0BB86E07FFB7D71'); } -{ - const decrypt = - crypto.createDecipheriv('BF-ECB', 'SomeRandomBlahz0c5GZVnR', ''); +{ + const decrypt = crypto.createDecipheriv('BF-ECB', 'SomeRandomBlahz0c5GZVnR'); let msg = decrypt.update('6D385F424AAB0CFBF0BB86E07FFB7D71', 'hex', 'ascii'); msg += decrypt.final('ascii'); assert.strictEqual(msg, 'Hello World!'); diff --git a/test/parallel/test-crypto.js b/test/parallel/test-crypto.js index e094d7475ea7a3..735a704a469872 100644 --- a/test/parallel/test-crypto.js +++ b/test/parallel/test-crypto.js @@ -135,9 +135,13 @@ testImmutability(crypto.getCurves); // Regression tests for #5725: hex input that's not a power of two should // throw, not assert in C++ land. +if (common.hasFipsCrypto) { + +} + assert.throws(function() { crypto.createCipher('aes192', 'test').update('0', 'hex'); -}, /no longer supported with ciphers that require initialization vectors/); +}, common.hasFipsCrypto ? /not supported in FIPS mode/ : /no longer supported with ciphers that require initialization vectors/); assert.throws(function() { crypto.createDecipher('aes192', 'test').update('0', 'hex'); From d0f2778cd354a5afa5a25d3239e38eacb5057ce6 Mon Sep 17 00:00:00 2001 From: Ian Carroll Date: Fri, 29 Sep 2017 09:27:27 -0400 Subject: [PATCH 09/13] crypto: fix style issues --- lib/crypto.js | 4 ++-- test/parallel/test-crypto-deprecated.js | 2 +- test/parallel/test-crypto-ecb.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/crypto.js b/lib/crypto.js index a4499249094e7b..6a71dcbbcdde20 100644 --- a/lib/crypto.js +++ b/lib/crypto.js @@ -243,7 +243,7 @@ function Cipheriv(cipher, key, iv, options) { if (!(this instanceof Cipheriv)) return new Cipheriv(cipher, key, iv, options); this._handle = new binding.CipherBase(true); - this._handle.initiv(cipher, toBuf(key), iv ? toBuf(iv) : Buffer([])); + this._handle.initiv(cipher, toBuf(key), iv ? toBuf(iv) : Buffer.alloc(0)); this._decoder = null; LazyTransform.call(this, options); @@ -291,7 +291,7 @@ function Decipheriv(cipher, key, iv, options) { return new Decipheriv(cipher, key, iv, options); this._handle = new binding.CipherBase(false); - this._handle.initiv(cipher, toBuf(key), iv ? toBuf(iv) : Buffer([])); + this._handle.initiv(cipher, toBuf(key), iv ? toBuf(iv) : Buffer.alloc(0)); this._decoder = null; LazyTransform.call(this, options); diff --git a/test/parallel/test-crypto-deprecated.js b/test/parallel/test-crypto-deprecated.js index 4c23639b249d89..d042be43938e90 100644 --- a/test/parallel/test-crypto-deprecated.js +++ b/test/parallel/test-crypto-deprecated.js @@ -36,4 +36,4 @@ if (!common.hasFipsCrypto) { assert.doesNotThrow(() => { crypto.createCipher('aes-128-ecb', 'this-should-warn'); }); -} \ No newline at end of file +} diff --git a/test/parallel/test-crypto-ecb.js b/test/parallel/test-crypto-ecb.js index db35bf81c0bed6..847d603e28e1c8 100644 --- a/test/parallel/test-crypto-ecb.js +++ b/test/parallel/test-crypto-ecb.js @@ -45,7 +45,7 @@ crypto.DEFAULT_ENCODING = 'buffer'; assert.strictEqual(hex.toUpperCase(), '6D385F424AAB0CFBF0BB86E07FFB7D71'); } -{ +{ const decrypt = crypto.createDecipheriv('BF-ECB', 'SomeRandomBlahz0c5GZVnR'); let msg = decrypt.update('6D385F424AAB0CFBF0BB86E07FFB7D71', 'hex', 'ascii'); msg += decrypt.final('ascii'); From 14e0566527d463e7127ac0590454ad1ed99ff2fc Mon Sep 17 00:00:00 2001 From: Ian Carroll Date: Fri, 29 Sep 2017 09:49:36 -0400 Subject: [PATCH 10/13] doc: add crypto.createCipher deprecation information --- doc/api/crypto.md | 42 +++++++++++++++++++++++++++++++++++++++++ doc/api/deprecations.md | 33 ++++++++++++++++++++++++-------- 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/doc/api/crypto.md b/doc/api/crypto.md index c4a1cad106c3d6..69a575651dfc99 100644 --- a/doc/api/crypto.md +++ b/doc/api/crypto.md @@ -1171,7 +1171,11 @@ currently in use. Setting to true requires a FIPS build of Node.js. ### crypto.createCipher(algorithm, password) + +> Stability: 0 - Deprecated: Use [`crypto.createCipheriv()`][] instead. + - `algorithm` {string} - `password` {string | Buffer | TypedArray | DataView} @@ -1214,6 +1218,44 @@ The `key` is the raw key used by the `algorithm` and `iv` is an [initialization vector][]. Both arguments must be `'utf8'` encoded strings, [Buffers][`Buffer`], `TypedArray`, or `DataView`s. +### crypto.generateLegacyKey(algorithm, key) +- `algorithm` {string} +- `key` {string | Buffer | TypedArray | DataView} + +Creates and returns a [Buffer][`Buffer`] object, with the given `algorithm` and +`key`. + +Use this function for applications previously reliant on +[`crypto.createCipher()`][]. Pass its return value to +[`crypto.createCipheriv()`][] as the `key`. + +The `algorithm` is dependent on OpenSSL, examples are `'aes192'`, etc. On +recent OpenSSL releases, `openssl list-cipher-algorithms` will display the +available cipher algorithms. + +The `key` must be a `'utf8'` encoded string, [Buffer][`Buffer`], `TypedArray`, +or `DataView`. + +### crypto.generateLegacyIV(algorithm, iv) +- `algorithm` {string} +- `iv` {string | Buffer | TypedArray | DataView} + +Creates and returns a [Buffer][`Buffer`] object, with the given `algorithm` and +`iv`. + +Use this function for applications previously reliant on +[`crypto.createCipher()`][]. Pass its return value to +[`crypto.createCipheriv()`][] as the `iv`. + +The `algorithm` is dependent on OpenSSL, examples are `'aes192'`, etc. On +recent OpenSSL releases, `openssl list-cipher-algorithms` will display the +available cipher algorithms. + +This function will throw an error if a cipher without an IV is passed. + +The `iv` must be a `'utf8'` encoded string, [Buffer][`Buffer`], `TypedArray`, +or `DataView`. + ### crypto.createCredentials(details)