VAULT-33758: IPv6 address conformance for proxy and agent

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

Signed-off-by: Ryan Cragun <[email protected]>

Author: ryancragun

command/agent.go

@@ -939,12 +939,13 @@ 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
+	config.Vault.Address = configutil.NormalizeAddr(c.flagAddress)
 	c.setStringFlag(f, config.Vault.CACert, &StringVar{
 		Name:    flagNameCACert,
 		Target:  &c.flagCACert,
@@ -1029,6 +1030,7 @@ func (c *AgentCommand) setStringFlag(f *FlagSets, configVal string, fVar *String
 	switch {
 	case isFlagSet:
 		// Don't do anything as the flag is already set from the command line
+		return
 	case flagEnvSet:
 		// Use value from env var
 		*fVar.Target = flagEnvValue

command/agent/config/config.go

@@ -743,6 +743,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 {
@@ -1030,10 +1034,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 it's 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"
+}