lnd 0.11.1: allow custom macaroon to be used instead of requiring all subserver macaroons#19
Conversation
carlaKC
left a comment
There was a problem hiding this comment.
Really excited to see this change come in!
Tried it out for Faraday here, and there's a little bit of a chicken-and-egg issue in that you need all of the macaroons to get a lndclient instance to then call MacaroonRecipe. If I'm using it in the way you were thinking we'd use it, could be missing the point?
What do you think about using a basic client rather than the lightning.Client interface for MacaroonRecipe? That could be a bit smoother for people who are starting up something like Faraday or Loop the first time, just give an admin macaroon and get the permissions needed, bake a new macaroon then run with it? If that wasn't the plan, it's still very useful to have to just update docs!
lightning_client.go
Outdated
| rpcCtx = s.adminMac.WithMacaroonAuth(rpcCtx) | ||
|
|
||
| perms, err := s.client.ListPermissions( | ||
| ctx, &lnrpc.ListPermissionsRequest{}, |
There was a problem hiding this comment.
needs to be rpcCtx, this fails with requires macaroon rn
There was a problem hiding this comment.
Ah yes, great catch, thanks!
| // From the pointer type we can find out the interface, its name | ||
| // and what methods it declares. | ||
| ifaceType := reflect.TypeOf(ifacePtr).Elem() | ||
| serverName := strings.ReplaceAll(ifaceType.Name(), "Client", "") |
There was a problem hiding this comment.
I like that we can use reflection to gather method names but maybe a simple constant map of rpc -> method name would also suffice (and is easier to check/debug). Is perfectly fine as is, just a comment.
There was a problem hiding this comment.
If the list of exceptions keeps growing, I agree that we can switch over to a full map instead. But for now, this keeps the code quite small and also doesn't require you to add/change anything, if you add a new method that's called the same as the RPC it calls.
There was a problem hiding this comment.
if you add a new method that's called the same as the RPC it calls
nittynit: can see this being forgotten, maybe add a PR template with a reminder about this? I find that pretty useful. Can be in a follow up.
There was a problem hiding this comment.
The unit tests should catch that. But to avoid an extra git push cycle I added it to the checklist 👍
43e125c to
bc07123
Compare
|
Thanks for all the good input, I addressed all comments. As mentioned, this bumps the requirement for |
carlaKC
left a comment
There was a problem hiding this comment.
LGTM, thanks for getting to this! Going to be really helpful to have 🌹
| // From the pointer type we can find out the interface, its name | ||
| // and what methods it declares. | ||
| ifaceType := reflect.TypeOf(ifacePtr).Elem() | ||
| serverName := strings.ReplaceAll(ifaceType.Name(), "Client", "") |
There was a problem hiding this comment.
if you add a new method that's called the same as the RPC it calls
nittynit: can see this being forgotten, maybe add a PR template with a reminder about this? I find that pretty useful. Can be in a follow up.
With custom macaroon baking now available in lnd, it doesn't make sense that a user must specify all subserver macaroons if a single, custom macaroon is sufficient. With this commit we add a new option to the lnd services that allows users to specify a single, custom macaroon that overwrites the macaroon path parameter.
To be able to write macaroon recipes, we need to be able to query all existing RPC endpoints and the macaroon permissions they need. This feature was only introduced in lnd v0.11.1-beta and therefore lifts the required version lndclient needs to function.
To make it easy to bake custom macaroons for just the subservers that lndclient supports, we add a function that returns all required macaroon permissions for a given RPC package. This function uses the ListPermissions RPC of lnd to query all existing RPC method URIs and their required permissions. With some reflection magic a subset of permissions is them assembled for the given package.
bc07123 to
90e91a9
Compare
Addresses the problem described in lightninglabs/loop#299.
This PR adds two functionalities:
CustomMacaroonPathinstead ofMacaroonDirinNewLndServices) instead of needing to have alllndmacaroons in the same directory. This allows users to use theadmin.macaroonor a custom baked one.MacaroonRecipeis added. This requires thelnrpc.Lightning.ListPermissionsRPC which was only added inlnd v0.11.1.NOTE: This PR is against the
lnd-11-1branch as it requires a compile-time dependency to that version. This code can also be used with olderlndversions but then theMacaroonRecipefunction will throw an error.