Aes-ctr yields different result openssl

[viniul]

When I provide a certain combination of key, iv and data, I get different results from the openssl and the aes-ctr encryption.

In particular, I used this wrapper:

pub extern crate generic_array;
extern crate crypto;
use aes_ctr::Aes128Ctr;
use aes_ctr::Aes192Ctr;
use aes_ctr::Aes256Ctr;
use aes_ctr::stream_cipher::generic_array::GenericArray;
use aes_ctr::stream_cipher::{
    NewStreamCipher, SyncStreamCipher, SyncStreamCipherSeek
};

use openssl::symm;

use std::fs;

use std::env;

//use crypto::aes::{ctr, KeySize};
//use crypto::symmetriccipher::SynchronousStreamCipher;
use crypto::aes;

use std::vec::from_elem;

fn transform_data(key_size: usize,nonce_size: usize, data: &[u8])
-> Result<(Vec<u8>,Vec<u8>, Vec<u8>),&'static str>
{
    if data.len() < key_size+nonce_size+1 {
        return Err("Data too short");
    }
    let key = data[0..key_size].to_owned();
    let nonce = data[key_size..key_size+nonce_size].to_owned();
    let mut crypto_data = data[key_size+nonce_size..].to_owned();
    Ok((key,nonce,crypto_data))
}

macro_rules! generate_aes_call {
    (128,$key_generic:expr,$nonce_generic:expr) => {
        Aes128Ctr::new(&$key_generic, &$nonce_generic);
    };
    (256,$key_generic:expr,$nonce_generic:expr) => {
        Aes256Ctr::new(&$key_generic, &$nonce_generic);
    }
}

macro_rules! generate_aes_openssl {
    (128) => {
        openssl::symm::Cipher::aes_128_ctr();
    };
    (256) => {
        openssl::symm::Cipher::aes_256_ctr();
    }
}

macro_rules! generate_crypto_aes_call {
    (128,$key:expr,$nonce:expr) => {
        crypto::aes::ctr(crypto::aes::KeySize::KeySize128, $key.as_slice(), $nonce.as_slice());
    };
    (256,$key:expr,$nonce:expr) => {
        crypto::aes::ctr(crypto::aes::KeySize::KeySize256, $key.as_slice(), $nonce.as_slice());
    };
}

fn main(){
    let args: Vec<String> = env::args().collect();
    let data = &fs::read(&args[1]).unwrap();
    let (key, nonce, crypto_data) = match(transform_data((128 / 8), 16, data)){
                Ok((key,nonce,crypto_data)) => (key,nonce,crypto_data),
                Err(err_str) => {println!("Err: {:?}", err_str); return}
            };
    let original_data = crypto_data.to_owned();
    let mut aes_ctr_crypto_data = crypto_data.to_owned();
    let key_generic = GenericArray::from_slice(&key);
    let nonce_generic = GenericArray::from_slice(&nonce);

    let mut cipher = Aes128Ctr::new(&key_generic, &nonce_generic);
    // apply keystream (encrypt)
    cipher.apply_keystream(&mut aes_ctr_crypto_data);
    println!("Keysize: {:?}", 128);
    let openssl_cipher = generate_aes_openssl!(128);
    let openssl_ciphertext = openssl::symm::encrypt(
                        openssl_cipher,
                        &key,
                        Some(&nonce),
                        &original_data).unwrap();
    let mut cipher_crypto_aes = generate_crypto_aes_call!(128,key,nonce);
    let mut output_crypto_aes: Vec<u8> = vec![0; original_data.len()];
    cipher_crypto_aes.process(&original_data, output_crypto_aes.as_mut_slice());
    println!("Key: {:?}",key);
    println!("Nonce: {:?}", nonce);
    println!("Key generic array: {:?}", key_generic);
    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");
    println!("All equal\n");
    // seek to the keystream beginning and apply it again to the `data` (decrypt)
    cipher.seek(0);
    cipher.apply_keystream(&mut aes_ctr_crypto_data);
    assert_eq!(aes_ctr_crypto_data, original_data, "Decrypted data not equal");
}

And caledl it like so:
cargo run debug.crash
(file is attached), which reveals the following difference in encryption results between openssl and aes-ctr is revealed:

aes-ctr: [108, 253, 73, 159, 41, 43, 94, 79, 15, 121, 128, 186, 135, 246, 194, 87, 210, 245, 143, 26, 252, 148, 40, 251, 221, 123, 151, 112, 198, 148, 194, 86, 112, 124, 212, 82, 16, 202, 32, 36, 21, 69, 127, 56, 201, 111, 252, 108, 123, 166, 116, 191, 155, 79, 254, 41, 214, 68, 210, 96, 235, 239, 143, 101, 236, 141, 110, 227, 88, 157, 94, 39, 237, 104, 244, 167, 255, 124, 114, 238, 9, 185, 155, 13, 123, 34, 248, 104, 110, 165, 81, 34, 14, 84, 153, 97, 166, 34, 52, 106, 75, 253]
openssl: [108, 253, 73, 159, 41, 43, 94, 79, 15, 121, 128, 186, 135, 246, 194, 87, 27, 222, 233, 216, 2, 74, 106, 79, 70, 239, 105, 93, 125, 169, 59, 243, 171, 225, 15, 165, 102, 87, 79, 1, 31, 125, 151, 72, 199, 184, 71, 14, 69, 200, 13, 5, 171, 26, 106, 86, 129, 55, 254, 219, 166, 51, 34, 105, 154, 166, 12, 108, 239, 100, 153, 125, 229, 136, 86, 30, 233, 149, 169, 77, 154, 25, 226, 107, 205, 53, 144, 233, 62, 225, 237, 218, 7, 246, 61, 146, 31, 189, 212, 178, 104, 88]

Oddly enough, if I change the nonce in the file, openssl and aes-ctr produce the same result. Is this a bug or am I misusing the library?

Attached is a zip to easily reproduce this and the input the triggers the difference:
poc.zip
Run like so:

cd poc/
cargo run debug.crash

[newpavlov]

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.

[viniul]

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.

[tarcieri]

@newpavlov implementing both makes sense to me

[koivunej]

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);

[viniul]

@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.

[viniul]

@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.

[koivunej]

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.

[tarcieri]

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.

[tcharding]

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.

[tarcieri]

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

[tcharding]

Thanks for the response @tarcieri.

[tarcieri]

Fixed in #195