Wednesday, July 10, 2013

Myself: Using the Same Key to Encrypt Everything

This is one of my own mistakes. I saw that a lot of PHP encryption code is deeply flawed, so I wrote my own PHP encryption class and released it to the world. Little did I know, it was even more flawed than CakePHP's rand() cipher.

The problem was in the way I split the key given by the user into two keys: one for encryption, and the other for authentication. I did this by HMACing a purpose string (either "encryption" or "authentication") with the key. That works, but I screwed up the implementation. I didn't include the user's key in the call to hash_hmac(), and because PHP doesn't do type checking, it blindly accepted the next parameter, boolean "true", as the key.

Because of this mistake, everything was encrypted with the same key. Really bad.

Here's the diff of the fix:

  --- /tmp/Crypto.php   2013-05-02 06:39:28.059745329 -0600  
 +++ source/Crypto.php  2013-05-02 07:05:16.959752519 -0600  
 @@ -116,7 +116,7 @@  
    */  
    public static function CreateSubkey($master, $purpose, $bytes)  
    {  
 -    $source = hash_hmac("sha512", $purpose, true);  
 +    $source = hash_hmac("sha512", $purpose, $master, true);  
      if(strlen($source) < $bytes) {  
        trigger_error("Subkey too big.", E_USER_ERROR);  
        return $source; // fail safe  
 @@ -168,6 +168,16 @@  
        return false;  
      }  
 +    $key = mcrypt_create_iv(16, MCRYPT_DEV_URANDOM);  
 +    $data = "abcdef";  
 +    $ciphertext = Crypto::Encrypt($data, $key);  
 +    $wrong_key = mcrypt_create_iv(16, MCRYPT_DEV_URANDOM);  
 +    if (Crypto::Decrypt($ciphertext, $wrong_key))  
 +    {  
 +      echo "FAIL: Ciphertext decrypts with an incorrect key.";  
 +      return false;  
 +    }  
 +  
      echo "PASS\n";  
      return true;  
    }   

Lessons learned:
  • Always write test suites for your encryption code. Always test that you can't decrypt with the wrong key.

No comments:

Post a Comment