fix(p2p): improve address validation rules

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
+}

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

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,

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 == ""

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,