Use more RustCrypto #328
Merged
Use more RustCrypto #328
Conversation
This file contains hidden or 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
jrose-signal
force-pushed
the
jrose/use-more-RustCrypto
branch
from
aa35f0e to
197f05a
Compare
|
Nice side benefit: |
|
Oh, looks like the benefit all came from bumping the toolchain version, and this actually increases the size a little (1,858,608 to 1,870,912). Oh well. |
Now that RustCrypto aes-gcm-siv supports runtime-detected ARMv8 and x86_64 crypto intrinsics, we don't need our own implementation, which will be removed from signal-crypto in a later commit.
Also turn on the AES crate's use of ARMv8 intrinsics
Same as before, but for the wrapper exposed to the app languages.
This was never exposed in Swift, so there's no effective change.
The signal-crypto struct Aes256Ctr32 is still useful because we use a different nonce size than RustCrypto's "full block", and we provide a convenience constructor to specify an initial counter value.
jrose-signal
force-pushed
the
jrose/use-more-RustCrypto
branch
from
8316a22 to
4519eb4
Compare
jim-signal
reviewed
Jul 2, 2021
| @@ -86,7 +86,7 @@ private Native() {} | |||
|
|
|||
| public static native byte[] Aes256GcmSiv_Decrypt(long aesGcmSiv, byte[] ctext, byte[] nonce, byte[] associatedData); | |||
| public static native void Aes256GcmSiv_Destroy(long handle); | |||
| public static native byte[] Aes256GcmSiv_Encrypt(long aesGcmSiv, byte[] ptext, byte[] nonce, byte[] associatedData); | |||
| public static native byte[] Aes256GcmSiv_Encrypt(long aesGcmSivObj, byte[] ptext, byte[] nonce, byte[] associatedData); | |||
There was a problem hiding this comment.
Why the Obj naming here?
There was a problem hiding this comment.
It comes from the Rust source, which renamed a parameter named aes_gcm_siv so that it wouldn't shadow the package name.
jim-signal
approved these changes
Jul 2, 2021
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.
Now that RustCrypto crates support runtime-detected ARMv8 and x86_64 crypto intrinsics, we don't need our own implementations of AES to get good performance.* We still have our own wrapper for AES-256-CTR, and we have a streaming version of AES-256-GCM, but overall this makes the signal-crypto crate a lot smaller.
This is a lot of changes for one PR, but that's because our PR tests ensure that Cargo doesn't include two incompatible versions of a dependency in the output binary. This means that in order to pick up the new version of the
aeadcrate, I had to remove all uses of the old version.*
blocked on RustCrypto/utils#435 getting into a releasedone!