From da4e59d392305aad92c9adf33bc69dc29333873f Mon Sep 17 00:00:00 2001 From: Lucas Vasconcelos Date: Wed, 4 Dec 2024 12:44:26 +0100 Subject: [PATCH] p2p: add dynamic name resolution for enodes --- p2p/dial.go | 88 +++++++++++++++++-------------- p2p/enode/node.go | 114 +++++++++------------------------------- p2p/enode/node_test.go | 40 ++------------ p2p/enode/urlv4.go | 85 ++++++++++++++++++------------ p2p/enode/urlv4_test.go | 18 +++---- p2p/server.go | 14 ----- 6 files changed, 138 insertions(+), 221 deletions(-) diff --git a/p2p/dial.go b/p2p/dial.go index 9752397f03..8ce736ad40 100644 --- a/p2p/dial.go +++ b/p2p/dial.go @@ -77,6 +77,7 @@ var ( errRecentlyDialed = errors.New("recently dialed") errNetRestrict = errors.New("not contained in netrestrict list") errNoPort = errors.New("node does not provide TCP port") + errNoResolvedIP = errors.New("node does not provide a resolved IP") ) // dialer creates outbound connections and submits them into Server. @@ -135,7 +136,6 @@ type dialConfig struct { log log.Logger clock mclock.Clock rand *mrand.Rand - ttl time.Duration } func (cfg dialConfig) withDefaults() dialConfig { @@ -275,15 +275,13 @@ loop: case node := <-d.addStaticCh: id := node.ID() _, exists := d.static[id] - d.log.Trace("Adding static node", "id", id, "addr", node.DisplayAddr(), "ip", node.IPAddr(), "added", !exists) + d.log.Trace("Adding static node", "id", id, "endpoint", node.Endpoint(), "added", !exists) if exists { continue loop } task := newDialTask(node, staticDialedConn) d.static[id] = task - if err := d.checkDial(node); err != nil { - d.log.Trace("Discarding dial candidate", "id", node.ID(), "addr", node.DisplayAddr(), "ip", node.IPAddr(), "reason", err) - } else { + if d.checkDial(node) == nil { d.addToStaticPool(task) } @@ -436,10 +434,42 @@ func (d *dialScheduler) removeFromStaticPool(idx int) { task.staticPoolIndex = -1 } +func (d *dialScheduler) resolve(n *enode.Node) (*enode.Node, error) { + if n.NeedResolve() { + d.log.Debug("Attempting DNS resolution", "id", n.ID(), "name", n.Hostname()) + ips, err := net.LookupIP(n.Hostname()) + if err != nil { + d.log.Debug("DNS resolution failed", "id", n.ID(), "name", n.Hostname(), "err", err) + return n, err + } + d.log.Debug("DNS lookup succeeded", "id", n.ID(), "name", n.Hostname(), "ipcount", len(ips)) + + // Try IPv4 first + for _, ip := range ips { + if ip4 := ip.To4(); ip4 != nil { + resolved := enode.NewV4WithDNS(n.Pubkey(), ip4, n.Hostname(), n.TCP(), n.UDP()) + d.log.Debug("DNS resolved to IPv4", "id", n.ID(), "name", n.Hostname(), "ip", ip4) + return resolved, nil + } + } + // Then try IPv6 + for _, ip := range ips { + if ip6 := ip.To16(); ip6 != nil { + resolved := enode.NewV4WithDNS(n.Pubkey(), ip6, n.Hostname(), n.TCP(), n.UDP()) + d.log.Debug("DNS resolved to IPv6", "id", n.ID(), "name", n.Hostname(), "ip", ip6) + return resolved, nil + } + } + d.log.Debug("DNS resolution found no usable IPs", "id", n.ID(), "name", n.Hostname()) + return n, errNoResolvedIP + } + return n, nil +} + // startDial runs the given dial task in a separate goroutine. func (d *dialScheduler) startDial(task *dialTask) { node := task.dest() - d.log.Trace("Starting p2p dial", "id", node.ID(), "addr", node.DisplayAddr(), "ip", node.IPAddr(), "flag", task.flags) + d.log.Trace("Starting p2p dial", "id", node.ID(), "endpoint", node.Endpoint(), "flag", task.flags) hkey := string(node.ID().Bytes()) d.history.add(hkey, d.clock.Now().Add(dialHistoryExpiration)) d.dialing[node.ID()] = task @@ -476,7 +506,16 @@ func (t *dialTask) dest() *enode.Node { } func (t *dialTask) run(d *dialScheduler) { - if !t.resolveIfNeeded(d) { + node := t.dest() + if node.NeedResolve() { + resolved, err := d.resolve(node) + if err != nil { + return + } + t.destPtr.Store(resolved) + } + + if t.needResolve() && !t.resolve(d) { return } @@ -491,36 +530,8 @@ func (t *dialTask) run(d *dialScheduler) { } } -// resolveIfNeeded attempts to resolve the node's IP address if it is invalid. -// It returns true if the node's IP address is valid after resolution attempts. -func (t *dialTask) resolveIfNeeded(d *dialScheduler) bool { - node := t.dest() - - if t.flags&staticDialedConn != 0 { - if t.resolve(d) && node.IPAddr().IsValid() { - return true - } - } - - if node.NeedsDNSResolve() { - if t.resolveDNS(d) && node.IPAddr().IsValid() { - return true - } - } - - return false -} - -// resolveDNS attempts to resolve the DNS name of the destination node. -// It returns true if resolution succeeds. -func (t *dialTask) resolveDNS(d *dialScheduler) bool { - node := t.dest() - d.log.Trace("Starting DNS resolution", "id", node.ID(), "addr", node.DisplayAddr()) - if err := node.RefreshDNS(d.dialConfig.ttl); err != nil { - d.log.Trace("DNS resolution failed", "id", node.ID(), "addr", node.DisplayAddr()) - return false - } - return true +func (t *dialTask) needResolve() bool { + return t.flags&staticDialedConn != 0 && !t.dest().IPAddr().IsValid() } // resolve attempts to find the current endpoint for the destination @@ -564,7 +575,8 @@ func (t *dialTask) dial(d *dialScheduler, dest *enode.Node) error { dialMeter.Mark(1) fd, err := d.dialer.Dial(d.ctx, dest) if err != nil { - d.log.Trace("Dial error", "id", dest.ID(), "addr", dest.DisplayAddr(), "ip", dest.IPAddr(), "conn", t.flags, "err", cleanupDialErr(err)) + addr, _ := dest.TCPEndpoint() + d.log.Trace("Dial error", "id", dest.ID(), "addr", addr, "conn", t.flags, "err", cleanupDialErr(err)) dialConnectionError.Mark(1) return &dialError{err} } diff --git a/p2p/enode/node.go b/p2p/enode/node.go index ad1aaae558..945343b29d 100644 --- a/p2p/enode/node.go +++ b/p2p/enode/node.go @@ -22,14 +22,12 @@ import ( "encoding/hex" "errors" "fmt" + "github.com/ethereum/go-ethereum/p2p/enr" + "github.com/ethereum/go-ethereum/rlp" "math/bits" "net" "net/netip" "strings" - "time" - - "github.com/ethereum/go-ethereum/p2p/enr" - "github.com/ethereum/go-ethereum/rlp" ) var errMissingPrefix = errors.New("missing 'enr:' prefix for base64-encoded record") @@ -39,13 +37,10 @@ type Node struct { r enr.Record id ID // endpoint information - ip netip.Addr - udp uint16 - tcp uint16 - // dns information - dnsName string - dnsResolved time.Time - dnsTTL time.Duration + ip netip.Addr + udp uint16 + tcp uint16 + hostname string } // New wraps a node record. The record must be valid according to the given @@ -189,6 +184,22 @@ func (n *Node) TCP() int { return int(n.tcp) } +// Endpoint returns the hostname of the node if set, otherwise the IP address. +func (n *Node) Endpoint() string { + if n.hostname != "" { + return n.hostname + } + if n.ip.IsValid() { + return n.ip.String() + } + return "" +} + +// NeedResolve checks if the node requires DNS resolution. +func (n *Node) NeedResolve() bool { + return n.hostname != "" //TODO: Add check for n.ip.IsValid(), but we need to implement invalidation for the previous resolved IP +} + // UDPEndpoint returns the announced UDP endpoint. func (n *Node) UDPEndpoint() (netip.AddrPort, bool) { if !n.ip.IsValid() || n.ip.IsUnspecified() || n.udp == 0 { @@ -234,6 +245,9 @@ func (n *Node) Record() *enr.Record { cpy := n.r return &cpy } +func (n *Node) Hostname() string { + return n.hostname +} // ValidateComplete checks whether n has a valid IP and UDP port. // Deprecated: don't use this method. @@ -262,84 +276,6 @@ func (n *Node) String() string { return "enr:" + b64 } -// resolveDNS attempts to resolve a DNS name to an IP address -func (n *Node) resolveDNS(dnsName string) (netip.Addr, error) { - ips, err := net.LookupIP(dnsName) - if err != nil { - return netip.Addr{}, err - } - for _, ip := range ips { - if ip4 := ip.To4(); ip4 != nil { - addr, ok := netip.AddrFromSlice(ip4) - if ok { - return addr, nil - } - } - } - // Fall back to IPv6 if no IPv4 is available - for _, ip := range ips { - addr, ok := netip.AddrFromSlice(ip) - if ok { - return addr, nil - } - } - - return netip.Addr{}, errors.New("no valid IP address found") -} - -// SetDNS sets the DNS name and resolves it to an IP address -func (n *Node) SetDNS(dnsName string, ttl time.Duration) error { - ip, err := n.resolveDNS(dnsName) - if err != nil { - return err - } - - n.dnsName = dnsName - n.dnsResolved = time.Now() - n.dnsTTL = ttl - - if ip.Is4() { - n.setIP4(ip) - } else { - n.setIP6(ip) - } - return nil -} - -// DNSName returns the stored DNS name -func (n *Node) DNSName() string { - return n.dnsName -} - -// RefreshDNS updates the IP address from the stored DNS name -func (n *Node) RefreshDNS(ttl time.Duration) error { - if n.dnsName == "" { - return errors.New("no DNS name set") - } - return n.SetDNS(n.dnsName, ttl) -} - -// DisplayAddr returns either "hostname:port" or "ip:port" -func (n *Node) DisplayAddr() string { - addr := n.dnsName - if addr == "" { - addr = n.ip.String() - } - return fmt.Sprintf("%s:%d", addr, n.tcp) -} - -// NeedsDNSResolve returns true if the node has a DNS name that needs resolution -func (n *Node) NeedsDNSResolve() bool { - if n.dnsName == "" { - return false - } - return !n.ip.IsValid() || time.Since(n.dnsResolved) > n.dnsTTL -} - -func (n *Node) GetTTL() time.Duration { - return n.dnsTTL -} - // MarshalText implements encoding.TextMarshaler. func (n *Node) MarshalText() ([]byte, error) { return []byte(n.String()), nil diff --git a/p2p/enode/node_test.go b/p2p/enode/node_test.go index eb52d5d6ec..7604f1c50e 100644 --- a/p2p/enode/node_test.go +++ b/p2p/enode/node_test.go @@ -274,7 +274,7 @@ func TestNodeEndpoints(t *testing.T) { node: func() *Node { var r enr.Record n := SignNull(&r, id) - n.dnsName = "example.com" + n.hostname = "example.com" n.tcp = 30303 n.udp = 30303 return n @@ -283,40 +283,6 @@ func TestNodeEndpoints(t *testing.T) { wantUDP: 30303, wantDNS: "example.com", }, - { - name: "dns-with-ports", - node: func() *Node { - var r enr.Record - r.Set(enr.TCP(9000)) - r.Set(enr.UDP(9001)) - n := SignNull(&r, id) - n.dnsName = "node.example.org" - n.tcp = 9000 - n.udp = 9001 - return n - }(), - wantTCP: 9000, - wantUDP: 9001, - wantDNS: "node.example.org", - }, - { - name: "dns-with-ip-fallback", - node: func() *Node { - var r enr.Record - r.Set(enr.IPv4Addr(netip.MustParseAddr("192.168.1.1"))) - r.Set(enr.TCP(9000)) - r.Set(enr.UDP(9000)) - n := SignNull(&r, id) - n.dnsName = "node.example.org" - n.tcp = 9000 - n.udp = 9000 - return n - }(), - wantIP: netip.MustParseAddr("192.168.1.1"), - wantTCP: 9000, - wantUDP: 9000, - wantDNS: "node.example.org", - }, } for _, test := range tests { @@ -333,8 +299,8 @@ func TestNodeEndpoints(t *testing.T) { if quic, _ := test.node.QUICEndpoint(); test.wantQUIC != int(quic.Port()) { t.Errorf("node has wrong QUIC port %d, want %d", quic.Port(), test.wantQUIC) } - if test.wantDNS != test.node.DNSName() { - t.Errorf("node has wrong DNS name %s, want %s", test.node.DNSName(), test.wantDNS) + if test.wantDNS != test.node.Hostname() { + t.Errorf("node has wrong DNS name %s, want %s", test.node.Hostname(), test.wantDNS) } }) } diff --git a/p2p/enode/urlv4.go b/p2p/enode/urlv4.go index 76b7a7cdf8..7a13039d18 100644 --- a/p2p/enode/urlv4.go +++ b/p2p/enode/urlv4.go @@ -101,17 +101,25 @@ func NewV4(pubkey *ecdsa.PublicKey, ip net.IP, tcp, udp int) *Node { return n } -func NewV4WithDNS(pubkey *ecdsa.PublicKey, ip net.IP, dnsName string, tcp, udp int) *Node { - n := NewV4(pubkey, ip, tcp, udp) - // Always set TCP/UDP ports regardless of IP - // This is to ensure that the node is always - // considered valid even if the IP is not - // set. - if len(ip) == 0 { - n.tcp = uint16(tcp) - n.udp = uint16(udp) +func NewV4WithDNS(pubkey *ecdsa.PublicKey, ip net.IP, hostname string, tcp, udp int) *Node { + var r enr.Record + if tcp != 0 { + r.Set(enr.TCP(tcp)) } - n.dnsName = dnsName + if udp != 0 { + r.Set(enr.UDP(udp)) + } + if len(ip) > 0 { + r.Set(enr.IP(ip)) + } + signV4Compat(&r, pubkey) + n, err := New(v4CompatID{}, &r) + if err != nil { + panic(err) + } + n.tcp = uint16(tcp) + n.udp = uint16(udp) + n.hostname = hostname return n } @@ -125,6 +133,7 @@ func parseComplete(rawurl string) (*Node, error) { var ( id *ecdsa.PublicKey tcpPort, udpPort uint64 + node *Node ) u, err := url.Parse(rawurl) if err != nil { @@ -140,6 +149,16 @@ func parseComplete(rawurl string) (*Node, error) { if id, err = parsePubkey(u.User.String()); err != nil { return nil, fmt.Errorf("invalid public key (%v)", err) } + + // Parse the IP address if its one. + ip := net.ParseIP(u.Hostname()) + if ip != 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(u.Port(), 10, 16); err != nil { return nil, errors.New("invalid port") } @@ -151,20 +170,14 @@ func parseComplete(rawurl string) (*Node, error) { return nil, errors.New("invalid discport in query") } } - // Check if hostname is an IP address and create node accordingly - hostname := u.Hostname() - ip := net.ParseIP(hostname) - if ip == nil { - ips, err := lookupIPFunc(hostname) - if err != nil { - return NewV4WithDNS(id, nil, hostname, int(tcpPort), int(udpPort)), nil - } - ip = ips[0] + + if ip != nil { + node = NewV4(id, ip, int(tcpPort), int(udpPort)) + } else { + node = NewV4WithDNS(id, nil, u.Hostname(), int(tcpPort), int(udpPort)) } - if ipv4 := ip.To4(); ipv4 != nil { - ip = ipv4 - } - return NewV4(id, ip, int(tcpPort), int(udpPort)), nil + + return node, nil } // parsePubkey parses a hex-encoded secp256k1 public key. @@ -194,19 +207,23 @@ func (n *Node) URLv4() string { nodeid = fmt.Sprintf("%s.%x", scheme, n.id[:]) } u := url.URL{Scheme: "enode"} - if !n.ip.IsValid() && n.dnsName == "" { - u.Host = nodeid - return u.String() - } - u.User = url.User(nodeid) - if n.dnsName != "" { - u.Host = fmt.Sprintf("%s:%d", n.dnsName, n.TCP()) - } else { + if n.NeedResolve() { + // For DNS nodes: include DNS name, TCP port, and optional UDP port + u.User = url.User(nodeid) + u.Host = fmt.Sprintf("%s:%d", n.Hostname(), n.TCP()) + if n.UDP() != n.TCP() { + u.RawQuery = "discport=" + strconv.Itoa(n.UDP()) + } + } else if n.ip.IsValid() { + // For IP-based nodes: include IP address, TCP port, and optional UDP port addr := net.TCPAddr{IP: n.IP(), Port: n.TCP()} + u.User = url.User(nodeid) u.Host = addr.String() - } - if n.UDP() != n.TCP() { - u.RawQuery = "discport=" + strconv.Itoa(n.UDP()) + if n.UDP() != n.TCP() { + u.RawQuery = "discport=" + strconv.Itoa(n.UDP()) + } + } else { + u.Host = nodeid } return u.String() } diff --git a/p2p/enode/urlv4_test.go b/p2p/enode/urlv4_test.go index 98728e6177..28d4896378 100644 --- a/p2p/enode/urlv4_test.go +++ b/p2p/enode/urlv4_test.go @@ -78,6 +78,15 @@ var parseNodeTests = []struct { input: "enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@127.0.0.1:3?discport=foo", wantError: `invalid discport in query`, }, + { + input: "enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@127.0.0.1:52150", + wantResult: NewV4( + hexPubkey("1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), + net.IP{127, 0, 0, 1}, + 52150, + 52150, + ), + }, { input: "enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@valid.:3", wantResult: NewV4WithDNS( @@ -88,15 +97,6 @@ var parseNodeTests = []struct { 3, ), }, - { - input: "enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@127.0.0.1:52150", - wantResult: NewV4( - hexPubkey("1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"), - net.IP{127, 0, 0, 1}, - 52150, - 52150, - ), - }, { input: "enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@[::]:52150", wantResult: NewV4( diff --git a/p2p/server.go b/p2p/server.go index 6b08858cae..172f0667eb 100644 --- a/p2p/server.go +++ b/p2p/server.go @@ -64,10 +64,6 @@ const ( // Maximum amount of time allowed for writing a complete message. frameWriteTimeout = 20 * time.Second - - defaultTTL = 5 * time.Minute - minTTL = 1 * time.Second - maxTTL = 24 * time.Hour ) var ( @@ -126,11 +122,6 @@ type Config struct { // allowed to connect, even above the peer limit. TrustedNodes []*enode.Node - // TTL is the time-to-live for DNS discovery queries - // This value is passed to the DNS resolvers when performing name - // resolutions. - TTL time.Duration `toml:",omitempty"` - // Connectivity can be restricted to certain IP networks. // If this option is set to a non-nil value, only hosts which match one of the // IP networks contained in the list are considered. @@ -629,11 +620,6 @@ func (srv *Server) setupDialScheduler() { if config.dialer == nil { config.dialer = tcpDialer{&net.Dialer{Timeout: defaultDialTimeout}} } - if srv.TTL >= minTTL && srv.TTL <= maxTTL { - config.ttl = srv.TTL - } else { - config.ttl = defaultTTL - } srv.dialsched = newDialScheduler(config, srv.discmix, srv.SetupConn) for _, n := range srv.StaticNodes { srv.dialsched.addStatic(n)