fix(core): zeroize the derived X25519 secret (#65)#66
Merged
Conversation
x25519_secret_from_seed derived the X25519 private scalar from the Ed25519 seed and returned it as a bare [u8; 32]. Because [u8; 32] is Copy with no Drop (and sha2's digest output likewise has no zeroize), the scalar, the SHA-512 digest, and the returned temporary were released without being scrubbed, leaving secret-derived material in freed memory. This is the same Copy-no-Drop class fixed for the raw seed in #41, on the derived secret. Return Zeroizing<[u8; 32]>, build the scalar directly into a zeroizing buffer so no bare secret local persists, and explicitly wipe the SHA-512 digest before it drops. Callers deref the result into crypto_box::SecretKey, which already scrubs its own copy. No behavior change: the derived scalar and all decryption output are byte-identical, covered by the existing ed25519_to_x25519_keypair_agrees and seal_open_round_trip_for_recipients tests. Stacked on #41 (#64).
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
beardthelion
added
kind:security
17 tasks
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
2 participants
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
x25519_secret_from_seedderived the X25519 private scalar from the Ed25519 seed and returned it as a bare[u8; 32]. Because[u8; 32]isCopywith noDrop(andsha2's digest output likewise has no zeroize), the scalar, the SHA-512 digest, and the returned temporary were released without being scrubbed. This routes the derived secret throughZeroizingand explicitly wipes the digest.Motivation & context
Closes #65
Direct sibling of #41/#64: same
Copy-no-Dropmemory-hygiene class, on the derived X25519 secret rather than the raw seed. #41 was deliberately scoped to the seed accessor and flagged this downstream leak for its own follow-up. The derived scalar is the actual private key fed intocrypto_box::SecretKeyinopen_blob, which runs on every withheld-blob read.Kind of change
What changed
Crate touched:
gitlawb-core.x25519_secret_from_seednow returnsZeroizing<[u8; 32]>and builds the scalar directly into a zeroizing buffer, so no bare[u8; 32]secret local persists.h.as_mut_slice().zeroize(), a volatile non-elidable write) before it drops.open_bloband a crypto test) deref the result intocrypto_box::SecretKey, which already scrubs its own copy.No behavior change: the derived scalar and all decryption output are byte-identical, covered by the existing
ed25519_to_x25519_keypair_agreesandseal_open_round_trip_for_recipientstests.How a reviewer can verify
cargo test --workspace cargo clippy --workspace --all-targets -- -D warningsBefore you request review
cargo test --workspacepasses locallycargo fmt --allandcargo clippy --workspace --all-targets -- -D warningsare cleanfeat(...),fix(...),docs(...)).env.exampleupdated if behavior or config changed (or N/A)Protocol & signing impact
did:key, Ed25519 / RFC 9421 signatures, UCAN, ref certs, or P2P wire formatsIn-memory hygiene only. No change to the encryption scheme, key derivation math, or wire format; output is byte-identical.