Shorten key ids in exception messages, check decoded bytes in the leak test, fix a README parameter name

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.
 

src/Exceptions/IncompleteKeyPairException.php

@@ -4,14 +4,15 @@
 namespace Spaze\Encryption\Exceptions;
 
 use Exception;
+use Spaze\Encryption\Format\LogSafeValue;
 use Throwable;
 
 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);
 	}
 
 }

src/Exceptions/InvalidKeyEncodingException.php

@@ -4,14 +4,15 @@
 namespace Spaze\Encryption\Exceptions;
 
 use Exception;
+use Spaze\Encryption\Format\LogSafeValue;
 use Throwable;
 
 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);
 	}
 
 }

src/Exceptions/InvalidKeyIdException.php

@@ -4,14 +4,15 @@
 namespace Spaze\Encryption\Exceptions;
 
 use Exception;
+use Spaze\Encryption\Format\LogSafeValue;
 use Throwable;
 
 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);
 	}
 
 }

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);
 	}
 
 }

src/Exceptions/InvalidKeyPrefixException.php

@@ -4,14 +4,15 @@
 namespace Spaze\Encryption\Exceptions;
 
 use Exception;
+use Spaze\Encryption\Format\LogSafeValue;
 use Throwable;
 
 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);
 	}
 
 }

src/Exceptions/InvalidKeyRoleException.php

@@ -5,14 +5,15 @@
 
 use Exception;
 use Spaze\Encryption\Format\AsymmetricKeyRole;
+use Spaze\Encryption\Format\LogSafeValue;
 use Throwable;
 
 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);
 	}
 
 }

src/Exceptions/KeyPairMismatchException.php

@@ -4,14 +4,16 @@
 namespace Spaze\Encryption\Exceptions;
 
 use Exception;
+use Spaze\Encryption\Format\LogSafeValue;
 use Throwable;
 
 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);
 	}
 
 }

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
 {

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) . "...'",
+		);
 	}
 
 

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",
+		);
 	}
 
 

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);
+		}
 	}
 
 
@@ -302,6 +306,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