crypto: fix Node_SignFinal

PR #11705 switched Node away from using using OpenSSL's legacy EVP_Sign*
and EVP_Verify* APIs. Instead, it computes a hash normally via
EVP_Digest* and then uses EVP_PKEY_sign and EVP_PKEY_verify to verify
the hash directly. This change corrects two problems:

1. The documentation still recommends the signature algorithm EVP_MD
   names of OpenSSL's legacy APIs. OpenSSL has since moved away from
   thosee, which is why ECDSA was strangely inconsistent. (This is why
   "ecdsa-with-SHA256" was missing.)

2. Node_SignFinal copied some code from EVP_SignFinal's internals. This
   is problematic for OpenSSL 1.1.0 and is missing a critical check
   that prevents pkey->pkey.ptr from being cast to the wrong type.

To resolve this, remove the non-EVP_PKEY_sign codepath. This codepath is
no longer necessary. PR #11705's verify half was already assuming all
EVP_PKEYs supported EVP_PKEY_sign and EVP_PKEY_verify. Also, in the
documentation, point users towards using hash function names which are
more consisent. This avoids an ECDSA special-case and some strangeness
around RSA-PSS ("RSA-SHA256" is the OpenSSL name of the
sha256WithRSAEncryption OID which is not used for RSA-PSS).

PR-URL: https://github.com/nodejs/node/pull/15024
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

1 files changed, 19 insertions, 28 deletions

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 23817f517d..174f502633 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -3981,7 +3981,8 @@ void SignBase::CheckThrow(SignBase::Error error) {
 
 static bool ApplyRSAOptions(EVP_PKEY* pkey, EVP_PKEY_CTX* pkctx, int padding,
                             int salt_len) {
-  if (pkey->type == EVP_PKEY_RSA || pkey->type == EVP_PKEY_RSA2) {
+  if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA ||
+      EVP_PKEY_id(pkey) == EVP_PKEY_RSA2) {
     if (EVP_PKEY_CTX_set_rsa_padding(pkctx, padding) <= 0)
       return false;
     if (padding == RSA_PKCS1_PSS_PADDING) {
@@ -4090,33 +4091,23 @@ static int Node_SignFinal(EVP_MD_CTX* mdctx, unsigned char* md,
   if (!EVP_DigestFinal_ex(mdctx, m, &m_len))
     return rv;
 
-  if (mdctx->digest->flags & EVP_MD_FLAG_PKEY_METHOD_SIGNATURE) {
-    size_t sltmp = static_cast<size_t>(EVP_PKEY_size(pkey));
-    pkctx = EVP_PKEY_CTX_new(pkey, nullptr);
-    if (pkctx == nullptr)
-      goto err;
-    if (EVP_PKEY_sign_init(pkctx) <= 0)
-      goto err;
-    if (!ApplyRSAOptions(pkey, pkctx, padding, pss_salt_len))
-      goto err;
-    if (EVP_PKEY_CTX_set_signature_md(pkctx, mdctx->digest) <= 0)
-      goto err;
-    if (EVP_PKEY_sign(pkctx, md, &sltmp, m, m_len) <= 0)
-      goto err;
-    *sig_len = sltmp;
-    rv = 1;
-   err:
-    EVP_PKEY_CTX_free(pkctx);
-    return rv;
-  }
-
-  if (mdctx->digest->sign == nullptr) {
-    EVPerr(EVP_F_EVP_SIGNFINAL, EVP_R_NO_SIGN_FUNCTION_CONFIGURED);
-    return 0;
-  }
-
-  return mdctx->digest->sign(mdctx->digest->type, m, m_len, md, sig_len,
-                             pkey->pkey.ptr);
+  size_t sltmp = static_cast<size_t>(EVP_PKEY_size(pkey));
+  pkctx = EVP_PKEY_CTX_new(pkey, nullptr);
+  if (pkctx == nullptr)
+    goto err;
+  if (EVP_PKEY_sign_init(pkctx) <= 0)
+    goto err;
+  if (!ApplyRSAOptions(pkey, pkctx, padding, pss_salt_len))
+    goto err;
+  if (EVP_PKEY_CTX_set_signature_md(pkctx, EVP_MD_CTX_md(mdctx)) <= 0)
+    goto err;
+  if (EVP_PKEY_sign(pkctx, md, &sltmp, m, m_len) <= 0)
+    goto err;
+  *sig_len = sltmp;
+  rv = 1;
+ err:
+  EVP_PKEY_CTX_free(pkctx);
+  return rv;
 }
 
 SignBase::Error Sign::SignFinal(const char* key_pem,