From cc067cc2020ea26436ce7c09d35048cb8a4a3482 Mon Sep 17 00:00:00 2001 From: Clemens Schwaighofer Date: Tue, 17 Dec 2024 15:18:06 +0900 Subject: [PATCH] Update symmetric encryption with compare/get key, empty key test, unset on end All key and messages are set SensitiveParameter type On end, unset the key parameter with sodium mem zero Get/Compare key set methods Additional check on empty key Add missing sodium mem zero for inner function variable clean up --- ...oreLibsSecuritySymmetricEncryptionTest.php | 88 ++++++++-- .../CoreLibs/Security/SymmetricEncryption.php | 154 +++++++++++++----- 2 files changed, 189 insertions(+), 53 deletions(-) diff --git a/4dev/tests/Security/CoreLibsSecuritySymmetricEncryptionTest.php b/4dev/tests/Security/CoreLibsSecuritySymmetricEncryptionTest.php index d7486501..a9e4311e 100644 --- a/4dev/tests/Security/CoreLibsSecuritySymmetricEncryptionTest.php +++ b/4dev/tests/Security/CoreLibsSecuritySymmetricEncryptionTest.php @@ -15,6 +15,56 @@ use CoreLibs\Security\SymmetricEncryption; */ final class CoreLibsSecuritySymmetricEncryptionTest extends TestCase { + /** + * Undocumented function + * + * @covers ::compareKey + * @covers ::getKey + * @testdox Check if init class set key matches to created key + * + * @return void + */ + public function testKeyInitGetCompare(): void + { + $key = CreateKey::generateRandomKey(); + $crypt = new SymmetricEncryption($key); + $this->assertTrue( + $crypt->compareKey($key), + 'set key not equal to original key' + ); + $this->assertEquals( + $key, + $crypt->getKey(), + 'set key returned not equal to original key' + ); + } + + /** + * Undocumented function + * + * @covers ::setKey + * @covers ::compareKey + * @covers ::getKey + * @testdox Check if set key after class init matches to created key + * + * @return void + */ + public function testKeySetGetCompare(): void + { + $key = CreateKey::generateRandomKey(); + $crypt = new SymmetricEncryption(); + $crypt->setKey($key); + $this->assertTrue( + $crypt->compareKey($key), + 'set key not equal to original key' + ); + $this->assertEquals( + $key, + $crypt->getKey(), + 'set key returned not equal to original key' + ); + } + /** * Undocumented function * @@ -216,6 +266,10 @@ final class CoreLibsSecuritySymmetricEncryptionTest extends TestCase 'key' => '1cabd5cba9e042f12522f4ff2de5c31d233b', 'excpetion_message' => 'Key is not the correct size (must be ' ], + 'empty key' => [ + 'key' => '', + 'excpetion_message' => 'Key cannot be empty' + ] ]; } @@ -236,6 +290,9 @@ final class CoreLibsSecuritySymmetricEncryptionTest extends TestCase $enc_key = CreateKey::generateRandomKey(); // class + if (empty($key)) { + $this->expectExceptionMessage($exception_message); + } $crypt = new SymmetricEncryption($key); $this->expectExceptionMessage($exception_message); $crypt->encrypt('test'); @@ -244,22 +301,6 @@ final class CoreLibsSecuritySymmetricEncryptionTest extends TestCase $this->expectExceptionMessage($exception_message); $crypt->setKey($key); $crypt->decrypt($encrypted); - - // class instance - $this->expectExceptionMessage($exception_message); - SymmetricEncryption::getInstance($key)->encrypt('test'); - // we must encrypt valid thing first so we can fail with the wrong key - $encrypted = SymmetricEncryption::getInstance($enc_key)->encrypt('test'); - $this->expectExceptionMessage($exception_message); - SymmetricEncryption::getInstance($key)->decrypt($encrypted); - - // class static - $this->expectExceptionMessage($exception_message); - SymmetricEncryption::encryptKey('test', $key); - // we must encrypt valid thing first so we can fail with the wrong key - $encrypted = SymmetricEncryption::encryptKey('test', $enc_key); - $this->expectExceptionMessage($exception_message); - SymmetricEncryption::decryptKey($encrypted, $key); } /** @@ -397,6 +438,21 @@ final class CoreLibsSecuritySymmetricEncryptionTest extends TestCase $this->expectExceptionMessage($exception_message); SymmetricEncryption::decryptKey($input, $key); } + + /** + * Undocumented function + * + * @covers ::decryptKey + * @covers ::decrypt + * @testdox Test empty encrypted string to decrypt + * + * @return void + */ + public function testEmptyDecryptionString(): void + { + $this->expectExceptionMessage('Encrypted string cannot be empty'); + SymmetricEncryption::decryptKey('', CreateKey::generateRandomKey()); + } } // __END__ diff --git a/www/lib/CoreLibs/Security/SymmetricEncryption.php b/www/lib/CoreLibs/Security/SymmetricEncryption.php index 2f8fb75e..d1dcff3f 100644 --- a/www/lib/CoreLibs/Security/SymmetricEncryption.php +++ b/www/lib/CoreLibs/Security/SymmetricEncryption.php @@ -24,19 +24,19 @@ class SymmetricEncryption /** @var SymmetricEncryption self instance */ private static SymmetricEncryption $instance; - /** @var string bin hex key */ - private string $key = ''; + /** @var ?string bin hex key */ + private ?string $key = null; /** * init class * if key not passed, key must be set with createKey * - * @param string|null|null $key + * @param string|null $key encryption key */ public function __construct( - string|null $key = null + ?string $key = null ) { - if ($key != null) { + if ($key !== null) { $this->setKey($key); } } @@ -45,9 +45,10 @@ class SymmetricEncryption * Returns the singleton self object. * For function wrapper use * + * @param string|null $key encryption key * @return SymmetricEncryption object */ - public static function getInstance(string|null $key = null): self + public static function getInstance(?string $key = null): self { // new if no instsance or key is different if ( @@ -59,6 +60,34 @@ class SymmetricEncryption return self::$instance; } + /** + * clean up + * + * @return void + */ + public function __deconstruct() + { + if (empty($this->key)) { + return; + } + try { + // would set it to null, but we we do not want to make key null + sodium_memzero($this->key); + return; + } catch (SodiumException) { + // empty catch + } + if (is_null($this->key)) { + return; + } + $zero = str_repeat("\0", mb_strlen($this->key, '8bit')); + $this->key = $this->key ^ ( + $zero ^ $this->key + ); + unset($zero); + unset($this->key); + } + /* ************************************************************************ * MARK: PRIVATE * *************************************************************************/ @@ -66,11 +95,16 @@ class SymmetricEncryption /** * create key and check validity * - * @param string $key The key from which the binary key will be created - * @return string Binary key string + * @param ?string $key The key from which the binary key will be created + * @return string Binary key string */ - private function createKey(string $key): string - { + private function createKey( + #[\SensitiveParameter] + ?string $key + ): string { + if (empty($key)) { + throw new \UnexpectedValueException('Key cannot be empty'); + } try { $key = CreateKey::hex2bin($key); } catch (SodiumException $e) { @@ -95,32 +129,38 @@ class SymmetricEncryption * @throws \UnexpectedValueException * @throws \UnexpectedValueException */ - private function decryptData(string $encrypted, ?string $key): string - { - if (empty($key)) { - throw new \UnexpectedValueException('Key not set'); + private function decryptData( + #[\SensitiveParameter] + string $encrypted, + #[\SensitiveParameter] + ?string $key + ): string { + if (empty($encrypted)) { + throw new \UnexpectedValueException('Encrypted string cannot be empty'); } $key = $this->createKey($key); $decoded = base64_decode($encrypted); $nonce = mb_substr($decoded, 0, SODIUM_CRYPTO_SECRETBOX_NONCEBYTES, '8bit'); $ciphertext = mb_substr($decoded, SODIUM_CRYPTO_SECRETBOX_NONCEBYTES, null, '8bit'); - $plain = false; + $plaintext = false; try { - $plain = sodium_crypto_secretbox_open( + $plaintext = sodium_crypto_secretbox_open( $ciphertext, $nonce, $key ); } catch (SodiumException $e) { + sodium_memzero($ciphertext); + sodium_memzero($key); throw new \UnexpectedValueException('Decipher message failed: ' . $e->getMessage()); } - if (!is_string($plain)) { - throw new \UnexpectedValueException('Invalid Key'); - } sodium_memzero($ciphertext); sodium_memzero($key); - return $plain; + if (!is_string($plaintext)) { + throw new \UnexpectedValueException('Invalid Key'); + } + return $plaintext; } /** @@ -128,15 +168,16 @@ class SymmetricEncryption * * @param string $message Message to encrypt * @param ?string $key Mandatory encryption key, will throw exception if empty - * @return string + * @return string Ciphered text * @throws \Exception * @throws \RangeException */ - private function encryptData(string $message, ?string $key): string - { - if ($key === null) { - throw new \UnexpectedValueException('Key not set'); - } + private function encryptData( + #[\SensitiveParameter] + string $message, + #[\SensitiveParameter] + ?string $key + ): string { $key = $this->createKey($key); $nonce = random_bytes(SODIUM_CRYPTO_SECRETBOX_NONCEBYTES); try { @@ -149,6 +190,8 @@ class SymmetricEncryption ) ); } catch (SodiumException $e) { + sodium_memzero($message); + sodium_memzero($key); throw new \UnexpectedValueException("Create encrypted message failed: " . $e->getMessage()); } sodium_memzero($message); @@ -160,19 +203,44 @@ class SymmetricEncryption * MARK: PUBLIC * *************************************************************************/ - /** * set a new key for encryption * * @param string $key * @return void */ - public function setKey(string $key) - { + public function setKey( + #[\SensitiveParameter] + string $key + ) { if (empty($key)) { throw new \UnexpectedValueException('Key cannot be empty'); } $this->key = $key; + sodium_memzero($key); + } + + /** + * Checks if set key is equal to parameter key + * + * @param string $key + * @return bool + */ + public function compareKey( + #[\SensitiveParameter] + string $key + ): bool { + return $key === $this->key; + } + + /** + * returns the current set key, null if not set + * + * @return ?string + */ + public function getKey(): ?string + { + return $this->key; } /** @@ -187,8 +255,12 @@ class SymmetricEncryption * @throws \UnexpectedValueException * @throws \UnexpectedValueException */ - public static function decryptKey(string $encrypted, string $key): string - { + public static function decryptKey( + #[\SensitiveParameter] + string $encrypted, + #[\SensitiveParameter] + string $key + ): string { return self::getInstance()->decryptData($encrypted, $key); } @@ -201,8 +273,10 @@ class SymmetricEncryption * @throws \UnexpectedValueException * @throws \UnexpectedValueException */ - public function decrypt(string $encrypted): string - { + public function decrypt( + #[\SensitiveParameter] + string $encrypted + ): string { return $this->decryptData($encrypted, $this->key); } @@ -216,8 +290,12 @@ class SymmetricEncryption * @throws \Exception * @throws \RangeException */ - public static function encryptKey(string $message, string $key): string - { + public static function encryptKey( + #[\SensitiveParameter] + string $message, + #[\SensitiveParameter] + string $key + ): string { return self::getInstance()->encryptData($message, $key); } @@ -229,8 +307,10 @@ class SymmetricEncryption * @throws \Exception * @throws \RangeException */ - public function encrypt(string $message): string - { + public function encrypt( + #[\SensitiveParameter] + string $message + ): string { return $this->encryptData($message, $this->key); } }