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 encrypt module and RSA OAEP support #1384

Merged
merged 8 commits into from
Dec 13, 2020
Merged

Conversation

dodomorandi
Copy link
Contributor

@dodomorandi dodomorandi commented Dec 4, 2020

Introduce a new module encrypt, with the two structs Encrypter and Decrypter, pretty similar to Signer and Verifier. With the introduction of EVP_PKEY_CTX_set_rsa_oaep_md in openssl-sys, this new abstraction allows to perform RSA OAEP encryption and decryption without directly relying of unsafe calls.

As already discussed with @sfackler, there is a pretty good amount of code duplication, there is a good opportunity to use traits to abstract away sign and encrypt algorithms (i.e. OAEP can only be used for encryption, PSS only for signing). This PR could be a good starting point to analyze the current situation and, maybe, to understand how in the future the abstractions could be improved.

I would like some feedback, mostly because I could have missed some potential problem. Let me know what you think and just tell me what's missing.

Closes #1383

EDIT: I forgot to mention: I copied the API from Signer/Verifier, but I think that having to preallocate a buffer without knowing the exact size needed is pretty inconvenient. IMHO two versions of encrypt and decrypt should be provided, one that returns a Vec<u8>, the other that takes a buffer. I would like to hear your opinion about naming, because if I follow something like encrypt(in: &[u8]) -> Vec<u8> and encrypt_buf(in: &[u8], out: &mut [u8]) (which would be my approach), I will immediately break the API convention of Signer/Verifier. Moreover, I also think that taking a &mut Vec<u8> could be convenient, to allow automatic buffer resizing, but on the other hand some questions would rise: the vec should be automatically cleared if non-empty? Or it should follow the same principles of Read::read_to_end and similar?

P.S.: there is an issue about invalid C compiler parameters, which breaks the CI because -Werror is enabled, you probably already know that.

TODO

  • Module documentation

@sfackler
Copy link
Owner

sfackler commented Dec 5, 2020

It looks like EVP_PKEY_CTRL_RSA_OAEP_MD isn't defined in OpenSSL 1.0.1 and old versions of LibreSSL.

openssl/src/encrypt.rs Outdated Show resolved Hide resolved
@dodomorandi
Copy link
Contributor Author

@sfackler Thank you so much for your review.

It looks like EVP_PKEY_CTRL_RSA_OAEP_MD isn't defined in OpenSSL 1.0.1 and old versions of LibreSSL.

I totally forgot about checking if I needed to use specific cfgs, fixing that.

@dodomorandi
Copy link
Contributor Author

@sfackler I found that using encrypt and decrypt can be counterintuitive, because the size of the output buffer is greater than the output data. I added a bit of documentation to clarify how get an expected behavior.

@dodomorandi dodomorandi marked this pull request as ready for review December 7, 2020 08:53
@sfackler sfackler merged commit 74f380e into sfackler:master Dec 13, 2020
@sfackler
Copy link
Owner

Thanks!

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 this pull request may close these issues.

Add support for RSA OAEP in openssl crate
2 participants