fix(crypto): wipe secret-key temporaries - #3456
Draft
Jr-kenny wants to merge 2 commits into
Draft
Conversation
huitseeker
reviewed
Aug 4, 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.
Ports 0xMiden/crypto#1061 now that the crypto crates live in this workspace. The original gap is tracked in 0xMiden/miden-vm#3533.
What was still leaking
SecretKeywipes its stored Falcon basis on drop, but the work around it was leaving extra copies of the same secret material behind.Polynomial<F>claimedZeroizeOnDropwithout aDropimplementation, so those coefficients were never actually wiped. Signing, serialization, deserialization, public-key derivation, and rejected key-generation retries all created more secret polynomials or encoded key buffers that dropped normally.The same encoded-key pattern also showed up in the ECDSA and EdDSA secret-key comparison and serialization paths.
What changed
FalconFeltnow implementsZeroize, and secret-carrying polynomials are wrapped inZeroizingwhere they are created. The unbackedZeroizeOnDropmarker is gone. The FFT chain inSecretKey::read_from, the encoded buffers inwrite_intoandgenerate_seed, the public-key derivation clones, and rejectedntru_gencandidates are all covered.Secret-key equality stays available to the crate's tests, but
EqandPartialEqare no longer exposed for Falcon, ECDSA, or EdDSA secret-key wrappers in production builds. ECDSA and EdDSA serialization now wipe their temporary encoded copies too.I ported the security changes onto the current
nextcode instead of copying the old crypto files across, so the newermiden-vmrefactors stay intact. TheBigIntandComplex64residue documented in 0xMiden/miden-vm#3533 is still outside this change because those paths need the same manual volatile treatment already used byLdlTree.Checks I ran
The focused regression failed before the fix because
FalconFeltdid not satisfyZeroize. It passes with this change, along with the full crypto suite and the build combinations used by this code.