Add asymmetric encryption between two parties and encryption to a public key - #38
Merged
Conversation
The upcoming asymmetric encryption classes need the same key validation (prefix, hex, length, key id checks) and the same `$keyId$ciphertext` output handling as `SymmetricKeyEncryption`, so the constructor loop moves to `KeyEnvelope::decodeKeys()` and `parseKeyCipherText()`/`formatKeyCipherText()` move along with it, all verbatim. A trait rather than a helper class so everything can stay `private` and nothing new becomes part of the public API. The trait lives in the `Format` subnamespace, keeping the root namespace to the classes people actually use. `decodeKeys()` takes the expected key length as a parameter instead of reading `SODIUM_CRYPTO_STREAM_KEYBYTES` directly, and `InvalidKeyLengthException` gets the same parameter so its message no longer silently depends on a constant unrelated to the key being validated. The default keeps the message unchanged. No behavior change: the stored ciphertext still decrypts and every constructor validation message stays the same, which the existing tests prove.
…ion` 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). The methods, the empty additionalData guards, and the `$keyId$ciphertext` output shape all mirror `SymmetricKeyEncryption`, and the encrypted part is even format-identical to the symmetric output, only the key id tells the two apart. Every key id must have both keys, a missing half throws `IncompleteKeyPairException` because encryption and decryption both need the two keys. The key values can carry a `secret`/`public` tag between the prefix and the key: `adek_secret_79e0...` or `adek_public_d22c...`. The tag is optional so configurations in the plain `adek_79e0...` format keep working unchanged, but a tagged key found in the wrong array throws `InvalidKeyRoleException` when the object is created. That matters because a secret key pasted where a public key belongs would otherwise silently produce data that nobody can ever decrypt, and there's no other way to tell the two kinds apart, both are 32 bytes. The tag also tells you how bad a leaked string is: `adek_secret_` means rotate now, a public key is not a secret. The symmetric class keeps treating the tags as what they are to it, invalid hex, and a test now pins that so the shared code can never start interpreting the tags for symmetric keys. Both constructor arrays are marked `#[SensitiveParameter]`: 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 stack trace. The no-leak test caught the role tag check doing just that through its raw trace, hence the attribute on the internal method too. The stored ciphertext fixtures include one encrypted by the other party, pinning the promise that the same two keys work in both directions.
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. The point is the split: a server that only stores data is configured with just the public keys and cannot read anything back, not even the values it has just encrypted itself, so its compromise doesn't expose the stored data. Decryption happens elsewhere, wherever the secret keys live. A key id missing from the public keys array is derived from the secret key with the same id, so a decrypting deployment configures just the secret keys. When both values are 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. Decrypting a key id that only has a public key configured throws the new `MissingSecretKeyException`, deliberately not a subclass of `UnknownEncryptionKeyIdException`: the id is known, the deployment just can't decrypt, which usually means the code runs on the encrypt-only server. Re-encryption after a rotation therefore runs where the secret keys live, with the old secret key and just the new public key. There are no `encryptWithAd()`/`decryptWithAd()` methods because this flavor cannot bind the encrypted value to a context, and no method at all beats a method that can only throw. The output has the usual `$keyId$ciphertext` shape but the encrypted part carries no version marker, and any well-formed value that cannot be decrypted is reported by Halite as `InvalidKey: Incorrect secret key for this sealed message`, whether it's a wrong key, corrupted data, or data created by one of the other two classes; only a value that is not even valid base64 gets `InvalidMessage` instead. The README documents how to tell these apart; the tests pin the behavior.
…ryption
Values written by the two public-key classes now carry a marker between the key id and the encrypted part: `AuthV1` from `AuthenticatedPublicKeyEncryption::encrypt()`, `AuthAdV1` from `encryptWithAd()`, and `AnonV1` from `AnonymousPublicKeyEncryption`. The marker says what created a value, so feeding it to the wrong class or the wrong method fails with an exception that says where the value belongs and what to call, instead of a decryption error that just says the key is wrong. The marker values can never change; a future format change introduces new ones, which is why the version number is part of the marker and of the enum case name (with a `V` so the version reads as a version, not as part of the name).
Values in the older format without the marker keep decrypting, because this library is a drop-in replacement for a previous one that wrote the same format, and the data written by it is still out there. `needsReEncrypt()` now returns true for such values too, so the usual re-encryption sweep migrates them to the marked format as a side effect. The symmetric class is untouched and keeps rejecting anything that isn't exactly `$keyId$ciphertext`.
In `AuthenticatedPublicKeyEncryption` the key id and the marker also go into what the decryption verifies: they become a small JSON document, `{"keyId":...,"marker":...}` with an `additionalData` member when there is one, that is passed to Halite alongside the data. Changing the key id or the marker in a stored marked value now makes decryption fail, and so does rewriting a marked value into the older format, so the "key id is not protected against tampering" caveat no longer applies to new two-party values. Halite's plain `encrypt()` is internally `encryptWithAD()` with an empty string, so this uses the same operation it always did, just with a non-empty value. Tests prove the protection with the same key configured under two ids, which is exactly the misconfiguration it defends against, and with a marked value downgraded to the older format.
JSON is used rather than a hand-rolled length-prefixed format so the combining is outsourced to a well-tested serializer instead of a custom one. The values inside are the URL-safe kind of Base64, which means no JSON character escaping ever kicks in and the document can be rebuilt anywhere with plain string formatting, should the data ever need to be decrypted with Halite directly. The exact recipe can never change either, a changed recipe would be a new marker.
Sealed values have no place to verify anything extra, so in `AnonymousPublicKeyEncryption` the marker is a label only and the key id keeps the caveat there. And because the marker and the key id in a stored value can be anything, the exceptions that repeat them shorten the values and keep them printable instead of pushing arbitrary bytes into whatever logs the message.
There was a problem hiding this comment.
Pull request overview
This PR expands the library beyond symmetric encryption by adding two asymmetric encryption wrappers around Halite (authenticated two-party encryption and anonymous public-key encryption), plus shared formatting/key-handling infrastructure and updated documentation/tests to support the new stored ciphertext formats and migration story.
Changes:
- Added
AuthenticatedPublicKeyEncryption(two-party, authenticated) andAnonymousPublicKeyEncryption(public-key, encrypt-only capable) with versioned format markers and legacy unmarked compatibility. - Introduced shared formatting/key parsing/key decoding utilities in
src/Format/*, including marker handling, stored-format typing, and log-safe shortening/sanitization for stored-data-derived values. - Updated
SymmetricKeyEncryptionto reuse the shared key decoding/envelope logic, added/expanded tests, and updated the README to document the new APIs and formats.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/SymmetricKeyEncryptionTest.phpt | Adds coverage ensuring symmetric keys don’t start interpreting asymmetric key-role tags. |
| tests/Format/LogSafeValueTest.phpt | Adds tests for shortening/sanitizing values before including them in exception messages. |
| tests/AuthenticatedPublicKeyEncryptionTest.phpt | Comprehensive fixtures and behavioral tests for authenticated two-party encryption, marker behavior, and migration. |
| tests/AnonymousPublicKeyEncryptionTest.phpt | Comprehensive fixtures and behavioral tests for anonymous public-key encryption, marker behavior, and encrypt-only deployments. |
| src/SymmetricKeyEncryption.php | Refactors to use the shared KeyEnvelope trait for key decoding and ciphertext envelope parsing/formatting. |
| src/Format/StoredFormat.php | Adds an internal enum describing accepted stored-value shapes for error messages/validation. |
| src/Format/LogSafeValue.php | Adds internal helper to keep stored-data-derived values short/printable in exception messages. |
| src/Format/KeyEnvelope.php | Adds shared key decoding and ciphertext envelope parsing/formatting (+ marker/AD binding helpers). |
| src/Format/FormatMarker.php | Adds internal enum for persistent format markers (AuthV1, AuthAdV1, AnonV1). |
| src/Format/AsymmetricKeyRole.php | Adds internal enum for optional key-role tags (secret/public) in key config strings. |
| src/Exceptions/UnknownFormatMarkerException.php | New exception for unknown markers with log-safe marker rendering. |
| src/Exceptions/UnknownEncryptionKeyIdException.php | Sanitizes key-id values in messages using LogSafeValue (stored-data derived). |
| src/Exceptions/MissingSecretKeyException.php | New exception for encrypt-only deployments trying to decrypt, with log-safe key-id rendering. |
| src/Exceptions/KeyPairMismatchException.php | New exception for mismatched secret/public key pair validation in anonymous public-key mode. |
| src/Exceptions/InvalidKeyRoleException.php | New exception for secret/public tag mismatch vs expected role in key arrays. |
| src/Exceptions/InvalidKeyLengthException.php | Adjusts constructor to include expected length (now used across symmetric/asymmetric decoding). |
| src/Exceptions/InvalidCipherTextFormatException.php | Adjusts constructor to include required stored-format shape(s) for clearer error messaging. |
| src/Exceptions/IncompleteKeyPairException.php | New exception for missing key counterparts (two-party mode requires both sides per key id). |
| src/Exceptions/FormatMarkerMismatchException.php | New exception with clear guidance when decrypting with the wrong class/method for a marked value. |
| src/AuthenticatedPublicKeyEncryption.php | New two-party authenticated public-key encryption implementation, including AD binding and marker-protected keyId/marker. |
| src/AnonymousPublicKeyEncryption.php | New public-key sealed-box encryption implementation supporting encrypt-only deployments and key-pair validation/derivation. |
| README.md | Documents new asymmetric APIs, configuration/tagging, format markers, migration behavior, and Nette service wiring examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
spaze
added a commit
that referenced
this pull request
Aug 1, 2026
…k test, fix a README parameter name (#39) Hardening leftovers from the reviews of #38, none of which belonged in that PR: - **Key ids shortened in exception messages** - a key pasted into an id slot in the configuration ends up repeated in constructor exception messages; the same shortening that already keeps markers and stored key ids out of logs now applies there too, so such a paste leaks at most the first 20 characters instead of the whole key. Ids of sane length are unaffected, every existing message stays the same. - **Symmetric `print_r()` leak test checks the decoded bytes** - the object stores only decoded raw bytes, never the `prefix_hex` config strings, so the old needles could not fail even with the `HiddenString` wrapping removed. The public-key class tests already check both forms; the symmetric test now does the same. - **README** - the runtime-call example read `%encryption.keyPrefixes.email%` but the parameters section defines the group as `prefixes`; copying the two snippets together would fail with an undefined parameter when the container compiles.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two new classes wrapping Halite's asymmetric encryption:
AuthenticatedPublicKeyEncryption- encryption between two parties: our secret key + the other side's public key per key id, both sides encrypt and decrypt, decryption also proves who created the data. Method-for-method mirror ofSymmetricKeyEncryption, includingencryptWithAd()/decryptWithAd().AnonymousPublicKeyEncryption- encryption to a public key: encrypt-only deployments configure just the public keys and cannot read data back. Public halves are derived from secret keys; a mismatched pair fails in the constructor. NoencryptWithAd(), sealed values can't bind a context.prefix_hexvalues as the symmetric class, so existing configs keep working. Recommended tagged formprefix_secret_hex/prefix_public_hex: a key pasted into the wrong slot fails at construction (InvalidKeyRoleException).$keyId$AuthV1$…,$keyId$AuthAdV1$…,$keyId$AnonV1$…. Unmarked values (in-house predecessor) decrypt forever;needsReEncrypt()flags them, so the usual re-encryption sweep migrates them. Wrong class or method fails with an exception naming where the value belongs.Format\:KeyEnvelopetrait,AsymmetricKeyRole,FormatMarker,StoredFormat,LogSafeValue. Values coming from stored data are shortened and made printable before landing in exception messages.InvalidCipherTextFormatExceptionrequires aStoredFormat,InvalidKeyLengthExceptionrequires the expected length). Consumers only catch these; catch sites are unaffected.Pinned ciphertext fixtures for both formats (including cross-direction, downgrade, and tamper cases), 100% coverage, PHPStan level max, business as usual.
Follow-ups: