Repository navigation
feat(zcash_keys): erase UnifiedSpendingKey secret material on drop - #633
Draft
ValarDragon wants to merge 2 commits into
Draft
ValarDragon wants to merge 2 commits into
ValarDragon wants to merge 2 commits into
Conversation
Add a default-on `zeroize` feature to zakura-keys. With it, `UnifiedSpendingKey` overwrites its Sapling extended spending key and its Orchard spending key with zeros when dropped. `to_bytes` (behind `unstable`) now returns `secrecy::SecretVec<u8>`, reserves the full length up front so growth never leaves freed copies, and erases the intermediate Sapling and transparent encodings it builds. The key types underneath gain `zeroize::Zeroize` impls behind new default-off features: - zakura-orchard `zeroize`: `keys::SpendingKey`. - zakura-sapling-crypto `zeroize`: `SpendAuthorizingKey`, `ExpandedSpendingKey`, `OutgoingViewingKey`, `zip32::DiversifierKey`, and `zip32::ExtendedSpendingKey`. - zakura-redjubjub `zeroize` and zakura-reddsa `zeroize-signing-key`: `SigningKey`, erasing the secret scalar and keeping the public key. reddsa's existing `zeroize` feature is inert and enabled by `std`, so the working impl takes a separate name to stay out of default builds. - zakura-jubjub and zakura-pasta-curves `zeroize`: `DefaultIsZeroes` for `Fr`, `Fp`, and `Fq`. Every change is additive. The key types keep `Copy`, and only `UnifiedSpendingKey`, which is not `Copy`, erases on drop. Callers who copy a key out erase their own copy. The orchard `SpendingKey` `Debug` output is now redacted. Not covered: the Sapling ZIP 32 chain code, since zip32 0.2 cannot overwrite a `ChainCode`, and the transparent spending key. Zakura enables none of these features. Resolving Zakura against this branch through `[patch.crates-io]` gives the same packages, enabled features, and `zeroize` reverse dependencies as against main. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01PvjDwWmc4La5L9yLQKBmUE
Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01PvjDwWmc4La5L9yLQKBmUE
This branch has not been deployed
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
Adds a default-on
zeroizefeature tozakura-keys. With it,UnifiedSpendingKeyoverwrites its Sapling extended spending key and its Orchard spending key with zeros when dropped.UnifiedSpendingKey::to_bytes(behindunstable) now returnssecrecy::SecretVec<u8>. It reserves the full encoded length up front, so the buffer never reallocates and leaves freed copies behind. It also erases the intermediate Sapling and transparent encodings it builds.To make that possible, the key types underneath gain
zeroize::Zeroizeimpls behind new default-off features:zakura-orchardzeroizeZeroizeforkeys::SpendingKeyzakura-sapling-cryptozeroizeZeroizeforSpendAuthorizingKey,ExpandedSpendingKey,OutgoingViewingKey,zip32::DiversifierKey,zip32::ExtendedSpendingKeyzakura-redjubjubzeroizeZeroizeforSigningKeyzakura-reddsazeroize-signing-keyZeroizeforSigningKey; erases the secret scalar and keeps the verification keyzakura-jubjubzeroizeDefaultIsZeroesforFrzakura-pasta-curveszeroizeDefaultIsZeroesforFpandFqreddsa's existing
zeroizefeature is an inert compatibility feature thatstdturns on. The working impl therefore takes a separate name: reusingzeroizewould switch it on in every default build, and removingzeroizefromstdis a semver break (feature_not_enabled_by_default).Orchard
SpendingKey'sDebugoutput is now redacted (SpendingKey { .. }). It previously printed the key bytes.Compatibility
Copy, and onlyUnifiedSpendingKey, which is notCopy, erases on drop. Callers who copy a key out of it erase their own copy.check_semver.py --default-featuresreports "no semver update required" for all seven crates.zakura-keysand enables none of the new features. I resolved Zakura against this branch and againstmainthrough[patch.crates-io]. Both give the same 348 packages, the same enabled features (normal, build and dev edges), and the samezeroizereverse dependencies.Not covered
zip320.2 has no way to overwrite aChainCode.zip320.3 adds one, but moving to it is a breaking bump shared with the wallet libraries, so it's left for a follow-up.Tests
Zeroizeimpl:Fr,FpandFqbecome zero;SigningKeyscalars become zero while the verification key is unchanged; OrchardSpendingKeybytes become zero; SaplingExtendedSpendingKeybytes after the chain code become zero, while the chain code survives.SpendingKey'sDebugoutput is redacted.prop_usk_roundtripnow readsto_bytesthroughexpose_secret, and its Sapling-only parts are gated onsapling.zakura-keystests with and withoutzeroize; the jubjub, reddsa, redjubjub and sapling-crypto suites with the features on; the orchardkeysand pasta_curvesfieldstests;cargo check --workspace --all-targets; the CI librustzcash all-supported-features check;cargo fmt --check. Clippy produces no diagnostics beyond those already onmain.Generated by Claude Code