Add API providing basic RSA pubkey encrypt and privkey decrypt #125
Conversation
|
Can one of the admins verify this patch? |
6 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
@swift-server-bot add to allowlist |
|
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. |
|
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. |
|
Is there any update on this getting merged? |
|
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. |
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. |
…eMutableBytes() by nudging the compiler to infer the "buffer" version.
…Done:) by using String.init(unsafeInitializedCapacity:initializingWith:) instead.
|
@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
added
the
🔼 needs-minor-version-bump
… directly. Don't use intermediate Array in BoringSSL implementations.
…seen (underscored name is assumed private by default)
|
@Lukasa I think I addressed everything you mentioned. I did have to add a |
…#125) * Add basic RSA encrypt/decrypt API. * Add test vectors and tests for RSA encryption API * Fix build failures * Fix soundness issue * Remove support for RSA encryption using the unsafe PKCS1 v1.5 padding * Fix deprecation warning about the "pointer" version of Data.withUnsafeMutableBytes() by nudging the compiler to infer the "buffer" version. * Fix deprecation warning about String.init(bytesNoCopy:length:freeWhenDone:) by using String.init(unsafeInitializedCapacity:initializingWith:) instead. * Make _RSA.Encryption.[Public|Private]Key their own types * Remove _RSA.Encryption.RSA[Encrypted|Decrypted]Data and just use Data directly. Don't use intermediate Array in BoringSSL implementations. * Document the message size limits on RSA encrypt/decrypt operations * Mark `_RSA` visible to documentation so the new docs can actually be seen (underscored name is assumed private by default) * Fix generic type parameter shadowing warning (new warning in Swift 5.9) * Fix pre-5.8 build * Un-correct switch case indentation to make it wrong (cherry picked from commit 5ac5632)
…#191) * Add basic RSA encrypt/decrypt API. * Add test vectors and tests for RSA encryption API * Fix build failures * Fix soundness issue * Remove support for RSA encryption using the unsafe PKCS1 v1.5 padding * Fix deprecation warning about the "pointer" version of Data.withUnsafeMutableBytes() by nudging the compiler to infer the "buffer" version. * Fix deprecation warning about String.init(bytesNoCopy:length:freeWhenDone:) by using String.init(unsafeInitializedCapacity:initializingWith:) instead. * Make _RSA.Encryption.[Public|Private]Key their own types * Remove _RSA.Encryption.RSA[Encrypted|Decrypted]Data and just use Data directly. Don't use intermediate Array in BoringSSL implementations. * Document the message size limits on RSA encrypt/decrypt operations * Mark `_RSA` visible to documentation so the new docs can actually be seen (underscored name is assumed private by default) * Fix generic type parameter shadowing warning (new warning in Swift 5.9) * Fix pre-5.8 build * Un-correct switch case indentation to make it wrong (cherry picked from commit 5ac5632) Co-authored-by: Gwynne Raskind <gwynne@darkrainfall.org>
Add API to
_CryptoExtrasto provide basic RSA pubkey encrypt and privkey decrypt operations.Closes #124.
Checklist
Motivation:
See #124 for details.
Modifications:
Added two new APIs to the
RSAsupport under a newRSA.Encryptionnamespace, 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.