From 31b2c22cf55035cd66f8e0bf9c9d92d1f28fc915 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 11 Dec 2019 08:56:41 +0100 Subject: [PATCH] routing: check for invalid routes --- go.mod | 2 +- go.sum | 4 +-- routing/payment_lifecycle.go | 22 +++++++++++- routing/route/route.go | 9 +++++ routing/router_test.go | 68 ++++++++++++++++++++++++++++++++++++ 5 files changed, 101 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index e94a27ec..94787091 100644 --- a/go.mod +++ b/go.mod @@ -34,7 +34,7 @@ require ( github.com/juju/version v0.0.0-20180108022336-b64dbd566305 // indirect github.com/kkdai/bstream v0.0.0-20181106074824-b3251f7901ec github.com/lightninglabs/neutrino v0.11.0 - github.com/lightningnetwork/lightning-onion v0.0.0-20190909101754-850081b08b6a + github.com/lightningnetwork/lightning-onion v0.0.0-20191214001659-f34e9dc1651d github.com/lightningnetwork/lnd/cert v1.0.0 github.com/lightningnetwork/lnd/queue v1.0.2 github.com/lightningnetwork/lnd/ticker v1.0.0 diff --git a/go.sum b/go.sum index 26dd211a..c34fd1a1 100644 --- a/go.sum +++ b/go.sum @@ -132,8 +132,8 @@ github.com/lightninglabs/gozmq v0.0.0-20191113021534-d20a764486bf h1:HZKvJUHlcXI github.com/lightninglabs/gozmq v0.0.0-20191113021534-d20a764486bf/go.mod h1:vxmQPeIQxPf6Jf9rM8R+B4rKBqLA2AjttNxkFBL2Plk= github.com/lightninglabs/neutrino v0.11.0 h1:lPpYFCtsfJX2W5zI4pWycPmbbBdr7zU+BafYdLoD6k0= github.com/lightninglabs/neutrino v0.11.0/go.mod h1:CuhF0iuzg9Sp2HO6ZgXgayviFTn1QHdSTJlMncK80wg= -github.com/lightningnetwork/lightning-onion v0.0.0-20190909101754-850081b08b6a h1:GoWPN4i4jTKRxhVNh9a2vvBBO1Y2seiJB+SopUYoKyo= -github.com/lightningnetwork/lightning-onion v0.0.0-20190909101754-850081b08b6a/go.mod h1:rigfi6Af/KqsF7Za0hOgcyq2PNH4AN70AaMRxcJkff4= +github.com/lightningnetwork/lightning-onion v0.0.0-20191214001659-f34e9dc1651d h1:U50MHOOeL6gR3Ee/l0eMvZMpmRo+ydzmlQuIruCyCsA= +github.com/lightningnetwork/lightning-onion v0.0.0-20191214001659-f34e9dc1651d/go.mod h1:rigfi6Af/KqsF7Za0hOgcyq2PNH4AN70AaMRxcJkff4= github.com/ltcsuite/ltcd v0.0.0-20190101042124-f37f8bf35796 h1:sjOGyegMIhvgfq5oaue6Td+hxZuf3tDC8lAPrFldqFw= github.com/ltcsuite/ltcd v0.0.0-20190101042124-f37f8bf35796/go.mod h1:3p7ZTf9V1sNPI5H8P3NkTFF4LuwMdPl2DodF60qAKqY= github.com/ltcsuite/ltcutil v0.0.0-20181217130922-17f3b04680b6/go.mod h1:8Vg/LTOO0KYa/vlHWJ6XZAevPQThGH5sufO0Hrou/lA= diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 89cdef18..093ee8bc 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -74,7 +74,9 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { } } else { // If this was a resumed attempt, we must regenerate the - // circuit. + // circuit. We don't need to check for errors resulting + // from an invalid route, because the sphinx packet has + // been successfully generated before. _, c, err := generateSphinxPacket( &p.attempt.Route, p.payment.PaymentHash[:], p.attempt.SessionKey, @@ -281,6 +283,24 @@ func (p *paymentLifecycle) createNewPaymentAttempt() (lnwire.ShortChannelID, onionBlob, c, err := generateSphinxPacket( rt, p.payment.PaymentHash[:], sessionKey, ) + + // With SendToRoute, it can happen that the route exceeds protocol + // constraints. Mark the payment as failed with an internal error. + if err == route.ErrMaxRouteHopsExceeded || + err == sphinx.ErrMaxRoutingInfoSizeExceeded { + + log.Debugf("Invalid route provided for payment %x: %v", + p.payment.PaymentHash, err) + + controlErr := p.router.cfg.Control.Fail( + p.payment.PaymentHash, channeldb.FailureReasonError, + ) + if controlErr != nil { + return lnwire.ShortChannelID{}, nil, controlErr + } + } + + // In any case, don't continue if there is an error. if err != nil { return lnwire.ShortChannelID{}, nil, err } diff --git a/routing/route/route.go b/routing/route/route.go index 1444b9df..2a27e3b5 100644 --- a/routing/route/route.go +++ b/routing/route/route.go @@ -26,6 +26,10 @@ var ( // each route. ErrNoRouteHopsProvided = fmt.Errorf("empty route hops provided") + // ErrMaxRouteHopsExceeded is returned when a caller attempts to + // construct a new sphinx packet, but provides too many hops. + ErrMaxRouteHopsExceeded = fmt.Errorf("route has too many hops") + // ErrIntermediateMPPHop is returned when a hop tries to deliver an MPP // record to an intermediate hop, only final hops can receive MPP // records. @@ -267,6 +271,11 @@ func NewRouteFromHops(amtToSend lnwire.MilliSatoshi, timeLock uint32, func (r *Route) ToSphinxPath() (*sphinx.PaymentPath, error) { var path sphinx.PaymentPath + // Check maximum route length. + if len(r.Hops) > sphinx.NumMaxHops { + return nil, ErrMaxRouteHopsExceeded + } + // For each hop encoded within the route, we'll convert the hop struct // to an OnionHop with matching per-hop payload within the path as used // by the sphinx package. diff --git a/routing/router_test.go b/routing/router_test.go index 9ea9737c..e3ae0108 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -3339,6 +3339,74 @@ func TestSendToRouteStructuredError(t *testing.T) { } } +// TestSendToRouteMaxHops asserts that SendToRoute fails when using a route that +// exceeds the maximum number of hops. +func TestSendToRouteMaxHops(t *testing.T) { + t.Parallel() + + // Setup a two node network. + chanCapSat := btcutil.Amount(100000) + testChannels := []*testChannel{ + symmetricTestChannel("a", "b", chanCapSat, &testChannelPolicy{ + Expiry: 144, + FeeRate: 400, + MinHTLC: 1, + MaxHTLC: lnwire.NewMSatFromSatoshis(chanCapSat), + }, 1), + } + + testGraph, err := createTestGraphFromChannels(testChannels, "a") + if err != nil { + t.Fatalf("unable to create graph: %v", err) + } + defer testGraph.cleanUp() + + const startingBlockHeight = 101 + + ctx, cleanUp, err := createTestCtxFromGraphInstance( + startingBlockHeight, testGraph, + ) + if err != nil { + t.Fatalf("unable to create router: %v", err) + } + defer cleanUp() + + // Create a 30 hop route that exceeds the maximum hop limit. + const payAmt = lnwire.MilliSatoshi(10000) + hopA := ctx.aliases["a"] + hopB := ctx.aliases["b"] + + var hops []*route.Hop + for i := 0; i < 15; i++ { + hops = append(hops, &route.Hop{ + ChannelID: 1, + PubKeyBytes: hopB, + AmtToForward: payAmt, + LegacyPayload: true, + }) + + hops = append(hops, &route.Hop{ + ChannelID: 1, + PubKeyBytes: hopA, + AmtToForward: payAmt, + LegacyPayload: true, + }) + } + + rt, err := route.NewRouteFromHops(payAmt, 100, ctx.aliases["a"], hops) + if err != nil { + t.Fatalf("unable to create route: %v", err) + } + + // Send off the payment request to the router. We expect an error back + // indicating that the route is too long. + var payment lntypes.Hash + _, err = ctx.router.SendToRoute(payment, rt) + if err != route.ErrMaxRouteHopsExceeded { + t.Fatalf("expected ErrMaxRouteHopsExceeded, but got %v", err) + } +} + // TestBuildRoute tests whether correct routes are built. func TestBuildRoute(t *testing.T) { // Setup a three node network.