crypto: allow passing null as IV unless required

tniessen · tniessen · commit 0a776f353265 · 2018-03-14T14:37:03.000+01:00
PR-URL: nodejs#18644 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/doc/api/crypto.md b/doc/api/crypto.md
@@ -1273,6 +1273,11 @@ Adversaries][] for details.
 ### crypto.createCipheriv(algorithm, key, iv[, options])
 <!-- YAML
 added: v0.1.94
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/18644
+    description: The `iv` parameter may now be `null` for ciphers which do not
+                 need an initialization vector.
 -->
 - `algorithm` {string}
 - `key` {string | Buffer | TypedArray | DataView}
@@ -1288,7 +1293,8 @@ 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,
-[Buffers][`Buffer`], `TypedArray`, or `DataView`s.
+[Buffers][`Buffer`], `TypedArray`, or `DataView`s. If the cipher does not need
+an initialization vector, `iv` may be `null`.
 
 ### crypto.createCredentials(details)
 <!-- YAML
@@ -1334,6 +1340,11 @@ to create the `Decipher` object.
 ### crypto.createDecipheriv(algorithm, key, iv[, options])
 <!-- YAML
 added: v0.1.94
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/18644
+    description: The `iv` parameter may now be `null` for ciphers which do not
+                 need an initialization vector.
 -->
 - `algorithm` {string}
 - `key` {string | Buffer | TypedArray | DataView}
@@ -1350,7 +1361,8 @@ 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,
-[Buffers][`Buffer`], `TypedArray`, or `DataView`s.
+[Buffers][`Buffer`], `TypedArray`, or `DataView`s. If the cipher does not need
+an initialization vector, `iv` may be `null`.
 
 ### crypto.createDiffieHellman(prime[, primeEncoding][, generator][, generatorEncoding])
 <!-- YAML
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -3760,8 +3760,17 @@ void CipherBase::InitIv(const char* cipher_type,
   const int expected_iv_len = EVP_CIPHER_iv_length(cipher);
   const int mode = EVP_CIPHER_mode(cipher);
   const bool is_gcm_mode = (EVP_CIPH_GCM_MODE == mode);
+  const bool has_iv = iv_len >= 0;
 
-  if (is_gcm_mode == false && iv_len != expected_iv_len) {
+  // Throw if no IV was passed and the cipher requires an IV
+  if (!has_iv && expected_iv_len != 0) {
+    char msg[128];
+    snprintf(msg, sizeof(msg), "Missing IV for cipher %s", cipher_type);
+    return env()->ThrowError(msg);
+  }
+
+  // Throw if an IV was passed which does not match the cipher's fixed IV length
+  if (is_gcm_mode == false && has_iv && iv_len != expected_iv_len) {
     return env()->ThrowError("Invalid IV length");
   }
 
@@ -3773,11 +3782,13 @@ void CipherBase::InitIv(const char* cipher_type,
   const bool encrypt = (kind_ == kCipher);
   EVP_CipherInit_ex(ctx_, cipher, nullptr, nullptr, nullptr, encrypt);
 
-  if (is_gcm_mode &&
-      !EVP_CIPHER_CTX_ctrl(ctx_, EVP_CTRL_GCM_SET_IVLEN, iv_len, nullptr)) {
-    EVP_CIPHER_CTX_free(ctx_);
-    ctx_ = nullptr;
-    return env()->ThrowError("Invalid IV length");
+  if (is_gcm_mode) {
+    CHECK(has_iv);
+    if (!EVP_CIPHER_CTX_ctrl(ctx_, EVP_CTRL_GCM_SET_IVLEN, iv_len, nullptr)) {
+      EVP_CIPHER_CTX_free(ctx_);
+      ctx_ = nullptr;
+      return env()->ThrowError("Invalid IV length");
+    }
   }
 
   if (!EVP_CIPHER_CTX_set_key_length(ctx_, key_len)) {
@@ -3806,13 +3817,23 @@ void CipherBase::InitIv(const FunctionCallbackInfo<Value>& 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 (!args[2]->IsNull() && !Buffer::HasInstance(args[2])) {
+    return env->ThrowTypeError("IV must be a buffer");
+  }
 
   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]);
+  ssize_t iv_len;
+  const char* iv_buf;
+  if (args[2]->IsNull()) {
+    iv_buf = nullptr;
+    iv_len = -1;
+  } else {
+    iv_buf = Buffer::Data(args[2]);
+    iv_len = Buffer::Length(args[2]);
+  }
   cipher->InitIv(*cipher_type, key_buf, key_len, iv_buf, iv_len);
 }
 
diff --git a/test/parallel/test-crypto-cipheriv-decipheriv.js b/test/parallel/test-crypto-cipheriv-decipheriv.js
@@ -89,8 +89,9 @@ if (!common.hasFipsCrypto) {
               Buffer.from('A6A6A6A6A6A6A6A6', 'hex'));
 }
 
-// Zero-sized IV should be accepted in ECB mode.
+// Zero-sized IV or null should be accepted in ECB mode.
 crypto.createCipheriv('aes-128-ecb', Buffer.alloc(16), Buffer.alloc(0));
+crypto.createCipheriv('aes-128-ecb', Buffer.alloc(16), null);
 
 const errMessage = /Invalid IV length/;
 
@@ -114,6 +115,11 @@ for (let n = 0; n < 256; n += 1) {
     errMessage);
 }
 
+// And so should null be.
+assert.throws(() => {
+  crypto.createCipheriv('aes-128-cbc', Buffer.alloc(16), null);
+}, /Missing IV for cipher aes-128-cbc/);
+
 // Zero-sized IV should be rejected in GCM mode.
 assert.throws(
   () => crypto.createCipheriv('aes-128-gcm', Buffer.alloc(16),
