Aes-ctr yields different result openssl about stream-ciphers HOT 12 CLOSED

Looks like OpenSSL and aes-ctr use a slightly different implementation of the AES-CTR algorithm. I think OpenSSL uses a honest 128-bit counter, while RustCrypto crates use only 64-bit wrapping counter and other 64-bits are left immutable. During implementation I was not sure which approach is considered "standard", so I've followed the 64-bit counter approach described for example by Intel. Due to SIMD friendliness it's a bit more performant than the 128-bit counter approach. I thought I described this difference in docs somewhere, but looks I forgot to do it.

For random nonces (which is a common recommendation) chances to stumble upon this difference for 1 MB payload is around 4.4e-16. But in your case you use 1aff ffff ffff ffff ffff ffff ffff ffff as your nonce, so you see the difference right away in the second block.

@tarcieri
What do you think we should do here? Implement 128-bit counter, leave 64-bit one or implement both? Currently I lean towards the last option, i.e. in aesni implement only 64-bit variant, but implement 128-bit variant generically in ctr crate and wrap aesni::Aes128/192/256 as part of aes-ctr.

from stream-ciphers.

Thanks for helping me out! To give a little bit of context: I discovered this while writing some differential fuzz testing tool to compare your implementation against openssl. If you change to a 128 bit variant, this could help you to ensure correctness of your implementation.

from stream-ciphers.

@newpavlov implementing both makes sense to me

from stream-ciphers.

I looked into fixing/implementing this. Supporting u64 (current), u128 (like openssl) and u32 from #41 ... Not sure how that should be realized given I know nothing of any design constraints that come with simd (re #41). Any suggestions how this should be implemented?

Simply switching the counter of Ctr128 to u128, fixing some casts makes a modified poc pass. I needed to modify the original attachments (main.rs):

--- poc/src/main.rs     2019-04-23 20:22:12.000000000 +0300
+++ poc1/src/main.rs    2019-12-10 21:16:00.882249954 +0200
@@ -90,7 +90,7 @@
     println!("Nonce generic array: {:?}", nonce_generic);
     println!("Data:\naes-ctr: {:?}\nopenssl: {:?}\ncrypto::aes: {:?}\noriginal_text: {:?}",aes_ctr_crypto_data,openssl_ciphertext,output_crypto_aes,original_data);
     assert_eq!(output_crypto_aes,openssl_ciphertext,"Opensll and rust::crypto::aes not equal!");
-    assert_eq!(crypto_data,openssl_ciphertext, "Openssl and aes_ctr not equal");
+    assert_eq!(aes_ctr_crypto_data, openssl_ciphertext, "Openssl and aes_ctr not equal");
     println!("All equal\n");
     // seek to the keystream beginning and apply it again to the `data` (decrypt)
     cipher.seek(0);

from stream-ciphers.

@koivunej I will open source the fuzzer I used to find this bug asap. Would you be interested in a PR? This could be in addition to the unit-test created by you.

from stream-ciphers.

@koivunej Find the fuzzer here: https://github.com/viniul/diff_fuzz_rust_aes. It should help you verify the correctness of the implementation. Longterm I would recommend integrating the libraries into https://github.com/google/oss-fuzz.

from stream-ciphers.

I didn't add the randomized tests against openssl as I didn't see (or look hard enough to find) any existing tests against other implementations. While these interop tests sound really good in general I did not research if there are any lisensing issues and such. I'll try to take a look at your fuzzer if for nothing else than out of interest.

Let's wait for RustCrypto maintainers on how to proceed.

from stream-ciphers.

The CTR mode presently exposed by the ctr crate is the one used by AES-EAX, AES-SIV, and I believe also AES-CCM (although the aes-ccm crate is not using the ctr crate).

There are also two different kinds of 32-bit counter mode presently being used (impl'd as in-crate private Ctr32 types) in the aes-gcm and aes-gcm-siv crates: a big endian version and a little endian version respectively.

I'd say the 32-bit counter modes are probably more important in terms of usage, and also with consideration to performance.

from stream-ciphers.

Is there any plan to expose some sort of configuration for this. So, for example, users of the library can select endianness and size of the counter? Do any other libraries do this sort of thing? Thanks.

from stream-ciphers.

It's rather tricky because of all of the variants of "AES-CTR"

In the cases of AES-GCM and AES-GCM-SIV (which use two different variants of AES-CTR!), I've vendored the CTR implementations (which both use a 32-bit counter, but with different endianness) into the respective crates:

• https://github.com/RustCrypto/AEADs/blob/master/aes-gcm/src/ctr.rs
• https://github.com/RustCrypto/AEADs/blob/master/aes-gcm-siv/src/ctr.rs

It should be possible to extract a common implementation useful for both into the ctr crate, e.g. generic over a ByteOrder

from stream-ciphers.

Thanks for the response @tarcieri.

from stream-ciphers.

Fixed in #195

from stream-ciphers.