Bad Crypto

Bad Crypto

Paul Moore [here] posted an interesting challenge this morning:

Here is an outline of the encrypt and decrypt code that was shared:

Unfortunately, it’s written in PHP, but it is fairly easy to review. For the encrypt function, we have input data ($data), and then use the required encryption key ($encryption_key), and generate a new salt value ($iv):

In this case, we are using 256-bit AES with CBC (Cipher Block Chaining). Once complete, the function returns a Base64 representation of the encrypted byte array, followed by “::”, the length of the cipher, the HMAC value, “::”, and then the salt value ($iv). We thus have:

Base64(Encrypted_cipher::XX.HHHHHHH::ZZZZZ)

and where XX is the length of the cipher, HHHHHHH is the HMAC, and ZZZZZ is the salt. The HMAC is signed by the encryption key being used, and with an input of the encrypted cipher stream and the salt value. And so the major problem is in the decryption method:

The problem code is highlighted:

if (isset($lenexp[1])) {                                 
 $hmac = str_replace($lenexp[0] . '.', '', $length); 
 $length = $lenexp[0];                              
}                               
else {                                 
  $hmac = hash_hmac('sha256', $encrypted_data . $iv, $encryption_key);                               
}                               

$hashcheck = hash_hmac('sha256', $encrypted_data . $iv, $encryption_key);                                

if ((strlen($encrypted_data) == $length) && ($hmac == $hashcheck)) 
{                                 
  return @openssl_decrypt($encrypted data, 'aes-256-cbc', $encryption_key, 0, $iv);                               
}

Basically an intruder could make sure that isset($lenexp[1]) is False, such as removing the HMAC part of the cipher, and then we will get:

$hmac = hash_hmac('sha256', $encrypted_data . $iv, $encryption_key);

and which will give the same result for the variable $hashcheck:

$hashcheck = hash_hmac('sha256', $encrypted_data . $iv, $encryption_key);

Thus the highlighted value will be true:

if ((strlen($encrypted_data) == $length) && ($hmac == $hashcheck)) 
{                                 
  return @openssl_decrypt($encrypted data, 'aes-256-cbc', $encryption_key, 0, $iv);                               
}

This will cause the cipher to be decrypted, without the requirement for a correct HMAC value.

There are quite a few other problems with the code, especially in the implementation of the HMAC code. I really can’t believe this code was signed off by an experienced tester.