Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: lock application #72

Open
chaserene opened this issue Dec 31, 2022 · 8 comments
Open

feature: lock application #72

chaserene opened this issue Dec 31, 2022 · 8 comments

Comments

@chaserene
Copy link

please provide a way to lock the wallet (without having to close the wallet, that is).

@maltfield
Copy link

maltfield commented Jan 16, 2023

Came here to ask for this. I've setup feather to lock itself after some minutes (File -> Settings -> Privacy -> Lock wallet on inactivity of X minutes), but I'd like to request a new feature with the ability to lock feather immediately, ideally with a hotkey like Ctrl+L.

Also, I don't know if this is already the case, but it would be ideal if the wallet could continue to sync with the blockchain in the background while locked -- it just wouldn't display anything in the UI (of course not allow sending funds) without the password being entered.

@chaserene
Copy link
Author

this...

the wallet could continue to sync with the blockchain in the background while locked -- it just wouldn't display anything in the UI

...and this...

(of course not allow sending funds) without the password being entered.

...are antithetical. a "fake locking" (blanking the UI) would leave keys in the memory and thus your privacy and funds at risk of being taken by an intruder, which is exactly why you would want to have a lock function.

scanning the blockchain for transactions that concern your wallet is very quick, you won't have to wait for too long at startup if your wallet is not open all the time. this is especially true if you run a full node locally.

@maltfield
Copy link

maltfield commented Jan 17, 2023

scanning the blockchain for transactions that concern your wallet is very quick, you won't have to wait for too long at startup if your wallet is not open all the time. this is especially true if you run a full node locally.

Some folks need to use Tor and have ISPs that routinely spike to ~30% packet loss several times per hour, so unfortunately it's not so quick for some users.

"fake locking" (blanking the UI) would leave keys in the memory

That's fair, and I'm grateful for the attention to this. Can you please just update the UI to make it clear that sync has stopped when it's locked? I wasn't sure if I had to unlock it to continue the sync or not.

Just a note on the "lock screen" saying that "sync is currently stopped while locked" would be fine.

@tobtoht
Copy link
Contributor

tobtoht commented Jan 19, 2023

Immediate locking (Ctrl+L) is implemented in: e5a61c6

Note: Synchronization continues in the background. Feather can't currently protect against an attacker dumping process memory to extract private keys or wallet passwords. I'm interested in supporting this threat model, but I need a better understanding of Qt's memory model to determine if this is feasible.

@tobtoht tobtoht added this to the settings rework milestone Jan 19, 2023
@chaserene
Copy link
Author

@tobtoht does this mean that when Feather currently auto-locks, private keys and the wallet password can similarly be dumped from the memory?

also, why is the password stored in the memory?

@tobtoht
Copy link
Contributor

tobtoht commented Jan 26, 2023

does this mean that when Feather currently auto-locks, private keys and the wallet password can similarly be dumped from the memory?

Yes.

also, why is the password stored in the memory?

It's cached in wallet/api (part of Monero, used by most wallets except CLI), but appears to serve no real purpose.

After analyzing VM memory dumps, I can only find secret key material where it is expected. It looks like Qt isn't spraying sensitive information all over the place, which is good.

Here is what needs to happen to move towards better security against memory attacks:

  • The Polyseed is currently stored in the wallet cache. This was done because we can't derive a Polyseed from the secret spend key and we can't touch the .keys file without breaking wallet file compatibility. The Polyseed should be stored encrypted in memory. Upstreaming Polyseed (so it is stored in .keys) is on the to-do list.

  • Wallet password caching should be removed from wallet/api. Password verification should use wallet2::verifyPassword instead of comparing against the cached value. This causes a slight delay when opening dialogs that require the password or when unlocking the wallet.

  • The secret spend key should be encrypted when the wallet is locked. This mode is already supported by wallet2, but needs some plumbing from our side to make it work. Some additional changes are necessary to allow synchronization to continue in the background (wallet: background sync with just the view key [master] monero-project/monero#8619).

  • Use something like sodium_memzero to securely wipe memory from password entry dialogs and the Keys and Seed dialog when they are closed.

Note: all of the above can only protect against an attacker learning your secret spend key from a memory dump. It can still be used to learn your secret view key, transaction metadata, contacts, etc. It will also not protect against an attacker that has access to your machine without you knowing it. If you enter your password after the system is compromised, they will obviously be able to learn the secret spend key.

Related ideas:

  • Lock all opened wallets when the screen is locked. Might be difficult to do cross-platform, but a best effort can be made.

Trade-offs:

Pwd entry to spend Pwd entry to receive output Sync when locked
Unencrypted spendkey
Spendkey encrypted on lock ☑*
Spendkey encrypted after initial sync ☑*

* with monero-project/monero/pull/8619

I don't see any real downsides to encrypting the spendkey on lock, so this should become the default behavior after these changes are implemented.

I would like to point out that a generic solution exists for the snatch-and-run attack scenario that these measures help protect against: https://www.buskill.in. Its author happens to participate in this thread ;)

@maltfield
Copy link

maltfield commented Jan 27, 2023

:) Ah, distinct spend and view keys allows sync while locked. I <3 Monero!

Since it seems that sync is not blocked with lock, can we also make that clear to the user in the UI? Can we have a message on the lockscreen that says "wallet is locked. sync is ongoing." or something?

@j-berman
Copy link

The secret spend key should be encrypted when the wallet is locked

Small comment: in monero-project/monero#8619, I simply wipe the spend key from memory when the wallet locks instead of encrypting it, and then when the user inputs their password to unlock the wallet, it'll re-read the encrypted key from disk and decrypt it.

unattended wallets (like RPC and GUI) would need a bit more plumbing to work nicely with wallet2's wallet_keys_unlocker; I figured this was a simpler approach.

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

No branches or pull requests

4 participants