crypto: fix rsa key gen with non-default exponent #27092
Conversation
EVP_PKEY_CTX_set_rsa_keygen_pubexp() accepts ownership of the exponent on success, so do not free it. Fixes: nodejs#27087
| @@ -146,7 +146,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); | |||
|
|
|||
| // Now do the same with an encrypted private key. | |||
| generateKeyPair('rsa', { | |||
| publicExponent: 0x10001, | |||
There was a problem hiding this comment.
Can we keep this exponent in both tests (here and the change above) as an additional test parameter?
There was a problem hiding this comment.
@mscdex exponent 0x10001 is tested 4 times in test-crypto-keygen.js, I don't mind copy and pasting the tests with non-default exponents, but don't think it will increase coverage. Can you confirm you want me to do this? For your reference:
core/node (fix-non-default-exp-rsa-keygen $%) % grep publicExponent test/parallel/test-crypto-keygen.js
publicExponent: 3, // I changed this from 0x0001
publicExponent: 0x10001,
publicExponent: 0x1001, // I changed this from 0x0001
publicExponent: 0x10001,
publicExponent: 0x10001,
publicExponent: 0x10001,
There was a problem hiding this comment.
I see, either way is fine then.
Doesn't release return the pointer? I am not sure, maybe it doesn't :)
|
yes, release both returns the pointer, and nulls it (so the auto ptr doesn't own it). But its the only function to use (I think). |
|
Landed in 0911e88 |
EVP_PKEY_CTX_set_rsa_keygen_pubexp() accepts ownership of the exponent on success, so do not free it. Fixes: #27087 PR-URL: #27092 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>
|
FWIW this fixes a crash that currently exists on latest v10.x. |
EVP_PKEY_CTX_set_rsa_keygen_pubexp() accepts ownership of the exponent on success, so do not free it. Fixes: #27087 Fixes: #29433 PR-URL: #27092 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>
EVP_PKEY_CTX_set_rsa_keygen_pubexp() accepts ownership of the exponent
on success, so do not free it.
Fixes: #27087
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes