From 39a59bbe6f38594cbaa8ecff48a49cfb0659b8cb Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Fri, 14 Jul 2017 21:32:00 +0200 Subject: [PATCH] routing: Require adding edge to node before adding node. This commit introduces the requirement specified in BOLT#7, where we ignore any node announcements for a specific node if we yet haven't seen any channel announcements where this node takes part. This is to prevent someone DoS-ing the network with cheap node announcements. In the router this is enforced by requiring a call to AddNode(node_id) to be preceded by an AddEdge(edge_id) call, where node_id is one of the nodes in edge_id. --- routing/errors.go | 3 +- routing/notifications_test.go | 141 ++++++++++++----- routing/pathfind_test.go | 13 +- routing/router.go | 58 ++++--- routing/router_test.go | 286 ++++++++++++++++++++++++++++++++-- 5 files changed, 423 insertions(+), 78 deletions(-) diff --git a/routing/errors.go b/routing/errors.go index 1a122c06..cdd94c4e 100644 --- a/routing/errors.go +++ b/routing/errors.go @@ -35,7 +35,8 @@ const ( ErrOutdated // ErrIgnored is returned when the update have been ignored because - // this update can't bring us something new. + // this update can't bring us something new, or because a node + // announcement was given for node not found in any channel. ErrIgnored ) diff --git a/routing/notifications_test.go b/routing/notifications_test.go index 4feddb5e..73f52088 100644 --- a/routing/notifications_test.go +++ b/routing/notifications_test.go @@ -52,13 +52,14 @@ func createTestNode() (*channeldb.LightningNode, error) { pub := priv.PubKey().SerializeCompressed() return &channeldb.LightningNode{ - LastUpdate: time.Unix(updateTime, 0), - Addresses: testAddrs, - PubKey: priv.PubKey(), - Color: color.RGBA{1, 2, 3, 0}, - Alias: "kek" + string(pub[:]), - AuthSig: testSig, - Features: testFeatures, + HaveNodeAnnouncement: true, + LastUpdate: time.Unix(updateTime, 0), + Addresses: testAddrs, + PubKey: priv.PubKey(), + Color: color.RGBA{1, 2, 3, 0}, + Alias: "kek" + string(pub[:]), + AuthSig: testSig, + Features: testFeatures, }, nil } @@ -297,7 +298,7 @@ func TestEdgeUpdateNotification(t *testing.T) { ctx.chain.addBlock(fundingBlock, chanID.BlockHeight) // Next we'll create two test nodes that the fake channel will be open - // between and add then as members of the channel graph. + // between. node1, err := createTestNode() if err != nil { t.Fatalf("unable to create test node: %v", err) @@ -307,15 +308,6 @@ func TestEdgeUpdateNotification(t *testing.T) { t.Fatalf("unable to create test node: %v", err) } - // Send the two node topology updates to the channel router so they - // can be validated and stored within the graph database. - if err := ctx.router.AddNode(node1); err != nil { - t.Fatal(err) - } - if err := ctx.router.AddNode(node2); err != nil { - t.Fatal(err) - } - // Finally, to conclude our test set up, we'll create a channel // update to announce the created channel between the two nodes. edge := &channeldb.ChannelEdgeInfo{ @@ -462,20 +454,34 @@ func TestEdgeUpdateNotification(t *testing.T) { func TestNodeUpdateNotification(t *testing.T) { t.Parallel() - ctx, cleanUp, err := createTestCtx(1) + const startingBlockHeight = 101 + ctx, cleanUp, err := createTestCtx(startingBlockHeight) defer cleanUp() if err != nil { t.Fatalf("unable to create router: %v", err) } - // Create a new client to receive notifications. - ntfnClient, err := ctx.router.SubscribeTopology() + // We only accept node announcements from nodes having a known channel, + // so create one now. + const chanValue = 10000 + fundingTx, _, chanID, err := createChannelEdge(ctx, + bitcoinKey1.SerializeCompressed(), + bitcoinKey2.SerializeCompressed(), + chanValue, startingBlockHeight) if err != nil { - t.Fatalf("unable to subscribe for channel notifications: %v", err) + t.Fatalf("unable create channel edge: %v", err) } - // Create two random nodes to add to send as node announcement messages - // to trigger notifications. + // We'll also add a record for the block that included our funding + // transaction. + fundingBlock := &wire.MsgBlock{ + Transactions: []*wire.MsgTx{fundingTx}, + } + ctx.chain.addBlock(fundingBlock, chanID.BlockHeight) + + // Create two nodes acting as endpoints in the created channel, and use + // them to trigger notifications by sending updated node announcement + // messages. node1, err := createTestNode() if err != nil { t.Fatalf("unable to create test node: %v", err) @@ -485,7 +491,34 @@ func TestNodeUpdateNotification(t *testing.T) { t.Fatalf("unable to create test node: %v", err) } - // Change network topology by adding nodes to the channel router. + edge := &channeldb.ChannelEdgeInfo{ + ChannelID: chanID.ToUint64(), + NodeKey1: node1.PubKey, + NodeKey2: node2.PubKey, + BitcoinKey1: bitcoinKey1, + BitcoinKey2: bitcoinKey2, + AuthProof: &channeldb.ChannelAuthProof{ + NodeSig1: testSig, + NodeSig2: testSig, + BitcoinSig1: testSig, + BitcoinSig2: testSig, + }, + } + + // Adding the edge will add the nodes to the graph, but with no info + // except the pubkey known. + if err := ctx.router.AddEdge(edge); err != nil { + t.Fatalf("unable to add edge: %v", err) + } + + // Create a new client to receive notifications. + ntfnClient, err := ctx.router.SubscribeTopology() + if err != nil { + t.Fatalf("unable to subscribe for channel notifications: %v", err) + } + + // Change network topology by adding the updated info for the two nodes + // to the channel router. if err := ctx.router.AddNode(node1); err != nil { t.Fatalf("unable to add node: %v", err) } @@ -610,25 +643,67 @@ func TestNotificationCancellation(t *testing.T) { t.Fatalf("unable to subscribe for channel notifications: %v", err) } + // We'll create the utxo for a new channel. + const chanValue = 10000 + fundingTx, _, chanID, err := createChannelEdge(ctx, + bitcoinKey1.SerializeCompressed(), + bitcoinKey2.SerializeCompressed(), + chanValue, startingBlockHeight) + if err != nil { + t.Fatalf("unable create channel edge: %v", err) + } + + // We'll also add a record for the block that included our funding + // transaction. + fundingBlock := &wire.MsgBlock{ + Transactions: []*wire.MsgTx{fundingTx}, + } + ctx.chain.addBlock(fundingBlock, chanID.BlockHeight) + // We'll create a fresh new node topology update to feed to the channel // router. - node, err := createTestNode() + node1, err := createTestNode() + if err != nil { + t.Fatalf("unable to create test node: %v", err) + } + node2, err := createTestNode() if err != nil { t.Fatalf("unable to create test node: %v", err) } // Before we send the message to the channel router, we'll cancel the // notifications for this client. As a result, the notification - // triggered by accepting this announcement shouldn't be sent to the - // client. + // triggered by accepting the channel announcements shouldn't be sent + // to the client. ntfnClient.Cancel() - if err := ctx.router.AddNode(node); err != nil { + edge := &channeldb.ChannelEdgeInfo{ + ChannelID: chanID.ToUint64(), + NodeKey1: node1.PubKey, + NodeKey2: node2.PubKey, + BitcoinKey1: bitcoinKey1, + BitcoinKey2: bitcoinKey2, + AuthProof: &channeldb.ChannelAuthProof{ + NodeSig1: testSig, + NodeSig2: testSig, + BitcoinSig1: testSig, + BitcoinSig2: testSig, + }, + } + if err := ctx.router.AddEdge(edge); err != nil { + t.Fatalf("unable to add edge: %v", err) + } + + if err := ctx.router.AddNode(node1); err != nil { + t.Fatalf("unable to add node: %v", err) + } + + if err := ctx.router.AddNode(node2); err != nil { t.Fatalf("unable to add node: %v", err) } select { - // The notification shouldn't be sent, however, the channel should be + // The notifications shouldn't be sent, however, the channel should be // closed, causing the second read-value to be false. case _, ok := <-ntfnClient.TopologyChanges: if !ok { @@ -671,21 +746,15 @@ func TestChannelCloseNotification(t *testing.T) { ctx.chain.addBlock(fundingBlock, chanID.BlockHeight) // Next we'll create two test nodes that the fake channel will be open - // between and add then as members of the channel graph. + // between. node1, err := createTestNode() if err != nil { t.Fatalf("unable to create test node: %v", err) } - if err := ctx.router.AddNode(node1); err != nil { - t.Fatal(err) - } node2, err := createTestNode() if err != nil { t.Fatalf("unable to create test node: %v", err) } - if err := ctx.router.AddNode(node2); err != nil { - t.Fatal(err) - } // Finally, to conclude our test set up, we'll create a channel // announcement to announce the created channel between the two nodes. diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index 2c212d15..7c5de8b0 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -165,12 +165,13 @@ func parseTestGraph(path string) (*channeldb.ChannelGraph, func(), aliasMap, err } dbNode := &channeldb.LightningNode{ - AuthSig: testSig, - LastUpdate: time.Now(), - Addresses: testAddrs, - PubKey: pub, - Alias: node.Alias, - Features: testFeatures, + HaveNodeAnnouncement: true, + AuthSig: testSig, + LastUpdate: time.Now(), + Addresses: testAddrs, + PubKey: pub, + Alias: node.Alias, + Features: testFeatures, } // We require all aliases within the graph to be unique for our diff --git a/routing/router.go b/routing/router.go index 01f07452..637d5ec6 100644 --- a/routing/router.go +++ b/routing/router.go @@ -28,8 +28,9 @@ import ( // and applying edges updates, return the current block with with out // topology is synchronized. type ChannelGraphSource interface { - // AddNode is used to add node to the topology of the router, after - // this node might be used in construction of payment path. + // AddNode is used to add information about a node to the router + // database. If the node with this pubkey is not present in an existing + // channel, it will be ignored. AddNode(node *channeldb.LightningNode) error // AddEdge is used to add edge/channel to the topology of the router, @@ -524,17 +525,23 @@ func (r *ChannelRouter) processUpdate(msg interface{}) error { switch msg := msg.(type) { case *channeldb.LightningNode: - // Before proceeding ensure that we aren't already away of this - // node, and if we are then this is a newer update that we - // known of. + // If we are not already aware of this node, it means that we + // don't know about any channel using this node. To avoid a DoS + // attack by node announcements, we will ignore such nodes. If + // we do know about this node, check that this update brings + // info newer than what we already have. lastUpdate, exists, err := r.cfg.Graph.HasLightningNode(msg.PubKey) if err != nil { return errors.Errorf("unable to query for the "+ "existence of node: %v", err) - + } + if !exists { + return newErrf(ErrIgnored, "Ignoring node announcement"+ + " for node not found in channel graph (%x)", + msg.PubKey.SerializeCompressed()) } - // If we've reached this pint then we're aware of th vertex + // If we've reached this point then we're aware of the vertex // being advertised. So we now check if the new message has a // new time stamp, if not then we won't accept the new data as // it would override newer data. @@ -565,20 +572,34 @@ func (r *ChannelRouter) processUpdate(msg interface{}) error { "chan_id=%v", msg.ChannelID) } - // If we don't yet know about this edge, then we'll do an - // additional check to ensure that we have information about - // the two nodes that this edge connects. + // Query the database for the existence of the two nodes in this + // channel. If not found, add a partial node to the database, + // containing only the node keys. _, exists, _ = r.cfg.Graph.HasLightningNode(msg.NodeKey1) if !exists { - return errors.Errorf("unable to add channel edge, info "+ - "for node %x is missing", - msg.NodeKey1.SerializeCompressed()) + node1 := &channeldb.LightningNode{ + PubKey: msg.NodeKey1, + HaveNodeAnnouncement: false, + } + err := r.cfg.Graph.AddLightningNode(node1) + if err != nil { + return errors.Errorf("unable to add node %v to"+ + " the graph: %v", + node1.PubKey.SerializeCompressed(), err) + } } _, exists, _ = r.cfg.Graph.HasLightningNode(msg.NodeKey2) if !exists { - return errors.Errorf("unable to add channel edge, info "+ - "for node %x is missing", - msg.NodeKey1.SerializeCompressed()) + node2 := &channeldb.LightningNode{ + PubKey: msg.NodeKey2, + HaveNodeAnnouncement: false, + } + err := r.cfg.Graph.AddLightningNode(node2) + if err != nil { + return errors.Errorf("unable to add node %v to"+ + " the graph: %v", + node2.PubKey.SerializeCompressed(), err) + } } // Before we can add the channel to the channel graph, we need @@ -1031,8 +1052,9 @@ func (r *ChannelRouter) SendPayment(payment *LightningPayment) ([32]byte, *Route return [32]byte{}, nil, sendError } -// AddNode is used to add node to the topology of the router, after this node -// might be used in construction of payment path. +// AddNode is used to add information about a node to the router database. If +// the node with this pubkey is not present in an existing channel, it will +// be ignored. // // NOTE: This method is part of the ChannelGraphSource interface. func (r *ChannelRouter) AddNode(node *channeldb.LightningNode) error { diff --git a/routing/router_test.go b/routing/router_test.go index c47a5a5d..662ba726 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -4,7 +4,9 @@ import ( "bytes" "errors" "fmt" + "image/color" "testing" + "time" "github.com/lightningnetwork/lnd/channeldb" "github.com/roasbeef/btcd/wire" @@ -229,16 +231,10 @@ func TestAddProof(t *testing.T) { if err != nil { t.Fatal(err) } - if err := ctx.router.AddNode(node1); err != nil { - t.Fatal(err) - } node2, err := createTestNode() if err != nil { t.Fatal(err) } - if err := ctx.router.AddNode(node2); err != nil { - t.Fatal(err) - } // In order to be able to add the edge we should have a valid funding // UTXO within the blockchain. @@ -279,34 +275,290 @@ func TestAddProof(t *testing.T) { } } +// TestIgnoreNodeAnnouncement tests that adding a node to the router that is +// not known from any channel annoucement, leads to the annoucement being +// ignored. +func TestIgnoreNodeAnnouncement(t *testing.T) { + t.Parallel() + + const startingBlockHeight = 101 + ctx, cleanUp, err := createTestCtx(startingBlockHeight, + basicGraphFilePath) + defer cleanUp() + if err != nil { + t.Fatalf("unable to create router: %v", err) + } + + node := &channeldb.LightningNode{ + HaveNodeAnnouncement: true, + LastUpdate: time.Unix(123, 0), + Addresses: testAddrs, + PubKey: priv1.PubKey(), + Color: color.RGBA{1, 2, 3, 0}, + Alias: "node11", + AuthSig: testSig, + Features: testFeatures, + } + + err = ctx.router.AddNode(node) + if !IsError(err, ErrIgnored) { + t.Fatalf("expected to get ErrIgnore, instead got: %v", err) + } +} + // TestAddEdgeUnknownVertexes tests that if an edge is added that contains two -// vertex which we don't know of, then the edge is rejected. +// vertexes which we don't know of, the edge should be available for use +// regardless. This is due to the fact that we don't actually need node +// announcements for the channel vertexes to be able to use the channel. func TestAddEdgeUnknownVertexes(t *testing.T) { t.Parallel() - ctx, cleanup, err := createTestCtx(0) + const startingBlockHeight = 101 + ctx, cleanUp, err := createTestCtx(startingBlockHeight, + basicGraphFilePath) + defer cleanUp() if err != nil { - t.Fatal(err) + t.Fatalf("unable to create router: %v", err) } - defer cleanup() - _, _, chanID, err := createChannelEdge(ctx, - bitcoinKey1.SerializeCompressed(), bitcoinKey2.SerializeCompressed(), - 10000, 500) + // The two nodes we are about to add should not exist yet. + _, exists1, err := ctx.graph.HasLightningNode(priv1.PubKey()) + if err != nil { + t.Fatalf("unable to query graph: %v", err) + } + if exists1 { + t.Fatalf("node already existed") + } + _, exists2, err := ctx.graph.HasLightningNode(priv2.PubKey()) + if err != nil { + t.Fatalf("unable to query graph: %v", err) + } + if exists2 { + t.Fatalf("node already existed") + } + + // Add the edge between the two unknown nodes to the graph, and check + // that the nodes are found after the fact. + fundingTx, _, chanID, err := createChannelEdge(ctx, + bitcoinKey1.SerializeCompressed(), + bitcoinKey2.SerializeCompressed(), 10000, 500) if err != nil { t.Fatalf("unable to create channel edge: %v", err) } + fundingBlock := &wire.MsgBlock{ + Transactions: []*wire.MsgTx{fundingTx}, + } + ctx.chain.addBlock(fundingBlock, chanID.BlockHeight) edge := &channeldb.ChannelEdgeInfo{ ChannelID: chanID.ToUint64(), NodeKey1: priv1.PubKey(), - NodeKey2: priv1.PubKey(), + NodeKey2: priv2.PubKey(), BitcoinKey1: bitcoinKey1, BitcoinKey2: bitcoinKey2, AuthProof: nil, } - if err := ctx.router.AddEdge(edge); err == nil { - t.Fatal("edge should have been rejected due to unknown " + - "vertexes, but wasn't") + if err := ctx.router.AddEdge(edge); err != nil { + t.Fatalf("expected to be able to add edge to the channel graph,"+ + " even though the vertexes were unknown: %v.", err) + } + + // We must add the edge policy to be able to use the edge for route + // finding. + edgePolicy := &channeldb.ChannelEdgePolicy{ + Signature: testSig, + ChannelID: edge.ChannelID, + LastUpdate: time.Now(), + TimeLockDelta: 10, + MinHTLC: btcutil.Amount(1), + FeeBaseMSat: btcutil.Amount(10), + FeeProportionalMillionths: btcutil.Amount(10000), + } + edgePolicy.Flags = 0 + + if err := ctx.router.UpdateEdge(edgePolicy); err != nil { + t.Fatalf("unable to update edge policy: %v", err) + } + + // Create edge in the other direction as well. + edgePolicy = &channeldb.ChannelEdgePolicy{ + Signature: testSig, + ChannelID: edge.ChannelID, + LastUpdate: time.Now(), + TimeLockDelta: 10, + MinHTLC: btcutil.Amount(1), + FeeBaseMSat: btcutil.Amount(10), + FeeProportionalMillionths: btcutil.Amount(10000), + } + edgePolicy.Flags = 1 + + if err := ctx.router.UpdateEdge(edgePolicy); err != nil { + t.Fatalf("unable to update edge policy: %v", err) + } + + // After adding the edge between the two previously unknown nodes, they + // should have been added to the graph. + _, exists1, err = ctx.graph.HasLightningNode(priv1.PubKey()) + if err != nil { + t.Fatalf("unable to query graph: %v", err) + } + if !exists1 { + t.Fatalf("node1 was not added to the graph") + } + _, exists2, err = ctx.graph.HasLightningNode(priv2.PubKey()) + if err != nil { + t.Fatalf("unable to query graph: %v", err) + } + if !exists2 { + t.Fatalf("node2 was not added to the graph") + } + + // We will connect node1 to the rest of the test graph, and make sure + // we can find a route to node2, which will use the just added channel + // edge. + + // We will connect node 1 to "sophon" + connectNode := ctx.aliases["sophon"] + if connectNode == nil { + t.Fatalf("could not find node to connect to") + } + + var ( + pubKey1 *btcec.PublicKey + pubKey2 *btcec.PublicKey + ) + node1Bytes := priv1.PubKey().SerializeCompressed() + node2Bytes := connectNode.SerializeCompressed() + if bytes.Compare(node1Bytes, node2Bytes) == -1 { + pubKey1 = priv1.PubKey() + pubKey2 = connectNode + } else { + pubKey1 = connectNode + pubKey2 = priv1.PubKey() + } + + fundingTx, _, chanID, err = createChannelEdge(ctx, + pubKey1.SerializeCompressed(), pubKey2.SerializeCompressed(), + 10000, 510) + if err != nil { + t.Fatalf("unable to create channel edge: %v", err) + } + fundingBlock = &wire.MsgBlock{ + Transactions: []*wire.MsgTx{fundingTx}, + } + ctx.chain.addBlock(fundingBlock, chanID.BlockHeight) + + edge = &channeldb.ChannelEdgeInfo{ + ChannelID: chanID.ToUint64(), + NodeKey1: pubKey1, + NodeKey2: pubKey2, + BitcoinKey1: pubKey1, + BitcoinKey2: pubKey2, + AuthProof: nil, + } + + if err := ctx.router.AddEdge(edge); err != nil { + t.Fatalf("unable to add edge to the channel graph: %v.", err) + } + + edgePolicy = &channeldb.ChannelEdgePolicy{ + Signature: testSig, + ChannelID: edge.ChannelID, + LastUpdate: time.Now(), + TimeLockDelta: 10, + MinHTLC: btcutil.Amount(1), + FeeBaseMSat: btcutil.Amount(10), + FeeProportionalMillionths: btcutil.Amount(10000), + } + edgePolicy.Flags = 0 + + if err := ctx.router.UpdateEdge(edgePolicy); err != nil { + t.Fatalf("unable to update edge policy: %v", err) + } + + edgePolicy = &channeldb.ChannelEdgePolicy{ + Signature: testSig, + ChannelID: edge.ChannelID, + LastUpdate: time.Now(), + TimeLockDelta: 10, + MinHTLC: btcutil.Amount(1), + FeeBaseMSat: btcutil.Amount(10), + FeeProportionalMillionths: btcutil.Amount(10000), + } + edgePolicy.Flags = 1 + + if err := ctx.router.UpdateEdge(edgePolicy); err != nil { + t.Fatalf("unable to update edge policy: %v", err) + } + + // We should now be able to find one route to node 2. + const paymentAmt = btcutil.Amount(100) + targetNode := priv2.PubKey() + routes, err := ctx.router.FindRoutes(targetNode, paymentAmt) + if err != nil { + t.Fatalf("unable to find any routes: %v", err) + } + if len(routes) != 1 { + t.Fatalf("expected to find 1 route, found: %v", len(routes)) + } + + // Now check that we can update the node info for the partial node + // without messing up the channel graph. + n1 := &channeldb.LightningNode{ + HaveNodeAnnouncement: true, + LastUpdate: time.Unix(123, 0), + Addresses: testAddrs, + PubKey: priv1.PubKey(), + Color: color.RGBA{1, 2, 3, 0}, + Alias: "node11", + AuthSig: testSig, + Features: testFeatures, + } + + if err := ctx.router.AddNode(n1); err != nil { + t.Fatalf("could not add node: %v", err) + } + + n2 := &channeldb.LightningNode{ + HaveNodeAnnouncement: true, + LastUpdate: time.Unix(123, 0), + Addresses: testAddrs, + PubKey: priv2.PubKey(), + Color: color.RGBA{1, 2, 3, 0}, + Alias: "node22", + AuthSig: testSig, + Features: testFeatures, + } + + if err := ctx.router.AddNode(n2); err != nil { + t.Fatalf("could not add node: %v", err) + } + + // Should still be able to find the route, and the info should be + // updated. + routes, err = ctx.router.FindRoutes(targetNode, paymentAmt) + if err != nil { + t.Fatalf("unable to find any routes: %v", err) + } + if len(routes) != 1 { + t.Fatalf("expected to find 1 route, found: %v", len(routes)) + } + + copy1, err := ctx.graph.FetchLightningNode(priv1.PubKey()) + if err != nil { + t.Fatalf("unable to fetch node: %v", err) + } + + if copy1.Alias != n1.Alias { + t.Fatalf("fetched node not equal to original") + } + + copy2, err := ctx.graph.FetchLightningNode(priv2.PubKey()) + if err != nil { + t.Fatalf("unable to fetch node: %v", err) + } + + if copy2.Alias != n2.Alias { + t.Fatalf("fetched node not equal to original") } }