From 72ab6d325555c742c6b70202d865ec23b50734d6 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 23 Apr 2015 12:11:21 +0200 Subject: [PATCH] p2p/discover: track sha3(ID) in Node --- p2p/discover/database.go | 2 + p2p/discover/database_test.go | 72 ++++++++++++++++++++--------------- p2p/discover/node.go | 55 ++++++++++++++++---------- p2p/discover/node_test.go | 71 ++++++++++++++++++++-------------- p2p/discover/table.go | 11 +++--- p2p/discover/table_test.go | 2 +- p2p/discover/udp.go | 32 +++++++++++++--- p2p/discover/udp_test.go | 20 ++++++---- 8 files changed, 166 insertions(+), 99 deletions(-) diff --git a/p2p/discover/database.go b/p2p/discover/database.go index 964f86b84..dc0b97ddf 100644 --- a/p2p/discover/database.go +++ b/p2p/discover/database.go @@ -10,6 +10,7 @@ import ( "sync" "time" + "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/logger" "github.com/ethereum/go-ethereum/logger/glog" "github.com/ethereum/go-ethereum/rlp" @@ -167,6 +168,7 @@ func (db *nodeDB) node(id NodeID) *Node { glog.V(logger.Warn).Infof("failed to decode node RLP: %v", err) return nil } + node.sha = crypto.Sha3Hash(node.ID[:]) return node } diff --git a/p2p/discover/database_test.go b/p2p/discover/database_test.go index 0cc0dec32..3ed84a099 100644 --- a/p2p/discover/database_test.go +++ b/p2p/discover/database_test.go @@ -86,12 +86,12 @@ func TestNodeDBInt64(t *testing.T) { } func TestNodeDBFetchStore(t *testing.T) { - node := &Node{ - ID: MustHexID("0x1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), - IP: net.IP([]byte{192, 168, 0, 1}), - UDP: 30303, - TCP: 30303, - } + node := newNode( + MustHexID("0x1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), + net.IP{192, 168, 0, 1}, + 30303, + 30303, + ) inst := time.Now() db, _ := newNodeDB("", Version) @@ -132,28 +132,34 @@ func TestNodeDBFetchStore(t *testing.T) { } var nodeDBSeedQueryNodes = []struct { - node Node + node *Node pong time.Time }{ { - node: Node{ - ID: MustHexID("0x01d9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), - IP: []byte{127, 0, 0, 1}, - }, + node: newNode( + MustHexID("0x01d9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), + net.IP{127, 0, 0, 1}, + 30303, + 30303, + ), pong: time.Now().Add(-2 * time.Second), }, { - node: Node{ - ID: MustHexID("0x02d9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), - IP: []byte{127, 0, 0, 2}, - }, + node: newNode( + MustHexID("0x02d9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), + net.IP{127, 0, 0, 2}, + 30303, + 30303, + ), pong: time.Now().Add(-3 * time.Second), }, { - node: Node{ - ID: MustHexID("0x03d9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), - IP: []byte{127, 0, 0, 3}, - }, + node: newNode( + MustHexID("0x03d9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), + net.IP{127, 0, 0, 3}, + 30303, + 30303, + ), pong: time.Now().Add(-1 * time.Second), }, } @@ -164,7 +170,7 @@ func TestNodeDBSeedQuery(t *testing.T) { // Insert a batch of nodes for querying for i, seed := range nodeDBSeedQueryNodes { - if err := db.updateNode(&seed.node); err != nil { + if err := db.updateNode(seed.node); err != nil { t.Fatalf("node %d: failed to insert: %v", i, err) } } @@ -204,7 +210,7 @@ func TestNodeDBSeedQueryContinuation(t *testing.T) { // Insert a batch of nodes for querying for i, seed := range nodeDBSeedQueryNodes { - if err := db.updateNode(&seed.node); err != nil { + if err := db.updateNode(seed.node); err != nil { t.Fatalf("node %d: failed to insert: %v", i, err) } } @@ -268,22 +274,26 @@ func TestNodeDBPersistency(t *testing.T) { } var nodeDBExpirationNodes = []struct { - node Node + node *Node pong time.Time exp bool }{ { - node: Node{ - ID: MustHexID("0x01d9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), - IP: []byte{127, 0, 0, 1}, - }, + node: newNode( + MustHexID("0x01d9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), + net.IP{127, 0, 0, 1}, + 30303, + 30303, + ), pong: time.Now().Add(-nodeDBNodeExpiration + time.Minute), exp: false, }, { - node: Node{ - ID: MustHexID("0x02d9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), - IP: []byte{127, 0, 0, 2}, - }, + node: newNode( + MustHexID("0x02d9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), + net.IP{127, 0, 0, 2}, + 30303, + 30303, + ), pong: time.Now().Add(-nodeDBNodeExpiration - time.Minute), exp: true, }, @@ -295,7 +305,7 @@ func TestNodeDBExpiration(t *testing.T) { // Add all the test nodes and set their last pong time for i, seed := range nodeDBExpirationNodes { - if err := db.updateNode(&seed.node); err != nil { + if err := db.updateNode(seed.node); err != nil { t.Fatalf("node %d: failed to insert: %v", i, err) } if err := db.updateLastPong(seed.node.ID, seed.pong); err != nil { diff --git a/p2p/discover/node.go b/p2p/discover/node.go index cfb6ff552..d922ed317 100644 --- a/p2p/discover/node.go +++ b/p2p/discover/node.go @@ -13,6 +13,7 @@ import ( "strconv" "strings" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/crypto/secp256k1" ) @@ -23,19 +24,26 @@ const nodeIDBits = 512 type Node struct { IP net.IP // len 4 for IPv4 or 16 for IPv6 UDP, TCP uint16 // port numbers - ID NodeID + ID NodeID // the node's public key + + // This is a cached copy of sha3(ID) which is used for node + // distance calculations. This is part of Node in order to make it + // possible to write tests that need a node at a certain distance. + // In those tests, the content of sha will not actually correspond + // with ID. + sha common.Hash } -func newNode(id NodeID, addr *net.UDPAddr) *Node { - ip := addr.IP.To4() - if ip == nil { - ip = addr.IP.To16() +func newNode(id NodeID, ip net.IP, udpPort, tcpPort uint16) *Node { + if ipv4 := ip.To4(); ipv4 != nil { + ip = ipv4 } return &Node{ IP: ip, - UDP: uint16(addr.Port), - TCP: uint16(addr.Port), + UDP: udpPort, + TCP: tcpPort, ID: id, + sha: crypto.Sha3Hash(id[:]), } } @@ -75,40 +83,47 @@ func (n *Node) String() string { // // enode://@10.3.58.6:30303?discport=30301 func ParseNode(rawurl string) (*Node, error) { - var n Node + var ( + id NodeID + ip net.IP + tcpPort, udpPort uint64 + ) u, err := url.Parse(rawurl) if u.Scheme != "enode" { return nil, errors.New("invalid URL scheme, want \"enode\"") } + // Parse the Node ID from the user portion. if u.User == nil { return nil, errors.New("does not contain node ID") } - if n.ID, err = HexID(u.User.String()); err != nil { + if id, err = HexID(u.User.String()); err != nil { return nil, fmt.Errorf("invalid node ID (%v)", err) } - ip, port, err := net.SplitHostPort(u.Host) + // Parse the IP address. + host, port, err := net.SplitHostPort(u.Host) if err != nil { return nil, fmt.Errorf("invalid host: %v", err) } - if n.IP = net.ParseIP(ip); n.IP == nil { + if ip = net.ParseIP(host); ip == nil { return nil, errors.New("invalid IP address") } - tcp, err := strconv.ParseUint(port, 10, 16) - if err != nil { + // Ensure the IP is 4 bytes long for IPv4 addresses. + if ipv4 := ip.To4(); ipv4 != nil { + ip = ipv4 + } + // Parse the port numbers. + if tcpPort, err = strconv.ParseUint(port, 10, 16); err != nil { return nil, errors.New("invalid port") } - n.TCP = uint16(tcp) + udpPort = tcpPort qv := u.Query() - if qv.Get("discport") == "" { - n.UDP = n.TCP - } else { - udp, err := strconv.ParseUint(qv.Get("discport"), 10, 16) + if qv.Get("discport") != "" { + udpPort, err = strconv.ParseUint(qv.Get("discport"), 10, 16) if err != nil { return nil, errors.New("invalid discport in query") } - n.UDP = uint16(udp) } - return &n, nil + return newNode(id, ip, uint16(udpPort), uint16(tcpPort)), nil } // MustParseNode parses a node URL. It panics if the URL is not valid. diff --git a/p2p/discover/node_test.go b/p2p/discover/node_test.go index 8ea9018c5..4c95d316f 100644 --- a/p2p/discover/node_test.go +++ b/p2p/discover/node_test.go @@ -48,46 +48,61 @@ var parseNodeTests = []struct { }, { rawurl: "enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@127.0.0.1:52150", - wantResult: &Node{ - ID: MustHexID("0x1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), - IP: net.ParseIP("127.0.0.1"), - UDP: 52150, - TCP: 52150, - }, + wantResult: newNode( + MustHexID("0x1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), + net.IP{0x7f, 0x0, 0x0, 0x1}, + 52150, + 52150, + ), }, { rawurl: "enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@[::]:52150", - wantResult: &Node{ - ID: MustHexID("0x1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), - IP: net.ParseIP("::"), - UDP: 52150, - TCP: 52150, - }, + wantResult: newNode( + MustHexID("0x1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), + net.ParseIP("::"), + 52150, + 52150, + ), + }, + { + rawurl: "enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@[2001:db8:3c4d:15::abcd:ef12]:52150", + wantResult: newNode( + MustHexID("0x1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), + net.ParseIP("2001:db8:3c4d:15::abcd:ef12"), + 52150, + 52150, + ), }, { rawurl: "enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@127.0.0.1:52150?discport=22334", - wantResult: &Node{ - ID: MustHexID("0x1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), - IP: net.ParseIP("127.0.0.1"), - UDP: 22334, - TCP: 52150, - }, + wantResult: newNode( + MustHexID("0x1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), + net.IP{0x7f, 0x0, 0x0, 0x1}, + 22334, + 52150, + ), }, } func TestParseNode(t *testing.T) { for i, test := range parseNodeTests { n, err := ParseNode(test.rawurl) - if err == nil && test.wantError != "" { - t.Errorf("test %d: got nil error, expected %#q", i, test.wantError) - continue - } - if err != nil && err.Error() != test.wantError { - t.Errorf("test %d: got error %#q, expected %#q", i, err.Error(), test.wantError) - continue - } - if !reflect.DeepEqual(n, test.wantResult) { - t.Errorf("test %d: result mismatch:\ngot: %#v, want: %#v", i, n, test.wantResult) + if test.wantError != "" { + if err == nil { + t.Errorf("test %d: got nil error, expected %#q", i, test.wantError) + continue + } else if err.Error() != test.wantError { + t.Errorf("test %d: got error %#q, expected %#q", i, err.Error(), test.wantError) + continue + } + } else { + if err != nil { + t.Errorf("test %d: unexpected error: %v", i, err) + continue + } + if !reflect.DeepEqual(n, test.wantResult) { + t.Errorf("test %d: result mismatch:\ngot: %#v, want: %#v", i, n, test.wantResult) + } } } } diff --git a/p2p/discover/table.go b/p2p/discover/table.go index bbd40fde9..ae10fed5b 100644 --- a/p2p/discover/table.go +++ b/p2p/discover/table.go @@ -12,6 +12,7 @@ import ( "sync" "time" + "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/logger" "github.com/ethereum/go-ethereum/logger/glog" ) @@ -71,7 +72,7 @@ func newTable(t transport, ourID NodeID, ourAddr *net.UDPAddr, nodeDBPath string tab := &Table{ net: t, db: db, - self: newNode(ourID, ourAddr), + self: newNode(ourID, ourAddr.IP, uint16(ourAddr.Port), uint16(ourAddr.Port)), bonding: make(map[NodeID]*bondproc), bondslots: make(chan struct{}, maxBondingPingPongs), } @@ -105,6 +106,7 @@ func (tab *Table) Bootstrap(nodes []*Node) { tab.nursery = make([]*Node, 0, len(nodes)) for _, n := range nodes { cpy := *n + cpy.sha = crypto.Sha3Hash(n.ID[:]) tab.nursery = append(tab.nursery, &cpy) } tab.mutex.Unlock() @@ -299,7 +301,7 @@ func (tab *Table) pingpong(w *bondproc, pinged bool, id NodeID, addr *net.UDPAdd tab.net.waitping(id) } // Bonding succeeded, update the node database - w.n = &Node{ID: id, IP: addr.IP, UDP: uint16(addr.Port), TCP: tcpPort} + w.n = newNode(id, addr.IP, uint16(addr.Port), tcpPort) tab.db.updateNode(w.n) close(w.done) } @@ -340,9 +342,8 @@ func (tab *Table) ping(id NodeID, addr *net.UDPAddr) error { func (tab *Table) add(entries []*Node) { outer: for _, n := range entries { - if n == nil || n.ID == tab.self.ID { - // skip bad entries. The RLP decoder returns nil for empty - // input lists. + if n.ID == tab.self.ID { + // don't add self. continue } bucket := tab.buckets[logdist(tab.self.ID, n.ID)] diff --git a/p2p/discover/table_test.go b/p2p/discover/table_test.go index e7394756d..41c57ce21 100644 --- a/p2p/discover/table_test.go +++ b/p2p/discover/table_test.go @@ -224,7 +224,7 @@ func TestTable_Lookup(t *testing.T) { t.Fatalf("lookup on empty table returned %d results: %#v", len(results), results) } // seed table with initial node (otherwise lookup will terminate immediately) - tab.add([]*Node{newNode(randomID(target, 200), &net.UDPAddr{Port: 200})}) + tab.add([]*Node{newNode(randomID(target, 200), net.ParseIP("127.0.0.1"), 200, 200)}) results := tab.Lookup(target) t.Logf("results:") diff --git a/p2p/discover/udp.go b/p2p/discover/udp.go index c81ca1a53..100a24e69 100644 --- a/p2p/discover/udp.go +++ b/p2p/discover/udp.go @@ -74,10 +74,17 @@ type ( // reply to findnode neighbors struct { - Nodes []*Node + Nodes []rpcNode Expiration uint64 } + rpcNode struct { + IP net.IP // len 4 for IPv4 or 16 for IPv6 + UDP uint16 // for discovery protocol + TCP uint16 // for RLPx protocol + ID NodeID + } + rpcEndpoint struct { IP net.IP // len 4 for IPv4 or 16 for IPv6 UDP uint16 // for discovery protocol @@ -93,9 +100,17 @@ func makeEndpoint(addr *net.UDPAddr, tcpPort uint16) rpcEndpoint { return rpcEndpoint{IP: ip, UDP: uint16(addr.Port), TCP: tcpPort} } -func validNode(n *Node) bool { +func nodeFromRPC(rn rpcNode) (n *Node, valid bool) { // TODO: don't accept localhost, LAN addresses from internet hosts - return !n.IP.IsMulticast() && !n.IP.IsUnspecified() && n.UDP != 0 + // TODO: check public key is on secp256k1 curve + if rn.IP.IsMulticast() || rn.IP.IsUnspecified() || rn.UDP == 0 { + return nil, false + } + return newNode(rn.ID, rn.IP, rn.UDP, rn.TCP), true +} + +func nodeToRPC(n *Node) rpcNode { + return rpcNode{ID: n.ID, IP: n.IP, UDP: n.UDP, TCP: n.TCP} } type packet interface { @@ -232,9 +247,9 @@ func (t *udp) findnode(toid NodeID, toaddr *net.UDPAddr, target NodeID) ([]*Node nreceived := 0 errc := t.pending(toid, neighborsPacket, func(r interface{}) bool { reply := r.(*neighbors) - for _, n := range reply.Nodes { + for _, rn := range reply.Nodes { nreceived++ - if validNode(n) { + if n, valid := nodeFromRPC(rn); valid { nodes = append(nodes, n) } } @@ -489,8 +504,13 @@ func (req *findnode) handle(t *udp, from *net.UDPAddr, fromID NodeID, mac []byte closest := t.closest(req.Target, bucketSize).entries t.mutex.Unlock() + // TODO: this conversion could use a cached version of the slice + closestrpc := make([]rpcNode, len(closest)) + for i, n := range closest { + closestrpc[i] = nodeToRPC(n) + } t.send(from, neighborsPacket, neighbors{ - Nodes: closest, + Nodes: closestrpc, Expiration: uint64(time.Now().Add(expiration).Unix()), }) return nil diff --git a/p2p/discover/udp_test.go b/p2p/discover/udp_test.go index 378edaaa7..167a5439b 100644 --- a/p2p/discover/udp_test.go +++ b/p2p/discover/udp_test.go @@ -159,12 +159,12 @@ func TestUDP_findnode(t *testing.T) { // ensure there's a bond with the test node, // findnode won't be accepted otherwise. - test.table.db.updateNode(&Node{ - ID: PubkeyID(&test.remotekey.PublicKey), - IP: test.remoteaddr.IP, - UDP: uint16(test.remoteaddr.Port), - TCP: 99, - }) + test.table.db.updateNode(newNode( + PubkeyID(&test.remotekey.PublicKey), + test.remoteaddr.IP, + uint16(test.remoteaddr.Port), + 99, + )) // check that closest neighbors are returned. test.packetIn(nil, findnodePacket, &findnode{Target: testTarget, Expiration: futureExp}) test.waitPacketOut(func(p *neighbors) { @@ -211,8 +211,12 @@ func TestUDP_findnodeMultiReply(t *testing.T) { MustParseNode("enode://9bffefd833d53fac8e652415f4973bee289e8b1a5c6c4cbe70abf817ce8a64cee11b823b66a987f51aaa9fba0d6a91b3e6bf0d5a5d1042de8e9eeea057b217f8@10.0.1.36:30301?discport=17"), MustParseNode("enode://1b5b4aa662d7cb44a7221bfba67302590b643028197a7d5214790f3bac7aaa4a3241be9e83c09cf1f6c69d007c634faae3dc1b1221793e8446c0b3a09de65960@10.0.1.16:30303"), } - test.packetIn(nil, neighborsPacket, &neighbors{Expiration: futureExp, Nodes: list[:2]}) - test.packetIn(nil, neighborsPacket, &neighbors{Expiration: futureExp, Nodes: list[2:]}) + rpclist := make([]rpcNode, len(list)) + for i := range list { + rpclist[i] = nodeToRPC(list[i]) + } + test.packetIn(nil, neighborsPacket, &neighbors{Expiration: futureExp, Nodes: rpclist[:2]}) + test.packetIn(nil, neighborsPacket, &neighbors{Expiration: futureExp, Nodes: rpclist[2:]}) // check that the sent neighbors are all returned by findnode select {