search

briansmith / ring

Safe, fast, small crypto using Rust
Other
3.76k stars 708 forks source link

vendor rust-crypto aes/soft implementation #2024

Open rohhamh opened 7 months ago

rohhamh commented 7 months ago

I've tried following the instructions at https://github.com/briansmith/ring/issues/1886#issue-2072949071

I ran the tests for fixslice64 and they all seem to pass. However I'm very reluctant about some of the changes made here, like increasing rd_key's size to 240 and having u32-u64 conversion functions or using self.inner.rounds to differentiate between the target functions or sending less than four actual blocks for encryption to the vendored code.

This should fail for fixslice32, I would like it if I could have your opinion on how you think it is best to handle fixslice32 as it takes u32 parameters.

The code in ctr32_encrypt_within is basically my attempt at replicating what aes_nohw_ctr32_encrypt_blocks in aes_nohw.c does, I imagine it's accurate as the tests did pass.

There's also the contribution guide. I'm not sure if the statement should be added to every commit message or one would suffice, or how the license would apply to the vendored code.

And finally, this is basically the first time I've done anything in rust or cryptography, so I imagine there would be many ways to improve the code. I'd be very happy to know about them.

rohhamh commented 4 months ago

Is the PR not relevant anymore? @briansmith

briansmith commented 4 months ago

Is the PR not relevant anymore? @briansmith

PR #2070 is somewhat of an alternative to this PR that is more conservative because it doesn't increase the size of the key. I think especially in the case where it is more common to have a hardware implementation, avoiding increasing the key size is important.

In the case where there are vector instructions available, my understanding is that the 128-bit SIMD version of the code in PR #2070 (to be done later) will be faster. So I think that it makes sense to merge PR #2070 and also to port the 128-bit SIMD version from BoringSSL to Rust.

I think this PR is still potentially useful for platforms where we're always going to use the fallback implementation and there are no vector instructions available.

And/or, like I mentioned before, if we can figure out a way to keep the key size the same as it is now, and "expand" the key schedule on-demand during each encryption operation, then this would be useful in the case where vector instructions cannot be used, even as the fallback implementation.