crypto: Deprecate createCipher for createCipheriv

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);
+
+  const alice_cipher = crypto.createCipheriv(conf.cipher, key, iv);
+  const bob_cipher = crypto.createDecipheriv(conf.cipher, key, iv);
 
   var message;
   var encoding;

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)
 <!-- YAML
 added: v0.1.94
+deprecated: vx.x.x
 -->
+
+> Stability: 0 - Deprecated: Use [`crypto.createCipheriv()`][] instead.
+
 - `algorithm` {string}
 - `password` {string | Buffer | TypedArray | DataView}
 
@@ -1204,16 +1208,64 @@ to create the `Cipher` object.
 - `iv` {string | Buffer | TypedArray | DataView}
 
 Creates and returns a `Cipher` object, with the given `algorithm`, `key` and
-initialization vector (`iv`).
+initialization vector (`iv`). The initialization vector is optional if the
+algorithm does not use one.
 
 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` is the raw key used by the `algorithm` and `iv` is an
-[initialization vector][]. Both arguments must be `'utf8'` encoded strings,
+The `key` is the raw key used by the `algorithm` and `iv` is an [initialization
+vector][]. When provided, both arguments must be `'utf8'` encoded strings,
 [Buffers][`Buffer`], `TypedArray`, or `DataView`s.
 
+### crypto.generateLegacyKey(algorithm, key)
+- `algorithm` {string}
+- `key` {string | Buffer | TypedArray | DataView}
+
+Generates the symmetric encryption key previously internally generated by
+[`crypto.createCipher()`][] and returns it as a [Buffer][`Buffer`].
+
+The function used to derive this key is a single round of MD5, and is
+**insecure**. Use a proper key derivation function such as [`crypto.pbkdf2()`][]
+with strong parameters. This function requires minimal compute time to generate
+a key from an input, which is typically unsafe.
+
+This function's return value can be passed 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}
+
+Generates the initialization vector previously internally generated by
+[`crypto.createCipher()`][] and returns it as a [Buffer][`Buffer`].
+
+The function used to derive this key is a single round of MD5, and is
+**insecure**. Use a securely random source of entropy such as
+[`crypto.randomBytes()`][] to create the initialization vector. It is almost
+always unsafe to deterministically generate the initialization vector, as this
+function does.
+
+This function's return value can be passed 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)
 <!-- YAML
 added: v0.1.92

doc/api/deprecations.md

@@ -634,6 +634,35 @@ Type: Runtime
 
 *Note*: change was made while `async_hooks` was an experimental API.
 
+<a id="DEP00XX"></a>
+### DEP00XX: crypto.createCipher()
+
+Type: Runtime
+
+[`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
+deprecated, 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()`][].
+
+If the previous key-derivation is required for backward compatiability, the new
+APIs [`crypto.generateLegacyKey()`][] and [`crypto.generateLegacyIV()`][] expose the old algorithms.
+
+Additionally, for ciphers that require an initialization vector, a proper-length
+initialization vector must be passed to [`crypto.createCipheriv()`][].
+Initialization vectors should never be re-used, especially in modes such as
+AES-CTR, where encryption is effectively removed upon reuse. Applications 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, 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 +676,10 @@ 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.generateLegacyKey()`]: crypto.html#crypto_crypto_generatelegacykey_algorithm_key
+[`crypto.generateLegacyIV()`]: crypto.html#crypto_crypto_generatelegacyiv_algorithm_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

lib/crypto.js

@@ -131,11 +131,40 @@ 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,
+  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 +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), toBuf(iv));
+  this._handle.initiv(cipher, toBuf(key), iv ? toBuf(iv) : Buffer.alloc(0));
   this._decoder = null;
 
   LazyTransform.call(this, options);
@@ -262,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.alloc(0));
   this._decoder = null;
 
   LazyTransform.call(this, options);

src/node_crypto.cc

@@ -3293,6 +3293,90 @@ void Connection::SetSNICallback(const FunctionCallbackInfo<Value>& args) {
 }
 #endif
 
+void GenerateLegacyKey(const FunctionCallbackInfo<Value>& 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_OR_BUFFER(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<const unsigned char*>(key_buf),
+                               key_buf_len,
+                               1,
+                               key,
+                               nullptr);
+
+  Local<Object> buf = Buffer::New(env,
+    reinterpret_cast<char *>(key),
+    (unsigned) key_len)
+    .ToLocalChecked();
+
+  args.GetReturnValue().Set(buf);
+}
+
+void GenerateLegacyIV(const FunctionCallbackInfo<Value>& 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_OR_BUFFER(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<const unsigned char*>(key_buf),
+                  key_buf_len,
+                  1,
+                  nullptr,
+                  iv);
+
+  Local<Object> buf = Buffer::New(env,
+    reinterpret_cast<char *>(iv),
+    (unsigned) expected_iv_len)
+    .ToLocalChecked();
+
+  args.GetReturnValue().Set(buf);
+}
 
 void CipherBase::Initialize(Environment* env, Local<Object> target) {
   Local<FunctionTemplate> t = env->NewFunctionTemplate(New);
@@ -3353,7 +3437,11 @@ 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);
+
+  const int iv_len = EVP_CIPHER_iv_length(cipher_);
+
   if (!EVP_CIPHER_CTX_set_key_length(&ctx_, key_len)) {
     EVP_CIPHER_CTX_cleanup(&ctx_);
     return env()->ThrowError("Invalid key length");
@@ -3448,9 +3536,14 @@ void CipherBase::InitIv(const FunctionCallbackInfo<Value>& args) {
   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.Length() < 3 || 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);
+  }
 }
 
 
@@ -6231,6 +6324,8 @@ void InitCrypto(Local<Object> 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

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,7 +391,7 @@ for (const i in TEST_CASES) {
   }
 
   if (test.password) {
-    if (common.hasFipsCrypto) {
+    if (!test.algo.endsWith('ecb') && common.hasFipsCrypto) {
       assert.throws(() => { crypto.createCipher(test.algo, test.password); },
                     errMessages.FIPS);
     } else {

test/parallel/test-crypto-binary-default.js

@@ -456,17 +456,21 @@ 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
   let ciph = cipher.update(plaintext, 'utf8', 'hex');
   // 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');