nodejs12/CVE-2023-30590.patch

commit 1a5c9284ebce5cd71cf7a3c29759a748c373ac85
Author: Tobias Nießen <tobias.niessen@tuwien.ac.at>
Date:   Mon Jun 12 19:44:48 2023 +0200

    doc,test: clarify behavior of DH generateKeys
    
    The DiffieHellman class is an old and thin wrapper around certain
    OpenSSL functions, many of which are deprecated in OpenSSL 3.0. Because
    the Node.js API mirrors the OpenSSL API, it adopts some of its
    peculiarities, but the Node.js documentation does not properly reflect
    these. Most importantly, despite the documentation saying otherwise,
    diffieHellman.generateKeys() does not generate a new private key when
    one has already been set or generated. Based on the documentation alone,
    users may be led to misuse the API in a way that results in key reuse,
    which can have drastic negative consequences for subsequent operations
    that consume the shared secret.
    
    These design issues in this old API have been around for many years, and
    we are not currently aware of any misuse in the ecosystem that falls
    into the above scenario. Changing the behavior of the API would be a
    significant breaking change and is thus not appropriate for a security
    release (nor is it a goal.) The reported issue is treated as CWE-1068
    (after a vast amount of uncertainty whether to treat it as a
    vulnerability at all), therefore, this change only updates the
    documentation to match the actual behavior. Tests are also added that
    demonstrate this particular oddity.
    
    Newer APIs exist that can be used for some, but not all, Diffie-Hellman
    operations (e.g., crypto.diffieHellman() that was added in 2020). We
    should keep modernizing crypto APIs, but that is a non-goal for this
    security release.
    
    The ECDH class mirrors the DiffieHellman class in many ways, but it does
    not appear to be affected by this particular peculiarity. In particular,
    ecdh.generateKeys() does appear to always generate a new private key.
    
    PR-URL: https://github.com/nodejs-private/node-private/pull/426
    Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
    Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
    CVE-ID: CVE-2023-30590

Index: node-v12.22.12/doc/api/crypto.md
===================================================================
--- node-v12.22.12.orig/doc/api/crypto.md
+++ node-v12.22.12/doc/api/crypto.md
@@ -632,12 +632,17 @@ added: v0.5.0
 * `encoding` {string} The [encoding][] of the return value.
 * Returns: {Buffer | string}
 
-Generates private and public Diffie-Hellman key values, and returns
+Generates private and public Diffie-Hellman key values unless they have been
+generated or computed already, and returns
 the public key in the specified `encoding`. This key should be
 transferred to the other party.
 If `encoding` is provided a string is returned; otherwise a
 [`Buffer`][] is returned.
 
+This function is a thin wrapper around [`DH_generate_key()`][]. In particular,
+once a private key has been generated or set, calling this function only updates
+the public key but does not generate a new private key.
+
 ### `diffieHellman.getGenerator([encoding])`
 <!-- YAML
 added: v0.5.0
@@ -699,6 +704,10 @@ Sets the Diffie-Hellman private key. If
 to be a string. If no `encoding` is provided, `privateKey` is expected
 to be a [`Buffer`][], `TypedArray`, or `DataView`.
 
+This function does not automatically compute the associated public key. Either
+[`diffieHellman.setPublicKey()`][] or [`diffieHellman.generateKeys()`][] can be
+used to manually provide the public key or to automatically derive it.
+
 ### `diffieHellman.setPublicKey(publicKey[, encoding])`
 <!-- YAML
 added: v0.5.0
@@ -3527,6 +3536,7 @@ See the [list of SSL OP Flags][] for det
 </table>
 
 [`Buffer`]: buffer.html
+[`DH_generate_key()`]: https://www.openssl.org/docs/man3.0/man3/DH_generate_key.html
 [`EVP_BytesToKey`]: https://www.openssl.org/docs/man1.1.0/crypto/EVP_BytesToKey.html
 [`KeyObject`]: #crypto_class_keyobject
 [`Sign`]: #crypto_class_sign
@@ -3559,6 +3569,7 @@ See the [list of SSL OP Flags][] for det
 [`crypto.scrypt()`]: #crypto_crypto_scrypt_password_salt_keylen_options_callback
 [`decipher.final()`]: #crypto_decipher_final_outputencoding
 [`decipher.update()`]: #crypto_decipher_update_data_inputencoding_outputencoding
+[`diffieHellman.generateKeys()`]: #diffiehellmangeneratekeysencoding
 [`diffieHellman.setPublicKey()`]: #crypto_diffiehellman_setpublickey_publickey_encoding
 [`ecdh.generateKeys()`]: #crypto_ecdh_generatekeys_encoding_format
 [`ecdh.setPrivateKey()`]: #crypto_ecdh_setprivatekey_privatekey_encoding
Index: node-v12.22.12/test/parallel/test-crypto-dh.js
===================================================================
--- node-v12.22.12.orig/test/parallel/test-crypto-dh.js
+++ node-v12.22.12/test/parallel/test-crypto-dh.js
@@ -8,7 +8,8 @@ const crypto = require('crypto');
 
 // Test Diffie-Hellman with two parties sharing a secret,
 // using various encodings as we go along
-const dh1 = crypto.createDiffieHellman(common.hasFipsCrypto ? 1024 : 256);
+const size = common.hasFipsCrypto ? 1024 : 256;
+const dh1 = crypto.createDiffieHellman(size);
 const p1 = dh1.getPrime('buffer');
 const dh2 = crypto.createDiffieHellman(p1, 'buffer');
 let key1 = dh1.generateKeys();
@@ -471,3 +472,59 @@ assert.throws(
   'crypto.getDiffieHellman(\'modp1\').setPublicKey(\'\') ' +
   'failed to throw the expected error.'
 );
+
+{
+  function unlessInvalidState(f) {
+    try {
+      return f();
+    } catch (err) {
+        // all errors thrown here are invalid state about missing keys
+//      if (err.code !== 'ERR_CRYPTO_INVALID_STATE') {
+//console.log(err);
+//        throw err;
+//      }
+    }
+  }
+
+  function testGenerateKeysChangesKeys(setup, expected) {
+    const dh = crypto.createDiffieHellman(size);
+    setup(dh);
+    const firstPublicKey = unlessInvalidState(() => dh.getPublicKey());
+    const firstPrivateKey = unlessInvalidState(() => dh.getPrivateKey());
+    dh.generateKeys();
+    const secondPublicKey = dh.getPublicKey();
+    const secondPrivateKey = dh.getPrivateKey();
+    function changed(shouldChange, first, second) {
+      if (shouldChange) {
+        assert.notDeepStrictEqual(first, second);
+      } else {
+        assert.deepStrictEqual(first, second);
+      }
+    }
+    changed(expected.includes('public'), firstPublicKey, secondPublicKey);
+    changed(expected.includes('private'), firstPrivateKey, secondPrivateKey);
+  }
+
+  // Both the private and the public key are missing: generateKeys() generates both.
+  testGenerateKeysChangesKeys(() => {
+    // No setup.
+  }, ['public', 'private']);
+
+  // Neither key is missing: generateKeys() does nothing.
+  testGenerateKeysChangesKeys((dh) => {
+    dh.generateKeys();
+  }, []);
+
+  // Only the public key is missing: generateKeys() generates only the public key.
+  testGenerateKeysChangesKeys((dh) => {
+    dh.setPrivateKey(Buffer.from('01020304', 'hex'));
+  }, ['public']);
+
+  // The public key is outdated: generateKeys() generates only the public key.
+  testGenerateKeysChangesKeys((dh) => {
+    const oldPublicKey = dh.generateKeys();
+    dh.setPrivateKey(Buffer.from('01020304', 'hex'));
+    assert.deepStrictEqual(dh.getPublicKey(), oldPublicKey);
+  }, ['public']);
+}
+