spesmilo
Default settings allow using `electrum lnpay $invoice` without any confirmation or password
The default settings allow using electrum lnpay $invoice without any confirmation or password. That seems a bit risky, as anyone who can get access to the socket gets to spend all available LN balance.
anyone who can get access to the socket gets to spend all available LN balance
Note: on Linux/macOS, the default is to use unix sockets for the RPC, on Windows we listen on a TCP port on localhost.
In either case, the RPC is protected with a random password, which is stored in the config file. So in practice to be able to run commands such as lnpay, the conditions are that (1) the wallet has to be open, (2) read access for the config file is needed.
(read access to the config file implies access to the unix socket or localhost tcp also)
@Midar questioned on IRC how https://github.com/spesmilo/electrum/commit/37d090c621006cc4caa7e0fb9d080b3e6a9da05c / https://github.com/spesmilo/electrum/pull/9238 interacts with hardware wallets.
This is a good question and it is complicated to answer. The short answer is that the RPC does not work for encrypted hw wallets.
If the standard wallet with hw keystore is not encrypted, then CLI/RPC works without auth both before and after the change. (note that there is always rpcpassword-based authentication, see prev comment, I am only considering any auth after that)
If the wallet file is encrypted with a hw device, then offline (meaning --offline, no daemon or GUI running) commands (such as signmessage) work both before and after change. The user is not prompted for the wallet password -- instead there is interaction with the hw device.
If there is a daemon running, note that load_wallet does not support hw-encrypted wallets.
If there is a GUI running, which I guess is the original motivation for this issue, the equivalent of load_wallet is opening a wallet in the GUI, which ofc supports hw-encrypted wallets.
- Before this change, all commands worked without password-authentication (compare e.g. signmessage (which started a CLI hw handler), and lnpay (which just executed), both of which worked). Quirk: the "requires_password" commands required the user to explicitly pass a dummy password.
- After the change, the commands that are marked as "requires_password" still require the user to pass a password argument, but a dummy password is no longer accepted. Only the actual xpub-derived password would be accepted, which the user is not supposed to know.
Hence, re encrypted hw wallets, to sum up, some commands worked prior to this change but in a really quirky/accidental fashion only -- and now they do not work.
Another further note is that maybe malware running on the PC could communicate with the connected hardware device and request the encryption password from it. It depends on hw device type and implementation specifics whether there would be any prompt about this (e.g. a trezor one would prompt for PIN, and potentially passphrase). (for the encryption password we use a serialised pubkey, bip32-derived along a fixed hardened bip32 path)
But note that if an attacker could pull this off, they might as well just copy the wallet file, decrypt it, and do whatever they want there without the RPC.
I am reopening this to signal that it warrants some more thought, but I think we could leave master as-is. Though we should probably at least add some comment about all this re https://github.com/spesmilo/electrum/commit/37d090c621006cc4caa7e0fb9d080b3e6a9da05c near line 163.
Hm, isn't the wallet always encrypted when using a HW wallet? In any case I'm seeing the lock symbol in Electrum and I can just use lnpay without any confirmation whatsoever.
Hm, isn't the wallet always encrypted when using a HW wallet?
No, you can click the lock and disable file encryption.
I can just use lnpay without any confirmation whatsoever.
On master? Can you give repro steps?
No, this was on 4.5.5, waiting for the next signed release before I let this touch my wallet ;). What I meant to say is that before this change at least, it already had access, even with a HW wallet.
Before this change, all commands worked without password-authentication (compare e.g. signmessage (which started a CLI hw handler), and lnpay (which just executed), both of which worked). Quirk: the "requires_password" commands required the user to explicitly pass a dummy password.
Yes that's already what I meant there ^
The mentioned changes were included in the recent releases. I am not fully satisfied with how it works now, but not sure how we can improve on this easily. We can revisit if there is a better idea.
I checked now and it told me to unlock my wallet. It was unlocked, but a HW wallet, so I would guess this means it's WAI now, except for maybe a slightly confusing error? (I guess "Command lnpay cannot be used with a hardware wallet" would be more accurate).