File CVE-2023-30590.patch of Package nodejs12
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']);
+}
+