From 6e7274fa53fe96b7582a8a8aabfbaf2792021355 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A1ng=20J=C3=B9nli=C3=A0ng?= Date: Fri, 6 Sep 2024 18:07:16 -0400 Subject: [PATCH] crypto: reject dh,x25519,x448 in {Sign,Verify}Final MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: https://github.com/nodejs/node/issues/53742 PR-URL: https://github.com/nodejs/node/pull/53774 Reviewed-By: James M Snell Reviewed-By: Tobias Nießen --- src/crypto/crypto_sig.cc | 33 ++++++++++++++---------- test/fixtures/keys/Makefile | 8 ++++++ test/fixtures/keys/dh_private.pem | 9 +++++++ test/fixtures/keys/dh_public.pem | 14 ++++++++++ test/parallel/test-crypto-sign-verify.js | 18 +++++++++++++ 5 files changed, 69 insertions(+), 13 deletions(-) create mode 100644 test/fixtures/keys/dh_private.pem create mode 100644 test/fixtures/keys/dh_public.pem diff --git a/src/crypto/crypto_sig.cc b/src/crypto/crypto_sig.cc index a0ed640acaaa89..e6731638aa2c87 100644 --- a/src/crypto/crypto_sig.cc +++ b/src/crypto/crypto_sig.cc @@ -92,12 +92,15 @@ std::unique_ptr 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(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(sig->Data()), + &sig_len, + m, + m_len) > 0) { CHECK_LE(sig_len, sig->ByteLength()); if (sig_len == 0) sig = ArrayBuffer::NewBackingStore(env->isolate(), 0); @@ -526,14 +529,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(); - 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(); + 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 index f562df27047526..3339f4b912dc92 100644 --- a/test/fixtures/keys/Makefile +++ b/test/fixtures/keys/Makefile @@ -26,6 +26,8 @@ all: \ dh2048.pem \ dh3072.pem \ dherror.pem \ + dh_private.pem \ + dh_public.pem \ dsa_params.pem \ dsa_private.pem \ dsa_private_encrypted.pem \ @@ -601,6 +603,12 @@ dh3072.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 new file mode 100644 index 00000000000000..25c4edc5ea5a3b --- /dev/null +++ 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 new file mode 100644 index 00000000000000..b32815e88acc8c --- /dev/null +++ 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 index 81adde1ba771c8..1d742c6801c233 100644 --- a/test/parallel/test-crypto-sign-verify.js +++ b/test/parallel/test-crypto-sign-verify.js @@ -794,3 +794,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/ }); + } +}