Stateless remote mode: Adds passing in macaroon and tls data, and a CheckMacaroonPermissions RPC #51

orbitalturtle · 2021-05-15T02:43:02Z

For Terminal's stateless-remote mode, as described in lightninglabs/lightning-terminal#160, we add a few changes in this PR:

Adds new configuration options to the lndclient, for passing in macaroon data and tls data directly in stateless remote mode.
Also adds these features to the "basic client." For stateless-remote, the Pool server uses the basic client for connecting to LND.
Adds a wrapper for the CheckMacaroonPermissions RPC in LND for checking the validity of a Macaroon, so that we can use it to check validity in the Lightning Terminal. (Will open a separate PR into LND for this soon.)

Relies on:
lightningnetwork/lnd#5304

guggero

Thank you very much for the PR (and the one in lnd)!

I did a first quick pass and it looks really good. Going to look at the lnd PR next.

Just one main thing that needs to be changed:
Since this PR requires functionality that isn't yet even merged to lnd, we won't be able to merge it for a while. It'll be merged to a branch lnd-14-0 once we start working on that. For now, can you please open this against the lnd-13-0 branch? That way you also get a new type for the MacaroonPermission. You can edit the base of your PR by clicking the edit button of the title. A new drop down then appears where you can switch from master to lnd-13-0.

lnd_services.go

lightning_client.go

orbitalturtle · 2021-05-20T22:04:44Z

Thanks for the initial review @guggero! Will get to these changes soon

orbitalturtle · 2021-05-27T09:41:17Z

I made those changes @guggero :)

guggero

Thanks for the rebase! Just a few more minor things, this PR is getting very close.

lnd_services.go

basic_client.go

lnd_services.go

lightning_client.go

orbitalturtle · 2021-07-15T04:56:59Z

Thanks @guggero! Made those changes

lightning_client.go

testdata/permissions.json

lightning_client.go

carlaKC

Thanks for the PR!

Great addition, and nice to have some test coverage :)

macaroon_pouch.go

lnd_services_test.go

lnd_services.go

basic_client.go

carlaKC

Looking good!

Once the lnd version here is updated and the dependent loop PR points to this I'll test some of these options out.

lnd_services.go

macaroon_pouch.go

lnd_services.go

lightning_client.go

orbitalturtle · 2021-09-16T03:38:49Z

Made those changes @carlaKC ! FYI I also made a few small additional changes, since we're moving in a new direction for integrated mode in the Terminal (described here lightninglabs/lightning-terminal#160 (comment)). In short, needed to allow the option for passing in no tls creds, so I added another boolean parameter in GetTLSCredentials to enable that option.

guggero

Almost there, just a few more minor things. Great work pushing forward on this quite respectable chain of dependent PRs 👏

lnd_services.go

lightning_client.go

go.mod

carlaKC

Looking good! Just need to bump deps to master to get around that kvdb issue.

I think we can bump lightninglabs/loop/pull/406 to depend on this version of lndclient just to test out everything works as expected then merge.

lightning_client.go

guggero

LGTM 🎉

guggero · 2021-09-21T07:51:42Z

The linter is complaining:

lnd_services.go:805: G402: TLS InsecureSkipVerify set true. (gosec)
			InsecureSkipVerify: true,

Probably needs a // nolint:gosec comment on that line.

orbitalturtle · 2021-09-21T08:26:26Z

@guggero made that linter change!

@carlaKC Looks like i was wrong about that loop PR requiring this lndclient change ~ will be submitting a terminal change soon that requires it though

Roasbeef · 2021-10-22T23:15:52Z

basic_client.go

@@ -70,54 +72,28 @@ func NewBasicClient(lndHost, tlsPath, macDir, network string,
 // NewBasicConn creates a new basic gRPC connection to lnd. We call this
 // connection "basic" as it falls back to expected defaults if the arguments
 // aren't provided.
-func NewBasicConn(lndHost, tlsPath, macDir, network string,
+func NewBasicConn(lndHost string, tlsPath, macDir, tlsData, macData,
+	network string, insecure, systemCert bool,


Drive by comment, but rather than breaking the interface here, this should've just used a BasicClientOption as that's how the API is intended to evolve.

IMO we should likely follow this path, as otherwise updating will break every user of NewBasicConn as is.

guggero self-requested a review May 17, 2021 10:13

guggero reviewed May 18, 2021

View reviewed changes

lnd_services.go Show resolved Hide resolved

lightning_client.go Show resolved Hide resolved

lightning_client.go Outdated Show resolved Hide resolved

orbitalturtle changed the base branch from master to lnd-13-0 May 20, 2021 05:58

bhandras force-pushed the lnd-13-0 branch from 8b08194 to 79c295f Compare May 26, 2021 16:11

orbitalturtle force-pushed the custom-macaroon-hex branch 4 times, most recently from 86b49c3 to 00860e1 Compare May 27, 2021 09:38

carlaKC force-pushed the lnd-13-0 branch from 79c295f to 57db0d9 Compare June 8, 2021 14:18

orbitalturtle force-pushed the custom-macaroon-hex branch from 00860e1 to 93d7f21 Compare July 12, 2021 08:39

orbitalturtle requested a review from guggero July 12, 2021 08:41

guggero reviewed Jul 12, 2021

View reviewed changes

lnd_services.go Outdated Show resolved Hide resolved

lnd_services.go Outdated Show resolved Hide resolved

basic_client.go Outdated Show resolved Hide resolved

lnd_services.go Outdated Show resolved Hide resolved

lightning_client.go Outdated Show resolved Hide resolved

guggero requested a review from carlaKC July 12, 2021 11:29

orbitalturtle mentioned this pull request Jul 14, 2021

Add stateless remote mode lightninglabs/lightning-terminal#245

Closed

guggero self-requested a review July 15, 2021 07:55

orbitalturtle force-pushed the custom-macaroon-hex branch from 93d7f21 to 318f93f Compare July 16, 2021 23:53

This was referenced Jul 19, 2021

Add option to stop creation of macaroons lightninglabs/pool#272

Closed

Add option to stop creation of macaroons lightninglabs/loop#406

Closed

Add option to not create new macaroons lightninglabs/faraday#127

Closed

guggero reviewed Jul 23, 2021

View reviewed changes

lightning_client.go Show resolved Hide resolved

testdata/permissions.json Outdated Show resolved Hide resolved

lightning_client.go Show resolved Hide resolved

carlaKC reviewed Jul 27, 2021

View reviewed changes

carlaKC force-pushed the lnd-13-0 branch from 5ae5f47 to b3d3ba7 Compare August 10, 2021 07:10

orbitalturtle force-pushed the custom-macaroon-hex branch from 318f93f to 446bb30 Compare August 12, 2021 21:32

orbitalturtle requested review from guggero and carlaKC August 12, 2021 21:36

carlaKC reviewed Aug 18, 2021

View reviewed changes

lnd_services.go Show resolved Hide resolved

macaroon_pouch.go Show resolved Hide resolved

lnd_services.go Show resolved Hide resolved

lnd_services.go Show resolved Hide resolved

lnd_services.go Outdated Show resolved Hide resolved

lightning_client.go Show resolved Hide resolved

guggero removed their request for review August 31, 2021 07:52

carlaKC force-pushed the lnd-13-0 branch from b3d3ba7 to cbde036 Compare September 3, 2021 08:15

carlaKC force-pushed the lnd-13-0 branch from cbde036 to cbabe4c Compare September 13, 2021 07:48

orbitalturtle force-pushed the custom-macaroon-hex branch 3 times, most recently from 4edbd3c to 5d56d7b Compare September 16, 2021 03:33

orbitalturtle requested review from guggero and carlaKC September 16, 2021 03:38

guggero reviewed Sep 16, 2021

View reviewed changes

lnd_services.go Show resolved Hide resolved

lnd_services.go Outdated Show resolved Hide resolved

lightning_client.go Show resolved Hide resolved

go.mod Outdated Show resolved Hide resolved

orbitalturtle force-pushed the custom-macaroon-hex branch 2 times, most recently from 6c31895 to 00cb063 Compare September 17, 2021 23:24

orbitalturtle added 2 commits September 17, 2021 18:28

Add CustomMacaroonHex as a config option

bc17a3b

Test that new CustomMacaroonHex option works properly

692d8ab

orbitalturtle force-pushed the custom-macaroon-hex branch from 00cb063 to 65fb831 Compare September 17, 2021 23:29

carlaKC approved these changes Sep 20, 2021

View reviewed changes

lightning_client.go Outdated Show resolved Hide resolved

orbitalturtle force-pushed the custom-macaroon-hex branch 2 times, most recently from 8397233 to b06054f Compare September 20, 2021 20:28

guggero approved these changes Sep 21, 2021

View reviewed changes

orbitalturtle added 4 commits September 21, 2021 03:21

Allow passing in tls certificate as data

3a5ec2d

Add support for passing in macaroons/tls cert as data to basic client

f374c88

Add CheckMacaroonPermissions command to client

577f819

Update dependencies with required change to lnd

bd95f28

orbitalturtle force-pushed the custom-macaroon-hex branch from b06054f to bd95f28 Compare September 21, 2021 08:21

guggero merged commit ab8634c into lightninglabs:lnd-13-0 Sep 21, 2021

Roasbeef reviewed Oct 22, 2021

View reviewed changes

This was referenced Oct 25, 2021

Implement RawTLS and RawMacaroons parameters in LndServicesConfig #35

Closed

basic_client: make latest change non-breaking #76

Merged

guggero mentioned this pull request Nov 3, 2021

Connecting to LND by supplying raw certs and macaroons #29

Closed

Stateless remote mode: Adds passing in macaroon and tls data, and a CheckMacaroonPermissions RPC #51

Stateless remote mode: Adds passing in macaroon and tls data, and a CheckMacaroonPermissions RPC #51

Uh oh!

Conversation

orbitalturtle commented May 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

guggero left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

orbitalturtle commented May 20, 2021

Uh oh!

orbitalturtle commented May 27, 2021

Uh oh!

guggero left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

orbitalturtle commented Jul 15, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

carlaKC left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

carlaKC left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

orbitalturtle commented Sep 16, 2021

Uh oh!

guggero left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

carlaKC left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

guggero left a comment

Choose a reason for hiding this comment

Uh oh!

guggero commented Sep 21, 2021

Uh oh!

orbitalturtle commented Sep 21, 2021

Uh oh!

Roasbeef Oct 22, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

orbitalturtle commented May 15, 2021 •

edited

Loading