diff --git a/README.md b/README.md index d5f8def..859d6b2 100644 --- a/README.md +++ b/README.md @@ -10,9 +10,9 @@ composer require spaze/encryption ``` ## Usage -This library provides authenticated symmetric encryption using [Halite](https://github.com/paragonie/halite), which relies on [libsodium](https://pecl.php.net/package/libsodium) for all of its underlying cryptography operations. +This library provides symmetric encryption, where one key both encrypts and decrypts, [encryption between two parties](#encryption-between-two-parties), where each side holds its own secret key and the other side's public key, and [encryption to a public key](#encryption-to-a-public-key), where the key used to encrypt cannot decrypt the data. It uses [Halite](https://github.com/paragonie/halite), which relies on [libsodium](https://pecl.php.net/package/libsodium) for all of its underlying cryptography operations. Read the [Halite documentation](https://github.com/paragonie/halite/tree/master/doc) for more details, including the [cryptography primitives](https://github.com/paragonie/halite/blob/master/doc/Primitives.md) it uses. -At the moment, asymmetric encryption and signatures are not supported by this library. +At the moment, signatures are not supported by this library. The library is framework-agnostic, with minimal dependencies. @@ -104,6 +104,102 @@ You can use `needsReEncrypt($ciphertext): bool` to see if the data is encrypted 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. +## Encryption between two parties + +`Spaze\Encryption\AuthenticatedPublicKeyEncryption` encrypts data exchanged between two parties. Each side configures its own secret key and the other side's public key under the same key id. Both sides can encrypt and decrypt, and decryption only succeeds when the data was created with one of the two configured keys, so a successful decryption also proves the data came from the other party, or from us. + +Because both sides can decrypt, this class does not hide the data from whoever can encrypt it. If both sides would be configured with the same keys anyway, use `SymmetricKeyEncryption` instead. + +### Create the object using the constructor +```php +Spaze\Encryption\AuthenticatedPublicKeyEncryption::__construct(array $secretKeys, array $publicKeys, string $activeKeyId, string $keyPrefix) +``` + +#### `array $secretKeys` +An array of our own secret keys, a _key id_ (will be part of the encrypted string) as the array key, the prefixed _key_ as the value. + +#### `array $publicKeys` +An array of the other party's public keys, one for every key id in `$secretKeys`. Every key id needs both keys, a key id with only one of them throws `IncompleteKeyPairException`. + +The values are validated like the symmetric keys (the prefix must match, the key must be valid hex and decode to exactly 32 bytes, the key id must be non-empty and must not contain `$`), with one addition: the value can carry a tag between the prefix and the key that says which kind of key it is, `adek_secret_79e0[...]8a8d` or `adek_public_d22c[...]cfa3`. The tag is optional, plain `adek_79e0[...]8a8d` values are accepted too, so existing configurations can be reused unchanged. Tagged keys are recommended though: a secret key pasted where a public key belongs (or the other way around) then throws `InvalidKeyRoleException` when the object is created, instead of producing encrypted data that nobody will ever be able to decrypt. And when you find a leaked string somewhere, the tag tells you how bad it is: `adek_secret_` means rotate the keys now, a public key is not a secret. + +#### `string $activeKeyId` and `string $keyPrefix` +Same meaning and validation as in `SymmetricKeyEncryption` above. + +Example: +```php +$secretKeys = [ + 'key1' => 'adek_secret_79e0[...]8a8d', +]; +$publicKeys = [ + 'key1' => 'adek_public_d22c[...]cfa3', +]; +$encryption = new Spaze\Encryption\AuthenticatedPublicKeyEncryption($secretKeys, $publicKeys, 'key1', 'adek'); +``` + +### Generating a key pair +Each party generates their own pair, keeps the secret key to themselves and gives the public key to the other party: +```php +$keyPair = sodium_crypto_box_keypair(); +$secretKey = 'adek_secret_' . bin2hex(sodium_crypto_box_secretkey($keyPair)); +$publicKey = 'adek_public_' . bin2hex(sodium_crypto_box_publickey($keyPair)); +``` + +### Encrypt & decrypt +The methods are the same as in `SymmetricKeyEncryption`: `encrypt()`, `decrypt()`, `encryptWithAd()` and `decryptWithAd()` for [context binding](#encrypt-with-additional-authenticated-data-aad), and `needsReEncrypt()` for [key rotation](#key-rotation). + +The output looks like `$$AuthV1$`, or `$$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":"","marker":""}` — so `AuthV1` or `AuthAdV1` — with an `"additionalData":""` 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 — 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. + +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. + +When either side replaces their keys, configure the new pair under a new key id on both sides and rotate the same way as with symmetric keys. + +## Encryption to a public key + +`Spaze\Encryption\AnonymousPublicKeyEncryption` encrypts data to a public key: anyone who has the public key can encrypt, only whoever holds the matching secret key can decrypt, and the encrypted value does not say who created it. + +The point is the split: a server that only stores the data can be configured with just the public keys and cannot read anything back, not even the values it has just encrypted itself. Decryption then happens elsewhere, in a back office application or a worker that holds the secret keys. If every deployment would hold the secret keys anyway, use `SymmetricKeyEncryption` instead, and if the reader also needs to know who created the data, use `AuthenticatedPublicKeyEncryption`. + +### Create the object using the constructor +```php +Spaze\Encryption\AnonymousPublicKeyEncryption::__construct(array $secretKeys, array $publicKeys, string $activeKeyId, string $keyPrefix) +``` + +#### `array $secretKeys` +The secret keys, needed only where the data is decrypted. Encrypt-only deployments pass an empty array. + +#### `array $publicKeys` +The public keys. A key id missing here is derived from the secret key with the same id, so a decrypting deployment can configure just the secret keys. When a key id has both values configured, the constructor verifies they belong together and throws `KeyPairMismatchException` when they don't, so a swapped or stale pair fails at construction and not weeks later on the first decrypt. + +The values are validated like in the other classes, including the optional `secret`/`public` tag (see [encryption between two parties](#encryption-between-two-parties)), and the pair is generated [the same way](#generating-a-key-pair). The active key id must have a public key, configured or derived. + +Example, the encrypting side: +```php +$encryption = new Spaze\Encryption\AnonymousPublicKeyEncryption([], ['key1' => 'adek_public_d22c[...]cfa3'], 'key1', 'adek'); +$encrypted = $encryption->encrypt($addressData); // works +$decrypted = $encryption->decrypt($encrypted); // throws MissingSecretKeyException +``` +The decrypting side: +```php +$encryption = new Spaze\Encryption\AnonymousPublicKeyEncryption(['key1' => 'adek_secret_79e0[...]8a8d'], [], '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 `$$AnonV1$`, 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. + +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. + +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. + ## Usage in Nette framework Although it can be used anywhere, this library doesn't depend on anything from the Nette Framework. @@ -130,7 +226,7 @@ Note that Nette compiles parameter values into the generated DI container file i That directory tends to leak into places nobody thinks about: backups, deploy artifacts, rsync copies, debug tarballs sent to hosting support. Either treat the temp directory accordingly and exclude it from backups and artifacts, or use [dynamic parameters](https://doc.nette.org/en/application/bootstrapping#toc-dynamic-parameters) or environment variables so the key values are not baked into the compiled container. -Exception logs are one of those places too. When a key is misconfigured, the `SymmetricKeyEncryption` constructor throws an exception while the container is creating the service, and Tracy logs that exception as an HTML file that includes the code around every line in the stack trace, the container line that passes the keys among them. Neither `#[SensitiveParameter]` nor `zend.exception_ignore_args` prevents that, both hide the values passed to a function, not the code printed around them. +Exception logs are one of those places too. When a key is misconfigured, the constructors of these classes throw an exception while the container is creating the service, and Tracy logs that exception as an HTML file that includes the code around every line in the stack trace, the container line that passes the keys among them. Neither `#[SensitiveParameter]` nor `zend.exception_ignore_args` prevents that, both hide the values passed to a function, not the code printed around them. Anything that keeps the keys out of the generated container keeps them out of such a log as well. Besides the options above, you can also pass them as a runtime call, because Nette compiles a `@service::method()` argument into a call instead of a literal: ```neon @@ -156,6 +252,13 @@ services: passwordHashEncryption: \Spaze\Encryption\SymmetricKeyEncryption(%encryption.keys.passwordHash%, %encryption.activeKeyIds.passwordHash%, %encryption.prefixes.passwordHash%) ``` +The two public-key classes take two key arrays, so their groups need two lists in the parameters: +``` +services: + invoiceEncryption: \Spaze\Encryption\AnonymousPublicKeyEncryption(%encryption.secretKeys.invoice%, %encryption.publicKeys.invoice%, %encryption.activeKeyIds.invoice%, %encryption.prefixes.invoice%) +``` +On a deployment that only encrypts, define the secret keys list as empty (`secretKeys: {invoice: []}`) and keep the secret keys out of its configuration entirely. + Use the services in this class which needs to encrypt and decrypt email addresses for whatever reason: ```php use Spaze\Encryption\SymmetricKeyEncryption; diff --git a/src/AnonymousPublicKeyEncryption.php b/src/AnonymousPublicKeyEncryption.php new file mode 100644 index 0000000..0ae396c --- /dev/null +++ b/src/AnonymousPublicKeyEncryption.php @@ -0,0 +1,163 @@ + */ + private array $secretKeys = []; + + /** @var array */ + private array $publicKeys = []; + + + /** + * Encryption to a public key: anyone with the public key can encrypt, only whoever holds the matching + * secret key can decrypt, and the encrypted value does not say who created it. + * A deployment that only encrypts needs only the public keys. + * + * @param array $secretKeys key id => secret key; needed only where the data is decrypted, may be empty + * @param array $publicKeys key id => public key; a key id missing here is derived from the secret key + * @throws ActiveKeyIdNotFoundException + * @throws InvalidKeyEncodingException + * @throws InvalidKeyIdException + * @throws InvalidKeyLengthException + * @throws InvalidKeyPrefixException + * @throws InvalidKeyRoleException + * @throws KeyPairMismatchException + * @throws MissingKeyPrefixException + * @throws SodiumException + */ + public function __construct( + #[SensitiveParameter] array $secretKeys, + #[SensitiveParameter] array $publicKeys, + private string $activeKeyId, + private string $keyPrefix, + ) { + $this->secretKeys = $this->decodeKeys($secretKeys, $this->keyPrefix, SODIUM_CRYPTO_BOX_SECRETKEYBYTES, AsymmetricKeyRole::Secret); + $this->publicKeys = $this->decodeKeys($publicKeys, $this->keyPrefix, SODIUM_CRYPTO_BOX_PUBLICKEYBYTES, AsymmetricKeyRole::Public); + foreach (array_keys($secretKeys) as $id) { + $id = (string)$id; + $derivedPublicKey = sodium_crypto_box_publickey_from_secretkey($this->secretKeys[$id]->getString()); + if (isset($this->publicKeys[$id])) { + // A swapped or stale pair should fail now, not weeks later on the first decrypt + if (!hash_equals($this->publicKeys[$id]->getString(), $derivedPublicKey)) { + throw new KeyPairMismatchException($id); + } + } else { + $this->publicKeys[$id] = new HiddenString($derivedPublicKey); + } + } + if (!isset($this->publicKeys[$this->activeKeyId])) { + throw new ActiveKeyIdNotFoundException($this->activeKeyId); + } + } + + + /** + * @throws InvalidKey + * @throws InvalidType + * @throws SodiumException + * @throws TypeError + */ + 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); + } + + + /** + * Halite reports any well-formed value it cannot decrypt as InvalidKey, a wrong key and corrupted data + * are indistinguishable here; only a value that is not even valid base64 throws InvalidMessage instead. + * + * @throws FormatMarkerMismatchException + * @throws InvalidKey + * @throws InvalidMessage + * @throws InvalidType + * @throws MissingSecretKeyException + * @throws SodiumException + * @throws TypeError + * @throws UnknownEncryptionKeyIdException + * @throws UnknownFormatMarkerException + * @throws InvalidCipherTextFormatException + * @throws InvalidNumberOfComponentsException + */ + public function decrypt(string $data): string + { + [$keyId, $marker, $cipherText] = $this->parseMarkedKeyCipherText($data); + $this->checkFormatMarker($marker, FormatMarker::AnonymousPublicKeyV1); + return Crypto::unseal($cipherText, $this->getSecretKey($keyId))->getString(); + } + + + /** + * Checks if the given data should be re-encrypted with the currently active key: + * either they are encrypted with an inactive key, or they are stored in the older format + * without the marker, and re-encrypting them adds it. + * + * @throws FormatMarkerMismatchException + * @throws InvalidCipherTextFormatException + * @throws InvalidNumberOfComponentsException + * @throws UnknownFormatMarkerException + */ + public function needsReEncrypt(string $data): bool + { + return $this->needsReEncryptMarked($data, $this->activeKeyId, FormatMarker::AnonymousPublicKeyV1); + } + + + /** + * @throws InvalidKey + * @throws MissingSecretKeyException + * @throws TypeError + * @throws UnknownEncryptionKeyIdException + */ + private function getSecretKey(string $keyId): EncryptionSecretKey + { + if (isset($this->secretKeys[$keyId])) { + return new EncryptionSecretKey($this->secretKeys[$keyId]); + } + if (isset($this->publicKeys[$keyId])) { + // The key id is known, this deployment just can't decrypt, only encrypt + throw new MissingSecretKeyException($keyId); + } + throw new UnknownEncryptionKeyIdException($keyId); + } + +} diff --git a/src/AuthenticatedPublicKeyEncryption.php b/src/AuthenticatedPublicKeyEncryption.php new file mode 100644 index 0000000..80c311a --- /dev/null +++ b/src/AuthenticatedPublicKeyEncryption.php @@ -0,0 +1,238 @@ + */ + private array $secretKeys = []; + + /** @var array */ + private array $publicKeys = []; + + + /** + * Encryption between two parties: each side configures its own secret key and the other side's public key under the same key id. + * Both sides can encrypt and decrypt, and decryption only succeeds for data created with one of the two configured keys, + * so it also proves the data came from the other party, or from us. + * + * @param array $secretKeys key id => our secret key + * @param array $publicKeys key id => the other party's public key + * @throws ActiveKeyIdNotFoundException + * @throws IncompleteKeyPairException + * @throws InvalidKeyEncodingException + * @throws InvalidKeyIdException + * @throws InvalidKeyLengthException + * @throws InvalidKeyPrefixException + * @throws InvalidKeyRoleException + * @throws MissingKeyPrefixException + */ + public function __construct( + #[SensitiveParameter] array $secretKeys, + #[SensitiveParameter] array $publicKeys, + private string $activeKeyId, + private string $keyPrefix, + ) { + $this->secretKeys = $this->decodeKeys($secretKeys, $this->keyPrefix, SODIUM_CRYPTO_BOX_SECRETKEYBYTES, AsymmetricKeyRole::Secret); + $this->publicKeys = $this->decodeKeys($publicKeys, $this->keyPrefix, SODIUM_CRYPTO_BOX_PUBLICKEYBYTES, AsymmetricKeyRole::Public); + foreach (array_keys($secretKeys) as $id) { + if (!isset($this->publicKeys[$id])) { + throw new IncompleteKeyPairException((string)$id); + } + } + foreach (array_keys($publicKeys) as $id) { + if (!isset($this->secretKeys[$id])) { + throw new IncompleteKeyPairException((string)$id); + } + } + if (!isset($this->secretKeys[$this->activeKeyId])) { + throw new ActiveKeyIdNotFoundException($this->activeKeyId); + } + } + + + /** + * The key id and the marker go into what the decryption verifies, so changing them + * in the stored value makes decryption fail. + * + * @throws CannotPerformOperation + * @throws InvalidDigestLength + * @throws InvalidKey + * @throws InvalidMessage + * @throws InvalidType + * @throws JsonException + * @throws SodiumException + * @throws TypeError + */ + 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); + } + + + /** + * The key id and the marker are combined with the given additional data into what the decryption verifies, + * so changing them in the stored value makes decryption fail. + * + * @throws CannotPerformOperation + * @throws EncryptWithAdNeedsAdditionalDataException + * @throws InvalidDigestLength + * @throws InvalidKey + * @throws InvalidMessage + * @throws InvalidType + * @throws JsonException + * @throws SodiumException + * @throws TypeError + */ + public function encryptWithAd(#[SensitiveParameter] string $data, string $additionalData): string + { + if ($additionalData === '') { + throw new EncryptWithAdNeedsAdditionalDataException(); + } + [$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); + } + + + /** + * @throws CannotPerformOperation + * @throws FormatMarkerMismatchException + * @throws InvalidDigestLength + * @throws InvalidKey + * @throws InvalidMessage + * @throws InvalidSignature + * @throws InvalidType + * @throws SodiumException + * @throws TypeError + * @throws JsonException + * @throws UnknownEncryptionKeyIdException + * @throws UnknownFormatMarkerException + * @throws InvalidCipherTextFormatException + * @throws InvalidNumberOfComponentsException + */ + public function decrypt(string $data): string + { + [$keyId, $marker, $cipherText] = $this->parseMarkedKeyCipherText($data); + $this->checkFormatMarker($marker, FormatMarker::AuthenticatedPublicKeyV1); + [$secretKey, $publicKey] = $this->getKeyPair($keyId); + if ($marker === 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); + return Crypto::decryptWithAD($cipherText, $secretKey, $publicKey, $boundData)->getString(); + } + + + /** + * @throws CannotPerformOperation + * @throws DecryptWithAdNeedsAdditionalDataException + * @throws FormatMarkerMismatchException + * @throws InvalidDigestLength + * @throws InvalidKey + * @throws InvalidMessage + * @throws InvalidSignature + * @throws InvalidType + * @throws SodiumException + * @throws TypeError + * @throws JsonException + * @throws UnknownEncryptionKeyIdException + * @throws UnknownFormatMarkerException + * @throws InvalidCipherTextFormatException + * @throws InvalidNumberOfComponentsException + */ + public function decryptWithAd(string $data, string $additionalData): string + { + if ($additionalData === '') { + throw new DecryptWithAdNeedsAdditionalDataException(); + } + [$keyId, $marker, $cipherText] = $this->parseMarkedKeyCipherText($data); + $this->checkFormatMarker($marker, FormatMarker::AuthenticatedPublicKeyWithAdV1); + [$secretKey, $publicKey] = $this->getKeyPair($keyId); + if ($marker === 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); + return Crypto::decryptWithAD($cipherText, $secretKey, $publicKey, $boundData)->getString(); + } + + + /** + * Checks if the given data should be re-encrypted with the currently active key: + * either they are encrypted with an inactive key, or they are stored in the older format + * without the marker, and re-encrypting them adds it. + * + * @throws FormatMarkerMismatchException + * @throws InvalidCipherTextFormatException + * @throws InvalidNumberOfComponentsException + * @throws UnknownFormatMarkerException + */ + public function needsReEncrypt(string $data): bool + { + return $this->needsReEncryptMarked($data, $this->activeKeyId, FormatMarker::AuthenticatedPublicKeyV1, FormatMarker::AuthenticatedPublicKeyWithAdV1); + } + + + /** + * The constructor guarantees a key id always has both keys, so one lookup can serve both. + * + * @return array{0:EncryptionSecretKey, 1:EncryptionPublicKey} + * @throws InvalidKey + * @throws TypeError + * @throws UnknownEncryptionKeyIdException + */ + private function getKeyPair(string $keyId): array + { + if (isset($this->secretKeys[$keyId], $this->publicKeys[$keyId])) { + return [new EncryptionSecretKey($this->secretKeys[$keyId]), new EncryptionPublicKey($this->publicKeys[$keyId])]; + } else { + throw new UnknownEncryptionKeyIdException($keyId); + } + } + +} diff --git a/src/Exceptions/FormatMarkerMismatchException.php b/src/Exceptions/FormatMarkerMismatchException.php new file mode 100644 index 0000000..9bbf41f --- /dev/null +++ b/src/Exceptions/FormatMarkerMismatchException.php @@ -0,0 +1,22 @@ + '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', + }, previous: $previous); + } + +} diff --git a/src/Exceptions/IncompleteKeyPairException.php b/src/Exceptions/IncompleteKeyPairException.php new file mode 100644 index 0000000..20756f8 --- /dev/null +++ b/src/Exceptions/IncompleteKeyPairException.php @@ -0,0 +1,17 @@ + "'\$keyId\$ciphertext'", + StoredFormat::MarkedOrPlain => "'\$keyId\$marker\$ciphertext' or '\$keyId\$ciphertext'", + }, previous: $previous); } } diff --git a/src/Exceptions/InvalidKeyLengthException.php b/src/Exceptions/InvalidKeyLengthException.php index 036717e..d3f4637 100644 --- a/src/Exceptions/InvalidKeyLengthException.php +++ b/src/Exceptions/InvalidKeyLengthException.php @@ -9,11 +9,10 @@ class InvalidKeyLengthException extends Exception { - public function __construct(string $id, int $actualLength, ?Throwable $previous = null) + public function __construct(string $id, int $actualLength, int $expectedLength, ?Throwable $previous = null) { - $expectedBytes = SODIUM_CRYPTO_STREAM_KEYBYTES; - $expectedHexChars = $expectedBytes * 2; - parent::__construct("Key '{$id}' must be {$expectedBytes} bytes ({$expectedHexChars} hexadecimal characters) but is {$actualLength} bytes", previous: $previous); + $expectedHexChars = $expectedLength * 2; + parent::__construct("Key '{$id}' must be {$expectedLength} bytes ({$expectedHexChars} hexadecimal characters) but is {$actualLength} bytes", previous: $previous); } } diff --git a/src/Exceptions/InvalidKeyRoleException.php b/src/Exceptions/InvalidKeyRoleException.php new file mode 100644 index 0000000..fe538cd --- /dev/null +++ b/src/Exceptions/InvalidKeyRoleException.php @@ -0,0 +1,18 @@ +value} key but is used as a {$expectedRole->value} key", previous: $previous); + } + +} diff --git a/src/Exceptions/KeyPairMismatchException.php b/src/Exceptions/KeyPairMismatchException.php new file mode 100644 index 0000000..1b9be1b --- /dev/null +++ b/src/Exceptions/KeyPairMismatchException.php @@ -0,0 +1,17 @@ + $keys key id => key + * @return array + * @throws InvalidKeyEncodingException + * @throws InvalidKeyIdException + * @throws InvalidKeyLengthException + * @throws InvalidKeyPrefixException + * @throws InvalidKeyRoleException + * @throws MissingKeyPrefixException + */ + private function decodeKeys(#[SensitiveParameter] array $keys, string $keyPrefix, int $expectedLength, ?AsymmetricKeyRole $role = null): array + { + $keyPrefix .= self::KEY_PREFIX_SEPARATOR; + $decodedKeys = []; + foreach ($keys as $id => $key) { + $id = (string)$id; + if ($id === '' || str_contains($id, self::KEY_CIPHERTEXT_SEPARATOR)) { + throw new InvalidKeyIdException($id, self::KEY_CIPHERTEXT_SEPARATOR); + } + if (!str_starts_with($key, $keyPrefix)) { + if (str_contains($key, self::KEY_PREFIX_SEPARATOR)) { + throw new InvalidKeyPrefixException($id, $keyPrefix); + } + throw new MissingKeyPrefixException($id, $keyPrefix); + } + $hexKey = substr($key, strlen($keyPrefix)); + if ($role !== null) { + $hexKey = $this->stripKeyRole($id, $hexKey, $role); + } + try { + $decodedKey = sodium_hex2bin($hexKey); + } catch (SodiumException $e) { + throw new InvalidKeyEncodingException($id, $e); + } + if (strlen($decodedKey) !== $expectedLength) { + throw new InvalidKeyLengthException($id, strlen($decodedKey), $expectedLength); + } + $decodedKeys[$id] = new HiddenString($decodedKey); + } + return $decodedKeys; + } + + + /** + * The role tag between the prefix and the key itself is optional, but when present, it has to match how the key is used. + * + * @throws InvalidKeyRoleException + */ + private function stripKeyRole(string $id, #[SensitiveParameter] string $key, AsymmetricKeyRole $expectedRole): string + { + foreach (AsymmetricKeyRole::cases() as $role) { + if (str_starts_with($key, $role->value . self::KEY_PREFIX_SEPARATOR)) { + if ($role !== $expectedRole) { + throw new InvalidKeyRoleException($id, $expectedRole, $role); + } + return substr($key, strlen($role->value . self::KEY_PREFIX_SEPARATOR)); + } + } + return $key; + } + + + /** + * @return array{0:non-empty-string, 1:non-empty-string} + * @throws InvalidCipherTextFormatException + * @throws InvalidNumberOfComponentsException + */ + private function parseKeyCipherText(string $data): array + { + $data = explode(self::KEY_CIPHERTEXT_SEPARATOR, $data); + if (count($data) !== 3) { + throw new InvalidNumberOfComponentsException(StoredFormat::PlainOnly); + } + if ($data[0] !== '' || $data[1] === '' || $data[2] === '') { + throw new InvalidCipherTextFormatException(StoredFormat::PlainOnly); + } + return [$data[1], $data[2]]; + } + + + private function formatKeyCipherText(string $keyId, string $cipherText): string + { + return self::KEY_CIPHERTEXT_SEPARATOR . $keyId . self::KEY_CIPHERTEXT_SEPARATOR . $cipherText; + } + + + /** + * Reads both the marked format and the one without a marker, because data encrypted + * before the marker existed has to keep decrypting. + * + * @return array{0:non-empty-string, 1:non-empty-string|null, 2:non-empty-string} + * @throws InvalidCipherTextFormatException + * @throws InvalidNumberOfComponentsException + */ + private function parseMarkedKeyCipherText(string $data): array + { + $data = explode(self::KEY_CIPHERTEXT_SEPARATOR, $data); + $count = count($data); + if ($count !== 3 && $count !== 4) { + throw new InvalidNumberOfComponentsException(StoredFormat::MarkedOrPlain); + } + if ($data[0] !== '' || $data[1] === '' || $data[2] === '' || ($count === 4 && $data[3] === '')) { + throw new InvalidCipherTextFormatException(StoredFormat::MarkedOrPlain); + } + return $count === 3 ? [$data[1], null, $data[2]] : [$data[1], $data[2], $data[3]]; + } + + + private function formatMarkedKeyCipherText(string $keyId, FormatMarker $marker, string $cipherText): string + { + return self::KEY_CIPHERTEXT_SEPARATOR . $keyId . self::KEY_CIPHERTEXT_SEPARATOR . $marker->value . self::KEY_CIPHERTEXT_SEPARATOR . $cipherText; + } + + + /** + * The value that ties the encrypted data to its key id and marker: it goes into what the decryption verifies, + * so changing the key id or the marker in the stored value makes decryption fail. + * The key id and the additional data are Base64-encoded (the URL-safe kind), so the result is the same + * no matter what bytes they contain, no JSON character escaping ever kicks in, and the value can be rebuilt + * anywhere with plain string formatting. This exact recipe can never change, a changed recipe is a new marker. + * + * @throws JsonException + * @throws SodiumException + */ + private function buildBoundAdditionalData(string $keyId, FormatMarker $marker, #[SensitiveParameter] ?string $additionalData = null): string + { + $values = [ + 'keyId' => sodium_bin2base64($keyId, SODIUM_BASE64_VARIANT_URLSAFE), + 'marker' => $marker->value, + ]; + if ($additionalData !== null) { + $values['additionalData'] = sodium_bin2base64($additionalData, SODIUM_BASE64_VARIANT_URLSAFE); + } + return json_encode($values, JSON_THROW_ON_ERROR); + } + + + /** + * Data needs re-encryption when encrypted with an inactive key, or when stored in the older format + * without the marker, because re-encrypting adds it. + * + * @throws FormatMarkerMismatchException + * @throws InvalidCipherTextFormatException + * @throws InvalidNumberOfComponentsException + * @throws UnknownFormatMarkerException + */ + private function needsReEncryptMarked(string $data, string $activeKeyId, FormatMarker ...$expectedMarkers): bool + { + [$keyId, $marker] = $this->parseMarkedKeyCipherText($data); + $this->checkFormatMarker($marker, ...$expectedMarkers); + return $keyId !== $activeKeyId || $marker === null; + } + + + /** + * No marker is fine, that's data from before the marker existed; a marker that is present + * has to be one of the expected ones. + * + * @throws FormatMarkerMismatchException + * @throws UnknownFormatMarkerException + */ + private function checkFormatMarker(?string $marker, FormatMarker ...$expectedMarkers): void + { + if ($marker === null) { + return; + } + $actualMarker = FormatMarker::tryFrom($marker); + if ($actualMarker === null) { + throw new UnknownFormatMarkerException($marker); + } + if (!in_array($actualMarker, $expectedMarkers, true)) { + throw new FormatMarkerMismatchException($actualMarker); + } + } + +} diff --git a/src/Format/LogSafeValue.php b/src/Format/LogSafeValue.php new file mode 100644 index 0000000..f83cd81 --- /dev/null +++ b/src/Format/LogSafeValue.php @@ -0,0 +1,21 @@ + self::MAX_LENGTH ? substr($value, 0, self::MAX_LENGTH) . '...' : $value; + return preg_replace('/[^!-~]/', '?', $shortened) ?? $shortened; + } + +} diff --git a/src/Format/StoredFormat.php b/src/Format/StoredFormat.php new file mode 100644 index 0000000..584baee --- /dev/null +++ b/src/Format/StoredFormat.php @@ -0,0 +1,15 @@ + */ private array $keys = []; @@ -54,28 +52,7 @@ public function __construct( private string $activeKeyId, private string $keyPrefix, ) { - $keyPrefix = $this->keyPrefix . self::KEY_PREFIX_SEPARATOR; - foreach ($keys as $id => $key) { - $id = (string)$id; - if ($id === '' || str_contains($id, self::KEY_CIPHERTEXT_SEPARATOR)) { - throw new InvalidKeyIdException($id, self::KEY_CIPHERTEXT_SEPARATOR); - } - if (!str_starts_with($key, $keyPrefix)) { - if (str_contains($key, self::KEY_PREFIX_SEPARATOR)) { - throw new InvalidKeyPrefixException($id, $keyPrefix); - } - throw new MissingKeyPrefixException($id, $keyPrefix); - } - try { - $decodedKey = sodium_hex2bin(substr($key, strlen($keyPrefix))); - } catch (SodiumException $e) { - throw new InvalidKeyEncodingException($id, $e); - } - if (strlen($decodedKey) !== SODIUM_CRYPTO_STREAM_KEYBYTES) { - throw new InvalidKeyLengthException($id, strlen($decodedKey)); - } - $this->keys[$id] = new HiddenString($decodedKey); - } + $this->keys = $this->decodeKeys($keys, $this->keyPrefix, SODIUM_CRYPTO_STREAM_KEYBYTES); if (!isset($this->keys[$this->activeKeyId])) { throw new ActiveKeyIdNotFoundException($this->activeKeyId); } @@ -194,28 +171,4 @@ private function getKey(string $keyId): EncryptionKey } } - - /** - * @return array{0:non-empty-string, 1:non-empty-string} - * @throws InvalidCipherTextFormatException - * @throws InvalidNumberOfComponentsException - */ - private function parseKeyCipherText(string $data): array - { - $data = explode(self::KEY_CIPHERTEXT_SEPARATOR, $data); - if (count($data) !== 3) { - throw new InvalidNumberOfComponentsException(); - } - if ($data[0] !== '' || $data[1] === '' || $data[2] === '') { - throw new InvalidCipherTextFormatException(); - } - return [$data[1], $data[2]]; - } - - - private function formatKeyCipherText(string $keyId, string $cipherText): string - { - return self::KEY_CIPHERTEXT_SEPARATOR . $keyId . self::KEY_CIPHERTEXT_SEPARATOR . $cipherText; - } - } diff --git a/tests/AnonymousPublicKeyEncryptionTest.phpt b/tests/AnonymousPublicKeyEncryptionTest.phpt new file mode 100644 index 0000000..7bbc905 --- /dev/null +++ b/tests/AnonymousPublicKeyEncryptionTest.phpt @@ -0,0 +1,593 @@ + */ + private array $secretKeys; + + /** @var array */ + private array $publicKeys; + + private AnonymousPublicKeyEncryption $encryption; + + + protected function setUp(): void + { + $this->secretKeys = []; + $this->publicKeys = []; + foreach ([self::INACTIVE_KEY, self::ACTIVE_KEY] as $id) { + $keyPair = sodium_crypto_box_keypair(); + $this->secretKeys[$id] = self::KEY_PREFIX . '_secret_' . bin2hex(sodium_crypto_box_secretkey($keyPair)); + $this->publicKeys[$id] = self::KEY_PREFIX . '_public_' . bin2hex(sodium_crypto_box_publickey($keyPair)); + } + $this->encryption = new AnonymousPublicKeyEncryption($this->secretKeys, $this->publicKeys, self::ACTIVE_KEY, self::KEY_PREFIX); + } + + + public function testEncryptDecrypt(): void + { + Assert::same(self::PLAINTEXT, $this->encryption->decrypt($this->encryption->encrypt(self::PLAINTEXT))); + } + + + public function testEncryptDecryptWithDerivedPublicKeys(): void + { + // Public keys left out completely, they are derived from the secret keys + $encryption = new AnonymousPublicKeyEncryption($this->secretKeys, [], self::ACTIVE_KEY, self::KEY_PREFIX); + Assert::same(self::PLAINTEXT, $encryption->decrypt($encryption->encrypt(self::PLAINTEXT))); + Assert::same(self::PLAINTEXT, $this->encryption->decrypt($encryption->encrypt(self::PLAINTEXT))); + } + + + public function testEncryptOnlyDeployment(): void + { + // The whole point: a deployment configured with just the public keys can store data it can never read back + $encryptOnly = new AnonymousPublicKeyEncryption([], $this->publicKeys, self::ACTIVE_KEY, self::KEY_PREFIX); + $encrypted = $encryptOnly->encrypt(self::PLAINTEXT); + Assert::same(self::PLAINTEXT, $this->encryption->decrypt($encrypted)); + Assert::exception( + function () use ($encryptOnly, $encrypted): void { + $encryptOnly->decrypt($encrypted); + }, + MissingSecretKeyException::class, + "No secret key configured for key id 'dev2', it can only be used to encrypt", + ); + } + + + public function testDecryptStoredCipherText(): void + { + // Generated once when the feature was added and kept verbatim, because the output of this library is stored + // in databases: anything that changes the format or the key handling has to fail here first + $encryption = $this->createFixtureEncryption(); + Assert::same(self::PLAINTEXT, $encryption->decrypt(self::FIXTURE_CIPHERTEXT)); + Assert::false($encryption->needsReEncrypt(self::FIXTURE_CIPHERTEXT)); + Assert::same('$fixture$AnonV1$', substr(self::FIXTURE_CIPHERTEXT, 0, 16)); + // Unlike in the other two classes, the encrypted part itself has no version header, it looks like random data. + // The leading '$' anchors the check to the envelope ('$' can never appear in the encrypted part itself) + Assert::notContains('$MUI', self::FIXTURE_CIPHERTEXT); + } + + + public function testDecryptStoredLegacyCipherText(): void + { + // Values in the format without the marker, like the ones an older library wrote, have to keep decrypting, + // and needsReEncrypt() reports them so a re-encryption sweep migrates them to the marked format + $encryption = $this->createFixtureEncryption(); + Assert::same(self::PLAINTEXT, $encryption->decrypt(self::LEGACY_FIXTURE_CIPHERTEXT)); + Assert::true($encryption->needsReEncrypt(self::LEGACY_FIXTURE_CIPHERTEXT)); + } + + + public function testFormatMarkerMismatch(): void + { + // A value created by the other class names its creator instead of failing with a misleading decryption error + Assert::exception( + function (): void { + $this->encryption->decrypt('$' . self::ACTIVE_KEY . '$AuthV1$MUIFAwhatever'); + }, + FormatMarkerMismatchException::class, + 'Data was encrypted with AuthenticatedPublicKeyEncryption::encrypt(), decrypt it there with decrypt()', + ); + } + + + public function testUnknownFormatMarker(): void + { + Assert::exception( + function (): void { + $this->encryption->needsReEncrypt('$' . self::ACTIVE_KEY . '$AnonV9$whatever'); + }, + UnknownFormatMarkerException::class, + "Unknown format marker 'AnonV9', was the data encrypted by a newer version of this library?", + ); + } + + + public function testMissingSecretKeyException(): void + { + // Deliberately not a subclass of UnknownEncryptionKeyIdException: the id is known, + // this deployment just can't decrypt with it, only encrypt + $e = Assert::exception( + function (): void { + $encryption = new AnonymousPublicKeyEncryption([], $this->publicKeys, self::ACTIVE_KEY, self::KEY_PREFIX); + $encryption->decrypt($encryption->encrypt(self::PLAINTEXT)); + }, + MissingSecretKeyException::class, + ); + Assert::type(OutOfRangeException::class, $e); + // The hierarchy is the contract here: code catching UnknownEncryptionKeyIdException must not swallow + // this one, and PHPStan reports any assertion of that as statically always-false, so it guards it instead + } + + + public function testKeyPairMismatch(): void + { + // The public keys swapped between the two ids: each secret key gets a public key from a different pair + $swappedPublicKeys = [ + self::INACTIVE_KEY => $this->publicKeys[self::ACTIVE_KEY], + self::ACTIVE_KEY => $this->publicKeys[self::INACTIVE_KEY], + ]; + Assert::exception( + function () use ($swappedPublicKeys): void { + new AnonymousPublicKeyEncryption($this->secretKeys, $swappedPublicKeys, self::ACTIVE_KEY, self::KEY_PREFIX); + }, + KeyPairMismatchException::class, + "Public key 'dev1' is not the public half of secret key 'dev1'", + ); + } + + + public function testRotationWithOldSecretAndNewPublicKey(): void + { + // Re-encryption after a rotation runs where the secret keys live: the old secret key decrypts, + // the new public key encrypts, and the new secret key doesn't have to be present at all + $oldKeyEncryption = new AnonymousPublicKeyEncryption([self::INACTIVE_KEY => $this->secretKeys[self::INACTIVE_KEY]], [], self::INACTIVE_KEY, self::KEY_PREFIX); + $oldData = $oldKeyEncryption->encrypt(self::PLAINTEXT); + + $rotation = new AnonymousPublicKeyEncryption( + [self::INACTIVE_KEY => $this->secretKeys[self::INACTIVE_KEY]], + [self::ACTIVE_KEY => $this->publicKeys[self::ACTIVE_KEY]], + self::ACTIVE_KEY, + self::KEY_PREFIX, + ); + Assert::true($rotation->needsReEncrypt($oldData)); + $newData = $rotation->encrypt($rotation->decrypt($oldData)); + Assert::false($rotation->needsReEncrypt($newData)); + Assert::same(self::PLAINTEXT, $this->encryption->decrypt($newData)); + } + + + public function testEncryptInactiveKeyDecrypt(): void + { + $inactiveKeyEncryption = new AnonymousPublicKeyEncryption($this->secretKeys, $this->publicKeys, self::INACTIVE_KEY, self::KEY_PREFIX); + Assert::same(self::PLAINTEXT, $this->encryption->decrypt($inactiveKeyEncryption->encrypt(self::PLAINTEXT))); + } + + + public function testNeedsReEncrypt(): void + { + $inactiveKeyEncryption = new AnonymousPublicKeyEncryption($this->secretKeys, $this->publicKeys, self::INACTIVE_KEY, self::KEY_PREFIX); + Assert::false($inactiveKeyEncryption->needsReEncrypt($inactiveKeyEncryption->encrypt(self::PLAINTEXT))); + Assert::true($this->encryption->needsReEncrypt($inactiveKeyEncryption->encrypt(self::PLAINTEXT))); + Assert::true($inactiveKeyEncryption->needsReEncrypt($this->encryption->encrypt(self::PLAINTEXT))); + } + + + public function testNoAdditionalDataMethods(): void + { + // Deliberately absent: this flavor can't bind the encrypted value to a context, + // and no method at all beats a method that can only throw + Assert::false(method_exists($this->encryption, 'encryptWithAd')); + Assert::false(method_exists($this->encryption, 'decryptWithAd')); + } + + + public function testConstructorActiveKeyIdNotFound(): void + { + $e = Assert::exception( + function (): void { + new AnonymousPublicKeyEncryption($this->secretKeys, $this->publicKeys, 'foo', self::KEY_PREFIX); + }, + ActiveKeyIdNotFoundException::class, + "Unknown encryption key id: 'foo'", + ); + Assert::type(UnknownEncryptionKeyIdException::class, $e); + Assert::type(OutOfRangeException::class, $e); + Assert::exception( + function (): void { + new AnonymousPublicKeyEncryption([], [], 'foo', self::KEY_PREFIX); + }, + ActiveKeyIdNotFoundException::class, + ); + } + + + public function testDecryptUnknownKeyId(): void + { + Assert::exception( + function (): void { + $this->encryption->decrypt('$unknown$x'); + }, + UnknownEncryptionKeyIdException::class, + "Unknown encryption key id: 'unknown'", + ); + // The key id comes from stored data, so a tampered value must not push arbitrary bytes into logs + Assert::exception( + function (): void { + $this->encryption->decrypt('$' . "bad\nid" . '$AnonV1$x'); + }, + UnknownEncryptionKeyIdException::class, + "Unknown encryption key id: 'bad?id'", + ); + } + + + /** @dataProvider getInvalidEncryptedData */ + public function testDecryptInvalidCipherTextFormat(string $invalidData): void + { + $e = Assert::exception( + function () use ($invalidData) { + $this->encryption->decrypt($invalidData); + }, + InvalidCipherTextFormatException::class, + "Data format must be '\$keyId\$marker\$ciphertext' or '\$keyId\$ciphertext'", + ); + Assert::type(OutOfBoundsException::class, $e); + } + + + /** + * @return list + */ + public function getInvalidEncryptedData(): array + { + return [ + ['nothing'], + [''], + ['$keyId'], + ['$keyId$marker$ciphertext$whatsDiz'], + ['garbage$keyId$ciphertext'], + ['$keyId$'], + ['$keyId$marker$'], + ['$$marker$ciphertext'], + ['$$ciphertext'], + ['$$'], + ]; + } + + + public function testDecryptInvalidNumberOfComponents(): void + { + $e = Assert::exception( + function (): void { + $this->encryption->decrypt('nothing'); + }, + InvalidNumberOfComponentsException::class, + "Data format must be '\$keyId\$marker\$ciphertext' or '\$keyId\$ciphertext'", + ); + Assert::type(InvalidCipherTextFormatException::class, $e); + Assert::type(OutOfBoundsException::class, $e); + } + + + public function testNeedsReEncryptInvalidCipherTextFormat(): void + { + $e = Assert::exception( + function (): void { + $this->encryption->needsReEncrypt('foo$bar$baz'); + }, + InvalidCipherTextFormatException::class, + ); + // The format guards throw the base class, only the component count check throws the subclass + Assert::false($e instanceof InvalidNumberOfComponentsException); + } + + + public function testForeignCipherTextFailsCleanly(): void + { + // The symmetric test's pinned ciphertext under the same key id: Halite reports any value + // it cannot decrypt as a wrong key, corrupted data and data from the other classes look the same + $symmetricCipherText = '$fixture$MUIFAFMn4bpPdCBV2amSVcLrvBf1a1wlFG_tchfj5GtWwmmYjSYoE7xC5eDbsBMUQ-DbSPW6SPDEJWsef_i2QSXASoOORvWozIIBAXs-Cpsu0kx4ANL81yzSKM8YR9_MqW9RcIpzu6YVYZNXz5DadkJcc8R52YrAr34i7K3QTyNPEg=='; + Assert::exception( + function () use ($symmetricCipherText): void { + $encryption = new AnonymousPublicKeyEncryption([self::FIXTURE_KEY_ID => self::KEY_PREFIX . '_secret_' . self::FIXTURE_SECRET_KEY_HEX], [], self::FIXTURE_KEY_ID, self::KEY_PREFIX); + $encryption->decrypt($symmetricCipherText); + }, + InvalidKey::class, + 'Incorrect secret key for this sealed message', + ); + } + + + public function testSensitiveParameterAttributes(): void + { + // Public keys aren't secret, but a secret key mispasted into the public keys array is exactly + // the value that would otherwise show up in a trace, so both constructor arrays are masked + $parameters = [ + (new ReflectionMethod(AnonymousPublicKeyEncryption::class, '__construct'))->getParameters()[0], + (new ReflectionMethod(AnonymousPublicKeyEncryption::class, '__construct'))->getParameters()[1], + (new ReflectionMethod(AnonymousPublicKeyEncryption::class, 'encrypt'))->getParameters()[0], + ]; + foreach ($parameters as $parameter) { + Assert::count(1, $parameter->getAttributes(SensitiveParameter::class)); + } + } + + + public function testHiddenStringKeys(): void + { + $object = print_r(new AnonymousPublicKeyEncryption($this->secretKeys, $this->publicKeys, self::ACTIVE_KEY, self::KEY_PREFIX), true); + // 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->secretKeys as $key) { + Assert::notContains($key, $object); + Assert::notContains(sodium_hex2bin(substr($key, strlen(self::KEY_PREFIX . '_secret_'))), $object); + } + foreach ($this->publicKeys as $key) { + Assert::notContains($key, $object); + Assert::notContains(sodium_hex2bin(substr($key, strlen(self::KEY_PREFIX . '_public_'))), $object); + } + } + + + public function testConstructorInvalidKeyId(): void + { + // An empty key id would produce '$$' which the parser rejects + Assert::exception( + function (): void { + new AnonymousPublicKeyEncryption(['' => $this->secretKeys[self::ACTIVE_KEY]], [], '', self::KEY_PREFIX); + }, + InvalidKeyIdException::class, + 'Key id must not be empty', + ); + // A key id with the separator would encrypt fine but produce output that can never be decrypted + Assert::exception( + function (): void { + new AnonymousPublicKeyEncryption([], ['key$1' => $this->publicKeys[self::ACTIVE_KEY]], 'key$1', self::KEY_PREFIX); + }, + InvalidKeyIdException::class, + "Key id 'key\$1' must not contain '\$'", + ); + } + + + 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 + $secretKeys = ['1' => $this->secretKeys[self::ACTIVE_KEY]]; + Assert::same([0 => 1], array_keys($secretKeys)); // the id is an int now, there's no way to keep it a string + $encryption = new AnonymousPublicKeyEncryption($secretKeys, [], '1', self::KEY_PREFIX); + $encrypted = $encryption->encrypt(self::PLAINTEXT); + Assert::same('$1$', substr($encrypted, 0, 3)); + Assert::same(self::PLAINTEXT, $encryption->decrypt($encrypted)); + Assert::false($encryption->needsReEncrypt($encrypted)); + // And a mismatched pair under a numeric id must still be reported with the id as configured + Assert::exception( + function () use ($secretKeys): void { + new AnonymousPublicKeyEncryption($secretKeys, ['1' => $this->publicKeys[self::INACTIVE_KEY]], '1', self::KEY_PREFIX); + }, + KeyPairMismatchException::class, + "Public key '1' is not the public half of secret key '1'", + ); + } + + + public function testConstructorInvalidKeyRole(): void + { + // A secret key pasted where a public key belongs would produce data nobody can decrypt, + // the tag makes that fail when the object is created instead + Assert::exception( + function (): void { + new AnonymousPublicKeyEncryption([], $this->secretKeys, self::ACTIVE_KEY, self::KEY_PREFIX); + }, + InvalidKeyRoleException::class, + "Key 'dev1' is tagged as a secret key but is used as a public key", + ); + Assert::exception( + function (): void { + new AnonymousPublicKeyEncryption($this->publicKeys, [], self::ACTIVE_KEY, self::KEY_PREFIX); + }, + InvalidKeyRoleException::class, + "Key 'dev1' is tagged as a public key but is used as a secret key", + ); + } + + + public function testConstructorUntaggedKeys(): void + { + // Values without the secret/public tag are accepted so existing configurations keep working unchanged, + // tagged and untagged values can live in the same array (which is what a migration looks like), + // and the pair check still runs on untagged values + $secretKeys = $this->secretKeys; + $secretKeys[self::INACTIVE_KEY] = str_replace('_secret_', '_', $secretKeys[self::INACTIVE_KEY]); + $publicKeys = []; + foreach ($this->publicKeys as $id => $key) { + $publicKeys[$id] = str_replace('_public_', '_', $key); + } + $untagged = new AnonymousPublicKeyEncryption($secretKeys, $publicKeys, self::ACTIVE_KEY, self::KEY_PREFIX); + Assert::same(self::PLAINTEXT, $untagged->decrypt($this->encryption->encrypt(self::PLAINTEXT))); + // And a round trip through the untagged key id proves it decodes the same with and without the tag + $inactiveKeyEncryption = new AnonymousPublicKeyEncryption($secretKeys, $publicKeys, self::INACTIVE_KEY, self::KEY_PREFIX); + Assert::same(self::PLAINTEXT, $untagged->decrypt($inactiveKeyEncryption->encrypt(self::PLAINTEXT))); + Assert::exception( + function () use ($secretKeys): void { + new AnonymousPublicKeyEncryption($secretKeys, [self::ACTIVE_KEY => str_replace('_public_', '_', $this->publicKeys[self::INACTIVE_KEY])], self::ACTIVE_KEY, self::KEY_PREFIX); + }, + KeyPairMismatchException::class, + ); + } + + + public function testConstructorInvalidKeyLength(): void + { + $shortKey = bin2hex(random_bytes(16)); + $e = Assert::exception( + function () use ($shortKey): void { + new AnonymousPublicKeyEncryption(['short' => self::KEY_PREFIX . '_secret_' . $shortKey], [], 'short', self::KEY_PREFIX); + }, + InvalidKeyLengthException::class, + "Key 'short' must be 32 bytes (64 hexadecimal characters) but is 16 bytes", + ); + assert($e instanceof InvalidKeyLengthException); + Assert::notContains($shortKey, $e->getMessage()); + // The public keys array is validated the same way as the secret keys array + Assert::exception( + function (): void { + new AnonymousPublicKeyEncryption([], ['bytes31' => self::KEY_PREFIX . '_public_' . bin2hex(random_bytes(31))], 'bytes31', self::KEY_PREFIX); + }, + InvalidKeyLengthException::class, + "Key 'bytes31' must be 32 bytes (64 hexadecimal characters) but is 31 bytes", + ); + } + + + public function testConstructorInvalidKeyEncoding(): void + { + $truncatedKey = substr(bin2hex(random_bytes(32)), 0, 63); + $e = Assert::exception( + function () use ($truncatedKey): void { + new AnonymousPublicKeyEncryption(['truncated' => self::KEY_PREFIX . '_secret_' . $truncatedKey], [], 'truncated', self::KEY_PREFIX); + }, + InvalidKeyEncodingException::class, + "Key 'truncated' is not a valid hex-encoded string", + ); + assert($e instanceof InvalidKeyEncodingException); + Assert::type(SodiumException::class, $e->getPrevious()); + // The public keys array is validated the same way as the secret keys array + Assert::exception( + function (): void { + new AnonymousPublicKeyEncryption([], ['nonhex' => self::KEY_PREFIX . '_public_' . str_repeat('xy', 32)], 'nonhex', self::KEY_PREFIX); + }, + InvalidKeyEncodingException::class, + ); + } + + + public function testConstructorExceptionsDoNotLeakKeyMaterial(): void + { + // No `use` capture on purpose: captured variables show up in raw traces of the closure frame itself + $exceptions = [ + Assert::exception( + function (): void { + new AnonymousPublicKeyEncryption(['truncated' => self::KEY_PREFIX . '_secret_' . self::TRUNCATED_KEY], [], 'truncated', self::KEY_PREFIX); + }, + InvalidKeyEncodingException::class, + ), + // The prefix appended instead of prepended, so it's the key material that comes before the separator + Assert::exception( + function (): void { + new AnonymousPublicKeyEncryption(['inverted' => self::TRUNCATED_KEY . '_' . self::KEY_PREFIX], [], 'inverted', self::KEY_PREFIX); + }, + InvalidKeyPrefixException::class, + ), + Assert::exception( + function (): void { + new AnonymousPublicKeyEncryption(['bare' => self::TRUNCATED_KEY], [], 'bare', self::KEY_PREFIX); + }, + MissingKeyPrefixException::class, + ), + // A secret-tagged value in the public keys array: the message names the id and the tags, never the key itself + Assert::exception( + function (): void { + new AnonymousPublicKeyEncryption([], ['swapped' => self::KEY_PREFIX . '_secret_' . self::TRUNCATED_KEY], 'swapped', self::KEY_PREFIX); + }, + InvalidKeyRoleException::class, + ), + // A mismatched pair: the message names the id only ('a' appended to make the truncated key valid hex again) + Assert::exception( + function (): void { + new AnonymousPublicKeyEncryption( + ['mismatched' => self::KEY_PREFIX . '_secret_' . self::TRUNCATED_KEY . 'a'], + ['mismatched' => self::KEY_PREFIX . '_public_' . self::TRUNCATED_KEY . 'a'], + 'mismatched', + self::KEY_PREFIX, + ); + }, + KeyPairMismatchException::class, + ), + ]; + $needle = substr(self::TRUNCATED_KEY, 0, 15); // getTraceAsString() truncates string arguments, check a prefix + foreach ($exceptions as $e) { + while ($e !== null) { + Assert::notContains($needle, $e->getMessage()); + Assert::notContains($needle, $e->getTraceAsString()); + Assert::notContains($needle, print_r($e->getTrace(), true)); + $e = $e->getPrevious(); + } + } + } + + + private function createFixtureEncryption(): AnonymousPublicKeyEncryption + { + return new AnonymousPublicKeyEncryption([self::FIXTURE_KEY_ID => self::KEY_PREFIX . '_secret_' . self::FIXTURE_SECRET_KEY_HEX], [], self::FIXTURE_KEY_ID, self::KEY_PREFIX); + } + + + public function testInvalidKeyPrefix(): void + { + // No separator at all, so there's no prefix to be wrong in the first place + $e = Assert::exception(function (): void { + new AnonymousPublicKeyEncryption(['foo' => 'keyMaterial'], [], self::ACTIVE_KEY, self::KEY_PREFIX); + }, MissingKeyPrefixException::class, "Key 'foo' must start with 'prefix_'"); + Assert::type(InvalidKeyPrefixException::class, $e); + $e = Assert::exception(function (): void { + new AnonymousPublicKeyEncryption(['foo' => self::KEY_PREFIX . 'Invalid_keyMaterial'], [], self::ACTIVE_KEY, self::KEY_PREFIX); + }, InvalidKeyPrefixException::class, "Key 'foo' must start with 'prefix_'"); + // There is a prefix, it's just the wrong one, only the no-separator case throws the subclass + Assert::false($e instanceof MissingKeyPrefixException); + } + +} + +(new AnonymousPublicKeyEncryptionTest())->run(); diff --git a/tests/AuthenticatedPublicKeyEncryptionTest.phpt b/tests/AuthenticatedPublicKeyEncryptionTest.phpt new file mode 100644 index 0000000..ead3034 --- /dev/null +++ b/tests/AuthenticatedPublicKeyEncryptionTest.phpt @@ -0,0 +1,719 @@ + */ + private array $ourSecretKeys; + + /** @var array */ + private array $ourPublicKeys; + + /** @var array */ + private array $theirSecretKeys; + + /** @var array */ + private array $theirPublicKeys; + + private AuthenticatedPublicKeyEncryption $encryption; + + + protected function setUp(): void + { + $this->ourSecretKeys = []; + $this->ourPublicKeys = []; + $this->theirSecretKeys = []; + $this->theirPublicKeys = []; + foreach ([self::INACTIVE_KEY, self::ACTIVE_KEY] as $id) { + $ourKeyPair = sodium_crypto_box_keypair(); + $theirKeyPair = sodium_crypto_box_keypair(); + $this->ourSecretKeys[$id] = self::KEY_PREFIX . '_secret_' . bin2hex(sodium_crypto_box_secretkey($ourKeyPair)); + $this->ourPublicKeys[$id] = self::KEY_PREFIX . '_public_' . bin2hex(sodium_crypto_box_publickey($ourKeyPair)); + $this->theirSecretKeys[$id] = self::KEY_PREFIX . '_secret_' . bin2hex(sodium_crypto_box_secretkey($theirKeyPair)); + $this->theirPublicKeys[$id] = self::KEY_PREFIX . '_public_' . bin2hex(sodium_crypto_box_publickey($theirKeyPair)); + } + $this->encryption = new AuthenticatedPublicKeyEncryption($this->ourSecretKeys, $this->theirPublicKeys, self::ACTIVE_KEY, self::KEY_PREFIX); + } + + + public function testEncryptDecrypt(): void + { + Assert::same(self::PLAINTEXT, $this->encryption->decrypt($this->encryption->encrypt(self::PLAINTEXT))); + } + + + public function testDecryptStoredCipherText(): void + { + // Generated once when the feature was added and kept verbatim, because the output of this library is stored + // in databases: anything that changes the format or the key handling has to fail here first + $encryption = $this->createFixtureEncryption(); + Assert::same(self::PLAINTEXT, $encryption->decrypt(self::FIXTURE_CIPHERTEXT)); + Assert::same(self::PLAINTEXT, $encryption->decryptWithAd(self::FIXTURE_CIPHERTEXT_WITH_AD, self::FIXTURE_AD)); + // The same two keys work in both directions, so a value encrypted by the other party decrypts with our config + Assert::same(self::PLAINTEXT, $encryption->decrypt(self::FIXTURE_CIPHERTEXT_FROM_OTHER_PARTY)); + Assert::false($encryption->needsReEncrypt(self::FIXTURE_CIPHERTEXT)); + Assert::false($encryption->needsReEncrypt(self::FIXTURE_CIPHERTEXT_WITH_AD)); + // The encrypted part has the same shape as the symmetric class's output, the marker tells them apart + Assert::same('$fixture$AuthV1$MUIFA', substr(self::FIXTURE_CIPHERTEXT, 0, 21)); + Assert::same('$fixture$AuthAdV1$MUIFA', substr(self::FIXTURE_CIPHERTEXT_WITH_AD, 0, 23)); + } + + + public function testDecryptStoredLegacyCipherText(): void + { + // Values in the format without the marker, like the ones an older library wrote, have to keep decrypting, + // and needsReEncrypt() reports them so a re-encryption sweep migrates them to the marked format + $encryption = $this->createFixtureEncryption(); + Assert::same(self::PLAINTEXT, $encryption->decrypt(self::LEGACY_FIXTURE_CIPHERTEXT)); + Assert::same(self::PLAINTEXT, $encryption->decryptWithAd(self::LEGACY_FIXTURE_CIPHERTEXT_WITH_AD, self::FIXTURE_AD)); + Assert::true($encryption->needsReEncrypt(self::LEGACY_FIXTURE_CIPHERTEXT)); + Assert::true($encryption->needsReEncrypt(self::LEGACY_FIXTURE_CIPHERTEXT_WITH_AD)); + } + + + public function testBothDirections(): void + { + // The other party configures the mirror image: their own secret key and our public key + $otherParty = new AuthenticatedPublicKeyEncryption($this->theirSecretKeys, $this->ourPublicKeys, self::ACTIVE_KEY, self::KEY_PREFIX); + Assert::same(self::PLAINTEXT, $otherParty->decrypt($this->encryption->encrypt(self::PLAINTEXT))); + Assert::same(self::PLAINTEXT, $this->encryption->decrypt($otherParty->encrypt(self::PLAINTEXT))); + + $ad = 'context'; + Assert::same(self::PLAINTEXT, $otherParty->decryptWithAd($this->encryption->encryptWithAd(self::PLAINTEXT, $ad), $ad)); + Assert::same(self::PLAINTEXT, $this->encryption->decryptWithAd($otherParty->encryptWithAd(self::PLAINTEXT, $ad), $ad)); + } + + + public function testEncryptDecryptWithAd(): void + { + $ad = 'context'; + Assert::same(self::PLAINTEXT, $this->encryption->decryptWithAd($this->encryption->encryptWithAd(self::PLAINTEXT, $ad), $ad)); + } + + + public function testDecryptWithWrongAdFails(): void + { + $encrypted = $this->encryption->encryptWithAd(self::PLAINTEXT, 'context1'); + Assert::exception(function () use ($encrypted) { + $this->encryption->decryptWithAd($encrypted, 'context2'); + }, InvalidMessage::class); + } + + + public function testPairingFails(): void + { + // New values carry the marker, so mixing up the methods is caught by this library with a message + // that says what to call, before the decryption itself would fail + $encryptedWithAd = $this->encryption->encryptWithAd(self::PLAINTEXT, 'context'); + Assert::exception(function () use ($encryptedWithAd) { + $this->encryption->decrypt($encryptedWithAd); + }, FormatMarkerMismatchException::class, 'Data was encrypted with AuthenticatedPublicKeyEncryption::encryptWithAd(), decrypt it there with decryptWithAd()'); + + $encrypted = $this->encryption->encrypt(self::PLAINTEXT); + Assert::exception(function () use ($encrypted) { + $this->encryption->decryptWithAd($encrypted, 'context'); + }, FormatMarkerMismatchException::class, 'Data was encrypted with AuthenticatedPublicKeyEncryption::encrypt(), decrypt it there with decrypt()'); + + // Values without the marker have no such protection and fail only when the decryption itself does + $fixtureEncryption = $this->createFixtureEncryption(); + Assert::exception(function () use ($fixtureEncryption) { + $fixtureEncryption->decrypt(self::LEGACY_FIXTURE_CIPHERTEXT_WITH_AD); + }, InvalidMessage::class); + Assert::exception(function () use ($fixtureEncryption) { + $fixtureEncryption->decryptWithAd(self::LEGACY_FIXTURE_CIPHERTEXT, self::FIXTURE_AD); + }, InvalidMessage::class); + } + + + public function testKeyIdTamperingDetected(): void + { + // The key id and the marker go into what the decryption verifies. With every id mapping + // to a different key a flipped id fails anyway, so the test uses the same key under two ids + // (which the docs forbid) to prove the id itself is verified, not just the key it selects + $encryption = new AuthenticatedPublicKeyEncryption( + ['key1' => $this->ourSecretKeys[self::ACTIVE_KEY], 'key2' => $this->ourSecretKeys[self::ACTIVE_KEY]], + ['key1' => $this->theirPublicKeys[self::ACTIVE_KEY], 'key2' => $this->theirPublicKeys[self::ACTIVE_KEY]], + 'key1', + self::KEY_PREFIX, + ); + $tampered = str_replace('$key1$', '$key2$', $encryption->encrypt(self::PLAINTEXT)); + Assert::exception( + function () use ($encryption, $tampered): void { + $encryption->decrypt($tampered); + }, + InvalidMessage::class, + ); + } + + + public function testMarkerStrippingDetected(): void + { + // Rewriting a marked value into the older format must not decrypt it as if it were genuine old data: + // the marker went into what the decryption verifies, so the downgrade fails there, not in the parser + $stripped = str_replace('$AuthV1$', '$', $this->encryption->encrypt(self::PLAINTEXT)); + Assert::exception(function () use ($stripped): void { + $this->encryption->decrypt($stripped); + }, InvalidMessage::class); + + $strippedWithAd = str_replace('$AuthAdV1$', '$', $this->encryption->encryptWithAd(self::PLAINTEXT, 'context')); + Assert::exception(function () use ($strippedWithAd): void { + $this->encryption->decryptWithAd($strippedWithAd, 'context'); + }, InvalidMessage::class); + } + + + public function testFormatMarkerMismatch(): void + { + // A value created by the other class names its creator instead of failing with a misleading decryption error + Assert::exception( + function (): void { + $this->encryption->decrypt('$' . self::ACTIVE_KEY . '$AnonV1$vJpOCa5fgshA9i4tKlRhW0OX6xd5iZeI'); + }, + FormatMarkerMismatchException::class, + 'Data was encrypted with AnonymousPublicKeyEncryption, decrypt it there', + ); + } + + + public function testUnknownFormatMarker(): void + { + Assert::exception( + function (): void { + $this->encryption->needsReEncrypt('$' . self::ACTIVE_KEY . '$AuthV9$whatever'); + }, + UnknownFormatMarkerException::class, + "Unknown format marker 'AuthV9', was the data encrypted by a newer version of this library?", + ); + // The marker comes from stored data, so a tampered value must not push arbitrary bytes into logs + $e = Assert::exception( + function (): void { + $this->encryption->needsReEncrypt('$' . self::ACTIVE_KEY . '$' . "bad\nmarker" . '$whatever'); + }, + UnknownFormatMarkerException::class, + "Unknown format marker 'bad?marker', was the data encrypted by a newer version of this library?", + ); + assert($e instanceof UnknownFormatMarkerException); + Assert::notContains("\n", $e->getMessage()); + Assert::exception( + function (): void { + $this->encryption->needsReEncrypt('$' . self::ACTIVE_KEY . '$' . str_repeat('x', 100) . '$whatever'); + }, + UnknownFormatMarkerException::class, + "Unknown format marker '" . str_repeat('x', 20) . "...', was the data encrypted by a newer version of this library?", + ); + } + + + public function testEmptyAdGuard(): void + { + Assert::exception(function () { + $this->encryption->encryptWithAd(self::PLAINTEXT, ''); + }, EncryptWithAdNeedsAdditionalDataException::class, 'additionalData must not be empty; use encrypt() for values that are not context-bound'); + + $encryptedWithAd = $this->encryption->encryptWithAd(self::PLAINTEXT, 'context'); + Assert::exception(function () use ($encryptedWithAd) { + $this->encryption->decryptWithAd($encryptedWithAd, ''); + }, DecryptWithAdNeedsAdditionalDataException::class, 'additionalData must not be empty; use decrypt() for values that are not context-bound'); + } + + + public function testEncryptInactiveKeyDecrypt(): void + { + $inactiveKeyEncryption = new AuthenticatedPublicKeyEncryption($this->ourSecretKeys, $this->theirPublicKeys, self::INACTIVE_KEY, self::KEY_PREFIX); + Assert::same(self::PLAINTEXT, $this->encryption->decrypt($inactiveKeyEncryption->encrypt(self::PLAINTEXT))); + } + + + public function testNeedsReEncrypt(): void + { + $inactiveKeyEncryption = new AuthenticatedPublicKeyEncryption($this->ourSecretKeys, $this->theirPublicKeys, self::INACTIVE_KEY, self::KEY_PREFIX); + Assert::false($inactiveKeyEncryption->needsReEncrypt($inactiveKeyEncryption->encrypt(self::PLAINTEXT))); + Assert::true($this->encryption->needsReEncrypt($inactiveKeyEncryption->encrypt(self::PLAINTEXT))); + Assert::true($inactiveKeyEncryption->needsReEncrypt($this->encryption->encrypt(self::PLAINTEXT))); + + $encryptedWithAd = $inactiveKeyEncryption->encryptWithAd(self::PLAINTEXT, 'context'); + Assert::false($inactiveKeyEncryption->needsReEncrypt($encryptedWithAd)); + Assert::true($this->encryption->needsReEncrypt($encryptedWithAd)); + } + + + public function testConstructorActiveKeyIdNotFound(): void + { + $e = Assert::exception( + function (): void { + new AuthenticatedPublicKeyEncryption($this->ourSecretKeys, $this->theirPublicKeys, 'foo', self::KEY_PREFIX); + }, + ActiveKeyIdNotFoundException::class, + "Unknown encryption key id: 'foo'", + ); + Assert::type(UnknownEncryptionKeyIdException::class, $e); + Assert::type(OutOfRangeException::class, $e); + Assert::exception( + function (): void { + new AuthenticatedPublicKeyEncryption([], [], 'foo', self::KEY_PREFIX); + }, + ActiveKeyIdNotFoundException::class, + ); + } + + + public function testConstructorIncompleteKeyPair(): void + { + Assert::exception( + function (): void { + new AuthenticatedPublicKeyEncryption($this->ourSecretKeys, [], self::ACTIVE_KEY, self::KEY_PREFIX); + }, + IncompleteKeyPairException::class, + "Key id 'dev1' needs both our secret key and the other party's public key", + ); + Assert::exception( + function (): void { + new AuthenticatedPublicKeyEncryption([], $this->theirPublicKeys, self::ACTIVE_KEY, self::KEY_PREFIX); + }, + IncompleteKeyPairException::class, + ); + // A numeric key id ends up as an integer array key but must still be reported as-is + Assert::exception( + function (): void { + new AuthenticatedPublicKeyEncryption(['1' => $this->ourSecretKeys[self::ACTIVE_KEY]], [], '1', self::KEY_PREFIX); + }, + IncompleteKeyPairException::class, + "Key id '1' needs both our secret key and the other party's public key", + ); + } + + + public function testConstructorInvalidKeyRole(): void + { + // A secret key pasted where a public key belongs would produce data nobody can decrypt, + // the tag makes that fail when the object is created instead + Assert::exception( + function (): void { + new AuthenticatedPublicKeyEncryption($this->ourSecretKeys, $this->theirSecretKeys, self::ACTIVE_KEY, self::KEY_PREFIX); + }, + InvalidKeyRoleException::class, + "Key 'dev1' is tagged as a secret key but is used as a public key", + ); + Assert::exception( + function (): void { + new AuthenticatedPublicKeyEncryption($this->theirPublicKeys, $this->theirPublicKeys, self::ACTIVE_KEY, self::KEY_PREFIX); + }, + InvalidKeyRoleException::class, + "Key 'dev1' is tagged as a public key but is used as a secret key", + ); + } + + + public function testConstructorUntaggedKeys(): void + { + // Values without the secret/public tag are accepted so existing configurations keep working unchanged, + // and tagged and untagged values can live in the same array, which is what a migration looks like + $secretKeys = $this->ourSecretKeys; + $secretKeys[self::INACTIVE_KEY] = str_replace('_secret_', '_', $secretKeys[self::INACTIVE_KEY]); + $publicKeys = []; + foreach ($this->theirPublicKeys as $id => $key) { + $publicKeys[$id] = str_replace('_public_', '_', $key); + } + $untagged = new AuthenticatedPublicKeyEncryption($secretKeys, $publicKeys, self::ACTIVE_KEY, self::KEY_PREFIX); + Assert::same(self::PLAINTEXT, $untagged->decrypt($this->encryption->encrypt(self::PLAINTEXT))); + Assert::same(self::PLAINTEXT, $this->encryption->decrypt($untagged->encrypt(self::PLAINTEXT))); + // And a round trip through the untagged key id proves it decodes the same with and without the tag + $inactiveKeyEncryption = new AuthenticatedPublicKeyEncryption($secretKeys, $publicKeys, self::INACTIVE_KEY, self::KEY_PREFIX); + Assert::same(self::PLAINTEXT, $untagged->decrypt($inactiveKeyEncryption->encrypt(self::PLAINTEXT))); + } + + + public function testDecryptUnknownKeyId(): void + { + Assert::exception( + function (): void { + $this->encryption->decrypt('$unknown$x'); + }, + UnknownEncryptionKeyIdException::class, + "Unknown encryption key id: 'unknown'", + ); + // The key id comes from stored data, so a tampered value must not push arbitrary bytes into logs + Assert::exception( + function (): void { + $this->encryption->decrypt('$' . "bad\nid" . '$AuthV1$x'); + }, + UnknownEncryptionKeyIdException::class, + "Unknown encryption key id: 'bad?id'", + ); + Assert::exception( + function (): void { + $this->encryption->decrypt('$' . str_repeat('k', 100) . '$AuthV1$x'); + }, + UnknownEncryptionKeyIdException::class, + "Unknown encryption key id: '" . str_repeat('k', 20) . "...'", + ); + } + + + /** @dataProvider getInvalidEncryptedData */ + public function testDecryptInvalidCipherTextFormat(string $invalidData): void + { + $e = Assert::exception( + function () use ($invalidData) { + $this->encryption->decrypt($invalidData); + }, + InvalidCipherTextFormatException::class, + "Data format must be '\$keyId\$marker\$ciphertext' or '\$keyId\$ciphertext'", + ); + Assert::type(OutOfBoundsException::class, $e); + Assert::exception( + function () use ($invalidData): void { + $this->encryption->decryptWithAd($invalidData, 'context'); + }, + InvalidCipherTextFormatException::class, + ); + } + + + /** + * @return list + */ + public function getInvalidEncryptedData(): array + { + return [ + ['nothing'], + [''], + ['$keyId'], + ['$keyId$marker$ciphertext$whatsDiz'], + ['garbage$keyId$ciphertext'], + ['$keyId$'], + ['$keyId$marker$'], + ['$$marker$ciphertext'], + ['$$ciphertext'], + ['$$'], + ]; + } + + + public function testDecryptInvalidNumberOfComponents(): void + { + $e = Assert::exception( + function (): void { + $this->encryption->decrypt('nothing'); + }, + InvalidNumberOfComponentsException::class, + "Data format must be '\$keyId\$marker\$ciphertext' or '\$keyId\$ciphertext'", + ); + Assert::type(InvalidCipherTextFormatException::class, $e); + Assert::type(OutOfBoundsException::class, $e); + } + + + public function testNeedsReEncryptInvalidCipherTextFormat(): void + { + $e = Assert::exception( + function (): void { + $this->encryption->needsReEncrypt('foo$bar$baz'); + }, + InvalidCipherTextFormatException::class, + ); + // The format guards throw the base class, only the component count check throws the subclass + Assert::false($e instanceof InvalidNumberOfComponentsException); + } + + + public function testSymmetricCipherTextFailsCleanly(): void + { + // The symmetric test's pinned ciphertext: same shape, same key id, different scheme — + // it parses fine and only fails once decryption detects the data wasn't made with these keys + $symmetricCipherText = '$fixture$MUIFAFMn4bpPdCBV2amSVcLrvBf1a1wlFG_tchfj5GtWwmmYjSYoE7xC5eDbsBMUQ-DbSPW6SPDEJWsef_i2QSXASoOORvWozIIBAXs-Cpsu0kx4ANL81yzSKM8YR9_MqW9RcIpzu6YVYZNXz5DadkJcc8R52YrAr34i7K3QTyNPEg=='; + Assert::exception( + function () use ($symmetricCipherText): void { + $this->createFixtureEncryption()->decrypt($symmetricCipherText); + }, + InvalidMessage::class, + 'Invalid message authentication code', + ); + } + + + public function testRandomDataFailsCleanly(): void + { + // Values that were never produced by this class miss the version marker and fail before any key is used + Assert::exception( + function (): void { + $this->encryption->decrypt('$' . self::ACTIVE_KEY . '$' . str_repeat('x', 96)); + }, + InvalidMessage::class, + 'Invalid version tag', + ); + } + + + public function testEncryptWithAdSensitiveParameter(): void + { + // The empty additionalData guard is the only throw site reachable with a valid construction, + // so this is where the SensitiveParameter masking of the plaintext argument can be observed in a trace + $e = Assert::exception( + function (): void { + $this->encryption->encryptWithAd(self::PLAINTEXT, ''); + }, + EncryptWithAdNeedsAdditionalDataException::class, + ); + assert($e instanceof EncryptWithAdNeedsAdditionalDataException); + Assert::notContains(self::PLAINTEXT, $e->getTraceAsString()); + Assert::contains('SensitiveParameterValue', $e->getTraceAsString()); + } + + + public function testSensitiveParameterAttributes(): void + { + // Public keys aren't secret, but a secret key mispasted into the public keys array is exactly + // the value that would otherwise show up in a trace, so both constructor arrays are masked + $parameters = [ + (new ReflectionMethod(AuthenticatedPublicKeyEncryption::class, '__construct'))->getParameters()[0], + (new ReflectionMethod(AuthenticatedPublicKeyEncryption::class, '__construct'))->getParameters()[1], + (new ReflectionMethod(AuthenticatedPublicKeyEncryption::class, 'encrypt'))->getParameters()[0], + (new ReflectionMethod(AuthenticatedPublicKeyEncryption::class, 'encryptWithAd'))->getParameters()[0], + ]; + foreach ($parameters as $parameter) { + Assert::count(1, $parameter->getAttributes(SensitiveParameter::class)); + } + } + + + public function testHiddenStringKeys(): void + { + $object = print_r(new AuthenticatedPublicKeyEncryption($this->ourSecretKeys, $this->theirPublicKeys, self::ACTIVE_KEY, self::KEY_PREFIX), true); + // 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->ourSecretKeys as $key) { + Assert::notContains($key, $object); + Assert::notContains(sodium_hex2bin(substr($key, strlen(self::KEY_PREFIX . '_secret_'))), $object); + } + foreach ($this->theirPublicKeys as $key) { + Assert::notContains($key, $object); + Assert::notContains(sodium_hex2bin(substr($key, strlen(self::KEY_PREFIX . '_public_'))), $object); + } + } + + + public function testConstructorInvalidKeyId(): void + { + // An empty key id would produce '$$' which the parser rejects + Assert::exception( + function (): void { + new AuthenticatedPublicKeyEncryption(['' => $this->ourSecretKeys[self::ACTIVE_KEY]], [], '', self::KEY_PREFIX); + }, + InvalidKeyIdException::class, + 'Key id must not be empty', + ); + // A key id with the separator would encrypt fine but produce output that can never be decrypted + Assert::exception( + function (): void { + new AuthenticatedPublicKeyEncryption(['key$1' => $this->ourSecretKeys[self::ACTIVE_KEY]], [], 'key$1', self::KEY_PREFIX); + }, + InvalidKeyIdException::class, + "Key id 'key\$1' must not contain '\$'", + ); + } + + + 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 + $secretKeys = ['1' => $this->ourSecretKeys[self::ACTIVE_KEY]]; + $publicKeys = ['1' => $this->theirPublicKeys[self::ACTIVE_KEY]]; + Assert::same([0 => 1], array_keys($secretKeys)); // the id is an int now, there's no way to keep it a string + $encryption = new AuthenticatedPublicKeyEncryption($secretKeys, $publicKeys, '1', self::KEY_PREFIX); + $encrypted = $encryption->encrypt(self::PLAINTEXT); + Assert::same('$1$', substr($encrypted, 0, 3)); + Assert::same(self::PLAINTEXT, $encryption->decrypt($encrypted)); + Assert::false($encryption->needsReEncrypt($encrypted)); + } + + + public function testConstructorInvalidKeyLength(): void + { + $shortKey = bin2hex(random_bytes(16)); + $e = Assert::exception( + function () use ($shortKey): void { + new AuthenticatedPublicKeyEncryption(['short' => self::KEY_PREFIX . '_secret_' . $shortKey], [], 'short', self::KEY_PREFIX); + }, + InvalidKeyLengthException::class, + "Key 'short' must be 32 bytes (64 hexadecimal characters) but is 16 bytes", + ); + assert($e instanceof InvalidKeyLengthException); + Assert::notContains($shortKey, $e->getMessage()); + // The public keys array is validated the same way as the secret keys array + Assert::exception( + function (): void { + new AuthenticatedPublicKeyEncryption($this->ourSecretKeys, [self::ACTIVE_KEY => self::KEY_PREFIX . '_public_' . bin2hex(random_bytes(31))], self::ACTIVE_KEY, self::KEY_PREFIX); + }, + InvalidKeyLengthException::class, + "Key 'dev2' must be 32 bytes (64 hexadecimal characters) but is 31 bytes", + ); + } + + + public function testConstructorInvalidKeyEncoding(): void + { + $truncatedKey = substr(bin2hex(random_bytes(32)), 0, 63); + $e = Assert::exception( + function () use ($truncatedKey): void { + new AuthenticatedPublicKeyEncryption(['truncated' => self::KEY_PREFIX . '_secret_' . $truncatedKey], [], 'truncated', self::KEY_PREFIX); + }, + InvalidKeyEncodingException::class, + "Key 'truncated' is not a valid hex-encoded string", + ); + assert($e instanceof InvalidKeyEncodingException); + Assert::type(SodiumException::class, $e->getPrevious()); + // The public keys array is validated the same way as the secret keys array + Assert::exception( + function (): void { + new AuthenticatedPublicKeyEncryption($this->ourSecretKeys, [self::ACTIVE_KEY => self::KEY_PREFIX . '_public_' . str_repeat('xy', 32)], self::ACTIVE_KEY, self::KEY_PREFIX); + }, + InvalidKeyEncodingException::class, + ); + } + + + public function testConstructorExceptionsDoNotLeakKeyMaterial(): void + { + // No `use` capture on purpose: captured variables show up in raw traces of the closure frame itself + $exceptions = [ + Assert::exception( + function (): void { + new AuthenticatedPublicKeyEncryption(['truncated' => self::KEY_PREFIX . '_secret_' . self::TRUNCATED_KEY], [], 'truncated', self::KEY_PREFIX); + }, + InvalidKeyEncodingException::class, + ), + // The prefix appended instead of prepended, so it's the key material that comes before the separator + Assert::exception( + function (): void { + new AuthenticatedPublicKeyEncryption(['inverted' => self::TRUNCATED_KEY . '_' . self::KEY_PREFIX], [], 'inverted', self::KEY_PREFIX); + }, + InvalidKeyPrefixException::class, + ), + Assert::exception( + function (): void { + new AuthenticatedPublicKeyEncryption(['bare' => self::TRUNCATED_KEY], [], 'bare', self::KEY_PREFIX); + }, + MissingKeyPrefixException::class, + ), + // A secret-tagged value in the public keys array: the message names the id and the tags, never the key itself + Assert::exception( + function (): void { + new AuthenticatedPublicKeyEncryption([], ['swapped' => self::KEY_PREFIX . '_secret_' . self::TRUNCATED_KEY], 'swapped', self::KEY_PREFIX); + }, + InvalidKeyRoleException::class, + ), + // A valid secret key with no matching public key: thrown after the key was decoded and stored + // ('a' appended to make the truncated key valid hex again) + Assert::exception( + function (): void { + new AuthenticatedPublicKeyEncryption(['incomplete' => self::KEY_PREFIX . '_secret_' . self::TRUNCATED_KEY . 'a'], [], 'incomplete', self::KEY_PREFIX); + }, + IncompleteKeyPairException::class, + ), + ]; + $needle = substr(self::TRUNCATED_KEY, 0, 15); // getTraceAsString() truncates string arguments, check a prefix + foreach ($exceptions as $e) { + while ($e !== null) { + Assert::notContains($needle, $e->getMessage()); + Assert::notContains($needle, $e->getTraceAsString()); + Assert::notContains($needle, print_r($e->getTrace(), true)); + $e = $e->getPrevious(); + } + } + } + + + public function testInvalidKeyPrefix(): void + { + // No separator at all, so there's no prefix to be wrong in the first place + $e = Assert::exception(function (): void { + new AuthenticatedPublicKeyEncryption(['foo' => 'keyMaterial'], [], self::ACTIVE_KEY, self::KEY_PREFIX); + }, MissingKeyPrefixException::class, "Key 'foo' must start with 'prefix_'"); + Assert::type(InvalidKeyPrefixException::class, $e); + $e = Assert::exception(function (): void { + new AuthenticatedPublicKeyEncryption(['foo' => self::KEY_PREFIX . 'Invalid_keyMaterial'], [], self::ACTIVE_KEY, self::KEY_PREFIX); + }, InvalidKeyPrefixException::class, "Key 'foo' must start with 'prefix_'"); + // There is a prefix, it's just the wrong one, only the no-separator case throws the subclass + Assert::false($e instanceof MissingKeyPrefixException); + } + + + private function createFixtureEncryption(): AuthenticatedPublicKeyEncryption + { + return new AuthenticatedPublicKeyEncryption( + [self::FIXTURE_KEY_ID => self::KEY_PREFIX . '_secret_' . self::FIXTURE_SECRET_KEY_HEX], + [self::FIXTURE_KEY_ID => self::KEY_PREFIX . '_public_' . $this->derivePublicKeyHex(self::FIXTURE_OTHER_PARTY_SECRET_KEY_HEX)], + self::FIXTURE_KEY_ID, + self::KEY_PREFIX, + ); + } + + + private function derivePublicKeyHex(string $secretKeyHex): string + { + return bin2hex(sodium_crypto_box_publickey_from_secretkey(sodium_hex2bin($secretKeyHex))); + } + +} + +(new AuthenticatedPublicKeyEncryptionTest())->run(); diff --git a/tests/Format/LogSafeValueTest.phpt b/tests/Format/LogSafeValueTest.phpt new file mode 100644 index 0000000..619d0fb --- /dev/null +++ b/tests/Format/LogSafeValueTest.phpt @@ -0,0 +1,42 @@ + + */ + public function getValues(): array + { + return [ + 'short printable value untouched' => ['key1', 'key1'], + 'empty value untouched' => ['', ''], + 'exactly at the limit untouched' => [str_repeat('a', 20), str_repeat('a', 20)], + 'one over the limit shortened' => [str_repeat('a', 21), str_repeat('a', 20) . '...'], + 'much longer shortened the same' => [str_repeat('a', 1000), str_repeat('a', 20) . '...'], + 'newline replaced' => ["bad\nid", 'bad?id'], + 'space replaced' => ['a b', 'a?b'], + 'control and non-ascii bytes replaced' => ["\x00\x09\xff", '???'], + 'shortened first, then sanitized' => [str_repeat('x', 19) . "\n" . str_repeat('y', 10), str_repeat('x', 19) . '?...'], + ]; + } + +} + +(new LogSafeValueTest())->run(); diff --git a/tests/SymmetricKeyEncryptionTest.phpt b/tests/SymmetricKeyEncryptionTest.phpt index f510ec2..5194f5f 100644 --- a/tests/SymmetricKeyEncryptionTest.phpt +++ b/tests/SymmetricKeyEncryptionTest.phpt @@ -362,6 +362,14 @@ class SymmetricKeyEncryptionTest extends TestCase }, InvalidKeyEncodingException::class, ); + // The secret/public tags of the public-key classes mean nothing here, a tagged value is just invalid hex: + // the symmetric class must never start interpreting the tags + Assert::exception( + function (): void { + new SymmetricKeyEncryption(['tagged' => self::KEY_PREFIX . '_secret_' . bin2hex(random_bytes(32))], 'tagged', self::KEY_PREFIX); + }, + InvalidKeyEncodingException::class, + ); }