From 58a49e6ce08643f4690ee25732042a2e7264e03c Mon Sep 17 00:00:00 2001 From: Ryan Cragun Date: Thu, 27 Feb 2025 15:57:46 -0700 Subject: [PATCH] VAULT-33758: IPv6 address conformance for proxy and agent (#29517) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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]: https://github.com/hashicorp/vault/pull/29228 --- changelog/29517.txt | 6 + command/agent.go | 15 +- command/agent/config/config.go | 57 ++++++ command/agent/config/config_test.go | 174 +++++++++++------- .../test-fixtures/config-auto-auth-aws.hcl | 69 +++++++ .../test-fixtures/config-auto-auth-azure.hcl | 71 +++++++ .../test-fixtures/config-auto-auth-gcp.hcl | 70 +++++++ .../test-fixtures/config-embedded-type.hcl | 35 ---- command/agent/config/test-fixtures/config.hcl | 37 ---- command/agent_test.go | 107 +++++++++++ command/base.go | 23 ++- command/base_flags.go | 81 +++++--- command/base_test.go | 146 +++++++++++---- command/debug.go | 5 +- command/operator_raft_join.go | 5 +- command/proxy.go | 15 +- command/proxy/config/config.go | 58 ++++++ command/proxy/config/config_test.go | 124 +++++++++++++ .../test-fixtures/config-auto-auth-aws.hcl | 76 ++++++++ .../test-fixtures/config-auto-auth-azure.hcl | 78 ++++++++ .../test-fixtures/config-auto-auth-gcp.hcl | 77 ++++++++ command/proxy_test.go | 112 +++++++++++ command/server.go | 13 +- command/server/config.go | 8 +- command/server/config_test_helpers.go | 12 +- command/server/listener_tcp.go | 2 +- http/sys_raft.go | 3 +- internalshared/configutil/normalize.go | 156 +++++++++------- internalshared/configutil/normalize_test.go | 127 ++++++++++++- vault/audit.go | 22 +++ vault/logical_system.go | 2 +- vault/raft.go | 3 +- 32 files changed, 1474 insertions(+), 315 deletions(-) create mode 100644 changelog/29517.txt create mode 100644 command/agent/config/test-fixtures/config-auto-auth-aws.hcl create mode 100644 command/agent/config/test-fixtures/config-auto-auth-azure.hcl create mode 100644 command/agent/config/test-fixtures/config-auto-auth-gcp.hcl delete mode 100644 command/agent/config/test-fixtures/config-embedded-type.hcl delete mode 100644 command/agent/config/test-fixtures/config.hcl create mode 100644 command/proxy/config/test-fixtures/config-auto-auth-aws.hcl create mode 100644 command/proxy/config/test-fixtures/config-auto-auth-azure.hcl create mode 100644 command/proxy/config/test-fixtures/config-auto-auth-gcp.hcl 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)