crypto: fix Node_SignFinal by davidben · Pull Request #15024 · nodejs/node

@nodejs-github-bot added c++

Issues and PRs that require attention from people who are familiar with C++.

crypto

Issues and PRs related to the crypto subsystem.

labels

Aug 24, 2017

shigeki

BridgeAR

@davidben

PR nodejs#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 nodejs#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).

BridgeAR

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request

Sep 11, 2017
PR nodejs#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 nodejs#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: nodejs#15024
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

addaleax pushed a commit to addaleax/node that referenced this pull request

Sep 13, 2017
PR nodejs#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 nodejs#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: nodejs#15024
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

jasnell pushed a commit that referenced this pull request

Sep 20, 2017
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: #15024
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

MylesBorins pushed a commit that referenced this pull request

Oct 17, 2017
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: #15024
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

MylesBorins pushed a commit that referenced this pull request

Oct 25, 2017
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: #15024
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

omsmith added a commit to omsmith/node-jwa that referenced this pull request

May 23, 2018

omsmith added a commit to omsmith/node-jwa that referenced this pull request

May 23, 2018