This repository was archived by the owner on Aug 7, 2026. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 99
fix: zeroize Falcon secret polynomial temporaries and encoded key buffers #1061
Open
Jr-kenny
wants to merge
4
commits into
0xMiden:next
Choose a base branch
from
Jr-kenny:zeroize-falcon-polynomial-temporaries
base: next
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
8a08a99
docs: changelog entry for the zeroizing read helper
Jr-kenny 2cb71c5
fix: wipe falcon secret polynomial temporaries and encoded key buffers
Jr-kenny 31a5023
fix: wipe wipeable secret temporaries in keygen and pubkey derivation
Jr-kenny 6a39e31
fix: remove production secret key equality leaks
huitseeker File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This handles the
generate_seedcopy, but the same full-key serialization still happens inPartialEqjust below:self.to_bytes().ct_eq(&other.to_bytes()).Both calls allocate encoded secret keys and drop them without
Zeroizing, so comparing two Falcon secret keys can still leave the same kind of buffer this PR is trying to wipe. Could those two serialized values be bound withZeroizingtoo, or compared without serializing?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that path slips the wipe too.
PartialEqserializes both keys and drops them in the clear on every compare. Bound bothto_bytes()inZeroizingin 53a0f03 so they get wiped after the compare instead.ct_eqstill runs over the bound bytes, so the comparison is unchanged and stays constant time.Swept the rest while I was in there: this was the last unwiped
to_bytes()path in Falcon,write_intoandgenerate_seedwere already covered. The same pattern showed up in the sibling secret keys though. The k256 ECDSA and ed25519 EdDSASecretKeys are bothZeroizeOnDropand theirPartialEqhad the identical un-wipedto_bytes().ct_eq(), so I folded the same fix into those two as well. Falcon, ecdsa and eddsa suites all pass locally.