diff --git a/changelog/29517.txt b/changelog/29517.txt new file mode 100644 index 0000000000..68c33a5cac --- /dev/null +++ b/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. +``` diff --git a/command/agent.go b/command/agent.go index 2e5f550a55..bb8a8aee32 100644 --- a/command/agent.go +++ b/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) } } diff --git a/command/agent/config/config.go b/command/agent/config/config.go index aeaba36b51..4094b4b30a 100644 --- a/command/agent/config/config.go +++ b/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" diff --git a/command/agent/config/config_test.go b/command/agent/config/config_test.go index df34e0db49..79068a72cc 100644 --- a/command/agent/config/config_test.go +++ b/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) - } - }() +// 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") - config, err := LoadConfigFile("./test-fixtures/config.hcl") - if err != nil { - t.Fatalf("err: %s", 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", - }, - MaxBackoff: 0, - }, - 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", - }, - }, + for name, method := range map[string]*Method{ + "aws": { + Type: "aws", + MountPath: "auth/aws", + Namespace: "aws-namespace/", + Config: map[string]any{ + "role": "foobar", }, }, - TemplateConfig: &TemplateConfig{ - MaxConnectionsPerHost: DefaultTemplateConfigMaxConnsPerHost, + "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) - config.Prune() - if diff := deep.Equal(config, expected); diff != nil { - t.Fatal(diff) - } + 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", + }, + 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 + }, + }, + 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, err = LoadConfigFile("./test-fixtures/config-embedded-type.hcl") - if err != nil { - t.Fatalf("err: %s", err) - } - - config.Prune() - if diff := deep.Equal(config, expected); diff != nil { - t.Fatal(diff) + config.Prune() + require.EqualValues(t, expected, config) + }) } } diff --git a/command/agent/config/test-fixtures/config-auto-auth-aws.hcl b/command/agent/config/test-fixtures/config-auto-auth-aws.hcl new file mode 100644 index 0000000000..3da04a6409 --- /dev/null +++ b/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" +} diff --git a/command/agent/config/test-fixtures/config-auto-auth-azure.hcl b/command/agent/config/test-fixtures/config-auto-auth-azure.hcl new file mode 100644 index 0000000000..1174f709d3 --- /dev/null +++ b/command/agent/config/test-fixtures/config-auto-auth-azure.hcl @@ -0,0 +1,71 @@ +# 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 = "azure" + namespace = "/azure-namespace" + config = { + authenticate_from_environment = true + role = "dev-role" + resource = "https://[2001:0:0:1:0:0:0:1]", + } + } + + 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" +} diff --git a/command/agent/config/test-fixtures/config-auto-auth-gcp.hcl b/command/agent/config/test-fixtures/config-auto-auth-gcp.hcl new file mode 100644 index 0000000000..3b27f7372a --- /dev/null +++ b/command/agent/config/test-fixtures/config-auto-auth-gcp.hcl @@ -0,0 +1,70 @@ +# 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 = "gcp" + namespace = "/gcp-namespace" + config = { + role = "dev-role" + service_account = "https://[2001:DB8:AC3:FE4::1]" + } + } + + 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" +} diff --git a/command/agent/config/test-fixtures/config-embedded-type.hcl b/command/agent/config/test-fixtures/config-embedded-type.hcl deleted file mode 100644 index cf3c182a85..0000000000 --- a/command/agent/config/test-fixtures/config-embedded-type.hcl +++ /dev/null @@ -1,35 +0,0 @@ -# Copyright (c) HashiCorp, Inc. -# SPDX-License-Identifier: BUSL-1.1 - -pid_file = "./pidfile" -log_file = "/var/log/vault/vault-agent.log" - -auto_auth { - method "aws" { - mount_path = "auth/aws" - namespace = "my-namespace" - config = { - role = "foobar" - } - } - - sink "file" { - config = { - path = "/tmp/file-foo" - } - aad = "foobar" - dh_type = "curve25519" - dh_path = "/tmp/file-foo-dhpath" - } - - sink "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" - } - } -} diff --git a/command/agent/config/test-fixtures/config.hcl b/command/agent/config/test-fixtures/config.hcl deleted file mode 100644 index f6ca0e684e..0000000000 --- a/command/agent/config/test-fixtures/config.hcl +++ /dev/null @@ -1,37 +0,0 @@ -# Copyright (c) HashiCorp, Inc. -# SPDX-License-Identifier: BUSL-1.1 - -pid_file = "./pidfile" -log_file = "/var/log/vault/vault-agent.log" - -auto_auth { - method { - type = "aws" - namespace = "/my-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" - } - } -} diff --git a/command/agent_test.go b/command/agent_test.go index 17c74fc316..c4ce3e2b68 100644 --- a/command/agent_test.go +++ b/command/agent_test.go @@ -3569,6 +3569,113 @@ template { } } +// TestAgent_Config_AddrConformance verifies that the vault address is correctly +// normalized to conform to RFC-5942 §4 when configured by a config file, +// environment variables, or CLI flags. +// See: https://rfc-editor.org/rfc/rfc5952.html +func TestAgent_Config_AddrConformance(t *testing.T) { + for name, test := range map[string]struct { + args []string + envVars map[string]string + cfg string + expected *agentConfig.Config + }{ + "ipv4 config": { + cfg: ` +vault { + address = "https://127.0.0.1:8200" +}`, + expected: &agentConfig.Config{ + Vault: &agentConfig.Vault{ + Address: "https://127.0.0.1:8200", + }, + }, + }, + "ipv6 config": { + cfg: ` +vault { + address = "https://[2001:0db8::0001]:8200" +}`, + expected: &agentConfig.Config{ + Vault: &agentConfig.Vault{ + // Use the normalized version in the config + Address: "https://[2001:db8::1]:8200", + }, + }, + }, + "ipv6 cli arg overrides": { + args: []string{"-address=https://[2001:0:0:1:0:0:0:1]:8200"}, + cfg: ` +vault { + address = "https://[2001:0db8::0001]:8200" +}`, + expected: &agentConfig.Config{ + Vault: &agentConfig.Vault{ + // Use a normalized version of the args address + Address: "https://[2001:0:0:1::1]:8200", + }, + }, + }, + "ipv6 env var overrides": { + envVars: map[string]string{ + "VAULT_ADDR": "https://[2001:DB8:AC3:FE4::1]:8200", + }, + cfg: ` +vault { + address = "https://[2001:0db8::0001]:8200" +}`, + expected: &agentConfig.Config{ + Vault: &agentConfig.Vault{ + // Use a normalized version of the env var address + Address: "https://[2001:db8:ac3:fe4::1]:8200", + }, + }, + }, + "ipv6 all uses cli overrides": { + args: []string{"-address=https://[2001:0:0:1:0:0:0:1]:8200"}, + envVars: map[string]string{ + "VAULT_ADDR": "https://[2001:DB8:AC3:FE4::1]:8200", + }, + cfg: ` +vault { + address = "https://[2001:0db8::0001]:8200" +}`, + expected: &agentConfig.Config{ + Vault: &agentConfig.Vault{ + // Use a normalized version of the args address + Address: "https://[2001:0:0:1::1]:8200", + }, + }, + }, + } { + t.Run(name, func(t *testing.T) { + // In CI our tests are run with VAULT_ADDR=, which will break our tests + // because it'll default to an unset address. Ensure that's cleared out + // of the environment. + t.Cleanup(func() { + os.Setenv(api.EnvVaultAddress, os.Getenv(api.EnvVaultAddress)) + }) + os.Unsetenv(api.EnvVaultAddress) + for k, v := range test.envVars { + t.Setenv(k, v) + } + + configFile := populateTempFile(t, "agent-"+strings.ReplaceAll(name, " ", "-"), test.cfg) + cfg, err := agentConfig.LoadConfigFile(configFile.Name()) + require.NoError(t, err) + require.NotEmptyf(t, cfg.Vault.Address, "agent config is missing address: %+v", cfg.Vault) + + cmd := &AgentCommand{BaseCommand: &BaseCommand{}} + f := cmd.Flags() + args := append([]string{}, test.args...) + require.NoError(t, f.Parse(args)) + + cmd.applyConfigOverrides(f, cfg) + require.Equalf(t, test.expected.Vault.Address, cfg.Vault.Address, "agent config is missing address: config: %+v, flags: %+v", cfg.Vault, f) + }) + } +} + // Get a randomly assigned port and then free it again before returning it. // There is still a race when trying to use it, but should work better // than a static port. diff --git a/command/base.go b/command/base.go index ceba362e34..f8c2a8a920 100644 --- a/command/base.go +++ b/command/base.go @@ -22,6 +22,7 @@ import ( "github.com/hashicorp/vault/api/tokenhelper" "github.com/hashicorp/vault/command/config" "github.com/hashicorp/vault/helper/namespace" + "github.com/hashicorp/vault/internalshared/configutil" "github.com/mattn/go-isatty" "github.com/mitchellh/go-homedir" "github.com/pkg/errors" @@ -387,11 +388,12 @@ func (c *BaseCommand) flagSet(bit FlagSetBit) *FlagSets { f := set.NewFlagSet("HTTP Options") addrStringVar := &StringVar{ - Name: flagNameAddress, - Target: &c.flagAddress, - EnvVar: api.EnvVaultAddress, - Completion: complete.PredictAnything, - Usage: "Address of the Vault server.", + Name: flagNameAddress, + Target: &c.flagAddress, + EnvVar: api.EnvVaultAddress, + Completion: complete.PredictAnything, + Normalizers: []func(string) string{configutil.NormalizeAddr}, + Usage: "Address of the Vault server.", } if c.flagAddress != "" { @@ -403,11 +405,12 @@ func (c *BaseCommand) flagSet(bit FlagSetBit) *FlagSets { f.StringVar(addrStringVar) agentAddrStringVar := &StringVar{ - Name: "agent-address", - Target: &c.flagAgentProxyAddress, - EnvVar: api.EnvVaultAgentAddr, - Completion: complete.PredictAnything, - Usage: "Address of the Agent.", + Name: "agent-address", + Target: &c.flagAgentProxyAddress, + EnvVar: api.EnvVaultAgentAddr, + Completion: complete.PredictAnything, + Normalizers: []func(string) string{configutil.NormalizeAddr}, + Usage: "Address of the Agent.", } f.StringVar(agentAddrStringVar) diff --git a/command/base_flags.go b/command/base_flags.go index 3f3fc1abda..238f06c93c 100644 --- a/command/base_flags.go +++ b/command/base_flags.go @@ -460,56 +460,89 @@ func (i *uint64Value) Hidden() bool { return i.hidden } // -- StringVar and stringValue type StringVar struct { - Name string - Aliases []string - Usage string - Default string - Hidden bool - EnvVar string - Target *string - Completion complete.Predictor + Name string + Aliases []string + Usage string + Default string + Hidden bool + EnvVar string + Target *string + Normalizers []func(string) string + Completion complete.Predictor +} + +func (s *StringVar) SetTarget(val string) { + if s == nil { + return + } + + for _, f := range s.Normalizers { + val = f(val) + } + + *s.Target = val } func (f *FlagSet) StringVar(i *StringVar) { - initial := i.Default - if v, exist := os.LookupEnv(i.EnvVar); exist { - initial = v + if i == nil { + return } - def := "" - if i.Default != "" { - def = i.Default + val := i.Default + envVar, ok := os.LookupEnv(i.EnvVar) + if ok { + val = envVar } f.VarFlag(&VarFlag{ Name: i.Name, Aliases: i.Aliases, Usage: i.Usage, - Default: def, + Default: i.Default, EnvVar: i.EnvVar, - Value: newStringValue(initial, i.Target, i.Hidden), + Value: newStringValue(val, i.Target, i.Hidden, i.Normalizers), Completion: i.Completion, }) } type stringValue struct { - hidden bool - target *string + hidden bool + target *string + normalizers []func(string) string } -func newStringValue(def string, target *string, hidden bool) *stringValue { - *target = def - return &stringValue{ - hidden: hidden, - target: target, +func newStringValue(val string, target *string, hidden bool, normalizers []func(string) string) *stringValue { + sv := &stringValue{ + hidden: hidden, + target: target, + normalizers: append(normalizers, strings.TrimSpace), } + sv.set(val) + + return sv } func (s *stringValue) Set(val string) error { - *s.target = val + s.set(val) return nil } +func (s *stringValue) set(val string) { + *s.target = s.normalize(val) +} + +func (s *stringValue) normalize(in string) string { + if s == nil || len(s.normalizers) < 1 { + return in + } + + for _, f := range s.normalizers { + in = f(in) + } + + return in +} + func (s *stringValue) Get() interface{} { return *s.target } func (s *stringValue) String() string { return *s.target } func (s *stringValue) Example() string { return "string" } diff --git a/command/base_test.go b/command/base_test.go index 2be878a09b..62edd66560 100644 --- a/command/base_test.go +++ b/command/base_test.go @@ -24,60 +24,64 @@ func getDefaultCliHeaders(t *testing.T) http.Header { } func TestClient_FlagHeader(t *testing.T) { - defaultHeaders := getDefaultCliHeaders(t) + t.Parallel() - cases := []struct { + cases := map[string]struct { Input map[string]string Valid bool }{ - { + "empty": { map[string]string{}, true, }, - { + "valid": { map[string]string{"foo": "bar", "header2": "value2"}, true, }, - { + "invalid": { map[string]string{"X-Vault-foo": "bar", "header2": "value2"}, false, }, } - for _, tc := range cases { - expectedHeaders := defaultHeaders.Clone() - for key, val := range tc.Input { - expectedHeaders.Add(key, val) - } - - bc := &BaseCommand{flagHeader: tc.Input} - cli, err := bc.Client() - - if err == nil && !tc.Valid { - t.Errorf("No error for input[%#v], but not valid", tc.Input) - continue - } - - if err != nil { - if tc.Valid { - t.Errorf("Error[%v] with input[%#v], but valid", err, tc.Input) + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + expectedHeaders := getDefaultCliHeaders(t) + for key, val := range tc.Input { + expectedHeaders.Add(key, val) } - continue - } - if cli == nil { - t.Error("client should not be nil") - } + bc := &BaseCommand{flagHeader: tc.Input} + cli, err := bc.Client() - actualHeaders := cli.Headers() - if !reflect.DeepEqual(expectedHeaders, actualHeaders) { - t.Errorf("expected [%#v] but got [%#v]", expectedHeaders, actualHeaders) - } + if err == nil && !tc.Valid { + t.Errorf("No error for input[%#v], but not valid", tc.Input) + } + + if err != nil { + if tc.Valid { + t.Errorf("Error[%v] with input[%#v], but valid", err, tc.Input) + } + return + } + + if cli == nil { + t.Error("client should not be nil") + } + + actualHeaders := cli.Headers() + if !reflect.DeepEqual(expectedHeaders, actualHeaders) { + t.Errorf("expected [%#v] but got [%#v]", expectedHeaders, actualHeaders) + } + }) } } // TestClient_HCPConfiguration tests that the HCP configuration is applied correctly when it exists in cache. func TestClient_HCPConfiguration(t *testing.T) { + t.Parallel() + cases := map[string]struct { Valid bool ExpectedAddr string @@ -94,7 +98,8 @@ func TestClient_HCPConfiguration(t *testing.T) { for n, tst := range cases { t.Run(n, func(t *testing.T) { - bc := &BaseCommand{hcpTokenHelper: &hcpvlib.TestingHCPTokenHelper{tst.Valid}} + t.Parallel() + bc := &BaseCommand{hcpTokenHelper: &hcpvlib.TestingHCPTokenHelper{ValidCache: tst.Valid}} cli, err := bc.Client() assert.NoError(t, err) @@ -109,3 +114,80 @@ func TestClient_HCPConfiguration(t *testing.T) { }) } } + +// Test_FlagSet_StringVar_Normalizers verifies that the normalizer callbacks +// works as expected. +func Test_FlagSet_StringVar_Normalizers(t *testing.T) { + appendA := func(in string) string { return in + "a" } + prependB := func(in string) string { return "b" + in } + + for name, test := range map[string]struct { + in func() *StringVar + envVars map[string]string + expectedValue string + }{ + "no normalizers no env vars uses default value": { + in: func() *StringVar { + resT := "" + return &StringVar{ + Name: "test", + Target: &resT, + EnvVar: "VAULT_TEST", + Default: "default", + } + }, + expectedValue: "default", + }, + "one normalizer no env vars normalizes default value": { + in: func() *StringVar { + resT := "" + return &StringVar{ + Name: "test", + Target: &resT, + EnvVar: "VAULT_TEST", + Default: "default", + Normalizers: []func(string) string{appendA}, + } + }, + expectedValue: "defaulta", + }, + "two normalizers no env vars normalizes default value with both": { + in: func() *StringVar { + resT := "" + return &StringVar{ + Name: "test", + Target: &resT, + EnvVar: "VAULT_TEST", + Default: "default", + Normalizers: []func(string) string{appendA, prependB}, + } + }, + expectedValue: "bdefaulta", + }, + "two normalizers with env vars normalizes env var value with both": { + in: func() *StringVar { + resT := "" + return &StringVar{ + Name: "test", + Target: &resT, + EnvVar: "VAULT_TEST", + Default: "default", + Normalizers: []func(string) string{appendA, prependB}, + } + }, + envVars: map[string]string{"VAULT_TEST": "env_override"}, + expectedValue: "benv_overridea", + }, + } { + t.Run(name, func(t *testing.T) { + for k, v := range test.envVars { + t.Setenv(k, v) + } + fsets := NewFlagSets(nil) + fs := fsets.NewFlagSet("test") + sv := test.in() + fs.StringVar(sv) + require.Equal(t, test.expectedValue, *sv.Target) + }) + } +} diff --git a/command/debug.go b/command/debug.go index 7355372b5e..8e738fa2b7 100644 --- a/command/debug.go +++ b/command/debug.go @@ -25,6 +25,7 @@ import ( "github.com/hashicorp/go-secure-stdlib/strutil" "github.com/hashicorp/vault/api" "github.com/hashicorp/vault/helper/osutil" + "github.com/hashicorp/vault/internalshared/configutil" "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/helper/logging" "github.com/hashicorp/vault/version" @@ -211,7 +212,7 @@ Usage: vault debug [options] output. The command uses the Vault address and token as specified via the login command, environment variables, or CLI flags. - To create a debug package using default duration and interval values in the + To create a debug package using default duration and interval values in the current directory that captures all applicable targets: $ vault debug @@ -495,7 +496,7 @@ func (c *DebugCommand) preflight(rawArgs []string) (string, error) { // Populate initial index fields c.debugIndex = &debugIndex{ - VaultAddress: client.Address(), + VaultAddress: configutil.NormalizeAddr(client.Address()), ClientVersion: version.GetVersion().VersionNumber(), ServerVersion: serverHealth.Version, Compress: c.flagCompress, diff --git a/command/operator_raft_join.go b/command/operator_raft_join.go index aaaaf2891e..ec31e8a286 100644 --- a/command/operator_raft_join.go +++ b/command/operator_raft_join.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/cli" "github.com/hashicorp/vault/api" + "github.com/hashicorp/vault/internalshared/configutil" "github.com/posener/complete" ) @@ -45,7 +46,7 @@ Usage: vault operator raft join [options] 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" diff --git a/command/proxy/config/config_test.go b/command/proxy/config/config_test.go index e0afc50de5..84b4c136e9 100644 --- a/command/proxy/config/config_test.go +++ b/command/proxy/config/config_test.go @@ -10,6 +10,7 @@ import ( "github.com/go-test/deep" "github.com/hashicorp/vault/command/agentproxyshared" "github.com/hashicorp/vault/internalshared/configutil" + "github.com/stretchr/testify/require" ) // TestLoadConfigFile_ProxyCache tests loading a config file containing a cache @@ -203,3 +204,126 @@ func TestLoadConfigFile_ProxyCacheStaticSecrets(t *testing.T) { t.Fatal(diff) } } + +// 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.Parallel() + + 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) { + t.Parallel() + + config, err := LoadConfigFile("./test-fixtures/config-auto-auth-" + name + ".hcl") + require.NoError(t, err) + + expected := &Config{ + SharedConfig: &configutil.SharedConfig{ + PidFile: "./pidfile", + Listeners: []*configutil.Listener{ + { + Type: "unix", + Address: "/path/to/socket", + TLSDisable: true, + SocketMode: "configmode", + SocketUser: "configuser", + SocketGroup: "configgroup", + }, + { + 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", + }, + }, + }, + Vault: &Vault{ + Address: "https://[2001:db8::1]:8200", // Normalized + CACert: "config_ca_cert", + CAPath: "config_ca_path", + TLSSkipVerifyRaw: interface{}("true"), + TLSSkipVerify: true, + ClientCert: "config_client_cert", + ClientKey: "config_client_key", + Retry: &Retry{ + NumRetries: 12, + }, + }, + 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", + }, + }, + }, + }, + APIProxy: &APIProxy{ + EnforceConsistency: "always", + WhenInconsistent: "retry", + UseAutoAuthTokenRaw: true, + UseAutoAuthToken: true, + ForceAutoAuthToken: false, + }, + Cache: &Cache{ + Persist: &agentproxyshared.PersistConfig{ + Type: "kubernetes", + Path: "/vault/agent-cache/", + KeepAfterImport: true, + ExitOnErr: true, + ServiceAccountTokenFile: "/tmp/serviceaccount/token", + }, + }, + } + + config.Prune() + require.EqualValues(t, expected, config) + }) + } +} diff --git a/command/proxy/config/test-fixtures/config-auto-auth-aws.hcl b/command/proxy/config/test-fixtures/config-auto-auth-aws.hcl new file mode 100644 index 0000000000..773f709f12 --- /dev/null +++ b/command/proxy/config/test-fixtures/config-auto-auth-aws.hcl @@ -0,0 +1,76 @@ +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: BUSL-1.1 + +pid_file = "./pidfile" + +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" + } +} + +api_proxy { + use_auto_auth_token = true + enforce_consistency = "always" + when_inconsistent = "retry" +} + +cache { + persist = { + type = "kubernetes" + path = "/vault/agent-cache/" + keep_after_import = true + exit_on_err = true + service_account_token_file = "/tmp/serviceaccount/token" + } +} + +listener "unix" { + address = "/path/to/socket" + tls_disable = true + socket_mode = "configmode" + socket_user = "configuser" + socket_group = "configgroup" +} + +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" +} + +vault { + address = "https://[2001:0db8::0001]:8200" + ca_cert = "config_ca_cert" + ca_path = "config_ca_path" + tls_skip_verify = "true" + client_cert = "config_client_cert" + client_key = "config_client_key" +} diff --git a/command/proxy/config/test-fixtures/config-auto-auth-azure.hcl b/command/proxy/config/test-fixtures/config-auto-auth-azure.hcl new file mode 100644 index 0000000000..3f942a37f4 --- /dev/null +++ b/command/proxy/config/test-fixtures/config-auto-auth-azure.hcl @@ -0,0 +1,78 @@ +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: BUSL-1.1 + +pid_file = "./pidfile" + +auto_auth { + method { + type = "azure" + namespace = "/azure-namespace" + config = { + authenticate_from_environment = true + role = "dev-role" + resource = "https://[2001:0:0:1:0:0:0:1]", + } + } + + sink { + type = "file" + config = { + path = "/tmp/file-foo" + } + aad = "foobar" + dh_type = "curve25519" + dh_path = "/tmp/file-foo-dhpath" + } +} + +api_proxy { + use_auto_auth_token = true + enforce_consistency = "always" + when_inconsistent = "retry" +} + +cache { + persist = { + type = "kubernetes" + path = "/vault/agent-cache/" + keep_after_import = true + exit_on_err = true + service_account_token_file = "/tmp/serviceaccount/token" + } +} + +listener "unix" { + address = "/path/to/socket" + tls_disable = true + socket_mode = "configmode" + socket_user = "configuser" + socket_group = "configgroup" +} + +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" +} + +vault { + address = "https://[2001:0db8::0001]:8200" + ca_cert = "config_ca_cert" + ca_path = "config_ca_path" + tls_skip_verify = "true" + client_cert = "config_client_cert" + client_key = "config_client_key" +} diff --git a/command/proxy/config/test-fixtures/config-auto-auth-gcp.hcl b/command/proxy/config/test-fixtures/config-auto-auth-gcp.hcl new file mode 100644 index 0000000000..a3fc6b701e --- /dev/null +++ b/command/proxy/config/test-fixtures/config-auto-auth-gcp.hcl @@ -0,0 +1,77 @@ +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: BUSL-1.1 + +pid_file = "./pidfile" + +auto_auth { + method { + type = "gcp" + namespace = "/gcp-namespace" + config = { + role = "dev-role" + service_account = "https://[2001:DB8:AC3:FE4::1]" + } + } + + sink { + type = "file" + config = { + path = "/tmp/file-foo" + } + aad = "foobar" + dh_type = "curve25519" + dh_path = "/tmp/file-foo-dhpath" + } +} + +api_proxy { + use_auto_auth_token = true + enforce_consistency = "always" + when_inconsistent = "retry" +} + +cache { + persist = { + type = "kubernetes" + path = "/vault/agent-cache/" + keep_after_import = true + exit_on_err = true + service_account_token_file = "/tmp/serviceaccount/token" + } +} + +listener "unix" { + address = "/path/to/socket" + tls_disable = true + socket_mode = "configmode" + socket_user = "configuser" + socket_group = "configgroup" +} + +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" +} + +vault { + address = "https://[2001:0db8::0001]:8200" + ca_cert = "config_ca_cert" + ca_path = "config_ca_path" + tls_skip_verify = "true" + client_cert = "config_client_cert" + client_key = "config_client_key" +} diff --git a/command/proxy_test.go b/command/proxy_test.go index 3c2fdd0016..90d9ceba69 100644 --- a/command/proxy_test.go +++ b/command/proxy_test.go @@ -2196,3 +2196,115 @@ func TestProxy_Config_ReloadTls(t *testing.T) { t.Fatalf("got a non-zero exit status: %d, stdout/stderr: %s", code, output) } } + +// TestProxy_Config_AddrConformance verifies that the vault address is correctly +// normalized to conform to RFC-5942 §4 when configured by a config file, +// environment variables, or CLI flags. +// See: https://rfc-editor.org/rfc/rfc5952.html +func TestProxy_Config_AddrConformance(t *testing.T) { + for name, test := range map[string]struct { + args []string + envVars map[string]string + cfg string + expected *proxyConfig.Config + }{ + "ipv4 config": { + cfg: ` +vault { + address = "https://127.0.0.1:8200" +} +`, + expected: &proxyConfig.Config{ + Vault: &proxyConfig.Vault{ + Address: "https://127.0.0.1:8200", + }, + }, + }, + "ipv6 config": { + cfg: ` +vault { + address = "https://[2001:0db8::0001]:8200" +} +`, + expected: &proxyConfig.Config{ + Vault: &proxyConfig.Vault{ + // Use the normalized version in the config + Address: "https://[2001:db8::1]:8200", + }, + }, + }, + "ipv6 cli arg overrides": { + args: []string{"-address=https://[2001:0:0:1:0:0:0:1]:8200"}, + cfg: ` +vault { + address = "https://[2001:0db8::0001]:8200" +} +`, + expected: &proxyConfig.Config{ + Vault: &proxyConfig.Vault{ + // Use a normalized version of the args address + Address: "https://[2001:0:0:1::1]:8200", + }, + }, + }, + "ipv6 env var overrides": { + envVars: map[string]string{ + "VAULT_ADDR": "https://[2001:DB8:AC3:FE4::1]:8200", + }, + cfg: ` +vault { + address = "https://[2001:0db8::0001]:8200" +} +`, + expected: &proxyConfig.Config{ + Vault: &proxyConfig.Vault{ + // Use a normalized version of the env var address + Address: "https://[2001:db8:ac3:fe4::1]:8200", + }, + }, + }, + "ipv6 all uses cli overrides": { + args: []string{"-address=https://[2001:0:0:1:0:0:0:1]:8200"}, + envVars: map[string]string{ + "VAULT_ADDR": "https://[2001:DB8:AC3:FE4::1]:8200", + }, + cfg: ` +vault { + address = "https://[2001:0db8::0001]:8200" +} +`, + expected: &proxyConfig.Config{ + Vault: &proxyConfig.Vault{ + // Use a normalized version of the args address + Address: "https://[2001:0:0:1::1]:8200", + }, + }, + }, + } { + t.Run(name, func(t *testing.T) { + // In CI our tests are run with VAULT_ADDR=, which will break our tests + // because it'll default to an unset address. Ensure that's cleared out + // of the environment. + t.Cleanup(func() { + os.Setenv(api.EnvVaultAddress, os.Getenv(api.EnvVaultAddress)) + }) + os.Unsetenv(api.EnvVaultAddress) + for k, v := range test.envVars { + t.Setenv(k, v) + } + + configFile := populateTempFile(t, "proxy-"+strings.ReplaceAll(name, " ", "-"), test.cfg) + cfg, err := proxyConfig.LoadConfigFile(configFile.Name()) + require.NoError(t, err) + require.NotEmptyf(t, cfg.Vault.Address, "proxy config is missing address: %+v", cfg.Vault) + + cmd := &ProxyCommand{BaseCommand: &BaseCommand{}} + f := cmd.Flags() + args := append([]string{}, test.args...) + require.NoError(t, f.Parse(args)) + + cmd.applyConfigOverrides(f, cfg) + require.Equalf(t, test.expected.Vault.Address, cfg.Vault.Address, "proxy config is missing address: config: %+v, flags: %+v", cfg.Vault, f) + }) + } +} diff --git a/command/server.go b/command/server.go index bff21f98fb..20abe21554 100644 --- a/command/server.go +++ b/command/server.go @@ -278,11 +278,12 @@ func (c *ServerCommand) Flags() *FlagSets { }) f.StringVar(&StringVar{ - Name: "dev-listen-address", - Target: &c.flagDevListenAddr, - Default: "127.0.0.1:8200", - EnvVar: "VAULT_DEV_LISTEN_ADDRESS", - Usage: "Address to bind to in \"dev\" mode.", + Name: "dev-listen-address", + Target: &c.flagDevListenAddr, + Default: "127.0.0.1:8200", + EnvVar: "VAULT_DEV_LISTEN_ADDRESS", + Usage: "Address to bind to in \"dev\" mode.", + Normalizers: []func(string) string{configutil.NormalizeAddr}, }) f.BoolVar(&BoolVar{ Name: "dev-no-store-token", @@ -798,7 +799,7 @@ func (c *ServerCommand) setupStorage(config *server.Config) (physical.Backend, e } case storageTypeRaft: if envCA := os.Getenv("VAULT_CLUSTER_ADDR"); envCA != "" { - config.ClusterAddr = envCA + config.ClusterAddr = configutil.NormalizeAddr(envCA) } if len(config.ClusterAddr) == 0 { return nil, errors.New("Cluster address must be set when using raft storage") diff --git a/command/server/config.go b/command/server/config.go index 5d41aeabb2..623e098c38 100644 --- a/command/server/config.go +++ b/command/server/config.go @@ -993,11 +993,11 @@ func ParseStorage(result *Config, list *ast.ObjectList, name string) error { // Override with top-level values if they are set if result.APIAddr != "" { - redirectAddr = result.APIAddr + redirectAddr = configutil.NormalizeAddr(result.APIAddr) } if result.ClusterAddr != "" { - clusterAddr = result.ClusterAddr + clusterAddr = configutil.NormalizeAddr(result.ClusterAddr) } if result.DisableClusteringRaw != nil { @@ -1205,11 +1205,11 @@ func parseHAStorage(result *Config, list *ast.ObjectList, name string) error { // Override with top-level values if they are set if result.APIAddr != "" { - redirectAddr = result.APIAddr + redirectAddr = configutil.NormalizeAddr(result.APIAddr) } if result.ClusterAddr != "" { - clusterAddr = result.ClusterAddr + clusterAddr = configutil.NormalizeAddr(result.ClusterAddr) } if result.DisableClusteringRaw != nil { diff --git a/command/server/config_test_helpers.go b/command/server/config_test_helpers.go index 0153b888da..9d609d2219 100644 --- a/command/server/config_test_helpers.go +++ b/command/server/config_test_helpers.go @@ -1247,13 +1247,13 @@ storage "cockroachdb" { "consul": { config: ` storage "consul" { - address = "2001:db8:0:0:0:0:2:1:8500" + address = "[2001:db8:0:0:0:0:2:1]:8500" path = "vault/" }`, expected: &Storage{ Type: "consul", Config: map[string]string{ - "address": "2001:db8::2:1:8500", + "address": "[2001:db8::2:1]:8500", "path": "vault/", }, }, @@ -1359,7 +1359,7 @@ storage "mssql" { "mysql": { config: ` storage "mysql" { - address = "2001:db8:0:0:0:0:2:1:3306" + address = "[2001:db8:0:0:0:0:2:1]:3306" username = "user1234" password = "secret123!" database = "vault" @@ -1367,7 +1367,7 @@ storage "mysql" { expected: &Storage{ Type: "mysql", Config: map[string]string{ - "address": "2001:db8::2:1:3306", + "address": "[2001:db8::2:1]:3306", "username": "user1234", "password": "secret123!", "database": "vault", @@ -1429,13 +1429,13 @@ storage "swift" { "zookeeper": { config: ` storage "zookeeper" { - address = "2001:db8:0:0:0:0:2:1:2181" + address = "[2001:db8:0:0:0:0:2:1]:2181" path = "vault/" }`, expected: &Storage{ Type: "zookeeper", Config: map[string]string{ - "address": "2001:db8::2:1:2181", + "address": "[2001:db8::2:1]:2181", "path": "vault/", }, }, diff --git a/command/server/listener_tcp.go b/command/server/listener_tcp.go index 5ca52faa04..c105a1ea5a 100644 --- a/command/server/listener_tcp.go +++ b/command/server/listener_tcp.go @@ -19,7 +19,7 @@ import ( ) func tcpListenerFactory(l *configutil.Listener, _ io.Writer, ui cli.Ui) (net.Listener, map[string]string, reloadutil.ReloadFunc, error) { - addr := l.Address + addr := configutil.NormalizeAddr(l.Address) if addr == "" { addr = "127.0.0.1:8200" } diff --git a/http/sys_raft.go b/http/sys_raft.go index e209f0a6f4..ae24a897f1 100644 --- a/http/sys_raft.go +++ b/http/sys_raft.go @@ -12,6 +12,7 @@ import ( "net/http" "github.com/hashicorp/go-secure-stdlib/tlsutil" + "github.com/hashicorp/vault/internalshared/configutil" "github.com/hashicorp/vault/physical/raft" "github.com/hashicorp/vault/vault" ) @@ -80,7 +81,7 @@ func handleSysRaftJoinPost(core *vault.Core, w http.ResponseWriter, r *http.Requ AutoJoin: req.AutoJoin, AutoJoinScheme: req.AutoJoinScheme, AutoJoinPort: req.AutoJoinPort, - LeaderAPIAddr: req.LeaderAPIAddr, + LeaderAPIAddr: configutil.NormalizeAddr(req.LeaderAPIAddr), TLSConfig: tlsConfig, Retry: req.Retry, }, diff --git a/internalshared/configutil/normalize.go b/internalshared/configutil/normalize.go index 902699e57b..80934e72fc 100644 --- a/internalshared/configutil/normalize.go +++ b/internalshared/configutil/normalize.go @@ -4,88 +4,106 @@ package configutil import ( - "fmt" "net" "net/url" "strings" ) -// NormalizeAddr takes an address as a string and returns a normalized copy. -// If the addr is a URL, IP Address, or host:port address that includes an IPv6 -// address, the normalized copy will be conformant with RFC-5942 §4 -// See: https://rfc-editor.org/rfc/rfc5952.html -func NormalizeAddr(address string) string { - if address == "" { +// 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 +func NormalizeAddr(addr string) string { + if addr == "" { return "" } - var ip net.IP - var port string - bracketedIPv6 := false + // Host + ip := net.ParseIP(addr) + if ip != nil { + // net.IP.String() is RFC 5942 §4 compliant + return ip.String() + } - // Try parsing it as a URL - pu, err := url.Parse(address) + // [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 { - // We've been given something that appears to be a URL. See if the hostname - // is an IP address - ip = net.ParseIP(pu.Hostname()) - } else { - // We haven't been given a URL. Try and parse it as an IP address - ip = net.ParseIP(address) + ip := net.ParseIP(host) if ip == nil { - // We haven't been given a URL or IP address, try parsing an IP:Port - // combination. - idx := strings.LastIndex(address, ":") - if idx > 0 { - // We've perhaps received an IP:Port address - addr := address[:idx] - port = address[idx+1:] - if strings.HasPrefix(addr, "[") && strings.HasSuffix(addr, "]") { - addr = strings.TrimPrefix(strings.TrimSuffix(addr, "]"), "[") - bracketedIPv6 = true - } - ip = net.ParseIP(addr) - } - } - } - - // If our IP is nil whatever was passed in does not contain an IP address. - if ip == nil { - return address - } - - if v4 := ip.To4(); v4 != nil { - return address - } - - if v6 := ip.To16(); v6 != nil { - // net.IP String() will return IPv6 RFC-5952 conformant addresses. - - if pu != nil { - // Return the URL in conformant fashion - if port := pu.Port(); port != "" { - pu.Host = fmt.Sprintf("[%s]:%s", v6.String(), port) - } else { - pu.Host = fmt.Sprintf("[%s]", v6.String()) - } - return pu.String() + // Our host isn't an IP address so we can return it unchanged + return addr } - // Handle IP:Port addresses - if port != "" { - // Return the address:port or [address]:port - if bracketedIPv6 { - return fmt.Sprintf("[%s]:%s", v6.String(), port) - } else { - return fmt.Sprintf("%s:%s", v6.String(), port) - } - } - - // Handle just an IP address - return v6.String() + // net.JoinHostPort handles bracketing for RFC 5952 §6 + return net.JoinHostPort(ip.String(), port) } - // It shouldn't be possible to get to this point. If we somehow we manage - // to, return the string unchanged. - return address + // 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 } diff --git a/internalshared/configutil/normalize_test.go b/internalshared/configutil/normalize_test.go index d1aec31e62..80d1d8fdd7 100644 --- a/internalshared/configutil/normalize_test.go +++ b/internalshared/configutil/normalize_test.go @@ -21,26 +21,86 @@ func TestNormalizeAddr(t *testing.T) { isErrorExpected bool }{ "hostname": { + addr: "vaultproject.io", + expected: "vaultproject.io", + }, + "hostname port": { + addr: "vaultproject.io:8200", + expected: "vaultproject.io:8200", + }, + "hostname URL": { + addr: "https://vaultproject.io", + expected: "https://vaultproject.io", + }, + "hostname port URL": { addr: "https://vaultproject.io:8200", expected: "https://vaultproject.io:8200", }, + "hostname destination address": { + addr: "user@vaultproject.io", + expected: "user@vaultproject.io", + }, + "hostname destination address URL": { + addr: "http://user@vaultproject.io", + expected: "http://user@vaultproject.io", + }, + "hostname destination address URL port": { + addr: "http://user@vaultproject.io:8200", + expected: "http://user@vaultproject.io:8200", + }, "ipv4": { addr: "10.10.1.10", expected: "10.10.1.10", }, + "ipv4 invalid bracketed": { + addr: "[10.10.1.10]", + expected: "10.10.1.10", + }, "ipv4 IP:Port addr": { addr: "10.10.1.10:8500", expected: "10.10.1.10:8500", }, + "ipv4 invalid IP:Port addr": { + addr: "[10.10.1.10]:8500", + expected: "10.10.1.10:8500", + }, "ipv4 URL": { addr: "https://10.10.1.10:8200", expected: "https://10.10.1.10:8200", }, - "ipv6 IP:Port addr no brackets": { - addr: "2001:0db8::0001:8500", - expected: "2001:db8::1:8500", + "ipv4 invalid URL": { + addr: "https://[10.10.1.10]:8200", + expected: "https://10.10.1.10:8200", }, - "ipv6 IP:Port addr with brackets": { + "ipv4 destination address": { + addr: "username@10.10.1.10", + expected: "username@10.10.1.10", + }, + "ipv4 invalid destination address": { + addr: "username@10.10.1.10", + expected: "username@10.10.1.10", + }, + "ipv4 destination address port": { + addr: "username@10.10.1.10:8200", + expected: "username@10.10.1.10:8200", + }, + "ipv4 invalid destination address port": { + addr: "username@[10.10.1.10]:8200", + expected: "username@10.10.1.10:8200", + }, + "ipv4 destination address URL": { + addr: "https://username@10.10.1.10", + expected: "https://username@10.10.1.10", + }, + "ipv4 destination address URL port": { + addr: "https://username@10.10.1.10:8200", + expected: "https://username@10.10.1.10:8200", + }, + "ipv6 invalid address": { + addr: "[2001:0db8::0001]", + expected: "2001:db8::1", + }, + "ipv6 IP:Port RFC-5952 4.1 conformance leading zeroes": { addr: "[2001:0db8::0001]:8500", expected: "[2001:db8::1]:8500", }, @@ -52,6 +112,26 @@ func TestNormalizeAddr(t *testing.T) { addr: "https://[2001:0db8::0001]:8200", expected: "https://[2001:db8::1]:8200", }, + "ipv6 bracketed destination address with port RFC-5952 4.1 conformance leading zeroes": { + addr: "username@[2001:0db8::0001]:8200", + expected: "username@[2001:db8::1]:8200", + }, + "ipv6 invalid ambiguous destination address with port": { + addr: "username@2001:0db8::0001:8200", + // Since the address and port are ambiguous the value appears to be + // only an address and as such is normalized as an address only + expected: "username@2001:db8::1:8200", + }, + "ipv6 invalid leading zeroes ambiguous destination address with port": { + addr: "username@2001:db8:0:1:1:1:1:1:8200", + // Since the address and port are ambiguous the value is treated as + // a string because it has too many colons to be a valid IPv6 address. + expected: "username@2001:db8:0:1:1:1:1:1:8200", + }, + "ipv6 destination address no port RFC-5952 4.1 conformance leading zeroes": { + addr: "username@2001:0db8::0001", + expected: "username@2001:db8::1", + }, "ipv6 RFC-5952 4.2.2 conformance one 16-bit 0 field": { addr: "2001:db8:0:1:1:1:1:1", expected: "2001:db8:0:1:1:1:1:1", @@ -60,6 +140,14 @@ func TestNormalizeAddr(t *testing.T) { addr: "https://[2001:db8:0:1:1:1:1:1]:8200", expected: "https://[2001:db8:0:1:1:1:1:1]:8200", }, + "ipv6 destination address with port RFC-5952 4.2.2 conformance one 16-bit 0 field": { + addr: "username@[2001:db8:0:1:1:1:1:1]:8200", + expected: "username@[2001:db8:0:1:1:1:1:1]:8200", + }, + "ipv6 destination address no port RFC-5952 4.2.2 conformance one 16-bit 0 field": { + addr: "username@2001:db8:0:1:1:1:1:1", + expected: "username@2001:db8:0:1:1:1:1:1", + }, "ipv6 RFC-5952 4.2.3 conformance longest run of 0 bits shortened": { addr: "2001:0:0:1:0:0:0:1", expected: "2001:0:0:1::1", @@ -68,14 +156,35 @@ func TestNormalizeAddr(t *testing.T) { addr: "https://[2001:0:0:1:0:0:0:1]:8200", expected: "https://[2001:0:0:1::1]:8200", }, + "ipv6 destination address with port RFC-5952 4.2.3 conformance longest run of 0 bits shortened": { + addr: "username@[2001:0:0:1:0:0:0:1]:8200", + expected: "username@[2001:0:0:1::1]:8200", + }, + "ipv6 destination address no port RFC-5952 4.2.3 conformance longest run of 0 bits shortened": { + addr: "username@2001:0:0:1:0:0:0:1", + expected: "username@2001:0:0:1::1", + }, "ipv6 RFC-5952 4.2.3 conformance equal runs of 0 bits shortened": { addr: "2001:db8:0:0:1:0:0:1", expected: "2001:db8::1:0:0:1", }, - "ipv6 URL RFC-5952 4.2.3 conformance equal runs of 0 bits shortened": { + "ipv6 URL no port RFC-5952 4.2.3 conformance equal runs of 0 bits shortened": { + addr: "https://[2001:db8:0:0:1:0:0:1]", + expected: "https://[2001:db8::1:0:0:1]", + }, + "ipv6 URL with port RFC-5952 4.2.3 conformance equal runs of 0 bits shortened": { addr: "https://[2001:db8:0:0:1:0:0:1]:8200", expected: "https://[2001:db8::1:0:0:1]:8200", }, + + "ipv6 destination address with port RFC-5952 4.2.3 conformance equal runs of 0 bits shortened": { + addr: "username@[2001:db8:0:0:1:0:0:1]:8200", + expected: "username@[2001:db8::1:0:0:1]:8200", + }, + "ipv6 destination address no port RFC-5952 4.2.3 conformance equal runs of 0 bits shortened": { + addr: "username@2001:db8:0:0:1:0:0:1", + expected: "username@2001:db8::1:0:0:1", + }, "ipv6 RFC-5952 4.3 conformance downcase hex letters": { addr: "2001:DB8:AC3:FE4::1", expected: "2001:db8:ac3:fe4::1", @@ -84,6 +193,14 @@ func TestNormalizeAddr(t *testing.T) { addr: "https://[2001:DB8:AC3:FE4::1]:8200", expected: "https://[2001:db8:ac3:fe4::1]:8200", }, + "ipv6 destination address with port RFC-5952 4.3 conformance downcase hex letters": { + addr: "username@[2001:DB8:AC3:FE4::1]:8200", + expected: "username@[2001:db8:ac3:fe4::1]:8200", + }, + "ipv6 destination address no port RFC-5952 4.3 conformance downcase hex letters": { + addr: "username@2001:DB8:AC3:FE4::1", + expected: "username@2001:db8:ac3:fe4::1", + }, } for name, tc := range tests { name := name diff --git a/vault/audit.go b/vault/audit.go index 5e2473b139..478251324a 100644 --- a/vault/audit.go +++ b/vault/audit.go @@ -8,6 +8,7 @@ import ( "crypto/sha256" "errors" "fmt" + "slices" "strconv" "strings" "time" @@ -16,6 +17,7 @@ import ( "github.com/hashicorp/go-uuid" "github.com/hashicorp/vault/audit" "github.com/hashicorp/vault/helper/namespace" + "github.com/hashicorp/vault/internalshared/configutil" "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/helper/salt" @@ -62,6 +64,15 @@ func (c *Core) generateAuditTestProbe() (*logical.LogInput, error) { }, nil } +// auditBackendEntryAddrs maps a backend type to entry options that may need +// to be normalized for conformance. All audit backends must have an entry. +var auditBackendEntryAddrs = map[string][]string{ + "file": {}, + "noop": {}, + "socket": {"address"}, + "syslog": {}, +} + // enableAudit is used to enable a new audit backend that didn't exist in storage beforehand. func (c *Core) enableAudit(ctx context.Context, entry *MountEntry, updateStorage bool) error { // Check ahead of time if the type of audit device we're trying to enable is configured in Vault. @@ -84,6 +95,17 @@ func (c *Core) enableAudit(ctx context.Context, entry *MountEntry, updateStorage return fmt.Errorf("backend path must be specified: %w", audit.ErrExternalOptions) } + // Normalize any entries that might have addresses + keys, ok := auditBackendEntryAddrs[entry.Type] + if !ok { + return fmt.Errorf("backend type '%s' is missing normalization entry", entry.Type) + } + for k, v := range entry.Options { + if slices.Contains(keys, k) { + entry.Options[k] = configutil.NormalizeAddr(v) + } + } + if skipTestRaw, ok := entry.Options["skip_test"]; ok { skipTest, err := parseutil.ParseBool(skipTestRaw) if err != nil { diff --git a/vault/logical_system.go b/vault/logical_system.go index 3520586e64..a96c40f4e6 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -6488,7 +6488,7 @@ This path responds to the following HTTP methods. }, "alias_identifier": { - `It is the name of the alias (user). For example, if the alias belongs to userpass backend, + `It is the name of the alias (user). For example, if the alias belongs to userpass backend, the name should be a valid username within userpass auth method. If the alias belongs to an approle auth method, the name should be a valid RoleID`, "", diff --git a/vault/raft.go b/vault/raft.go index 14b0d75904..4adcab6211 100644 --- a/vault/raft.go +++ b/vault/raft.go @@ -27,6 +27,7 @@ import ( raftlib "github.com/hashicorp/raft" "github.com/hashicorp/vault/api" httpPriority "github.com/hashicorp/vault/http/priority" + "github.com/hashicorp/vault/internalshared/configutil" "github.com/hashicorp/vault/physical/raft" "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/logical" @@ -1301,7 +1302,7 @@ func (c *Core) raftLeaderInfo(leaderInfo *raft.LeaderJoinInfo, disco *discover.D } for _, ip := range clusterIPs { addr := formatDiscoveredAddr(ip, port) - u := fmt.Sprintf("%s://%s", scheme, addr) + u := configutil.NormalizeAddr(fmt.Sprintf("%s://%s", scheme, addr)) info := *leaderInfo info.LeaderAPIAddr = u ret = append(ret, &info)