fix: inconsistent key handling in encryption (#9868)

michalsn · web-flow · commit a46eeee5d2e5 · 2026-01-04T19:14:58.000+01:00
diff --git a/system/Encryption/Handlers/OpenSSLHandler.php b/system/Encryption/Handlers/OpenSSLHandler.php
@@ -83,16 +83,16 @@ class OpenSSLHandler extends BaseHandler
     public function encrypt(#[SensitiveParameter] $data, #[SensitiveParameter] $params = null)
     {
         // Allow key override
-        if ($params !== null) {
-            $this->key = is_array($params) && isset($params['key']) ? $params['key'] : $params;
-        }
+        $key = $params !== null
+            ? (is_array($params) && isset($params['key']) ? $params['key'] : $params)
+            : $this->key;
 
-        if (empty($this->key)) {
+        if (empty($key)) {
             throw EncryptionException::forNeedsStarterKey();
         }
 
         // derive a secret key
-        $encryptKey = \hash_hkdf($this->digest, $this->key, 0, $this->encryptKeyInfo);
+        $encryptKey = \hash_hkdf($this->digest, $key, 0, $this->encryptKeyInfo);
 
         // basic encryption
         $iv = ($ivSize = \openssl_cipher_iv_length($this->cipher)) ? \openssl_random_pseudo_bytes($ivSize) : null;
@@ -106,7 +106,7 @@ public function encrypt(#[SensitiveParameter] $data, #[SensitiveParameter] $para
         $result = $this->rawData ? $iv . $data : base64_encode($iv . $data);
 
         // derive a secret key
-        $authKey = \hash_hkdf($this->digest, $this->key, 0, $this->authKeyInfo);
+        $authKey = \hash_hkdf($this->digest, $key, 0, $this->authKeyInfo);
 
         $hmacKey = \hash_hmac($this->digest, $result, $authKey, $this->rawData);
 
@@ -119,16 +119,16 @@ public function encrypt(#[SensitiveParameter] $data, #[SensitiveParameter] $para
     public function decrypt($data, #[SensitiveParameter] $params = null)
     {
         // Allow key override
-        if ($params !== null) {
-            $this->key = is_array($params) && isset($params['key']) ? $params['key'] : $params;
-        }
+        $key = $params !== null
+            ? (is_array($params) && isset($params['key']) ? $params['key'] : $params)
+            : $this->key;
 
-        if (empty($this->key)) {
+        if (empty($key)) {
             throw EncryptionException::forNeedsStarterKey();
         }
 
         // derive a secret key
-        $authKey = \hash_hkdf($this->digest, $this->key, 0, $this->authKeyInfo);
+        $authKey = \hash_hkdf($this->digest, $key, 0, $this->authKeyInfo);
 
         $hmacLength = $this->rawData
             ? $this->digestSize[$this->digest]
@@ -152,7 +152,7 @@ public function decrypt($data, #[SensitiveParameter] $params = null)
         }
 
         // derive a secret key
-        $encryptKey = \hash_hkdf($this->digest, $this->key, 0, $this->encryptKeyInfo);
+        $encryptKey = \hash_hkdf($this->digest, $key, 0, $this->encryptKeyInfo);
 
         return \openssl_decrypt($data, $this->cipher, $encryptKey, OPENSSL_RAW_DATA, $iv);
     }
diff --git a/system/Encryption/Handlers/SodiumHandler.php b/system/Encryption/Handlers/SodiumHandler.php
@@ -43,28 +43,36 @@ class SodiumHandler extends BaseHandler
      */
     public function encrypt(#[SensitiveParameter] $data, #[SensitiveParameter] $params = null)
     {
-        $this->parseParams($params);
+        // Allow key override
+        $key = $params !== null
+            ? (is_array($params) && isset($params['key']) ? $params['key'] : $params)
+            : $this->key;
 
-        if (empty($this->key)) {
+        // Allow blockSize override
+        $blockSize = (is_array($params) && isset($params['blockSize']))
+            ? $params['blockSize']
+            : $this->blockSize;
+
+        if (empty($key)) {
             throw EncryptionException::forNeedsStarterKey();
         }
 
         // create a nonce for this operation
         $nonce = random_bytes(SODIUM_CRYPTO_SECRETBOX_NONCEBYTES); // 24 bytes
 
         // add padding before we encrypt the data
-        if ($this->blockSize <= 0) {
+        if ($blockSize <= 0) {
             throw EncryptionException::forEncryptionFailed();
         }
 
-        $data = sodium_pad($data, $this->blockSize);
+        $data = sodium_pad($data, $blockSize);
 
         // encrypt message and combine with nonce
-        $ciphertext = $nonce . sodium_crypto_secretbox($data, $nonce, $this->key);
+        $ciphertext = $nonce . sodium_crypto_secretbox($data, $nonce, $key);
 
         // cleanup buffers
         sodium_memzero($data);
-        sodium_memzero($this->key);
+        sodium_memzero($key);
 
         return $ciphertext;
     }
@@ -74,9 +82,17 @@ public function encrypt(#[SensitiveParameter] $data, #[SensitiveParameter] $para
      */
     public function decrypt($data, #[SensitiveParameter] $params = null)
     {
-        $this->parseParams($params);
+        // Allow key override
+        $key = $params !== null
+            ? (is_array($params) && isset($params['key']) ? $params['key'] : $params)
+            : $this->key;
+
+        // Allow blockSize override
+        $blockSize = (is_array($params) && isset($params['blockSize']))
+            ? $params['blockSize']
+            : $this->blockSize;
 
-        if (empty($this->key)) {
+        if (empty($key)) {
             throw EncryptionException::forNeedsStarterKey();
         }
 
@@ -90,23 +106,23 @@ public function decrypt($data, #[SensitiveParameter] $params = null)
         $ciphertext = self::substr($data, SODIUM_CRYPTO_SECRETBOX_NONCEBYTES);
 
         // decrypt data
-        $data = sodium_crypto_secretbox_open($ciphertext, $nonce, $this->key);
+        $data = sodium_crypto_secretbox_open($ciphertext, $nonce, $key);
 
         if ($data === false) {
             // message was tampered in transit
             throw EncryptionException::forAuthenticationFailed(); // @codeCoverageIgnore
         }
 
         // remove extra padding during encryption
-        if ($this->blockSize <= 0) {
+        if ($blockSize <= 0) {
             throw EncryptionException::forAuthenticationFailed();
         }
 
-        $data = sodium_unpad($data, $this->blockSize);
+        $data = sodium_unpad($data, $blockSize);
 
         // cleanup buffers
         sodium_memzero($ciphertext);
-        sodium_memzero($this->key);
+        sodium_memzero($key);
 
         return $data;
     }
@@ -119,6 +135,8 @@ public function decrypt($data, #[SensitiveParameter] $params = null)
      * @return void
      *
      * @throws EncryptionException If key is empty
+     *
+     * @deprecated 4.7.0 No longer used.
      */
     protected function parseParams($params)
     {
diff --git a/tests/system/Encryption/Handlers/OpenSSLHandlerTest.php b/tests/system/Encryption/Handlers/OpenSSLHandlerTest.php
@@ -137,4 +137,27 @@ public function testWithWrongKeyArray(): void
         $key2 = 'Holy cow, batman!';
         $this->assertNotSame($message1, $encrypter->decrypt($encoded, ['key' => $key2]));
     }
+
+    public function testInternalKeyNotModifiedByParams(): void
+    {
+        $params         = new EncryptionConfig();
+        $params->driver = 'OpenSSL';
+        $params->key    = 'original-key-value';
+
+        $encrypter = $this->encryption->initialize($params);
+
+        $this->assertSame('original-key-value', $encrypter->key);
+
+        $message      = 'This is a plain-text message.';
+        $differentKey = 'temporary-param-key';
+        $encoded      = $encrypter->encrypt($message, ['key' => $differentKey]);
+
+        $this->assertSame('original-key-value', $encrypter->key);
+
+        $message2 = 'Another message.';
+        $encoded2 = $encrypter->encrypt($message2);
+        $this->assertSame($message2, $encrypter->decrypt($encoded2));
+
+        $this->assertSame($message, $encrypter->decrypt($encoded, ['key' => $differentKey]));
+    }
 }
diff --git a/tests/system/Encryption/Handlers/SodiumHandlerTest.php b/tests/system/Encryption/Handlers/SodiumHandlerTest.php
@@ -78,14 +78,22 @@ public function testInvalidBlockSizeThrowsErrorOnEncrypt(): void
         $encrypter->encrypt('Some message.');
     }
 
-    public function testEmptyKeyThrowsErrorOnDecrypt(): void
+    public function testHandlerCanBeReusedAfterEncryption(): void
     {
-        $this->expectException(EncryptionException::class);
+        $encrypter = $this->encryption->initialize($this->config);
+        $message   = 'Some message to encrypt';
 
-        $encrypter  = $this->encryption->initialize($this->config);
-        $ciphertext = $encrypter->encrypt('Some message to encrypt');
-        // After encrypt, the message and key are wiped from buffer
-        $encrypter->decrypt($ciphertext);
+        $ciphertext = $encrypter->encrypt($message);
+        $plaintext  = $encrypter->decrypt($ciphertext);
+
+        $this->assertSame($message, $plaintext);
+
+        // Should also work for another encryption
+        $message2    = 'Another message';
+        $ciphertext2 = $encrypter->encrypt($message2);
+        $plaintext2  = $encrypter->decrypt($ciphertext2);
+
+        $this->assertSame($message2, $plaintext2);
     }
 
     public function testInvalidBlockSizeThrowsErrorOnDecrypt(): void
@@ -121,4 +129,26 @@ public function testDecryptingMessages(): void
         $this->assertSame($msg, $encrypter->decrypt($ciphertext, $key));
         $this->assertNotSame('A plain-text message for you.', $encrypter->decrypt($ciphertext, $key));
     }
+
+    public function testInternalKeyNotModifiedByParams(): void
+    {
+        $originalKey = sodium_crypto_secretbox_keygen();
+
+        $this->config->key = $originalKey;
+        $encrypter         = $this->encryption->initialize($this->config);
+
+        $this->assertSame($originalKey, $encrypter->key);
+
+        $message      = 'This is a plain-text message.';
+        $differentKey = sodium_crypto_secretbox_keygen();
+        $encoded      = $encrypter->encrypt($message, ['key' => $differentKey]);
+
+        $this->assertSame($originalKey, $encrypter->key);
+
+        $message2 = 'Another message.';
+        $encoded2 = $encrypter->encrypt($message2);
+        $this->assertSame($message2, $encrypter->decrypt($encoded2, ['key' => $originalKey]));
+
+        $this->assertSame($message, $encrypter->decrypt($encoded, ['key' => $differentKey]));
+    }
 }
diff --git a/user_guide_src/source/changelogs/v4.7.0.rst b/user_guide_src/source/changelogs/v4.7.0.rst
@@ -111,6 +111,61 @@ parameter is ``true``. Previously, properties containing arrays were not recursi
 If you were relying on the old behavior where arrays remained unconverted, you will need to update
 your code.
 
+Encryption Handlers
+-------------------
+
+The ``OpenSSLHandler`` and ``SodiumHandler`` no longer modify the handler's ``$key`` property
+when encryption/decryption parameters are passed via the ``$params`` argument. Keys passed through
+``$params`` are now used as local variables, ensuring the handler's state remains unchanged.
+
+**What changed:**
+
+- Previously, passing a key via ``$params`` to ``encrypt()`` or ``decrypt()`` would permanently
+  modify the handler's internal ``$key`` property.
+- Now, the handler's ``$key`` property is only set during handler creation via ``Config\Encryption``.
+  Passing keys through ``$params`` uses them as temporary local variables without modifying the handler's state.
+- ``SodiumHandler::encrypt()`` no longer calls ``sodium_memzero($this->key)``, which previously
+  destroyed the encryption key after the first use, preventing handler reuse.
+
+**Impact:**
+
+**You are only affected if** you passed a key via ``$params`` to ``encrypt()`` or ``decrypt()``
+and expected that the ``key`` will persist for subsequent operations. Most users are **not affected**:
+
+- **Not affected:** You always pass the key via ``$params`` for each operation
+- **Not affected:** You never use ``$params`` and always configure keys via ``Config\Encryption``
+- **Affected:** You passed a key via ``$params`` once and expected it to be remembered
+
+If affected, configure the key properly via ``Config\Encryption`` or pass a custom config to the
+service instead of relying on ``$params`` side effects.
+
+**Example of affected code:**
+
+.. code-block:: php
+
+    $config      = config('Encryption');
+    $config->key = 'your-encryption-key';
+    $handler     = service('encrypter', $config);
+    $handler->encrypt($data, 'temporary-key');
+    // Old: $handler->key is now 'temporary-key'
+    // New: $handler->key remains unchanged ('your-encryption-key')
+
+    $handler->encrypt($moreData);
+    // Old: Would use 'temporary-key'
+    // New: Uses default key ('your-encryption-key')
+
+**Migration:**
+
+To use a different encryption key permanently, pass a custom config when creating the service:
+
+.. code-block:: php
+
+    $config      = config('Encryption');
+    $config->key = 'your-custom-encryption-key';
+
+    // Get a new handler instance with the custom config (not shared)
+    $handler = service('encrypter', $config, false);
+
 Interface Changes
 =================
 
@@ -254,6 +309,8 @@ Changes
 Deprecations
 ************
 
+- **Encryption:**
+    - The method ``CodeIgniter\Encryption\Handlers\SodiumHandler::parseParams()`` has been deprecated. Parameters are now handled directly in ``encrypt()`` and ``decrypt()`` methods.
 - **Image:**
     - The config property ``Config\Image::libraryPath`` has been deprecated. No longer used.
     - The exception method ``CodeIgniter\Images\Exceptions\ImageException::forInvalidImageLibraryPath`` has been deprecated. No longer used.
diff --git a/user_guide_src/source/libraries/encryption.rst b/user_guide_src/source/libraries/encryption.rst
@@ -223,10 +223,6 @@ sending secret messages in an end-to-end scenario. To encrypt and/or authenticat
 a shared-key, such as symmetric encryption, Sodium uses the XSalsa20 algorithm to encrypt and
 HMAC-SHA512 for the authentication.
 
-.. note:: CodeIgniter's ``SodiumHandler`` uses ``sodium_memzero`` in every encryption or decryption
-    session. After each session, the message (whether plaintext or ciphertext) and starter key are
-    wiped out from the buffers. You may need to provide again the key before starting a new session.
-
 Message Length
 ==============
 
diff --git a/utils/phpstan-baseline/loader.neon b/utils/phpstan-baseline/loader.neon
@@ -1,4 +1,4 @@
-# total 2637 errors
+# total 2639 errors
 
 includes:
     - argument.type.neon
diff --git a/utils/phpstan-baseline/property.notFound.neon b/utils/phpstan-baseline/property.notFound.neon
@@ -1,4 +1,4 @@
-# total 45 errors
+# total 47 errors
 
 parameters:
     ignoreErrors:
@@ -74,7 +74,7 @@ parameters:
 
         -
             message: '#^Access to an undefined property CodeIgniter\\Encryption\\EncrypterInterface\:\:\$key\.$#'
-            count: 2
+            count: 3
             path: ../../tests/system/Encryption/Handlers/OpenSSLHandlerTest.php
 
         -
@@ -89,7 +89,7 @@ parameters:
 
         -
             message: '#^Access to an undefined property CodeIgniter\\Encryption\\EncrypterInterface\:\:\$key\.$#'
-            count: 1
+            count: 2
             path: ../../tests/system/Encryption/Handlers/SodiumHandlerTest.php
 
         -

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-# total 2637 errors`
	`1`	`+# total 2639 errors`
`2`	`2`
`3`	`3`	`includes:`
`4`	`4`	`- argument.type.neon`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-# total 45 errors`
	`1`	`+# total 47 errors`
`2`	`2`
`3`	`3`	`parameters:`
`4`	`4`	`ignoreErrors:`
`@@ -74,7 +74,7 @@ parameters:`
`74`	`74`
`75`	`75`	`-`
`76`	`76`	`message: '#^Access to an undefined property CodeIgniter\\Encryption\\EncrypterInterface\:\:\$key\.$#'`
`77`		`- count: 2`
	`77`	`+ count: 3`
`78`	`78`	`path: ../../tests/system/Encryption/Handlers/OpenSSLHandlerTest.php`
`79`	`79`
`80`	`80`	`-`
`@@ -89,7 +89,7 @@ parameters:`
`89`	`89`
`90`	`90`	`-`
`91`	`91`	`message: '#^Access to an undefined property CodeIgniter\\Encryption\\EncrypterInterface\:\:\$key\.$#'`
`92`		`- count: 1`
	`92`	`+ count: 2`
`93`	`93`	`path: ../../tests/system/Encryption/Handlers/SodiumHandlerTest.php`
`94`	`94`
`95`	`95`	`-`