From e795f7fce4f39ac93c50e6250012ebf2b3b6671f Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Oct 2018 15:35:54 -0700 Subject: [PATCH 1/9] channeldb/graph: add method to determine if a node is public In this commit, we add a method to the ChannelGraph struct that determines whether a node is seen as public based on graph's source node's point of view. --- channeldb/graph.go | 71 ++++++++++++++++++ channeldb/graph_test.go | 156 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 227 insertions(+) diff --git a/channeldb/graph.go b/channeldb/graph.go index eb681395e..0fd9e0c8a 100644 --- a/channeldb/graph.go +++ b/channeldb/graph.go @@ -4,6 +4,7 @@ import ( "bytes" "crypto/sha256" "encoding/binary" + "errors" "fmt" "image/color" "io" @@ -1804,6 +1805,47 @@ func (l *LightningNode) NodeAnnouncement(signed bool) (*lnwire.NodeAnnouncement, return nodeAnn, nil } +// isPublic determines whether the node is seen as public within the graph from +// the source node's point of view. An existing database transaction can also be +// specified. +func (l *LightningNode) isPublic(tx *bolt.Tx, sourcePubKey []byte) (bool, error) { + // In order to determine whether this node is publicly advertised within + // the graph, we'll need to look at all of its edges and check whether + // they extend to any other node than the source node. errDone will be + // used to terminate the check early. + nodeIsPublic := false + errDone := errors.New("done") + err := l.ForEachChannel(tx, func(_ *bolt.Tx, info *ChannelEdgeInfo, + _, _ *ChannelEdgePolicy) error { + + // If this edge doesn't extend to the source node, we'll + // terminate our search as we can now conclude that the node is + // publicly advertised within the graph due to the local node + // knowing of the current edge. + if !bytes.Equal(info.NodeKey1Bytes[:], sourcePubKey) && + !bytes.Equal(info.NodeKey2Bytes[:], sourcePubKey) { + + nodeIsPublic = true + return errDone + } + + // Since the edge _does_ extend to the source node, we'll also + // need to ensure that this is a public edge. + if info.AuthProof != nil { + nodeIsPublic = true + return errDone + } + + // Otherwise, we'll continue our search. + return nil + }) + if err != nil && err != errDone { + return false, err + } + + return nodeIsPublic, nil +} + // FetchLightningNode attempts to look up a target node by its identity public // key. If the node isn't found in the database, then ErrGraphNodeNotFound is // returned. @@ -2566,6 +2608,35 @@ func (c *ChannelGraph) FetchChannelEdgesByID(chanID uint64) (*ChannelEdgeInfo, * return edgeInfo, policy1, policy2, nil } +// IsPublicNode is a helper method that determines whether the node with the +// given public key is seen as a public node in the graph from the graph's +// source node's point of view. +func (c *ChannelGraph) IsPublicNode(pubKey [33]byte) (bool, error) { + var nodeIsPublic bool + err := c.db.View(func(tx *bolt.Tx) error { + nodes := tx.Bucket(nodeBucket) + if nodes == nil { + return ErrGraphNodesNotFound + } + ourPubKey := nodes.Get(sourceKey) + if ourPubKey == nil { + return ErrSourceNodeNotSet + } + node, err := fetchLightningNode(nodes, pubKey[:]) + if err != nil { + return err + } + + nodeIsPublic, err = node.isPublic(tx, ourPubKey) + return err + }) + if err != nil { + return false, err + } + + return nodeIsPublic, nil +} + // genMultiSigP2WSH generates the p2wsh'd multisig script for 2 of 2 pubkeys. func genMultiSigP2WSH(aPub, bPub []byte) ([]byte, error) { if len(aPub) != 33 || len(bPub) != 33 { diff --git a/channeldb/graph_test.go b/channeldb/graph_test.go index 92fcd4f1d..369bdc6f7 100644 --- a/channeldb/graph_test.go +++ b/channeldb/graph_test.go @@ -2438,6 +2438,162 @@ func TestNodePruningUpdateIndexDeletion(t *testing.T) { } } +// TestNodeIsPublic ensures that we properly detect nodes that are seen as +// public within the network graph. +func TestNodeIsPublic(t *testing.T) { + t.Parallel() + + // We'll start off the test by creating a small network of 3 + // participants with the following graph: + // + // Alice <-> Bob <-> Carol + // + // We'll need to create a separate database and channel graph for each + // participant to replicate real-world scenarios (private edges being in + // some graphs but not others, etc.). + aliceDB, cleanUp, err := makeTestDB() + defer cleanUp() + if err != nil { + t.Fatalf("unable to make test database: %v", err) + } + aliceNode, err := createTestVertex(aliceDB) + if err != nil { + t.Fatalf("unable to create test node: %v", err) + } + aliceGraph := aliceDB.ChannelGraph() + if err := aliceGraph.SetSourceNode(aliceNode); err != nil { + t.Fatalf("unable to set source node: %v", err) + } + + bobDB, cleanUp, err := makeTestDB() + defer cleanUp() + if err != nil { + t.Fatalf("unable to make test database: %v", err) + } + bobNode, err := createTestVertex(bobDB) + if err != nil { + t.Fatalf("unable to create test node: %v", err) + } + bobGraph := bobDB.ChannelGraph() + if err := bobGraph.SetSourceNode(bobNode); err != nil { + t.Fatalf("unable to set source node: %v", err) + } + + carolDB, cleanUp, err := makeTestDB() + defer cleanUp() + if err != nil { + t.Fatalf("unable to make test database: %v", err) + } + carolNode, err := createTestVertex(carolDB) + if err != nil { + t.Fatalf("unable to create test node: %v", err) + } + carolGraph := carolDB.ChannelGraph() + if err := carolGraph.SetSourceNode(carolNode); err != nil { + t.Fatalf("unable to set source node: %v", err) + } + + aliceBobEdge, _ := createEdge(10, 0, 0, 0, aliceNode, bobNode) + bobCarolEdge, _ := createEdge(10, 1, 0, 1, bobNode, carolNode) + + // After creating all of our nodes and edges, we'll add them to each + // participant's graph. + nodes := []*LightningNode{aliceNode, bobNode, carolNode} + edges := []*ChannelEdgeInfo{&aliceBobEdge, &bobCarolEdge} + dbs := []*DB{aliceDB, bobDB, carolDB} + graphs := []*ChannelGraph{aliceGraph, bobGraph, carolGraph} + for i, graph := range graphs { + for _, node := range nodes { + node.db = dbs[i] + if err := graph.AddLightningNode(node); err != nil { + t.Fatalf("unable to add node: %v", err) + } + } + for _, edge := range edges { + edge.db = dbs[i] + if err := graph.AddChannelEdge(edge); err != nil { + t.Fatalf("unable to add edge: %v", err) + } + } + } + + // checkNodes is a helper closure that will be used to assert that the + // given nodes are seen as public/private within the given graphs. + checkNodes := func(nodes []*LightningNode, graphs []*ChannelGraph, + public bool) { + + t.Helper() + + for _, node := range nodes { + for _, graph := range graphs { + isPublic, err := graph.IsPublicNode(node.PubKeyBytes) + if err != nil { + t.Fatalf("unable to determine if pivot "+ + "is public: %v", err) + } + + switch { + case isPublic && !public: + t.Fatalf("expected %x to be private", + node.PubKeyBytes) + case !isPublic && public: + t.Fatalf("expected %x to be public", + node.PubKeyBytes) + } + } + } + } + + // Due to the way the edges were set up above, we'll make sure each node + // can correctly determine that every other node is public. + checkNodes(nodes, graphs, true) + + // Now, we'll remove the edge between Alice and Bob from everyone's + // graph. This will make Alice be seen as a private node as it no longer + // has any advertised edges. + for _, graph := range graphs { + err := graph.DeleteChannelEdge(&aliceBobEdge.ChannelPoint) + if err != nil { + t.Fatalf("unable to remove edge: %v", err) + } + } + checkNodes( + []*LightningNode{aliceNode}, + []*ChannelGraph{bobGraph, carolGraph}, + false, + ) + + // We'll also make the edge between Bob and Carol private. Within Bob's + // and Carol's graph, the edge will exist, but it will not have a proof + // that allows it to be advertised. Within Alice's graph, we'll + // completely remove the edge as it is not possible for her to know of + // it without it being advertised. + for i, graph := range graphs { + err := graph.DeleteChannelEdge(&bobCarolEdge.ChannelPoint) + if err != nil { + t.Fatalf("unable to remove edge: %v", err) + } + + if graph == aliceGraph { + continue + } + + bobCarolEdge.AuthProof = nil + bobCarolEdge.db = dbs[i] + if err := graph.AddChannelEdge(&bobCarolEdge); err != nil { + t.Fatalf("unable to add edge: %v", err) + } + } + + // With the modifications above, Bob should now be seen as a private + // node from both Alice's and Carol's perspective. + checkNodes( + []*LightningNode{bobNode}, + []*ChannelGraph{aliceGraph, carolGraph}, + false, + ) +} + // compareNodes is used to compare two LightningNodes while excluding the // Features struct, which cannot be compared as the semantics for reserializing // the featuresMap have not been defined. From 18ecb319832b0bf6aa1e5877199899897fa6f8e6 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Oct 2018 15:47:12 -0700 Subject: [PATCH 2/9] routing/router: extend ChannelGraphSource interface with IsPublicNode method --- routing/router.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/routing/router.go b/routing/router.go index 14c084a0d..a3c913888 100644 --- a/routing/router.go +++ b/routing/router.go @@ -72,6 +72,10 @@ type ChannelGraphSource interface { // the target node. IsStaleNode(node Vertex, timestamp time.Time) bool + // IsPublicNode determines whether the given vertex is seen as a public + // node in the graph from the graph's source node's point of view. + IsPublicNode(node Vertex) (bool, error) + // IsKnownEdge returns true if the graph source already knows of the // passed channel ID. IsKnownEdge(chanID lnwire.ShortChannelID) bool @@ -2222,6 +2226,14 @@ func (r *ChannelRouter) IsStaleNode(node Vertex, timestamp time.Time) bool { return r.assertNodeAnnFreshness(node, timestamp) != nil } +// IsPublicNode determines whether the given vertex is seen as a public node in +// the graph from the graph's source node's point of view. +// +// NOTE: This method is part of the ChannelGraphSource interface. +func (r *ChannelRouter) IsPublicNode(node Vertex) (bool, error) { + return r.cfg.Graph.IsPublicNode(node) +} + // IsKnownEdge returns true if the graph source already knows of the passed // channel ID. // From 80196eb20f9c099167abb1df83254fcaf6994ae3 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Oct 2018 15:49:14 -0700 Subject: [PATCH 3/9] discovery: ensure we only broadcast NodeAnnouncements of public nodes In this commit, we modify the gossiper to no longer broadcast NodeAnnouncements of nodes who intend to remain private. We do this to prevent leaking their information to the greater network. --- discovery/gossiper.go | 27 +++++-- discovery/gossiper_test.go | 152 ++++++++++++++++++++++++++++++++++--- 2 files changed, 160 insertions(+), 19 deletions(-) diff --git a/discovery/gossiper.go b/discovery/gossiper.go index 641f4c9ed..88fd401aa 100644 --- a/discovery/gossiper.go +++ b/discovery/gossiper.go @@ -1613,13 +1613,26 @@ func (d *AuthenticatedGossiper) processNetworkAnnouncement( return nil } - // Node announcement was successfully proceeded and know it - // might be broadcast to other connected nodes. - announcements = append(announcements, networkMsg{ - peer: nMsg.peer, - source: nMsg.source, - msg: msg, - }) + // In order to ensure we don't leak unadvertised nodes, we'll + // make a quick check to ensure this node intends to publicly + // advertise itself to the network. + isPublic, err := d.cfg.Router.IsPublicNode(node.PubKeyBytes) + if err != nil { + log.Errorf("Unable to determine if node %x is "+ + "advertised: %v", node.PubKeyBytes, err) + nMsg.err <- err + return nil + } + + // If it does, we'll add their announcement to our batch so that + // it can be broadcast to the rest of our peers. + if isPublic { + announcements = append(announcements, networkMsg{ + peer: nMsg.peer, + source: nMsg.source, + msg: msg, + }) + } nMsg.err <- nil // TODO(roasbeef): get rid of the above diff --git a/discovery/gossiper_test.go b/discovery/gossiper_test.go index 560125259..56d6bdfd7 100644 --- a/discovery/gossiper_test.go +++ b/discovery/gossiper_test.go @@ -1,23 +1,19 @@ package discovery import ( + "bytes" "encoding/hex" "fmt" + "io/ioutil" + "math/big" + prand "math/rand" "net" + "os" "reflect" "sync" - - prand "math/rand" - "testing" - - "math/big" - "time" - "io/ioutil" - "os" - "github.com/btcsuite/btcd/btcec" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/wire" @@ -213,6 +209,22 @@ func (r *mockGraphSource) IsStaleNode(nodePub routing.Vertex, timestamp time.Tim return false } +// IsPublicNode determines whether the given vertex is seen as a public node in +// the graph from the graph's source node's point of view. +func (r *mockGraphSource) IsPublicNode(node routing.Vertex) (bool, error) { + for _, info := range r.infos { + if !bytes.Equal(node[:], info.NodeKey1Bytes[:]) && + !bytes.Equal(node[:], info.NodeKey2Bytes[:]) { + continue + } + + if info.AuthProof != nil { + return true, nil + } + } + return false, nil +} + // IsKnownEdge returns true if the graph source already knows of the passed // channel ID. func (r *mockGraphSource) IsKnownEdge(chanID lnwire.ShortChannelID) bool { @@ -441,10 +453,9 @@ func createUpdateAnnouncement(blockHeight uint32, flags lnwire.ChanUpdateFlag, return a, nil } -func createRemoteChannelAnnouncement(blockHeight uint32, - extraBytes ...[]byte) (*lnwire.ChannelAnnouncement, error) { +func createAnnouncementWithoutProof(blockHeight uint32, + extraBytes ...[]byte) *lnwire.ChannelAnnouncement { - var err error a := &lnwire.ChannelAnnouncement{ ShortChannelID: lnwire.ShortChannelID{ BlockHeight: blockHeight, @@ -461,6 +472,14 @@ func createRemoteChannelAnnouncement(blockHeight uint32, a.ExtraOpaqueData = extraBytes[0] } + return a +} + +func createRemoteChannelAnnouncement(blockHeight uint32, + extraBytes ...[]byte) (*lnwire.ChannelAnnouncement, error) { + + a := createAnnouncementWithoutProof(blockHeight, extraBytes...) + pub := nodeKeyPriv1.PubKey() signer := mockSigner{nodeKeyPriv1} sig, err := SignAnnouncement(&signer, pub, a) @@ -1966,6 +1985,115 @@ func TestDeDuplicatedAnnouncements(t *testing.T) { } } +// TestForwardPrivateNodeAnnouncement ensures that we do not forward node +// announcements for nodes who do not intend to publicly advertise themselves. +func TestForwardPrivateNodeAnnouncement(t *testing.T) { + t.Parallel() + + const ( + startingHeight = 100 + timestamp = 123456 + ) + + ctx, cleanup, err := createTestCtx(startingHeight) + if err != nil { + t.Fatalf("can't create context: %v", err) + } + defer cleanup() + + // We'll start off by processing a channel announcement without a proof + // (i.e., an unadvertised channel), followed by a node announcement for + // this same channel announcement. + chanAnn := createAnnouncementWithoutProof(startingHeight - 2) + pubKey := nodeKeyPriv1.PubKey() + + select { + case err := <-ctx.gossiper.ProcessLocalAnnouncement(chanAnn, pubKey): + if err != nil { + t.Fatalf("unable to process local announcement: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatalf("local announcement not processed") + } + + // The gossiper should not broadcast the announcement due to it not + // having its announcement signatures. + select { + case <-ctx.broadcastedMessage: + t.Fatal("gossiper should not have broadcast channel announcement") + case <-time.After(2 * trickleDelay): + } + + nodeAnn, err := createNodeAnnouncement(nodeKeyPriv1, timestamp) + if err != nil { + t.Fatalf("unable to create node announcement: %v", err) + } + + select { + case err := <-ctx.gossiper.ProcessLocalAnnouncement(nodeAnn, pubKey): + if err != nil { + t.Fatalf("unable to process remote announcement: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("remote announcement not processed") + } + + // The gossiper should also not broadcast the node announcement due to + // it not being part of any advertised channels. + select { + case <-ctx.broadcastedMessage: + t.Fatal("gossiper should not have broadcast node announcement") + case <-time.After(2 * trickleDelay): + } + + // Now, we'll attempt to forward the NodeAnnouncement for the same node + // by opening a public channel on the network. We'll create a + // ChannelAnnouncement and hand it off to the gossiper in order to + // process it. + remoteChanAnn, err := createRemoteChannelAnnouncement(startingHeight - 1) + if err != nil { + t.Fatalf("unable to create remote channel announcement: %v", err) + } + peer := &mockPeer{pubKey, nil, nil} + + select { + case err := <-ctx.gossiper.ProcessRemoteAnnouncement(remoteChanAnn, peer): + if err != nil { + t.Fatalf("unable to process remote announcement: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("remote announcement not processed") + } + + select { + case <-ctx.broadcastedMessage: + case <-time.After(2 * trickleDelay): + t.Fatal("gossiper should have broadcast the channel announcement") + } + + // We'll recreate the NodeAnnouncement with an updated timestamp to + // prevent a stale update. The NodeAnnouncement should now be forwarded. + nodeAnn, err = createNodeAnnouncement(nodeKeyPriv1, timestamp+1) + if err != nil { + t.Fatalf("unable to create node announcement: %v", err) + } + + select { + case err := <-ctx.gossiper.ProcessRemoteAnnouncement(nodeAnn, peer): + if err != nil { + t.Fatalf("unable to process remote announcement: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("remote announcement not processed") + } + + select { + case <-ctx.broadcastedMessage: + case <-time.After(2 * trickleDelay): + t.Fatal("gossiper should have broadcast the node announcement") + } +} + // TestReceiveRemoteChannelUpdateFirst tests that if we receive a // ChannelUpdate from the remote before we have processed our // own ChannelAnnouncement, it will be reprocessed later, after From 748da2f50a8c5de467d5f28fc2f3408bdda5d9d3 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Oct 2018 15:55:46 -0700 Subject: [PATCH 4/9] discovery/gossiper_test: modify TestProcessAnnouncement to process node ann last In this commit, we modify TestProcessAnnouncement to process the node announcement last. We do this due to the recent change in the gossiper where we'll only forward node announcements of nodes who intend to advertised themselves within the network. This change was needed in order to allow the node announcement to be broadcast to the greater network, as otherwise the gossiper would assume the node intends to stay private due to not having any advertised edges. --- discovery/gossiper_test.go | 70 +++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/discovery/gossiper_test.go b/discovery/gossiper_test.go index 56d6bdfd7..d9499a4a3 100644 --- a/discovery/gossiper_test.go +++ b/discovery/gossiper_test.go @@ -616,40 +616,10 @@ func TestProcessAnnouncement(t *testing.T) { } } - // Create node valid, signed announcement, process it with - // gossiper service, check that valid announcement have been - // propagated farther into the lightning network, and check that we - // added new node into router. - na, err := createNodeAnnouncement(nodeKeyPriv1, timestamp) - if err != nil { - t.Fatalf("can't create node announcement: %v", err) - } - nodePeer := &mockPeer{nodeKeyPriv1.PubKey(), nil, nil} - select { - case err = <-ctx.gossiper.ProcessRemoteAnnouncement(na, nodePeer): - case <-time.After(2 * time.Second): - t.Fatal("remote announcement not processed") - } - if err != nil { - t.Fatalf("can't process remote announcement: %v", err) - } - - select { - case msg := <-ctx.broadcastedMessage: - assertSenderExistence(nodePeer.IdentityKey(), msg) - case <-time.After(2 * trickleDelay): - t.Fatal("announcement wasn't proceeded") - } - - if len(ctx.router.nodes) != 1 { - t.Fatalf("node wasn't added to router: %v", err) - } - - // Pretending that we receive the valid channel announcement from - // remote side, and check that we broadcasted it to the our network, - // and added channel info in the router. + // First, we'll craft a valid remote channel announcement and send it to + // the gossiper so that it can be processed. ca, err := createRemoteChannelAnnouncement(0) if err != nil { t.Fatalf("can't create channel announcement: %v", err) @@ -664,6 +634,8 @@ func TestProcessAnnouncement(t *testing.T) { t.Fatalf("can't process remote announcement: %v", err) } + // The announcement should be broadcast and included in our local view + // of the graph. select { case msg := <-ctx.broadcastedMessage: assertSenderExistence(nodePeer.IdentityKey(), msg) @@ -675,9 +647,8 @@ func TestProcessAnnouncement(t *testing.T) { t.Fatalf("edge wasn't added to router: %v", err) } - // Pretending that we received valid channel policy update from remote - // side, and check that we broadcasted it to the other network, and - // added updates to the router. + // We'll then craft the channel policy of the remote party and also send + // it to the gossiper. ua, err := createUpdateAnnouncement(0, 0, nodeKeyPriv1, timestamp) if err != nil { t.Fatalf("can't create update announcement: %v", err) @@ -692,6 +663,7 @@ func TestProcessAnnouncement(t *testing.T) { t.Fatalf("can't process remote announcement: %v", err) } + // The channel policy should be broadcast to the rest of the network. select { case msg := <-ctx.broadcastedMessage: assertSenderExistence(nodePeer.IdentityKey(), msg) @@ -702,6 +674,34 @@ func TestProcessAnnouncement(t *testing.T) { if len(ctx.router.edges) != 1 { t.Fatalf("edge update wasn't added to router: %v", err) } + + // Finally, we'll craft the remote party's node announcement. + na, err := createNodeAnnouncement(nodeKeyPriv1, timestamp) + if err != nil { + t.Fatalf("can't create node announcement: %v", err) + } + + select { + case err = <-ctx.gossiper.ProcessRemoteAnnouncement(na, nodePeer): + case <-time.After(2 * time.Second): + t.Fatal("remote announcement not processed") + } + if err != nil { + t.Fatalf("can't process remote announcement: %v", err) + } + + // It should also be broadcast to the network and included in our local + // view of the graph. + select { + case msg := <-ctx.broadcastedMessage: + assertSenderExistence(nodePeer.IdentityKey(), msg) + case <-time.After(2 * trickleDelay): + t.Fatal("announcement wasn't proceeded") + } + + if len(ctx.router.nodes) != 1 { + t.Fatalf("node wasn't added to router: %v", err) + } } // TestPrematureAnnouncement checks that premature announcements are From 85cdb18b73964c01ba064c881f19679375f25a3c Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Oct 2018 15:59:02 -0700 Subject: [PATCH 5/9] fundingmanager: send NodeAnnouncement to unadvertised channel counterparty In this commit, we modify the funding manager to send our NodeAnnouncement to our channel counterparty in the event of an unadvertised channel. We do this to ensure that our counterparty learns about some information about us that may aid them in one way or another (e.g., addresses to reconnect, features supported, etc.). --- fundingmanager.go | 48 ++++++++++++++++++++++++------ fundingmanager_test.go | 66 ++++++++++++++++++++++++++++++++++++------ 2 files changed, 96 insertions(+), 18 deletions(-) diff --git a/fundingmanager.go b/fundingmanager.go index 0acfadcfb..abd7df3fd 100644 --- a/fundingmanager.go +++ b/fundingmanager.go @@ -15,9 +15,6 @@ import ( "github.com/coreos/bbolt" "github.com/davecgh/go-spew/spew" "github.com/go-errors/errors" - "golang.org/x/crypto/salsa20" - "google.golang.org/grpc" - "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/htlcswitch" @@ -27,6 +24,8 @@ import ( "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing" + "golang.org/x/crypto/salsa20" + "google.golang.org/grpc" ) const ( @@ -2149,16 +2148,44 @@ func (f *fundingManager) addToRouterGraph(completeChan *channeldb.OpenChannel, func (f *fundingManager) annAfterSixConfs(completeChan *channeldb.OpenChannel, shortChanID *lnwire.ShortChannelID) error { - // If this channel is meant to be announced to the greater network, - // wait until the funding tx has reached 6 confirmations before - // announcing it. + // If this channel is not meant to be announced to the greater network, + // we'll only send our NodeAnnouncement to our counterparty to ensure we + // don't leak any of our information. announceChan := completeChan.ChannelFlags&lnwire.FFAnnounceChannel != 0 if !announceChan { fndgLog.Debugf("Will not announce private channel %v.", shortChanID.ToUint64()) + + peerChan := make(chan lnpeer.Peer, 1) + f.cfg.NotifyWhenOnline(completeChan.IdentityPub, peerChan) + + var peer lnpeer.Peer + select { + case peer = <-peerChan: + case <-f.quit: + return ErrFundingManagerShuttingDown + } + + nodeAnn, err := f.cfg.CurrentNodeAnnouncement() + if err != nil { + return fmt.Errorf("unable to retrieve current node "+ + "announcement: %v", err) + } + + chanID := lnwire.NewChanIDFromOutPoint( + &completeChan.FundingOutpoint, + ) + pubKey := peer.PubKey() + fndgLog.Debugf("Sending our NodeAnnouncement for "+ + "ChannelID(%v) to %x", chanID, pubKey) + + if err := peer.SendMessage(true, &nodeAnn); err != nil { + return fmt.Errorf("unable to send node announcement "+ + "to peer %x: %v", pubKey, err) + } } else { - // Register with the ChainNotifier for a notification once the - // funding transaction reaches at least 6 confirmations. + // Otherwise, we'll wait until the funding transaction has + // reached 6 confirmations before announcing it. numConfs := uint32(completeChan.NumConfsRequired) if numConfs < 6 { numConfs = 6 @@ -2176,8 +2203,11 @@ func (f *fundingManager) annAfterSixConfs(completeChan *channeldb.OpenChannel, completeChan.FundingOutpoint, err) } + // Register with the ChainNotifier for a notification once the + // funding transaction reaches at least 6 confirmations. confNtfn, err := f.cfg.Notifier.RegisterConfirmationsNtfn( - &txid, fundingScript, numConfs, completeChan.FundingBroadcastHeight, + &txid, fundingScript, numConfs, + completeChan.FundingBroadcastHeight, ) if err != nil { return fmt.Errorf("Unable to register for "+ diff --git a/fundingmanager_test.go b/fundingmanager_test.go index 2cfd80972..969e4efe4 100644 --- a/fundingmanager_test.go +++ b/fundingmanager_test.go @@ -1933,7 +1933,7 @@ func TestFundingManagerPrivateChannel(t *testing.T) { bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} // Since this is a private channel, we shouldn't receive the - // announcement signatures or node announcement messages. + // announcement signatures. select { case ann := <-alice.announceChan: t.Fatalf("unexpectedly got channel announcement message: %v", ann) @@ -1948,6 +1948,25 @@ func TestFundingManagerPrivateChannel(t *testing.T) { // Expected } + // We should however receive each side's node announcement. + select { + case msg := <-alice.msgChan: + if _, ok := msg.(*lnwire.NodeAnnouncement); !ok { + t.Fatalf("expected to receive node announcement") + } + case <-time.After(time.Second): + t.Fatalf("expected to receive node announcement") + } + + select { + case msg := <-bob.msgChan: + if _, ok := msg.(*lnwire.NodeAnnouncement); !ok { + t.Fatalf("expected to receive node announcement") + } + case <-time.After(time.Second): + t.Fatalf("expected to receive node announcement") + } + // The internal state-machine should now have deleted the channelStates // from the database, as the channel is announced. assertNoChannelState(t, alice, bob, fundingOutPoint) @@ -2013,19 +2032,48 @@ func TestFundingManagerPrivateRestart(t *testing.T) { // channel. assertHandleFundingLocked(t, alice, bob) - // Restart Alice's fundingManager so we can prove that the public - // channel announcements are not sent upon restart and that the private - // setting persists upon restart. - recreateAliceFundingManager(t, alice) - time.Sleep(300 * time.Millisecond) - // Notify that six confirmations has been reached on funding transaction. alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} // Since this is a private channel, we shouldn't receive the public - // channel announcement messages announcement signatures or - // node announcement. + // channel announcement messages. + select { + case ann := <-alice.announceChan: + t.Fatalf("unexpectedly got channel announcement message: %v", ann) + case <-time.After(300 * time.Millisecond): + } + + select { + case ann := <-bob.announceChan: + t.Fatalf("unexpectedly got channel announcement message: %v", ann) + case <-time.After(300 * time.Millisecond): + } + + // We should however receive each side's node announcement. + select { + case msg := <-alice.msgChan: + if _, ok := msg.(*lnwire.NodeAnnouncement); !ok { + t.Fatalf("expected to receive node announcement") + } + case <-time.After(time.Second): + t.Fatalf("expected to receive node announcement") + } + + select { + case msg := <-bob.msgChan: + if _, ok := msg.(*lnwire.NodeAnnouncement); !ok { + t.Fatalf("expected to receive node announcement") + } + case <-time.After(time.Second): + t.Fatalf("expected to receive node announcement") + } + + // Restart Alice's fundingManager so we can prove that the public + // channel announcements are not sent upon restart and that the private + // setting persists upon restart. + recreateAliceFundingManager(t, alice) + select { case ann := <-alice.announceChan: t.Fatalf("unexpectedly got channel announcement message: %v", ann) From cbab298154b85c37c23cf19f0e07aa0716bd258d Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Oct 2018 16:09:34 -0700 Subject: [PATCH 6/9] chan_series: filter out nodes who intend to remain private In this commit, we filter out nodes who intend to remain private. We do this to prevent leaking information about them by forwarding their NodeAnnouncements. --- chan_series.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/chan_series.go b/chan_series.go index 83b05b028..7f592503a 100644 --- a/chan_series.go +++ b/chan_series.go @@ -89,6 +89,22 @@ func (c *chanSeries) UpdatesInHorizon(chain chainhash.Hash, return nil, err } for _, nodeAnn := range nodeAnnsInHorizon { + // Ensure we only forward nodes that are publicly advertised to + // prevent leaking information about nodes. + isNodePublic, err := c.graph.IsPublicNode(nodeAnn.PubKeyBytes) + if err != nil { + srvrLog.Errorf("Unable to determine if node %x is "+ + "advertised: %v", nodeAnn.PubKeyBytes, err) + continue + } + + if !isNodePublic { + srvrLog.Tracef("Skipping forwarding announcement for "+ + "node %x due to being unadvertised", + nodeAnn.PubKeyBytes) + continue + } + nodeUpdate, err := nodeAnn.NodeAnnouncement(true) if err != nil { return nil, err From e87d61b7f792a2784259d8bfaf97992de25558e5 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Oct 2018 16:13:37 -0700 Subject: [PATCH 7/9] rpc: ensure we don't leak unadvertised nodes within invoice routing hints In this commit, we ensure that we don't include routing hints for unadvertised nodes at the time of invoice creation. Otherwise, this would lead us to leak these unadvertised nodes to anyone who can get their hands on the invoice being created. To prevent this, we'll now look at the network graph and ensure that the node in unadvertised if all of their edges are unadvertised and only extend to us. --- rpcserver.go | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/rpcserver.go b/rpcserver.go index 5afcc6cc1..a9052af1d 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -2759,12 +2759,34 @@ func (r *rpcServer) AddInvoice(ctx context.Context, } if !link.EligibleToForward() { - rpcsLog.Debugf("Skipping link %v due to not "+ + rpcsLog.Debugf("Skipping channel %v due to not "+ "being eligible to forward payments", chanPoint) continue } + // To ensure we don't leak unadvertised nodes, we'll + // make sure our counterparty is publicly advertised + // within the network. Otherwise, we'll end up leaking + // information about nodes that intend to stay + // unadvertised, like in the case of a node only having + // private channels. + var remotePub [33]byte + copy(remotePub[:], channel.IdentityPub.SerializeCompressed()) + isRemoteNodePublic, err := graph.IsPublicNode(remotePub) + if err != nil { + rpcsLog.Errorf("Unable to determine if node %x "+ + "is advertised: %v", remotePub, err) + continue + } + + if !isRemoteNodePublic { + rpcsLog.Debugf("Skipping channel %v due to "+ + "counterparty %x being unadvertised", + chanPoint, remotePub) + continue + } + // Fetch the policies for each end of the channel. chanID := channel.ShortChanID().ToUint64() info, p1, p2, err := graph.FetchChannelEdgesByID(chanID) @@ -2778,8 +2800,7 @@ func (r *rpcServer) AddInvoice(ctx context.Context, // Now, we'll need to determine which is the correct // policy for HTLCs being sent from the remote node. var remotePolicy *channeldb.ChannelEdgePolicy - remotePub := channel.IdentityPub.SerializeCompressed() - if bytes.Equal(remotePub, info.NodeKey1Bytes[:]) { + if bytes.Equal(remotePub[:], info.NodeKey1Bytes[:]) { remotePolicy = p1 } else { remotePolicy = p2 From 6686ae300179722a5929b25ecd3126f85b35dd04 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Oct 2018 16:14:31 -0700 Subject: [PATCH 8/9] lnd_test: update testInvoiceRoutingHints to account In this commit, we open an additional channel between Bob and Carol to ensure that Bob gets selected as the only routing hint. Previously Bob would get selected, but with the recent changes, it would no longer happen due to him not having any advertised edges. --- lnd_test.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/lnd_test.go b/lnd_test.go index 9a72e994c..f3e2b93d5 100644 --- a/lnd_test.go +++ b/lnd_test.go @@ -4661,6 +4661,23 @@ func testInvoiceRoutingHints(net *lntest.NetworkHarness, t *harnessTest) { }, ) + // We'll also create a public channel between Bob and Carol to ensure + // that Bob gets selected as the only routing hint. We do this as + // we should only include routing hints for nodes that are publicly + // advertised, otherwise we'd end up leaking information about nodes + // that wish to stay unadvertised. + if err := net.ConnectNodes(ctxb, net.Bob, carol); err != nil { + t.Fatalf("unable to connect alice to carol: %v", err) + } + ctxt, _ = context.WithTimeout(ctxb, timeout) + chanPointBobCarol := openChannelAndAssert( + ctxt, t, net, net.Bob, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: chanAmt / 2, + }, + ) + // Then, we'll create Dave's node and open a private channel between him // and Alice. We will not include a push amount in order to not consider // this channel as a routing hint as it will not have enough remote @@ -4707,7 +4724,8 @@ func testInvoiceRoutingHints(net *lntest.NetworkHarness, t *harnessTest) { // Make sure all the channels have been opened. nodeNames := []string{"bob", "carol", "dave", "eve"} aliceChans := []*lnrpc.ChannelPoint{ - chanPointBob, chanPointCarol, chanPointDave, chanPointEve, + chanPointBob, chanPointCarol, chanPointBobCarol, chanPointDave, + chanPointEve, } for i, chanPoint := range aliceChans { ctxt, _ := context.WithTimeout(ctxb, timeout) @@ -4799,6 +4817,8 @@ func testInvoiceRoutingHints(net *lntest.NetworkHarness, t *harnessTest) { ctxt, _ = context.WithTimeout(ctxb, timeout) closeChannelAndAssert(ctxt, t, net, net.Alice, chanPointCarol, false) ctxt, _ = context.WithTimeout(ctxb, timeout) + closeChannelAndAssert(ctxt, t, net, net.Bob, chanPointBobCarol, false) + ctxt, _ = context.WithTimeout(ctxb, timeout) closeChannelAndAssert(ctxt, t, net, net.Alice, chanPointDave, false) // The channel between Alice and Eve should be force closed since Eve From da56aa3528ae95398060de99101c4b7b5d2ae5c3 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 1 Oct 2018 16:45:11 -0700 Subject: [PATCH 9/9] lnd_test: process all node/edge updates in graph_top_itest This commit modifies the graph topology test to properly count channel updates and node announcments in the event that they are batched into a single topology update. The prior logic made the assumption that they were always in distinct topology updates, so this method should be more general and robust. --- lnd_test.go | 58 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 24 deletions(-) diff --git a/lnd_test.go b/lnd_test.go index f3e2b93d5..00506f8b1 100644 --- a/lnd_test.go +++ b/lnd_test.go @@ -7677,20 +7677,16 @@ func testGraphTopologyNotifications(net *lntest.NetworkHarness, t *harnessTest) // The channel opening above should have triggered a few notifications // sent to the notification client. We'll expect two channel updates, // and two node announcements. - const numExpectedUpdates = 4 - for i := 0; i < numExpectedUpdates; i++ { + var numChannelUpds int + var numNodeAnns int + for numChannelUpds < 2 && numNodeAnns < 2 { select { // Ensure that a new update for both created edges is properly // dispatched to our registered client. case graphUpdate := <-graphSub.updateChan: - - if len(graphUpdate.ChannelUpdates) > 0 { - chanUpdate := graphUpdate.ChannelUpdates[0] - if chanUpdate.Capacity != int64(chanAmt) { - t.Fatalf("channel capacities mismatch:"+ - " expected %v, got %v", chanAmt, - btcutil.Amount(chanUpdate.Capacity)) - } + // Process all channel updates prsented in this update + // message. + for _, chanUpdate := range graphUpdate.ChannelUpdates { switch chanUpdate.AdvertisingNode { case net.Alice.PubKeyStr: case net.Bob.PubKeyStr: @@ -7705,10 +7701,16 @@ func testGraphTopologyNotifications(net *lntest.NetworkHarness, t *harnessTest) t.Fatalf("unknown connecting node: %v", chanUpdate.ConnectingNode) } + + if chanUpdate.Capacity != int64(chanAmt) { + t.Fatalf("channel capacities mismatch:"+ + " expected %v, got %v", chanAmt, + btcutil.Amount(chanUpdate.Capacity)) + } + numChannelUpds++ } - if len(graphUpdate.NodeUpdates) > 0 { - nodeUpdate := graphUpdate.NodeUpdates[0] + for _, nodeUpdate := range graphUpdate.NodeUpdates { switch nodeUpdate.IdentityKey { case net.Alice.PubKeyStr: case net.Bob.PubKeyStr: @@ -7716,11 +7718,14 @@ func testGraphTopologyNotifications(net *lntest.NetworkHarness, t *harnessTest) t.Fatalf("unknown node: %v", nodeUpdate.IdentityKey) } + numNodeAnns++ } case err := <-graphSub.errChan: t.Fatalf("unable to recv graph update: %v", err) case <-time.After(time.Second * 10): - t.Fatalf("timeout waiting for graph notification %v", i) + t.Fatalf("timeout waiting for graph notifications, "+ + "only received %d/2 chanupds and %d/2 nodeanns", + numChannelUpds, numNodeAnns) } } @@ -7819,11 +7824,12 @@ out: // We should receive an update advertising the newly connected node, // Bob's new node announcement, and the channel between Bob and Carol. - for i := 0; i < 3; i++ { + numNodeAnns = 0 + numChannelUpds = 0 + for numChannelUpds < 2 && numNodeAnns < 1 { select { case graphUpdate := <-graphSub.updateChan: - if len(graphUpdate.NodeUpdates) > 0 { - nodeUpdate := graphUpdate.NodeUpdates[0] + for _, nodeUpdate := range graphUpdate.NodeUpdates { switch nodeUpdate.IdentityKey { case carol.PubKeyStr: case net.Bob.PubKeyStr: @@ -7831,15 +7837,10 @@ out: t.Fatalf("unknown node update pubey: %v", nodeUpdate.IdentityKey) } + numNodeAnns++ } - if len(graphUpdate.ChannelUpdates) > 0 { - chanUpdate := graphUpdate.ChannelUpdates[0] - if chanUpdate.Capacity != int64(chanAmt) { - t.Fatalf("channel capacities mismatch:"+ - " expected %v, got %v", chanAmt, - btcutil.Amount(chanUpdate.Capacity)) - } + for _, chanUpdate := range graphUpdate.ChannelUpdates { switch chanUpdate.AdvertisingNode { case carol.PubKeyStr: case net.Bob.PubKeyStr: @@ -7854,11 +7855,20 @@ out: t.Fatalf("unknown connecting node: %v", chanUpdate.ConnectingNode) } + + if chanUpdate.Capacity != int64(chanAmt) { + t.Fatalf("channel capacities mismatch:"+ + " expected %v, got %v", chanAmt, + btcutil.Amount(chanUpdate.Capacity)) + } + numChannelUpds++ } case err := <-graphSub.errChan: t.Fatalf("unable to recv graph update: %v", err) case <-time.After(time.Second * 10): - t.Fatalf("timeout waiting for graph notification %v", i) + t.Fatalf("timeout waiting for graph notifications, "+ + "only received %d/2 chanupds and %d/2 nodeanns", + numChannelUpds, numNodeAnns) } }