LibCrypto: RSA improvements and OpenSSL #3225
Conversation
Not currently needed as it cannot fail, but useful for future commits.
The current default is unsafe, but determining a safe value is not easy. Leave it up to the caller to decide.
Make `encrypt`, `decrypt`, `sign` and `verify` return `ErrorOr` for better error propagation.
devgianlu
force-pushed
the
rsa-openssl
branch
5 times, most recently
from
d00e453 to
b605460
Compare
|
CI is finally ok, had to restart it a couple of times because of flaky tests :( |
|
I don't have any comment on the code, but here are the results I got from running Before: After Which is about 12x faster! 🚀 |
Didn't think about doing a benchmark, this is awesome! |
Actually, it's better than that, since the "before" run includes some tests that timed out! |
These methods allow to convert between OpenSSL big numbers and ours.
Replace our slow, possibly incorrect RSA key generation with OpenSSL. This should fix many WPT tests that are timing out because we were too slow at computing keys.
devgianlu
force-pushed
the
rsa-openssl
branch
from
b605460 to
80493b3
Compare
| IntegerType p; | ||
| IntegerType q; | ||
| IntegerType lambda; | ||
| auto ctx = TRY(OpenSSL_PKEY_CTX::wrap(EVP_PKEY_CTX_new_from_name(nullptr, "RSA", nullptr))); |
There was a problem hiding this comment.
🤔 I don't quite understand what the difference between TRY(OpenSSL_FOO::wrap(operation)) and OPENSSL_TRY(operation) is supposed to be, is the first one just making a smart pointer?
There was a problem hiding this comment.
is the first one just making a smart pointer?
Yes, that is something that was discussed in my previous PR: instead of using a bunch of scope guards to free memory we introduced some RAII wrappers. However for stuff that is lightly used, I haven't made wrappers (e.g. params builder).
There was a problem hiding this comment.
🤔 why not OwnPtr with a custom deleter then?
It also feels the wrong way around, I'd expect OpenSSL_FOO::wrap(OPENSSL_TRY(operation))?
There was a problem hiding this comment.
why not OwnPtr with a custom deleter then?
We were unsure whether at some it might be useful to have methods on the wrappers to simplify the most common OpenSSL patterns.
It also feels the wrong way around
I've put it that way so that you cannot misuse it and not call OPENSSL_TRY because it is called internally in ::wrap.
There was a problem hiding this comment.
Okay, thanks for clarifying!
Continuing the saga of improving the crypto suite with OpenSSL, this PR brings some improvements:
PKSystemandRSA::generate_key_pairto returnErrorOrinstead of failing and printing to stderrThe last step allows us to complete more WPT tests by not timing out because we are too slow.