diff --git a/system/Encryption/Handlers/OpenSSLHandler.php b/system/Encryption/Handlers/OpenSSLHandler.php index 9dca5b90304c..3df802c1b602 100644 --- 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 index 45f9ac2fa383..248436c28799 100644 --- a/system/Encryption/Handlers/SodiumHandler.php +++ b/system/Encryption/Handlers/SodiumHandler.php @@ -43,9 +43,17 @@ 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(); } @@ -53,18 +61,18 @@ public function encrypt(#[SensitiveParameter] $data, #[SensitiveParameter] $para $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,7 +106,7 @@ 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 @@ -98,15 +114,15 @@ public function decrypt($data, #[SensitiveParameter] $params = null) } // 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 index f82e9151cd3c..e883c5b2fbc6 100644 --- 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 index bf30036eb2a4..4e963762b997 100644 --- 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 index 0fabe898719c..85d30224559d 100644 --- 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 index 8c7a200d94eb..27192550ae1d 100644 --- 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 index 461f4d95e0b1..1c77572e3b3b 100644 --- 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 index c522781b4f09..78a050cb7fb0 100644 --- 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 -