Skip to content

fix(keystore): clamp unlock timeout and zeroize cached identities#467

Closed
ouicate wants to merge 2 commits into
zcash:mainfrom
ouicate:fix/keystore-unlock-zeroize-timeout
Closed

fix(keystore): clamp unlock timeout and zeroize cached identities#467
ouicate wants to merge 2 commits into
zcash:mainfrom
ouicate:fix/keystore-unlock-zeroize-timeout

Conversation

@ouicate

@ouicate ouicate commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Cap walletpassphrase unlock duration at 100,000,000 seconds, matching Bitcoin Core's walletpassphrase behavior.
  • Enable x25519-dalek's zeroize feature so native age identity secret keys are overwritten when cached identities are dropped during walletlock or auto-relock.
  • Replace bare Vec::clear() with clear_cached_identities() that drains identities and releases the Vec allocation via shrink_to_fit().

Addresses GHSA-49gv-v767-5q9w.

Motivation

KeyStore::unlock accepted timeout values up to u64::MAX, allowing RPC callers to disable auto-relock for effectively forever. Separately, age depends on x25519-dalek without the zeroize feature, so native identity secret keys were not guaranteed to be overwritten when identities were cleared from the in-memory cache.

Cap walletpassphrase unlock duration at 100,000,000 seconds to match
Bitcoin Core, and ensure native age identity secret keys are zeroized on
drop when the keystore locks or auto-relocks.

Co-Authored-By: Composer <noreply@cursor.com>
@ouicate

ouicate commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Please review @nullcopy

@nullcopy nullcopy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestions. The ideas are good, but I would prefer different solutions.

  1. capping the timeout to 100K secs to match Bitcoin. This does prevent a panic if the caller specifies an unlock time large enough to trigger overflow. Rather than silently falling back to an unlock time the caller did not request, I would prefer to just do checked_add() and return an error to the caller if their requested unlock time is too large, and allow them to try again with a smaller value.

  2. the helper to clear cached identities is unnecessary. Simply enabling the zeroized feature accomplishes the stated goal.

@nullcopy

Copy link
Copy Markdown
Contributor

Superseded by #641

@nullcopy nullcopy closed this Jul 17, 2026
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.

2 participants