ring icon indicating copy to clipboard operation
ring copied to clipboard

vendor rust-crypto aes/soft implementation

Open rohhamh opened this issue 1 year ago • 2 comments

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 avatar Apr 10 '24 20:04 rohhamh

Is the PR not relevant anymore? @briansmith

rohhamh avatar Jul 08 '24 06:07 rohhamh

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.

briansmith avatar Jul 09 '24 15:07 briansmith

@rohhamh Thank you for working on this. In https://github.com/RustCrypto/block-ciphers/issues/475 and https://github.com/RustCrypto/block-ciphers/issues/191 I attempted to transfer the knowledge from what we learned here to the RustCrypto team.

briansmith avatar Feb 28 '25 01:02 briansmith

I wish you good luck during your break @briansmith

rohhamh avatar Feb 28 '25 06:02 rohhamh