diff --git a/lib/Cake/Controller/Component/CookieComponent.php b/lib/Cake/Controller/Component/CookieComponent.php index e2831783d..1b7d6dea0 100644 --- a/lib/Cake/Controller/Component/CookieComponent.php +++ b/lib/Cake/Controller/Component/CookieComponent.php @@ -132,7 +132,9 @@ class CookieComponent extends Component { * Type of encryption to use. * * Currently two methods are available: cipher and rijndael - * Defaults to Security::cipher(); + * Defaults to Security::cipher(). Cipher is horribly insecure and only + * the default because of backwards compatibility. In new applications you should + * always change this to 'aes' or 'rijndael'. * * @var string */ @@ -364,10 +366,11 @@ class CookieComponent extends Component { public function type($type = 'cipher') { $availableTypes = array( 'cipher', - 'rijndael' + 'rijndael', + 'aes' ); if (!in_array($type, $availableTypes)) { - trigger_error(__d('cake_dev', 'You must use cipher or rijndael for cookie encryption type'), E_USER_WARNING); + trigger_error(__d('cake_dev', 'You must use cipher, rijndael or aes for cookie encryption type'), E_USER_WARNING); $type = 'cipher'; } $this->_type = $type; @@ -455,12 +458,20 @@ class CookieComponent extends Component { if (is_array($value)) { $value = $this->_implode($value); } - - if ($this->_encrypted === true) { - $type = $this->_type; - $value = "Q2FrZQ==." . base64_encode(Security::$type($value, $this->key, 'encrypt')); + if (!$this->_encrypted) { + return $value; } - return $value; + $prefix = "Q2FrZQ==."; + if ($this->_type === 'rijndael') { + $cipher = Security::rijndael($value, $this->key, 'encrypt'); + } + if ($this->_type === 'cipher') { + $cipher = Security::cipher($value, $this->key); + } + if ($this->_type === 'aes') { + $cipher = Security::encrypt($value, $this->key); + } + return $prefix . base64_encode($cipher); } /** @@ -476,27 +487,40 @@ class CookieComponent extends Component { foreach ((array)$values as $name => $value) { if (is_array($value)) { foreach ($value as $key => $val) { - $pos = strpos($val, 'Q2FrZQ==.'); - $decrypted[$name][$key] = $this->_explode($val); - - if ($pos !== false) { - $val = substr($val, 8); - $decrypted[$name][$key] = $this->_explode(Security::$type(base64_decode($val), $this->key, 'decrypt')); - } + $decrypted[$name][$key] = $this->_decode($val); } } else { - $pos = strpos($value, 'Q2FrZQ==.'); - $decrypted[$name] = $this->_explode($value); - - if ($pos !== false) { - $value = substr($value, 8); - $decrypted[$name] = $this->_explode(Security::$type(base64_decode($value), $this->key, 'decrypt')); - } + $decrypted[$name] = $this->_decode($value); } } return $decrypted; } +/** + * Decodes and decrypts a single value. + * + * @param string $value The value to decode & decrypt. + * @return string Decoded value. + */ + protected function _decode($value) { + $prefix = 'Q2FrZQ==.'; + $pos = strpos($value, $prefix); + if ($pos === false) { + return $this->_explode($value); + } + $value = base64_decode(substr($value, strlen($prefix))); + if ($this->_type === 'rijndael') { + $plain = Security::rijndael($value, $this->key, 'decrypt'); + } + if ($this->_type === 'cipher') { + $plain = Security::cipher($value, $this->key); + } + if ($this->_type === 'aes') { + $plain = Security::decrypt($value, $this->key); + } + return $this->_explode($plain); + } + /** * Implode method to keep keys are multidimensional arrays * diff --git a/lib/Cake/Test/Case/Utility/SecurityTest.php b/lib/Cake/Test/Case/Utility/SecurityTest.php index f1a0e33f8..3fe83b27c 100644 --- a/lib/Cake/Test/Case/Utility/SecurityTest.php +++ b/lib/Cake/Test/Case/Utility/SecurityTest.php @@ -302,4 +302,115 @@ class SecurityTest extends CakeTestCase { Security::rijndael($txt, $key, 'encrypt'); } +/** + * Test encrypt/decrypt. + * + * @return void + */ + public function testEncryptDecrypt() { + $txt = 'The quick brown fox'; + $key = 'This key is longer than 32 bytes long.'; + $result = Security::encrypt($txt, $key); + $this->assertNotEquals($txt, $result, 'Should be encrypted.'); + $this->assertNotEquals($result, Security::encrypt($txt, $key), 'Each result is unique.'); + $this->assertEquals($txt, Security::decrypt($result, $key)); + } + +/** + * Test that changing the key causes decryption to fail. + * + * @return void + */ + public function testDecryptKeyFailure() { + $txt = 'The quick brown fox'; + $key = 'This key is longer than 32 bytes long.'; + $result = Security::encrypt($txt, $key); + + $key = 'Not the same key. This one will fail'; + $this->assertFalse(Security::decrypt($txt, $key), 'Modified key will fail.'); + } + +/** + * Test that decrypt fails when there is an hmac error. + * + * @return void + */ + public function testDecryptHmacFailure() { + $txt = 'The quick brown fox'; + $key = 'This key is quite long and works well.'; + $salt = 'this is a delicious salt!'; + $result = Security::encrypt($txt, $key, $salt); + + // Change one of the bytes in the hmac. + $result[10] = 'x'; + $this->assertFalse(Security::decrypt($result, $key, $salt), 'Modified hmac causes failure.'); + } + +/** + * Test that changing the hmac salt will cause failures. + * + * @return void + */ + public function testDecryptHmacSaltFailure() { + $txt = 'The quick brown fox'; + $key = 'This key is quite long and works well.'; + $salt = 'this is a delicious salt!'; + $result = Security::encrypt($txt, $key, $salt); + + $salt = 'humpty dumpty had a great fall.'; + $this->assertFalse(Security::decrypt($result, $key, $salt), 'Modified salt causes failure.'); + } + +/** + * Test that short keys cause errors + * + * @expectedException CakeException + * @expectedExceptionMessage Invalid key for encrypt(), key must be at least 256 bits (32 bytes) long. + * @return void + */ + public function testEncryptInvalidKey() { + $txt = 'The quick brown fox jumped over the lazy dog.'; + $key = 'this is too short'; + Security::encrypt($txt, $key); + } + +/** + * Test that empty data cause errors + * + * @expectedException CakeException + * @expectedExceptionMessage The data to encrypt cannot be empty. + * @return void + */ + public function testEncryptInvalidData() { + $txt = ''; + $key = 'This is a key that is long enough to be ok.'; + Security::encrypt($txt, $key); + } + +/** + * Test that short keys cause errors + * + * @expectedException CakeException + * @expectedExceptionMessage Invalid key for decrypt(), key must be at least 256 bits (32 bytes) long. + * @return void + */ + public function testDecryptInvalidKey() { + $txt = 'The quick brown fox jumped over the lazy dog.'; + $key = 'this is too short'; + Security::decrypt($txt, $key); + } + +/** + * Test that empty data cause errors + * + * @expectedException CakeException + * @expectedExceptionMessage The data to decrypt cannot be empty. + * @return void + */ + public function testDecryptInvalidData() { + $txt = ''; + $key = 'This is a key that is long enough to be ok.'; + Security::decrypt($txt, $key); + } + } diff --git a/lib/Cake/Utility/Security.php b/lib/Cake/Utility/Security.php index 5df50f923..e9017c195 100644 --- a/lib/Cake/Utility/Security.php +++ b/lib/Cake/Utility/Security.php @@ -289,4 +289,94 @@ class Security { return crypt($password, $salt); } +/** + * Encrypt a value using AES-256. + * + * *Caveat* You cannot properly encrypt/decrypt data with trailing null bytes. + * Any trailing null bytes will be removed on decryption due to how PHP pads messages + * with nulls prior to encryption. + * + * @param string $plain The value to encrypt. + * @param string $key The 256 bit/32 byte key to use as a cipher key. + * @param string $hmacSalt The salt to use for the HMAC process. Leave null to use Security.salt. + * @return string Encrypted data. + * @throws CakeException On invalid data or key. + */ + public static function encrypt($plain, $key, $hmacSalt = null) { + self::_checkKey($key, 'encrypt()'); + if (empty($plain)) { + throw new CakeException(__d('cake_dev', 'The data to encrypt cannot be empty.')); + } + if ($hmacSalt === null) { + $hmacSalt = Configure::read('Security.salt'); + } + + // Generate the encryption and hmac key. + $key = substr(hash('sha256', $key . $hmacSalt), 0, 32); + + $algorithm = MCRYPT_RIJNDAEL_128; + $mode = MCRYPT_MODE_CBC; + + $ivSize = mcrypt_get_iv_size($algorithm, $mode); + $iv = mcrypt_create_iv($ivSize, MCRYPT_DEV_URANDOM); + $ciphertext = $iv . mcrypt_encrypt($algorithm, $key, $plain, $mode, $iv); + $hmac = hash_hmac('sha256', $ciphertext, $key); + return $hmac . $ciphertext; + } + +/** + * Check the encryption key for proper length. + * + * @param string $key + * @param string $method The method the key is being checked for. + * @return void + * @throws CakeException When key length is not 256 bit/32 bytes + */ + protected static function _checkKey($key, $method) { + if (strlen($key) < 32) { + throw new CakeException(__d('cake_dev', 'Invalid key for %s, key must be at least 256 bits (32 bytes) long.', $method)); + } + } + +/** + * Decrypt a value using AES-256. + * + * @param string $cipher The ciphertext to decrypt. + * @param string $key The 256 bit/32 byte key to use as a cipher key. + * @param string $hmacSalt The salt to use for the HMAC process. Leave null to use Security.salt. + * @return string Decrypted data. Any trailing null bytes will be removed. + * @throws CakeException On invalid data or key. + */ + public static function decrypt($cipher, $key, $hmacSalt = null) { + self::_checkKey($key, 'decrypt()'); + if (empty($cipher)) { + throw new CakeException(__d('cake_dev', 'The data to decrypt cannot be empty.')); + } + if ($hmacSalt === null) { + $hmacSalt = Configure::read('Security.salt'); + } + + // Generate the encryption and hmac key. + $key = substr(hash('sha256', $key . $hmacSalt), 0, 32); + + // Split out hmac for comparison + $macSize = 64; + $hmac = substr($cipher, 0, $macSize); + $cipher = substr($cipher, $macSize); + + $compareHmac = hash_hmac('sha256', $cipher, $key); + if ($hmac !== $compareHmac) { + return false; + } + + $algorithm = MCRYPT_RIJNDAEL_128; + $mode = MCRYPT_MODE_CBC; + $ivSize = mcrypt_get_iv_size($algorithm, $mode); + + $iv = substr($cipher, 0, $ivSize); + $cipher = substr($cipher, $ivSize); + $plain = mcrypt_decrypt($algorithm, $key, $cipher, $mode, $iv); + return rtrim($plain, "\0"); + } + }