Mark new symmetric output with a format marker and verify the key id during decryption

README.md

@@ -48,11 +48,15 @@ $encryption = new Spaze\Encryption\SymmetricKeyEncryption($keys, $activeKeyId, $
 ```php
 Spaze\Encryption\SymmetricKeyEncryption::encrypt(string $data): string
 ```
-The output will be formatted as `$<keyId>$<base64 ciphertext>`, for example `$key2$MUI...`, where `<keyId>` (`key2`) is the active key id set in the constructor. Store the whole value, don't parse it.
+The output will be formatted as `$<keyId>$SymV1$<base64 ciphertext>`, for example `$key2$SymV1$MUI...`, where `<keyId>` (`key2`) is the active key id set in the constructor. Store the whole value, don't parse it.
 
-The key id in the output is a hint that selects the decryption key. It is the only part of the output not protected against tampering, even by `encryptWithAd()`: changing the encrypted part makes decryption fail, while changing the key id just makes decryption try a different key, and fail because the key is different. Never configure the same key under two different ids.
+The marker between the key id and the encrypted part says what created the value: feeding an `encryptWithAd()` value (marked `SymAdV1`) to `decrypt()`, or a value from a different class to this one, fails with an exception that says what to call instead. The markers can never change; a future format change would introduce new marker values, so the digit works as a format version.
 
-This method does not use any context binding (Additional Authenticated Data). Use `encryptWithAd()` if you want to bind the ciphertext to a specific context.
+The key id and the marker are protected against tampering: both go into what decryption verifies, so changing either of them in a stored value makes decryption fail. (The verified value is `{"keyId":"<base64>","marker":"<the marker from the stored value>"}` — so `SymV1` or `SymAdV1` — with an `"additionalData":"<base64>"` member added by `encryptWithAd()`; Base64 being the URL-safe kind with padding. This only matters if you ever need to decrypt the data with Halite directly, without this library.)
+
+Values in the older format without the marker, `$<keyId>$<base64 ciphertext>`, written by previous versions, still decrypt. Their key id is not protected against tampering though: changing it just makes decryption try a different key, and fail because the key is different — so for them, never configure the same key under two different ids. Versions without marker support cannot read the marked values, so when multiple deployments share the data, upgrade all of them before writing anything new.
+
+This method does not bind the ciphertext to a context of your own. Use `encryptWithAd()` if you want that.
 
 Example:
 ```php
@@ -100,9 +104,11 @@ Unless you remove the old key, it will be possible to decrypt data encrypted wit
 You can then take all the data encrypted with the old key and re-encrypt them just to change the key which was used to encrypt them.
 Once done you can delete the old key.
 
-You can use `needsReEncrypt($ciphertext): bool` to see if the data is encrypted with an inactive key and thus should be re-encrypted with the currently active one.
+You can use `needsReEncrypt($ciphertext): bool` to see if the data is encrypted with an inactive key and thus should be re-encrypted with the currently active one. It also returns true for values stored in the [older format](#encrypt) without the marker, so the same re-encryption sweep migrates them to the marked format.
+
+Values created by `encryptWithAd()` have to be re-encrypted with `decryptWithAd()` and `encryptWithAd()`, using the same additional data the row was encrypted with. Marked values say which method created them, values in the older format don't, so a sweep over old data has to know on its own which rows are context-bound and what their context is.
 
-When rotating, always generate a fresh key for the new key id. The key id in the encrypted output is not protected against tampering (see [Encrypt](#encrypt)), so two different key ids must never point to the same key.
+When rotating, always generate a fresh key for the new key id. In values written in the older format the key id is not protected against tampering (see [Encrypt](#encrypt)), so two different key ids must never point to the same key.
 
 ## Encryption between two parties
 
@@ -150,9 +156,9 @@ The methods are the same as in `SymmetricKeyEncryption`: `encrypt()`, `decrypt()
 
 The output looks like `$<keyId>$AuthV1$<base64 ciphertext>`, or `$<keyId>$AuthAdV1$<...>` when created by `encryptWithAd()`. The marker between the key id and the encrypted part says what created the value: feeding an `encryptWithAd()` value to `decrypt()`, or a value from a different class to this one, fails with an exception that says what to call instead. The markers can never change; a future format change would introduce new marker values, so the digit works as a format version.
 
-Unlike in `SymmetricKeyEncryption`, the key id and the marker are protected against tampering: both go into what decryption verifies, so changing either of them in a stored value makes decryption fail. (The verified value is `{"keyId":"<base64>","marker":"<the marker from the stored value>"}` — so `AuthV1` or `AuthAdV1` — with an `"additionalData":"<base64>"` member added by `encryptWithAd()`; Base64 being the URL-safe kind with padding. This only matters if you ever need to decrypt the data with Halite directly, without this library.)
+Like in `SymmetricKeyEncryption`, the key id and the marker are protected against tampering: both go into what decryption verifies, so changing either of them in a stored value makes decryption fail. (The verified value is built [the same way as in `SymmetricKeyEncryption`](#encrypt), with `AuthV1` or `AuthAdV1` as the marker.)
 
-Values in the older format without the marker — for example written by a previous library that used the same format — still decrypt, though their key id keeps the [old caveat](#encrypt), and `needsReEncrypt()` returns true for them, so a usual re-encryption sweep migrates them to the marked format.
+Values in the older format without the marker — for example written by a previous library that used the same format — still decrypt, though their key id is not protected against tampering, and `needsReEncrypt()` returns true for them, so a usual re-encryption sweep migrates them to the marked format.
 
 One thing deserves a special mention: a configuration with the two keys accidentally swapped can still encrypt and decrypt its own data just fine, only the data from the other party will fail to decrypt. When setting up, always verify by decrypting a value the other party encrypted, not one you encrypted yourself.
 
@@ -191,12 +197,12 @@ $encryption = new Spaze\Encryption\AnonymousPublicKeyEncryption(['key1' => 'adek
 ### Encrypt & decrypt
 `encrypt()`, `decrypt()` and `needsReEncrypt()` work like in the other two classes, but there are no `encryptWithAd()`/`decryptWithAd()` methods, this flavor cannot bind the encrypted value to a context.
 
-The output looks like `$<keyId>$AnonV1$<base64 ciphertext>`, where `AnonV1` is the marker saying what created the value — a value from a different class fails with an exception that names its creator. Values in the older format without the marker still decrypt, and `needsReEncrypt()` returns true for them, so a re-encryption sweep migrates them to the marked format. Unlike in `AuthenticatedPublicKeyEncryption`, the key id and the marker are not protected against tampering here — a sealed value has no place to verify them, so the [old caveat](#encrypt) stays.
+The output looks like `$<keyId>$AnonV1$<base64 ciphertext>`, where `AnonV1` is the marker saying what created the value — a value from a different class fails with an exception that names its creator. Values in the older format without the marker still decrypt, and `needsReEncrypt()` returns true for them, so a re-encryption sweep migrates them to the marked format. Unlike in the other two classes, the key id and the marker are not protected against tampering here — a sealed value has no place to verify them, so changing the key id just makes decryption try a different key, and never configure the same key under two different ids.
 
 Trying to decrypt a key id that only has a public key configured throws `MissingSecretKeyException`, which usually means the code runs on an encrypt-only deployment. Re-encryption after a key rotation therefore has to run where the secret keys live: configure the old secret key and the new public key (or the new pair), and the data encrypted with the old key can be decrypted and re-encrypted with the new one.
 
 ### When decryption fails
-Values with a marker say what created them: the two public-key classes refuse each other's marked values with an exception that names the creator, and `SymmetricKeyEncryption` rejects any marked value as a format error, so mixed-up values are easy to diagnose. The detective work below is only needed for values in the older format without the marker.
+Values with a marker say what created them: every class refuses another class's marked values with an exception that names the creator, so mixed-up values are easy to diagnose. The detective work below is only needed for values in the older format without the marker.
 
 Halite reports any well-formed unmarked value that `AnonymousPublicKeyEncryption` cannot decrypt as `InvalidKey: Incorrect secret key for this sealed message`: a wrong key, corrupted data, and data that was actually created by `SymmetricKeyEncryption` or `AuthenticatedPublicKeyEncryption` all look the same. Only a value that is not even valid base64 gets a different error, `InvalidMessage: Invalid character encoding`. If you see the wrong-key error on data that should be fine, check which class created the value: `SymmetricKeyEncryption` and `AuthenticatedPublicKeyEncryption` fail with `InvalidMessage` when fed each other's data or data created by `AnonymousPublicKeyEncryption`. Their encrypted part also always starts with `MUI` — the beginning of a header Halite adds to the output of those two classes, with the next characters changing with the Halite version — while the encrypted part made by `AnonymousPublicKeyEncryption` has no header and looks random. For unmarked values the key id is the only reliable way to tell them apart, so don't reuse a key id across classes.
 

src/AnonymousPublicKeyEncryption.php

@@ -98,7 +98,7 @@ public function encrypt(#[SensitiveParameter] string $data): string
 	{
 		// The constructor guarantees the active key id has a public key, configured or derived
 		$cipherText = Crypto::seal(new HiddenString($data), new EncryptionPublicKey($this->publicKeys[$this->activeKeyId]));
-		return $this->formatMarkedKeyCipherText($this->activeKeyId, FormatMarker::AnonymousPublicKeyV1, $cipherText);
+		return $this->formatKeyCipherText($this->activeKeyId, FormatMarker::AnonymousPublicKeyV1, $cipherText);
 	}
 
 
@@ -120,7 +120,7 @@ public function encrypt(#[SensitiveParameter] string $data): string
 	 */
 	public function decrypt(string $data): string
 	{
-		[$keyId, $marker, $cipherText] = $this->parseMarkedKeyCipherText($data);
+		[$keyId, $marker, $cipherText] = $this->parseKeyCipherText($data);
 		$this->checkFormatMarker($marker, FormatMarker::AnonymousPublicKeyV1);
 		return Crypto::unseal($cipherText, $this->getSecretKey($keyId))->getString();
 	}

src/AuthenticatedPublicKeyEncryption.php

@@ -108,7 +108,7 @@ public function encrypt(#[SensitiveParameter] string $data): string
 		[$secretKey, $publicKey] = $this->getKeyPair($this->activeKeyId);
 		$boundData = $this->buildBoundAdditionalData($this->activeKeyId, FormatMarker::AuthenticatedPublicKeyV1);
 		$cipherText = Crypto::encryptWithAD(new HiddenString($data), $secretKey, $publicKey, $boundData);
-		return $this->formatMarkedKeyCipherText($this->activeKeyId, FormatMarker::AuthenticatedPublicKeyV1, $cipherText);
+		return $this->formatKeyCipherText($this->activeKeyId, FormatMarker::AuthenticatedPublicKeyV1, $cipherText);
 	}
 
 
@@ -134,7 +134,7 @@ public function encryptWithAd(#[SensitiveParameter] string $data, string $additi
 		[$secretKey, $publicKey] = $this->getKeyPair($this->activeKeyId);
 		$boundData = $this->buildBoundAdditionalData($this->activeKeyId, FormatMarker::AuthenticatedPublicKeyWithAdV1, $additionalData);
 		$cipherText = Crypto::encryptWithAD(new HiddenString($data), $secretKey, $publicKey, $boundData);
-		return $this->formatMarkedKeyCipherText($this->activeKeyId, FormatMarker::AuthenticatedPublicKeyWithAdV1, $cipherText);
+		return $this->formatKeyCipherText($this->activeKeyId, FormatMarker::AuthenticatedPublicKeyWithAdV1, $cipherText);
 	}
 
 
@@ -156,14 +156,14 @@ public function encryptWithAd(#[SensitiveParameter] string $data, string $additi
 	 */
 	public function decrypt(string $data): string
 	{
-		[$keyId, $marker, $cipherText] = $this->parseMarkedKeyCipherText($data);
-		$this->checkFormatMarker($marker, FormatMarker::AuthenticatedPublicKeyV1);
+		[$keyId, $marker, $cipherText] = $this->parseKeyCipherText($data);
+		$validMarker = $this->checkFormatMarker($marker, FormatMarker::AuthenticatedPublicKeyV1);
 		[$secretKey, $publicKey] = $this->getKeyPair($keyId);
-		if ($marker === null) {
+		if ($validMarker === null) {
 			// Data from before the marker existed, nothing was added to what the decryption verifies back then
 			return Crypto::decrypt($cipherText, $secretKey, $publicKey)->getString();
 		}
-		$boundData = $this->buildBoundAdditionalData($keyId, FormatMarker::AuthenticatedPublicKeyV1);
+		$boundData = $this->buildBoundAdditionalData($keyId, $validMarker);
 		return Crypto::decryptWithAD($cipherText, $secretKey, $publicKey, $boundData)->getString();
 	}
 
@@ -190,14 +190,14 @@ public function decryptWithAd(string $data, string $additionalData): string
 		if ($additionalData === '') {
 			throw new DecryptWithAdNeedsAdditionalDataException();
 		}
-		[$keyId, $marker, $cipherText] = $this->parseMarkedKeyCipherText($data);
-		$this->checkFormatMarker($marker, FormatMarker::AuthenticatedPublicKeyWithAdV1);
+		[$keyId, $marker, $cipherText] = $this->parseKeyCipherText($data);
+		$validMarker = $this->checkFormatMarker($marker, FormatMarker::AuthenticatedPublicKeyWithAdV1);
 		[$secretKey, $publicKey] = $this->getKeyPair($keyId);
-		if ($marker === null) {
+		if ($validMarker === null) {
 			// Data from before the marker existed, the additional data was used alone back then
 			return Crypto::decryptWithAD($cipherText, $secretKey, $publicKey, $additionalData)->getString();
 		}
-		$boundData = $this->buildBoundAdditionalData($keyId, FormatMarker::AuthenticatedPublicKeyWithAdV1, $additionalData);
+		$boundData = $this->buildBoundAdditionalData($keyId, $validMarker, $additionalData);
 		return Crypto::decryptWithAD($cipherText, $secretKey, $publicKey, $boundData)->getString();
 	}
 

src/Exceptions/FormatMarkerMismatchException.php

@@ -12,10 +12,14 @@ class FormatMarkerMismatchException extends OutOfBoundsException
 
 	public function __construct(FormatMarker $actualMarker, ?Throwable $previous = null)
 	{
+		// The message names the method with the class because the value may come from a different class
+		// than the one throwing this, and also from a different method of the same class
 		parent::__construct(match ($actualMarker) {
-			FormatMarker::AuthenticatedPublicKeyV1 => 'Data was encrypted with AuthenticatedPublicKeyEncryption::encrypt(), decrypt it there with decrypt()',
-			FormatMarker::AuthenticatedPublicKeyWithAdV1 => 'Data was encrypted with AuthenticatedPublicKeyEncryption::encryptWithAd(), decrypt it there with decryptWithAd()',
-			FormatMarker::AnonymousPublicKeyV1 => 'Data was encrypted with AnonymousPublicKeyEncryption, decrypt it there',
+			FormatMarker::AuthenticatedPublicKeyV1 => 'Data was encrypted with AuthenticatedPublicKeyEncryption::encrypt(), decrypt it with AuthenticatedPublicKeyEncryption::decrypt()',
+			FormatMarker::AuthenticatedPublicKeyWithAdV1 => 'Data was encrypted with AuthenticatedPublicKeyEncryption::encryptWithAd(), decrypt it with AuthenticatedPublicKeyEncryption::decryptWithAd()',
+			FormatMarker::AnonymousPublicKeyV1 => 'Data was encrypted with AnonymousPublicKeyEncryption, decrypt it with AnonymousPublicKeyEncryption::decrypt()',
+			FormatMarker::SymmetricKeyV1 => 'Data was encrypted with SymmetricKeyEncryption::encrypt(), decrypt it with SymmetricKeyEncryption::decrypt()',
+			FormatMarker::SymmetricKeyWithAdV1 => 'Data was encrypted with SymmetricKeyEncryption::encryptWithAd(), decrypt it with SymmetricKeyEncryption::decryptWithAd()',
 		}, previous: $previous);
 	}
 

src/Exceptions/InvalidCipherTextFormatException.php

@@ -4,18 +4,14 @@
 namespace Spaze\Encryption\Exceptions;
 
 use OutOfBoundsException;
-use Spaze\Encryption\Format\StoredFormat;
 use Throwable;
 
 class InvalidCipherTextFormatException extends OutOfBoundsException
 {
 
-	public function __construct(StoredFormat $requiredFormats, ?Throwable $previous = null)
+	public function __construct(?Throwable $previous = null)
 	{
-		parent::__construct('Data format must be ' . match ($requiredFormats) {
-			StoredFormat::PlainOnly => "'\$keyId\$ciphertext'",
-			StoredFormat::MarkedOrPlain => "'\$keyId\$marker\$ciphertext' or '\$keyId\$ciphertext'",
-		}, previous: $previous);
+		parent::__construct("Data format must be '\$keyId\$marker\$ciphertext' or '\$keyId\$ciphertext'", previous: $previous);
 	}
 
 }

src/Exceptions/UnknownFormatMarkerException.php

@@ -12,7 +12,7 @@ class UnknownFormatMarkerException extends OutOfBoundsException
 
 	public function __construct(string $marker, ?Throwable $previous = null)
 	{
-		parent::__construct("Unknown format marker '" . LogSafeValue::from($marker) . "', was the data encrypted by a newer version of this library?", previous: $previous);
+		parent::__construct("Unknown format marker '" . LogSafeValue::from($marker) . "', is the data corrupted, or encrypted by a newer version of this library?", previous: $previous);
 	}
 
 }

src/Format/FormatMarker.php

@@ -12,5 +12,7 @@ enum FormatMarker: string
 	case AuthenticatedPublicKeyV1 = 'AuthV1';
 	case AuthenticatedPublicKeyWithAdV1 = 'AuthAdV1';
 	case AnonymousPublicKeyV1 = 'AnonV1';
+	case SymmetricKeyV1 = 'SymV1';
+	case SymmetricKeyWithAdV1 = 'SymAdV1';
 
 }