What the developer wrote
Authored by ziggie
witnessbeacon: avoid interceptor deadlock
Release the preimage beacon lock before invoking the on-chain
interceptor. The interceptor path can block on the htlcswitch event
loop, while resolution of another held on-chain HTLC can call back
into the beacon to add a preimage.
If interceptor delivery fails after the subscriber was registered,
cancel the subscription before returning the error.
On-chain held entries are replay handles for the interceptor while
contractcourt waits for a preimage or on-chain expiry. Once the resolver
tears down, keeping the handle until the refund timeout can replay a stale
HTLC to a reconnecting interceptor.
Thread a dedicated cleanup signal from the witness subscription cancel path
back through the interceptable switch event loop. The held set only removes
on-chain entries for that signal, leaving off-chain entries under the link
flow lifecycle.
What changed, and why it matters
This patch fixes a deadlock and a stale-data replay problem in LND's 'witness beacon,' the component that notifies external interceptors about HTLCs that might need a preimage. The deadlock occurred because a lock was held while calling into another subsystem that could call back into the same locked code. The stale-replay problem occurred because on-chain HTLC handles were not cleaned up when a subscription was canceled, so a reconnecting interceptor could be replayed an old, already-resolved HTLC. The patch also cancels the subscription if interceptor delivery fails. These are reliability/availability issues with a plausible, though not directly demonstrated, security angle: a stuck node or replayed HTLC could be abused in a Lightning payment-channel attack.
Review the new cancelInterceptor wiring in production deployments and ensure the interceptable switch's RemoveOnChainIntercept correctly synchronizes with the event loop. Monitor for any regressions in HTLC interception behavior. Consider whether the stale-replay scenario could be exploited as a payment-channel griefing vector and assess if additional mitigations are needed.
Security signals we found
Deadlock between preimage beacon mutex and htlcswitch interceptor event loop
Potential stale on-chain HTLC replay to reconnecting interceptor after resolver teardown
Missing cleanup of on-chain intercept handle on subscription cancellation or interceptor failure
Lock ordering/callback re-entrancy fix in a security-sensitive preimage-handling path
Evidence from the diff
The commit modifies witness_beacon.go and server.go. It adds a cancelInterceptor callback (RemoveOnChainIntercept from the interceptable switch) to preimageBeacon. SubscribeUpdates now releases the beacon’s mutex before invoking the interceptor, removing a deadlock where the interceptor blocks on the htlcswitch event loop while an on-chain resolution re-enters the beacon. If interceptor delivery errors, CancelSubscription is called to remove the subscriber and the on-chain intercept handle. CancelSubscription itself now also invokes cancelInterceptor, so on-chain held entries are removed when the witness subscription is torn down, preventing stale HTLC replay to a reconnecting interceptor. Tests are added for both the cancel path and the error-cancel path.
Changed components
witness_beacon.goserver.gointerceptableSwitchcontractcourt.WitnessSubscriptionhtlcswitch InterceptedForward/InterceptedPacketInspect captured patch +84 / −18
diff --git a/server.go b/server.go
index 6baeaea..bb6743e 100644
--- a/server.go
+++ b/server.go
@@ -935,6 +935,7 @@ func newServer(ctx context.Context, cfg *Config, listenAddrs []net.Addr,
s.witnessBeacon = newPreimageBeacon(
dbs.ChanStateDB.NewWitnessCache(),
s.interceptableSwitch.ForwardPacket,
+ s.interceptableSwitch.RemoveOnChainIntercept,
)
chanStatusMgrCfg := &netann.ChanStatusConfig{
diff --git a/witness_beacon.go b/witness_beacon.go
index 7226182..68c096a 100644
--- a/witness_beacon.go
+++ b/witness_beacon.go
@@ -45,15 +45,19 @@ type preimageBeacon struct {
subscribers map[uint64]*preimageSubscriber
interceptor func(htlcswitch.InterceptedForward) error
+
+ cancelInterceptor func(models.CircuitKey) error
}
func newPreimageBeacon(wCache witnessCache,
- interceptor func(htlcswitch.InterceptedForward) error) *preimageBeacon {
+ interceptor func(htlcswitch.InterceptedForward) error,
+ cancelInterceptor func(models.CircuitKey) error) *preimageBeacon {
return &preimageBeacon{
- wCache: wCache,
- interceptor: interceptor,
- subscribers: make(map[uint64]*preimageSubscriber),
+ wCache: wCache,
+ interceptor: interceptor,
+ cancelInterceptor: cancelInterceptor,
+ subscribers: make(map[uint64]*preimageSubscriber),
}
}
@@ -65,43 +69,50 @@ func (p *preimageBeacon) SubscribeUpdates(
nextHopOnionBlob []byte) (*contractcourt.WitnessSubscription, error) {
p.Lock()
- defer p.Unlock()
-
clientID := p.clientCounter
client := &preimageSubscriber{
updateChan: make(chan lntypes.Preimage, 10),
quit: make(chan struct{}),
}
- p.subscribers[p.clientCounter] = client
+ p.subscribers[clientID] = client
p.clientCounter++
+ p.Unlock()
srvrLog.Debugf("Creating new witness beacon subscriber, id=%v",
- p.clientCounter)
+ clientID)
+
+ inKey := models.CircuitKey{
+ ChanID: chanID,
+ HtlcID: htlc.HtlcIndex,
+ }
sub := &contractcourt.WitnessSubscription{
WitnessUpdates: client.updateChan,
CancelSubscription: func() {
p.Lock()
- defer p.Unlock()
delete(p.subscribers, clientID)
close(client.quit)
+ p.Unlock()
+
+ err := p.cancelInterceptor(inKey)
+ if err != nil {
+ srvrLog.Errorf("Cannot remove on-chain "+
+ "intercept %v: %v", inKey, err)
+ }
},
}
// Notify the htlc interceptor. There may be a client connected
// and willing to supply a preimage.
packet := &htlcswitch.InterceptedPacket{
- Hash: htlc.RHash,
- IncomingExpiry: htlc.RefundTimeout,
- IncomingAmount: htlc.Amt,
- IncomingCircuit: models.CircuitKey{
- ChanID: chanID,
- HtlcID: htlc.HtlcIndex,
- },
+ Hash: htlc.RHash,
+ IncomingExpiry: htlc.RefundTimeout,
+ IncomingAmount: htlc.Amt,
+ IncomingCircuit: inKey,
OutgoingChanID: payload.FwdInfo.NextHop,
OutgoingExpiry: payload.FwdInfo.OutgoingCLTV,
OutgoingAmount: payload.FwdInfo.AmountToForward,
@@ -120,6 +131,8 @@ func (p *preimageBeacon) SubscribeUpdates(
err := p.interceptor(fwd)
if err != nil {
+ sub.CancelSubscription()
+
return nil, err
}
diff --git a/witness_beacon_test.go b/witness_beacon_test.go
index d98c276..1edbada 100644
--- a/witness_beacon_test.go
+++ b/witness_beacon_test.go
@@ -1,9 +1,11 @@
package lnd
import (
+ "errors"
"testing"
"github.com/lightningnetwork/lnd/channeldb"
+ "github.com/lightningnetwork/lnd/graph/db/models"
"github.com/lightningnetwork/lnd/htlcswitch"
"github.com/lightningnetwork/lnd/htlcswitch/hop"
"github.com/lightningnetwork/lnd/lntypes"
@@ -20,9 +22,15 @@ func TestWitnessBeaconIntercept(t *testing.T) {
return nil
}
+ var canceledKey models.CircuitKey
+ cancelInterceptor := func(key models.CircuitKey) error {
+ canceledKey = key
+
+ return nil
+ }
p := newPreimageBeacon(
- &mockWitnessCache{}, interceptor,
+ &mockWitnessCache{}, interceptor, cancelInterceptor,
)
preimage := lntypes.Preimage{1, 2, 3}
@@ -37,12 +45,56 @@ func TestWitnessBeaconIntercept(t *testing.T) {
[]byte{2},
)
require.NoError(t, err)
- t.Cleanup(subscription.CancelSubscription)
require.NoError(t, interceptedFwd.Settle(preimage))
update := <-subscription.WitnessUpdates
require.Equal(t, preimage, update)
+
+ subscription.CancelSubscription()
+ require.Equal(t, interceptedFwd.Packet().IncomingCircuit, canceledKey)
+}
+
+// TestWitnessBeaconInterceptErrorCancels tests that a failed interceptor offer
+// tears down the witness subscription and on-chain intercept handle.
+func TestWitnessBeaconInterceptErrorCancels(t *testing.T) {
+ errInterceptor := errors.New("interceptor error")
+
+ interceptor := func(htlcswitch.InterceptedForward) error {
+ return errInterceptor
+ }
+
+ var canceledKey models.CircuitKey
+ cancelInterceptor := func(key models.CircuitKey) error {
+ canceledKey = key
+
+ return nil
+ }
+
+ p := newPreimageBeacon(
+ &mockWitnessCache{}, interceptor, cancelInterceptor,
+ )
+
+ chanID := lnwire.NewShortChanIDFromInt(1)
+ htlc := &channeldb.HTLC{
+ HtlcIndex: 2,
+ RHash: lntypes.Hash{3},
+ }
+
+ subscription, err := p.SubscribeUpdates(
+ chanID, htlc, &hop.Payload{}, []byte{2},
+ )
+ require.ErrorIs(t, err, errInterceptor)
+ require.Nil(t, subscription)
+
+ require.Equal(t, models.CircuitKey{
+ ChanID: chanID,
+ HtlcID: htlc.HtlcIndex,
+ }, canceledKey)
+
+ p.RLock()
+ require.Empty(t, p.subscribers)
+ p.RUnlock()
}
type mockWitnessCache struct {
Why this scored 57/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.