lightningnetwork / lnd

Lightning Network Daemon ⚡️
MIT License
7.63k stars 2.07k forks source link

Add encryption of lnd database #226

Closed BitfuryLightning closed 6 years ago

BitfuryLightning commented 7 years ago

Currently, if somebody gets access or stole machine with lnd, he can easily claim all funds in lnd. The solution is to save some parts of lnd database in encrypted format. To open it password/passphrase should be entered.

Roasbeef commented 7 years ago

The wallet database is already stored in an encrypted format, with both an optional public passphrase (for addresses, public keys, imported scripts, etc), and a private pass phrase (for private keys, etc).

Currently, both the public and private passphrase are set to hard-coded default values. This was done early in development as the software wasn't ready for use with real funds, and it made scripting and testing lnd much easer.

The missing component here is to poll the users for the private (and optionally also the public passphrase) upon startup of the daemon. RPC calls (once macaroons have been implemented) will be guarded by the presentation of the particular bearer credential for a particular RPC call. As a result, I don't foresee see any issues with attacker exploitable RPC race conditions.

The implementation SHOULD NOT accept the passphrase as a command line argument, or a configuration option. Instead, IMO the software should poll the users for their passphrase upon startup, in order to avoid a local tty echo when entering the password, the ReadPassword function within the terminal package should be used.

The underlying passphrase implementation uses scrypt along with a salt derived via a CSPRNG, as a key-derivation function to stretch out the passphrase into a strong key for usage to decrypt/encrypt the sensitive portions of the wallet database.

Finally, to complete the set up (to ensure macaroons aren't transmitted in clear text), RPC server should be extended to either use self-signed certificates, or create a custom transport which uses brontide.

BitfuryLightning commented 7 years ago

Nice explanation.

I have one concern. If somebody wants to create GUI interface to lnd he would not be able to unlock wallet programmatically in a simple way. Like when GUI application launches lnd, user provide passphrase to GUI. GUI need some way to provide passphrase to lnd. GUI cannot write to stdin of lnd because ReadPassword checks device and if it is not a tty, it fails with an error inappropriate ioctl for device.

Maybe add command option:

  1. (Default) Unlock wallet from command line using ReadPassword
  2. Unlock using RPC call.
  3. Use default values for locking/unlocking. Useful for tests.
Roasbeef commented 7 years ago

I fail to see how with what I described above, one wouldn't be able to unlock the wallet from the GUI in a programmatic manner. Seems all one would need to do is pipe the standard input from the GUI into lnd. The GUI wouldn't attempt to start lnd before the passphrase was entered.

Agreed that it would be nice to have an option which reverted to using the default values (as is used by default currently).

BitfuryLightning commented 7 years ago

ReadPassword "checks" if STDIN is a tty device. When stdin is piped it is not a tty. So ReadPassword fails. It is not possible to simply pipe into a program which requires input from a terminal. Consider this small program:

package main

import (
    "fmt"
    "golang.org/x/crypto/ssh/terminal"
)

func main() {
    p, err := terminal.ReadPassword(0)
    fmt.Println("p=", p)
    fmt.Println("err=", err)
}

When it is launched from terminal

$ ./readpass
<Enter in terminal 'some'>                       
p=some
err= <nil>

$ echo 'some' | ./readpass
p=
err= inappropriate ioctl for device

This possibly can be solved using expect, but unbuffer does not work with Go ReadTerminal. That is why it would be nice to add some programmatic way of entering passphrase like RPC call or --read-passphrase-from-file command line option.

BitfuryLightning commented 7 years ago

@Roasbeef Do you still think that using terminal only for the passphrase is enough?

Roasbeef commented 7 years ago

Heya,

Thanks for that example, and the sample code as well! I agree that there needs to be at least one more option w.r.t initial authentication.

@aakselrod is currently working on our initial support for macaroons. Along with that set of changes, we'll also be implementing default (with an option to toggle off) required TLS connections in order to communicate with the gRPC server. This is required as certain RPC's will now at times be carrying sensitive data (like the macaroons).

I'm currently leaning towards an option along the liens of --read-passphrase-from-file as that'll be the most minimal diff in order to implementation this functionality in a useful way. If we go the RPC route, then some of the initial daemon initialization will need to be modified as currently the wallet is created and started before the RPC server is brought up.

Roasbeef commented 7 years ago

Mulled it over a bit and decided that the read from a file option promotes poor security practices on the side of the client. Instead, I think we should pursue the RPC pass. However, this can only land in after Alex's initial macaroon work which will add the TLS defaults.

BitfuryLightning commented 7 years ago

It seems that macaroon gets merged, so I will proceed.

halseth commented 6 years ago

@BitfuryLightning did you continue working on this? If not, I will take it over :)