Fix for Symmetric encryption key handling

publish/last.published

@@ -1 +1 @@
-9.23.1
+9.23.2

src/DB/IO.php

@@ -1332,7 +1332,6 @@ class IO
 	 */
 	private function __dbCheckQueryParams(string $query, array $params): bool
 	{
-		// $this->log->debug('DB QUERY PARAMS REGEX', ConvertPlaceholder::REGEX_LOOKUP_PLACEHOLDERS);
 		$placeholder_count = $this->__dbCountQueryParams($query);
 		$params_count = count($params);
 		if ($params_count != $placeholder_count) {

src/DB/Support/ConvertPlaceholder.php

@@ -18,17 +18,20 @@ class ConvertPlaceholder
 	// NOTE some combinations are allowed, but the query will fail before this
 	/** @var string split regex, entries before $ group */
 	private const PATTERN_QUERY_SPLIT =
-		',|' // for ',' mostly in INSERT
-		. '[(<>=]|' // general set for (, <, >, = in any query with any combination
-		. '(?:[\(,]\s*\-\-[\s\w]*)\r?\n|' // a comment that starts after a ( or ,
+		'\?\?|' // UNKNOWN: double ??, is this to avoid something?
+		. '[\(,]|' // for ',' and '(' mostly in INSERT or ANY()
+		. '[<>=]|' // general set for <, >, = in any query with any combination
 		. '\^@|' // text search for start from text with ^@
 		. '\|\||' // concats two elements
 		. '&&|' // array overlap
-		. '\-\|\-|' // range overlap
+		. '\-\|\-|' // range overlap for array
 		. '[^-]-{1}|' // single -, used in JSON too
 		. '->|->>|#>|#>>|@>|<@|@@|@\?|\?{1}|\?\||\?&|#-'; //JSON searches, Array searchs, etc
 	/** @var string the main regex including  the pattern query split */
-	private const PATTERN_ELEMENT = '(?:\'.*?\')?\s*(?:\?\?|' . self::PATTERN_QUERY_SPLIT . ')\s*';
+	private const PATTERN_ELEMENT = '(?:\'.*?\')?\s*(?:' . self::PATTERN_QUERY_SPLIT . ')\s*';
+	/** @var string comment regex
+	 * anything that starts with -- and ends with a line break but any character that is not line break inbetween */
+	private const PATTERN_COMMENT = '(?:\-\-[^\r\n]*?\r?\n)*\s*';
 	/** @var string parts to ignore in the SQL */
 	private const PATTERN_IGNORE =
 		// digit -> ignore
@@ -45,6 +48,7 @@ class ConvertPlaceholder
 	/** @var string replace regex for named (:...) entries */
 	public const REGEX_REPLACE_NAMED = '/'
 		. '(' . self::PATTERN_ELEMENT . ')'
+		. self::PATTERN_COMMENT
 		. '('
 		. self::PATTERN_IGNORE
 		. self::PATTERN_NAMED
@@ -53,6 +57,7 @@ class ConvertPlaceholder
 	/** @var string replace regex for question mark (?) entries */
 	public const REGEX_REPLACE_QUESTION_MARK = '/'
 		. '(' . self::PATTERN_ELEMENT . ')'
+		. self::PATTERN_COMMENT
 		. '('
 		. self::PATTERN_IGNORE
 		. self::PATTERN_QUESTION_MARK
@@ -61,6 +66,7 @@ class ConvertPlaceholder
 	/** @var string replace regex for numbered ($n) entries */
 	public const REGEX_REPLACE_NUMBERED = '/'
 		. '(' . self::PATTERN_ELEMENT . ')'
+		. self::PATTERN_COMMENT
 		. '('
 		. self::PATTERN_IGNORE
 		. self::PATTERN_NUMBERED
@@ -71,6 +77,7 @@ class ConvertPlaceholder
 		// prefix string part, must match towards
 		// seperator for ( = , ? - [and json/jsonb in pg doc section 9.15]
 		. self::PATTERN_ELEMENT
+		. self::PATTERN_COMMENT
 		// match for replace part
 		. '(?:'
 		// ignore parts

src/Security/SymmetricEncryption.php

@@ -49,7 +49,11 @@ class SymmetricEncryption
 	 */
 	public static function getInstance(string|null $key = null): self
 	{
-		if (empty(self::$instance)) {
+		// new if no instsance or key is different
+		if (
+			empty(self::$instance) ||
+			self::$instance->key != $key
+		) {
 			self::$instance = new self($key);
 		}
 		return self::$instance;
@@ -130,7 +134,7 @@ class SymmetricEncryption
 	 */
 	private function encryptData(string $message, ?string $key): string
 	{
-		if (empty($this->key) || $key === null) {
+		if ($key === null) {
 			throw new \UnexpectedValueException('Key not set');
 		}
 		$key = $this->createKey($key);

test/phpunit/DB/CoreLibsDBIOTest.php

@@ -5141,9 +5141,9 @@ final class CoreLibsDBIOTest extends TestCase
 				INSERT INTO table_with_primary_key (
 					row_int, row_numeric, row_varchar, row_varchar_literal
 				) VALUES (
-					-- comment 1
+					-- comment 1 かな
 					$1, $2,
-					-- comment 2
+					-- comment 2 -
 					$3
 					-- comment 3
 					, $4
@@ -5152,6 +5152,23 @@ final class CoreLibsDBIOTest extends TestCase
 				'count' => 4,
 				'convert' => false
 			],
+			'comment in update' => [
+				'query' => <<<SQL
+				UPDATE table_with_primary_key SET
+					row_int =
+					-- COMMENT 1
+					$1,
+					row_numeric =
+					$2 -- COMMENT 2
+					,
+					row_varchar -- COMMENT 3
+					= $3
+				WHERE
+					row_varchar = $4
+				SQL,
+				'count' => 4,
+				'convert' => false,
+			],
 			// Note some are not set
 			'a complete set of possible' => [
 				'query' => <<<SQL
@@ -5168,6 +5185,17 @@ final class CoreLibsDBIOTest extends TestCase
 				SQL,
 				'count' => 12,
 				'convert' => false,
+			],
+			// all the same
+			'all the same numbered' => [
+				'query' => <<<SQL
+				UPDATE table_with_primary_key SET
+					row_int = $1::INT, row_numeric = $1::NUMERIC, row_varchar = $1
+				WHERE
+					row_varchar = $1
+				SQL,
+				'count' => 1,
+				'convert' => false,
 			]
 		];
 	}

test/phpunit/Security/CoreLibsSecuritySymmetricEncryptionTest.php

@@ -56,7 +56,24 @@ final class CoreLibsSecuritySymmetricEncryptionTest extends TestCase
 			$decrypted,
 			'Class call',
 		);
+	}
 
+	/**
+	 * test encrypt/decrypt produce correct output
+	 *
+	 * @covers ::generateRandomKey
+	 * @covers ::encrypt
+	 * @covers ::decrypt
+	 * @dataProvider providerEncryptDecryptSuccess
+	 * @testdox encrypt/decrypt indirect $input must be $expected [$_dataName]
+	 *
+	 * @param  string $input
+	 * @param  string $expected
+	 * @return void
+	 */
+	public function testEncryptDecryptSuccessIndirect(string $input, string $expected): void
+	{
+		$key = CreateKey::generateRandomKey();
 		// test indirect
 		$encrypted = SymmetricEncryption::getInstance($key)->encrypt($input);
 		$decrypted = SymmetricEncryption::getInstance($key)->decrypt($encrypted);
@@ -65,7 +82,24 @@ final class CoreLibsSecuritySymmetricEncryptionTest extends TestCase
 			$decrypted,
 			'Class Instance call',
 		);
+	}
 
+	/**
+	 * test encrypt/decrypt produce correct output
+	 *
+	 * @covers ::generateRandomKey
+	 * @covers ::encrypt
+	 * @covers ::decrypt
+	 * @dataProvider providerEncryptDecryptSuccess
+	 * @testdox encrypt/decrypt static $input must be $expected [$_dataName]
+	 *
+	 * @param  string $input
+	 * @param  string $expected
+	 * @return void
+	 */
+	public function testEncryptDecryptSuccessStatic(string $input, string $expected): void
+	{
+		$key = CreateKey::generateRandomKey();
 		// test static
 		$encrypted = SymmetricEncryption::encryptKey($input, $key);
 		$decrypted = SymmetricEncryption::decryptKey($encrypted, $key);
@@ -114,13 +148,51 @@ final class CoreLibsSecuritySymmetricEncryptionTest extends TestCase
 		$crypt = new SymmetricEncryption($key);
 		$encrypted = $crypt->encrypt($input);
 		$this->expectExceptionMessage($exception_message);
-		$crypt->setKey($key);
+		$crypt->setKey($wrong_key);
 		$crypt->decrypt($encrypted);
+	}
+
+	/**
+	 * Test decryption with wrong key
+	 *
+	 * @covers ::generateRandomKey
+	 * @covers ::encrypt
+	 * @covers ::decrypt
+	 * @dataProvider providerEncryptFailed
+	 * @testdox decrypt indirect with wrong key $input throws $exception_message [$_dataName]
+	 *
+	 * @param  string $input
+	 * @param  string $exception_message
+	 * @return void
+	 */
+	public function testEncryptFailedIndirect(string $input, string $exception_message): void
+	{
+		$key = CreateKey::generateRandomKey();
+		$wrong_key = CreateKey::generateRandomKey();
 
 		// class instance
 		$encrypted = SymmetricEncryption::getInstance($key)->encrypt($input);
 		$this->expectExceptionMessage($exception_message);
 		SymmetricEncryption::getInstance($wrong_key)->decrypt($encrypted);
+	}
+
+	/**
+	 * Test decryption with wrong key
+	 *
+	 * @covers ::generateRandomKey
+	 * @covers ::encrypt
+	 * @covers ::decrypt
+	 * @dataProvider providerEncryptFailed
+	 * @testdox decrypt static with wrong key $input throws $exception_message [$_dataName]
+	 *
+	 * @param  string $input
+	 * @param  string $exception_message
+	 * @return void
+	 */
+	public function testEncryptFailedStatic(string $input, string $exception_message): void
+	{
+		$key = CreateKey::generateRandomKey();
+		$wrong_key = CreateKey::generateRandomKey();
 
 		// class static
 		$encrypted = SymmetricEncryption::encryptKey($input, $key);
@@ -190,6 +262,56 @@ final class CoreLibsSecuritySymmetricEncryptionTest extends TestCase
 		SymmetricEncryption::decryptKey($encrypted, $key);
 	}
 
+	/**
+	 * test invalid key provided to decrypt or encrypt
+	 *
+	 * @covers ::encrypt
+	 * @covers ::decrypt
+	 * @dataProvider providerWrongKey
+	 * @testdox wrong key indirect $key throws $exception_message [$_dataName]
+	 *
+	 * @param  string $key
+	 * @param  string $exception_message
+	 * @return void
+	 */
+	public function testWrongKeyIndirect(string $key, string $exception_message): void
+	{
+		$enc_key = CreateKey::generateRandomKey();
+
+		// 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);
+	}
+
+		/**
+	 * test invalid key provided to decrypt or encrypt
+	 *
+	 * @covers ::encrypt
+	 * @covers ::decrypt
+	 * @dataProvider providerWrongKey
+	 * @testdox wrong key static $key throws $exception_message [$_dataName]
+	 *
+	 * @param  string $key
+	 * @param  string $exception_message
+	 * @return void
+	 */
+	public function testWrongKeyStatic(string $key, string $exception_message): void
+	{
+		$enc_key = CreateKey::generateRandomKey();
+
+		// 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);
+	}
+
 	/**
 	 * Undocumented function
 	 *
@@ -232,6 +354,49 @@ final class CoreLibsSecuritySymmetricEncryptionTest extends TestCase
 		$this->expectExceptionMessage($exception_message);
 		SymmetricEncryption::decryptKey($input, $key);
 	}
+
+	/**
+	 * Undocumented function
+	 *
+	 * @covers ::decrypt
+	 * @dataProvider providerWrongCiphertext
+	 * @testdox too short ciphertext indirect $input throws $exception_message [$_dataName]
+	 *
+	 * @param  string $input
+	 * @param  string $exception_message
+	 * @return void
+	 */
+	public function testWrongCiphertextIndirect(string $input, string $exception_message): void
+	{
+		$key = CreateKey::generateRandomKey();
+
+		// class instance
+		$this->expectExceptionMessage($exception_message);
+		SymmetricEncryption::getInstance($key)->decrypt($input);
+
+		// class static
+		$this->expectExceptionMessage($exception_message);
+		SymmetricEncryption::decryptKey($input, $key);
+	}
+
+		/**
+	 * Undocumented function
+	 *
+	 * @covers ::decrypt
+	 * @dataProvider providerWrongCiphertext
+	 * @testdox too short ciphertext static $input throws $exception_message [$_dataName]
+	 *
+	 * @param  string $input
+	 * @param  string $exception_message
+	 * @return void
+	 */
+	public function testWrongCiphertextStatic(string $input, string $exception_message): void
+	{
+		$key = CreateKey::generateRandomKey();
+		// class static
+		$this->expectExceptionMessage($exception_message);
+		SymmetricEncryption::decryptKey($input, $key);
+	}
 }
 
 // __END__