multi: add macaroon service and remove ui password use in litcli #308

ellemouton · 2022-01-07T11:16:36Z

Add a macaroon service to litd for its session rpc server and use this to validate incoming LitURI calls. Remove ui password requirement from litcli.

config.go

Roasbeef · 2022-01-08T01:00:58Z

session_rpcserver.go

+
+var (
+	// TODO(elle): Temp values. change these before landing pr.
+	nBytes, _ = hex.DecodeString(


Should instead be a key derived from the user's seed?

from the users seed in LND? yes this is the idea. So we have a static pubkey here (need to still gen a proper one) and then we derive a shared secret with lnd using this static key and a seed in LND using the DeriveSharedSecret endpoint. the macaroon service does this.
Is this what you mean or is there a litd user seed that you mean?

Here's the public key derived with numsgen from LNC with the phrase "Shared Secret": 0215b5a3e0ef58b101431e1e513dd017d1018b420bd2e89dcd71f45c031f00469e
Should probably mention the phrase and link to https://github.com/lightninglabs/lightning-node-connect/tree/master/mailbox/numsgen

terminal.go

guggero

Great next step towards getting rid of the UI password!
Code looks very good, did an initial pass.

Will do some manual testing next.

session_rpcserver.go

guggero · 2022-01-12T17:25:52Z

session_rpcserver.go

+
+var (
+	// TODO(elle): Temp values. change these before landing pr.
+	nBytes, _ = hex.DecodeString(


Here's the public key derived with numsgen from LNC with the phrase "Shared Secret": 0215b5a3e0ef58b101431e1e513dd017d1018b420bd2e89dcd71f45c031f00469e
Should probably mention the phrase and link to https://github.com/lightninglabs/lightning-node-connect/tree/master/mailbox/numsgen

session_rpcserver.go

cmd/litcli/main.go

ellemouton · 2022-02-14T12:50:59Z

this is pretty much ready for re-review. Just waiting on tagged releases for pool & faraday so the go.mod here can be bumped to include the macaroon service changes in those repos.

guggero · 2022-03-29T12:13:20Z

Can you rebase this when you have time please so I can do another pass?

ellemouton · 2022-03-30T14:20:55Z

Mmmm ok I rebased but need to still figure out how to update the itests to use the macaroon path instead of ui password....

ellemouton · 2022-03-31T13:06:50Z

ok, the itest is fixed now @guggero , this is ready for re-review 👍

(btw, the lnc-auth itest for remote mote consistently fails for me on my machine (on master branch too). any idea why that could be?)

guggero

tACK, LGTM. Only nits and itest changes required. Also made sure the UI still works with the UI password.

rpc_proxy.go

terminal.go

itest/litd_mode_integrated_test.go

ellemouton

Thanks @guggero 🚀 updated 👍

rpc_proxy.go

ellemouton · 2022-04-13T10:52:32Z

erg the itests are failing now. dont think i quite understand what do want/dont want on the lnd vs litd port...

ellemouton · 2022-04-13T11:24:46Z

nvm, i found the issue. will push up in a minute

ellemouton · 2022-05-16T07:30:22Z

will update this today 👍

ellemouton · 2022-05-16T13:23:10Z

updated @guggero 😊

guggero

Two small things, otherwise looks good to me. Nice work 💯

itest/litd_mode_integrated_test.go

session_rpcserver.go

lightninglabs-deploy · 2022-06-03T09:33:55Z

@Roasbeef: review reminder

Currently, if in lnd-remote mode, the tls cert path will be constructed to be in the network directory which it should not be.

Let sessionRpcServer handle the session db and session server instead of LightningTerminal handling those direclty. Then start the sessionRpcServer as if it was a subserver like loop/pool.

Add a macaroonService to the main LightningTerminal struct.

Use the new macaroon service to verify LitURI calls.

ellemouton · 2022-06-05T08:53:22Z

@Roasbeef , could you take a quick look at this pr? I think we are just waiting for your ACK here since you previously left some review comments on it

Roasbeef

PR is looking pretty good! Only one main comment re allowing a read only session type when we go to validate the session type itself.

itest/litd_mode_integrated_test.go

Roasbeef · 2022-06-06T22:58:25Z

terminal.go

-		wrap := fmt.Errorf("invalid basic auth")
-		_, err := g.rpcProxy.convertBasicAuth(ctx, fullMethod, wrap)
-		if err != nil {
+		if !g.macaroonServiceStarted {


How are we planning on handling the rollout here re people with old litd installs and TW? Specifically, lets say a new TW client is running in the browser and goes to connect to my existing (unupgraded) litd node, as is this operation will fail.

LitURI calls are not allowed through TW.

Roasbeef · 2022-06-06T22:59:28Z

cmd/litcli/main.go

@@ -1,14 +1,11 @@
 package main

 import (
-	"context"


Note, this means that in stateless mode, litcli calls wont work and all calls will need to go through tht UI.

Which UI?

Isn't it the case that litcli can still just use the macaroon to interact w/ litd? Stateless just means we don't write the macaroon to disk, but as long as litcli can find that macaroon somehow, then things should work.

Which UI?

The UI served from the Litd binary.

but as long as litcli can find that macaroon somehow, then things should work.

yeah 👍 updated the commit message 👍

itest/litd_mode_integrated_test.go

Roasbeef · 2022-06-06T23:01:38Z

session_rpcserver.go

-				"the necessary macaroon: %w", pubKeyBytes, err)
-			return nil
-		}
+	if sess.Type != session.TypeMacaroonAdmin &&


Below we check TypeMacaroonReadonly, so presumably we want to allow it here as well?

see line 184?

this check errors out if type is not either admin or readonly. If it is one of those then it is allowed.

Remove the use of the UI password from litcl. Use the litd macaroon instead. Note, this means that in stateless mode, litcli won't have a macaroon to use on disk and one must be baked specifically.

Roasbeef

LGTM 🥟

ellemouton requested a review from guggero January 7, 2022 11:16

ellemouton force-pushed the macaroonService branch 2 times, most recently from b69ed87 to ab37af6 Compare January 7, 2022 11:27

Roasbeef reviewed Jan 8, 2022

View reviewed changes

ellemouton force-pushed the macaroonService branch from ab37af6 to ef27e42 Compare January 10, 2022 14:18

guggero reviewed Jan 12, 2022

View reviewed changes

ellemouton force-pushed the macaroonService branch 3 times, most recently from 3d664cc to e4f3ab4 Compare January 17, 2022 14:50

ellemouton force-pushed the macaroonService branch from e4f3ab4 to b2035c5 Compare January 31, 2022 10:58

ellemouton requested a review from guggero January 31, 2022 11:17

ellemouton force-pushed the macaroonService branch 2 times, most recently from dd942fd to 5b93b03 Compare February 14, 2022 12:49

ellemouton force-pushed the macaroonService branch from 5b93b03 to 3d08303 Compare March 30, 2022 14:19

ellemouton force-pushed the macaroonService branch from 3d08303 to 7597dd7 Compare March 31, 2022 12:59

guggero previously approved these changes Apr 12, 2022

View reviewed changes

rpc_proxy.go Show resolved Hide resolved

terminal.go Outdated Show resolved Hide resolved

itest/litd_mode_integrated_test.go Outdated Show resolved Hide resolved

itest/litd_mode_integrated_test.go Outdated Show resolved Hide resolved

ellemouton dismissed guggero’s stale review via ee35686 April 13, 2022 10:11

ellemouton force-pushed the macaroonService branch 3 times, most recently from c767f6a to 86655ab Compare April 13, 2022 10:20

ellemouton commented Apr 13, 2022

View reviewed changes

rpc_proxy.go Show resolved Hide resolved

ellemouton force-pushed the macaroonService branch from 86655ab to c63d828 Compare April 13, 2022 10:39

ellemouton force-pushed the macaroonService branch from c63d828 to d0d7b27 Compare April 13, 2022 11:32

guggero previously approved these changes Apr 13, 2022

View reviewed changes

ellemouton dismissed guggero’s stale review via 5d65a6e May 16, 2022 11:51

ellemouton force-pushed the macaroonService branch 3 times, most recently from 22b9432 to 29806d1 Compare May 16, 2022 13:04

ellemouton requested review from guggero and Roasbeef May 18, 2022 13:33

guggero previously approved these changes May 19, 2022

View reviewed changes

itest/litd_mode_integrated_test.go Show resolved Hide resolved

session_rpcserver.go Outdated Show resolved Hide resolved

ellemouton dismissed guggero’s stale review via 52981ea May 20, 2022 08:23

ellemouton force-pushed the macaroonService branch from 29806d1 to 52981ea Compare May 20, 2022 08:23

guggero previously approved these changes May 20, 2022

View reviewed changes

ellemouton added 4 commits June 5, 2022 10:51

cmd/litcli: fix tls cert path

bcf8006

Currently, if in lnd-remote mode, the tls cert path will be constructed to be in the network directory which it should not be.

multi: extract sessionRPCserver stuff into its own struct

1aee594

Let sessionRpcServer handle the session db and session server instead of LightningTerminal handling those direclty. Then start the sessionRpcServer as if it was a subserver like loop/pool.

multi: add macroon service to litd

50cfd3b

Add a macaroonService to the main LightningTerminal struct.

multi: validate macaroons for lit calls

f011bc8

Use the new macaroon service to verify LitURI calls.

ellemouton dismissed guggero’s stale review via 79d2531 June 5, 2022 08:51

ellemouton force-pushed the macaroonService branch from 52981ea to 79d2531 Compare June 5, 2022 08:51

Roasbeef requested changes Jun 6, 2022

View reviewed changes

ellemouton force-pushed the macaroonService branch from 79d2531 to 371e17f Compare June 7, 2022 09:30

ellemouton added 2 commits June 7, 2022 11:46

cmd/litcli: use macaroon instead of ui password

7a4d84a

Remove the use of the UI password from litcl. Use the litd macaroon instead. Note, this means that in stateless mode, litcli won't have a macaroon to use on disk and one must be baked specifically.

multi: deprecate UIPassword session type

6ebab93

ellemouton force-pushed the macaroonService branch from 371e17f to 6ebab93 Compare June 7, 2022 09:47

ellemouton requested a review from Roasbeef June 7, 2022 09:48

Roasbeef approved these changes Jun 7, 2022

View reviewed changes

guggero approved these changes Jun 7, 2022

View reviewed changes

guggero merged commit 53618d9 into lightninglabs:master Jun 7, 2022

ellemouton deleted the macaroonService branch June 7, 2022 18:15

multi: add macaroon service and remove ui password use in litcli #308

multi: add macaroon service and remove ui password use in litcli #308

Uh oh!

Conversation

ellemouton commented Jan 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

guggero left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ellemouton commented Feb 14, 2022

Uh oh!

guggero commented Mar 29, 2022

Uh oh!

ellemouton commented Mar 30, 2022

Uh oh!

ellemouton commented Mar 31, 2022

Uh oh!

guggero left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ellemouton left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ellemouton commented Apr 13, 2022

Uh oh!

ellemouton commented Apr 13, 2022

Uh oh!

ellemouton commented May 16, 2022

Uh oh!

ellemouton commented May 16, 2022

Uh oh!

guggero left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lightninglabs-deploy commented Jun 3, 2022

Uh oh!

ellemouton commented Jun 5, 2022

Uh oh!

Roasbeef left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ellemouton Jun 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ellemouton commented Jan 7, 2022 •

edited

Loading

ellemouton Jun 7, 2022 •

edited

Loading