From 1d97cf1229fd85dc01d8a3a4aca3724d51949a21 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Mon, 15 Oct 2018 08:41:56 +0200 Subject: [PATCH] htlcswitch+routing: implement expiry_too_far failure In this commit we add a check to HtlcSatifiesPolicy to verify that the time lock for the outgoing htlc that is requested in the onion packet isn't too far in the future. Without this check, anyone could force an unreasonably long time lock on the forwarding node. --- htlcswitch/link.go | 19 +++++++++++ htlcswitch/link_test.go | 74 +++++++++++++++++++++++++++++++++++++++++ lnwire/onion_error.go | 28 ++++++++++++++++ routing/router.go | 15 +++++++++ 4 files changed, 136 insertions(+) diff --git a/htlcswitch/link.go b/htlcswitch/link.go index cf6c1a34..71563592 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -36,6 +36,17 @@ const ( // TODO(roasbeef): must be < default delta expiryGraceDelta = 2 + // maxCltvExpiry is the maximum outgoing time lock that the node accepts + // for forwarded payments. The value is relative to the current block + // height. The reason to have a maximum is to prevent funds getting + // locked up unreasonably long. Otherwise, an attacker willing to lock + // its own funds too, could force the funds of this node to be locked up + // for an indefinite (max int32) number of blocks. + // + // The value 5000 is based on the maximum number of hops (20), the + // default cltv delta (144) and some extra margin. + maxCltvExpiry = 5000 + // DefaultMinLinkFeeUpdateTimeout represents the minimum interval in // which a link should propose to update its commitment fee rate. DefaultMinLinkFeeUpdateTimeout = 10 * time.Minute @@ -1950,6 +1961,14 @@ func (l *channelLink) HtlcSatifiesPolicy(payHash [32]byte, return failure } + if outgoingTimeout-heightNow > maxCltvExpiry { + l.errorf("outgoing htlc(%x) has a time lock too far in the "+ + "future: got %v, but maximum is %v", payHash[:], + outgoingTimeout-heightNow, maxCltvExpiry) + + return &lnwire.FailExpiryTooFar{} + } + // Finally, we'll ensure that the time-lock on the outgoing HTLC meets // the following constraint: the incoming time-lock minus our time-lock // delta should equal the outgoing time lock. Otherwise, whether the diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 2fe1dabf..1fb4ccc3 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -5119,3 +5119,77 @@ func TestForwardingAsymmetricTimeLockPolicies(t *testing.T) { t.Fatalf("unable to send payment: %v", err) } } + +// TestHtlcSatisfyPolicy tests that a link is properly enforcing the HTLC +// forwarding policy. +func TestHtlcSatisfyPolicy(t *testing.T) { + + fetchLastChannelUpdate := func(lnwire.ShortChannelID) ( + *lnwire.ChannelUpdate, error) { + + return &lnwire.ChannelUpdate{}, nil + } + + link := channelLink{ + cfg: ChannelLinkConfig{ + FwrdingPolicy: ForwardingPolicy{ + TimeLockDelta: 20, + MinHTLC: 500, + BaseFee: 10, + }, + FetchLastChannelUpdate: fetchLastChannelUpdate, + }, + } + + var hash [32]byte + + t.Run("satisfied", func(t *testing.T) { + result := link.HtlcSatifiesPolicy(hash, 1500, 1000, + 200, 150, 0) + if result != nil { + t.Fatalf("expected policy to be satisfied") + } + }) + + t.Run("below minhtlc", func(t *testing.T) { + result := link.HtlcSatifiesPolicy(hash, 100, 50, + 200, 150, 0) + if _, ok := result.(*lnwire.FailAmountBelowMinimum); !ok { + t.Fatalf("expected FailAmountBelowMinimum failure code") + } + }) + + t.Run("insufficient fee", func(t *testing.T) { + result := link.HtlcSatifiesPolicy(hash, 1005, 1000, + 200, 150, 0) + if _, ok := result.(*lnwire.FailFeeInsufficient); !ok { + t.Fatalf("expected FailFeeInsufficient failure code") + } + }) + + t.Run("expiry too soon", func(t *testing.T) { + result := link.HtlcSatifiesPolicy(hash, 1500, 1000, + 200, 150, 190) + if _, ok := result.(*lnwire.FailExpiryTooSoon); !ok { + t.Fatalf("expected FailExpiryTooSoon failure code") + } + }) + + t.Run("incorrect cltv expiry", func(t *testing.T) { + result := link.HtlcSatifiesPolicy(hash, 1500, 1000, + 200, 190, 0) + if _, ok := result.(*lnwire.FailIncorrectCltvExpiry); !ok { + t.Fatalf("expected FailIncorrectCltvExpiry failure code") + } + + }) + + t.Run("cltv expiry too far in the future", func(t *testing.T) { + // Check that expiry isn't too far in the future. + result := link.HtlcSatifiesPolicy(hash, 1500, 1000, + 10200, 10100, 0) + if _, ok := result.(*lnwire.FailExpiryTooFar); !ok { + t.Fatalf("expected FailExpiryTooFar failure code") + } + }) +} diff --git a/lnwire/onion_error.go b/lnwire/onion_error.go index 43b3c766..94afeed7 100644 --- a/lnwire/onion_error.go +++ b/lnwire/onion_error.go @@ -77,6 +77,7 @@ const ( CodeFinalExpiryTooSoon FailCode = 17 CodeFinalIncorrectCltvExpiry FailCode = 18 CodeFinalIncorrectHtlcAmount FailCode = 19 + CodeExpiryTooFar FailCode = 21 ) // String returns the string representation of the failure code. @@ -145,6 +146,9 @@ func (c FailCode) String() string { case CodeFinalIncorrectHtlcAmount: return "FinalIncorrectHtlcAmount" + case CodeExpiryTooFar: + return "ExpiryTooFar" + default: return "" } @@ -1038,6 +1042,26 @@ func (f *FailFinalIncorrectHtlcAmount) Encode(w io.Writer, pver uint32) error { return writeElement(w, f.IncomingHTLCAmount) } +// FailExpiryTooFar is returned if the CLTV expiry in the HTLC is too far in the +// future. +// +// NOTE: May be returned by any node in the payment route. +type FailExpiryTooFar struct{} + +// Code returns the failure unique code. +// +// NOTE: Part of the FailureMessage interface. +func (f FailExpiryTooFar) Code() FailCode { + return CodeExpiryTooFar +} + +// Returns a human readable string describing the target FailureMessage. +// +// NOTE: Implements the error interface. +func (f FailExpiryTooFar) Error() string { + return f.Code().String() +} + // DecodeFailure decodes, validates, and parses the lnwire onion failure, for // the provided protocol version. func DecodeFailure(r io.Reader, pver uint32) (FailureMessage, error) { @@ -1199,6 +1223,10 @@ func makeEmptyOnionError(code FailCode) (FailureMessage, error) { case CodeFinalIncorrectHtlcAmount: return &FailFinalIncorrectHtlcAmount{}, nil + + case CodeExpiryTooFar: + return &FailExpiryTooFar{}, nil + default: return nil, errors.Errorf("unknown error code: %v", code) } diff --git a/routing/router.go b/routing/router.go index 5186476f..8662b774 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1957,6 +1957,21 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment, ) continue + // If we crafted a route that contains a too long time + // lock for an intermediate node, we'll prune the node. + // As there currently is no way of knowing that node's + // maximum acceptable cltv, we cannot take this + // constraint into account during routing. + // + // TODO(joostjager): Record the rejected cltv and use + // that as a hint during future path finding through + // that node. + case *lnwire.FailExpiryTooFar: + pruneVertexFailure( + paySession, route, errSource, false, + ) + continue + // If we get a permanent channel or node failure, then // we'll note this (exclude the vertex/edge), and // continue with the rest of the routes.