Skip to content

macaroons+rpcperms: add protector caveats to restrict RPC request fields - #11117

Open
GeorgeTsagk wants to merge 5 commits into
lightningnetwork:masterfrom
GeorgeTsagk:protector-caveat
Open

macaroons+rpcperms: add protector caveats to restrict RPC request fields#11117
GeorgeTsagk wants to merge 5 commits into
lightningnetwork:masterfrom
GeorgeTsagk:protector-caveat

Conversation

@GeorgeTsagk

@GeorgeTsagk GeorgeTsagk commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Change Description

This PR adds a first-party macaroon caveat, protector <profile-name>, that
restricts which request fields may be set on the RPC methods covered by a
named, compiled-in profile. Permissions decide which methods a macaroon may
call; protector caveats add a finer layer inside those methods.

Motivation: delegating channel management to a third party (a liquidity or
rebalancing service, an operator's own tooling, an LSP agent) without also
handing over the ability to redirect channel funds. Today a macaroon that can
call OpenChannel can also set push_sat or close_address, and one that can
call CloseChannel can set delivery_address, so "may manage channels"
implies "may move value to an arbitrary destination".

Design

  • The caveat carries only a profile name; the rules live in lnd code. That
    makes the guarantee upgradeable: a release can tighten a profile (for example
    when a new value-carrying field is added to a request) and every macaroon
    already issued benefits on upgrade.
  • A released profile name is frozen: its rules may only ever be tightened,
    never loosened. New semantics require a new name (channel-management-v2).
  • A macaroon naming a profile the validating node does not know is rejected as
    a whole, and an older lnd rejects the caveat as unrecognized. Both directions
    fail closed.
  • Added at bake time (lncli bakemacaroon --protector) or appended offline to
    an existing macaroon (lncli constrainmacaroon --protector).

First profile: channel-management-v1

Guarantee: the covered methods cannot redirect value to a third party.

Method Denied fields
OpenChannel, OpenChannelSync push_sat, close_address, funding_shim
BatchOpenChannel push_sat, close_address (per batched channel)
CloseChannel delivery_address
UpdateChannelPolicy none (reviewed, no redirection vector; the entry records the review and reserves the slot)

Note the open/close pairing: close_address sets the upfront shutdown script a
later cooperative close pays to, so denying delivery_address is only
meaningful if close_address is denied at open time too. Out of scope for v1:
fee-based value burn and peer restrictions.

Enforcement, and the two parts worth reviewer scrutiny

Enforcement is a dedicated pair of interceptors placed after (inside of) both
the macaroon and the middleware interceptors:

  1. After the middleware interceptors, because a registered RPC middleware
    may replace the request message. Enforcing at validation time would judge
    the original request while the handler executes the replacement. The
    protector stream wrapper wraps the middleware stream wrapper for the same
    reason.
  2. Outside the validator dispatch, so a registered ExternalValidator that
    knows nothing about protector caveats cannot be used to accept a macaroon
    and thereby skip the field rules. Because the credential is then resolved
    independently, ambiguity there fails closed: more than one macaroon
    metadata value, an unparseable macaroon, a malformed caveat or an unknown
    profile are all rejected.

OpenChannel and CloseChannel are server-streaming, so the request only
exists after stream open. Streams whose macaroon carries a covering profile get
RecvMsg wrapped, with profiles resolved once at wrap time; all other streams
pass through unwrapped, so there is no cost on normal traffic.

Rules fail closed by construction: a populated field not explicitly classified
as allowed, denied or nested is rejected, as is a request of the wrong proto
type or one that cannot be inspected on a covered method. Denials use
codes.PermissionDenied. A unit test additionally walks the proto descriptors
of every covered message and requires every field to be classified, so a field
added in a later release fails CI until someone vets it rather than silently
falling outside a deny-list.

Scoping

A profile constrains only the methods it lists; uncovered methods are
unaffected. Narrowing the callable set stays the job of permissions, which also
lets several profiles combine on one macaroon. In practice the caveat is meant
to be paired with uri: ops:

lncli bakemacaroon --protector channel-management-v1 --timeout 7776000 \
    uri:/lnrpc.Lightning/GetInfo uri:/lnrpc.Lightning/ListChannels \
    uri:/lnrpc.Lightning/ListPeers uri:/lnrpc.Lightning/ConnectPeer \
    uri:/lnrpc.Lightning/OpenChannel uri:/lnrpc.Lightning/OpenChannelSync \
    uri:/lnrpc.Lightning/BatchOpenChannel uri:/lnrpc.Lightning/CloseChannel \
    uri:/lnrpc.Lightning/UpdateChannelPolicy

With broad entity permissions instead, uncovered methods such as SendCoins
remain fully usable. This is stated in the flag help, docs/macaroons.md and
the release notes.

Compatibility

Additive: no proto or database changes, and no behavior change for macaroons
without a protector caveat. One narrowing worth flagging: on non-whitelisted
methods, a present macaroon metadata value must now contain exactly one hex
encoded, parseable macaroon. This closes a fail-open path where a lenient
external validator could accept a request whose credential enforcement could
not resolve. Documented on RegisterExternalValidator.

Follow-up, not in this PR: a listprotectors-style RPC so a profile's rules can
be read from a running node, since printmacaroon shows the name but not its
meaning.

Steps to Test

go test ./macaroons/ ./rpcperms/
go test -race ./macaroons/ ./rpcperms/
cd macaroons && go test -fuzz FuzzGetProtectorProfiles -fuzztime 60s -run '^$' .
make itest icase=protector_macaroon backend=btcd

The itest covers the deny matrix on unary and streaming methods, batch open,
unknown profiles, a control case with an unconstrained macaroon, a full
open/policy-update/coop-close lifecycle through a protected macaroon, and a
registered request-rewriting middleware asserting the replaced request is the
one judged.

Manually, against a regtest node with a funded wallet and a peer: bake as
above, then

# denied, with codes.PermissionDenied
lncli --macaroonpath=chanmgr.macaroon openchannel --node_key <peer> \
    --local_amt 5000000 --push_amt 2000000
lncli --macaroonpath=chanmgr.macaroon closechannel --funding_txid <txid> \
    --output_index 0 --delivery_addr <addr>

# allowed
lncli --macaroonpath=chanmgr.macaroon openchannel --node_key <peer> \
    --local_amt 5000000
lncli --macaroonpath=chanmgr.macaroon updatechanpolicy --base_fee_msat 1500 \
    --fee_rate_ppm 600 --time_lock_delta 80

I ran this on a local regtest, including a control showing the identical
--push_amt 2000000 request succeeds with an unconstrained macaroon and really
does move 2,000,000 sats to the peer, which is the transfer the caveat blocks.

Design questions for reviewers

  1. Uncovered methods. This PR says a profile has no opinion on methods it
    does not cover, leaving method scoping to uri: ops. The alternative is a
    default of "deny any write-capable method the profile has not vetted",
    derived from the permission map: stricter and self-defending against an
    over-broad bake, at the cost of coupling profiles to the entity/action model.
    Happy to switch if preferred.
  2. Caveat grammar. Name in the caveat with rules in code, versus encoding
    the field list in the caveat. I chose the former so profiles tighten on
    upgrade and printmacaroon output stays auditable, at the cost of holders
    not being able to invent their own field restrictions offline.

Pull Request Checklist

Testing

  • Your PR passes all CI checks.
  • Tests covering the positive and negative (error paths) are included.
  • Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

  • The change is not insubstantial.
  • The change obeys the Code Documentation and Commenting guidelines, and lines wrap at 80.
  • Commits follow the Ideal Git Commit Structure.
  • Any new logging statements use an appropriate subsystem and logging level.
  • Any new lncli commands have appropriate tags in the comments for the rpc in the proto file.
  • There is a change description in the release notes.

@GeorgeTsagk
macaroons: add protector caveat condition
006b8f2 
Add a new first-party caveat condition, "protector <profile-name>", that
references a named, compiled-in protector profile. A protector profile
restricts which request fields may be set on the RPC methods it covers,
allowing a macaroon to grant access to a method while denying specific
dangerous parameters, for example fields that could redirect channel
funds to a third party.

The checker added here is the bakery level half of the mechanism: it
verifies that the referenced profile name is well formed and known to
this lnd instance, so a macaroon referencing an unknown profile is
rejected as a whole. That makes future profile names, and older lnd
versions that know no profiles at all, fail closed rather than validate
without the intended restrictions. The field level enforcement itself
runs in the RPC interceptor chain, where the request message is
available.

Caveats are decoded with checkers.ParseCaveat, the same parser the
bakery uses for checking, so bake time encoding and enforcement time
decoding cannot drift apart. Caveats that are almost, but not exactly,
the canonical "protector <name>" encoding (wrong letter case, or a
separator other than the single space) fail closed instead of being
silently ignored, while genuinely different conditions that merely
share the prefix are left alone.

Fuzz targets assert that the parser never panics, never returns an
invalid profile name without an error and never silently drops a well
formed protector caveat.
@GeorgeTsagk
rpcperms: enforce protector caveats in the interceptor chain
bc2fb60 
Add a pair of interceptors that enforce the field rules of a macaroon's
protector caveats against the request message, and register the
protector checker with the macaroon service.

The interceptors are placed after (inside of) both the macaroon and the
middleware interceptors. After the macaroon interceptor because
enforcement assumes an already validated macaroon, and after the
middleware interceptors because a registered RPC middleware may replace
the request message: enforcing earlier would judge the original request
while the handler executes the replacement. For the same reason the
protector stream wrapper wraps the middleware stream wrapper, so
per-message enforcement sees the message after any replacement.

Enforcement deliberately does not run inside the validator dispatch of
checkMacaroon, so an external macaroon validator that knows nothing
about protector caveats cannot be used to bypass the field rules by
accepting the macaroon. As a consequence the credential is resolved
independently, and any ambiguity there fails closed: a request carrying
more than one macaroon metadata value, an unparseable macaroon, a
malformed protector caveat or an unknown profile name are all rejected.
This strict contract is documented on RegisterExternalValidator.

OpenChannel and CloseChannel are server streaming, so their request
message only exists after stream open. Streams whose macaroon carries a
protector caveat covering the called method get their RecvMsg wrapped;
all other streams are passed through unwrapped, and the profiles are
resolved once at wrap time, so there is no per-message macaroon parsing
and no cost at all on normal traffic.

Field rules fail closed by construction: a populated field that a rule
table does not explicitly classify as allowed, denied or nested is
rejected, a request whose proto type does not match the rule set is
rejected, and a covered method whose request cannot be inspected as a
proto message is rejected. Denials carry the PermissionDenied gRPC
status code. Methods not covered by any of the macaroon's profiles pass
without inspection, so externally registered services whose request
types lnd cannot inspect keep working.

This commit also adds the first profile, channel-management-v1: channel
management without the ability to redirect value to a third party. It
denies push_sat, close_address and funding_shim on OpenChannel,
OpenChannelSync and BatchOpenChannel (per batched channel), and
delivery_address on CloseChannel. UpdateChannelPolicy is covered with
all fields vetted as safe, which documents the review and reserves the
slot for future tightening.

Profile rules are code owned: rules under an existing profile name may
only ever be tightened, never loosened. A unit test walks the proto
descriptors of every covered message and requires every field to be
classified as exactly one of allowed, denied or nested, so a request
field added in a future release cannot ship unclassified. The same test
verifies that every rule references an existing field, catching typos
that would otherwise silently disable a denial, and that every method
URI is bound to the correct request message type.
@GeorgeTsagk
lncli: add --protector flag to bakemacaroon and constrainmacaroon
58392e8 
The flag can be specified multiple times and attaches one protector
caveat per given profile name. Like the other constraint flags it is
applied client side, so it also works for offline baking with
--root_key and for tightening existing macaroons through
constrainmacaroon.
@GeorgeTsagk
itest: add protector macaroon integration test
981e482 
Cover the protector caveat end to end against a real node: the deny
matrix for push_sat, close_address and delivery_address on both unary
and streaming methods, batch open enforcement inside the repeated
channels field, fail closed rejection of unknown profile names, a
control case proving the same request passes with an unconstrained
macaroon, and the positive path where a protected macaroon opens a
channel, updates the channel policy and cooperatively closes it.

The test also registers a real request rewriting RPC middleware and
asserts that the rewritten request is the one judged by the field
rules, which pins the interceptor ordering on the real server rather
than in a hand built chain.
@github-actions github-actions Bot added the severity-critical Requires expert review - security/consensus critical label Aug 24, 2026
@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown

🔴 PR Severity: CRITICAL

file classification | 13 files | 2642 lines changed

🟠 High (2 files)
  • macaroons/protector.go - new macaroon caveat protection logic (macaroons/*)
  • macaroons/service.go - macaroon service changes (macaroons/*)
🟡 Medium (4 files)
  • cmd/commands/cmd_macaroon.go - CLI command (cmd/*)
  • config_builder.go - core config wiring, uncategorized Go file
  • rpcperms/interceptor.go - RPC permission interceptor, uncategorized package
  • rpcperms/protector.go - new RPC permission protector, uncategorized package
🟢 Low (7 files)
  • docs/macaroons.md - documentation
  • docs/release-notes/release-notes-0.22.0.md - release notes
  • itest/list_on_test.go - test-only
  • itest/lnd_protector_macaroon_test.go - test-only
  • macaroons/fuzz_test.go - test-only
  • macaroons/protector_test.go - test-only
  • rpcperms/protector_test.go - test-only

Analysis

The highest individually-classified tier is HIGH, driven by macaroons/protector.go and macaroons/service.go (macaroons/* is an auth/security-sensitive package). However, the PR is bumped one level to CRITICAL because non-test, non-generated lines changed total 907 (well above the 500-line threshold): cmd_macaroon.go (35), config_builder.go (1), docs/macaroons.md (48), release notes (25), macaroons/protector.go (186), macaroons/service.go (6), rpcperms/interceptor.go (34), rpcperms/protector.go (572).

This PR introduces a substantial new macaroon/RPC-permission "protector" mechanism spanning both the macaroons and rpcperms packages, touching macaroon caveat validation and the RPC interceptor path. Given the security-sensitive nature (macaroon auth, RPC permission gating) and the size of the change, expert review is warranted.

To override, add a severity-override-{critical,high,medium,low} label.

@GeorgeTsagk

GeorgeTsagk commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator Author

Extra notes for reviewer context, from a security-focused pass over this
branch. It found no hole in the enforcement mechanism itself; two edges are
worth attention, both verified against the code:

  • The guarantee rests entirely on a correct bake and fails silently when the
    bake is wrong: broad entity permissions route around the profile
    (onchain:write reaches SendCoins), and a bake including
    macaroon:generate lets the holder mint itself a caveat-free macaroon, since
    BakeMacaroon neither checks that requested permissions are a subset of the
    caller's nor carries the caller's caveats over. Pre-existing lnd behavior,
    not introduced here, but a total bypass of the guarantee.

  • Coverage erodes as lnd adds methods rather than fields: the exhaustiveness
    test only forces classification of new fields on already covered messages,
    so I plan a test requiring any method whose request carries one of the denied
    field names to be either covered by the profile or explicitly recorded as
    reviewed.

@GeorgeTsagk
docs: document protector caveats
63253e0 
Add a section to the macaroon documentation describing the caveat, the
channel-management-v1 profile and the tighten-only versioning contract,
plus an explicit limitations subsection: only the methods a profile
covers are constrained (so the caveat must be paired with uri: scoped
permissions), a protector caveat must never be combined with
macaroon:generate because such a macaroon can bake itself a replacement
without the caveat, the guarantee is about redirection rather than value
loss in general (fee rate fields and force closes stay available), and
profiles place no restriction on counterparties.

The release notes entry explains the mechanism (name in the caveat,
rules in lnd, tighten-only, fail closed on unknown profiles), how to
attach the caveat, and the same limitations.
@bufo24

bufo24 commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

This is great 🙏, thanks for working on this. This PR should close my feature request here: #10336

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

severity-critical Requires expert review - security/consensus critical

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@GeorgeTsagk @bufo24