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

Fixes: nodejs#53742
JLHwung · Jul 9, 2024 · 7518c53 · 7518c53
1 parent b9289a6
commit 7518c53
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 11 deletions.
diff --git a/src/crypto/crypto_sig.cc b/src/crypto/crypto_sig.cc
@@ -93,11 +93,12 @@ std::unique_ptr<BackingStore> Node_SignFinal(Environment* env,
   }
   EVPKeyCtxPointer pkctx(EVP_PKEY_CTX_new(pkey.get(), nullptr));
   if (pkctx &&
-      EVP_PKEY_sign_init(pkctx.get()) &&
+      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_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)) {
+                    &sig_len, m, m_len) > 0) {
     CHECK_LE(sig_len, sig->ByteLength());
     if (sig_len == 0) {
       sig = ArrayBuffer::NewBackingStore(env->isolate(), 0);
@@ -527,14 +528,19 @@ 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/dh_private.pem b/test/fixtures/keys/dh_private.pem
@@ -0,0 +1,9 @@
+-----BEGIN PRIVATE KEY-----
+MIIBIQIBADCBlQYJKoZIhvcNAQMBMIGHAoGBANEfWLkepFV7Ym8nPQblm/B3+mGl
+ptmFYnrDvCpm+cw3w8SSoVIhZTB/q561jy/zDh1ZAT+K/gO0Go80sUhpv9XeNSxw
+eY9bZx6LVqWltgkuWmjtZRUwwExXopZpvcpw0Cn/XH9fb+o7+RDFTk/VJvEbJXcY
+mDBF7to/skujEjiHAgECBIGDAoGAVxqjqDJvQY9R+XmxYM1SCaT9gJh8f+TYHn4y
+0j5/7c7rej9toPLX3Et72182AZdw87y/AUexfrXT/F31v3wxYFxk2n1j8/7hTmpH
+MZnWLYoa5Rjs0X3a3BExN08O1X7pfB+qI4E+Dpzeqx5dcELWcfKS9YCPBBfwaUyP
+RXVC7TA=
+-----END PRIVATE KEY-----
diff --git a/test/fixtures/keys/dh_public.pem b/test/fixtures/keys/dh_public.pem
@@ -0,0 +1,9 @@
+-----BEGIN PUBLIC KEY-----
+MIIBIDCBlQYJKoZIhvcNAQMBMIGHAoGBANEfWLkepFV7Ym8nPQblm/B3+mGlptmF
+YnrDvCpm+cw3w8SSoVIhZTB/q561jy/zDh1ZAT+K/gO0Go80sUhpv9XeNSxweY9b
+Zx6LVqWltgkuWmjtZRUwwExXopZpvcpw0Cn/XH9fb+o7+RDFTk/VJvEbJXcYmDBF
+7to/skujEjiHAgECA4GFAAKBgQDEEE8yLWxIej02E5FeKHpPvO6e2+nV/hhEdlrK
+0N5awvX/xex4R/VCyKSdycv9dgPE+q84d+iwYhrEwZeUPzWwOpCuqvOZyeF9V63V
+iNecJEKHjRR3SRh95+6BVB04JASNVj+YHKybdOhptAGgZVa+vUG8jznCamHtJB/h
+Ulxzvw==
+-----END PUBLIC KEY-----
diff --git a/test/parallel/test-crypto-sign-verify.js b/test/parallel/test-crypto-sign-verify.js
@@ -793,3 +793,31 @@ assert.throws(
     }, { code: 'ERR_CRYPTO_UNSUPPORTED_OPERATION', message: 'Unsupported crypto operation' });
   }
 }
+
+{
+  const keys = [
+    {
+      privateKey: fixtures.readKey('dh_private.pem', 'ascii'),
+      publicKey: fixtures.readKey('dh_public.pem', 'ascii'),
+    },
+    {
+      privateKey: fixtures.readKey('x25519_private.pem', 'ascii'),
+      publicKey: fixtures.readKey('x25519_public.pem', 'ascii'),
+    },
+    {
+      privateKey: fixtures.readKey('x448_private.pem', 'ascii'),
+      publicKey: fixtures.readKey('x448_public.pem', 'ascii'),
+    },
+  ];
+  for (const { publicKey, privateKey } of keys) {
+    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/ });
+  }
+}