Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto: sign/verify support for RSASSA-PSS #1127

Closed
wolfie82 opened this issue Mar 11, 2015 · 15 comments
Closed

crypto: sign/verify support for RSASSA-PSS #1127

wolfie82 opened this issue Mar 11, 2015 · 15 comments
Labels
crypto Issues and PRs related to the crypto subsystem. feature request Issues that request new features to be added to Node.js.

Comments

@wolfie82
Copy link

It would be good if the crypto.createSign / crypto.createVerify implementations supported different openssl padding schemes instead of the default PKCS1.5. Specifically, I'm interested in PSS and PSS with MGF1.

RSA_padding_add_PKCS1_PSS
RSA_padding_add_PKCS1_PSS_mgf1

I found the following forum post that may help in the development effort.

http://openssl.6102.n7.nabble.com/RSA-sign-and-verify-td44219.html

I'm a bit out of my depth with C++, however I'm willing to help implement if someone could point me in the right direction with crypto.cc.

Thanks

@mscdex mscdex added enhancement crypto Issues and PRs related to the crypto subsystem. labels Mar 11, 2015
@wolfie82
Copy link
Author

I did a bit more digging. It might be easier to update node_crypto.cc to use the later OpenSSL EVP API's.

EVP_Sign* & EVP_Verify* to EVP_DigestSign* & EVP_DigestVerify*

Then it should be a matter of calling EVP_PKEY_CTX_set_rsa_padding() with 'RSA_PKCS1_PSS_PADDING' & EVP_PKEY_CTX_set_rsa_pss_saltlen()

The only thing I'm a bit unsure of is MGF1.

@wolfie82 wolfie82 changed the title Crypto sign support for RSASSA-PSS crypto: sign/verify support for RSASSA-PSS Mar 12, 2015
@brendanashworth brendanashworth added feature request Issues that request new features to be added to Node.js. and removed enhancement labels May 6, 2015
@James-Firth
Copy link

+1 I would appreciate this feature, and it would help keep node compatible with more products in the future as adoption grows.

@Trott
Copy link
Member

Trott commented Jun 24, 2016

@nodejs/crypto Any thoughts on this? Specifically, is this a reasonable feature to include in core? And given as the issue is more than a year old, has anything changed that might make this issue obsolete? ("Oh, we already added support for that six months ago!")

@bnoordhuis
Copy link
Member

No strong opinion. I think it could be retrofitted onto crypto.createSign() and crypto.createVerify().

PSS appears to be rare. Not a reason to reject it but it is arguably a reason to not spend too much effort on it. I'd be interested in hearing use cases.

It might be easier to update node_crypto.cc to use the later OpenSSL EVP API's.

Not strictly necessary. They're aliases for their EVP_Digest counterparts in OpenSSL 1.1.0.

@mwain
Copy link
Contributor

mwain commented Oct 25, 2016

Is there any movement on this? I have a requirement to use SHA256withRSA/PSS but its not supported in node crypto.

@bnoordhuis
Copy link
Member

@mwain No movement whatsoever.

@mwain
Copy link
Contributor

mwain commented Oct 25, 2016

Would it be considered if i put something together?

@bnoordhuis
Copy link
Member

If it's not too intrusive I don't see why not. I'm curious what your use case is though.

@mwain
Copy link
Contributor

mwain commented Oct 25, 2016

Ok, you mentioned it might be able to be retrofitted into crypto.createSign() / crypto.createVerify() but they have an extra param options which is passed to stream.Writable, which is not documented :/

So i was thinking of adding an extra function to each to set padding type and option padding length?

@bnoordhuis
Copy link
Member

That sounds reasonable. Documenting and extending options is probably acceptable too.

@tniessen
Copy link
Member

I'm curious what your use case is though.

@bnoordhuis Just one possible use case I recently stumbled upon: The WebCrypto API specifies two algorithms for RSA-based signatures, RSASSA-PSS and RSASSA-PKCS1-v1_5. I think it would be nice to provide interoperability with as many of the operations defined as part of the WebCrypto API as possible, including RSASSA-PSS.

It might be easier to update node_crypto.cc to use the later OpenSSL EVP API's.

Not strictly necessary. They're aliases for their EVP_Digest counterparts in OpenSSL 1.1.0.

Could you elaborate on this? EVP_VerifyInit_ex and EVP_VerifyUpdate are aliases for EVP_DigestInit_ex and EVP_DigestUpdate, but as far as I know, EVP_VerifyFinal has its own implementation in both OpenSSL 1.0.2 and 1.1.0 (crypto/evp/p_verify.c) which does not allow to specify padding parameters. (It creates a temporary EVP_PKEY_CTX which it does not expose to the caller, making it impossible to use EVP_PKEY_CTX_set_rsa_padding.) Correct me if I am wrong.

Then it should be a matter of calling EVP_PKEY_CTX_set_rsa_padding() with 'RSA_PKCS1_PSS_PADDING' & EVP_PKEY_CTX_set_rsa_pss_saltlen()

@patbaker82 The new API (EVP_Digest*, see crypto/evp/m_sigver.c) supports such parameters, but expects a public key during initialization (EVP_DigestVerifyInit) instead of finalization as the old API (EVP_VerifyFinal, now EVP_DigestVerifyFinal) did. Assuming that it is unacceptable to change the node.js crypto API accordingly, we would need to find a way to avoid specifying the key during initialization. Maybe it is possible to get the EVP_Digest* API to work with an existing message digest?

The only thing I'm a bit unsure of is MGF1.

I am by no means an expert when it comes to RSA, but isn't MGF1 the default MGF for RSASSA-PSS?

@bnoordhuis
Copy link
Member

EVP_VerifyFinal has its own implementation in both OpenSSL 1.0.2 and 1.1.0 (crypto/evp/p_verify.c) which does not allow to specify padding parameters.

You're right, good point.

@tniessen
Copy link
Member

tniessen commented Mar 3, 2017

@bnoordhuis Is it okay to add an optional options argument to Sign.sign and Verify.verify? I guess that wouldn't cause any incompatibilities. I think it does not really make sense to ask for those options before accepting the public/private key as OpenSSL doesn't allow to do so either.

@bnoordhuis
Copy link
Member

@tniessen Yes, I think that would be alright.

tniessen added a commit to tniessen/node that referenced this issue Mar 26, 2017
Adds support for the PSS padding scheme. Until now, the sign/verify
functions used the old EVP_Sign*/EVP_Verify* OpenSSL API, making it
impossible to change the padding scheme. Fixed by first computing the
message digest and then signing/verifying with a custom EVP_PKEY_CTX,
allowing us to specify options such as the padding scheme and the PSS
salt length.

Fixes: nodejs#1127
tniessen added a commit to tniessen/node that referenced this issue Jun 3, 2017
Adds support for the PSS padding scheme. Until now, the sign/verify
functions used the old EVP_Sign*/EVP_Verify* OpenSSL API, making it
impossible to change the padding scheme. Fixed by first computing the
message digest and then signing/verifying with a custom EVP_PKEY_CTX,
allowing us to specify options such as the padding scheme and the PSS
salt length.

Fixes: nodejs#1127
PR-URL: nodejs#11705
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
tniessen added a commit to tniessen/node that referenced this issue Sep 20, 2017
Adds support for the PSS padding scheme. Until now, the sign/verify
functions used the old EVP_Sign*/EVP_Verify* OpenSSL API, making it
impossible to change the padding scheme. Fixed by first computing the
message digest and then signing/verifying with a custom EVP_PKEY_CTX,
allowing us to specify options such as the padding scheme and the PSS
salt length.

Fixes: nodejs#1127
PR-URL: nodejs#11705
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this issue Oct 10, 2017
Adds support for the PSS padding scheme. Until now, the sign/verify
functions used the old EVP_Sign*/EVP_Verify* OpenSSL API, making it
impossible to change the padding scheme. Fixed by first computing the
message digest and then signing/verifying with a custom EVP_PKEY_CTX,
allowing us to specify options such as the padding scheme and the PSS
salt length.

Fixes: #1127
PR-URL: #11705
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this issue Oct 25, 2017
Adds support for the PSS padding scheme. Until now, the sign/verify
functions used the old EVP_Sign*/EVP_Verify* OpenSSL API, making it
impossible to change the padding scheme. Fixed by first computing the
message digest and then signing/verifying with a custom EVP_PKEY_CTX,
allowing us to specify options such as the padding scheme and the PSS
salt length.

Fixes: #1127
PR-URL: #11705
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@Abhishek2kr
Copy link

Abhishek2kr commented Oct 12, 2019

Is there any movement on this? I have a requirement to use SHA256withRSA/PSS but its not supported in node crypto.

Can you help me to use SHA256withRSA/PSS ? @mwain

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. feature request Issues that request new features to be added to Node.js.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants