From 4969e504ecf44d49cd0500e2ebb39b66418da67e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=A0pa=C4=8Dek?= Date: Sat, 1 Aug 2026 02:52:31 +0200 Subject: [PATCH 1/3] Shorten key ids in exception messages the same way markers and stored key ids already are A key pasted into an id slot in the configuration is a real mistake this library already defends against elsewhere: the same shortening that keeps markers and key ids from stored values out of logs now also applies to the constructor exceptions that repeat a configured key id, so such a paste leaks at most the first 20 characters instead of the whole key. Ids of sane length are unaffected, every existing message stays the same. The comment on `LogSafeValue` now names both sources of hostile values, stored data and misconfigured slots. --- src/Exceptions/IncompleteKeyPairException.php | 3 ++- src/Exceptions/InvalidKeyEncodingException.php | 3 ++- src/Exceptions/InvalidKeyIdException.php | 3 ++- src/Exceptions/InvalidKeyLengthException.php | 3 ++- src/Exceptions/InvalidKeyPrefixException.php | 3 ++- src/Exceptions/InvalidKeyRoleException.php | 3 ++- src/Exceptions/KeyPairMismatchException.php | 4 +++- src/Format/LogSafeValue.php | 2 +- tests/AnonymousPublicKeyEncryptionTest.phpt | 14 ++++++++++++++ tests/AuthenticatedPublicKeyEncryptionTest.phpt | 9 +++++++++ tests/SymmetricKeyEncryptionTest.phpt | 17 +++++++++++++++++ 11 files changed, 56 insertions(+), 8 deletions(-) diff --git a/src/Exceptions/IncompleteKeyPairException.php b/src/Exceptions/IncompleteKeyPairException.php index 20756f8..18940b9 100644 --- a/src/Exceptions/IncompleteKeyPairException.php +++ b/src/Exceptions/IncompleteKeyPairException.php @@ -4,6 +4,7 @@ namespace Spaze\Encryption\Exceptions; use Exception; +use Spaze\Encryption\Format\LogSafeValue; use Throwable; class IncompleteKeyPairException extends Exception @@ -11,7 +12,7 @@ class IncompleteKeyPairException extends Exception public function __construct(string $keyId, ?Throwable $previous = null) { - parent::__construct("Key id '{$keyId}' needs both our secret key and the other party's public key", previous: $previous); + parent::__construct("Key id '" . LogSafeValue::from($keyId) . "' needs both our secret key and the other party's public key", previous: $previous); } } diff --git a/src/Exceptions/InvalidKeyEncodingException.php b/src/Exceptions/InvalidKeyEncodingException.php index 3efdca2..a4fbc31 100644 --- a/src/Exceptions/InvalidKeyEncodingException.php +++ b/src/Exceptions/InvalidKeyEncodingException.php @@ -4,6 +4,7 @@ namespace Spaze\Encryption\Exceptions; use Exception; +use Spaze\Encryption\Format\LogSafeValue; use Throwable; class InvalidKeyEncodingException extends Exception @@ -11,7 +12,7 @@ class InvalidKeyEncodingException extends Exception public function __construct(string $id, ?Throwable $previous = null) { - parent::__construct("Key '{$id}' is not a valid hex-encoded string", previous: $previous); + parent::__construct("Key '" . LogSafeValue::from($id) . "' is not a valid hex-encoded string", previous: $previous); } } diff --git a/src/Exceptions/InvalidKeyIdException.php b/src/Exceptions/InvalidKeyIdException.php index 6120364..39b8a05 100644 --- a/src/Exceptions/InvalidKeyIdException.php +++ b/src/Exceptions/InvalidKeyIdException.php @@ -4,6 +4,7 @@ namespace Spaze\Encryption\Exceptions; use Exception; +use Spaze\Encryption\Format\LogSafeValue; use Throwable; class InvalidKeyIdException extends Exception @@ -11,7 +12,7 @@ class InvalidKeyIdException extends Exception public function __construct(string $id, string $keyCipherTextSeparator, ?Throwable $previous = null) { - parent::__construct($id === '' ? 'Key id must not be empty' : "Key id '{$id}' must not contain '{$keyCipherTextSeparator}'", previous: $previous); + parent::__construct($id === '' ? 'Key id must not be empty' : "Key id '" . LogSafeValue::from($id) . "' must not contain '{$keyCipherTextSeparator}'", previous: $previous); } } diff --git a/src/Exceptions/InvalidKeyLengthException.php b/src/Exceptions/InvalidKeyLengthException.php index d3f4637..f74f592 100644 --- a/src/Exceptions/InvalidKeyLengthException.php +++ b/src/Exceptions/InvalidKeyLengthException.php @@ -4,6 +4,7 @@ namespace Spaze\Encryption\Exceptions; use Exception; +use Spaze\Encryption\Format\LogSafeValue; use Throwable; class InvalidKeyLengthException extends Exception @@ -12,7 +13,7 @@ class InvalidKeyLengthException extends Exception public function __construct(string $id, int $actualLength, int $expectedLength, ?Throwable $previous = null) { $expectedHexChars = $expectedLength * 2; - parent::__construct("Key '{$id}' must be {$expectedLength} bytes ({$expectedHexChars} hexadecimal characters) but is {$actualLength} bytes", previous: $previous); + parent::__construct("Key '" . LogSafeValue::from($id) . "' must be {$expectedLength} bytes ({$expectedHexChars} hexadecimal characters) but is {$actualLength} bytes", previous: $previous); } } diff --git a/src/Exceptions/InvalidKeyPrefixException.php b/src/Exceptions/InvalidKeyPrefixException.php index d9e5d82..bdd7d8e 100644 --- a/src/Exceptions/InvalidKeyPrefixException.php +++ b/src/Exceptions/InvalidKeyPrefixException.php @@ -4,6 +4,7 @@ namespace Spaze\Encryption\Exceptions; use Exception; +use Spaze\Encryption\Format\LogSafeValue; use Throwable; class InvalidKeyPrefixException extends Exception @@ -11,7 +12,7 @@ class InvalidKeyPrefixException extends Exception public function __construct(string $id, string $prefix, ?Throwable $previous = null) { - parent::__construct("Key '{$id}' must start with '{$prefix}'", previous: $previous); + parent::__construct("Key '" . LogSafeValue::from($id) . "' must start with '{$prefix}'", previous: $previous); } } diff --git a/src/Exceptions/InvalidKeyRoleException.php b/src/Exceptions/InvalidKeyRoleException.php index fe538cd..3b87ea2 100644 --- a/src/Exceptions/InvalidKeyRoleException.php +++ b/src/Exceptions/InvalidKeyRoleException.php @@ -5,6 +5,7 @@ use Exception; use Spaze\Encryption\Format\AsymmetricKeyRole; +use Spaze\Encryption\Format\LogSafeValue; use Throwable; class InvalidKeyRoleException extends Exception @@ -12,7 +13,7 @@ class InvalidKeyRoleException extends Exception public function __construct(string $id, AsymmetricKeyRole $expectedRole, AsymmetricKeyRole $actualRole, ?Throwable $previous = null) { - parent::__construct("Key '{$id}' is tagged as a {$actualRole->value} key but is used as a {$expectedRole->value} key", previous: $previous); + parent::__construct("Key '" . LogSafeValue::from($id) . "' is tagged as a {$actualRole->value} key but is used as a {$expectedRole->value} key", previous: $previous); } } diff --git a/src/Exceptions/KeyPairMismatchException.php b/src/Exceptions/KeyPairMismatchException.php index 1b9be1b..1816e50 100644 --- a/src/Exceptions/KeyPairMismatchException.php +++ b/src/Exceptions/KeyPairMismatchException.php @@ -4,6 +4,7 @@ namespace Spaze\Encryption\Exceptions; use Exception; +use Spaze\Encryption\Format\LogSafeValue; use Throwable; class KeyPairMismatchException extends Exception @@ -11,7 +12,8 @@ class KeyPairMismatchException extends Exception public function __construct(string $keyId, ?Throwable $previous = null) { - parent::__construct("Public key '{$keyId}' is not the public half of secret key '{$keyId}'", previous: $previous); + $id = LogSafeValue::from($keyId); + parent::__construct("Public key '{$id}' is not the public half of secret key '{$id}'", previous: $previous); } } diff --git a/src/Format/LogSafeValue.php b/src/Format/LogSafeValue.php index f83cd81..85e8659 100644 --- a/src/Format/LogSafeValue.php +++ b/src/Format/LogSafeValue.php @@ -4,7 +4,7 @@ namespace Spaze\Encryption\Format; /** - * @internal Values repeated in exception messages can come from stored data, so they can be anything: keep them short and printable before they hit a log. + * @internal Values repeated in exception messages can come from stored data or from a key pasted into the wrong config slot, so they can be anything: keep them short and printable before they hit a log. */ class LogSafeValue { diff --git a/tests/AnonymousPublicKeyEncryptionTest.phpt b/tests/AnonymousPublicKeyEncryptionTest.phpt index 7bbc905..d550e9d 100644 --- a/tests/AnonymousPublicKeyEncryptionTest.phpt +++ b/tests/AnonymousPublicKeyEncryptionTest.phpt @@ -184,6 +184,20 @@ class AnonymousPublicKeyEncryptionTest extends TestCase KeyPairMismatchException::class, "Public key 'dev1' is not the public half of secret key 'dev1'", ); + // A whole key pasted into the id slot ends up repeated in the message, so only its beginning may show + $keyAsId = self::TRUNCATED_KEY . 'a'; + Assert::exception( + function () use ($keyAsId): void { + new AnonymousPublicKeyEncryption( + [$keyAsId => $this->secretKeys[self::ACTIVE_KEY]], + [$keyAsId => $this->publicKeys[self::INACTIVE_KEY]], + $keyAsId, + self::KEY_PREFIX, + ); + }, + KeyPairMismatchException::class, + "Public key '" . substr($keyAsId, 0, 20) . "...' is not the public half of secret key '" . substr($keyAsId, 0, 20) . "...'", + ); } diff --git a/tests/AuthenticatedPublicKeyEncryptionTest.phpt b/tests/AuthenticatedPublicKeyEncryptionTest.phpt index ead3034..5f26bc0 100644 --- a/tests/AuthenticatedPublicKeyEncryptionTest.phpt +++ b/tests/AuthenticatedPublicKeyEncryptionTest.phpt @@ -339,6 +339,15 @@ class AuthenticatedPublicKeyEncryptionTest extends TestCase IncompleteKeyPairException::class, "Key id '1' needs both our secret key and the other party's public key", ); + // A whole key pasted into the id slot ends up repeated in the message, so only its beginning may show + $keyAsId = self::TRUNCATED_KEY . 'a'; + Assert::exception( + function () use ($keyAsId): void { + new AuthenticatedPublicKeyEncryption([$keyAsId => $this->ourSecretKeys[self::ACTIVE_KEY]], [], $keyAsId, self::KEY_PREFIX); + }, + IncompleteKeyPairException::class, + "Key id '" . substr($keyAsId, 0, 20) . "...' needs both our secret key and the other party's public key", + ); } diff --git a/tests/SymmetricKeyEncryptionTest.phpt b/tests/SymmetricKeyEncryptionTest.phpt index 5194f5f..3637966 100644 --- a/tests/SymmetricKeyEncryptionTest.phpt +++ b/tests/SymmetricKeyEncryptionTest.phpt @@ -302,6 +302,23 @@ class SymmetricKeyEncryptionTest extends TestCase } + public function testConstructorKeyPastedAsKeyIdShortened(): void + { + // A whole key pasted into the id slot ends up repeated in the exception message, + // so the message shows only the beginning of the id, like everywhere a stored value is repeated + $keyAsId = self::TRUNCATED_KEY . 'a'; + $e = Assert::exception( + function () use ($keyAsId): void { + new SymmetricKeyEncryption([$keyAsId => 'garbage'], $keyAsId, self::KEY_PREFIX); + }, + MissingKeyPrefixException::class, + "Key '" . substr($keyAsId, 0, 20) . "...' must start with 'prefix_'", + ); + assert($e instanceof MissingKeyPrefixException); + Assert::notContains(substr($keyAsId, 20), $e->getMessage()); + } + + public function testConstructorNumericKeyId(): void { // PHP casts a numeric key id to an integer, the constructor has to cope with that and not just with strings From d5f992d2aa1eee8cfc7e853e33a446aa67db7ed4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=A0pa=C4=8Dek?= Date: Sat, 1 Aug 2026 02:53:38 +0200 Subject: [PATCH 2/3] Check the decoded key bytes in the symmetric `print_r()` leak test, not just the config strings The object never stores the `prefix_hex` config strings, only the decoded raw bytes wrapped in `HiddenString`, so checking the config strings alone can't fail even if the `HiddenString` wrapping is removed and the raw keys become dumpable. The tests of the two public-key classes already check both forms; the symmetric test now does the same. --- tests/SymmetricKeyEncryptionTest.phpt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/SymmetricKeyEncryptionTest.phpt b/tests/SymmetricKeyEncryptionTest.phpt index 3637966..1163690 100644 --- a/tests/SymmetricKeyEncryptionTest.phpt +++ b/tests/SymmetricKeyEncryptionTest.phpt @@ -276,8 +276,12 @@ class SymmetricKeyEncryptionTest extends TestCase public function testHiddenStringKeys(): void { $object = print_r(new SymmetricKeyEncryption($this->keys, self::ACTIVE_KEY, self::KEY_PREFIX), true); - Assert::notContains($this->keys[self::ACTIVE_KEY], $object); - Assert::notContains($this->keys[self::INACTIVE_KEY], $object); + // The object stores only the decoded bytes, so those are the needles that matter: + // checking just the config strings would pass even if the keys were stored as plain strings + foreach ($this->keys as $key) { + Assert::notContains($key, $object); + Assert::notContains(sodium_hex2bin(substr($key, strlen(self::KEY_PREFIX . '_'))), $object); + } } From 8de0e459c167ce2cd86cd67ab6b6c11b44425b0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=A0pa=C4=8Dek?= Date: Sat, 1 Aug 2026 03:02:07 +0200 Subject: [PATCH 3/3] README: the runtime-call example used a parameter name that the parameters example doesn't define --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 859d6b2..f6d9ebe 100644 --- a/README.md +++ b/README.md @@ -232,7 +232,7 @@ Anything that keeps the keys out of the generated container keeps them out of su ```neon services: encryptionKeys: Your\EncryptionKeys(%encryption.keyFile%) - emailEncryption: Spaze\Encryption\SymmetricKeyEncryption(@encryptionKeys::get('email'), %encryption.activeKeyIds.email%, %encryption.keyPrefixes.email%) + emailEncryption: Spaze\Encryption\SymmetricKeyEncryption(@encryptionKeys::get('email'), %encryption.activeKeyIds.email%, %encryption.prefixes.email%) ``` A file the service reads is a good fit if you want to keep the keys in a file: make it a PHP file that returns an array and OPcache will keep it compiled, so there's no parsing on each request. Whichever way you go, grep the generated container for a key prefix to confirm the keys are no longer in it.