From b08e03103a7de9c86c67d29c2a936026b6155a72 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 23 May 2024 14:27:03 +0200 Subject: [PATCH] p2p/enode: fix endpoint determination for IPv6 (#29801) enode.Node has separate accessor functions for getting the IP, UDP port and TCP port. These methods performed separate checks for attributes set in the ENR. With this PR, the accessor methods will now return cached information, and the endpoint is determined when the node is created. The logic to determine the preferred endpoint is now more correct, and considers how 'global' each address is when both IPv4 and IPv6 addresses are present in the ENR. --- p2p/enode/idscheme.go | 2 +- p2p/enode/node.go | 137 +++++++++++++++++++++++++--------- p2p/enode/node_test.go | 162 +++++++++++++++++++++++++++++++++++++++++ p2p/enode/nodedb.go | 12 +-- p2p/enode/urlv4.go | 2 +- p2p/enr/entries.go | 55 ++++++++++++++ 6 files changed, 330 insertions(+), 40 deletions(-) diff --git a/p2p/enode/idscheme.go b/p2p/enode/idscheme.go index 5936f1bfe0..fa32424fda 100644 --- a/p2p/enode/idscheme.go +++ b/p2p/enode/idscheme.go @@ -157,5 +157,5 @@ func SignNull(r *enr.Record, id ID) *Node { if err := r.SetSig(NullID{}, []byte{}); err != nil { panic(err) } - return &Node{r: *r, id: id} + return newNodeWithID(r, id) } diff --git a/p2p/enode/node.go b/p2p/enode/node.go index 592cba76fa..f468e52019 100644 --- a/p2p/enode/node.go +++ b/p2p/enode/node.go @@ -24,6 +24,7 @@ import ( "fmt" "math/bits" "net" + "net/netip" "strings" "github.com/CortexFoundation/CortexTheseus/p2p/enr" @@ -36,6 +37,10 @@ var errMissingPrefix = errors.New("missing 'enr:' prefix for base64-encoded reco type Node struct { r enr.Record id ID + // endpoint information + ip netip.Addr + udp uint16 + tcp uint16 } // New wraps a node record. The record must be valid according to the given @@ -44,11 +49,76 @@ func New(validSchemes enr.IdentityScheme, r *enr.Record) (*Node, error) { if err := r.VerifySignature(validSchemes); err != nil { return nil, err } - node := &Node{r: *r} - if n := copy(node.id[:], validSchemes.NodeAddr(&node.r)); n != len(ID{}) { - return nil, fmt.Errorf("invalid node ID length %d, need %d", n, len(ID{})) + var id ID + if n := copy(id[:], validSchemes.NodeAddr(r)); n != len(id) { + return nil, fmt.Errorf("invalid node ID length %d, need %d", n, len(id)) + } + return newNodeWithID(r, id), nil +} + +func newNodeWithID(r *enr.Record, id ID) *Node { + n := &Node{r: *r, id: id} + // Set the preferred endpoint. + // Here we decide between IPv4 and IPv6, choosing the 'most global' address. + var ip4 netip.Addr + var ip6 netip.Addr + n.Load((*enr.IPv4Addr)(&ip4)) + n.Load((*enr.IPv6Addr)(&ip6)) + valid4 := validIP(ip4) + valid6 := validIP(ip6) + switch { + case valid4 && valid6: + if localityScore(ip4) >= localityScore(ip6) { + n.setIP4(ip4) + } else { + n.setIP6(ip6) + } + case valid4: + n.setIP4(ip4) + case valid6: + n.setIP6(ip6) + } + return n +} + +// validIP reports whether 'ip' is a valid node endpoint IP address. +func validIP(ip netip.Addr) bool { + return ip.IsValid() && !ip.IsMulticast() +} + +func localityScore(ip netip.Addr) int { + switch { + case ip.IsUnspecified(): + return 0 + case ip.IsLoopback(): + return 1 + case ip.IsLinkLocalUnicast(): + return 2 + case ip.IsPrivate(): + return 3 + default: + return 4 + } +} + +func (n *Node) setIP4(ip netip.Addr) { + n.ip = ip + n.Load((*enr.UDP)(&n.udp)) + n.Load((*enr.TCP)(&n.tcp)) +} + +func (n *Node) setIP6(ip netip.Addr) { + if ip.Is4In6() { + n.setIP4(ip) + return + } + n.ip = ip + if err := n.Load((*enr.UDP6)(&n.udp)); err != nil { + n.Load((*enr.UDP)(&n.udp)) + } + if err := n.Load((*enr.TCP6)(&n.tcp)); err != nil { + n.Load((*enr.TCP)(&n.tcp)) } - return node, nil } // MustParse parses a node record or enode:// URL. It panics if the input is invalid. @@ -89,43 +159,45 @@ func (n *Node) Seq() uint64 { return n.r.Seq() } -// Incomplete returns true for nodes with no IP address. -func (n *Node) Incomplete() bool { - return n.IP() == nil -} - // Load retrieves an entry from the underlying record. func (n *Node) Load(k enr.Entry) error { return n.r.Load(k) } -// IP returns the IP address of the node. This prefers IPv4 addresses. +// IP returns the IP address of the node. func (n *Node) IP() net.IP { - var ( - ip4 enr.IPv4 - ip6 enr.IPv6 - ) - if n.Load(&ip4) == nil { - return net.IP(ip4) - } - if n.Load(&ip6) == nil { - return net.IP(ip6) - } - return nil + return net.IP(n.ip.AsSlice()) +} + +// IPAddr returns the IP address of the node. +func (n *Node) IPAddr() netip.Addr { + return n.ip } // UDP returns the UDP port of the node. func (n *Node) UDP() int { - var port enr.UDP - n.Load(&port) - return int(port) + return int(n.udp) } // TCP returns the TCP port of the node. func (n *Node) TCP() int { - var port enr.TCP - n.Load(&port) - return int(port) + return int(n.tcp) +} + +// UDPEndpoint returns the announced TCP endpoint. +func (n *Node) UDPEndpoint() (netip.AddrPort, bool) { + if !n.ip.IsValid() || n.ip.IsUnspecified() || n.udp == 0 { + return netip.AddrPort{}, false + } + return netip.AddrPortFrom(n.ip, n.udp), true +} + +// TCPEndpoint returns the announced TCP endpoint. +func (n *Node) TCPEndpoint() (netip.AddrPort, bool) { + if !n.ip.IsValid() || n.ip.IsUnspecified() || n.tcp == 0 { + return netip.AddrPort{}, false + } + return netip.AddrPortFrom(n.ip, n.udp), true } // Pubkey returns the secp256k1 public key of the node, if present. @@ -147,16 +219,15 @@ func (n *Node) Record() *enr.Record { // ValidateComplete checks whether n has a valid IP and UDP port. // Deprecated: don't use this method. func (n *Node) ValidateComplete() error { - if n.Incomplete() { + if !n.ip.IsValid() { return errors.New("missing IP address") } - if n.UDP() == 0 { - return errors.New("missing UDP port") - } - ip := n.IP() - if ip.IsMulticast() || ip.IsUnspecified() { + if n.ip.IsMulticast() || n.ip.IsUnspecified() { return errors.New("invalid IP (multicast/unspecified)") } + if n.udp == 0 { + return errors.New("missing UDP port") + } // Validate the node key (on curve, etc.). var key Secp256k1 return n.Load(&key) diff --git a/p2p/enode/node_test.go b/p2p/enode/node_test.go index c6ea92440e..1ef71e3148 100644 --- a/p2p/enode/node_test.go +++ b/p2p/enode/node_test.go @@ -21,6 +21,7 @@ import ( "encoding/hex" "fmt" "math/big" + "net/netip" "testing" "testing/quick" @@ -64,6 +65,167 @@ func TestPythonInterop(t *testing.T) { } } +func TestNodeEndpoints(t *testing.T) { + id := HexID("00000000000000806ad9b61fa5ae014307ebdc964253adcd9f2c0a392aa11abc") + type endpointTest struct { + name string + node *Node + wantIP netip.Addr + wantUDP int + wantTCP int + } + tests := []endpointTest{ + { + name: "no-addr", + node: func() *Node { + var r enr.Record + return SignNull(&r, id) + }(), + }, + { + name: "udp-only", + node: func() *Node { + var r enr.Record + r.Set(enr.UDP(9000)) + return SignNull(&r, id) + }(), + }, + { + name: "tcp-only", + node: func() *Node { + var r enr.Record + r.Set(enr.TCP(9000)) + return SignNull(&r, id) + }(), + }, + { + name: "ipv4-only-loopback", + node: func() *Node { + var r enr.Record + r.Set(enr.IPv4Addr(netip.MustParseAddr("127.0.0.1"))) + return SignNull(&r, id) + }(), + wantIP: netip.MustParseAddr("127.0.0.1"), + }, + { + name: "ipv4-only-unspecified", + node: func() *Node { + var r enr.Record + r.Set(enr.IPv4Addr(netip.MustParseAddr("0.0.0.0"))) + return SignNull(&r, id) + }(), + wantIP: netip.MustParseAddr("0.0.0.0"), + }, + { + name: "ipv4-only", + node: func() *Node { + var r enr.Record + r.Set(enr.IPv4Addr(netip.MustParseAddr("99.22.33.1"))) + return SignNull(&r, id) + }(), + wantIP: netip.MustParseAddr("99.22.33.1"), + }, + { + name: "ipv6-only", + node: func() *Node { + var r enr.Record + r.Set(enr.IPv6Addr(netip.MustParseAddr("2001::ff00:0042:8329"))) + return SignNull(&r, id) + }(), + wantIP: netip.MustParseAddr("2001::ff00:0042:8329"), + }, + { + name: "ipv4-loopback-and-ipv6-global", + node: func() *Node { + var r enr.Record + r.Set(enr.IPv4Addr(netip.MustParseAddr("127.0.0.1"))) + r.Set(enr.UDP(30304)) + r.Set(enr.IPv6Addr(netip.MustParseAddr("2001::ff00:0042:8329"))) + r.Set(enr.UDP6(30306)) + return SignNull(&r, id) + }(), + wantIP: netip.MustParseAddr("2001::ff00:0042:8329"), + wantUDP: 30306, + }, + { + name: "ipv4-unspecified-and-ipv6-loopback", + node: func() *Node { + var r enr.Record + r.Set(enr.IPv4Addr(netip.MustParseAddr("0.0.0.0"))) + r.Set(enr.IPv6Addr(netip.MustParseAddr("::1"))) + return SignNull(&r, id) + }(), + wantIP: netip.MustParseAddr("::1"), + }, + { + name: "ipv4-private-and-ipv6-global", + node: func() *Node { + var r enr.Record + r.Set(enr.IPv4Addr(netip.MustParseAddr("192.168.2.2"))) + r.Set(enr.UDP(30304)) + r.Set(enr.IPv6Addr(netip.MustParseAddr("2001::ff00:0042:8329"))) + r.Set(enr.UDP6(30306)) + return SignNull(&r, id) + }(), + wantIP: netip.MustParseAddr("2001::ff00:0042:8329"), + wantUDP: 30306, + }, + { + name: "ipv4-local-and-ipv6-global", + node: func() *Node { + var r enr.Record + r.Set(enr.IPv4Addr(netip.MustParseAddr("169.254.2.6"))) + r.Set(enr.UDP(30304)) + r.Set(enr.IPv6Addr(netip.MustParseAddr("2001::ff00:0042:8329"))) + r.Set(enr.UDP6(30306)) + return SignNull(&r, id) + }(), + wantIP: netip.MustParseAddr("2001::ff00:0042:8329"), + wantUDP: 30306, + }, + { + name: "ipv4-private-and-ipv6-private", + node: func() *Node { + var r enr.Record + r.Set(enr.IPv4Addr(netip.MustParseAddr("192.168.2.2"))) + r.Set(enr.UDP(30304)) + r.Set(enr.IPv6Addr(netip.MustParseAddr("fd00::abcd:1"))) + r.Set(enr.UDP6(30306)) + return SignNull(&r, id) + }(), + wantIP: netip.MustParseAddr("192.168.2.2"), + wantUDP: 30304, + }, + { + name: "ipv4-private-and-ipv6-link-local", + node: func() *Node { + var r enr.Record + r.Set(enr.IPv4Addr(netip.MustParseAddr("192.168.2.2"))) + r.Set(enr.UDP(30304)) + r.Set(enr.IPv6Addr(netip.MustParseAddr("fe80::1"))) + r.Set(enr.UDP6(30306)) + return SignNull(&r, id) + }(), + wantIP: netip.MustParseAddr("192.168.2.2"), + wantUDP: 30304, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if test.wantIP != test.node.IPAddr() { + t.Errorf("node has wrong IP %v, want %v", test.node.IPAddr(), test.wantIP) + } + if test.wantUDP != test.node.UDP() { + t.Errorf("node has wrong UDP port %d, want %d", test.node.UDP(), test.wantUDP) + } + if test.wantTCP != test.node.TCP() { + t.Errorf("node has wrong TCP port %d, want %d", test.node.TCP(), test.wantTCP) + } + }) + } +} + func TestHexID(t *testing.T) { ref := ID{0, 0, 0, 0, 0, 0, 0, 128, 106, 217, 182, 31, 165, 174, 1, 67, 7, 235, 220, 150, 66, 83, 173, 205, 159, 44, 10, 57, 42, 161, 26, 188} id1 := HexID("0x00000000000000806ad9b61fa5ae014307ebdc964253adcd9f2c0a392aa11abc") diff --git a/p2p/enode/nodedb.go b/p2p/enode/nodedb.go index d9eb0f6007..455a479db9 100644 --- a/p2p/enode/nodedb.go +++ b/p2p/enode/nodedb.go @@ -26,6 +26,7 @@ import ( "sync" "time" + "github.com/CortexFoundation/CortexTheseus/p2p/enr" "github.com/CortexFoundation/CortexTheseus/rlp" "github.com/syndtr/goleveldb/leveldb" "github.com/syndtr/goleveldb/leveldb/errors" @@ -242,13 +243,14 @@ func (db *DB) Node(id ID) *Node { } func mustDecodeNode(id, data []byte) *Node { - node := new(Node) - if err := rlp.DecodeBytes(data, &node.r); err != nil { + var r enr.Record + if err := rlp.DecodeBytes(data, &r); err != nil { panic(fmt.Errorf("p2p/enode: can't decode node %x in DB: %v", id, err)) } - // Restore node id cache. - copy(node.id[:], id) - return node + if len(id) != len(ID{}) { + panic(fmt.Errorf("invalid id length %d", len(id))) + } + return newNodeWithID(&r, ID(id)) } // UpdateNode inserts - potentially overwriting - a node into the peer database. diff --git a/p2p/enode/urlv4.go b/p2p/enode/urlv4.go index 3fab45c5b7..31b4f113b4 100644 --- a/p2p/enode/urlv4.go +++ b/p2p/enode/urlv4.go @@ -181,7 +181,7 @@ func (n *Node) URLv4() string { nodeid = fmt.Sprintf("%s.%x", scheme, n.id[:]) } u := url.URL{Scheme: "enode"} - if n.Incomplete() { + if !n.ip.IsValid() { u.Host = nodeid } else { addr := net.TCPAddr{IP: n.IP(), Port: n.TCP()} diff --git a/p2p/enr/entries.go b/p2p/enr/entries.go index 2bc4d7c3da..e8c9b1c214 100644 --- a/p2p/enr/entries.go +++ b/p2p/enr/entries.go @@ -21,6 +21,7 @@ import ( "fmt" "io" "net" + "net/netip" "github.com/CortexFoundation/CortexTheseus/rlp" ) @@ -167,6 +168,60 @@ func (v *IPv6) DecodeRLP(s *rlp.Stream) error { return nil } +// IPv4Addr is the "ip" key, which holds the IP address of the node. +type IPv4Addr netip.Addr + +func (v IPv4Addr) ENRKey() string { return "ip" } + +// EncodeRLP implements rlp.Encoder. +func (v IPv4Addr) EncodeRLP(w io.Writer) error { + addr := netip.Addr(v) + if !addr.Is4() { + return fmt.Errorf("address is not IPv4") + } + enc := rlp.NewEncoderBuffer(w) + bytes := addr.As4() + enc.WriteBytes(bytes[:]) + return enc.Flush() +} + +// DecodeRLP implements rlp.Decoder. +func (v *IPv4Addr) DecodeRLP(s *rlp.Stream) error { + var bytes [4]byte + if err := s.ReadBytes(bytes[:]); err != nil { + return err + } + *v = IPv4Addr(netip.AddrFrom4(bytes)) + return nil +} + +// IPv6Addr is the "ip6" key, which holds the IP address of the node. +type IPv6Addr netip.Addr + +func (v IPv6Addr) ENRKey() string { return "ip6" } + +// EncodeRLP implements rlp.Encoder. +func (v IPv6Addr) EncodeRLP(w io.Writer) error { + addr := netip.Addr(v) + if !addr.Is6() { + return fmt.Errorf("address is not IPv6") + } + enc := rlp.NewEncoderBuffer(w) + bytes := addr.As16() + enc.WriteBytes(bytes[:]) + return enc.Flush() +} + +// DecodeRLP implements rlp.Decoder. +func (v *IPv6Addr) DecodeRLP(s *rlp.Stream) error { + var bytes [16]byte + if err := s.ReadBytes(bytes[:]); err != nil { + return err + } + *v = IPv6Addr(netip.AddrFrom16(bytes)) + return nil +} + // KeyError is an error related to a key. type KeyError struct { Key string