VAULT-33758: IPv6 address conformance for proxy and agent (#29517)

This is a follow-up to our initial work[0] to address RFC-5952 §4 conformance for IPv6 addresses in Vault. The initial pass focused on the vault server configuration and start-up routines. This follow-up focuses on Agent and Proxy, with a few minor improvements for server.

The approach generally mirrors the server implementation but also adds support for normalization with CLI configuration overrides.

One aspect we do not normalize currently is Agent/Proxy client creation to the Vault server with credentials taken from environment variables, as it would require larger changes to the `api` module. In practice this ought to be fine for the majority of cases.

[0]: #29228

Author: ryancragun

changelog/29517.txt

@@ -0,0 +1,6 @@
+```release-note:change
+agent/config: Configuration values including IPv6 addresses will be automatically translated and displayed conformant to RFC-5952 §4.
+```
+```release-note:change
+proxy/config: Configuration values including IPv6 addresses will be automatically translated and displayed conformant to RFC-5952 §4.
+```

command/agent.go

@@ -939,10 +939,11 @@ func (c *AgentCommand) applyConfigOverrides(f *FlagSets, config *agentConfig.Con
 	})
 
 	c.setStringFlag(f, config.Vault.Address, &StringVar{
-		Name:    flagNameAddress,
-		Target:  &c.flagAddress,
-		Default: "https://127.0.0.1:8200",
-		EnvVar:  api.EnvVaultAddress,
+		Name:        flagNameAddress,
+		Target:      &c.flagAddress,
+		Default:     "https://127.0.0.1:8200",
+		EnvVar:      api.EnvVaultAddress,
+		Normalizers: []func(string) string{configutil.NormalizeAddr},
 	})
 	config.Vault.Address = c.flagAddress
 	c.setStringFlag(f, config.Vault.CACert, &StringVar{
@@ -1031,13 +1032,13 @@ func (c *AgentCommand) setStringFlag(f *FlagSets, configVal string, fVar *String
 		// Don't do anything as the flag is already set from the command line
 	case flagEnvSet:
 		// Use value from env var
-		*fVar.Target = flagEnvValue
+		fVar.SetTarget(flagEnvValue)
 	case configVal != "":
 		// Use value from config
-		*fVar.Target = configVal
+		fVar.SetTarget(configVal)
 	default:
 		// Use the default value
-		*fVar.Target = fVar.Default
+		fVar.SetTarget(fVar.Default)
 	}
 }
 

command/agent/config/config.go

@@ -751,6 +751,10 @@ func parseVault(result *Config, list *ast.ObjectList) error {
 		return err
 	}
 
+	if v.Address != "" {
+		v.Address = configutil.NormalizeAddr(v.Address)
+	}
+
 	if v.TLSSkipVerifyRaw != nil {
 		v.TLSSkipVerify, err = parseutil.ParseBool(v.TLSSkipVerifyRaw)
 		if err != nil {
@@ -1038,10 +1042,63 @@ func parseMethod(result *Config, list *ast.ObjectList) error {
 	// Canonicalize namespace path if provided
 	m.Namespace = namespace.Canonicalize(m.Namespace)
 
+	// Normalize any configuration addresses
+	if len(m.Config) > 0 {
+		var err error
+		for k, v := range m.Config {
+			vStr, ok := v.(string)
+			if !ok {
+				continue
+			}
+			m.Config[k], err = normalizeAutoAuthMethod(m.Type, k, vStr)
+			if err != nil {
+				return err
+			}
+		}
+	}
+
 	result.AutoAuth.Method = &m
 	return nil
 }
 
+// autoAuthMethodKeys maps an auto-auth method type to its associated
+// configuration whose values are URLs, IP addresses, or host:port style
+// addresses. All auto-auth types must have an entry in this map, otherwise our
+// normalization check will fail when parsing the storage entry config.
+// Auto-auth method types which don't contain such keys should include an empty
+// array.
+var autoAuthMethodKeys = map[string][]string{
+	"alicloud":   {""},
+	"approle":    {""},
+	"aws":        {""},
+	"azure":      {"resource"},
+	"cert":       {""},
+	"cf":         {""},
+	"gcp":        {"service_account"},
+	"jwt":        {""},
+	"ldap":       {""},
+	"kerberos":   {""},
+	"kubernetes": {""},
+	"oci":        {""},
+	"token_file": {""},
+}
+
+// normalizeAutoAuthMethod takes a storage name, a configuration key
+// and its associated value and will normalize any URLs, IP addresses, or
+// host:port style addresses.
+func normalizeAutoAuthMethod(method string, key string, value string) (string, error) {
+	keys, ok := autoAuthMethodKeys[method]
+	if !ok {
+		return "", fmt.Errorf("unknown auto-auth method type %s", method)
+	}
+
+	if slices.Contains(keys, key) {
+		return configutil.NormalizeAddr(value), nil
+	}
+
+	return value, nil
+}
+
 func parseSinks(result *Config, list *ast.ObjectList) error {
 	name := "sink"
 

command/agent/config/config_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/hashicorp/vault/command/agentproxyshared"
 	"github.com/hashicorp/vault/internalshared/configutil"
 	"github.com/hashicorp/vault/sdk/helper/pointerutil"
+	"github.com/stretchr/testify/require"
 	"golang.org/x/exp/slices"
 )
 
@@ -230,6 +231,9 @@ func TestLoadConfigDir_AgentCache(t *testing.T) {
 		t.Fatal(err)
 	}
 	config2, err := LoadConfigFile("./test-fixtures/config-dir-cache/config-cache2.hcl")
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	mergedConfig := config.Merge(config2)
 
@@ -441,77 +445,117 @@ func TestLoadConfigFile_AgentCache_NoListeners(t *testing.T) {
 	}
 }
 
-func TestLoadConfigFile(t *testing.T) {
-	if err := os.Setenv("TEST_AAD_ENV", "aad"); err != nil {
-		t.Fatal(err)
-	}
-	defer func() {
-		if err := os.Unsetenv("TEST_AAD_ENV"); err != nil {
-			t.Fatal(err)
-		}
-	}()
-
-	config, err := LoadConfigFile("./test-fixtures/config.hcl")
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
+// Test_LoadConfigFile_AutoAuth_AddrConformance verifies basic config file
+// loading in addition to RFC-5942 §4 normalization of auto-auth methods.
+// See: https://rfc-editor.org/rfc/rfc5952.html
+func Test_LoadConfigFile_AutoAuth_AddrConformance(t *testing.T) {
+	t.Setenv("TEST_AAD_ENV", "aad")
+
+	for name, method := range map[string]*Method{
+		"aws": {
+			Type:      "aws",
+			MountPath: "auth/aws",
+			Namespace: "aws-namespace/",
+			Config: map[string]any{
+				"role": "foobar",
+			},
+		},
+		"azure": {
+			Type:      "azure",
+			MountPath: "auth/azure",
+			Namespace: "azure-namespace/",
+			Config: map[string]any{
+				"authenticate_from_environment": true,
+				"role":                          "dev-role",
+				"resource":                      "https://[2001:0:0:1::1]",
+			},
+		},
+		"gcp": {
+			Type:      "gcp",
+			MountPath: "auth/gcp",
+			Namespace: "gcp-namespace/",
+			Config: map[string]any{
+				"role":            "dev-role",
+				"service_account": "https://[2001:db8:ac3:fe4::1]",
+			},
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			config, err := LoadConfigFile("./test-fixtures/config-auto-auth-" + name + ".hcl")
+			require.NoError(t, err)
 
-	expected := &Config{
-		SharedConfig: &configutil.SharedConfig{
-			PidFile: "./pidfile",
-			LogFile: "/var/log/vault/vault-agent.log",
-		},
-		AutoAuth: &AutoAuth{
-			Method: &Method{
-				Type:      "aws",
-				MountPath: "auth/aws",
-				Namespace: "my-namespace/",
-				Config: map[string]interface{}{
-					"role": "foobar",
+			expected := &Config{
+				SharedConfig: &configutil.SharedConfig{
+					PidFile: "./pidfile",
+					Listeners: []*configutil.Listener{
+						{
+							Type:       "unix",
+							Address:    "/path/to/socket",
+							TLSDisable: true,
+							AgentAPI: &configutil.AgentAPI{
+								EnableQuit: true,
+							},
+						},
+						{
+							Type:       "tcp",
+							Address:    "2001:db8::1:8200", // Normalized
+							TLSDisable: true,
+						},
+						{
+							Type:       "tcp",
+							Address:    "[2001:0:0:1::1]:3000", // Normalized
+							Role:       "metrics_only",
+							TLSDisable: true,
+						},
+						{
+							Type:        "tcp",
+							Role:        "default",
+							Address:     "2001:db8:0:1:1:1:1:1:8400", // Normalized
+							TLSKeyFile:  "/path/to/cakey.pem",
+							TLSCertFile: "/path/to/cacert.pem",
+						},
+					},
+					LogFile: "/var/log/vault/vault-agent.log",
 				},
-				MaxBackoff: 0,
-			},
-			Sinks: []*Sink{
-				{
-					Type:   "file",
-					DHType: "curve25519",
-					DHPath: "/tmp/file-foo-dhpath",
-					AAD:    "foobar",
-					Config: map[string]interface{}{
-						"path": "/tmp/file-foo",
+				Vault: &Vault{
+					Address: "https://[2001:db8::1]:8200", // Address is normalized
+					Retry: &Retry{
+						NumRetries: 12, // Default number of retries when a vault stanza is set
 					},
 				},
-				{
-					Type:      "file",
-					WrapTTL:   5 * time.Minute,
-					DHType:    "curve25519",
-					DHPath:    "/tmp/file-foo-dhpath2",
-					AAD:       "aad",
-					DeriveKey: true,
-					Config: map[string]interface{}{
-						"path": "/tmp/file-bar",
+				AutoAuth: &AutoAuth{
+					Method: method, // Method properties are normalized correctly
+					Sinks: []*Sink{
+						{
+							Type:   "file",
+							DHType: "curve25519",
+							DHPath: "/tmp/file-foo-dhpath",
+							AAD:    "foobar",
+							Config: map[string]interface{}{
+								"path": "/tmp/file-foo",
+							},
+						},
+						{
+							Type:      "file",
+							WrapTTL:   5 * time.Minute,
+							DHType:    "curve25519",
+							DHPath:    "/tmp/file-foo-dhpath2",
+							AAD:       "aad",
+							DeriveKey: true,
+							Config: map[string]interface{}{
+								"path": "/tmp/file-bar",
+							},
+						},
 					},
 				},
-			},
-		},
-		TemplateConfig: &TemplateConfig{
-			MaxConnectionsPerHost: DefaultTemplateConfigMaxConnsPerHost,
-		},
-	}
-
-	config.Prune()
-	if diff := deep.Equal(config, expected); diff != nil {
-		t.Fatal(diff)
-	}
-
-	config, err = LoadConfigFile("./test-fixtures/config-embedded-type.hcl")
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
+				TemplateConfig: &TemplateConfig{
+					MaxConnectionsPerHost: DefaultTemplateConfigMaxConnsPerHost,
+				},
+			}
 
-	config.Prune()
-	if diff := deep.Equal(config, expected); diff != nil {
-		t.Fatal(diff)
+			config.Prune()
+			require.EqualValues(t, expected, config)
+		})
 	}
 }
 

command/agent/config/test-fixtures/config-auto-auth-aws.hcl

@@ -0,0 +1,69 @@
+# Copyright (c) HashiCorp, Inc.
+# SPDX-License-Identifier: BUSL-1.1
+
+pid_file = "./pidfile"
+log_file = "/var/log/vault/vault-agent.log"
+
+vault {
+  address = "https://[2001:0db8::0001]:8200"
+}
+
+auto_auth {
+  method {
+    type      = "aws"
+    namespace = "/aws-namespace"
+    config = {
+      role = "foobar"
+    }
+  }
+
+  sink {
+    type = "file"
+    config = {
+      path = "/tmp/file-foo"
+    }
+    aad     = "foobar"
+    dh_type = "curve25519"
+    dh_path = "/tmp/file-foo-dhpath"
+  }
+
+  sink {
+    type        = "file"
+    wrap_ttl    = "5m"
+    aad_env_var = "TEST_AAD_ENV"
+    dh_type     = "curve25519"
+    dh_path     = "/tmp/file-foo-dhpath2"
+    derive_key  = true
+    config = {
+      path = "/tmp/file-bar"
+    }
+  }
+}
+
+listener "unix" {
+  address     = "/path/to/socket"
+  tls_disable = true
+
+  agent_api {
+    enable_quit = true
+  }
+}
+
+listener "tcp" {
+  address     = "2001:0db8::0001:8200"
+  tls_disable = true
+}
+
+listener {
+  type        = "tcp"
+  address     = "[2001:0:0:1:0:0:0:1]:3000"
+  tls_disable = true
+  role        = "metrics_only"
+}
+
+listener "tcp" {
+  role          = "default"
+  address       = "2001:db8:0:1:1:1:1:1:8400"
+  tls_key_file  = "/path/to/cakey.pem"
+  tls_cert_file = "/path/to/cacert.pem"
+}