Summary
The on-disk cache of a SecureStore identity's public key (added in #2533) is populated on creation and on first read, but it is never refreshed afterwards. If the identity TOML is tampered with — e.g. by an attacker who can write to ~/.config/stellar/identity/ — the wrong public key would be returned indefinitely from keys address and used for signature hint computation, with no way to detect the divergence short of re-running the migration manually.
The signing path already opens the OS keychain to fetch the seed phrase, so deriving the real public key at that moment is essentially free. Comparing it to the cached value (and overwriting on mismatch) would close the cache-poisoning gap without re-introducing keychain prompts on read-only flows.
Context
Raised by @leighmcculloch in the review of #2533: #2533 (review)
I can't tell, but does the cached public key get updated every time the secret key is read? I don't think so, but I could be wrong. It would be good for that to happen if not, so that the public key cache can't be poisoned through some nefarious actor and then never updated.
Proposed approach
- Inside
signer/secure_store.rs::sign_tx_data (or the wrapping signer entry point), derive the public key from the unlocked seed phrase before signing.
- If it differs from the cached value passed in via
SecureStoreEntry, overwrite the identity TOML with the corrected value (best-effort write-back, as in locator::read_key_with_secure_store_cache).
- Optionally print a warning when a mismatch is detected, so users notice the tampering rather than silently accepting it.
Out of scope
Summary
The on-disk cache of a
SecureStoreidentity's public key (added in #2533) is populated on creation and on first read, but it is never refreshed afterwards. If the identity TOML is tampered with — e.g. by an attacker who can write to~/.config/stellar/identity/— the wrong public key would be returned indefinitely fromkeys addressand used for signature hint computation, with no way to detect the divergence short of re-running the migration manually.The signing path already opens the OS keychain to fetch the seed phrase, so deriving the real public key at that moment is essentially free. Comparing it to the cached value (and overwriting on mismatch) would close the cache-poisoning gap without re-introducing keychain prompts on read-only flows.
Context
Raised by @leighmcculloch in the review of #2533: #2533 (review)
Proposed approach
signer/secure_store.rs::sign_tx_data(or the wrapping signer entry point), derive the public key from the unlocked seed phrase before signing.SecureStoreEntry, overwrite the identity TOML with the corrected value (best-effort write-back, as inlocator::read_key_with_secure_store_cache).Out of scope
keys address). That would defeat the purpose of Cache secure-store public key in identity files. #2533, since it forces a keychain prompt on every invocation. The defense should hook into operations that already prompt.