AES encryption wrapper

Question

After taking into account answers for my questions here and here I created (well may-be) improved version of my wrapper. The key issue was what if an attacker is knowing what is encoded - he might then find the key and encode another messages. So I added XOR before encryption. I also in this version prepend IV to the data as was suggested.

SHA256 on key is only for making sure the key is as long as needed for the AES algorithm, but I know that key should not be plain text but calculated with many iterations to prevent dictionary attack

function aes192ctr_en($data,$key) {
    $iv = mcrypt_create_iv(24,MCRYPT_DEV_URANDOM);
    $xor = mcrypt_create_iv(24,MCRYPT_DEV_URANDOM);
    $key = hash_hmac('sha256',$key,$iv,true);
    $data = $xor.((string)$data ^ (string)str_repeat($xor,(strlen($data)/24)+1));
    $data = hash('md5',$data,true).$data;
    return $iv.mcrypt_encrypt('rijndael-192',$key,$data,'ctr',$iv);
}

function aes192ctr_de($data,$key) {
    $iv = substr($data,0,24);
    $data = substr($data,24);
    $key = hash_hmac('sha256',$key,$iv,true);
    $data = mcrypt_decrypt('rijndael-192',$key,$data,'ctr',$iv);
    $md5 = substr($data,0,16);
    $data = substr($data,16);
    if (hash('md5',$data,true)!==$md5) return false;
    $xor = substr($data,0,24);
    $data = substr($data,24);
    $data = ((string)$data ^ (string)str_repeat($xor,(strlen($data)/24)+1));
    return $data;
}

$encrypted = aes192ctr_en('secret text','password');
echo $encrypted;
echo aes192ctr_de($encrypted,'password');

Another question is: Is CTR mode is okay in this context, would it be better if I use CBC mode instead?

Again, by safe I mean, could an attacker derive the password if he knows which exact text was encrypted, and knows above method. I assume a random and long password here.

Maybe instead of XOR, will it be safer to use random initial data with another run of AES or other simpler algorithm like TEA or trivium?

Answer 1

Your XOR-step is nonsense, it doesn't add any security.

The reason is the way the CTR mode works: It encrypts a counter (thus the name) using the key (and the IV as starting point), and XORs the plaintext with the result. XORing the plaintext with some other constant before encryption can be undone by XORing it with the same value after encryption.

Still, CTR mode (with or without this XOR) should be enough for protection, as long as your IV is non-repeating. To break it with some known plaintext, you basically have to brute-force encrypt the counter with different keys, until you get the same output as plaintext XOR ciphertext (for one block).

Also, you are using a simple MD5 hash for integrity protection. While MD5 might be fine in this case, it now has the reputation of being broken (i.e. there are quite easy collision attacks), which makes it not safe. Use a MAC algorithm for integrity protection, for example with hash_hmac (and a strong hash function like sha256, which you used before).

Other than that, PHP's rijndael-192 is not one of the AES algorithms. AES is Rijndael with block size of 128 bits (and key sizes of 128, 192 or 256 bits), and you are using an 192-bit block size version here. (This does not necessarily mean it is less secure, you just shouldn't call it AES. It did receive less cryptoanalysis than the variant that got standardized as AES, though.)