Skip to content

Commit 1fd2d2f

Browse files
fjljakub-freebit
fjl
authored and
jakub-freebit
committed
p2p/enode: fix endpoint determination for IPv6 (ethereum#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.
1 parent a417615 commit 1fd2d2f

File tree

6 files changed

+334
-40
lines changed

6 files changed

+334
-40
lines changed

‎p2p/enode/idscheme.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,5 +157,5 @@ func SignNull(r *enr.Record, id ID) *Node {
157157
if err := r.SetSig(NullID{}, []byte{}); err != nil {
158158
panic(err)
159159
}
160-
return &Node{r: *r, id: id}
160+
return newNodeWithID(r, id)
161161
}

‎p2p/enode/node.go‎

Lines changed: 104 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"fmt"
2525
"math/bits"
2626
"net"
27+
"net/netip"
2728
"strings"
2829

2930
"github.com/ethereum/go-ethereum/p2p/enr"
@@ -36,6 +37,10 @@ var errMissingPrefix = errors.New("missing 'enr:' prefix for base64-encoded reco
3637
type Node struct {
3738
r enr.Record
3839
id ID
40+
// endpoint information
41+
ip netip.Addr
42+
udp uint16
43+
tcp uint16
3944
}
4045

4146
// 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) {
4449
if err := r.VerifySignature(validSchemes); err != nil {
4550
return nil, err
4651
}
47-
node := &Node{r: *r}
48-
if n := copy(node.id[:], validSchemes.NodeAddr(&node.r)); n != len(ID{}) {
49-
return nil, fmt.Errorf("invalid node ID length %d, need %d", n, len(ID{}))
52+
var id ID
53+
if n := copy(id[:], validSchemes.NodeAddr(r)); n != len(id) {
54+
return nil, fmt.Errorf("invalid node ID length %d, need %d", n, len(id))
55+
}
56+
return newNodeWithID(r, id), nil
57+
}
58+
59+
func newNodeWithID(r *enr.Record, id ID) *Node {
60+
n := &Node{r: *r, id: id}
61+
// Set the preferred endpoint.
62+
// Here we decide between IPv4 and IPv6, choosing the 'most global' address.
63+
var ip4 netip.Addr
64+
var ip6 netip.Addr
65+
n.Load((*enr.IPv4Addr)(&ip4))
66+
n.Load((*enr.IPv6Addr)(&ip6))
67+
valid4 := validIP(ip4)
68+
valid6 := validIP(ip6)
69+
switch {
70+
case valid4 && valid6:
71+
if localityScore(ip4) >= localityScore(ip6) {
72+
n.setIP4(ip4)
73+
} else {
74+
n.setIP6(ip6)
75+
}
76+
case valid4:
77+
n.setIP4(ip4)
78+
case valid6:
79+
n.setIP6(ip6)
80+
}
81+
return n
82+
}
83+
84+
// validIP reports whether 'ip' is a valid node endpoint IP address.
85+
func validIP(ip netip.Addr) bool {
86+
return ip.IsValid() && !ip.IsMulticast()
87+
}
88+
89+
func localityScore(ip netip.Addr) int {
90+
switch {
91+
case ip.IsUnspecified():
92+
return 0
93+
case ip.IsLoopback():
94+
return 1
95+
case ip.IsLinkLocalUnicast():
96+
return 2
97+
case ip.IsPrivate():
98+
return 3
99+
default:
100+
return 4
101+
}
102+
}
103+
104+
func (n *Node) setIP4(ip netip.Addr) {
105+
n.ip = ip
106+
n.Load((*enr.UDP)(&n.udp))
107+
n.Load((*enr.TCP)(&n.tcp))
108+
}
109+
110+
func (n *Node) setIP6(ip netip.Addr) {
111+
if ip.Is4In6() {
112+
n.setIP4(ip)
113+
return
114+
}
115+
n.ip = ip
116+
if err := n.Load((*enr.UDP6)(&n.udp)); err != nil {
117+
n.Load((*enr.UDP)(&n.udp))
118+
}
119+
if err := n.Load((*enr.TCP6)(&n.tcp)); err != nil {
120+
n.Load((*enr.TCP)(&n.tcp))
50121
}
51-
return node, nil
52122
}
53123

54124
// MustParse parses a node record or enode:// URL. It panics if the input is invalid.
@@ -89,43 +159,45 @@ func (n *Node) Seq() uint64 {
89159
return n.r.Seq()
90160
}
91161

92-
// Incomplete returns true for nodes with no IP address.
93-
func (n *Node) Incomplete() bool {
94-
return n.IP() == nil
95-
}
96-
97162
// Load retrieves an entry from the underlying record.
98163
func (n *Node) Load(k enr.Entry) error {
99164
return n.r.Load(k)
100165
}
101166

102-
// IP returns the IP address of the node. This prefers IPv4 addresses.
167+
// IP returns the IP address of the node.
103168
func (n *Node) IP() net.IP {
104-
var (
105-
ip4 enr.IPv4
106-
ip6 enr.IPv6
107-
)
108-
if n.Load(&ip4) == nil {
109-
return net.IP(ip4)
110-
}
111-
if n.Load(&ip6) == nil {
112-
return net.IP(ip6)
113-
}
114-
return nil
169+
return net.IP(n.ip.AsSlice())
170+
}
171+
172+
// IPAddr returns the IP address of the node.
173+
func (n *Node) IPAddr() netip.Addr {
174+
return n.ip
115175
}
116176

117177
// UDP returns the UDP port of the node.
118178
func (n *Node) UDP() int {
119-
var port enr.UDP
120-
n.Load(&port)
121-
return int(port)
179+
return int(n.udp)
122180
}
123181

124182
// TCP returns the TCP port of the node.
125183
func (n *Node) TCP() int {
126-
var port enr.TCP
127-
n.Load(&port)
128-
return int(port)
184+
return int(n.tcp)
185+
}
186+
187+
// UDPEndpoint returns the announced TCP endpoint.
188+
func (n *Node) UDPEndpoint() (netip.AddrPort, bool) {
189+
if !n.ip.IsValid() || n.ip.IsUnspecified() || n.udp == 0 {
190+
return netip.AddrPort{}, false
191+
}
192+
return netip.AddrPortFrom(n.ip, n.udp), true
193+
}
194+
195+
// TCPEndpoint returns the announced TCP endpoint.
196+
func (n *Node) TCPEndpoint() (netip.AddrPort, bool) {
197+
if !n.ip.IsValid() || n.ip.IsUnspecified() || n.tcp == 0 {
198+
return netip.AddrPort{}, false
199+
}
200+
return netip.AddrPortFrom(n.ip, n.udp), true
129201
}
130202

131203
// Pubkey returns the secp256k1 public key of the node, if present.
@@ -147,16 +219,15 @@ func (n *Node) Record() *enr.Record {
147219
// ValidateComplete checks whether n has a valid IP and UDP port.
148220
// Deprecated: don't use this method.
149221
func (n *Node) ValidateComplete() error {
150-
if n.Incomplete() {
222+
if !n.ip.IsValid() {
151223
return errors.New("missing IP address")
152224
}
153-
if n.UDP() == 0 {
154-
return errors.New("missing UDP port")
155-
}
156-
ip := n.IP()
157-
if ip.IsMulticast() || ip.IsUnspecified() {
225+
if n.ip.IsMulticast() || n.ip.IsUnspecified() {
158226
return errors.New("invalid IP (multicast/unspecified)")
159227
}
228+
if n.udp == 0 {
229+
return errors.New("missing UDP port")
230+
}
160231
// Validate the node key (on curve, etc.).
161232
var key Secp256k1
162233
return n.Load(&key)

‎p2p/enode/node_test.go‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"encoding/hex"
2222
"fmt"
2323
"math/big"
24+
"net/netip"
2425
"testing"
2526
"testing/quick"
2627

@@ -64,6 +65,167 @@ func TestPythonInterop(t *testing.T) {
6465
}
6566
}
6667

68+
func TestNodeEndpoints(t *testing.T) {
69+
id := HexID("00000000000000806ad9b61fa5ae014307ebdc964253adcd9f2c0a392aa11abc")
70+
type endpointTest struct {
71+
name string
72+
node *Node
73+
wantIP netip.Addr
74+
wantUDP int
75+
wantTCP int
76+
}
77+
tests := []endpointTest{
78+
{
79+
name: "no-addr",
80+
node: func() *Node {
81+
var r enr.Record
82+
return SignNull(&r, id)
83+
}(),
84+
},
85+
{
86+
name: "udp-only",
87+
node: func() *Node {
88+
var r enr.Record
89+
r.Set(enr.UDP(9000))
90+
return SignNull(&r, id)
91+
}(),
92+
},
93+
{
94+
name: "tcp-only",
95+
node: func() *Node {
96+
var r enr.Record
97+
r.Set(enr.TCP(9000))
98+
return SignNull(&r, id)
99+
}(),
100+
},
101+
{
102+
name: "ipv4-only-loopback",
103+
node: func() *Node {
104+
var r enr.Record
105+
r.Set(enr.IPv4Addr(netip.MustParseAddr("127.0.0.1")))
106+
return SignNull(&r, id)
107+
}(),
108+
wantIP: netip.MustParseAddr("127.0.0.1"),
109+
},
110+
{
111+
name: "ipv4-only-unspecified",
112+
node: func() *Node {
113+
var r enr.Record
114+
r.Set(enr.IPv4Addr(netip.MustParseAddr("0.0.0.0")))
115+
return SignNull(&r, id)
116+
}(),
117+
wantIP: netip.MustParseAddr("0.0.0.0"),
118+
},
119+
{
120+
name: "ipv4-only",
121+
node: func() *Node {
122+
var r enr.Record
123+
r.Set(enr.IPv4Addr(netip.MustParseAddr("99.22.33.1")))
124+
return SignNull(&r, id)
125+
}(),
126+
wantIP: netip.MustParseAddr("99.22.33.1"),
127+
},
128+
{
129+
name: "ipv6-only",
130+
node: func() *Node {
131+
var r enr.Record
132+
r.Set(enr.IPv6Addr(netip.MustParseAddr("2001::ff00:0042:8329")))
133+
return SignNull(&r, id)
134+
}(),
135+
wantIP: netip.MustParseAddr("2001::ff00:0042:8329"),
136+
},
137+
{
138+
name: "ipv4-loopback-and-ipv6-global",
139+
node: func() *Node {
140+
var r enr.Record
141+
r.Set(enr.IPv4Addr(netip.MustParseAddr("127.0.0.1")))
142+
r.Set(enr.UDP(30304))
143+
r.Set(enr.IPv6Addr(netip.MustParseAddr("2001::ff00:0042:8329")))
144+
r.Set(enr.UDP6(30306))
145+
return SignNull(&r, id)
146+
}(),
147+
wantIP: netip.MustParseAddr("2001::ff00:0042:8329"),
148+
wantUDP: 30306,
149+
},
150+
{
151+
name: "ipv4-unspecified-and-ipv6-loopback",
152+
node: func() *Node {
153+
var r enr.Record
154+
r.Set(enr.IPv4Addr(netip.MustParseAddr("0.0.0.0")))
155+
r.Set(enr.IPv6Addr(netip.MustParseAddr("::1")))
156+
return SignNull(&r, id)
157+
}(),
158+
wantIP: netip.MustParseAddr("::1"),
159+
},
160+
{
161+
name: "ipv4-private-and-ipv6-global",
162+
node: func() *Node {
163+
var r enr.Record
164+
r.Set(enr.IPv4Addr(netip.MustParseAddr("192.168.2.2")))
165+
r.Set(enr.UDP(30304))
166+
r.Set(enr.IPv6Addr(netip.MustParseAddr("2001::ff00:0042:8329")))
167+
r.Set(enr.UDP6(30306))
168+
return SignNull(&r, id)
169+
}(),
170+
wantIP: netip.MustParseAddr("2001::ff00:0042:8329"),
171+
wantUDP: 30306,
172+
},
173+
{
174+
name: "ipv4-local-and-ipv6-global",
175+
node: func() *Node {
176+
var r enr.Record
177+
r.Set(enr.IPv4Addr(netip.MustParseAddr("169.254.2.6")))
178+
r.Set(enr.UDP(30304))
179+
r.Set(enr.IPv6Addr(netip.MustParseAddr("2001::ff00:0042:8329")))
180+
r.Set(enr.UDP6(30306))
181+
return SignNull(&r, id)
182+
}(),
183+
wantIP: netip.MustParseAddr("2001::ff00:0042:8329"),
184+
wantUDP: 30306,
185+
},
186+
{
187+
name: "ipv4-private-and-ipv6-private",
188+
node: func() *Node {
189+
var r enr.Record
190+
r.Set(enr.IPv4Addr(netip.MustParseAddr("192.168.2.2")))
191+
r.Set(enr.UDP(30304))
192+
r.Set(enr.IPv6Addr(netip.MustParseAddr("fd00::abcd:1")))
193+
r.Set(enr.UDP6(30306))
194+
return SignNull(&r, id)
195+
}(),
196+
wantIP: netip.MustParseAddr("192.168.2.2"),
197+
wantUDP: 30304,
198+
},
199+
{
200+
name: "ipv4-private-and-ipv6-link-local",
201+
node: func() *Node {
202+
var r enr.Record
203+
r.Set(enr.IPv4Addr(netip.MustParseAddr("192.168.2.2")))
204+
r.Set(enr.UDP(30304))
205+
r.Set(enr.IPv6Addr(netip.MustParseAddr("fe80::1")))
206+
r.Set(enr.UDP6(30306))
207+
return SignNull(&r, id)
208+
}(),
209+
wantIP: netip.MustParseAddr("192.168.2.2"),
210+
wantUDP: 30304,
211+
},
212+
}
213+
214+
for _, test := range tests {
215+
t.Run(test.name, func(t *testing.T) {
216+
if test.wantIP != test.node.IPAddr() {
217+
t.Errorf("node has wrong IP %v, want %v", test.node.IPAddr(), test.wantIP)
218+
}
219+
if test.wantUDP != test.node.UDP() {
220+
t.Errorf("node has wrong UDP port %d, want %d", test.node.UDP(), test.wantUDP)
221+
}
222+
if test.wantTCP != test.node.TCP() {
223+
t.Errorf("node has wrong TCP port %d, want %d", test.node.TCP(), test.wantTCP)
224+
}
225+
})
226+
}
227+
}
228+
67229
func TestHexID(t *testing.T) {
68230
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}
69231
id1 := HexID("0x00000000000000806ad9b61fa5ae014307ebdc964253adcd9f2c0a392aa11abc")

0 commit comments

Comments
 (0)