Repository navigation
feat: wiping keys on drop is opt-in, under a wipe feature - #461
Merged
Merged
Conversation
Quest-Claim: e6d098ca9b744014
`low::zeroise` and `zeroise_value` do nothing without `wipe`, so every Drop that only wipes compiles away. `cfg!` keeps `zero_bytes` compiled and its unit test running in every build. The zeroing tests run under the feature in the gate, nightly on each OS, and on armv7's two paths. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
wipe feature
kixelated
marked this pull request as ready for review
November 7, 2026 08:03
|
Your OpenCode Go account is out of credit. Top it up, then re-trigger Pullfrog. OpenCode Go's response
|
|
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 58 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. |
kixelated
added a commit
that referenced
this pull request
Nov 7, 2026
The maintainer kept the panics behind snow's infallible `Cipher::encrypt` and `Hash::result`; each method now says when it panics. The adapter wipes nothing itself, so the scalar comment no longer promises an unconditional wipe that #461 makes opt-in. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Nov 7, 2026
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.

Finishes quest/m2/speed/wipe-feature.md and deletes it.
Summary
rind now wipes keys, keyed state, and heap scratch derived from a private key only under a new additive
wipeCargo feature. The default build skips the wipe, as the maintainer decided on 2026-11-07 (see the quest's Plan).rind/Cargo.tomlgainswipe = [].low::zeroiseandzeroise_valuewrap theirzero_bytescall inif cfg!(feature = "wipe"). EveryDropthat only wipes compiles away, and the Drop impls are unchanged.cfg!rather than#[cfg]keepszero_bytesand every wipe site type-checked in the default build, so nothing in it goes dead.rind/tests/zeroing.rsis gated onfeature = "wipe". The unit test that checkedzeroisenow callszero_bytesdirectly, so it keeps running in every build. Without the feature, all 12 zeroing tests fail (checked locally by dropping the gate), so the feature does take the wipe out.sh/dispatch.sh check:rs) runscargo nextest run -p rind --features wipe --test zeroingwhenever rind is selected, as the zeroing tests ran in the gate before. Nightlybuild.ymladds the same test on every OS (macOS and the aarch64 runners cover aarch64's key layouts), andsh/armv7.shruns it on both armv7 paths.proofs/README.md, MIGRATING.md (a new "Across the API" row tells aws-lc-rs users to enablerind/wipe), and the README's new "Wiping keys" section, which says adapters don't forward the feature. Each "wiped on drop" doc comment in rind and rind-ring now says "under thewipefeature".Impact
wipefeature. No signature changes.rind/wipe, and Cargo's feature unification turns it on beneath every adapter. Following moq-dev/rind#394, which also removed wipes, the title carries no!, since no code stops compiling.wipe. rind-rcgen'sto_pkcs8still returns azeroize::Zeroizingbuffer, as it did before; see Choices.Benchmarks
A one-off A/B dispatch (run, from the scratch branch
bench-probe/wipe-feature) built main (ff503970, always wipes) and this change (aff51fd5, default build, no wipe) on each runner and ran them alternately, main, head, main, head, so each row compares the same CPU. aws-lc-rs ran in the same runs. Each cell is the minimum of the two rounds.The saving is where a key is built and dropped around little work: one-shot HMAC on short messages is 1.3 to 2.9% faster, and HKDF expanding into an AES-256-GCM key, which builds and drops the AEAD key with its GHASH table, is 2.3 to 6% faster. Reused-key HMAC, which drops nothing in its loop, is unchanged, as expected. ECDH, signing, ML-KEM, and full rustls handshakes move by under 1%, inside the runners' noise (the arm runner's handshake rows drift +0.4 to 0.7% with no wipe left to cost anything). rind's standing against aws-lc-rs is unchanged on every row; the HMAC and HKDF rows widen a lead it already held. The full tables are in the run's step summaries.
Choices made without asking
cfg!insidezeroise, not#[cfg]onzero_bytes. The default build still type-checks every wipe site and keepszero_bytes's unit test, with nocfgsprinkled over the per-archcpu.rsfiles. The optimizer drops the dead branch.wipeon. In the gate, only the zeroing test binary is built with--features wipe, a second rind build when rind changes. The zeroing tests were in the gate before, and a key type that misses its wipe is a REVIEW.md finding, so leaving them to nightly alone would weaken the gate. Nightly adds macOS, Windows (where the test is compiled out), aarch64, and armv7 coverage.!in the title, per moq-dev/rind#394's precedent for removing wipes. release-plz therefore bumps rind as a feature, not a breaking change.Zeroizingstays.to_pkcs8returnsZeroizing<PrivatePkcs8KeyDer>; dropping it changes rind-rcgen's public return type, and key export is a cold path. The quest scopes the feature torind.Validation
just checkonaff51fd5plus the MIGRATING.md edit (b8045cbf, docs only): every module passes (rs, msrv, cross, deny, versions, license, inplace, compat, bench, fixtures, patch, quest, gh, hooks, shell, nix): 625 tests, then the 12 zeroing tests under--features wipe.cargo clippy -p rind --all-targetswith and without--features wipe: clean.wipegate removed, the default build fails all 12, so the default build really skips the wipe.just checkon357895b7, the merge of main (165545ef): every module passes again, 625 tests, then the 12 zeroing tests under--features wipe.just armv7and the nightlybuild.ymlmatrix, which this PR extends.Caused by
None: this is a new maintainer decision, not a fix.
Suggested follow-ups
Zeroizingonto_pkcs8: drop. It costs nothing measurable on a cold export path, and changing it is a breaking change in rind-rcgen for consistency alone.Models
Size: S
Implementer: Claude Code, claude-opus-5-5, high effort
Review
Rewritten by
quest reviewafter each round.01a1b4de-0187-72f3-b34a-9c1a2ad13436, round 1The opt-in wipe behavior matches the recorded maintainer decision, consistently gates existing zeroisation helpers, and retains dedicated CI coverage; tests were not run in the read-only environment.Merge of main at
357895b7: main added no.rschange sinceff503970, so it brought no new wipe site to gate. The only overlap,quest/m2/speed/README.md, resolved without conflict (main appended a Required link after the line this PR removes), andgit diff origin/main...357895b7matchesgit diff ff503970...b8045cbfapart from that hunk's context. As a conflict-free merge from the base, it needs no new review round; the round above covers the PR's changes.(Written by Claude Opus 5.5 with high)