ipv6: normalize addrs per RFC-5942 §4 (#25921)

https://datatracker.ietf.org/doc/html/rfc5952#section-4

* copy NormalizeAddr func from vault
  * PRs hashicorp/vault#29228 & hashicorp/vault#29517
* normalize bind/advertise addrs
* normalize consul/vault addrs

Author: gulducat

command/agent/config.go

@@ -26,6 +26,7 @@ import (
 	client "github.com/hashicorp/nomad/client/config"
 	"github.com/hashicorp/nomad/client/fingerprint"
 	"github.com/hashicorp/nomad/helper"
+	"github.com/hashicorp/nomad/helper/ipaddr"
 	"github.com/hashicorp/nomad/helper/pointer"
 	"github.com/hashicorp/nomad/helper/users"
 	"github.com/hashicorp/nomad/nomad"
@@ -1995,6 +1996,7 @@ func (c *Config) normalizeAddrs() error {
 		}
 		c.BindAddr = ipStr
 	}
+	c.BindAddr = ipaddr.NormalizeAddr(c.BindAddr)
 
 	httpAddrs, err := normalizeMultipleBind(c.Addresses.HTTP, c.BindAddr)
 	if err != nil {
@@ -2015,9 +2017,12 @@ func (c *Config) normalizeAddrs() error {
 	c.Addresses.Serf = addr
 
 	c.normalizedAddrs = &NormalizedAddrs{
-		HTTP: joinHostPorts(httpAddrs, strconv.Itoa(c.Ports.HTTP)),
-		RPC:  net.JoinHostPort(c.Addresses.RPC, strconv.Itoa(c.Ports.RPC)),
-		Serf: net.JoinHostPort(c.Addresses.Serf, strconv.Itoa(c.Ports.Serf)),
+		RPC:  normalizeAddrWithPort(c.Addresses.RPC, c.Ports.RPC),
+		Serf: normalizeAddrWithPort(c.Addresses.Serf, c.Ports.Serf),
+	}
+	c.normalizedAddrs.HTTP = make([]string, len(httpAddrs))
+	for i, addr := range httpAddrs {
+		c.normalizedAddrs.HTTP[i] = normalizeAddrWithPort(addr, c.Ports.HTTP)
 	}
 
 	addr, err = normalizeAdvertise(c.AdvertiseAddrs.HTTP, httpAddrs[0], c.Ports.HTTP, c.DevMode)
@@ -2100,14 +2105,21 @@ func parseMultipleIPTemplate(ipTmpl string) ([]string, error) {
 	return deduplicateAddrs(ips), nil
 }
 
+// normalizeAddrWithPort assumes that addr does not contain a port,
+// noramlizes it per ipv6 RFC-5942 §4, and appends ":{port}".
+func normalizeAddrWithPort(addr string, port int) string {
+	return ipaddr.NormalizeAddr(net.JoinHostPort(addr, strconv.Itoa(port)))
+}
+
 // normalizeBind returns a normalized bind address.
 //
 // If addr is set it is used, if not the default bind address is used.
 func normalizeBind(addr, bind string) (string, error) {
 	if addr == "" {
 		return bind, nil
 	}
-	return listenerutil.ParseSingleIPTemplate(addr)
+	addr, err := listenerutil.ParseSingleIPTemplate(addr)
+	return ipaddr.NormalizeAddr(addr), err
 }
 
 // normalizeMultipleBind returns normalized bind addresses.
@@ -2117,7 +2129,11 @@ func normalizeMultipleBind(addr, bind string) ([]string, error) {
 	if addr == "" {
 		return []string{bind}, nil
 	}
-	return parseMultipleIPTemplate(addr)
+	addrs, err := parseMultipleIPTemplate(addr)
+	for i, addr := range addrs {
+		addrs[i] = ipaddr.NormalizeAddr(addr)
+	}
+	return addrs, err
 }
 
 // normalizeAdvertise returns a normalized advertise address.
@@ -2147,10 +2163,10 @@ func normalizeAdvertise(addr string, bind string, defport int, dev bool) (string
 			}
 
 			// missing port, append the default
-			return net.JoinHostPort(addr, strconv.Itoa(defport)), nil
+			return normalizeAddrWithPort(addr, defport), nil
 		}
 
-		return addr, nil
+		return ipaddr.NormalizeAddr(addr), nil
 	}
 
 	// Fallback to bind address first, and then try resolving the local hostname
@@ -2162,12 +2178,12 @@ func normalizeAdvertise(addr string, bind string, defport int, dev bool) (string
 	// Return the first non-localhost unicast address
 	for _, ip := range ips {
 		if ip.IsLinkLocalUnicast() || ip.IsGlobalUnicast() {
-			return net.JoinHostPort(ip.String(), strconv.Itoa(defport)), nil
+			return normalizeAddrWithPort(ip.String(), defport), nil
 		}
 		if ip.IsLoopback() {
 			if dev {
 				// loopback is fine for dev mode
-				return net.JoinHostPort(ip.String(), strconv.Itoa(defport)), nil
+				return normalizeAddrWithPort(ip.String(), defport), nil
 			}
 			return "", fmt.Errorf("Defaulting advertise to localhost is unsafe, please set advertise manually")
 		}
@@ -2178,7 +2194,7 @@ func normalizeAdvertise(addr string, bind string, defport int, dev bool) (string
 	if err != nil {
 		return "", fmt.Errorf("Unable to parse default advertise address: %v", err)
 	}
-	return net.JoinHostPort(addr, strconv.Itoa(defport)), nil
+	return normalizeAddrWithPort(addr, defport), nil
 }
 
 // isMissingPort returns true if an error is a "missing port" error from
@@ -2923,17 +2939,6 @@ func LoadConfigDir(dir string) (*Config, error) {
 	return result, nil
 }
 
-// joinHostPorts joins every addr in addrs with the specified port
-func joinHostPorts(addrs []string, port string) []string {
-	localAddrs := make([]string, len(addrs))
-	for i, k := range addrs {
-		localAddrs[i] = net.JoinHostPort(k, port)
-
-	}
-
-	return localAddrs
-}
-
 // isTemporaryFile returns true or false depending on whether the
 // provided file name is a temporary file for the following editors:
 // emacs or vim.

command/agent/config_parse.go

@@ -17,6 +17,7 @@ import (
 	"github.com/hashicorp/hcl/hcl/ast"
 	client "github.com/hashicorp/nomad/client/config"
 	"github.com/hashicorp/nomad/helper"
+	"github.com/hashicorp/nomad/helper/ipaddr"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/nomad/structs/config"
 	"github.com/mitchellh/mapstructure"
@@ -434,6 +435,10 @@ func parseVaults(c *Config, list *ast.ObjectList) error {
 			c.Vaults = append(c.Vaults, v)
 		}
 
+		for _, conf := range c.Vaults {
+			conf.Addr = ipaddr.NormalizeAddr(conf.Addr)
+		}
+
 		// Decode the default identity.
 		var listVal *ast.ObjectList
 		if ot, ok := obj.Val.(*ast.ObjectType); ok {
@@ -505,6 +510,11 @@ func parseConsuls(c *Config, list *ast.ObjectList) error {
 			c.Consuls = append(c.Consuls, cc)
 		}
 
+		for _, conf := range c.Consuls {
+			conf.Addr = ipaddr.NormalizeAddr(conf.Addr)
+			conf.GRPCAddr = ipaddr.NormalizeAddr(conf.GRPCAddr)
+		}
+
 		// decode service and template identity blocks
 		var listVal *ast.ObjectList
 		if ot, ok := obj.Val.(*ast.ObjectType); ok {

command/agent/config_parse_test.go

@@ -1067,7 +1067,7 @@ func TestConfig_MultipleVault(t *testing.T) {
 
 			must.Eq(t, "alternate", cfg.Vaults[1].Name)
 			must.True(t, *cfg.Vaults[1].Enabled)
-			must.Eq(t, "127.0.0.1:9501", cfg.Vaults[1].Addr)
+			must.Eq(t, "[::1f]:9501", cfg.Vaults[1].Addr)
 
 			must.Eq(t, "other", cfg.Vaults[2].Name)
 			must.Nil(t, cfg.Vaults[2].Enabled)
@@ -1119,7 +1119,7 @@ func TestConfig_MultipleConsul(t *testing.T) {
 			must.Eq(t, "abracadabra", defaultConsul.Token)
 
 			must.Eq(t, "alternate", cfg.Consuls[1].Name)
-			must.Eq(t, "127.0.0.2:8501", cfg.Consuls[1].Addr)
+			must.Eq(t, "[::1f]:8501", cfg.Consuls[1].Addr)
 			must.Eq(t, "xyzzy", cfg.Consuls[1].Token)
 
 			must.Eq(t, "other", cfg.Consuls[2].Name)

command/agent/config_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/hashicorp/nomad/helper/pointer"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/nomad/structs/config"
+	"github.com/shoenig/test"
 	"github.com/shoenig/test/must"
 	"github.com/stretchr/testify/require"
 )
@@ -956,6 +957,33 @@ func TestConfig_normalizeAddrs_IPv6Loopback(t *testing.T) {
 	}
 }
 
+// TestConfig_normalizeAddrs_IPv6 asserts that bind and advertise addrs conform
+// to RFC 5942 §4: https://www.rfc-editor.org/rfc/rfc5942.html#section-4
+// Full coverage is provided by tests for ipaddr.NormalizeAddr
+func TestConfig_normalizeAddrs_IPv6(t *testing.T) {
+	c := &Config{
+		Addresses: &Addresses{},
+
+		BindAddr: "0:0::1F",
+		Ports: &Ports{
+			HTTP: 4646,
+			RPC:  4647,
+		},
+		AdvertiseAddrs: &AdvertiseAddrs{
+			HTTP: "[A110::0:0:C8]:8080",
+			RPC:  "0:00FA:0:0:0::CE",
+		},
+		DevMode: false,
+	}
+	must.NoError(t, c.normalizeAddrs())
+	test.Eq(t, "::1f", c.Addresses.HTTP, test.Sprint("bind HTTP"))
+	test.Eq(t, "::1f", c.Addresses.RPC, test.Sprint("bind RPC"))
+	test.Eq(t, []string{"[::1f]:4646"}, c.normalizedAddrs.HTTP, test.Sprint("normalized HTTP"))
+	test.Eq(t, "[::1f]:4647", c.normalizedAddrs.RPC, test.Sprint("normalized RPC"))
+	test.Eq(t, "[a110::c8]:8080", c.AdvertiseAddrs.HTTP, test.Sprint("advertise HTTP"))
+	test.Eq(t, "[0:fa::ce]:4647", c.AdvertiseAddrs.RPC, test.Sprint("advertise RPC"))
+}
+
 // TestConfig_normalizeAddrs_MultipleInterface asserts that normalizeAddrs will
 // handle normalizing multiple interfaces in a single protocol.
 func TestConfig_normalizeAddrs_MultipleInterfaces(t *testing.T) {

command/agent/testdata/extra-consul.hcl

@@ -18,7 +18,7 @@ consul {
   server_rpc_check_name  = "nomad-server-rpc-health-check"
   client_service_name    = "nomad-client"
   client_http_check_name = "nomad-client-http-health-check"
-  address                = "127.0.0.2:8501"
+  address                = "[0:0::1F]:8501"
   allow_unauthenticated  = true
   token                  = "xyzzy"
   auth                   = "username:pass"

command/agent/testdata/extra-consul.json

@@ -14,7 +14,7 @@
       "server_rpc_check_name": "nomad-server-rpc-health-check",
       "client_service_name": "nomad-client",
       "client_http_check_name": "nomad-client-http-health-check",
-      "address": "127.0.0.2:8501",
+      "address": "[0:0::1F]:8501",
       "allow_unauthenticated": true,
       "token": "xyzzy",
       "auth": "username:pass"

command/agent/testdata/extra-vault.hcl

@@ -10,7 +10,7 @@ vault {
 # these alternate configs should be added as an extra vault configs
 vault {
   name                  = "alternate"
-  address               = "127.0.0.1:9501"
+  address               = "[0:0::1F]:9501"
   allow_unauthenticated = true
   task_token_ttl        = "5s"
   enabled               = true

command/agent/testdata/extra-vault.json

@@ -6,7 +6,7 @@
     },
     {
       "name": "alternate",
-      "address": "127.0.0.1:9501",
+      "address": "[0:0::1F]:9501",
       "allow_unauthenticated": true,
       "task_token_ttl": "5s",
       "enabled": true,

helper/ipaddr/ipaddr.go

@@ -3,6 +3,12 @@
 
 package ipaddr
 
+import (
+	"net"
+	"net/url"
+	"strings"
+)
+
 // IsAny checks if the given IP address is an IPv4 or IPv6 ANY address.
 func IsAny(ip string) bool {
 	return isAnyV4(ip) || isAnyV6(ip)
@@ -11,3 +17,104 @@ func IsAny(ip string) bool {
 func isAnyV4(ip string) bool { return ip == "0.0.0.0" }
 
 func isAnyV6(ip string) bool { return ip == "::" || ip == "[::]" }
+
+// NormalizeAddr takes a string of a Host, Host:Port, URL, or Destination
+// Address and returns a copy where any IP addresses have been normalized to be
+// conformant with RFC 5942 §4. If the input string does not match any of the
+// supported syntaxes, or the "host" section is not an IP address, the input
+// will be returned unchanged. Supported syntaxes are:
+//
+//	Host                 host                                or [host]
+//	Host:Port            host:port                           or [host]:port
+//	URL                  scheme://user@host/path?query#frag  or scheme://user@[host]/path?query#frag
+//	Destination Address  user@host:port                      or user@[host]:port
+//
+// See:
+//
+//	https://rfc-editor.org/rfc/rfc3986.html
+//	https://rfc-editor.org/rfc/rfc5942.html
+//	https://rfc-editor.org/rfc/rfc5952.html
+//
+// Note: This function was copied verbatim from Vault:
+// https://github.com/hashicorp/vault/blob/58a49e6/internalshared/configutil/normalize.go
+func NormalizeAddr(addr string) string {
+	if addr == "" {
+		return ""
+	}
+
+	// Host
+	ip := net.ParseIP(addr)
+	if ip != nil {
+		// net.IP.String() is RFC 5942 §4 compliant
+		return ip.String()
+	}
+
+	// [Host]
+	if strings.HasPrefix(addr, "[") && strings.HasSuffix(addr, "]") {
+		if len(addr) < 3 {
+			return addr
+		}
+
+		// If we've been given a bracketed IP address, return the address
+		// normalized without brackets.
+		ip := net.ParseIP(addr[1 : len(addr)-1])
+		if ip != nil {
+			return ip.String()
+		}
+
+		// Our input is not a valid schema.
+		return addr
+	}
+
+	// Host:Port
+	host, port, err := net.SplitHostPort(addr)
+	if err == nil {
+		ip := net.ParseIP(host)
+		if ip == nil {
+			// Our host isn't an IP address so we can return it unchanged
+			return addr
+		}
+
+		// net.JoinHostPort handles bracketing for RFC 5952 §6
+		return net.JoinHostPort(ip.String(), port)
+	}
+
+	// URL
+	u, err := url.Parse(addr)
+	if err == nil {
+		uhost := u.Hostname()
+		ip := net.ParseIP(uhost)
+		if ip == nil {
+			// Our URL doesn't contain an IP address so we can return our input unchanged.
+			return addr
+		} else {
+			uhost = ip.String()
+		}
+
+		if uport := u.Port(); uport != "" {
+			uhost = net.JoinHostPort(uhost, uport)
+		} else if !strings.HasPrefix(uhost, "[") && !strings.HasSuffix(uhost, "]") {
+			// Ensure the IPv6 URL host is bracketed post-normalization.
+			// When*url.URL.String() reassembles the URL it will not consider
+			// whether or not the  *url.URL.Host is RFC 5952 §6 and RFC 3986 §3.2.2
+			// conformant.
+			uhost = "[" + uhost + "]"
+
+		}
+		u.Host = uhost
+
+		return u.String()
+	}
+
+	// Destination Address
+	if idx := strings.LastIndex(addr, "@"); idx > 0 {
+		if idx+1 > len(addr) {
+			return addr
+		}
+
+		return addr[:idx+1] + NormalizeAddr(addr[idx+1:])
+	}
+
+	// Our input did not match our supported schemas. Return it unchanged.
+	return addr
+}