Merge pull request #11028 from ziggie1984/cltv-interceptor-deadline-range

htlcswitch: validate intercepted auto-fail height

docs/release-notes/release-notes-0.21.2.md

@@ -89,6 +89,10 @@
 
 ## Functional Updates
 
+* [The HTLC forward interceptor now validates](https://github.com/lightningnetwork/lnd/pull/11028)
+  that derived auto-fail heights are within the supported range before they are
+  exposed through the interceptor API.
+
 ## RPC Updates
 
 * `ForwardHtlcInterceptRequest.outgoing_requested_chan_id` now holds a reserved

htlcswitch/interceptable_switch.go

@@ -4,6 +4,7 @@ import (
 	"crypto/sha256"
 	"errors"
 	"fmt"
+	"math"
 	"sync"
 	"sync/atomic"
 
@@ -661,13 +662,35 @@ func (s *InterceptableSwitch) removeOnChainIntercept(key models.CircuitKey) {
 	}
 }
 
-// handleExpired checks that the htlc isn't too close to the channel
-// force-close broadcast height. If it is, it is cancelled back.
+// handleExpired checks that the htlc's expiry is within the range that can be
+// offered to the interceptor. Expiries near the channel force-close broadcast
+// height and expiries whose auto-fail height cannot be represented are failed
+// back.
 func (s *InterceptableSwitch) handleExpired(fwd *interceptedForward) (
 	bool, error) {
 
 	height := uint32(s.currentHeight)
-	if fwd.packet.incomingTimeout >= height+s.cltvInterceptDelta {
+	incomingTimeout := fwd.packet.incomingTimeout
+
+	// The interceptor auto-fail height is the incoming timeout less the
+	// reject delta and is exposed as an int32 block height. Calculate it in
+	// int64 so that we can check the representable range before conversion.
+	autoFailHeight := int64(incomingTimeout) - int64(s.cltvRejectDelta)
+	if autoFailHeight > math.MaxInt32 {
+		log.Debugf("Interception rejected because htlc expires too "+
+			"far in the future: circuit=%v, height=%v, "+
+			"incoming_timeout=%v", fwd.packet.inKey(), height,
+			incomingTimeout)
+
+		err := fwd.FailWithCode(lnwire.CodeExpiryTooFar)
+		if err != nil {
+			return false, err
+		}
+
+		return true, nil
+	}
+
+	if incomingTimeout >= height+s.cltvInterceptDelta {
 		return false, nil
 	}
 
@@ -675,7 +698,7 @@ func (s *InterceptableSwitch) handleExpired(fwd *interceptedForward) (
 		"expires too soon: circuit=%v, "+
 		"height=%v, incoming_timeout=%v",
 		fwd.packet.inKey(), height,
-		fwd.packet.incomingTimeout)
+		incomingTimeout)
 
 	err := fwd.FailWithCode(
 		lnwire.CodeExpiryTooSoon,
@@ -859,6 +882,9 @@ func (f *interceptedForward) FailWithCode(code lnwire.FailCode) error {
 
 		failureMsg = lnwire.NewExpiryTooSoon(*update)
 
+	case lnwire.CodeExpiryTooFar:
+		failureMsg = &lnwire.FailExpiryTooFar{}
+
 	default:
 		return ErrUnsupportedFailureCode
 	}

htlcswitch/switch_test.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"math"
 	mrand "math/rand"
 	"reflect"
 	"testing"
@@ -3888,15 +3889,19 @@ func assertOutgoingLinkReceive(t *testing.T, targetLink *mockChannelLink,
 }
 
 func assertOutgoingLinkReceiveIntercepted(t *testing.T,
-	targetLink *mockChannelLink) {
+	targetLink *mockChannelLink) *htlcPacket {
 
 	t.Helper()
 
 	select {
-	case <-targetLink.packets:
+	case packet := <-targetLink.packets:
+		return packet
+
 	case <-time.After(time.Second):
 		t.Fatal("request was not propagated to destination")
 	}
+
+	return nil
 }
 
 type interceptableSwitchTestContext struct {
@@ -4363,6 +4368,70 @@ func TestInterceptableSwitchWatchDog(t *testing.T) {
 	}))
 }
 
+// TestInterceptableSwitchExpiryTooFar asserts that an intercepted forward with
+// an incoming expiry outside the supported auto-fail height range is failed
+// back and that subsequent forwards can still be intercepted.
+func TestInterceptableSwitchExpiryTooFar(t *testing.T) {
+	t.Parallel()
+
+	c := newInterceptableSwitchTestContext(t)
+	defer c.finish()
+
+	notifier := &mock.ChainNotifier{
+		EpochChan: make(chan *chainntnfs.BlockEpoch, 1),
+	}
+	notifier.EpochChan <- &chainntnfs.BlockEpoch{Height: testStartingHeight}
+
+	switchForwardInterceptor, err := NewInterceptableSwitch(
+		&InterceptableSwitchConfig{
+			Switch:             c.s,
+			CltvRejectDelta:    c.cltvRejectDelta,
+			CltvInterceptDelta: c.cltvInterceptDelta,
+			Notifier:           notifier,
+		},
+	)
+	require.NoError(t, err)
+	require.NoError(t, switchForwardInterceptor.Start())
+
+	switchForwardInterceptor.SetInterceptor(
+		c.forwardInterceptor.InterceptForwardHtlc,
+	)
+	linkQuit := make(chan struct{})
+
+	packet := c.createTestPacket()
+	packet.incomingTimeout = math.MaxUint32
+
+	err = switchForwardInterceptor.ForwardPackets(linkQuit, false, packet)
+	require.NoError(t, err, "can't forward htlc packet")
+
+	// The forward is failed back rather than being intercepted or sent to
+	// the outgoing link.
+	assertOutgoingLinkReceive(t, c.bobChannelLink, false)
+	failPacket := assertOutgoingLinkReceiveIntercepted(
+		t, c.aliceChannelLink,
+	)
+	failHtlc, ok := failPacket.htlc.(*lnwire.UpdateFailHTLC)
+	require.True(t, ok)
+
+	fwdErr, err := newMockDeobfuscator().DecryptError(failHtlc.Reason)
+	require.NoError(t, err)
+	require.IsType(t, &lnwire.FailExpiryTooFar{}, fwdErr.WireMessage())
+	assertNumCircuits(t, c.s, 0, 0)
+
+	// A later forward with a representable auto-fail height is intercepted
+	// normally.
+	require.NoError(t, switchForwardInterceptor.ForwardPackets(
+		linkQuit, false, c.createTestPacket(),
+	))
+
+	intercepted := c.forwardInterceptor.getIntercepted()
+	require.Equal(t,
+		int32(testStartingHeight+c.cltvInterceptDelta+1-
+			c.cltvRejectDelta),
+		intercepted.AutoFailHeight(),
+	)
+}
+
 // TestSwitchDustForwarding tests that the switch properly fails HTLC's which
 // have incoming or outgoing links that breach their fee thresholds.
 func TestSwitchDustForwarding(t *testing.T) {