From b863a27da7d1a2bd143c67a7c05881dcf4d82e2e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Sun, 27 Mar 2022 01:28:19 +0100
Subject: [PATCH] crypto: make authTagLength optional for CC20P1305

PR-URL: https://github.com/nodejs/node/pull/42427
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
---
 doc/api/crypto.md                          | 32 ++++++++++----
 src/crypto/crypto_cipher.cc                | 14 +++++--
 test/parallel/test-crypto-authenticated.js | 49 ++++++++++++++++++++--
 3 files changed, 80 insertions(+), 15 deletions(-)

diff --git a/doc/api/crypto.md b/doc/api/crypto.md
index a2796a0b27f301..9aa0829ac70bc6 100644
--- a/doc/api/crypto.md
+++ b/doc/api/crypto.md
@@ -2930,6 +2930,10 @@ Checks the primality of the `candidate`.
 added: v0.1.94
 deprecated: v10.0.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/42427
+    description: The `authTagLength` option is now optional when using the
+                 `chacha20-poly1305` cipher and defaults to 16 bytes.
   - version: v15.0.0
     pr-url: https://github.com/nodejs/node/pull/35093
     description: The password argument can be an ArrayBuffer and is limited to
@@ -2954,12 +2958,12 @@ Creates and returns a `Cipher` object that uses the given `algorithm` and
 `password`.
 
 The `options` argument controls stream behavior and is optional except when a
-cipher in CCM or OCB mode (e.g. `'aes-128-ccm'`) or `chacha20-poly1305` is used.
-In that case, the
+cipher in CCM or OCB mode (e.g. `'aes-128-ccm'`) is used. In that case, the
 `authTagLength` option is required and specifies the length of the
 authentication tag in bytes, see [CCM mode][]. In GCM mode, the `authTagLength`
 option is not required but can be used to set the length of the authentication
 tag that will be returned by `getAuthTag()` and defaults to 16 bytes.
+For `chacha20-poly1305`, the `authTagLength` option defaults to 16 bytes.
 
 The `algorithm` is dependent on OpenSSL, examples are `'aes192'`, etc. On
 recent OpenSSL releases, `openssl list -cipher-algorithms` will
@@ -2990,6 +2994,10 @@ Adversaries][] for details.
 <!-- YAML
 added: v0.1.94
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/42427
+    description: The `authTagLength` option is now optional when using the
+                 `chacha20-poly1305` cipher and defaults to 16 bytes.
   - version: v15.0.0
     pr-url: https://github.com/nodejs/node/pull/35093
     description: The password and iv arguments can be an ArrayBuffer and are
@@ -3026,12 +3034,12 @@ Creates and returns a `Cipher` object, with the given `algorithm`, `key` and
 initialization vector (`iv`).
 
 The `options` argument controls stream behavior and is optional except when a
-cipher in CCM or OCB mode (e.g. `'aes-128-ccm'`) or `chacha20-poly1305` is used.
-In that case, the
+cipher in CCM or OCB mode (e.g. `'aes-128-ccm'`) is used. In that case, the
 `authTagLength` option is required and specifies the length of the
 authentication tag in bytes, see [CCM mode][]. In GCM mode, the `authTagLength`
 option is not required but can be used to set the length of the authentication
 tag that will be returned by `getAuthTag()` and defaults to 16 bytes.
+For `chacha20-poly1305`, the `authTagLength` option defaults to 16 bytes.
 
 The `algorithm` is dependent on OpenSSL, examples are `'aes192'`, etc. On
 recent OpenSSL releases, `openssl list -cipher-algorithms` will
@@ -3059,6 +3067,10 @@ given IV will be.
 added: v0.1.94
 deprecated: v10.0.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/42427
+    description: The `authTagLength` option is now optional when using the
+                 `chacha20-poly1305` cipher and defaults to 16 bytes.
   - version: v10.10.0
     pr-url: https://github.com/nodejs/node/pull/21447
     description: Ciphers in OCB mode are now supported.
@@ -3075,10 +3087,10 @@ Creates and returns a `Decipher` object that uses the given `algorithm` and
 `password` (key).
 
 The `options` argument controls stream behavior and is optional except when a
-cipher in CCM or OCB mode (e.g. `'aes-128-ccm'`) or `chacha20-poly1305` is used.
-In that case, the
+cipher in CCM or OCB mode (e.g. `'aes-128-ccm'`) is used. In that case, the
 `authTagLength` option is required and specifies the length of the
 authentication tag in bytes, see [CCM mode][].
+For `chacha20-poly1305`, the `authTagLength` option defaults to 16 bytes.
 
 The implementation of `crypto.createDecipher()` derives keys using the OpenSSL
 function [`EVP_BytesToKey`][] with the digest algorithm set to MD5, one
@@ -3097,6 +3109,10 @@ to create the `Decipher` object.
 <!-- YAML
 added: v0.1.94
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/42427
+    description: The `authTagLength` option is now optional when using the
+                 `chacha20-poly1305` cipher and defaults to 16 bytes.
   - version: v11.6.0
     pr-url: https://github.com/nodejs/node/pull/24234
     description: The `key` argument can now be a `KeyObject`.
@@ -3129,12 +3145,12 @@ Creates and returns a `Decipher` object that uses the given `algorithm`, `key`
 and initialization vector (`iv`).
 
 The `options` argument controls stream behavior and is optional except when a
-cipher in CCM or OCB mode (e.g. `'aes-128-ccm'`) or `chacha20-poly1305` is used.
-In that case, the
+cipher in CCM or OCB mode (e.g. `'aes-128-ccm'`) is used. In that case, the
 `authTagLength` option is required and specifies the length of the
 authentication tag in bytes, see [CCM mode][]. In GCM mode, the `authTagLength`
 option is not required but can be used to restrict accepted authentication tags
 to those with the specified length.
+For `chacha20-poly1305`, the `authTagLength` option defaults to 16 bytes.
 
 The `algorithm` is dependent on OpenSSL, examples are `'aes192'`, etc. On
 recent OpenSSL releases, `openssl list -cipher-algorithms` will
diff --git a/src/crypto/crypto_cipher.cc b/src/crypto/crypto_cipher.cc
index 05acefb60113c1..f65b7c41bd996d 100644
--- a/src/crypto/crypto_cipher.cc
+++ b/src/crypto/crypto_cipher.cc
@@ -571,9 +571,17 @@ bool CipherBase::InitAuthenticated(
     }
   } else {
     if (auth_tag_len == kNoAuthTagLength) {
-      THROW_ERR_CRYPTO_INVALID_AUTH_TAG(
-        env(), "authTagLength required for %s", cipher_type);
-      return false;
+      // We treat ChaCha20-Poly1305 specially. Like GCM, the authentication tag
+      // length defaults to 16 bytes when encrypting. Unlike GCM, the
+      // authentication tag length also defaults to 16 bytes when decrypting,
+      // whereas GCM would accept any valid authentication tag length.
+      if (EVP_CIPHER_CTX_nid(ctx_.get()) == NID_chacha20_poly1305) {
+        auth_tag_len = 16;
+      } else {
+        THROW_ERR_CRYPTO_INVALID_AUTH_TAG(
+          env(), "authTagLength required for %s", cipher_type);
+        return false;
+      }
     }
 
     // TODO(tniessen) Support CCM decryption in FIPS mode
diff --git a/test/parallel/test-crypto-authenticated.js b/test/parallel/test-crypto-authenticated.js
index b8b0f5b54fc91c..162b451c5b459c 100644
--- a/test/parallel/test-crypto-authenticated.js
+++ b/test/parallel/test-crypto-authenticated.js
@@ -96,10 +96,9 @@ for (const test of TEST_CASES) {
 
   const isCCM = /^aes-(128|192|256)-ccm$/.test(test.algo);
   const isOCB = /^aes-(128|192|256)-ocb$/.test(test.algo);
-  const isChacha20Poly1305 = test.algo === 'chacha20-poly1305';
 
   let options;
-  if (isCCM || isOCB || isChacha20Poly1305)
+  if (isCCM || isOCB)
     options = { authTagLength: test.tag.length / 2 };
 
   const inputEncoding = test.plainIsHex ? 'hex' : 'ascii';
@@ -659,8 +658,7 @@ for (const test of TEST_CASES) {
     assert.throws(() => crypto.createCipheriv(
       valid.algo,
       Buffer.from(valid.key, 'hex'),
-      Buffer.from(H(prefix) + valid.iv, 'hex'),
-      { authTagLength: valid.tag.length / 2 }
+      Buffer.from(H(prefix) + valid.iv, 'hex')
     ), errMessages.length, `iv length ${ivLength} was not rejected`);
 
     function H(length) { return '00'.repeat(length); }
@@ -745,3 +743,46 @@ for (const test of TEST_CASES) {
     }
   }
 }
+
+// ChaCha20-Poly1305 should default to an authTagLength of 16. When encrypting,
+// this matches the behavior of GCM ciphers. When decrypting, however, it is
+// stricter than GCM in that it only allows authentication tags that are exactly
+// 16 bytes long, whereas, when no authTagLength was specified, GCM would accept
+// shorter tags as long as their length was valid according to NIST SP 800-38D.
+// For ChaCha20-Poly1305, we intentionally deviate from that because there are
+// no recommended or approved authentication tag lengths below 16 bytes.
+{
+  const rfcTestCases = TEST_CASES.filter(({ algo, tampered }) => {
+    return algo === 'chacha20-poly1305' && tampered === false;
+  });
+  assert.strictEqual(rfcTestCases.length, 1);
+
+  const [testCase] = rfcTestCases;
+  const key = Buffer.from(testCase.key, 'hex');
+  const iv = Buffer.from(testCase.iv, 'hex');
+  const aad = Buffer.from(testCase.aad, 'hex');
+
+  for (const opt of [
+    undefined,
+    { authTagLength: undefined },
+    { authTagLength: 16 },
+  ]) {
+    const cipher = crypto.createCipheriv('chacha20-poly1305', key, iv, opt);
+    const ciphertext = Buffer.concat([
+      cipher.setAAD(aad).update(testCase.plain, 'hex'),
+      cipher.final(),
+    ]);
+    const authTag = cipher.getAuthTag();
+
+    assert.strictEqual(ciphertext.toString('hex'), testCase.ct);
+    assert.strictEqual(authTag.toString('hex'), testCase.tag);
+
+    const decipher = crypto.createDecipheriv('chacha20-poly1305', key, iv, opt);
+    const plaintext = Buffer.concat([
+      decipher.setAAD(aad).update(ciphertext),
+      decipher.setAuthTag(authTag).final(),
+    ]);
+
+    assert.strictEqual(plaintext.toString('hex'), testCase.plain);
+  }
+}