From 57f019481f822be8259085434499bea84ea151ce Mon Sep 17 00:00:00 2001
From: Adam Majer <amajer@suse.de>
Date: Wed, 22 Jun 2022 13:41:16 +0200
Subject: [PATCH] crypto: disable ciphers not supported by EVP

OpenSSL lists certain ciphers as not conforming with the standard
EVP AEAD interface and these ciphers should be avoided,

    aes-128-cbc-hmac-sha1
    aes-128-cbc-hmac-sha256
    aes-256-cbc-hmac-sha1
    aes-256-cbc-hmac-sha256

While experimenting with these, the cipher text produced by Node
for `aes-128-cbc-hmac-sha1` does not differ from cipher text
produced by `aes-128-cbc` meaning that the HMAC is not automatically
taken into account. Since there is no facility to set the HMAC key
in Node, it's best to have these disabled when using crypto.Cipher
as all usage is wrong by definition.

Fixes: https://github.com/nodejs/node/issues/43040
---
 doc/api/errors.md             |  8 ++++++++
 lib/internal/crypto/cipher.js |  9 +++++++++
 lib/internal/errors.js        |  3 +++
 test/parallel/test-crypto.js  | 20 ++++++++++++++++----
 4 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/doc/api/errors.md b/doc/api/errors.md
index af96956002f05b..6672d621c68223 100644
--- a/doc/api/errors.md
+++ b/doc/api/errors.md
@@ -791,6 +791,14 @@ when an error occurs (and is caught) during the creation of the
 context, for example, when the allocation fails or the maximum call stack
 size is reached when the context is created.
 
+<a id="ERR_CRYPTO_CIPHER_DISABLED"></a>
+
+### `ERR_CRYPTO_CIPHER_DISABLED`
+
+A cipher was requested that does not conform to OpenSSL's standard
+EVP interface for AEAD ciphers and requires usage of undocumented
+interfaces.
+
 <a id="ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED"></a>
 
 ### `ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED`
diff --git a/lib/internal/crypto/cipher.js b/lib/internal/crypto/cipher.js
index d85606ba52b5ac..5c3de730545070 100644
--- a/lib/internal/crypto/cipher.js
+++ b/lib/internal/crypto/cipher.js
@@ -24,6 +24,7 @@ const {
 
 const {
   codes: {
+    ERR_CRYPTO_CIPHER_DISABLED,
     ERR_CRYPTO_INVALID_STATE,
     ERR_INVALID_ARG_TYPE,
     ERR_INVALID_ARG_VALUE,
@@ -108,6 +109,14 @@ function getUIntOption(options, key) {
 }
 
 function createCipherBase(cipher, credential, options, decipher, iv) {
+  switch (cipher.toLowerCase()) {
+    case 'aes-128-cbc-hmac-sha1':
+    case 'aes-128-cbc-hmac-sha256':
+    case 'aes-256-cbc-hmac-sha1':
+    case 'aes-256-cbc-hmac-sha256':
+      throw new ERR_CRYPTO_CIPHER_DISABLED(cipher);
+  }
+
   const authTagLength = getUIntOption(options, 'authTagLength');
   this[kHandle] = new CipherBase(decipher);
   if (iv === undefined) {
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 306dcbad998134..cf95e83c506732 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -936,6 +936,9 @@ E('ERR_CHILD_PROCESS_STDIO_MAXBUFFER', '%s maxBuffer length exceeded',
 E('ERR_CONSOLE_WRITABLE_STREAM',
   'Console expects a writable stream instance for %s', TypeError);
 E('ERR_CONTEXT_NOT_INITIALIZED', 'context used is not initialized', Error);
+E('ERR_CRYPTO_CIPHER_DISABLED',
+  'Cipher %s is disabled as it\'s not compatible with OpenSSL\'s ' +
+  'EVP AEAD interfaces', Error);
 E('ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED',
   'Custom engines not supported by this OpenSSL', Error);
 E('ERR_CRYPTO_ECDH_INVALID_FORMAT', 'Invalid ECDH format: %s', TypeError);
diff --git a/test/parallel/test-crypto.js b/test/parallel/test-crypto.js
index a8ceb169de2b3d..30ccb88203c1c1 100644
--- a/test/parallel/test-crypto.js
+++ b/test/parallel/test-crypto.js
@@ -129,10 +129,22 @@ for (const algo of cryptoCiphers) {
     options = { authTagLength: 8 };
   else if (mode === 'ocb' || algo === 'chacha20-poly1305')
     options = { authTagLength: 16 };
-  crypto.createCipheriv(algo,
-                        crypto.randomBytes(keyLength),
-                        crypto.randomBytes(ivLength || 0),
-                        options);
+
+  const create_cipher = () =>
+    crypto.createCipheriv(algo,
+                          crypto.randomBytes(keyLength),
+                          crypto.randomBytes(ivLength || 0),
+                          options);
+
+  // Disabled ciphers will throw
+  if (algo.includes('hmac-sha'))
+    assert.throws(create_cipher, {
+      code: 'ERR_CRYPTO_CIPHER_DISABLED',
+      name: 'Error',
+      message: `Cipher ${algo} is disabled as it's not compatible with OpenSSL's EVP AEAD interfaces`
+    });
+  else
+    create_cipher();
 }
 
 // Assume that we have at least AES256-SHA.