docs: document mandatory .zeroize() call for secret types#49
Conversation
The zeroize feature (#14) derives Zeroize but not ZeroizeOnDrop, because the secret types are Copy. Secret material is therefore not wiped on drop and callers MUST call .zeroize() explicitly. Document this contract in the crate-level docs and add a MUST-call annotation to every SharedSecret, SecretKey and UserSecretKey across all KEM and IBE schemes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Rules Dobby 2 — consolidated review. Ran all 80 memory rules via parallel Haiku checks (all compliant) and merged with Review Dobby 2's findings. Docs build clean, --all-features suite passed. One non-blocking accuracy nit on the PR's scope claim.
Finding (nit — promised-vs-delivered): src/ibe/boyen_waters.rs:94 — the PR description states it annotates every SharedSecret across all KEM & IBE schemes, but boyen_waters::SharedSecret(Gt) (a public secret type whose bytes derive an AES key) received neither the # Zeroization doc section nor a Zeroize derive. This is arguably intentional/out-of-scope: the crate-level docs scope SharedSecret to crate::kem::SharedSecret (the KEMs), and this type does not derive Zeroize, so there is no .zeroize() to document. It is also pre-existing (predates this PR). Not blocking — the cleanest resolution is to narrow the PR-description claim (or add a one-line note that boyen_waters::SharedSecret is intentionally excluded because it is not Zeroize).
(Anchored in the body rather than inline: line 94 is outside this PR's diff hunks, so an inline comment there would be rejected by the reviews API.)
|
Addressed the scope nit in Re-verified: |
* feat: optional zeroize feature for secret types (#7) Adds a `zeroize` cargo feature that derives `zeroize::Zeroize` on the secret-bearing structs across every scheme: - `kem::cgw_kv::SecretKey` / `UserSecretKey` - `kem::cgw_fo::UserSecretKey` - `kem::kiltz_vahlis_one::SecretKey` / `UserSecretKey` - `ibe::cgw::SecretKey` / `UserSecretKey` - `ibe::waters::SecretKey` / `UserSecretKey` - `ibe::waters_naccache::SecretKey` / `UserSecretKey` - `ibe::boyen_waters::SecretKey` / `UserSecretKey` - `kem::SharedSecret` - `util::Identity` (used as `Id` in cgw_fo) The feature chains through to `pg-curve/zeroize` so the leaf `Scalar`, `G1Affine`, `G2Affine` and `Gt` types get their zeroize impls from the upstream crate. As noted in issue #7, these types are all `Copy` so `ZeroizeOnDrop` is not possible; users must call `.zeroize()` explicitly. The feature gives them the machinery to do so. Added a round-trip test under `cgwkv` (the scheme PostGuard uses): derives an MSK/USK/SharedSecret, zeroizes each, asserts the bytes change and that the shared-secret bytes are fully zero. Does NOT add `Zeroize` as a bound on the `IBKEM` / `IBE` traits themselves — that's a larger API decision and would retroactively require every external impl to opt in. Deferring. * style: cargo fmt * docs: document mandatory .zeroize() call for secret types (#49) The zeroize feature (#14) derives Zeroize but not ZeroizeOnDrop, because the secret types are Copy. Secret material is therefore not wiped on drop and callers MUST call .zeroize() explicitly. Document this contract in the crate-level docs and add a MUST-call annotation to every SharedSecret, SecretKey and UserSecretKey across all KEM and IBE schemes. Co-authored-by: dobby-yivi-agent[bot] <275734547+dobby-yivi-agent[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * fix: zeroize coverage for boyen_waters SharedSecret (#14) The PKE SharedSecret(Gt) was the one public secret-bearing type left without the zeroize derive or the MUST-call note. Its inner Gt field is private, so callers could neither .zeroize() it nor reach .0 to wipe it manually, leaving the crate's zeroization contract unachievable for this type. Add the same derive + # Zeroization note used on every other secret type, plus a shared_secret_zeroize test. Also correct the decrypt doc comment (returns Msg, not SharedSecret). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: dobby-yivi-agent[bot] <275734547+dobby-yivi-agent[bot]@users.noreply.github.com> Co-authored-by: dobby-coder[bot] <275734547+dobby-coder[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
What
Documents the zeroization contract for the secret types introduced by #14.
Because
SharedSecret,SecretKeyandUserSecretKeyareCopy, they cannot deriveZeroizeOnDrop(aCopytype cannot implementDrop). With thezeroizefeature they only deriveZeroize, so secret material is not wiped automatically on drop — callers MUST call.zeroize()explicitly.This PR adds the interim documentation requested in #45:
# Zeroizing secret materialsection in the crate-level docs (lib.rs) explaining the contract, with a usage example.# ZeroizationMUST-call annotation on every affected public secret type across all KEM and IBE schemes:kem::SharedSecret, the Boyen-Waters IBESharedSecret, and theSecretKey/UserSecretKeyof Waters, Waters-Naccache, Boyen-Waters, CGW, CGW-KV, CGW-FO and Kiltz-Vahlis.Each per-type note links back to the crate-level section (anchor verified in generated docs).
The Boyen-Waters
SharedSecret(Gt)was the one secret type initially left unannotated;GtimplementsZeroize, so it now derivesZeroizeand carries the same note as every other secret type — coverage is complete.Why draft / scope
The actual fix — making these types
!CopysoZeroizeOnDropcan be derived and wiping happens automatically — is a breaking API change deferred to a future major release, per the maintainer note on #45. This PR only adds the interim documentation.Stacked on #14 (
feat/zeroize-secret-types), since it documents the feature that PR introduces. Base will retarget tomainautomatically once #14 merges.Testing
cargo fmt --all -- --check— cleancargo build --all-features --lib— cleancargo test --all-features— 18 passed (incl. thesecret_types_zeroizetest from feat: optional zeroize feature for secret types (#7) #14)cargo doc --all-featureswith-D rustdoc::broken_intra_doc_links— clean; the#zeroizing-secret-materialanchor and cross-links resolveCloses #45
🤖 Generated with Claude Code