Make sure data encrypted with SimpleSAML\Utils\Crypto::aesEncrypt() is also authenticated.

lib/SimpleSAML/Utils/Crypto.php

@@ -13,7 +13,7 @@ class Crypto
     /**
      * Decrypt data using AES-256-CBC and the key provided as a parameter.
      *
-     * @param string $ciphertext The IV and the encrypted data, concatenated.
+     * @param string $ciphertext The HMAC of the encrypted data, the IV used and the encrypted data, concatenated.
      * @param string $secret The secret to use to decrypt the data.
      *
      * @return string The decrypted data.
@@ -31,21 +31,36 @@ class Crypto
             throw new \SimpleSAML_Error_Exception("The openssl PHP module is not loaded.");
         }
 
-        $raw    = defined('OPENSSL_RAW_DATA') ? OPENSSL_RAW_DATA : true;
-        $key    = openssl_digest($secret, 'sha256');
-        $method = 'AES-256-CBC';
-        $ivSize = 16;
-        $iv     = substr($ciphertext, 0, $ivSize);
-        $data   = substr($ciphertext, $ivSize);
+        // derive encryption and authentication keys from the secret
+        $key  = openssl_digest($secret, 'sha512');
+
+        $hmac = mb_substr($ciphertext, 0, 32, '8bit');
+        $iv   = mb_substr($ciphertext, 32, 16, '8bit');
+        $msg  = mb_substr($ciphertext, 48, null, '8bit');
+
+        // authenticate the ciphertext
+        if (self::secureCompare(hash_hmac('sha256', $iv.$msg, substr($key, 64, 64), true), $hmac)) {
+            $plaintext = openssl_decrypt(
+                $msg,
+                'AES-256-CBC',
+                substr($key, 0, 64),
+                defined('OPENSSL_RAW_DATA') ? OPENSSL_RAW_DATA : true,
+                $iv
+            );
+
+            if ($plaintext != false) {
+                return $plaintext;
+            }
+        }
 
-        return openssl_decrypt($data, $method, $key, $raw, $iv);
+        throw new \SimpleSAML_Error_Exception("Failed to decrypt ciphertext.");
     }
 
 
     /**
      * Decrypt data using AES-256-CBC and the system-wide secret salt as key.
      *
-     * @param string $ciphertext The IV used and the encrypted data, concatenated.
+     * @param string $ciphertext The HMAC of the encrypted data, the IV used and the encrypted data, concatenated.
      *
      * @return string The decrypted data.
      * @throws \InvalidArgumentException If $ciphertext is not a string.
@@ -66,7 +81,7 @@ class Crypto
      * @param string $data The data to encrypt.
      * @param string $secret The secret to use to encrypt the data.
      *
-     * @return string The IV and encrypted data concatenated.
+     * @return string An HMAC of the encrypted data, the IV and the encrypted data, concatenated.
      * @throws \InvalidArgumentException If $data is not a string.
      * @throws \SimpleSAML_Error_Exception If the openssl module is not loaded.
      *
@@ -82,13 +97,27 @@ class Crypto
             throw new \SimpleSAML_Error_Exception('The openssl PHP module is not loaded.');
         }
 
-        $raw    = defined('OPENSSL_RAW_DATA') ? OPENSSL_RAW_DATA : true;
-        $key    = openssl_digest($secret, 'sha256');
-        $method = 'AES-256-CBC';
-        $ivSize = 16;
-        $iv     = openssl_random_pseudo_bytes($ivSize);
+        // derive encryption and authentication keys from the secret
+        $key = openssl_digest($secret, 'sha512');
+
+        // generate a random IV
+        $iv = openssl_random_pseudo_bytes(16);
+
+        // encrypt the message
+        $ciphertext = $iv.openssl_encrypt(
+            $data,
+            'AES-256-CBC',
+            substr($key, 0, 64),
+            defined('OPENSSL_RAW_DATA') ? OPENSSL_RAW_DATA : true,
+            $iv
+        );
+
+        if ($ciphertext === false) {
+            throw new \SimpleSAML_Error_Exception("Failed to encrypt plaintext.");
+        }
 
-        return $iv.openssl_encrypt($data, $method, $key, $raw, $iv);
+        // return the ciphertext with proper authentication
+        return hash_hmac('sha256', $ciphertext, substr($key, 64, 64), true).$ciphertext;
     }
 
 
@@ -97,7 +126,7 @@ class Crypto
      *
      * @param string $data The data to encrypt.
      *
-     * @return string The IV and encrypted data concatenated.
+     * @return string An HMAC of the encrypted data, the IV and the encrypted data, concatenated.
      * @throws \InvalidArgumentException If $data is not a string.
      * @throws \SimpleSAML_Error_Exception If the openssl module is not loaded.
      *

tests/lib/SimpleSAML/Utils/CryptoTest.php

@@ -59,7 +59,7 @@ class CryptoTest extends \PHPUnit_Framework_TestCase
         $m->setAccessible(true);
 
         $plaintext = 'SUPER_SECRET_TEXT';
-        $ciphertext = 'NmRkODJlZGE2OTA3YTYwMm9En+KAReUk2z7Xi/b3c39kF/c1n6Vdj/zNARQt+UHU';
+        $ciphertext = 'uR2Yu0r4itInKx91D/l9y/08L5CIQyev9nAr27fh3Sshous4vbXRRcMcjqHDOrquD+2vqLyw7ygnbA9jA9TpB4hLZocvAWcTN8tyO82hiSY=';
         $this->assertEquals($plaintext, $m->invokeArgs(null, array(base64_decode($ciphertext), $secret)));
     }