[tor] Add ability to encrypt the Tor private key on disk

[orbitalturtle]

This PR replaces #4458. Since I've been making some of the changes for @gkrizek and it's been a bit tricky to manage making PRs into his PRs... we thought it would be nice to have a fresh start. :)

Depends on #6526

Graham's original PR description:

It's possible that a user might not want the Tor private key to sit on the disk in plaintext (it is a private key after all). So this commit adds a new flag to encrypt the Tor private key on disk using the wallet's seed. When the --tor.encryptkey flag is used, LND will still write the Tor key to the same file, however it will now be encrypted instead of plaintext. This essentially uses the same method to encrypt the Tor private key as is used to encrypt the Static Channel Backup file.

Summary of Changes:

• Moves the crypto.go file from chanbackup into its own package called lnencrypt. This was needed because we can't directly reference the encryptPayloadToWriter and decryptPayloadFromReader functions from the chanbackup package due to circular dependency issues. Also, I think it's very likely that future enhancements will also want to use these same functions. Thus I've moved them to their own package.

• Creates a flag called --tor.encryptkey which encrypts the Tor private key.

When this is set it encrypts the private key and writes the encrypted blob to the same file on disk. This allows users to still keep the same functionality and flows as before (like refreshing a hidden service), but adds protection when running in untrusted environments.

• Add necessary tests and documentation

[orbitalturtle]

@guggero Oops, sorry, looks like I broke something when trying to fix the onion file stuff, as discussed here: #4458 (comment) But I'm having trouble fixing the issue...

First question, if you get a chance: Does moving the onion file stuff to a new package within Tor make sense? Re: this convo #4458 (comment)?

If it does make sense to move it... I'm getting the following error that I am struggling to fix

no required module provides package github.com/lightningnetwork/lnd/tor/onionfile; to add it: go get github.com/lightningnetwork/lnd/tor/onionfile

and if I try to do that...

go get: module github.com/lightningnetwork/lnd/tor@upgrade found (v1.0.0), but does not contain package github.com/lightningnetwork/lnd/tor/onionfile

[guggero]

Since you last rebased we made the tor package its own submodule.
That fixed a dependency issue but makes it a bit harder to create PRs that change the functionality, unfortunately.

You need to do the following now (that goes for any change to a package that is a submodule, meaning it has its own go.mod file):

• create PR 1 with the changes to the submodule only (in this case al the changes in tor).
• after PR 1 is merged, a new tag for the affected submodule is pushed
• create PR 2 that uses the updated submodule in the rest of the code base

Of course for development it makes sense to create PR 1 and PR2 at the same time and just stack the commits of both in the second PR and use a replace directive until PR 1 is merged and tagged.

[orbitalturtle]

@guggero Ah got it, thank you for the explanation!

[orbitalturtle]

Hey @guggero one more question for you!

Now I've split the Tor code out into a separate PR. But the Tor changes depend on changes in the keychain and lnencrypt packages. So now I'm getting this response when I try to run "go test" in the Tor module

onionfile/onion_file.go:10:2: no required module provides package github.com/lightningnetwork/lnd/keychain; to add it:
	go get github.com/lightningnetwork/lnd/keychain
onionfile/onion_file.go:11:2: no required module provides package github.com/lightningnetwork/lnd/lnencrypt; to add it:
	go get github.com/lightningnetwork/lnd/lnencrypt

And when I do go get github.com/lightningnetwork/lnd/keychain I get:

go get: module github.com/lightningnetwork/lnd@upgrade found (v0.0.2), but does not contain package github.com/lightningnetwork/lnd/keychain

Wondering if I need to split out the keychain & lnencrypt changes into a third PR? Or if something else is going on

[guggero]

Hmm, this is a tough one. I guess what I forgot to mention is that the tor submodule, to be independent and to work standalone and to avoid circular dependencies, shouldn't have any references to the parent lnd packages.

So this requires some additional refactoring...

My suggestion:

• refactor the EncryptPayloadtoWriter and DecryptPayloadFromReader to be an interface that is independent of the keychain package (requires the genEncryptionKey part to be pulled out and the encryption key be a []byte directly).
• copy that new interface to the tor package and accept it as the input for the NewOnionFile instead of the keyRing (that's the way to decouple packages: if an interface has exactly the same methods with the same signatures they are compatible, even if the interface is declared under different names in different packages)
• Create a simple mock for that new interface instead of using the lntest/mock mock one

[orbitalturtle]

Got it, thank you @guggero . Will let you know when I have the latest PRs up.

[guggero]

Sounds good! Will review once you give me the heads up.

[orbitalturtle]

@guggero One more question... so because we're getting rid of all the lnd dependencies in the Tor module, we don't need to split the onionfile code into a new subpackage of Tor, correct? Just want to make sure I before I move that back

[guggero]

I think the OnionFile struct (and its file) can be moved directly to the main tor package/submodule. Or it can stay in the tor/onionfile package (but shouldn't be a submodule, so shouldn't have its own go.mod file) and be renamed to just File (since it will be used as onionfile.File externally, with the current name onionfile.OnionFile it's a bit too verbose IMO).

[orbitalturtle]

Rebased this just fyi. It's ready for a look!

[guggero]

Thanks for the latest updates!
I think we're getting closer here. The diff can be quite a bit smaller though if we follow @Crypt-iQ's suggestion.

[orbitalturtle]

@Crypt-iQ and @guggero, thanks again for the reviews! I made those updates.

[guggero]

Can be rebased now and use the tor/v1.1.0 dependency in go.mod.

[guggero]

Thanks for the rebase and update. Tested the latest version, all seems to work as advertised 🎉

There are still quite a few nits in there. I'd appreciate a scan of all commits before requesting review (in future PRs) to avoid too many rounds of review.

[guggero]

Very nice, LGTM 🎉

[orbitalturtle]

Thanks @guggero! I made those changes. And sure, I'll definitely keep a closer eye out for those nits & code style guidelines in the future!

[lightninglabs-deploy]

@Crypt-iQ: review reminder

[guggero]

Needs a rebase.

[orbitalturtle]

@guggero rebased