crypto: reject dh,x25519,x448 in {Sign,Verify}Final

Fixes: #53742 PR-URL: #53774 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
nodejs · Sep 12, 2024 · 5bba078 · 5bba078
1 parent 82c08ef
commit 5bba078
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 13 deletions.
diff --git a/src/crypto/crypto_sig.cc b/src/crypto/crypto_sig.cc
@@ -92,12 +92,15 @@ std::unique_ptr<BackingStore> Node_SignFinal(Environment* env,
  sig = ArrayBuffer::NewBackingStore(env->isolate(), sig_len);
  }
  EVPKeyCtxPointer pkctx(EVP_PKEY_CTX_new(pkey.get(), nullptr));
- if (pkctx &&
- EVP_PKEY_sign_init(pkctx.get()) &&
+ if (pkctx && EVP_PKEY_sign_init(pkctx.get()) > 0 &&
  ApplyRSAOptions(pkey, pkctx.get(), padding, pss_salt_len) &&
- EVP_PKEY_CTX_set_signature_md(pkctx.get(), EVP_MD_CTX_md(mdctx.get())) &&
- EVP_PKEY_sign(pkctx.get(), static_cast<unsigned char*>(sig->Data()),
- &sig_len, m, m_len)) {
+ EVP_PKEY_CTX_set_signature_md(pkctx.get(), EVP_MD_CTX_md(mdctx.get())) >
+ 0 &&
+ EVP_PKEY_sign(pkctx.get(),
+ static_cast<unsigned char*>(sig->Data()),
+ &sig_len,
+ m,
+ m_len) > 0) {
  CHECK_LE(sig_len, sig->ByteLength());
  if (sig_len == 0) {
  sig = ArrayBuffer::NewBackingStore(env->isolate(), 0);
@@ -527,14 +530,18 @@ SignBase::Error Verify::VerifyFinal(const ManagedEVPPKey& pkey,
  return kSignPublicKey;
 
  EVPKeyCtxPointer pkctx(EVP_PKEY_CTX_new(pkey.get(), nullptr));
- if (pkctx &&
- EVP_PKEY_verify_init(pkctx.get()) > 0 &&
- ApplyRSAOptions(pkey, pkctx.get(), padding, saltlen) &&
- EVP_PKEY_CTX_set_signature_md(pkctx.get(),
- EVP_MD_CTX_md(mdctx.get())) > 0) {
- const unsigned char* s = sig.data<unsigned char>();
- const int r = EVP_PKEY_verify(pkctx.get(), s, sig.size(), m, m_len);
- *verify_result = r == 1;
+ if (pkctx) {
+ const int init_ret = EVP_PKEY_verify_init(pkctx.get());
+ if (init_ret == -2) {
+ return kSignPublicKey;
+ }
+ if (init_ret > 0 && ApplyRSAOptions(pkey, pkctx.get(), padding, saltlen) &&
+ EVP_PKEY_CTX_set_signature_md(pkctx.get(), EVP_MD_CTX_md(mdctx.get())) >
+ 0) {
+ const unsigned char* s = sig.data<unsigned char>();
+ const int r = EVP_PKEY_verify(pkctx.get(), s, sig.size(), m, m_len);
+ *verify_result = r == 1;
+ }
  }
 
  return kSignOk;

diff --git a/test/fixtures/keys/Makefile b/test/fixtures/keys/Makefile
@@ -25,6 +25,8 @@ all: \
  dh1024.pem \
  dh2048.pem \
  dherror.pem \
+ dh_private.pem \
+ dh_public.pem \
  dsa_params.pem \
  dsa_private.pem \
  dsa_private_encrypted.pem \
@@ -597,6 +599,12 @@ dh2048.pem:
 dherror.pem: dh1024.pem
  sed 's/^[^-].*/AAAAAAAAAA/g' dh1024.pem > dherror.pem
 
+dh_private.pem:
+ openssl genpkey -algorithm dh -out dh_private.pem -pkeyopt dh_param:ffdhe2048
+
+dh_public.pem: dh_private.pem
+ openssl pkey -in dh_private.pem -pubout -out dh_public.pem
+
 dsa_params.pem:
  openssl dsaparam -out dsa_params.pem 2048
 

diff --git a/test/fixtures/keys/dh_private.pem b/test/fixtures/keys/dh_private.pem
@@ -0,0 +1,9 @@
+-----BEGIN PRIVATE KEY-----
+MIIBPgIBADCCARcGCSqGSIb3DQEDATCCAQgCggEBAP//////////rfhUWKK7Spqv
+3FYgJz088di5xYPOLTaVqeE2QRRkM/vMk53OJJs++X0v42NjDHXY9oGyAq7EYXrT
+3x7V1f1lYSQz9R9fBm7QhWNlVT3tGvO1VxNef1fJNZhPDHDg5ot34qaJ2vPv6HId
+8VihNq3nNTCsyk9IOnl6vAqxgrMk+2HRCKlLssjj+7lq2rdg1/RoHU9Co945TfSu
+Vu3nY3K7GQsHp8juCm1wngL84c334uzANATNKDQvYZFy/pzphYP/jk8SMu7ygYPD
+/jsbTG+tczu1/LwuwiAFxY7xg30Wg7LG80omwbLv+ohrQjhhKFyX//////////8C
+AQIEHgIcKNGyhQRxIhVXoyktdymwbN6MgXv85vPax+8eqQ==
+-----END PRIVATE KEY-----
diff --git a/test/fixtures/keys/dh_public.pem b/test/fixtures/keys/dh_public.pem
@@ -0,0 +1,14 @@
+-----BEGIN PUBLIC KEY-----
+MIICJTCCARcGCSqGSIb3DQEDATCCAQgCggEBAP//////////rfhUWKK7Spqv3FYg
+Jz088di5xYPOLTaVqeE2QRRkM/vMk53OJJs++X0v42NjDHXY9oGyAq7EYXrT3x7V
+1f1lYSQz9R9fBm7QhWNlVT3tGvO1VxNef1fJNZhPDHDg5ot34qaJ2vPv6HId8Vih
+Nq3nNTCsyk9IOnl6vAqxgrMk+2HRCKlLssjj+7lq2rdg1/RoHU9Co945TfSuVu3n
+Y3K7GQsHp8juCm1wngL84c334uzANATNKDQvYZFy/pzphYP/jk8SMu7ygYPD/jsb
+TG+tczu1/LwuwiAFxY7xg30Wg7LG80omwbLv+ohrQjhhKFyX//////////8CAQID
+ggEGAAKCAQEA2whDVdYtNbr/isSFdw7rOSdbmcWrxiX6ppqDZ6yp8XjUj3/CEf/P
+60X7HndX+nXD7YaPtVZxktkIpArI7C+AH7fZxBduuv2eLnvYwK82jFHKe7zvfdMr
+26akMCV0kBA3ktgcftHlqYsIj52BaJlG37FRha3SDOL2yJOij3hNQhHCXTWLg7tP
+GtXmD202OoZ6Ll+LxBzBCFnxVauiKnzBGeawy4gDycUEHmq5oDRR68I2gmxmsLg5
+MQVAP5ljp+FEu4+TZm6hR4wQ5PRjCQ+teq+VqMro7EbbvZpn+X9kAgKSl2WDu0fT
+FbUnBn3HPBmUa/Fv/ooXrlckTUDjLkbWZQ==
+-----END PUBLIC KEY-----
diff --git a/test/parallel/test-crypto-sign-verify.js b/test/parallel/test-crypto-sign-verify.js
@@ -793,3 +793,21 @@ assert.throws(
  }, { code: 'ERR_CRYPTO_UNSUPPORTED_OPERATION', message: 'Unsupported crypto operation' });
  }
 }
+
+{
+ // Dh, x25519 and x448 should not be used for signing/verifying
+ // https://github.com/nodejs/node/issues/53742
+ for (const algo of ['dh', 'x25519', 'x448']) {
+ const privateKey = fixtures.readKey(`${algo}_private.pem`, 'ascii');
+ const publicKey = fixtures.readKey(`${algo}_public.pem`, 'ascii');
+ assert.throws(() => {
+ crypto.createSign('SHA256').update('Test123').sign(privateKey);
+ }, { code: 'ERR_OSSL_EVP_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE', message: /operation not supported for this keytype/ });
+ assert.throws(() => {
+ crypto.createVerify('SHA256').update('Test123').verify(privateKey, 'sig');
+ }, { code: 'ERR_OSSL_EVP_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE', message: /operation not supported for this keytype/ });
+ assert.throws(() => {
+ crypto.createVerify('SHA256').update('Test123').verify(publicKey, 'sig');
+ }, { code: 'ERR_OSSL_EVP_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE', message: /operation not supported for this keytype/ });
+ }
+}