Skip to content

Conversation

@guggero
Copy link
Contributor

@guggero guggero commented Sep 9, 2020

Next we add macaroon authentication to the RPC to be able to restrict access to authorized users only.
With these two features enabled by default, the loop RPC port can be opened to the internet because unauthorized access is no longer possible.

By default only one macaroon is created for faraday and stored in a file called ~/.faraday//faraday.macaroon

We update to the newest verion of lnd so we can use the updated macaroon
service.
NOTE: This is a compile time dependency update only, no RPC level update
is required.
To make it easier to use faraday as an external subserver, it is
necessary to extract the config validation into its own function that is
separate from loading the config.
@guggero guggero force-pushed the macaroons branch 2 times, most recently from 6d3f032 to 133400b Compare September 11, 2020 11:00
@guggero guggero requested a review from carlaKC September 11, 2020 11:20
Copy link
Contributor

@carlaKC carlaKC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just nits 🌵

// unlocks the macaroon database and creates the default macaroon if it doesn't
// exist yet. If macaroons are disabled in general in the configuration, none of
// these actions are taken.
func (s *RPCServer) startMacaroonService() error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is "if macaroons are disabled.." relevant here since we don't have a --no-macaroons option?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that was left over from when we still had the --no-macaroons in the loop PR.

if err != nil {
// The macaroon service is the only thing we started yet, so
// clean that up now before we exit.
if err := s.stopMacaroonService(); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure whether this is more simple, but we could have:

shutdownFuncs []func()error
defer func(){
  s:= range shutdownFuncs; s()
}

Then just append as we go and set the slice to nil if we startup successfully.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that idea, done.

To secure access to faraday's RPC server, we add a macaroon
authentication service and its gRPC interceptors to the daemon's server
connection.
When faraday runs in the same process as lnd (in LiT), it hooks itself
into lnd's RPC server as an external subserver. But because the user
should still be able to use the default faraday macaroon, the faraday
daemon must be able to validate its own macaroons as lnd's macaroon
service doesn't know the root key for it.
Copy link
Contributor

@carlaKC carlaKC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a look at the latest diff, looks good!

@guggero guggero merged commit c38c9b9 into lightninglabs:master Sep 11, 2020
@guggero guggero deleted the macaroons branch September 11, 2020 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants