LibCrypto: RSA encryption/decryption and modes with OpenSSL #3234
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It used to be that the caller would supply a buffer to write the output to. This created an anti-pattern in multiple places where the caller would allocate a `ByteBuffer` and then use `.bytes()` to provide it to the `PKSystem` method. Then the callee would resize the output buffer and reassign it, but because the resize was on `Bytes` and not on `ByteBuffer`, the caller using the latter would cause a bug. Additionally, in pretty much all cases the buffer was pre-allocated shortly before.
|
I took a very brief look over the changes and they seem reasonable (actual review pending though), openssl sure hasn't gotten any nicer to look at. |
devgianlu
force-pushed
the
rework-rsa
branch
2 times, most recently
from
99278ee to
7c4854e
Compare
Add a forwarding header for OpenSSL types so that we can build without propagating the OpenSSL dependency.
This is a small change to allow subclasses of `RSA` to configure the `EVP_PKEY_CTX` without rewriting everything.
This serve as a regression check for the next commits
This commit replaces the old implementation of `EMSA_PKCS1_V1_5` with one backed by OpenSSL. In doing so, the `sign` and `verify` methods of RSA have been modified to behave like expected and not just be encryption and decryption. I was not able to split this commit because the changes to `verify` and `sign` break pretty much everything.
This replaces the old `OAEP` implementation with one backed by OpenSSL. The changes also include some added modularity to the RSA class by making the `RSA_EME` and `RSA_EMSE` for encryption/decryption and signing/verifying respectively.
|
Fixed the macOS build issue. If this got too big to review let me know and I'll make a separate one for the build things. The CI failure is for a flaky test. @alimpfard Can you restart that single job?
I've started to get a feeling for it, the documentation of the single methods is usually pretty decent, but it lacks in explaining how to use the things together. I've been looking at the Once most of the stuff is in place we can work on abstracting the most common patterns into the wrappers. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is where OpenSSL comes to the rescue by letting us replace our own implementation of RSA-PCKS1 and RSA-OAEP with a few lines of code (mind all the framework around it 😄).
The commit separation isn't the best because the new
RSA::verifyandRSA::signmethods are not compatible with the old padding scheme classes so everything had to be replaced at once.