From f214597e14a447d4345fe63ca7e60e7c08f8753e Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 28 Jan 2026 12:01:06 +0100 Subject: [PATCH 1/4] fix(p2p): improve address validation rules --- internal/p2p/address.go | 18 +++++++++++++ internal/p2p/address_test.go | 38 +++++++++++++------------- internal/p2p/transport.go | 16 +++++++++++ types/validator_address.go | 47 +++++++++++++++++++++++++++++++++ types/validator_address_test.go | 30 +++++++++++++++++++++ 5 files changed, 130 insertions(+), 19 deletions(-) diff --git a/internal/p2p/address.go b/internal/p2p/address.go index 45224f506f..71649386bd 100644 --- a/internal/p2p/address.go +++ b/internal/p2p/address.go @@ -169,8 +169,26 @@ func (a NodeAddress) Validate() error { } else if err := a.NodeID.Validate(); err != nil { return fmt.Errorf("invalid peer ID: %w", err) } + if a.Hostname != "" { + if err := validateHostname(a.Hostname); err != nil { + return err + } + } if a.Port > 0 && a.Hostname == "" { return errors.New("cannot specify port without hostname") } return nil } + +func validateHostname(hostname string) error { + if _, _, err := net.SplitHostPort(hostname); err == nil { + return errors.New("hostname must not include port") + } + if net.ParseIP(hostname) != nil { + return nil + } + if !types.IsValidHostname(hostname) { + return errors.New("invalid hostname") + } + return nil +} diff --git a/internal/p2p/address_test.go b/internal/p2p/address_test.go index f8c0572d73..04fe530e53 100644 --- a/internal/p2p/address_test.go +++ b/internal/p2p/address_test.go @@ -134,16 +134,6 @@ func TestParseNodeAddress(t *testing.T) { p2p.NodeAddress{Protocol: "mconn", NodeID: id, Hostname: "hostname.domain", Port: 80}, true, }, - { - user + "@%F0%9F%91%8B", - p2p.NodeAddress{Protocol: "mconn", NodeID: id, Hostname: "👋"}, - true, - }, - { - user + "@%F0%9F%91%8B:80/path", - p2p.NodeAddress{Protocol: "mconn", NodeID: id, Hostname: "👋", Port: 80, Path: "/path"}, - true, - }, { user + "@127.0.0.1:26657", p2p.NodeAddress{Protocol: "mconn", NodeID: id, Hostname: "127.0.0.1", Port: 26657}, @@ -182,6 +172,8 @@ func TestParseNodeAddress(t *testing.T) { {"scheme:", p2p.NodeAddress{}, false}, {"memory:foo", p2p.NodeAddress{}, false}, {user + "@%F%F0", p2p.NodeAddress{}, false}, + {user + "@%F0%9F%91%8B", p2p.NodeAddress{}, false}, + {user + "@%F0%9F%91%8B:80/path", p2p.NodeAddress{}, false}, {"//" + user + "@127.0.0.1", p2p.NodeAddress{}, false}, {"://" + user + "@127.0.0.1", p2p.NodeAddress{}, false}, {"mconn://foo@127.0.0.1", p2p.NodeAddress{}, false}, @@ -220,13 +212,9 @@ func TestNodeAddress_Resolve(t *testing.T) { true, }, { - p2p.NodeAddress{Protocol: "tcp", Hostname: "localhost", Port: 80, Path: "/path"}, - &p2p.Endpoint{Protocol: "tcp", IP: net.IPv4(127, 0, 0, 1), Port: 80, Path: "/path"}, - true, - }, - { - p2p.NodeAddress{Protocol: "tcp", Hostname: "localhost", Port: 80, Path: "/path"}, - &p2p.Endpoint{Protocol: "tcp", IP: net.IPv6loopback, Port: 80, Path: "/path"}, + // note we don't check localhost, as it may resolve to IPv4 or IPv6 depending on the system + p2p.NodeAddress{Protocol: "tcp", Hostname: "one.one.one.one", Port: 80, Path: "/path"}, + &p2p.Endpoint{Protocol: "tcp", IP: net.IPv4(1, 1, 1, 1), Port: 80, Path: "/path"}, true, }, { @@ -287,7 +275,15 @@ func TestNodeAddress_Resolve(t *testing.T) { require.Error(t, err) return } - require.Contains(t, endpoints, tc.expect) + + for _, ep := range endpoints { + // in some cases, ip address match fails in Contains due to different representations + // e.g. IPv4-mapped IPv6 address vs IPv4 address + if ep.Equal(tc.expect) { + return + } + } + require.Contains(t, endpoints, tc.expect, "expected endpoint not found in resolved endpoints %+v", endpoints) }) } } @@ -365,14 +361,18 @@ func TestNodeAddress_Validate(t *testing.T) { {p2p.NodeAddress{Protocol: "mconn", NodeID: id, Hostname: "host", Port: 80, Path: "/path"}, true}, {p2p.NodeAddress{Protocol: "mconn", NodeID: id, Hostname: "host"}, true}, {p2p.NodeAddress{Protocol: "mconn", NodeID: id, Path: "path"}, true}, - {p2p.NodeAddress{Protocol: "mconn", NodeID: id, Hostname: "👋", Path: "👋"}, true}, + {p2p.NodeAddress{Protocol: "mconn", NodeID: id, Hostname: "google.com", Path: "👋"}, true}, // Invalid addresses. + {p2p.NodeAddress{Protocol: "mconn", NodeID: id, Hostname: "127.0.0.1:26656", Port: 26656}, false}, + {p2p.NodeAddress{Protocol: "mconn", NodeID: id, Hostname: "[127.0.0.1:26656]", Port: 26656}, false}, + {p2p.NodeAddress{Protocol: "mconn", NodeID: id, Hostname: "[127.0.0.1:26656]:26656", Port: 26656}, false}, {p2p.NodeAddress{}, false}, {p2p.NodeAddress{NodeID: "foo", Hostname: "host"}, false}, {p2p.NodeAddress{Protocol: "mconn", NodeID: id}, true}, {p2p.NodeAddress{Protocol: "mconn", NodeID: "foo", Hostname: "host"}, false}, {p2p.NodeAddress{Protocol: "mconn", NodeID: id, Port: 80, Path: "path"}, false}, + {p2p.NodeAddress{Protocol: "mconn", NodeID: id, Hostname: "👋", Path: "👋"}, false}, } for _, tc := range testcases { tc := tc diff --git a/internal/p2p/transport.go b/internal/p2p/transport.go index 1caaecbdb1..23bf7e0038 100644 --- a/internal/p2p/transport.go +++ b/internal/p2p/transport.go @@ -157,6 +157,22 @@ func (e Endpoint) NodeAddress(nodeID types.NodeID) NodeAddress { return address } +func (e Endpoint) Equal(other *Endpoint) bool { + if e.Protocol != other.Protocol { + return false + } + if !e.IP.Equal(other.IP) { + return false + } + if e.Port != other.Port { + return false + } + if e.Path != other.Path { + return false + } + return true +} + // String formats the endpoint as a URL string. func (e Endpoint) String() string { // If this is a non-networked endpoint with a valid node ID as a path, diff --git a/types/validator_address.go b/types/validator_address.go index caa74ecd15..cb8b2d3358 100644 --- a/types/validator_address.go +++ b/types/validator_address.go @@ -102,6 +102,9 @@ func (va ValidatorAddress) Validate() error { if va.Hostname == "" { return ErrNoHostname } + if err := validateHostname(va.Hostname); err != nil { + return err + } if va.Port <= 0 { return ErrNoPort } @@ -114,6 +117,50 @@ func (va ValidatorAddress) Validate() error { return nil } +func validateHostname(hostname string) error { + if _, _, err := net.SplitHostPort(hostname); err == nil { + return fmt.Errorf("hostname must not include port") + } + if net.ParseIP(hostname) != nil { + return nil + } + if !IsValidHostname(hostname) { + return fmt.Errorf("invalid hostname") + } + return nil +} + +// IsValidHostname reports whether hostname is a valid DNS name. +func IsValidHostname(hostname string) bool { + if hostname == "" { + return false + } + hostname = strings.TrimSuffix(hostname, ".") + if len(hostname) > 253 { + return false + } + labels := strings.Split(hostname, ".") + for _, label := range labels { + if len(label) == 0 || len(label) > 63 { + return false + } + if label[0] == '-' || label[len(label)-1] == '-' { + return false + } + for _, r := range label { + switch { + case r >= 'a' && r <= 'z': + case r >= 'A' && r <= 'Z': + case r >= '0' && r <= '9': + case r == '-': + default: + return false + } + } + } + return true +} + // Zero returns true if the ValidatorAddress is not initialized func (va ValidatorAddress) Zero() bool { return va.Hostname == "" && va.Port == 0 && va.NodeID == "" diff --git a/types/validator_address_test.go b/types/validator_address_test.go index 7929000587..abcb7c7172 100644 --- a/types/validator_address_test.go +++ b/types/validator_address_test.go @@ -99,6 +99,36 @@ func TestValidatorAddress_HostPortProto(t *testing.T) { wantPort: 22, wantProto: "tcp", }, + { + uri: "tcp://127.0.0.1:26656:26656", + wantError: true, + }, + { + uri: "tcp://[127.0.0.1:26656]:26656", + wantError: true, + }, + { + uri: "tcp://[127.0.0.1:26656]", + wantError: true, + }, + { + uri: "tcp://[127.0.0.1]:26656", + wantError: true, + }, + { + uri: "tcp://[::1]:26656", + wantHost: "::1", + wantPort: 26656, + wantProto: "tcp", + wantError: false, + }, + { + uri: "tcp://google.com:26656", + wantHost: "google.com", + wantPort: 26656, + wantProto: "tcp", + wantError: false, + }, { uri: "", wantError: true, From f9e6fea2d734605526cd2d79307e78f252496ee0 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 28 Jan 2026 13:09:32 +0100 Subject: [PATCH 2/4] chore: fix nil deref --- internal/p2p/transport.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/p2p/transport.go b/internal/p2p/transport.go index 23bf7e0038..cfdfc2ca57 100644 --- a/internal/p2p/transport.go +++ b/internal/p2p/transport.go @@ -158,6 +158,10 @@ func (e Endpoint) NodeAddress(nodeID types.NodeID) NodeAddress { } func (e Endpoint) Equal(other *Endpoint) bool { + if other == nil { + return false + } + if e.Protocol != other.Protocol { return false } From 6db13e12a49ff3a9705de64b3cde6c55fa47ff9a Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 11 Feb 2026 16:00:46 +0100 Subject: [PATCH 3/4] chore: minor fixes Signed-off-by: Lukasz Klimek <842586+lklimek@users.noreply.github.com> --- internal/p2p/address.go | 15 +-------------- types/validator_address.go | 10 ++++++---- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/internal/p2p/address.go b/internal/p2p/address.go index 71649386bd..8b99a5084c 100644 --- a/internal/p2p/address.go +++ b/internal/p2p/address.go @@ -170,7 +170,7 @@ func (a NodeAddress) Validate() error { return fmt.Errorf("invalid peer ID: %w", err) } if a.Hostname != "" { - if err := validateHostname(a.Hostname); err != nil { + if err := types.ValidateHostname(a.Hostname); err != nil { return err } } @@ -179,16 +179,3 @@ func (a NodeAddress) Validate() error { } return nil } - -func validateHostname(hostname string) error { - if _, _, err := net.SplitHostPort(hostname); err == nil { - return errors.New("hostname must not include port") - } - if net.ParseIP(hostname) != nil { - return nil - } - if !types.IsValidHostname(hostname) { - return errors.New("invalid hostname") - } - return nil -} diff --git a/types/validator_address.go b/types/validator_address.go index cb8b2d3358..bb1396755a 100644 --- a/types/validator_address.go +++ b/types/validator_address.go @@ -102,7 +102,7 @@ func (va ValidatorAddress) Validate() error { if va.Hostname == "" { return ErrNoHostname } - if err := validateHostname(va.Hostname); err != nil { + if err := ValidateHostname(va.Hostname); err != nil { return err } if va.Port <= 0 { @@ -117,15 +117,17 @@ func (va ValidatorAddress) Validate() error { return nil } -func validateHostname(hostname string) error { +// ValidateHostname checks that hostname is a valid IP address or DNS name +// and does not contain an embedded port. +func ValidateHostname(hostname string) error { if _, _, err := net.SplitHostPort(hostname); err == nil { - return fmt.Errorf("hostname must not include port") + return errors.New("hostname must not include port") } if net.ParseIP(hostname) != nil { return nil } if !IsValidHostname(hostname) { - return fmt.Errorf("invalid hostname") + return errors.New("invalid hostname") } return nil } From 17ad9cc7a65b80d4e300a67ac381962068a47611 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 12 Feb 2026 12:26:48 +0100 Subject: [PATCH 4/4] fix(p2p): improve validation error context and add Endpoint.Equal tests Add hostname value to ValidateHostname error messages for easier debugging, fix uint16 port comparison, document IDN rejection policy, and add dedicated unit tests for Endpoint.Equal covering IPv4/IPv6 representations. Co-Authored-By: Claude Opus 4.6 --- internal/p2p/address_test.go | 4 +- internal/p2p/transport_test.go | 76 ++++++++++++++++++++++++++++++++++ types/validator_address.go | 8 ++-- 3 files changed, 84 insertions(+), 4 deletions(-) diff --git a/internal/p2p/address_test.go b/internal/p2p/address_test.go index 04fe530e53..d28584564b 100644 --- a/internal/p2p/address_test.go +++ b/internal/p2p/address_test.go @@ -212,7 +212,9 @@ func TestNodeAddress_Resolve(t *testing.T) { true, }, { - // note we don't check localhost, as it may resolve to IPv4 or IPv6 depending on the system + // We intentionally use an external DNS name (one.one.one.one) rather than + // localhost, because localhost may resolve to either IPv4 or IPv6 depending + // on the system, making the expected IP unpredictable. p2p.NodeAddress{Protocol: "tcp", Hostname: "one.one.one.one", Port: 80, Path: "/path"}, &p2p.Endpoint{Protocol: "tcp", IP: net.IPv4(1, 1, 1, 1), Port: 80, Path: "/path"}, true, diff --git a/internal/p2p/transport_test.go b/internal/p2p/transport_test.go index 1d87c5179c..a80b3e245b 100644 --- a/internal/p2p/transport_test.go +++ b/internal/p2p/transport_test.go @@ -561,6 +561,82 @@ func TestEndpoint_String(t *testing.T) { } } +func TestEndpoint_Equal(t *testing.T) { + testcases := []struct { + name string + a p2p.Endpoint + b *p2p.Endpoint + expect bool + }{ + { + "identical IPv4 endpoints", + p2p.Endpoint{Protocol: "tcp", IP: net.IPv4(1, 2, 3, 4), Port: 80, Path: "/path"}, + &p2p.Endpoint{Protocol: "tcp", IP: net.IPv4(1, 2, 3, 4), Port: 80, Path: "/path"}, + true, + }, + { + "IPv4 vs IPv4-mapped IPv6", + p2p.Endpoint{Protocol: "tcp", IP: net.IPv4(127, 0, 0, 1), Port: 80}, + &p2p.Endpoint{Protocol: "tcp", IP: net.ParseIP("::ffff:127.0.0.1"), Port: 80}, + true, + }, + { + "identical IPv6 endpoints", + p2p.Endpoint{Protocol: "tcp", IP: net.IPv6loopback, Port: 80}, + &p2p.Endpoint{Protocol: "tcp", IP: net.IPv6loopback, Port: 80}, + true, + }, + { + "different protocols", + p2p.Endpoint{Protocol: "tcp", IP: net.IPv4(1, 2, 3, 4), Port: 80}, + &p2p.Endpoint{Protocol: "udp", IP: net.IPv4(1, 2, 3, 4), Port: 80}, + false, + }, + { + "different IPs", + p2p.Endpoint{Protocol: "tcp", IP: net.IPv4(1, 2, 3, 4), Port: 80}, + &p2p.Endpoint{Protocol: "tcp", IP: net.IPv4(5, 6, 7, 8), Port: 80}, + false, + }, + { + "different ports", + p2p.Endpoint{Protocol: "tcp", IP: net.IPv4(1, 2, 3, 4), Port: 80}, + &p2p.Endpoint{Protocol: "tcp", IP: net.IPv4(1, 2, 3, 4), Port: 443}, + false, + }, + { + "different paths", + p2p.Endpoint{Protocol: "tcp", IP: net.IPv4(1, 2, 3, 4), Port: 80, Path: "/a"}, + &p2p.Endpoint{Protocol: "tcp", IP: net.IPv4(1, 2, 3, 4), Port: 80, Path: "/b"}, + false, + }, + { + "nil other", + p2p.Endpoint{Protocol: "tcp", IP: net.IPv4(1, 2, 3, 4), Port: 80}, + nil, + false, + }, + { + "both nil IPs with same path", + p2p.Endpoint{Protocol: "memory", Path: "foo"}, + &p2p.Endpoint{Protocol: "memory", Path: "foo"}, + true, + }, + { + "nil IP vs set IP", + p2p.Endpoint{Protocol: "tcp", Path: "foo"}, + &p2p.Endpoint{Protocol: "tcp", IP: net.IPv4(1, 2, 3, 4), Path: "foo"}, + false, + }, + } + for _, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.expect, tc.a.Equal(tc.b)) + }) + } +} + func TestEndpoint_Validate(t *testing.T) { var ( ip4 = []byte{1, 2, 3, 4} diff --git a/types/validator_address.go b/types/validator_address.go index bb1396755a..6fa284c202 100644 --- a/types/validator_address.go +++ b/types/validator_address.go @@ -105,7 +105,7 @@ func (va ValidatorAddress) Validate() error { if err := ValidateHostname(va.Hostname); err != nil { return err } - if va.Port <= 0 { + if va.Port == 0 { return ErrNoPort } if len(va.NodeID) > 0 { @@ -119,15 +119,17 @@ func (va ValidatorAddress) Validate() error { // ValidateHostname checks that hostname is a valid IP address or DNS name // and does not contain an embedded port. +// It rejects internationalized domain names (IDN/punycode); only ASCII +// labels are accepted. func ValidateHostname(hostname string) error { if _, _, err := net.SplitHostPort(hostname); err == nil { - return errors.New("hostname must not include port") + return fmt.Errorf("hostname %q must not include port", hostname) } if net.ParseIP(hostname) != nil { return nil } if !IsValidHostname(hostname) { - return errors.New("invalid hostname") + return fmt.Errorf("invalid hostname %q", hostname) } return nil }