mirror of
https://github.com/lightninglabs/lightning-terminal.git
synced 2026-08-13 12:33:36 +02:00
accounts: add a configurable maximum account payment size
Add an opt-in cap on the total amount a single account payment may debit, including routing fees, enforced at the account interceptor for both SendPaymentV2 and SendToRouteV2. When the new accounts.max-payment-size-msat config option is set to a non-zero value, payments whose amount plus fees exceeds it are rejected with ErrPaymentExceedsMaxSize before the balance check and before any funds are reserved. For SendPaymentV2, the configured fee limit is included in the capped amount. For SendToRouteV2, the route's stated fee is included. This gives operators a guard rail against a compromised or misbehaving account macaroon draining its balance in a single large payment. The cap defaults to 0 (disabled), preserving existing behaviour, and is a first step towards the finer-grained per-account spending controls requested in the issue.
This commit is contained in:
parent
9907dabc9f
commit
ae74392433
6 changed files with 229 additions and 19 deletions
|
|
@ -3,6 +3,7 @@ package accounts
|
|||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"math/bits"
|
||||
|
||||
"github.com/btcsuite/btcd/btcutil"
|
||||
"github.com/btcsuite/btcd/chaincfg"
|
||||
|
|
@ -79,8 +80,10 @@ type AccountChecker struct {
|
|||
|
||||
// NewAccountChecker creates a new account checker that can keep track of all
|
||||
// account related requests, including invoices, payments and account balances.
|
||||
func NewAccountChecker(service Service,
|
||||
chainParams *chaincfg.Params) *AccountChecker {
|
||||
// If maxPaymentSize is greater than zero, payments whose amount exceeds it are
|
||||
// rejected.
|
||||
func NewAccountChecker(service Service, chainParams *chaincfg.Params,
|
||||
maxPaymentSize lnwire.MilliSatoshi) *AccountChecker {
|
||||
|
||||
// nolint:ll
|
||||
checkers := CheckerMap{
|
||||
|
|
@ -162,9 +165,9 @@ func NewAccountChecker(service Service,
|
|||
}
|
||||
|
||||
return checkSend(
|
||||
ctx, chainParams, service, r.Amt,
|
||||
r.AmtMsat, r.PaymentRequest,
|
||||
r.PaymentHash,
|
||||
ctx, chainParams, service,
|
||||
maxPaymentSize, r.Amt, r.AmtMsat,
|
||||
r.PaymentRequest, r.PaymentHash,
|
||||
&lnrpc.FeeLimit{
|
||||
Limit: &lnrpc.FeeLimit_FixedMsat{
|
||||
FixedMsat: feeLimitMsat,
|
||||
|
|
@ -196,7 +199,8 @@ func NewAccountChecker(service Service,
|
|||
r *routerrpc.SendToRouteRequest) error {
|
||||
|
||||
return checkSendToRoute(
|
||||
ctx, service, r.PaymentHash, r.Route,
|
||||
ctx, service, maxPaymentSize,
|
||||
r.PaymentHash, r.Route,
|
||||
)
|
||||
},
|
||||
sendToRouteHTLCResponseHandler(service),
|
||||
|
|
@ -465,11 +469,22 @@ func filterPayments(ctx context.Context,
|
|||
return filteredPayments, nil
|
||||
}
|
||||
|
||||
// checkMaxPaymentSize returns an error if a maximum account payment size is
|
||||
// configured (non-zero) and the given total payment amount exceeds it.
|
||||
func checkMaxPaymentSize(maxPaymentSize, amt lnwire.MilliSatoshi) error {
|
||||
if maxPaymentSize != 0 && amt > maxPaymentSize {
|
||||
return fmt.Errorf("%w: amount %v exceeds the maximum of %v",
|
||||
ErrPaymentExceedsMaxSize, amt, maxPaymentSize)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// checkSend checks if a payment can be initiated by making sure the account in
|
||||
// the context has enough balance to pay for it.
|
||||
func checkSend(ctx context.Context, chainParams *chaincfg.Params,
|
||||
service Service, amt, amtMsat int64, invoice string,
|
||||
paymentHash []byte, feeLimit *lnrpc.FeeLimit) error {
|
||||
service Service, maxPaymentSize lnwire.MilliSatoshi, amt, amtMsat int64,
|
||||
invoice string, paymentHash []byte, feeLimit *lnrpc.FeeLimit) error {
|
||||
|
||||
log, acct, reqID, err := requestScopedValuesFromCtx(ctx)
|
||||
if err != nil {
|
||||
|
|
@ -540,7 +555,19 @@ func checkSend(ctx context.Context, chainParams *chaincfg.Params,
|
|||
limit = &lnrpc.FeeLimit{}
|
||||
}
|
||||
fee := lnrpc.CalculateFeeLimit(limit, sendAmt)
|
||||
sendAmt += fee
|
||||
|
||||
// Add the fee explicitly and reject amounts that cannot be represented.
|
||||
total, carry := bits.Add64(uint64(sendAmt), uint64(fee), 0)
|
||||
if carry != 0 {
|
||||
return ErrAccBalanceInsufficient
|
||||
}
|
||||
sendAmt = lnwire.MilliSatoshi(total)
|
||||
|
||||
// Enforce the configured maximum payment size on the full amount that
|
||||
// may be debited from the account.
|
||||
if err := checkMaxPaymentSize(maxPaymentSize, sendAmt); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
err = service.CheckBalance(ctx, acct.ID, sendAmt)
|
||||
if err != nil {
|
||||
|
|
@ -607,7 +634,8 @@ func checkSendResponse(ctx context.Context, service Service,
|
|||
|
||||
// checkSendToRoute checks if a payment can be sent to the route by making sure
|
||||
// the account in the context has enough balance to pay for it.
|
||||
func checkSendToRoute(ctx context.Context, service Service, paymentHash []byte,
|
||||
func checkSendToRoute(ctx context.Context, service Service,
|
||||
maxPaymentSize lnwire.MilliSatoshi, paymentHash []byte,
|
||||
route *lnrpc.Route) error {
|
||||
|
||||
log, acct, reqID, err := requestScopedValuesFromCtx(ctx)
|
||||
|
|
@ -640,7 +668,19 @@ func checkSendToRoute(ctx context.Context, service Service, paymentHash []byte,
|
|||
if lnwire.MilliSatoshi(route.TotalFeesMsat) > fee {
|
||||
fee = lnwire.MilliSatoshi(route.TotalFeesMsat)
|
||||
}
|
||||
sendAmt += fee
|
||||
|
||||
// Add the fee explicitly and reject amounts that cannot be represented.
|
||||
total, carry := bits.Add64(uint64(sendAmt), uint64(fee), 0)
|
||||
if carry != 0 {
|
||||
return ErrAccBalanceInsufficient
|
||||
}
|
||||
sendAmt = lnwire.MilliSatoshi(total)
|
||||
|
||||
// Enforce the configured maximum payment size on the full amount that
|
||||
// may be debited from the account.
|
||||
if err := checkMaxPaymentSize(maxPaymentSize, sendAmt); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
err = service.CheckBalance(ctx, acct.ID, sendAmt)
|
||||
if err != nil {
|
||||
|
|
|
|||
|
|
@ -144,7 +144,7 @@ var _ Service = (*mockService)(nil)
|
|||
func TestAccountChecker(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
checker := NewAccountChecker(nil, nil)
|
||||
checker := NewAccountChecker(nil, nil, 0)
|
||||
for checkerName := range checker.checkers {
|
||||
t.Logf("Checker registered: %v", checkerName)
|
||||
}
|
||||
|
|
@ -442,7 +442,7 @@ func TestAccountCheckers(t *testing.T) {
|
|||
tt.Parallel()
|
||||
|
||||
service := newMockService()
|
||||
checkers := NewAccountChecker(service, chainParams)
|
||||
checkers := NewAccountChecker(service, chainParams, 0)
|
||||
acct := &OffChainBalanceAccount{
|
||||
ID: testID,
|
||||
Type: TypeInitialBalance,
|
||||
|
|
@ -923,3 +923,109 @@ func assertMessagesEqual(t *testing.T, expected, actual proto.Message) {
|
|||
|
||||
require.Equal(t, string(expectedJSON), string(actualJSON))
|
||||
}
|
||||
|
||||
// TestSendPaymentV2MaxPaymentSize tests that, when a maximum account payment
|
||||
// size is configured, the SendPaymentV2 checker rejects payments whose total
|
||||
// amount, including the fee limit, exceeds the cap (issue #583).
|
||||
func TestSendPaymentV2MaxPaymentSize(t *testing.T) {
|
||||
var (
|
||||
uri = "/routerrpc.Router/SendPaymentV2"
|
||||
ctx = context.Background()
|
||||
requestID uint64
|
||||
)
|
||||
|
||||
nextRequestID := func() uint64 {
|
||||
requestID++
|
||||
|
||||
return requestID
|
||||
}
|
||||
|
||||
lndMock := newMockLnd()
|
||||
routerMock := newMockRouter()
|
||||
errFunc := func(err error) {
|
||||
lndMock.mainErrChan <- err
|
||||
}
|
||||
clk := clock.NewTestClock(time.Now())
|
||||
store := NewTestDB(t, clk)
|
||||
service, err := NewService(store, errFunc, WithMaxPaymentSize(2000))
|
||||
require.NoError(t, err)
|
||||
|
||||
err = service.Start(ctx, lndMock, routerMock, chainParams)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() {
|
||||
_ = service.Stop()
|
||||
})
|
||||
|
||||
acct, err := service.NewAccount(
|
||||
ctx, 1_000_000, clk.Now().Add(time.Hour), "max",
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
ctxWithAcct := AddAccountToContext(ctx, acct)
|
||||
|
||||
// A payment whose total amount is exactly at the cap is allowed.
|
||||
ctx1 := AddRequestIDToContext(ctxWithAcct, nextRequestID())
|
||||
err = service.checkers.checkIncomingRequest(
|
||||
ctx1, uri, &routerrpc.SendPaymentRequest{
|
||||
AmtMsat: 2000,
|
||||
PaymentHash: testHash[:],
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
// A payment whose amount exceeds the cap is rejected.
|
||||
ctx2 := AddRequestIDToContext(ctxWithAcct, nextRequestID())
|
||||
err = service.checkers.checkIncomingRequest(
|
||||
ctx2, uri, &routerrpc.SendPaymentRequest{
|
||||
AmtMsat: 2001,
|
||||
PaymentHash: testHash2[:],
|
||||
},
|
||||
)
|
||||
require.ErrorIs(t, err, ErrPaymentExceedsMaxSize)
|
||||
|
||||
// A fee limit that brings an otherwise valid payment over the cap is
|
||||
// rejected as well.
|
||||
ctx3 := AddRequestIDToContext(ctxWithAcct, nextRequestID())
|
||||
err = service.checkers.checkIncomingRequest(
|
||||
ctx3, uri, &routerrpc.SendPaymentRequest{
|
||||
AmtMsat: 1900,
|
||||
FeeLimitMsat: 101,
|
||||
PaymentHash: testHash3[:],
|
||||
},
|
||||
)
|
||||
require.ErrorIs(t, err, ErrPaymentExceedsMaxSize)
|
||||
}
|
||||
|
||||
func TestSendToRouteV2MaxPaymentSize(t *testing.T) {
|
||||
const uri = "/routerrpc.Router/SendToRouteV2"
|
||||
|
||||
ctx := context.Background()
|
||||
lndMock := newMockLnd()
|
||||
routerMock := newMockRouter()
|
||||
service, err := NewService(
|
||||
NewTestDB(t, clock.NewTestClock(time.Now())),
|
||||
func(err error) { lndMock.mainErrChan <- err },
|
||||
WithMaxPaymentSize(2000),
|
||||
)
|
||||
require.NoError(t, err)
|
||||
err = service.Start(ctx, lndMock, routerMock, chainParams)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = service.Stop() })
|
||||
|
||||
acct, err := service.NewAccount(
|
||||
ctx, 1_000_000, time.Now().Add(time.Hour), "max",
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
ctx = AddRequestIDToContext(AddAccountToContext(ctx, acct), 1)
|
||||
err = service.checkers.checkIncomingRequest(
|
||||
ctx, uri, &routerrpc.SendToRouteRequest{
|
||||
PaymentHash: testHash[:],
|
||||
Route: &lnrpc.Route{
|
||||
TotalAmtMsat: 1900,
|
||||
TotalFeesMsat: 101,
|
||||
},
|
||||
},
|
||||
)
|
||||
require.ErrorIs(t, err, ErrPaymentExceedsMaxSize)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -194,6 +194,11 @@ var (
|
|||
// account
|
||||
ErrAccBalanceInsufficient = errors.New("account balance insufficient")
|
||||
|
||||
// ErrPaymentExceedsMaxSize is returned when a maximum account payment
|
||||
// size is configured and a payment's total amount exceeds it.
|
||||
ErrPaymentExceedsMaxSize = errors.New("payment amount exceeds the " +
|
||||
"maximum allowed account payment size")
|
||||
|
||||
// ErrNotSupportedWithAccounts is the error that is returned when an RPC
|
||||
// is called that isn't supported to be handled by the account
|
||||
// interceptor.
|
||||
|
|
|
|||
|
|
@ -19,9 +19,21 @@ import (
|
|||
)
|
||||
|
||||
// Config holds the configuration options for the accounts service.
|
||||
//
|
||||
//nolint:ll
|
||||
type Config struct {
|
||||
// Disable will disable the accounts service if set.
|
||||
Disable bool `long:"disable" description:"disable the accounts service"`
|
||||
|
||||
// MaxPaymentSizeMsat, when greater than zero, is the maximum value (in
|
||||
// millisatoshis) that a single account payment may have. Payments whose
|
||||
// amount exceeds this cap are rejected by the account interceptor. This
|
||||
// provides a guard rail against a compromised or misbehaving account
|
||||
// macaroon draining its balance in a single large payment.
|
||||
//
|
||||
// It defaults to 0, which disables the cap and preserves the historical
|
||||
// behaviour of allowing payments up to the full account balance.
|
||||
MaxPaymentSizeMsat uint64 `long:"max-payment-size-msat" description:"the maximum total amount in millisatoshis, including fees, that a single account payment may debit; 0 (the default value) disables the cap"`
|
||||
}
|
||||
|
||||
// trackedPayment is a struct that holds all information that identifies a
|
||||
|
|
@ -56,6 +68,10 @@ type InterceptorService struct {
|
|||
|
||||
routerClient lndclient.RouterClient
|
||||
|
||||
// maxPaymentSize, when greater than zero, is the maximum value that a
|
||||
// single account payment may have. See Config.MaxPaymentSizeMsat.
|
||||
maxPaymentSize lnwire.MilliSatoshi
|
||||
|
||||
mainCtx context.Context
|
||||
contextCancel fn.Option[context.CancelFunc]
|
||||
|
||||
|
|
@ -77,12 +93,24 @@ type InterceptorService struct {
|
|||
isEnabled bool
|
||||
}
|
||||
|
||||
// ServiceOption is a functional option that can be used to modify the behaviour
|
||||
// of the InterceptorService.
|
||||
type ServiceOption func(*InterceptorService)
|
||||
|
||||
// WithMaxPaymentSize sets the maximum value that a single account payment may
|
||||
// have. A value of zero (the default) disables the cap.
|
||||
func WithMaxPaymentSize(maxPaymentSize lnwire.MilliSatoshi) ServiceOption {
|
||||
return func(s *InterceptorService) {
|
||||
s.maxPaymentSize = maxPaymentSize
|
||||
}
|
||||
}
|
||||
|
||||
// NewService returns a service backed by the macaroon Bolt DB stored in the
|
||||
// passed-in directory.
|
||||
func NewService(store Store, errCallback func(error)) (*InterceptorService,
|
||||
error) {
|
||||
func NewService(store Store, errCallback func(error),
|
||||
opts ...ServiceOption) (*InterceptorService, error) {
|
||||
|
||||
return &InterceptorService{
|
||||
s := &InterceptorService{
|
||||
store: store,
|
||||
invoiceToAccount: make(map[lntypes.Hash]AccountID),
|
||||
pendingPayments: make(map[lntypes.Hash]*trackedPayment),
|
||||
|
|
@ -90,7 +118,13 @@ func NewService(store Store, errCallback func(error)) (*InterceptorService,
|
|||
mainErrCallback: errCallback,
|
||||
quit: make(chan struct{}),
|
||||
isEnabled: false,
|
||||
}, nil
|
||||
}
|
||||
|
||||
for _, opt := range opts {
|
||||
opt(s)
|
||||
}
|
||||
|
||||
return s, nil
|
||||
}
|
||||
|
||||
// Start starts the account service and its interceptor capability.
|
||||
|
|
@ -103,7 +137,7 @@ func (s *InterceptorService) Start(ctx context.Context,
|
|||
s.contextCancel = fn.Some(contextCancel)
|
||||
|
||||
s.routerClient = routerClient
|
||||
s.checkers = NewAccountChecker(s, params)
|
||||
s.checkers = NewAccountChecker(s, params, s.maxPaymentSize)
|
||||
|
||||
s.isEnabled = true
|
||||
|
||||
|
|
|
|||
|
|
@ -59,6 +59,20 @@
|
|||
sub-servers on startup. If the macaroon already exists but has different
|
||||
permissions, it will be automatically regenerated.
|
||||
|
||||
* [Add a configurable maximum account payment
|
||||
size](https://github.com/lightninglabs/lightning-terminal/pull/1369):
|
||||
Addresses [
|
||||
#583](https://github.com/lightninglabs/lightning-terminal/issues/583).
|
||||
Added an `accounts.max-payment-size-msat` config option. When set to a
|
||||
non-zero value, the account interceptor rejects any single account payment
|
||||
(`SendPaymentV2`/`SendToRouteV2`) whose total amount, including fees,
|
||||
exceeds the configured cap,
|
||||
providing a guard rail against a compromised or misbehaving account macaroon
|
||||
draining its balance in one large payment. It defaults to 0 (no cap),
|
||||
preserving existing behaviour. This is a first step towards the finer-grained
|
||||
per-account spending controls tracked in the issue (e.g. per-interval spend
|
||||
limits).
|
||||
|
||||
### Technical and Architectural Updates
|
||||
|
||||
* [Report litd's own version for `litd
|
||||
|
|
|
|||
13
terminal.go
13
terminal.go
|
|
@ -58,6 +58,7 @@ import (
|
|||
"github.com/lightningnetwork/lnd/lnwallet"
|
||||
"github.com/lightningnetwork/lnd/lnwallet/btcwallet"
|
||||
"github.com/lightningnetwork/lnd/lnwallet/chancloser"
|
||||
"github.com/lightningnetwork/lnd/lnwire"
|
||||
"github.com/lightningnetwork/lnd/macaroons"
|
||||
"github.com/lightningnetwork/lnd/msgmux"
|
||||
"github.com/lightningnetwork/lnd/rpcperms"
|
||||
|
|
@ -782,8 +783,18 @@ func (g *LightningTerminal) start(ctx context.Context) error {
|
|||
return fmt.Errorf("could not start firewall DB: %v", err)
|
||||
}
|
||||
|
||||
var accountsOpts []accounts.ServiceOption
|
||||
if g.cfg.Accounts.MaxPaymentSizeMsat > 0 {
|
||||
accountsOpts = append(
|
||||
accountsOpts, accounts.WithMaxPaymentSize(
|
||||
lnwire.MilliSatoshi(
|
||||
g.cfg.Accounts.MaxPaymentSizeMsat,
|
||||
),
|
||||
),
|
||||
)
|
||||
}
|
||||
g.accountService, err = accounts.NewService(
|
||||
g.stores.accounts, accountServiceErrCallback,
|
||||
g.stores.accounts, accountServiceErrCallback, accountsOpts...,
|
||||
)
|
||||
if err != nil {
|
||||
return fmt.Errorf("error creating account service: %v", err)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue