crypto: deprecate implicitly shortened GCM tags

tniessen · marco-ippolito · commit 03e05b092ce6 · 2024-05-03T08:30:29.000+02:00
This introduces a doc-only deprecation of using GCM authentication tags that are shorter than the cipher's block size, unless the user specified the authTagLength option. Refs: #52327 PR-URL: #52345 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/doc/api/crypto.md b/doc/api/crypto.md
@@ -891,6 +891,11 @@ When passing a string as the `buffer`, please consider
 <!-- YAML
 added: v1.0.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/52345
+    description: Using GCM tag lengths other than 128 bits without specifying
+                 the `authTagLength` option when creating `decipher` is
+                 deprecated.
   - version: v15.0.0
     pr-url: https://github.com/nodejs/node/pull/35093
     description: The buffer argument can be a string or ArrayBuffer and is
diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md
@@ -3507,6 +3507,25 @@ Calling `Hmac` class directly with `Hmac()` or `new Hmac()` is
 deprecated due to being internals, not intended for public use.
 Please use the [`crypto.createHmac()`][] method to create Hmac instances.
 
+### DEP0182: Short GCM authentication tags without explicit `authTagLength`
+
+<!-- YAML
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/52345
+    description: Documentation-only deprecation.
+-->
+
+Type: Documentation-only (supports [`--pending-deprecation`][])
+
+Applications that intend to use authentication tags that are shorter than the
+default authentication tag length should set the `authTagLength` option of the
+[`crypto.createDecipheriv()`][] function to the appropriate length.
+
+For ciphers in GCM mode, the [`decipher.setAuthTag()`][] function accepts
+authentication tags of any valid length (see [DEP0090](#DEP0090)). This behavior
+is deprecated to better align with recommendations per [NIST SP 800-38D][].
+
 [NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf
 [RFC 6066]: https://tools.ietf.org/html/rfc6066#section-3
 [RFC 8247 Section 2.4]: https://www.rfc-editor.org/rfc/rfc8247#section-2.4
diff --git a/src/crypto/crypto_cipher.cc b/src/crypto/crypto_cipher.cc
@@ -707,6 +707,19 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) {
       env, "Invalid authentication tag length: %u", tag_len);
   }
 
+  if (mode == EVP_CIPH_GCM_MODE && cipher->auth_tag_len_ == kNoAuthTagLength &&
+      tag_len != 16 && env->options()->pending_deprecation &&
+      env->EmitProcessEnvWarning()) {
+    if (ProcessEmitDeprecationWarning(
+            env,
+            "Using AES-GCM authentication tags of less than 128 bits without "
+            "specifying the authTagLength option when initializing decryption "
+            "is deprecated.",
+            "DEP0182")
+            .IsNothing())
+      return;
+  }
+
   cipher->auth_tag_len_ = tag_len;
   cipher->auth_tag_state_ = kAuthTagKnown;
   CHECK_LE(cipher->auth_tag_len_, sizeof(cipher->auth_tag_));
diff --git a/test/common/index.js b/test/common/index.js
@@ -654,12 +654,12 @@ function _expectWarning(name, expected, code) {
     expected = [[expected, code]];
   } else if (!Array.isArray(expected)) {
     expected = Object.entries(expected).map(([a, b]) => [b, a]);
-  } else if (!(Array.isArray(expected[0]))) {
+  } else if (expected.length !== 0 && !Array.isArray(expected[0])) {
     expected = [[expected[0], expected[1]]];
   }
   // Deprecation codes are mandatory, everything else is not.
   if (name === 'DeprecationWarning') {
-    expected.forEach(([_, code]) => assert(code, expected));
+    expected.forEach(([_, code]) => assert(code, `Missing deprecation code: ${expected}`));
   }
   return mustCall((warning) => {
     const expectedProperties = expected.shift();
diff --git a/test/parallel/test-crypto-gcm-explicit-short-tag.js b/test/parallel/test-crypto-gcm-explicit-short-tag.js
@@ -0,0 +1,47 @@
+// Flags: --pending-deprecation
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const assert = require('assert');
+const { createDecipheriv, randomBytes } = require('crypto');
+
+common.expectWarning({
+  DeprecationWarning: []
+});
+
+const key = randomBytes(32);
+const iv = randomBytes(16);
+
+{
+  // Full 128-bit tag.
+
+  const tag = randomBytes(16);
+  createDecipheriv('aes-256-gcm', key, iv).setAuthTag(tag);
+}
+
+{
+  // Shortened tag with explicit length option.
+
+  const tag = randomBytes(12);
+  createDecipheriv('aes-256-gcm', key, iv, {
+    authTagLength: tag.byteLength
+  }).setAuthTag(tag);
+}
+
+{
+  // Shortened tag with explicit but incorrect length option.
+
+  const tag = randomBytes(12);
+  assert.throws(() => {
+    createDecipheriv('aes-256-gcm', key, iv, {
+      authTagLength: 14
+    }).setAuthTag(tag);
+  }, {
+    name: 'TypeError',
+    message: 'Invalid authentication tag length: 12',
+    code: 'ERR_CRYPTO_INVALID_AUTH_TAG'
+  });
+}
diff --git a/test/parallel/test-crypto-gcm-implicit-short-tag.js b/test/parallel/test-crypto-gcm-implicit-short-tag.js
@@ -0,0 +1,21 @@
+// Flags: --pending-deprecation
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const { createDecipheriv, randomBytes } = require('crypto');
+
+common.expectWarning({
+  DeprecationWarning: [
+    ['Using AES-GCM authentication tags of less than 128 bits without ' +
+     'specifying the authTagLength option when initializing decryption is ' +
+     'deprecated.',
+     'DEP0182'],
+  ]
+});
+
+const key = randomBytes(32);
+const iv = randomBytes(16);
+const tag = randomBytes(12);
+createDecipheriv('aes-256-gcm', key, iv).setAuthTag(tag);
