feat(protected): NonEmpty context wrapper checked once at construction - #297
Open
coderdan wants to merge 3 commits into
Open
feat(protected): NonEmpty context wrapper checked once at construction#297coderdan wants to merge 3 commits into
coderdan wants to merge 3 commits into
Conversation
…ction
An AEAD associated-data value and a PRF context can both be empty, and
for callers that use one value to domain-separate fields that is a
security bug: equal plaintexts in different fields derive identical
index terms, every field shares one derived key, and ciphertexts become
transplantable. Downstream today rejects empties at runtime, on every
encrypt, by parsing vitaminc's PAE framing and mirroring private tag
constants — which has already produced a real false-rejection bug.
Move the invariant upstream and into the type:
- `IsEmpty`: structural emptiness decided on the source value before
any encoding, so no parsing and no mirrored constants. A composite is
empty only when it contributes no caller bytes at all.
- `NonEmpty<T>`: checked once at construction (`new`), or at compile
time for literals via `nonempty!` / `from_static`.
- `TryIntoNonEmpty`: one bound satisfied by both a plain `"users/email"`
and a proven `NonEmpty`, so APIs keep the plain-string call site while
still rejecting an empty runtime value.
- `IntoPrfContext` / `IntoAad` for `NonEmpty<T>` are byte-transparent,
so opting in changes nothing on the wire. `IsEmpty` for `PrfContext`
and `Aad` judges an already-encoded value on its bytes.
Empty AAD stays legal for anyone who does not opt in.
`nonempty!` expands to a named `const` item rather than an inline
`const {}` block: inline-const panics are post-monomorphisation errors
that `cargo check` (and trybuild) never see.
Closes #291
Claude-Session: https://claude.ai/code/session_01PWcS13jzUo9vjU7toeFhVY
Contributor
🧬 Mutation testing (cargo-mutants,
|
| caught | missed | unviable | timeout |
|---|---|---|---|
| 26 | 0 | 12 | 0 |
✅ Every mutant in the changed lines was caught by a test.
The nonempty_rejects_empty_literal negative-space test snapshots a const-eval panic backtrace. rustc renders that backtrace with core's source line when rust-src is installed and with a bare "the failure occurred here" note when it is not. The .stderr was blessed on a dev machine (where rust-analyzer already requires rust-src), so the Test and CRAP gates failed on the minimal toolchain profile. Installing the component in CI makes both environments render the same diagnostic. Claude-Session: https://claude.ai/code/session_01PWcS13jzUo9vjU7toeFhVY
Contributor
✅ No CRAP threshold violations556 function(s) analyzed · threshold 30 |
The mutants baseline runs the whole test suite, including the trybuild negative-space snapshots, so it needs rust-src for the same reason the Test and CRAP gates do. Without it the unmutated baseline failed and cargo-mutants exited 4 having tested nothing — yet the gate passed, because it only treated a missing outcomes.json as a failure and cargo-mutants still writes that file for the baseline attempt. Key the gate on exit 4 as well, so a broken baseline can no longer show up as a clean 0/0/0/0 run. Claude-Session: https://claude.ai/code/session_01PWcS13jzUo9vjU7toeFhVY
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.
Summary
Closes #291.
An AEAD associated-data value and a PRF context can both be empty; for callers that use one value to domain-separate fields (
stack-encrypt), an empty one collapses that separation. Downstream currently rejects empties at runtime, per encrypt, by parsing vitaminc's PAE framing and mirroring private tag constants — which already produced a real false-rejection bug (cipherstash-suite#2147).This moves the invariant upstream and into the type, without giving up the plain-string call site.
What's added (
vitaminc-protected)IsEmpty— structural emptiness, decided on the source value before encoding. No parsing, no mirrored constants.(),"",b"",None,Some(""),("", "")are empty; integers never are; a composite is empty only when it contributes no caller bytes at all (("", "email")is not empty).NonEmpty<T>— checked once at construction:NonEmpty::new(value)at runtime,nonempty!("users/email")/NonEmpty::from_staticat compile time (an empty literal fails to compile).TryIntoNonEmpty— one bound satisfied by both a plain"users/email"(checked structurally, once) and an already-provenNonEmpty(no check). This is what keepsencrypt_into(&cipher, "users/email")compiling while still rejecting an empty runtime value.EmptyError.vitaminc-prf/vitaminc-aeadIntoPrfContext/IntoAadforNonEmpty<T>— byte-transparent, so opting in changes nothing on the wire.IsEmptyforPrfContext/Aad— an already-encoded value is judged on its bytes (documented caveat: framing makes most encoded empties non-empty; the check belongs before encoding).Empty AAD remains legal for anyone who does not opt in.
Design notes
&'static strargument cannot be value-checked at compile time (""and"users/email"are the same type), so literals and dynamic values take different paths that meet at theTryIntoNonEmptybound.nonempty!expands to a namedconstitem rather than an inlineconst {}block: inline-const panics are post-monomorphisation errors thatcargo check— and therefore trybuild — never see.TryFrom<T> for NonEmpty<T>was left out: it conflicts with std's blanket impl since a downstream crate may legally addFrom<Local> for NonEmpty<Local>.0u64.into_aad()happens to equalNone.into_aad()(untaggedu64AAD leaf); that is anIntoAadencoding quirk, to be fixed separately.Downstream follow-up (
stack-encrypt)Add
TryIntoNonEmptyto theEncryptContext/DecryptContextsupertraits, convert once per target, then deleterequire_context,is_degenerate_aad,is_degenerate_prf_context,parse_pae, the mirroredprf_framingconstants, and bothEmptyContexterror variants. Call sites stay"users/email".Test plan
cargo test -p vitaminc-protected -p vitaminc-prf -p vitaminc-aead --all-features— unit tables mirroring downstream's pinned cases, quickcheck properties, trybuild compile-fail fornonempty!(""), doctestscargo clippy --workspace --all-targets --all-features -- -D warningsRUSTDOCFLAGS=-D warnings cargo doc --no-depscargo +1.85.1 check(MSRV)cargo fmt --all -- --checkhttps://claude.ai/code/session_01PWcS13jzUo9vjU7toeFhVY