fix(core): route seed access through the zeroizing wrapper (#41)#64
fix(core): route seed access through the zeroizing wrapper (#41)#64beardthelion wants to merge 1 commit into
Conversation
Remove Keypair::seed_bytes(), which returned a bare [u8; 32] copy of the Ed25519 private seed. Because [u8; 32] is Copy with no Drop, each caller's copy was released without being scrubbed, leaving secret key material in freed memory. The zeroizing accessor to_seed() -> Zeroizing<[u8; 32]> already existed beside it; this makes it the sole seed accessor and migrates the two in-crate callers (envelope open path and a crypto test) to it via deref coercion. No behavior change to envelope encryption: the derived X25519 secret is byte-identical, covered by the existing round-trip tests. Adds an identity test pinning to_seed() as the only seed accessor.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesKeypair Seed Zeroization Hardening
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Summary
Keypair::seed_bytes()handed out a bare[u8; 32]copy of the Ed25519 private seed. Because[u8; 32]isCopywith noDrop, each caller's copy was released without being scrubbed, leaving secret key material in freed memory. This removes that accessor and routes seed access through the existing zeroizing wrapper.Motivation & context
Closes #41
to_seed() -> Zeroizing<[u8; 32]>already existed right besideseed_bytes()and scrubs the copy on drop. There was no reason to keep an unzeroized accessor: it was duplicated, and the only two callers were both ingitlawb-core. Removing it makesto_seed()the single seed accessor and closes the exposure window to core dumps, swap, and memory-disclosure bugs.Kind of change
What changed
Crate touched:
gitlawb-core.Keypair::seed_bytes()(raw, unzeroized seed accessor) inidentity.rs.to_seed(): the envelope-decryption path inencrypt.rs(open_blob) and a crypto agreement test. The migration is mechanical:Zeroizing<[u8; 32]>deref-coerces to&[u8; 32], which is whatx25519_secret_from_seedalready takes.to_seed_is_sole_seed_accessordocumenting and guarding the single-accessor invariant.No behavior change to envelope encryption/decryption: the X25519 secret derived from the seed is byte-identical, covered by the existing round-trip tests.
How a reviewer can verify
Before 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 formatsThis is in-memory hygiene only. It does not change the signature scheme, key format, DID derivation, or any wire format. The bytes produced and consumed are identical; only the container type of a transient changes.
Notes for reviewers
There is a related, deliberately out-of-scope leak one hop downstream:
x25519_secret_from_seedreturns the derived X25519 secret as a bare[u8; 32]. It will be filed as its own follow-up issue to keep this PR scoped to exactly #41.Summary by CodeRabbit
Refactor
Tests