fix(p2p): improve address validation rules

internal/p2p/address.go

@@ -169,6 +169,11 @@ 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 := types.ValidateHostname(a.Hostname); err != nil {
+			return err
+		}
+	}
 	if a.Port > 0 && a.Hostname == "" {
 		return errors.New("cannot specify port without hostname")
 	}

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,11 @@ 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"},
+			// 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,
 		},
 		{
@@ -287,7 +277,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 +363,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

internal/p2p/transport.go

@@ -157,6 +157,26 @@ func (e Endpoint) NodeAddress(nodeID types.NodeID) NodeAddress {
 	return address
 }
 
+func (e Endpoint) Equal(other *Endpoint) bool {
+	if other == nil {
+		return false
+	}
+
+	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,

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}

types/validator_address.go

@@ -102,7 +102,10 @@ func (va ValidatorAddress) Validate() error {
 	if va.Hostname == "" {
 		return ErrNoHostname
 	}
-	if va.Port <= 0 {
+	if err := ValidateHostname(va.Hostname); err != nil {
+		return err
+	}
+	if va.Port == 0 {
 		return ErrNoPort
 	}
 	if len(va.NodeID) > 0 {
@@ -114,6 +117,54 @@ func (va ValidatorAddress) Validate() error {
 	return nil
 }
 
+// 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 fmt.Errorf("hostname %q must not include port", hostname)
+	}
+	if net.ParseIP(hostname) != nil {
+		return nil
+	}
+	if !IsValidHostname(hostname) {
+		return fmt.Errorf("invalid hostname %q", 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 == ""

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,