Skip to content

Closes #662 - #854

Merged
Nanle-code merged 5 commits into
Nanle-code:masterfrom
mansur-codes:feat/issue-662-zeroize-secret-material
Aug 31, 2026
Merged

Closes #662#854
Nanle-code merged 5 commits into
Nanle-code:masterfrom
mansur-codes:feat/issue-662-zeroize-secret-material

Conversation

@mansur-codes

Copy link
Copy Markdown
Contributor

Description

Minimizes the in-memory lifetime of seed phrases, passphrases, and secret keys by wrapping them in Zeroizing wrappers from the zeroize crate. When these values go out of scope, their backing memory is overwritten with zeros before the allocator reclaims it, closing the window where a memory scrape could recover live key material.

Closes #662

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Changes Made

  • Added zeroize = { version = "1", features = ["derive"] } to Cargo.toml as a direct dependency (was already an indirect dep at v1.9.0 via argon2)
  • Updated encrypt_secret and decrypt_secret in src/utils/crypto.rs to store Argon2-derived AES keys in Zeroizing<[u8; 32]> so key bytes are overwritten on drop; updated prompt functions to return Result<Zeroizing<String>>
  • Updated keypair_from_phrase in src/utils/mnemonic.rs to wrap the BIP39 seed, raw ed25519 private key, and all SLIP-0010 intermediate key/chain pairs in Zeroizing wrappers; return type changed to Result<(String, Zeroizing<String>)>
  • Updated SigningRequest.local_secret in src/utils/wallet_signer.rs to Option<Zeroizing<String>> and resolve_local_secret to return Result<Zeroizing<String>>; updated all callsites across commands, horizon, and multisig modules
  • Appended "Secret material lifetime and zeroization" section to docs/WALLET_IMPORT_SECURITY.md covering what is zeroized, the mechanism used, and caveats for WASM, heap realloc, and swap

Testing

How has this been tested?

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed

Test Coverage

  • Happy path: keypair_secret_key_is_wrapped_in_zeroizing confirms the returned secret key derefs correctly to a valid S... strkey after wrapping
  • Edge cases: zeroizing_array_explicit_call_clears_all_bytes verifies a Zeroizing<[u8; 32]> initialized to 0xFF reads as all zeros after an explicit .zeroize() call; aes_key_zeroizes_on_drop reads the stack slot via read_volatile after drop and asserts all zeros
  • Error handling: encrypt_error_path_still_compiles_with_zeroizing_key passes a zero mem_cost Argon2 config (which Argon2 rejects) and confirms the key is zeroized even on the error path; bad_phrase_returns_error_without_panicking confirms an invalid mnemonic checksum returns Err without touching any secret material

Results:

cargo test --lib -- utils::crypto::tests utils::mnemonic::tests utils::wallet_signer::tests
24 passed, 0 failed

cargo test --test wallet_encryption_integration
8 passed, 0 failed

cargo test --test wallet_import_property_tests
18 passed, 0 failed

Code Quality Checklist

  • My code follows the style guidelines of this project (cargo fmt)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings (cargo clippy -- -D warnings)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • The CI checks pass (format, clippy, tests)

Breaking Changes

  • This PR introduces breaking changes

Internal return types changed (Zeroizing<String> instead of String for secret values), but all public-facing CLI behavior is identical. Callsites within the crate were updated as part of this PR.

Documentation

  • README.md updated
  • DEVELOPER_GUIDE.md updated (if applicable)
  • API_REFERENCE.md updated (if applicable)
  • No documentation changes needed

docs/WALLET_IMPORT_SECURITY.md updated with a new section on secret material lifetime and zeroization.

Additional Context

zeroize uses volatile_write plus a compiler fence to prevent the optimizer from eliding the zeroing pass. This is effective on native targets. The WASM build does not handle raw secret material directly so the weaker volatile guarantees there are not a practical gap. See the documentation section for full caveats on heap realloc and swap.

@drips-wave

drips-wave Bot commented Aug 28, 2026

Copy link
Copy Markdown

@mansur-codes Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@Manuelshub

Copy link
Copy Markdown
Collaborator

@mansur-codes Please fix the CI checks failure!!!

@mansur-codes

Copy link
Copy Markdown
Contributor Author

Please review and merge

@Nanle-code
Nanle-code merged commit 29fec38 into Nanle-code:master Aug 31, 2026
1 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[2026 Security] Zeroize secret material after use

3 participants