Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpc+lnd: add new invoice-only macaroon #904

Merged
merged 4 commits into from
Mar 21, 2018

Conversation

Roasbeef
Copy link
Member

@Roasbeef Roasbeef commented Mar 20, 2018

In this PR, we add a new default macaroon type: invoice.macaroon. The capabilities that this macaroon can unlock are restricted only to: listing invoices, generating invoices, subscribing for invoice notifications, and also generating new addresses.

We've received several requests from those working to integrate lnd into their exchange or merchant workflow to have an macaroon that can only generate invoices. The advantage of this is that a developer can provision a box or process, that can speak to lnd but only to generate new invoices, or to poll for the status of existing invoices. This is a boon for security as it implements a strict separation of invoices, allowing services to create isolated instances with locked down permission only sufficient to carry out their task.

NOTE: as implemented now, users will need to regenerate their existing read and write macaroons due to the addition of a more specific entity.

One other thing to note is that we'll only generate the invoice macaroon if none of the macaroons are found. This mirrors the existing behavior to allow an instance to be spun up, only having a particular macaroon in its data directory.

This replaces #496.

@joaodealmeida
Copy link

Is there any read only macaroon permission level in the roadmap? Such as access to see the stats without being able to perform actions (open/close channels, sendcoins)?

@Roasbeef
Copy link
Member Author

Roasbeef commented Mar 21, 2018

@joaodealmeida that already exists. It's called readonly.macaroon on disk and does precisely that!

@junderw
Copy link
Contributor

junderw commented Mar 21, 2018

awesome!

Copy link
Contributor

@junderw junderw left a comment

Choose a reason for hiding this comment

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

LGTM

aakselrod
aakselrod previously approved these changes Mar 21, 2018
Copy link
Contributor

@aakselrod aakselrod left a comment

Choose a reason for hiding this comment

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

This looks great!

…acaroon

In this commit, we add a new invoicePermissions slice. This contains
all the permission that a holder of an invoice.macaroon is able to
access, and no others. We also include read and write access to
addresses as this may be useful from the PoV of a merchant or exchange.
In the prior commit, we added a new set of permissions and also a new
entity: “invoices”. We’ll add this set of entities to the read and
write permissions accordingly as well to ensure that the existing
macaroons have access to all the items that the invoice.macaroon does.
… entity

In this commit, we modify the existing invoice RPC macaroon permissions
to target a more specific entity: “invoices”. As a result of this
commit, once node operators update, they’ll need to regenerate their
readonly.macaroon as it now needs this additional entity encoded within
it.
In this commit, we wrap up the prior ones and introduce config
settings, as well as proper generation for a new invoice-only macaroon.
All prior invoice path rules are also properly enforced of this new
invoice.macaroon.
@Roasbeef
Copy link
Member Author

Pushed to resolve the conflict, waiting on built, then will merge!

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.

4 participants