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