Repository navigation
chore(quest): plan wiping each key type's whole value - #547
Conversation
Key types already wipe their public parts so the zeroing test checks every byte; agreement::PrivateKey rejoins that rule and AGENTS.md states it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 35 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — two minor suggestions inline.
Reviewed changes
- New quest
quest/m2/api/wipe-whole-key.md(XS) — plans the follow-up to #529: underwipe, every key type wipes its whole value (public parts included),agreement::PrivateKeyreturns to a whole-value wipe, the zeroing test carries no per-type skip, and AGENTS.md and REVIEW.md state the rule.
Verified against the repo and cited sources: all six named types do wipe non-secret parts on purpose, each with a comment saying so; rind/tests/zeroing.rs currently checks every byte with no skip on main; the "padding and enum-tail leaks" attribution matches #524 (i686 RsaKeyPair padding found by the zeroing test) and #513 (ARM AES enum tail); and the FIPS 140-3 claim is supported — ISO/IEC 19790 7.9.7 requires zeroisation of all unprotected SSPs (new in 140-3 vs 140-2), and the FIPS 140-3 IG's 9.7 items state that zeroisation of unprotected PSPs (public security parameters) is required except when protected. The m2/api epic README links children only in prose, so the unlinked quest file follows the established pattern.
glm-5.3 ｜ 𝕏
| Decided with the maintainer on 2026-11-09, while planning #529's follow-up (✅ chosen): | ||
|
|
||
| - Rule: ✅ a key type wipes its whole value (rejected: secrets and stored public keys only; secrets only, with per-type skips in the zeroing test; change only `Ed25519KeyPair`; leave #529 as the lone exception). | ||
| - Why: six types already wipe non-secret parts so the zeroing test can check every byte (`Ed25519KeyPair`, `EcdsaKeyPair`, `RsaKeyPair`, `oaep::DecryptingKey`, `AesCtr`, `AesCbcEncryptingKey`). That check is what caught the padding and enum-tail leaks fixed in moq-dev/rind#513 and moq-dev/rind#524. FIPS 140-3 (ISO/IEC 19790 7.9.7, NIST IG 9.7) also requires zeroising unprotected public security parameters such as stored public keys. A few more bytes written when a key drops cost nothing measurable. |
There was a problem hiding this comment.
The list is short by two: AesCbcDecryptingKey (rind/src/mid/aes_cbc.rs:198) and AesKek (rind/src/mid/aes_kw.rs:213) also wipe a non-secret key_len on purpose via zeroise_value(self) — the same rationale the comment above AesCbcEncryptingKey's drop names ("The key length is not secret, but is wiped with the key so that the whole value reads zero"), and both are checked whole by tests/zeroing.rs. So the survey reads as exhaustive at six when it's at least eight. Harmless for the rule's direction — both already conform — but future agent runs cite these lists, so worth naming them.
| - AGENTS.md: ✅ state the rule there (rejected: leave its wording alone). This replaces #529's 2026-11-09 decision to wipe only `PrivateKey`'s private part. | ||
| - Placement: ✅ m2 api epic (rejected: m0, m3). | ||
|
|
||
| Start once #529 has landed. Update REVIEW.md's "Wiped secrets" check to match. |
There was a problem hiding this comment.
The plan's doc updates name AGENTS.md and REVIEW.md, but two other docs restate the rule this quest retires: ARCHITECTURE.md (~line 16, "Keys wipe their secrets on drop under the wipe feature") and the README's ## Wiping keys section ("rind wipes keys, keyed state, and heap scratch derived from a private key on drop"), which mirrors AGENTS.md's rule sentence. Under the whole-value rule, "their secrets" describes the old scope — adding these two to the update line keeps the repo's grep-the-tree convention for changed AGENTS.md rules from leaving them behind.
Plans quest/m2/api/wipe-whole-key.md, the follow-up that moq-dev/rind#529 suggested. Under
wipe, every key type wipes its whole value, public parts included.agreement::PrivateKeygoes back to that rule, and AGENTS.md states it.Decisions
Settled with the maintainer on 2026-11-09:
Ed25519KeyPair. Six types turned out to wipe non-secret parts on purpose:Ed25519KeyPair,EcdsaKeyPair,RsaKeyPair,oaep::DecryptingKey,AesCtr, andAesCbcEncryptingKey. ✅ every key type wipes its whole value, and the rule goes in AGENTS.md (rejected: secrets and stored public keys only; secrets only, with per-type skips in the zeroing test; change onlyEd25519KeyPair; leave feat(agreement)!: keys keep their public key and algorithm #529 as the lone exception). The maintainer first leaned toward secrets only "unless FIPS requires it". FIPS 140-3 (ISO/IEC 19790 7.9.7, NIST IG 9.7) does require zeroising unprotected public security parameters, so the whole-value rule was picked.Public API
None. Quest file only.
Models
Size: XS
Implementer: Claude Code, claude-opus-5-5, high effort
(Written by Claude Opus 5.5 with high)
Review
Rewritten by
quest reviewafter each round.01a1c306-36b8-7653-859e-99afe27e4fa8, round 1The documentation-only change adds a planning document, changes no executable code or tests, and has no actionable defects.