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

lncli: add command to delegate macaroons #1147

Closed
wants to merge 1 commit into from

Conversation

guggero
Copy link
Collaborator

@guggero guggero commented Apr 28, 2018

This PR addresses #283 and introduces the new command delegatemacaroon to the lncli tool.

Currently, this just exposes the functionality that the lncli already uses internally to the command line interface:

  • Take an existing macaroon (by specifying --macaroonpath, default is ~/.lnd/admin.macaroon)
  • [optional] Add a timeout to it (by specifying --timeout)
  • [optional] Restrict it to a specific IP address (by specifying --ip_address)

Depending on the --format parameter, the following output is produced (truncated for brevity):

lncli delegatemacaroon --format=json --timeout=3600 --ip_address=123.123.123.123:

{
        "c": [
                {
                        "i": "time-before 2018-04-28T16:02:23.47112259Z"
                },
                {
                        "i": "ipaddr 123.123.123.123"
                }
        ],
        "l": "lnd",
        "i64": "AwoQ7V1quyefYWYwh9TyFCJCBhIBMBoWCgdhZGRyZXNzEgRy....",
        "s64": "BqjN2qrARiy4dRfX7H__3t0XRWqXtVjNs5gyXL7oU9Q"
}

lncli delegatemacaroon --format=hex --timeout=3600 --ip_address=123.123.123.123:

{
        "macaroonHex": "0201036c6e6402bb01030a10ed5d6abb279f61663..."
}

Once new first-party caveats are introduced, they can be implemented here as well.

@aakselrod:
Does this cover your idea as described in #283?
As I see it, there are at least two ways to delegate macaroons in lnd:

  1. Take one of the three existing macaroon files (admin.macaroon, invoice.macaroon and readonly.macaroon) and add first-party caveats to them (as implemented in this PR).
  2. Bake fresh macaroons by using the encrypted root key in macaroons.db and define custom operations/permissions. Then add first-party caveats to those newly baked macaroons.
    The second option has the advantage that the user can customize the allowed operations/permissions. But the big disadvantage that the macaroons.db need to be unlocked (lnd needs to be stopped) and decrypted with the wallet password.

Am I correct in the assumption that you meant to implement option 1 in your issue, instead of option 2?

Thanks for your feedback!

@vegardengen
Copy link
Contributor

There is definitely a need for option two. For example in the woocommerce LND plugin, there is the need for invoice macaroon rights plus some other read-rights. For now, that means we need to give it the admin macaroon, which is not too good security wise.

@guggero
Copy link
Collaborator Author

guggero commented Apr 28, 2018

I see, thanks for your input!

In that case I propose to add a new gRPC method named NewMacaroon:

  • Introduce a new permission entity named macaroon (could be used for macaroon based RPCs mentioned in Add support for accounting-based macaroons #291 too).
  • write access to the entity macaroon is necessary to call the method NewMacaroon.
  • The admin.macaroon gets write access to macaroon.
  • Add a new command to lncli called newmacaroon that calls this gRPC method. This removes the need to read the macaroons.db in the command line client and therefore removes the disadvantages mentioned in option 2 above.
  • As parameters to the NewMacaroon method we could pass the list of entity/action pairs for the allowed operations, as well as first-party caveats.

What do you think?
If there are no objections, should I create a new PR or continue here?

@guggero guggero force-pushed the delegate-macaroons branch 2 times, most recently from ffba6ef to fbc8844 Compare April 30, 2018 06:26
@meshcollider meshcollider added authentication macaroons cli Related to the command line interface labels May 1, 2018
@Roasbeef Roasbeef added this to the 0.5 milestone May 2, 2018
@guggero guggero force-pushed the delegate-macaroons branch 6 times, most recently from 2eb1898 to 5c1749e Compare May 8, 2018 10:00
@guggero guggero force-pushed the delegate-macaroons branch 2 times, most recently from 4183739 to 585a488 Compare May 12, 2018 10:51
@guggero guggero force-pushed the delegate-macaroons branch 2 times, most recently from 726b942 to a580366 Compare May 23, 2018 10:58
@guggero guggero force-pushed the delegate-macaroons branch 2 times, most recently from c7c6d8b to 06ff9b8 Compare June 1, 2018 06:14
@guggero guggero force-pushed the delegate-macaroons branch 3 times, most recently from 3bbd316 to 2449a9d Compare June 9, 2018 06:04
@guggero guggero force-pushed the delegate-macaroons branch 2 times, most recently from 94c6ec6 to 7346a67 Compare June 14, 2018 06:20
@guggero guggero force-pushed the delegate-macaroons branch 2 times, most recently from 6bf6300 to c7b8f70 Compare July 1, 2018 10:30
@guggero guggero force-pushed the delegate-macaroons branch from c7b8f70 to 9a4dfc2 Compare July 10, 2018 06:55
@Roasbeef Roasbeef added the P3 might get fixed, nice to have label Jul 10, 2018
@aakselrod
Copy link
Contributor

Tested the new changes (rebased on current master). This LGTM.

aakselrod
aakselrod previously approved these changes Oct 17, 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.

Tested ACK. LGTM, again. Rebases on current master with no issues AFAICT.

@guggero guggero force-pushed the delegate-macaroons branch from 4dde55d to d2934d5 Compare October 21, 2018 15:42
@halseth halseth added this to the 0.5.2 milestone Oct 29, 2018
@guggero guggero force-pushed the delegate-macaroons branch from d2934d5 to 4a86d01 Compare November 3, 2018 08:11
@guggero guggero force-pushed the delegate-macaroons branch 2 times, most recently from f3df516 to 18cfc3f Compare December 29, 2018 13:26
@guggero guggero force-pushed the delegate-macaroons branch 2 times, most recently from 12153de to 85647f4 Compare December 30, 2018 21:11
@guggero guggero force-pushed the delegate-macaroons branch from 85647f4 to abdc63d Compare January 4, 2019 07:52
@Roasbeef Roasbeef removed this from the 0.5.2 milestone Jan 16, 2019
@guggero guggero force-pushed the delegate-macaroons branch from abdc63d to bb940fa Compare April 17, 2019 10:24
@guggero guggero force-pushed the delegate-macaroons branch from bb940fa to 4c752cc Compare May 26, 2019 12:43
@guggero guggero force-pushed the delegate-macaroons branch from 4c752cc to fc42658 Compare June 4, 2019 06:44
@guggero
Copy link
Collaborator Author

guggero commented Oct 28, 2019

Now that #1160 is merged, this doesn't offer any new functionality. Closing as redundant.

@guggero guggero closed this Oct 28, 2019
@guggero guggero deleted the delegate-macaroons branch October 28, 2019 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
authentication cli Related to the command line interface macaroons needs review PR needs review by regular contributors needs testing PR hasn't yet been actively tested on testnet/mainnet P3 might get fixed, nice to have
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants