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

Add support for RSA OAEP in openssl crate #1383

Closed
dodomorandi opened this issue Dec 3, 2020 · 3 comments · Fixed by #1384
Closed

Add support for RSA OAEP in openssl crate #1383

dodomorandi opened this issue Dec 3, 2020 · 3 comments · Fixed by #1384

Comments

@dodomorandi
Copy link
Contributor

Josekit heavily relies on openssl crate, but directly implements RSA OAEP encryption/decryption using openssl-sys and, obviously, unsafe code.

It should be considered to implement the same feature directly in openssl crate. If the code does not have any particular issue, it could be implemented without particular efforts and, at the same time, it will avoid others to re-implement the same feature with unsafe code. On the other hand, if there are some concerns about the soundness or security, people involved in rust-openssl have surely a strong know-how to correctly implement the feature and expose a safe and sound abstraction for the ecosystem.

If you want, I could try to send a PR for this, but I am a bit intimidated by crypto issues -- I never worked on this field!

@sfackler
Copy link
Owner

sfackler commented Dec 3, 2020

Sure, happy to take a PR! We can work through any safety issues there as necessary.

@dodomorandi
Copy link
Contributor Author

Let's see if I can understand what should be done and in which way.

I need to implement an encryption and a decryption abstraction layer. If I am not wrong, currently openssl crate has a very good support for signing and verifying RSA, through Signer and Verifier structs. Moreover, RsaRef supports public_encrypt, which relies on RSA_public_encrypt.

However, josekit needs to setup padding and message digests (both available from Signer and Verifier) and then encrypt the data. Instead of relying on RSA_public_encrypt, josekit uses EVP_PKEY_encrypt, which takes an EVP_PKEY_CTX that can be set appropriately before encryption.

As I already said, I am pretty new in this particular field, so please correct my assumptions if I am totally wrong. If my understandings are correct, I am still unsure how the feature from josekit should be appropriately abstracted inside the openssl crate. My takes are:

  • Create Encrypter and Decrypter, similar to Signer and Verifier. I am not sure this is a good idea, because I can smell code duplication just from the beginning.
  • Create something like RsaRef::public_encrypt_with, which takes an extra parameter respect to RsaRef::public_encrypt that contains the information about padding and message digest.
  • Implement a PKey<Public>::public_encrypt, reflecting EVP_PKEY_encrypt, and add auxiliary methods to set padding and message digest to the inner context. This looks a bit C-ish, and I think this could result in a bad and confusing abstraction.

I mostly wrote about the encryption, the same things should apply to decryption as well. Please let me know what do you think, I think I need to better understand the situation and how it should be handled.

@sfackler
Copy link
Owner

sfackler commented Dec 3, 2020

I think adding Encrypter/Decrypter makes sense for now - in the next breaking release I'm planning on looking into a more low-level PkeyCtx that would replace Signer/Verifier/Encrypter/Decrypter in theory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants