swift-crypto
swift-crypto copied to clipboard
apple
Add API providing basic RSA pubkey encrypt and privkey decrypt
Add API to _CryptoExtras to provide basic RSA pubkey encrypt and privkey decrypt operations.
Closes #124.
Checklist
- [x] I've run tests to see all new and existing tests pass
- [x] I've followed the code style of the rest of the project
- [x] I've read the Contribution Guidelines
- [x] I've updated the documentation if necessary
Motivation:
See #124 for details.
Modifications:
Added two new APIs to the RSA support under a new RSA.Encryption namespace, with associated data types necessary for representing the appropriate inputs and outputs. Added new test vectors and associated test cases exercising the new API surface.
Note: The extent of the exposed API has been deliberately limited to the bare minimum, as it is not desirable to expose more of RSA's core primitives than absolutely necessary.
Thanks for this PR @gwynne. For now I'm going to circulate this with some colleagues and get back to you.
There is no option to set the algorithm (only SHA1 is available). It would be nice to add this parameter. Both security and BoringSSL support that. For setting alg in BoringSSL we can use
EVP_PKEY_CTX_set_rsa_oaep_md
EVP_PKEY_CTX_set_rsa_mgf1_md
Is this something that could be merged any time soon? I've just stumbled upon this while trying to sign a JWT for a GitHub app that I could use RSA support from Crypto. ... Ah but this PR does not include "signing" correct?
RSA signing is already merged.
🙈 Thanks for the pointer! Looked for it in the wrong place.
Currently, no. It remains a source of real tension including a PKCS1v1.5 decrypt operation, and certainly as implemented here we wouldn't want to merge it, because it's not possible to use it safely (no constant-time padding check and no appropriate mitigation path). Marking the PKCS1v1.5 padding insecure is helping, but less than ideal, especially as the API does not provide a mechanism by which users could hold it safely. If we can find a way to not needing it at all, that would be supremely helpful to getting this merged.
@Lukasa If the PKCS1v1.5 padding were removed and we were left with only PKCS1_OAEP padding, would that open a path forward for merging this PR? Or are there safety concerns when using OAEP as well?
OAEP does not have a safety concern to the same degree. However, I'm open to renegotiating the question of marking the PKCS 1v1.5 padding insecure in this context if @gwynne is willing to update the PR.
OAEP does not have a safety concern to the same degree. However, I'm open to renegotiating the question of marking the PKCS 1v1.5 padding insecure in this context if @gwynne is willing to update the PR.
I'm willing to just remove support for the PKCS1 v1.5 padding altogether. It's not needed for my purposes in any event. I'll rebase the PR against the current tip of trunk and take the insecure padding out.
@Lukasa Fully rebased, PKCS1 v1.5 padding support removed. Also fixed two deprecation warnings in as minimal a fashion as I could manage. All tests passing on Linux in my local Docker environment. (There's 6 failing tests related to unsupported non-power-of-2 key sizes on macOS, but it seems unrelated to my changes.)
@Lukasa I think I addressed everything you mentioned. I did have to add a @_documentation(visibility: public) attribute to the _RSA type to make the docs visible.