| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
@tniessen build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/3084/pipeline |
Sorry, something went wrong.
There was a problem hiding this comment.
What's the use case for this? rsa-pss keys can only come from X.509 SubjectPublicKeyInfo, AFAICT, so its not clear to me where node could get them from until we support extraction of public keys from certs, anyhow, so I guess this is prep for that? I don't think jamming "sign" parameters into a "key" is great from an API usage point of view, but since this is mirroring openssl, I guess we have to go along with it. The docs might recommend against using these key types unless required for interop. I'm puzzled by EVP_PKEY_RSA2, that's a pretty opaque name for an OpenSSL key type :-(. I'm not sure any of these concerns are addressable, given OpenSSL is our crypto lib. I left a few small change requests/reminders, but basically LGTM. I'll approve once its updated.
Sorry, something went wrong.
Let me ask a different question: What is the alternative to this? OpenSSL supports RSA-PSS since 1.1.1, and so does node (kind of). One can already load RSA-PSS keys, except that node will crash on asymmetricKeyType and that signing and verification don't work as expected.
You can also load RSA-PSS keys just like any other RSA key from a file, see the test scripts.
I assume the motivation behind this is to prevent people and applications from using the key for anything other than the algorithm it was meant for, and, if required, with the exact parameters it was meant for. Using the same key for different algorithms is often considered bad practice. WebCrypto restricts all keys to specific parameterized algorithms IIRC.
I don't know about OpenSSL and their motivation, but I don't see many alternatives for them either. I think OpenSSL does exactly what it should do when given an RSA-PSS key.
I did not add anything related to EVP_PKEY_RSA2, the code already existed, it's just an alternative OID for "normal" RSA, see #26960 (comment). The key type for RSA-PSS is EVP_PKEY_RSA_PSS.
I think this is similar to ed25519 and X25519. They are essentially the same thing (except encoded differently), but they are used for different algorithms. We could use the same key type as we do for EVP_PKEY_RSA ('rsa'), but:
I think these downsides justify a separate key type, but feel free to disagree. Maybe I'm wrong and these implications aren't true! |
Sorry, something went wrong.
+1 for keeping the two separate key types. |
Sorry, something went wrong.
|
OK, that background helps. Particularly your point about the non-x.509 transport of these kinds of keys in PEM (PKCS8 I assume). So, I've no objections. I'm also, a little, heartened that the number of EVP_PKEY types isn't endless, so the list won't grow too much larger. Still a couple nits with the docs, but otherwise LGTM. |
Sorry, something went wrong.
I think it's getting too long too quickly, but I don't think there is anything we can do about it. On the brighter side, OpenSSL doesn't support much more right now, and as long as the rest of our APIs do not depend on the exact key type, we should be fine.
Thank you for reviewing so quickly, I'll try to get to it ASAP. |
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
This commit adds support for RSA-PSS keys, including - KeyObjects of type rsa-pss, - key pair generation for RSA-PSS, and - signing and verification using RSA-PSS keys.
Sorry, something went wrong.
|
Ping @nodejs/crypto |
Sorry, something went wrong.
This commit adds support for RSA-PSS keys, including - KeyObjects of type rsa-pss, - key pair generation for RSA-PSS, and - signing and verification using RSA-PSS keys. PR-URL: #26960 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
|
Thank you, Daniel! |
Sorry, something went wrong.
These functions are used by Node.js in nodejs/node#26960. BoringSSL does not support EVP_PKEY_RSA_PSS keys, so they always fail. This simplifies building Node with BoringSSL. Change-Id: I81c4cdba8791a60d965bc176d09e5c818153860c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/38524 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>
| Back | FazBrowse Home | New Git URL |
This commit adds support for RSA-PSS keys, including
cc @nodejs/crypto
Checklist