Skip to content

Expose AES-GCM-SIV to Java and Swift #98

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

Merged
merged 3 commits into from
Dec 7, 2020
Merged

Conversation

jack-signal
Copy link
Contributor

One thing I ran into, Android's libc crate doesn't support getauxval. Probably because it was not available in old API versions (added in 18) and Rust's support for weak symbols isn't great. So this PR disables detection of the ARMv8 instructions on Android. Hopefully I can just get getauxval added for Android (rust-lang/libc#1987) otherwise I'll have to do something horrible like parsing /proc/cpuinfo 😭 I missed this problem in my testing as I built for Linux/aarch64 so I could test it under qemu.

I'll do TS in a followup PR on top of #95

Copy link
Contributor

@jrose-signal jrose-signal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect the overhead of heap-allocating the Aes256 instances swamps the benefit of getting to reuse them, and that's even assuming it even makes sense to reuse them (I don't know where this is going to be used). What do you think about making the encrypt and decrypt operations stateless functions that allocate and throw away an Aes256 on the stack each time? Or is it better to do it this way?

@jack-signal
Copy link
Contributor Author

A good question. It will ultimately depend on where and how it is used. But putting the object in the heap allows the key schedule to be cached. On ARMv8 an AES key schedule probably costs between 1-2 thousand cycles, as the extensions there do not support the key schedule. [*] That will be much higher using the aes_soft fallback, which is what is going to be used on ARMv7, old x86, and some Aarch64 Android devices. On my laptop aes_softs key schedule is about 2700 cycles, and a malloc and free of 240 bytes (size of an expanded AES-256 key) is about 1000 cycles.

If the key is only used once, then we pay for the malloc and free each time but with no savings since the key schedule is never reused. If a key is used 2**20 times (say because we start using AES-GCM-SIV in voice calls) it certainly pays off. Somewhere in between is breakeven, depending on the relative cost of malloc vs AES. The one place we currently use AES-GCM-SIV is in groups v2 where the same key is used persistently https://github.com/signalapp/zkgroup/blob/2153e4aad35067881bbb19ef31b151dc481c76cc/rust/src/api/groups/group_params.rs#L171 and generally AES-GCM-SIV is designed precisely for encrypting many messages under a single key and maintaining security far past what would be safe using GCM or ChaCha20Poly1305.

My take would be to leave it as is, and if we find ourselves using AES-GCM-SIV with just one message per key, add a new function for doing one shot encryption/decryption that avoids putting the key schedule on the heap.

[*] You can use the encryption and decryption instructions for certain portions of the key schedule, and we do, but it mostly matters for decryption which AES-GCM-SIV doesn't use. For the encryption key schedule you can use the instructions but only inefficiently (processing a whole SIMD vector to get a single 32-bit result, due to how the AES key schedule works). Using the AES-NI instructions on x86, it is something around 300-500 cycles.

Copy link
Contributor

@jrose-signal jrose-signal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I didn't realize it was that expensive to set up, given that the constructor wasn't parameterized! Okay, this looks good to me, then.

@jack-signal jack-signal merged commit 48ac3ca into master Dec 7, 2020
@jack-signal jack-signal deleted the jack/aes-gcm-siv-ffi branch December 7, 2020 17:46
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.

2 participants