From 784b473e46cb7776cc0ac9bdbb73ce279500c750 Mon Sep 17 00:00:00 2001 From: Ryan Cragun Date: Wed, 11 Dec 2024 15:08:16 -0700 Subject: [PATCH 01/12] =?UTF-8?q?VAULT-33008:=20ipv6:=20always=20display?= =?UTF-8?q?=20RFC-5952=20=C2=A74=20conformant=20addresses?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit USGv6[0] requires implementing §4.1.1 of the NISTv6-r1 profile[1] for IPv6-Only capabilities. This section requires that whenever Vault displays IPv6 addresses (including CLI output, Web UI, logs, etc.) that _all_ IPv6 addresses must conform to RFC-5952 §4 text representation recommendations[2]. These recommendations do not prevent us from accepting RFC-4241 IPv6 addresses, however, whenever these same addresses are displayed they must conform to the strict RFC-5952 §4 guidelines. This PR implements handling of IPv6 address conformance in our `vault server` routine. We handling conformance normalization in all server, http_proxy, listener, seal, storage and telemetry configuration where an input could contain an IPv6 address, whether configued via an HCL file or via corresponding environment variables. The approach I've taken is to handle conformance normalization at parse time to ensure that all output log output and subsequent usage inside of Vaults various subsystems always reference a conformant address, that way we don't need concern ourselves with conformance later. This approach ought to be backwards compatible to prior loose address configuration requirements, with the understanding that goinging forward all IPv6 representation will be strict regardless of what has been configured. In many cases I've updated our various parser functions to call the new `configutil.NormalizeAddr()` to apply conformance normalization. Others required no changes because they rely on standard library URL string output, which always displays IPv6 URLs in a conformant way. Not included in this changes is any other vault exec mode other than server. Client, operator commands, agent mode, proxy mode, etc. will be included in subsequent changes if necessary. [0]: https://www.nist.gov/publications/usgv6-profile [1]: https://www.nist.gov/publications/nist-ipv6-profile [2]: https://www.rfc-editor.org/rfc/rfc5952.html#section-4 [3]: https://www.rfc-editor.org/rfc/rfc4291 Signed-off-by: Ryan Cragun --- internalshared/configutil/listener.go | 40 +++++++++++++++++++++- internalshared/configutil/listener_test.go | 40 ++++++++++++++++++++-- 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/internalshared/configutil/listener.go b/internalshared/configutil/listener.go index b9ed168abf7e..7489109939c0 100644 --- a/internalshared/configutil/listener.go +++ b/internalshared/configutil/listener.go @@ -6,7 +6,9 @@ package configutil import ( "errors" "fmt" + "net" "net/textproto" + "net/url" "regexp" "strings" "time" @@ -177,7 +179,7 @@ func (l *Listener) Validate(path string) []ConfigError { func ParseSingleIPTemplate(ipTmpl string) (string, error) { r := regexp.MustCompile("{{.*?}}") if !r.MatchString(ipTmpl) { - return ipTmpl, nil + return normalizeIfURL(ipTmpl), nil } out, err := template.Parse(ipTmpl) @@ -196,6 +198,42 @@ func ParseSingleIPTemplate(ipTmpl string) (string, error) { } } +// normalizeIfURL takes a URL as a string and returns a conformant copy of the +// same URL. If the given string is not a URL, or if the URL's Hostname is either +// a DNS hostname or IPv4 address, the given URL will be returned unchanged. +// If the URL hostname is an IPv6 address then the address will normalized to +// conform to RFC-5952. +// See: https://rfc-editor.org/rfc/rfc5952.html +func normalizeIfURL(u string) string { + pu, err := url.Parse(u) + if err != nil { + return u + } + + ip := net.ParseIP(pu.Hostname()) + if ip == nil { + // We've been given a valid URL but the Hostname is not an IP address. + return pu.String() + } + + if v4 := ip.To4(); v4 != nil { + // We don't need to normalize IPv4 addresses. + return pu.String() + } + + if v6 := ip.To16(); v6 != nil { + // net.IP String() will return IPv6 RFC-5952 conformant addresses. + 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() + } + + return pu.String() +} + // ParseListeners attempts to parse the AST list of objects into listeners. func ParseListeners(list *ast.ObjectList) ([]*Listener, error) { listeners := make([]*Listener, len(list.Items)) diff --git a/internalshared/configutil/listener_test.go b/internalshared/configutil/listener_test.go index 51d0c094ed3b..e5e43e9b2160 100644 --- a/internalshared/configutil/listener_test.go +++ b/internalshared/configutil/listener_test.go @@ -16,17 +16,49 @@ import ( // ensure that we only attempt to parse templates when the input contains a // template placeholder (see: go-sockaddr/template). func TestListener_ParseSingleIPTemplate(t *testing.T) { + t.Parallel() + tests := map[string]struct { arg string want string isErrorExpected bool errorMessage string }{ - "test https addr": { + "test hostname": { arg: "https://vaultproject.io:8200", want: "https://vaultproject.io:8200", isErrorExpected: false, }, + "test ipv4": { + arg: "https://10.10.1.10:8200", + want: "https://10.10.1.10:8200", + isErrorExpected: false, + }, + "test ipv6 RFC-5952 4.1 leading zeroes": { + arg: "https://[2001:0db8::0001]:8200", + want: "https://[2001:db8::1]:8200", + isErrorExpected: false, + }, + "test ipv6 RFC-5952 4.2.2 one 16-bit 0 field": { + arg: "https://[2001:db8:0:1:1:1:1:1]:8200", + want: "https://[2001:db8:0:1:1:1:1:1]:8200", + isErrorExpected: false, + }, + "test ipv6 RFC-5952 4.2.3 longest run of 0 bits shortened": { + arg: "https://[2001:0:0:1:0:0:0:1]:8200", + want: "https://[2001:0:0:1::1]:8200", + isErrorExpected: false, + }, + "test ipv6 RFC-5952 4.2.3 equal runs of 0 bits shortened": { + arg: "https://[2001:db8:0:0:1:0:0:1]:8200", + want: "https://[2001:db8::1:0:0:1]:8200", + isErrorExpected: false, + }, + "test ipv6 RFC-5952 4.3 downcase hex letters": { + arg: "https://[2001:DB8:AC3:FE4::1]:8200", + want: "https://[2001:db8:ac3:fe4::1]:8200", + isErrorExpected: false, + }, "test invalid template func": { arg: "{{ FooBar }}", want: "", @@ -34,8 +66,9 @@ func TestListener_ParseSingleIPTemplate(t *testing.T) { errorMessage: "unable to parse address template", }, "test partial template": { - arg: "{{FooBar", - want: "{{FooBar", + arg: "{{FooBar", + // Partial templates get treated as literal hostname and get URL encoded + want: "%7B%7BFooBar", isErrorExpected: false, }, } @@ -43,6 +76,7 @@ func TestListener_ParseSingleIPTemplate(t *testing.T) { name := name tc := tc t.Run(name, func(t *testing.T) { + t.Parallel() got, err := ParseSingleIPTemplate(tc.arg) if tc.isErrorExpected { From 24b44b4f67eac5a69e1fc2fe87ab94e424c44a12 Mon Sep 17 00:00:00 2001 From: Ryan Cragun Date: Wed, 11 Dec 2024 17:19:32 -0700 Subject: [PATCH 02/12] ipv6: normalize telemetry addresses Signed-off-by: Ryan Cragun --- internalshared/configutil/telemetry.go | 36 ++++++++++++++++- internalshared/configutil/telemetry_test.go | 44 +++++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/internalshared/configutil/telemetry.go b/internalshared/configutil/telemetry.go index 7c49fce00917..af81f945bfce 100644 --- a/internalshared/configutil/telemetry.go +++ b/internalshared/configutil/telemetry.go @@ -16,6 +16,8 @@ import ( "github.com/armon/go-metrics/prometheus" stackdriver "github.com/google/go-metrics-stackdriver" stackdrivervault "github.com/google/go-metrics-stackdriver/vault" + "google.golang.org/api/option" + "github.com/hashicorp/cli" "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-secure-stdlib/parseutil" @@ -23,7 +25,6 @@ import ( "github.com/hashicorp/hcl/hcl/ast" "github.com/hashicorp/vault/helper/metricsutil" "github.com/hashicorp/vault/sdk/helper/metricregistry" - "google.golang.org/api/option" ) const ( @@ -34,7 +35,8 @@ const ( NumLeaseMetricsTimeBucketsDefault = 168 ) -// Telemetry is the telemetry configuration for the server +// Telemetry is the telemetry configuration for the server. +// NOTE: When adding new address or URL fields be sure to update normalizeTelemetryAddresses(). type Telemetry struct { FoundKeys []string `hcl:",decodedFields"` UnusedKeys UnusedKeyMap `hcl:",unusedKeyPositions"` @@ -192,6 +194,9 @@ func parseTelemetry(result *SharedConfig, list *ast.ObjectList) error { return multierror.Prefix(err, "telemetry:") } + // Make sure addresses conform to RFC-5952 + normalizeTelemetryAddresses(result.Telemetry) + if result.Telemetry.PrometheusRetentionTimeRaw != nil { var err error if result.Telemetry.PrometheusRetentionTime, err = parseutil.ParseDurationSecond(result.Telemetry.PrometheusRetentionTimeRaw); err != nil { @@ -241,6 +246,33 @@ func parseTelemetry(result *SharedConfig, list *ast.ObjectList) error { return nil } +// normalizeTelemetryAddresses takes a reference to Telemetry and ensures that +// any address and URL configuration options that are IPv6 conform to RFC-5952. +// See: https://rfc-editor.org/rfc/rfc5952.html +func normalizeTelemetryAddresses(in *Telemetry) { + if in == nil { + return + } + + // Make sure addresses conform to RFC-5952 + for _, addr := range []*string{ + &in.CirconusAPIURL, + &in.CirconusCheckSubmissionURL, + &in.DogStatsDAddr, + &in.StatsdAddr, + &in.StatsiteAddr, + } { + if addr == nil { + continue + } + + if url := *addr; url != "" { + n := normalizeIfURL(url) + *addr = n + } + } +} + type SetupTelemetryOpts struct { Config *Telemetry Ui cli.Ui diff --git a/internalshared/configutil/telemetry_test.go b/internalshared/configutil/telemetry_test.go index 285278eeaeba..8b6ecb255e86 100644 --- a/internalshared/configutil/telemetry_test.go +++ b/internalshared/configutil/telemetry_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestParsePrefixFilters(t *testing.T) { @@ -54,3 +55,46 @@ func TestParsePrefixFilters(t *testing.T) { } }) } + +// Test_normalizeTelemetryAddresses verifies that addr and URL configuration is +// normalized to conform to RFC-5952. +func Test_normalizeTelemetryAddresses(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + given *Telemetry + expected *Telemetry + }{ + "ipv6-conformance": { + given: &Telemetry{ + // RFC-5952 4.1 leading zeroes + CirconusAPIURL: "https://[2001:0db8::0001]:443", + // RFC-5952 4.2.3 longest run of 0 bits shortened + CirconusCheckSubmissionURL: "https://[2001:0:0:1:0:0:0:1]:443", + // RFC-5952 4.2.3 equal runs of 0 bits shortened + DogStatsDAddr: "https://[2001:db8:0:0:1:0:0:1]:443", + // RFC-5952 4.3 downcase hex letters + StatsdAddr: "https://[2001:DB8:AC3:FE4::1]:443", + StatsiteAddr: "https://[2001:DB8:AC3:FE4::1]:443", + }, + expected: &Telemetry{ + CirconusAPIURL: "https://[2001:db8::1]:443", + CirconusCheckSubmissionURL: "https://[2001:0:0:1::1]:443", + DogStatsDAddr: "https://[2001:db8::1:0:0:1]:443", + StatsdAddr: "https://[2001:db8:ac3:fe4::1]:443", + StatsiteAddr: "https://[2001:db8:ac3:fe4::1]:443", + }, + }, + } + + for name, tc := range tests { + name := name + tc := tc + t.Run(name, func(t *testing.T) { + t.Parallel() + + normalizeTelemetryAddresses(tc.given) + require.EqualValues(t, tc.expected, tc.given) + }) + } +} From f6b14170b11ce97242ceaf1a245ce90b7892df88 Mon Sep 17 00:00:00 2001 From: Ryan Cragun Date: Fri, 13 Dec 2024 14:40:24 -0700 Subject: [PATCH 03/12] ipv6: normalize storage addresses Signed-off-by: Ryan Cragun --- command/server/config.go | 85 +++++- command/server/config_test.go | 7 +- command/server/config_test_helpers.go | 307 ++++++++++++++++++++- internalshared/configutil/listener.go | 94 +++++-- internalshared/configutil/listener_test.go | 96 ++++++- internalshared/configutil/telemetry.go | 2 +- 6 files changed, 551 insertions(+), 40 deletions(-) diff --git a/command/server/config.go b/command/server/config.go index 4c1951c0249a..cfd57ce0d96c 100644 --- a/command/server/config.go +++ b/command/server/config.go @@ -11,10 +11,13 @@ import ( "math" "os" "path/filepath" + "slices" "strconv" "strings" "time" + "github.com/mitchellh/mapstructure" + "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-secure-stdlib/parseutil" "github.com/hashicorp/hcl" @@ -25,7 +28,6 @@ import ( "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/sdk/helper/strutil" "github.com/hashicorp/vault/sdk/helper/testcluster" - "github.com/mitchellh/mapstructure" ) const ( @@ -934,33 +936,39 @@ func ParseStorage(result *Config, list *ast.ObjectList, name string) error { } m := make(map[string]string) - for key, val := range config { + for k, val := range config { valStr, ok := val.(string) if ok { - m[key] = valStr + var err error + m[k], err = normalizeStorageConfigAddresses(key, k, valStr) + if err != nil { + return err + } continue } + // TODO(ryancragun): We don't currently normalize nested configuration here + // that we might have otherwise, e.g.: raft's leader_api_addr. valBytes, err := json.Marshal(val) if err != nil { return err } - m[key] = string(valBytes) + m[k] = string(valBytes) } // Pull out the redirect address since it's common to all backends var redirectAddr string if v, ok := m["redirect_addr"]; ok { - redirectAddr = v + redirectAddr = configutil.NormalizeAddr(v) delete(m, "redirect_addr") } else if v, ok := m["advertise_addr"]; ok { - redirectAddr = v + redirectAddr = configutil.NormalizeAddr(v) delete(m, "advertise_addr") } // Pull out the cluster address since it's common to all backends var clusterAddr string if v, ok := m["cluster_addr"]; ok { - clusterAddr = v + clusterAddr = configutil.NormalizeAddr(v) delete(m, "cluster_addr") } @@ -997,6 +1005,51 @@ func ParseStorage(result *Config, list *ast.ObjectList, name string) error { return nil } +// storageAddressKeys is a map to physical storage configuration keys that might +// contain URLs, IP addresses, or IP:port style addresses. All physical storage +// types must have an entry in this map, otherwise our normalization check will +// fail when parsing the storage entry config. +var storageAddressKeys = map[string][]string{ + "aerospike": {"hostname"}, + "alicloudoss": {"endpoint"}, + "azure": {"arm_endpoint"}, + "cassandra": {"hosts"}, + "cockroachdb": {"connection_url"}, + "consul": {"address"}, + "couchdb": {"endpoint"}, + "dynamodb": {"endpoint"}, + "etcd": {"address", "discovery_srv"}, + "filesystem": {}, + "foundationdb": {}, + "gcs": {}, + "inmem": {}, + "manta": {"url"}, + "mssql": {"server"}, + "mysql": {"address"}, + "oci": {}, + "postgresql": {"connection_url"}, + "raft": {}, + "s3": {"endpoint"}, + "spanner": {}, + "swift": {"auth_url", "storage_url"}, + "zookeeper": {"address"}, +} + +// normalizeStorageConfigAddresses takes a storage name, config key, and config +// value and will normalize any URLs, IP addresses, or IP:port style addresses. +func normalizeStorageConfigAddresses(storage string, key string, value string) (string, error) { + keys, ok := storageAddressKeys[storage] + if !ok { + return "", fmt.Errorf("unknown storage type %s", storage) + } + + if slices.Contains(keys, key) { + return configutil.NormalizeAddr(value), nil + } + + return value, nil +} + func parseHAStorage(result *Config, list *ast.ObjectList, name string) error { if len(list.Items) > 1 { return fmt.Errorf("only one %q block is permitted", name) @@ -1016,33 +1069,39 @@ func parseHAStorage(result *Config, list *ast.ObjectList, name string) error { } m := make(map[string]string) - for key, val := range config { + for k, val := range config { valStr, ok := val.(string) if ok { - m[key] = valStr + var err error + m[k], err = normalizeStorageConfigAddresses(key, k, valStr) + if err != nil { + return err + } continue } + // TODO(ryancragun): We don't currently normalize nested configuration here + // that we might have otherwise, e.g.: raft's leader_api_addr. valBytes, err := json.Marshal(val) if err != nil { return err } - m[key] = string(valBytes) + m[k] = string(valBytes) } // Pull out the redirect address since it's common to all backends var redirectAddr string if v, ok := m["redirect_addr"]; ok { - redirectAddr = v + redirectAddr = configutil.NormalizeAddr(v) delete(m, "redirect_addr") } else if v, ok := m["advertise_addr"]; ok { - redirectAddr = v + redirectAddr = configutil.NormalizeAddr(v) delete(m, "advertise_addr") } // Pull out the cluster address since it's common to all backends var clusterAddr string if v, ok := m["cluster_addr"]; ok { - clusterAddr = v + clusterAddr = configutil.NormalizeAddr(v) delete(m, "cluster_addr") } diff --git a/command/server/config_test.go b/command/server/config_test.go index 9fa20b182fd2..036409c2dbe0 100644 --- a/command/server/config_test.go +++ b/command/server/config_test.go @@ -9,8 +9,9 @@ import ( "strings" "testing" - "github.com/hashicorp/vault/internalshared/configutil" "github.com/stretchr/testify/require" + + "github.com/hashicorp/vault/internalshared/configutil" ) func TestLoadConfigFile(t *testing.T) { @@ -65,6 +66,10 @@ func TestParseStorage(t *testing.T) { testParseStorageTemplate(t) } +func TestParseStorageURLConformance(t *testing.T) { + testParseStorageURLConformance(t) +} + // TestConfigWithAdministrativeNamespace tests that .hcl and .json configurations are correctly parsed when the administrative_namespace_path is present. func TestConfigWithAdministrativeNamespace(t *testing.T) { testConfigWithAdministrativeNamespaceHcl(t) diff --git a/command/server/config_test_helpers.go b/command/server/config_test_helpers.go index 258801dbfe4e..6301d558a459 100644 --- a/command/server/config_test_helpers.go +++ b/command/server/config_test_helpers.go @@ -12,11 +12,12 @@ import ( "time" "github.com/go-test/deep" + "github.com/stretchr/testify/require" + "github.com/hashicorp/hcl" "github.com/hashicorp/hcl/hcl/ast" "github.com/hashicorp/hcl/hcl/token" "github.com/hashicorp/vault/internalshared/configutil" - "github.com/stretchr/testify/require" ) var DefaultCustomHeaders = map[string]map[string]string{ @@ -1124,6 +1125,310 @@ ha_storage "consul" { } } +func testParseStorageURLConformance(t *testing.T) { + t.Parallel() + + for name, tc := range map[string]struct { + config string + expected *Storage + shouldFail bool + }{ + "aerospike": { + config: ` +storage "aerospike" { + hostname = "2001:db8:0:0:0:0:2:1" + port = "3000" + namespace = "test" + set = "vault" + username = "admin" + password = "admin" +}`, + expected: &Storage{ + Type: "aerospike", + Config: map[string]string{ + "hostname": "2001:db8::2:1", + "port": "3000", + "namespace": "test", + "set": "vault", + "username": "admin", + "password": "admin", + }, + }, + }, + "alicloudoss": { + config: ` +storage "alicloudoss" { + access_key = "abcd1234" + secret_key = "defg5678" + endpoint = "2001:db8:0:0:0:0:2:1" + bucket = "my-bucket" +}`, + expected: &Storage{ + Type: "alicloudoss", + Config: map[string]string{ + "access_key": "abcd1234", + "secret_key": "defg5678", + "endpoint": "2001:db8::2:1", + "bucket": "my-bucket", + }, + }, + }, + "azure": { + config: ` +storage "azure" { + accountName = "my-storage-account" + accountKey = "abcd1234" + arm_endpoint = "2001:db8:0:0:0:0:2:1" + container = "container-efgh5678" + environment = "AzurePublicCloud" +}`, + expected: &Storage{ + Type: "azure", + Config: map[string]string{ + "accountName": "my-storage-account", + "accountKey": "abcd1234", + "arm_endpoint": "2001:db8::2:1", + "container": "container-efgh5678", + "environment": "AzurePublicCloud", + }, + }, + }, + "cassandra": { + config: ` +storage "cassandra" { + hosts = "2001:db8:0:0:0:0:2:1" + consistency = "LOCAL_QUORUM" + protocol_version = 3 +}`, + expected: &Storage{ + Type: "cassandra", + Config: map[string]string{ + "hosts": "2001:db8::2:1", + "consistency": "LOCAL_QUORUM", + "protocol_version": "3", + }, + }, + }, + "cockroachdb": { + config: ` +storage "cockroachdb" { + connection_url = "postgres://user123:secret123!@2001:db8:0:0:0:0:2:1:5432/vault" + table = "vault_kv_store" +}`, + expected: &Storage{ + Type: "cockroachdb", + Config: map[string]string{ + "connection_url": "postgres://user123:secret123%21@[2001:db8::2:1]:5432/vault", + "table": "vault_kv_store", + }, + }, + }, + "consul": { + config: ` +storage "consul" { + 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", + "path": "vault/", + }, + }, + }, + "couchdb": { + config: ` +storage "couchdb" { + endpoint = "https://[2001:db8:0:0:0:0:2:1]:5984/my-database" + username = "admin" + password = "admin" +}`, + expected: &Storage{ + Type: "couchdb", + Config: map[string]string{ + "endpoint": "https://[2001:db8::2:1]:5984/my-database", + "username": "admin", + "password": "admin", + }, + }, + }, + "dynamodb": { + config: ` +storage "dynamodb" { + endpoint = "https://[2001:db8:0:0:0:0:2:1]:5984/my-aws-endpoint" + ha_enabled = "true" + region = "us-west-2" + table = "vault-data" +}`, + expected: &Storage{ + Type: "dynamodb", + Config: map[string]string{ + "endpoint": "https://[2001:db8::2:1]:5984/my-aws-endpoint", + "ha_enabled": "true", + "region": "us-west-2", + "table": "vault-data", + }, + }, + }, + "etcd": { + config: ` +storage "etcd" { + address = "https://[2001:db8:0:0:0:0:2:1]:2379" + discovery_srv = "https://[2001:db8:0:0:1:0:0:1]" + etcd_api = "v3" +}`, + expected: &Storage{ + Type: "etcd", + Config: map[string]string{ + "address": "https://[2001:db8::2:1]:2379", + "discovery_srv": "https://[2001:db8::1:0:0:1]", + "etcd_api": "v3", + }, + }, + }, + "manta": { + config: ` +storage "manta" { + directory = "manta-directory" + user = "myuser" + key_id = "40:9d:d3:f9:0b:86:62:48:f4:2e:a5:8e:43:00:2a:9b" + url = "https://[2001:db8:0:0:0:0:2:1]" +}`, + expected: &Storage{ + Type: "manta", + Config: map[string]string{ + "directory": "manta-directory", + "user": "myuser", + "key_id": "40:9d:d3:f9:0b:86:62:48:f4:2e:a5:8e:43:00:2a:9b", + "url": "https://[2001:db8::2:1]", + }, + }, + }, + "mssql": { + config: ` +storage "mssql" { + server = "2001:db8:0:0:0:0:2:1" + port = 1433 + username = "user1234" + password = "secret123!" + database = "vault" + table = "vault" + appname = "vault" + schema = "dbo" + connectionTimeout = 30 + logLevel = 0 +}`, + expected: &Storage{ + Type: "mssql", + Config: map[string]string{ + "server": "2001:db8::2:1", + "port": "1433", + "username": "user1234", + "password": "secret123!", + "database": "vault", + "table": "vault", + "appname": "vault", + "schema": "dbo", + "connectionTimeout": "30", + "logLevel": "0", + }, + }, + }, + "mysql": { + config: ` +storage "mysql" { + address = "2001:db8:0:0:0:0:2:1:3306" + username = "user1234" + password = "secret123!" + database = "vault" +}`, + expected: &Storage{ + Type: "mysql", + Config: map[string]string{ + "address": "2001:db8::2:1:3306", + "username": "user1234", + "password": "secret123!", + "database": "vault", + }, + }, + }, + "postgresql": { + config: ` +storage "postgresql" { + connection_url = "postgres://user123:secret123!@2001:db8:0:0:0:0:2:1:5432/vault" + table = "vault_kv_store" +}`, + expected: &Storage{ + Type: "postgresql", + Config: map[string]string{ + "connection_url": "postgres://user123:secret123%21@[2001:db8::2:1]:5432/vault", + "table": "vault_kv_store", + }, + }, + }, + "s3": { + config: ` +storage "s3" { + endpoint = "https://[2001:db8:0:0:0:0:2:1]:5984/my-aws-endpoint" + access_key = "abcd1234" + secret_key = "defg5678" + bucket = "my-bucket" +}`, + expected: &Storage{ + Type: "s3", + Config: map[string]string{ + "endpoint": "https://[2001:db8::2:1]:5984/my-aws-endpoint", + "access_key": "abcd1234", + "secret_key": "defg5678", + "bucket": "my-bucket", + }, + }, + }, + "swift": { + config: ` +storage "swift" { + auth_url = "https://[2001:db8:0:0:0:0:2:1]/auth" + storage_url = "https://[2001:db8:0:0:0:0:2:1]/storage" + username = "admin" + password = "secret123!" + container = "my-storage-container" +}`, + expected: &Storage{ + Type: "swift", + Config: map[string]string{ + "auth_url": "https://[2001:db8::2:1]/auth", + "storage_url": "https://[2001:db8::2:1]/storage", + "username": "admin", + "password": "secret123!", + "container": "my-storage-container", + }, + }, + }, + "zookeeper": { + config: ` +storage "zookeeper" { + 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", + "path": "vault/", + }, + }, + }, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + config, err := ParseConfig(tc.config, "") + require.NoError(t, err) + require.EqualValues(t, tc.expected, config.Storage) + }) + } +} + func testParseSeals(t *testing.T) { config, err := LoadConfigFile("./test-fixtures/config_seals.hcl") if err != nil { diff --git a/internalshared/configutil/listener.go b/internalshared/configutil/listener.go index 7489109939c0..5564f6fa98f2 100644 --- a/internalshared/configutil/listener.go +++ b/internalshared/configutil/listener.go @@ -179,7 +179,7 @@ func (l *Listener) Validate(path string) []ConfigError { func ParseSingleIPTemplate(ipTmpl string) (string, error) { r := regexp.MustCompile("{{.*?}}") if !r.MatchString(ipTmpl) { - return normalizeIfURL(ipTmpl), nil + return NormalizeAddr(ipTmpl), nil } out, err := template.Parse(ipTmpl) @@ -198,40 +198,96 @@ func ParseSingleIPTemplate(ipTmpl string) (string, error) { } } -// normalizeIfURL takes a URL as a string and returns a conformant copy of the -// same URL. If the given string is not a URL, or if the URL's Hostname is either -// a DNS hostname or IPv4 address, the given URL will be returned unchanged. -// If the URL hostname is an IPv6 address then the address will normalized to -// conform to RFC-5952. +// NormalizeAddr takes a string and returns a conformant copy of it. If the given +// string is not a URL, or if the URL's Hostname is either a DNS hostname or +// IPv4 address, the given URL will be returned unchanged. If the URL hostname +// is an IPv6 address then the address will normalized to conform to RFC-5952. // See: https://rfc-editor.org/rfc/rfc5952.html -func normalizeIfURL(u string) string { +func NormalizeAddr(u string) string { + if u == "" { + return "" + } + + var ip net.IP + var port string + bracketedIPv6 := false + + // Try parsing it as a URL pu, err := url.Parse(u) - if err != nil { - return u + 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(u) + if ip == nil { + // We haven't been given a URL or IP address, try parsing an IP:Port + // combination. + idx := strings.LastIndex(u, ":") + if idx > 0 { + // We've perhaps received an IP:Port address + addr := u[:idx] + port = u[idx+1:] + if strings.HasPrefix(addr, "[") && strings.HasSuffix(addr, "]") { + addr = strings.TrimPrefix(strings.TrimSuffix(addr, "]"), "[") + bracketedIPv6 = true + } + ip = net.ParseIP(addr) + } + } } - ip := net.ParseIP(pu.Hostname()) + // If our IP is nil whatever was passed in does not contain an IP address. if ip == nil { - // We've been given a valid URL but the Hostname is not an IP address. - return pu.String() + return u } if v4 := ip.To4(); v4 != nil { // We don't need to normalize IPv4 addresses. - return pu.String() + if pu != nil && port == "" { + return pu.String() + } + + if port != "" { + // Return the address:port + return fmt.Sprintf("%s:%s", v4.String(), port) + } + + // Return the ip addres + return v4.String() } if v6 := ip.To16(); v6 != nil { // net.IP String() will return IPv6 RFC-5952 conformant addresses. - if port := pu.Port(); port != "" { - pu.Host = fmt.Sprintf("[%s]:%s", v6.String(), port) - } else { - pu.Host = fmt.Sprintf("[%s]", v6.String()) + + 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() } - return pu.String() + + // 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() } - return pu.String() + // It shouldn't be possible to get to this point. If we somehow we manage + // to, return the string unchanged. + return u } // ParseListeners attempts to parse the AST list of objects into listeners. diff --git a/internalshared/configutil/listener_test.go b/internalshared/configutil/listener_test.go index e5e43e9b2160..e246a3c57ced 100644 --- a/internalshared/configutil/listener_test.go +++ b/internalshared/configutil/listener_test.go @@ -34,27 +34,27 @@ func TestListener_ParseSingleIPTemplate(t *testing.T) { want: "https://10.10.1.10:8200", isErrorExpected: false, }, - "test ipv6 RFC-5952 4.1 leading zeroes": { + "test ipv6 RFC-5952 4.1 conformance leading zeroes": { arg: "https://[2001:0db8::0001]:8200", want: "https://[2001:db8::1]:8200", isErrorExpected: false, }, - "test ipv6 RFC-5952 4.2.2 one 16-bit 0 field": { + "test ipv6 RFC-5952 4.2.2 conformance one 16-bit 0 field": { arg: "https://[2001:db8:0:1:1:1:1:1]:8200", want: "https://[2001:db8:0:1:1:1:1:1]:8200", isErrorExpected: false, }, - "test ipv6 RFC-5952 4.2.3 longest run of 0 bits shortened": { + "test ipv6 RFC-5952 4.2.3 conformance longest run of 0 bits shortened": { arg: "https://[2001:0:0:1:0:0:0:1]:8200", want: "https://[2001:0:0:1::1]:8200", isErrorExpected: false, }, - "test ipv6 RFC-5952 4.2.3 equal runs of 0 bits shortened": { + "test ipv6 RFC-5952 4.2.3 conformance equal runs of 0 bits shortened": { arg: "https://[2001:db8:0:0:1:0:0:1]:8200", want: "https://[2001:db8::1:0:0:1]:8200", isErrorExpected: false, }, - "test ipv6 RFC-5952 4.3 downcase hex letters": { + "test ipv6 RFC-5952 4.3 conformance downcase hex letters": { arg: "https://[2001:DB8:AC3:FE4::1]:8200", want: "https://[2001:db8:ac3:fe4::1]:8200", isErrorExpected: false, @@ -91,6 +91,92 @@ func TestListener_ParseSingleIPTemplate(t *testing.T) { } } +// TestNormalizeAddr ensures that strings that match either an IP address or URL +// and contain an IPv6 address conform to RFC-5952 +// See: https://rfc-editor.org/rfc/rfc5952.html +func TestNormalizeAddr(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + addr string + expected string + isErrorExpected bool + }{ + "hostname": { + addr: "https://vaultproject.io:8200", + expected: "https://vaultproject.io:8200", + }, + "ipv4": { + 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 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", + }, + "ipv6 IP:Port addr with brackets": { + addr: "[2001:0db8::0001]:8500", + expected: "[2001:db8::1]:8500", + }, + "ipv6 RFC-5952 4.1 conformance leading zeroes": { + addr: "2001:0db8::0001", + expected: "2001:db8::1", + }, + "ipv6 URL RFC-5952 4.1 conformance leading zeroes": { + addr: "https://[2001:0db8::0001]:8200", + expected: "https://[2001:db8::1]:8200", + }, + "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", + }, + "ipv6 URL RFC-5952 4.2.2 conformance one 16-bit 0 field": { + addr: "https://[2001:db8:0:1:1:1:1:1]:8200", + expected: "https://[2001:db8:0:1:1:1:1:1]:8200", + }, + "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", + }, + "ipv6 URL RFC-5952 4.2.3 conformance longest run of 0 bits shortened": { + addr: "https://[2001:0:0:1:0:0:0:1]:8200", + expected: "https://[2001:0:0:1::1]:8200", + }, + "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": { + addr: "https://[2001:db8:0:0:1:0:0:1]:8200", + expected: "https://[2001:db8::1:0:0:1]:8200", + }, + "ipv6 RFC-5952 4.3 conformance downcase hex letters": { + addr: "2001:DB8:AC3:FE4::1", + expected: "2001:db8:ac3:fe4::1", + }, + "ipv6 URL RFC-5952 4.3 conformance downcase hex letters": { + addr: "https://[2001:DB8:AC3:FE4::1]:8200", + expected: "https://[2001:db8:ac3:fe4::1]:8200", + }, + } + for name, tc := range tests { + name := name + tc := tc + t.Run(name, func(t *testing.T) { + t.Parallel() + require.Equal(t, tc.expected, NormalizeAddr(tc.addr)) + }) + } +} + // TestListener_parseType exercises the listener receiver parseType. // We check various inputs to ensure we can parse the values as expected and // assign the relevant value on the SharedConfig struct. diff --git a/internalshared/configutil/telemetry.go b/internalshared/configutil/telemetry.go index af81f945bfce..88ffc7e54118 100644 --- a/internalshared/configutil/telemetry.go +++ b/internalshared/configutil/telemetry.go @@ -267,7 +267,7 @@ func normalizeTelemetryAddresses(in *Telemetry) { } if url := *addr; url != "" { - n := normalizeIfURL(url) + n := NormalizeAddr(url) *addr = n } } From 55a2eeaf75fb4f8e6db3183362283a9010c95b7a Mon Sep 17 00:00:00 2001 From: Ryan Cragun Date: Fri, 13 Dec 2024 14:57:35 -0700 Subject: [PATCH 04/12] ipv6: normalize service_registration addresses Signed-off-by: Ryan Cragun --- command/server/config.go | 6 +++ command/server/config_test_helpers.go | 3 +- command/server/test-fixtures/config2.hcl | 51 ++++++++++++------------ 3 files changed, 34 insertions(+), 26 deletions(-) diff --git a/command/server/config.go b/command/server/config.go index cfd57ce0d96c..9cd404efb346 100644 --- a/command/server/config.go +++ b/command/server/config.go @@ -1155,6 +1155,12 @@ func parseServiceRegistration(result *Config, list *ast.ObjectList, name string) return multierror.Prefix(err, fmt.Sprintf("%s.%s:", name, key)) } + if key == "consul" { + if addr, ok := m["address"]; ok { + m["address"] = configutil.NormalizeAddr(addr) + } + } + result.ServiceRegistration = &ServiceRegistration{ Type: strings.ToLower(key), Config: m, diff --git a/command/server/config_test_helpers.go b/command/server/config_test_helpers.go index 6301d558a459..f632a612e52e 100644 --- a/command/server/config_test_helpers.go +++ b/command/server/config_test_helpers.go @@ -144,7 +144,8 @@ func testLoadConfigFile_topLevel(t *testing.T, entropy *configutil.Entropy) { ServiceRegistration: &ServiceRegistration{ Type: "consul", Config: map[string]string{ - "foo": "bar", + "foo": "bar", + "address": "https://[2001:db8::1]:8500", }, }, diff --git a/command/server/test-fixtures/config2.hcl b/command/server/test-fixtures/config2.hcl index 0e383fb25910..7bf6c72a41c0 100644 --- a/command/server/test-fixtures/config2.hcl +++ b/command/server/test-fixtures/config2.hcl @@ -6,60 +6,61 @@ disable_mlock = true ui = true -api_addr = "top_level_api_addr" +api_addr = "top_level_api_addr" cluster_addr = "top_level_cluster_addr" listener "tcp" { - address = "127.0.0.1:443" + address = "127.0.0.1:443" } storage "consul" { - foo = "bar" - redirect_addr = "foo" + foo = "bar" + redirect_addr = "foo" } ha_storage "consul" { - bar = "baz" - redirect_addr = "snafu" - disable_clustering = "true" + bar = "baz" + redirect_addr = "snafu" + disable_clustering = "true" } service_registration "consul" { - foo = "bar" + foo = "bar" + address = "https://[2001:0db8::0001]:8500" } telemetry { - statsd_address = "bar" - usage_gauge_period = "5m" - maximum_gauge_cardinality = 125 - statsite_address = "foo" - dogstatsd_addr = "127.0.0.1:7254" - dogstatsd_tags = ["tag_1:val_1", "tag_2:val_2"] - prometheus_retention_time = "30s" + statsd_address = "bar" + usage_gauge_period = "5m" + maximum_gauge_cardinality = 125 + statsite_address = "foo" + dogstatsd_addr = "127.0.0.1:7254" + dogstatsd_tags = ["tag_1:val_1", "tag_2:val_2"] + prometheus_retention_time = "30s" } entropy "seal" { - mode = "augmentation" + mode = "augmentation" } sentinel { - additional_enabled_modules = [] + additional_enabled_modules = [] } kms "commastringpurpose" { - purpose = "foo,bar" + purpose = "foo,bar" } kms "slicepurpose" { - purpose = ["zip", "zap"] + purpose = ["zip", "zap"] } seal "nopurpose" { } seal "stringpurpose" { - purpose = "foo" + purpose = "foo" } -max_lease_ttl = "10h" -default_lease_ttl = "10h" -cluster_name = "testcluster" -pid_file = "./pidfile" +max_lease_ttl = "10h" +default_lease_ttl = "10h" +cluster_name = "testcluster" +pid_file = "./pidfile" raw_storage_endpoint = true -disable_sealwrap = true +disable_sealwrap = true From b6827975670d04fe374fcd3b40d11f1c8ad2b562 Mon Sep 17 00:00:00 2001 From: Ryan Cragun Date: Mon, 16 Dec 2024 15:19:36 -0700 Subject: [PATCH 05/12] ipv6: normalize seal addresses Signed-off-by: Ryan Cragun --- internalshared/configutil/kms.go | 34 +++- internalshared/configutil/kms_test.go | 207 ++++++++++++++++++++- internalshared/configutil/listener_test.go | 5 +- 3 files changed, 233 insertions(+), 13 deletions(-) diff --git a/internalshared/configutil/kms.go b/internalshared/configutil/kms.go index f0948118dd95..a81cfbbf54b6 100644 --- a/internalshared/configutil/kms.go +++ b/internalshared/configutil/kms.go @@ -11,6 +11,7 @@ import ( "io" "os" "regexp" + "slices" "strings" "github.com/hashicorp/errwrap" @@ -157,7 +158,10 @@ func parseKMS(result *[]*KMS, list *ast.ObjectList, blockName string, maxKMS int if err != nil { return multierror.Prefix(err, fmt.Sprintf("%s.%s:", blockName, key)) } - strMap[k] = s + strMap[k], err = normalizeKMSSealAddresses(name, k, s) + if err != nil { + return multierror.Prefix(err, fmt.Sprintf("%s.%s:", blockName, key)) + } } seal := &KMS{ @@ -214,6 +218,34 @@ func ParseKMSes(d string) ([]*KMS, error) { return result.Seals, nil } +// kmsSealAddressKeys is a map to seal keys that might contain URLs, IP addresses, or IP:port style addresses. All physical storage +// types must have an entry in this map, otherwise our normalization check will +// fail when parsing the storage entry config. +var kmsSealAddressKeys = map[string][]string{ + "alicloudkms": {"domain"}, + "awskms": {"endpoint"}, + "azurekeyvault": {"resource"}, + "gcpckms": {}, + "ocikms": {"key_id", "crypto_endpoint", "management_endpoint"}, + "pkcs11": {}, + "transit": {"address"}, +} + +// normalizeKMSSealAddresses takes a storage name, config key, and config +// value and will normalize any URLs, IP addresses, or IP:port style addresses. +func normalizeKMSSealAddresses(seal string, key string, value string) (string, error) { + keys, ok := kmsSealAddressKeys[seal] + if !ok { + return "", fmt.Errorf("unknown seal type %s", seal) + } + + if slices.Contains(keys, key) { + return NormalizeAddr(value), nil + } + + return value, nil +} + func configureWrapper(configKMS *KMS, infoKeys *[]string, info *map[string]string, logger hclog.Logger, opts ...wrapping.Option) (wrapping.Wrapper, error) { var wrapper wrapping.Wrapper var kmsInfo map[string]string diff --git a/internalshared/configutil/kms_test.go b/internalshared/configutil/kms_test.go index 9eb19a3e3d7d..e797e0aa52db 100644 --- a/internalshared/configutil/kms_test.go +++ b/internalshared/configutil/kms_test.go @@ -4,9 +4,10 @@ package configutil import ( - "os" "reflect" "testing" + + "github.com/stretchr/testify/require" ) func Test_getEnvConfig(t *testing.T) { @@ -83,20 +84,208 @@ func Test_getEnvConfig(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { for envName, envVal := range tt.envVars { - if err := os.Setenv(envName, envVal); err != nil { - t.Errorf("error setting environment vars for test: %s", err) - } + t.Setenv(envName, envVal) } if got := GetEnvConfigFunc(tt.kms); !reflect.DeepEqual(got, tt.want) { t.Errorf("getEnvConfig() = %v, want %v", got, tt.want) } + }) + } +} - for env := range tt.envVars { - if err := os.Unsetenv(env); err != nil { - t.Errorf("error unsetting environment vars for test: %s", err) - } - } +func TestParseKMSesURLConformance(t *testing.T) { + t.Parallel() + + for name, tc := range map[string]struct { + config string + expected map[string]string + env map[string]string + shouldFail bool + }{ + "alicloudkms ipv4": { + config: ` +seal "alicloudkms" { + region = "us-east-1" + domain = "kms.us-east-1.aliyuncs.com" + access_key = "0wNEpMMlzy7szvai" + secret_key = "PupkTg8jdmau1cXxYacgE736PJj4cA" + kms_key_id = "08c33a6f-4e0a-4a1b-a3fa-7ddfa1d4fb73" +}`, + expected: map[string]string{ + "region": "us-east-1", + "domain": "kms.us-east-1.aliyuncs.com", + "access_key": "0wNEpMMlzy7szvai", + "secret_key": "PupkTg8jdmau1cXxYacgE736PJj4cA", + "kms_key_id": "08c33a6f-4e0a-4a1b-a3fa-7ddfa1d4fb73", + }, + }, + "alicloudkms ipv6": { + config: ` +seal "alicloudkms" { + region = "us-east-1" + domain = "2001:db8:0:0:0:0:2:1" + access_key = "0wNEpMMlzy7szvai" + secret_key = "PupkTg8jdmau1cXxYacgE736PJj4cA" + kms_key_id = "08c33a6f-4e0a-4a1b-a3fa-7ddfa1d4fb73" +}`, + expected: map[string]string{ + "region": "us-east-1", + "domain": "2001:db8::2:1", + "access_key": "0wNEpMMlzy7szvai", + "secret_key": "PupkTg8jdmau1cXxYacgE736PJj4cA", + "kms_key_id": "08c33a6f-4e0a-4a1b-a3fa-7ddfa1d4fb73", + }, + }, + "awskms ipv4": { + config: ` +seal "awskms" { + region = "us-east-1" + access_key = "AKIAIOSFODNN7EXAMPLE" + secret_key = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY" + kms_key_id = "19ec80b0-dfdd-4d97-8164-c6examplekey" + endpoint = "https://vpce-0e1bb1852241f8cc6-pzi0do8n.kms.us-east-1.vpce.amazonaws.com" +}`, + expected: map[string]string{ + "region": "us-east-1", + "access_key": "AKIAIOSFODNN7EXAMPLE", + "secret_key": "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", + "kms_key_id": "19ec80b0-dfdd-4d97-8164-c6examplekey", + "endpoint": "https://vpce-0e1bb1852241f8cc6-pzi0do8n.kms.us-east-1.vpce.amazonaws.com", + }, + }, + "awskms ipv6": { + config: ` +seal "awskms" { + region = "us-east-1" + access_key = "AKIAIOSFODNN7EXAMPLE" + secret_key = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY" + kms_key_id = "19ec80b0-dfdd-4d97-8164-c6examplekey" + endpoint = "https://[2001:db8:0:0:0:0:2:1]:5984/my-aws-endpoint" +}`, + expected: map[string]string{ + "region": "us-east-1", + "access_key": "AKIAIOSFODNN7EXAMPLE", + "secret_key": "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", + "kms_key_id": "19ec80b0-dfdd-4d97-8164-c6examplekey", + "endpoint": "https://[2001:db8::2:1]:5984/my-aws-endpoint", + }, + }, + "azurekeyvault ipv4": { + config: ` +seal "azurekeyvault" { + tenant_id = "46646709-b63e-4747-be42-516edeaf1e14" + client_id = "03dc33fc-16d9-4b77-8152-3ec568f8af6e" + client_secret = "DUJDS3..." + vault_name = "hc-vault" + key_name = "vault_key" + resource = "vault.azure.net" +}`, + expected: map[string]string{ + "tenant_id": "46646709-b63e-4747-be42-516edeaf1e14", + "client_id": "03dc33fc-16d9-4b77-8152-3ec568f8af6e", + "client_secret": "DUJDS3...", + "vault_name": "hc-vault", + "key_name": "vault_key", + "resource": "vault.azure.net", + }, + }, + "azurekeyvault ipv6": { + config: ` +seal "azurekeyvault" { + tenant_id = "46646709-b63e-4747-be42-516edeaf1e14" + client_id = "03dc33fc-16d9-4b77-8152-3ec568f8af6e" + client_secret = "DUJDS3..." + vault_name = "hc-vault" + key_name = "vault_key" + resource = "2001:db8:0:0:0:0:2:1", +}`, + expected: map[string]string{ + "tenant_id": "46646709-b63e-4747-be42-516edeaf1e14", + "client_id": "03dc33fc-16d9-4b77-8152-3ec568f8af6e", + "client_secret": "DUJDS3...", + "vault_name": "hc-vault", + "key_name": "vault_key", + "resource": "2001:db8::2:1", + }, + }, + "ocikms ipv4": { + config: ` +seal "ocikms" { + key_id = "ocid1.key.oc1.iad.afnxza26aag4s.abzwkljsbapzb2nrha5nt3s7s7p42ctcrcj72vn3kq5qx" + crypto_endpoint = "https://afnxza26aag4s-crypto.kms.us-ashburn-1.oraclecloud.com" + management_endpoint = "https://afnxza26aag4s-management.kms.us-ashburn-1.oraclecloud.com" + auth_type_api_key = "true" +}`, + expected: map[string]string{ + "key_id": "ocid1.key.oc1.iad.afnxza26aag4s.abzwkljsbapzb2nrha5nt3s7s7p42ctcrcj72vn3kq5qx", + "crypto_endpoint": "https://afnxza26aag4s-crypto.kms.us-ashburn-1.oraclecloud.com", + "management_endpoint": "https://afnxza26aag4s-management.kms.us-ashburn-1.oraclecloud.com", + "auth_type_api_key": "true", + }, + }, + "ocikms ipv6": { + config: ` +seal "ocikms" { + key_id = "https://[2001:db8:0:0:0:0:2:1]/abzwkljsbapzb2nrha5nt3s7s7p42ctcrcj72vn3kq5qx" + crypto_endpoint = "https://[2001:db8:0:0:0:0:2:1]/afnxza26aag4s-crypto" + management_endpoint = "https://[2001:db8:0:0:0:0:2:1]/afnxza26aag4s-management" + auth_type_api_key = "true" +}`, + expected: map[string]string{ + "key_id": "https://[2001:db8::2:1]/abzwkljsbapzb2nrha5nt3s7s7p42ctcrcj72vn3kq5qx", + "crypto_endpoint": "https://[2001:db8::2:1]/afnxza26aag4s-crypto", + "management_endpoint": "https://[2001:db8::2:1]/afnxza26aag4s-management", + "auth_type_api_key": "true", + }, + }, + "transit ipv4": { + config: ` +seal "transit" { + address = "https://vault:8200" + token = "s.Qf1s5zigZ4OX6akYjQXJC1jY" + disable_renewal = "false" + key_name = "transit_key_name" + mount_path = "transit/" + namespace = "ns1/" +} +`, + expected: map[string]string{ + "address": "https://vault:8200", + "token": "s.Qf1s5zigZ4OX6akYjQXJC1jY", + "disable_renewal": "false", + "key_name": "transit_key_name", + "mount_path": "transit/", + "namespace": "ns1/", + }, + }, + "transit ipv6": { + config: ` +seal "transit" { + address = "https://[2001:db8:0:0:0:0:2:1]:8200" + token = "s.Qf1s5zigZ4OX6akYjQXJC1jY" + disable_renewal = "false" + key_name = "transit_key_name" + mount_path = "transit/" + namespace = "ns1/" +} +`, + expected: map[string]string{ + "address": "https://[2001:db8::2:1]:8200", + "token": "s.Qf1s5zigZ4OX6akYjQXJC1jY", + "disable_renewal": "false", + "key_name": "transit_key_name", + "mount_path": "transit/", + "namespace": "ns1/", + }, + }, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + kmses, err := ParseKMSes(tc.config) + require.NoError(t, err) + require.Len(t, kmses, 1) + require.EqualValues(t, tc.expected, kmses[0].Config) }) } } diff --git a/internalshared/configutil/listener_test.go b/internalshared/configutil/listener_test.go index e246a3c57ced..4d8cb63c267c 100644 --- a/internalshared/configutil/listener_test.go +++ b/internalshared/configutil/listener_test.go @@ -66,9 +66,8 @@ func TestListener_ParseSingleIPTemplate(t *testing.T) { errorMessage: "unable to parse address template", }, "test partial template": { - arg: "{{FooBar", - // Partial templates get treated as literal hostname and get URL encoded - want: "%7B%7BFooBar", + arg: "{{FooBar", + want: "{{FooBar", isErrorExpected: false, }, } From 5a0550808251cc002d6557ac74ec335bd543a109 Mon Sep 17 00:00:00 2001 From: Ryan Cragun Date: Tue, 17 Dec 2024 14:32:15 -0700 Subject: [PATCH 06/12] ipv6: normalize seal addresses from env vars Signed-off-by: Ryan Cragun --- internalshared/configutil/kms.go | 65 ++++++---- internalshared/configutil/kms_test.go | 165 +++++++++++++++++++++++++- 2 files changed, 207 insertions(+), 23 deletions(-) diff --git a/internalshared/configutil/kms.go b/internalshared/configutil/kms.go index a81cfbbf54b6..7fdcdcf11dec 100644 --- a/internalshared/configutil/kms.go +++ b/internalshared/configutil/kms.go @@ -158,7 +158,7 @@ func parseKMS(result *[]*KMS, list *ast.ObjectList, blockName string, maxKMS int if err != nil { return multierror.Prefix(err, fmt.Sprintf("%s.%s:", blockName, key)) } - strMap[k], err = normalizeKMSSealAddresses(name, k, s) + strMap[k], err = normalizeKMSSealConfigAddrs(name, k, s) if err != nil { return multierror.Prefix(err, fmt.Sprintf("%s.%s:", blockName, key)) } @@ -218,22 +218,23 @@ func ParseKMSes(d string) ([]*KMS, error) { return result.Seals, nil } -// kmsSealAddressKeys is a map to seal keys that might contain URLs, IP addresses, or IP:port style addresses. All physical storage -// types must have an entry in this map, otherwise our normalization check will -// fail when parsing the storage entry config. +// kmsSealAddressKeys is a map to seal keys that might contain URLs, IP +// addresses, or IP:port style addresses. All physical storage types must have +// an entry in this map, otherwise our normalization check will fail when +// parsing the storage entry config. var kmsSealAddressKeys = map[string][]string{ - "alicloudkms": {"domain"}, - "awskms": {"endpoint"}, - "azurekeyvault": {"resource"}, - "gcpckms": {}, - "ocikms": {"key_id", "crypto_endpoint", "management_endpoint"}, - "pkcs11": {}, - "transit": {"address"}, + wrapping.WrapperTypeAliCloudKms.String(): {"domain"}, + wrapping.WrapperTypeAwsKms.String(): {"endpoint"}, + wrapping.WrapperTypeAzureKeyVault.String(): {"resource"}, + wrapping.WrapperTypeGcpCkms.String(): {}, + wrapping.WrapperTypeOciKms.String(): {"key_id", "crypto_endpoint", "management_endpoint"}, + wrapping.WrapperTypePkcs11.String(): {}, + wrapping.WrapperTypeTransit.String(): {"address"}, } -// normalizeKMSSealAddresses takes a storage name, config key, and config +// normalizeKMSSealConfigAddrs takes a kms seal type, a config key, and config // value and will normalize any URLs, IP addresses, or IP:port style addresses. -func normalizeKMSSealAddresses(seal string, key string, value string) (string, error) { +func normalizeKMSSealConfigAddrs(seal string, key string, value string) (string, error) { keys, ok := kmsSealAddressKeys[seal] if !ok { return "", fmt.Errorf("unknown seal type %s", seal) @@ -246,24 +247,38 @@ func normalizeKMSSealAddresses(seal string, key string, value string) (string, e return value, nil } -func configureWrapper(configKMS *KMS, infoKeys *[]string, info *map[string]string, logger hclog.Logger, opts ...wrapping.Option) (wrapping.Wrapper, error) { - var wrapper wrapping.Wrapper - var kmsInfo map[string]string - var err error - +func mergeKMSEnvConfig(configKMS *KMS) error { envConfig := GetEnvConfigFunc(configKMS) if len(envConfig) > 0 && configKMS.Config == nil { configKMS.Config = make(map[string]string) } // transit is a special case, because some config values take precedence over env vars if configKMS.Type == wrapping.WrapperTypeTransit.String() { - mergeTransitConfig(configKMS.Config, envConfig) + if err := mergeTransitConfig(configKMS.Config, envConfig); err != nil { + return err + } } else { for name, val := range envConfig { - configKMS.Config[name] = val + var err error + configKMS.Config[name], err = normalizeKMSSealConfigAddrs(configKMS.Type, name, val) + if err != nil { + return err + } } } + return nil +} + +func configureWrapper(configKMS *KMS, infoKeys *[]string, info *map[string]string, logger hclog.Logger, opts ...wrapping.Option) (wrapping.Wrapper, error) { + var wrapper wrapping.Wrapper + var kmsInfo map[string]string + var err error + + if err = mergeKMSEnvConfig(configKMS); err != nil { + return nil, err + } + switch wrapping.WrapperType(configKMS.Type) { case wrapping.WrapperTypeShamir: return nil, nil @@ -488,7 +503,7 @@ func getEnvConfig(kms *KMS) map[string]string { return envValues } -func mergeTransitConfig(config map[string]string, envConfig map[string]string) { +func mergeTransitConfig(config map[string]string, envConfig map[string]string) error { useFileTlsConfig := false for _, varName := range TransitTLSConfigVars { if _, ok := config[varName]; ok { @@ -503,14 +518,20 @@ func mergeTransitConfig(config map[string]string, envConfig map[string]string) { } } + var err error for varName, val := range envConfig { // for some values, file config takes precedence if strutil.StrListContains(TransitPrioritizeConfigValues, varName) && config[varName] != "" { continue } - config[varName] = val + config[varName], err = normalizeKMSSealConfigAddrs(wrapping.WrapperTypeTransit.String(), varName, val) + if err != nil { + return err + } } + + return nil } func (k *KMS) Clone() *KMS { diff --git a/internalshared/configutil/kms_test.go b/internalshared/configutil/kms_test.go index e797e0aa52db..969ac6f1ae3f 100644 --- a/internalshared/configutil/kms_test.go +++ b/internalshared/configutil/kms_test.go @@ -8,6 +8,8 @@ import ( "testing" "github.com/stretchr/testify/require" + + "github.com/hashicorp/go-kms-wrapping/wrappers/ocikms/v2" ) func Test_getEnvConfig(t *testing.T) { @@ -100,7 +102,6 @@ func TestParseKMSesURLConformance(t *testing.T) { for name, tc := range map[string]struct { config string expected map[string]string - env map[string]string shouldFail bool }{ "alicloudkms ipv4": { @@ -289,3 +290,165 @@ seal "transit" { }) } } + +func TestMergeKMSEnvConfigAddrConformance(t *testing.T) { + for name, tc := range map[string]struct { + sealType string // default to name if none given + kmsConfig map[string]string + envVars map[string]string + expected map[string]string + }{ + "alicloudkms": { + kmsConfig: map[string]string{ + "region": "us-east-1", + "domain": "kms.us-east-1.aliyuncs.com", + "access_key": "0wNEpMMlzy7szvai", + "secret_key": "PupkTg8jdmau1cXxYacgE736PJj4cA", + "kms_key_id": "08c33a6f-4e0a-4a1b-a3fa-7ddfa1d4fb73", + }, + envVars: map[string]string{"ALICLOUD_DOMAIN": "2001:db8:0:0:0:0:2:1"}, + expected: map[string]string{ + "region": "us-east-1", + "domain": "2001:db8::2:1", + "access_key": "0wNEpMMlzy7szvai", + "secret_key": "PupkTg8jdmau1cXxYacgE736PJj4cA", + "kms_key_id": "08c33a6f-4e0a-4a1b-a3fa-7ddfa1d4fb73", + }, + }, + "awskms": { + kmsConfig: map[string]string{ + "region": "us-east-1", + "access_key": "AKIAIOSFODNN7EXAMPLE", + "secret_key": "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", + "kms_key_id": "19ec80b0-dfdd-4d97-8164-c6examplekey", + "endpoint": "https://vpce-0e1bb1852241f8cc6-pzi0do8n.kms.us-east-1.vpce.amazonaws.com", + }, + envVars: map[string]string{"AWS_KMS_ENDPOINT": "https://[2001:db8:0:0:0:0:2:1]:5984/my-aws-endpoint"}, + expected: map[string]string{ + "region": "us-east-1", + "access_key": "AKIAIOSFODNN7EXAMPLE", + "secret_key": "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", + "kms_key_id": "19ec80b0-dfdd-4d97-8164-c6examplekey", + "endpoint": "https://[2001:db8::2:1]:5984/my-aws-endpoint", + }, + }, + "azurekeyvault": { + kmsConfig: map[string]string{ + "tenant_id": "46646709-b63e-4747-be42-516edeaf1e14", + "client_id": "03dc33fc-16d9-4b77-8152-3ec568f8af6e", + "client_secret": "DUJDS3...", + "vault_name": "hc-vault", + "key_name": "vault_key", + "resource": "vault.azure.net", + }, + envVars: map[string]string{"AZURE_AD_RESOURCE": "2001:db8:0:0:0:0:2:1"}, + expected: map[string]string{ + "tenant_id": "46646709-b63e-4747-be42-516edeaf1e14", + "client_id": "03dc33fc-16d9-4b77-8152-3ec568f8af6e", + "client_secret": "DUJDS3...", + "vault_name": "hc-vault", + "key_name": "vault_key", + "resource": "2001:db8::2:1", + }, + }, + "ocikms wrapper env vars": { + sealType: "ocikms", + kmsConfig: map[string]string{ + "key_id": "ocid1.key.oc1.iad.afnxza26aag4s.abzwkljsbapzb2nrha5nt3s7s7p42ctcrcj72vn3kq5qx", + "crypto_endpoint": "https://afnxza26aag4s-crypto.kms.us-ashburn-1.oraclecloud.com", + "management_endpoint": "https://afnxza26aag4s-management.kms.us-ashburn-1.oraclecloud.com", + "auth_type_api_key": "true", + }, + envVars: map[string]string{ + ocikms.EnvOciKmsWrapperKeyId: "https://[2001:db8:0:0:0:0:2:1]/abzwkljsbapzb2nrha5nt3s7s7p42ctcrcj72vn3kq5qx", + ocikms.EnvOciKmsWrapperCryptoEndpoint: "https://[2001:db8:0:0:0:0:2:1]/afnxza26aag4s-crypto", + ocikms.EnvOciKmsWrapperManagementEndpoint: "https://[2001:db8:0:0:0:0:2:1]/afnxza26aag4s-management", + }, + expected: map[string]string{ + "key_id": "https://[2001:db8::2:1]/abzwkljsbapzb2nrha5nt3s7s7p42ctcrcj72vn3kq5qx", + "crypto_endpoint": "https://[2001:db8::2:1]/afnxza26aag4s-crypto", + "management_endpoint": "https://[2001:db8::2:1]/afnxza26aag4s-management", + "auth_type_api_key": "true", + }, + }, + "ocikms vault env vars": { + sealType: "ocikms", + kmsConfig: map[string]string{ + "key_id": "ocid1.key.oc1.iad.afnxza26aag4s.abzwkljsbapzb2nrha5nt3s7s7p42ctcrcj72vn3kq5qx", + "crypto_endpoint": "https://afnxza26aag4s-crypto.kms.us-ashburn-1.oraclecloud.com", + "management_endpoint": "https://afnxza26aag4s-management.kms.us-ashburn-1.oraclecloud.com", + "auth_type_api_key": "true", + }, + envVars: map[string]string{ + ocikms.EnvVaultOciKmsSealKeyId: "https://[2001:db8:0:0:0:0:2:1]/abzwkljsbapzb2nrha5nt3s7s7p42ctcrcj72vn3kq5qx", + ocikms.EnvVaultOciKmsSealCryptoEndpoint: "https://[2001:db8:0:0:0:0:2:1]/afnxza26aag4s-crypto", + ocikms.EnvVaultOciKmsSealManagementEndpoint: "https://[2001:db8:0:0:0:0:2:1]/afnxza26aag4s-management", + }, + expected: map[string]string{ + "key_id": "https://[2001:db8::2:1]/abzwkljsbapzb2nrha5nt3s7s7p42ctcrcj72vn3kq5qx", + "crypto_endpoint": "https://[2001:db8::2:1]/afnxza26aag4s-crypto", + "management_endpoint": "https://[2001:db8::2:1]/afnxza26aag4s-management", + "auth_type_api_key": "true", + }, + }, + "transit addr not in config": { + sealType: "transit", + kmsConfig: map[string]string{ + "token": "s.Qf1s5zigZ4OX6akYjQXJC1jY", + "disable_renewal": "false", + "key_name": "transit_key_name", + "mount_path": "transit/", + "namespace": "ns1/", + }, + envVars: map[string]string{"VAULT_ADDR": "https://[2001:db8:0:0:0:0:2:1]:8200"}, + expected: map[string]string{ + // NOTE: If our address has not been configured we'll fall back to VAULT_ADDR for transit. + "address": "https://[2001:db8::2:1]:8200", + "token": "s.Qf1s5zigZ4OX6akYjQXJC1jY", + "disable_renewal": "false", + "key_name": "transit_key_name", + "mount_path": "transit/", + "namespace": "ns1/", + }, + }, + "transit addr in config": { + sealType: "transit", + kmsConfig: map[string]string{ + "address": "https://vault:8200", + "token": "s.Qf1s5zigZ4OX6akYjQXJC1jY", + "disable_renewal": "false", + "key_name": "transit_key_name", + "mount_path": "transit/", + "namespace": "ns1/", + }, + envVars: map[string]string{"VAULT_ADDR": "https://[2001:db8:0:0:0:0:2:1]:8200"}, + expected: map[string]string{ + // NOTE: If our address has been configured we don't consider VAULT_ADDR + "address": "https://vault:8200", + "token": "s.Qf1s5zigZ4OX6akYjQXJC1jY", + "disable_renewal": "false", + "key_name": "transit_key_name", + "mount_path": "transit/", + "namespace": "ns1/", + }, + }, + } { + t.Run(name, func(t *testing.T) { + typ := name + if tc.sealType != "" { + typ = tc.sealType + } + kms := &KMS{ + Type: typ, + Config: tc.kmsConfig, + } + + for envName, envVal := range tc.envVars { + t.Setenv(envName, envVal) + } + + require.NoError(t, mergeKMSEnvConfig(kms)) + require.EqualValues(t, tc.expected, kms.Config) + }) + } +} From dec12553104609fb6fec959214e4c1dacec63146 Mon Sep 17 00:00:00 2001 From: Ryan Cragun Date: Wed, 18 Dec 2024 10:17:10 -0700 Subject: [PATCH 07/12] ipv6: normalize storage retry_join and various other run config Signed-off-by: Ryan Cragun --- command/server.go | 33 +++---- command/server/config.go | 92 ++++++++++++++++--- command/server/config_test_helpers.go | 24 ++++- .../server/test-fixtures/raft_retry_join.hcl | 38 +++++--- internalshared/configutil/kms.go | 1 + 5 files changed, 142 insertions(+), 46 deletions(-) diff --git a/command/server.go b/command/server.go index c07b1acc959b..77129e0a7c22 100644 --- a/command/server.go +++ b/command/server.go @@ -26,6 +26,12 @@ import ( systemd "github.com/coreos/go-systemd/daemon" "github.com/google/go-cmp/cmp" + "github.com/posener/complete" + "github.com/sasha-s/go-deadlock" + "go.uber.org/atomic" + "golang.org/x/net/http/httpproxy" + "google.golang.org/grpc/grpclog" + "github.com/hashicorp/cli" "github.com/hashicorp/errwrap" "github.com/hashicorp/go-hclog" @@ -62,11 +68,6 @@ import ( "github.com/hashicorp/vault/vault/plugincatalog" vaultseal "github.com/hashicorp/vault/vault/seal" "github.com/hashicorp/vault/version" - "github.com/posener/complete" - "github.com/sasha-s/go-deadlock" - "go.uber.org/atomic" - "golang.org/x/net/http/httpproxy" - "google.golang.org/grpc/grpclog" ) var ( @@ -514,7 +515,7 @@ func (c *ServerCommand) runRecoveryMode() int { } if config.Storage.Type == storageTypeRaft || (config.HAStorage != nil && config.HAStorage.Type == storageTypeRaft) { if envCA := os.Getenv("VAULT_CLUSTER_ADDR"); envCA != "" { - config.ClusterAddr = envCA + config.ClusterAddr = configutil.NormalizeAddr(envCA) } if len(config.ClusterAddr) == 0 { @@ -742,9 +743,9 @@ func (c *ServerCommand) runRecoveryMode() int { func logProxyEnvironmentVariables(logger hclog.Logger) { proxyCfg := httpproxy.FromEnvironment() cfgMap := map[string]string{ - "http_proxy": proxyCfg.HTTPProxy, - "https_proxy": proxyCfg.HTTPSProxy, - "no_proxy": proxyCfg.NoProxy, + "http_proxy": configutil.NormalizeAddr(proxyCfg.HTTPProxy), + "https_proxy": configutil.NormalizeAddr(proxyCfg.HTTPSProxy), + "no_proxy": configutil.NormalizeAddr(proxyCfg.NoProxy), } for k, v := range cfgMap { u, err := url.Parse(v) @@ -2243,7 +2244,7 @@ func (c *ServerCommand) detectRedirect(detect physical.RedirectDetect, } // Return the URL string - return url.String(), nil + return configutil.NormalizeAddr(url.String()), nil } func (c *ServerCommand) Reload(lock *sync.RWMutex, reloadFuncs *map[string][]reloadutil.ReloadFunc, configPath []string, core *vault.Core) error { @@ -2749,11 +2750,11 @@ func initHaBackend(c *ServerCommand, config *server.Config, coreConfig *vault.Co func determineRedirectAddr(c *ServerCommand, coreConfig *vault.CoreConfig, config *server.Config) error { var retErr error if envRA := os.Getenv("VAULT_API_ADDR"); envRA != "" { - coreConfig.RedirectAddr = envRA + coreConfig.RedirectAddr = configutil.NormalizeAddr(envRA) } else if envRA := os.Getenv("VAULT_REDIRECT_ADDR"); envRA != "" { - coreConfig.RedirectAddr = envRA + coreConfig.RedirectAddr = configutil.NormalizeAddr(envRA) } else if envAA := os.Getenv("VAULT_ADVERTISE_ADDR"); envAA != "" { - coreConfig.RedirectAddr = envAA + coreConfig.RedirectAddr = configutil.NormalizeAddr(envAA) } // Attempt to detect the redirect address, if possible @@ -2785,7 +2786,7 @@ func determineRedirectAddr(c *ServerCommand, coreConfig *vault.CoreConfig, confi if c.flagDevTLS { protocol = "https" } - coreConfig.RedirectAddr = fmt.Sprintf("%s://%s", protocol, config.Listeners[0].Address) + coreConfig.RedirectAddr = configutil.NormalizeAddr(fmt.Sprintf("%s://%s", protocol, config.Listeners[0].Address)) } return retErr } @@ -2794,7 +2795,7 @@ func findClusterAddress(c *ServerCommand, coreConfig *vault.CoreConfig, config * if disableClustering { coreConfig.ClusterAddr = "" } else if envCA := os.Getenv("VAULT_CLUSTER_ADDR"); envCA != "" { - coreConfig.ClusterAddr = envCA + coreConfig.ClusterAddr = configutil.NormalizeAddr(envCA) } else { var addrToUse string switch { @@ -2826,7 +2827,7 @@ func findClusterAddress(c *ServerCommand, coreConfig *vault.CoreConfig, config * u.Host = net.JoinHostPort(host, strconv.Itoa(nPort+1)) // Will always be TLS-secured u.Scheme = "https" - coreConfig.ClusterAddr = u.String() + coreConfig.ClusterAddr = configutil.NormalizeAddr(u.String()) } CLUSTER_SYNTHESIS_COMPLETE: diff --git a/command/server/config.go b/command/server/config.go index 9cd404efb346..3031decef29c 100644 --- a/command/server/config.go +++ b/command/server/config.go @@ -946,12 +946,22 @@ func ParseStorage(result *Config, list *ast.ObjectList, name string) error { } continue } - // TODO(ryancragun): We don't currently normalize nested configuration here - // that we might have otherwise, e.g.: raft's leader_api_addr. - valBytes, err := json.Marshal(val) - if err != nil { - return err + + var valBytes []byte + var err error + // Raft's retry_join requires special normalization due to its complexity + if key == "raft" && k == "retry_join" { + valBytes, err = normalizeRaftRetryJoin(val) + if err != nil { + return err + } + } else { + valBytes, err = json.Marshal(val) + if err != nil { + return err + } } + m[k] = string(valBytes) } @@ -1015,7 +1025,7 @@ var storageAddressKeys = map[string][]string{ "azure": {"arm_endpoint"}, "cassandra": {"hosts"}, "cockroachdb": {"connection_url"}, - "consul": {"address"}, + "consul": {"address", "service_address"}, "couchdb": {"endpoint"}, "dynamodb": {"endpoint"}, "etcd": {"address", "discovery_srv"}, @@ -1028,7 +1038,7 @@ var storageAddressKeys = map[string][]string{ "mysql": {"address"}, "oci": {}, "postgresql": {"connection_url"}, - "raft": {}, + "raft": {}, // handled separately in normalizeRaftRetryJoin() "s3": {"endpoint"}, "spanner": {}, "swift": {"auth_url", "storage_url"}, @@ -1050,6 +1060,54 @@ func normalizeStorageConfigAddresses(storage string, key string, value string) ( return value, nil } +// normalizeRaftRetryJoin takes the hcl value representation of a retry_join +// stanza and normalizes any URLs, IP addresses, or IP:port style addresses, +// and returns the value encoded as JSON. +func normalizeRaftRetryJoin(val any) ([]byte, error) { + res := []map[string]any{} + + retryJoin, ok := val.([]any) + if !ok { + return nil, fmt.Errorf("malformed retry_join stanza: %v+", val) + } + + for _, rj := range retryJoin { + rjMap := rj.(map[string]any) + if !ok { + return nil, fmt.Errorf("malformed retry_join stanza: %v+", rj) + } + + rjRes := map[string]any{} + for k, v := range rjMap { + switch k { + case "auto_join": + pairs := strings.Split(v.(string), " ") + for i, pair := range pairs { + pairParts := strings.Split(pair, "=") + if len(pairParts) != 2 { + return nil, fmt.Errorf("malformed auto_join pair %s, expected key=value", pair) + } + // These are auto_join keys that a valid for the provider in go-discover + if slices.Contains([]string{"domain", "auth_url", "url", "host"}, pairParts[0]) { + pairParts[1] = configutil.NormalizeAddr(pairParts[1]) + pair = strings.Join(pairParts, "=") + pairs[i] = pair + } + } + rjRes[k] = strings.Join(pairs, " ") + case "leader_api_addr": + rjRes[k] = configutil.NormalizeAddr(v.(string)) + default: + rjRes[k] = v + } + } + + res = append(res, rjRes) + } + + return json.Marshal(res) +} + func parseHAStorage(result *Config, list *ast.ObjectList, name string) error { if len(list.Items) > 1 { return fmt.Errorf("only one %q block is permitted", name) @@ -1079,12 +1137,22 @@ func parseHAStorage(result *Config, list *ast.ObjectList, name string) error { } continue } - // TODO(ryancragun): We don't currently normalize nested configuration here - // that we might have otherwise, e.g.: raft's leader_api_addr. - valBytes, err := json.Marshal(val) - if err != nil { - return err + + var err error + var valBytes []byte + // Raft's retry_join requires special normalization due to its complexity + if key == "raft" && k == "retry_join" { + valBytes, err = normalizeRaftRetryJoin(val) + if err != nil { + return err + } + } else { + valBytes, err = json.Marshal(val) + if err != nil { + return err + } } + m[k] = string(valBytes) } diff --git a/command/server/config_test_helpers.go b/command/server/config_test_helpers.go index f632a612e52e..d3add9a4f0bc 100644 --- a/command/server/config_test_helpers.go +++ b/command/server/config_test_helpers.go @@ -4,6 +4,7 @@ package server import ( + "encoding/json" "fmt" "reflect" "sort" @@ -30,12 +31,27 @@ func boolPointer(x bool) *bool { return &x } +// testConfigRaftRetryJoin ensures that we properly decode and normalize +// config which contains IPv6 addresses. func testConfigRaftRetryJoin(t *testing.T) { + t.Helper() config, err := LoadConfigFile("./test-fixtures/raft_retry_join.hcl") if err != nil { t.Fatal(err) } - retryJoinConfig := `[{"leader_api_addr":"http://127.0.0.1:8200"},{"leader_api_addr":"http://127.0.0.2:8200"},{"leader_api_addr":"http://127.0.0.3:8200"}]` + + retryJoinExpected := []map[string]string{ + {"leader_api_addr": "http://127.0.0.1:8200"}, + {"leader_api_addr": "http://[2001:db8::2:1]:8200"}, + {"auto_join": "provider=mdns service=consul domain=2001:db8::2:1"}, + {"auto_join": "provider=os tag_key=consul tag_value=server username=foo password=bar auth_url=https://[2001:db8::2:1]/auth"}, + {"auto_join": "provider=triton account=testaccount url=https://[2001:db8::2:1] key_id=1234 tag_key=consul-role tag_value=server"}, + {"auto_join": "provider=packet auth_token=token project=uuid url=https://[2001:db8::2:1] address_type=public_v6"}, + {"auto_join": "provider=vsphere category_name=consul-role tag_name=consul-server host=https://[2001:db8::2:1] user=foo password=bar insecure_ssl=false"}, + } + retryJoinJSON, err := json.Marshal(retryJoinExpected) + require.NoError(t, err) + expected := &Config{ SharedConfig: &configutil.SharedConfig{ Listeners: []*configutil.Listener{ @@ -53,14 +69,12 @@ func testConfigRaftRetryJoin(t *testing.T) { Config: map[string]string{ "path": "/storage/path/raft", "node_id": "raft1", - "retry_join": retryJoinConfig, + "retry_join": string(retryJoinJSON), }, }, } config.Prune() - if diff := deep.Equal(config, expected); diff != nil { - t.Fatal(diff) - } + require.EqualValues(t, expected, config) } func testLoadConfigFile_topLevel(t *testing.T, entropy *configutil.Entropy) { diff --git a/command/server/test-fixtures/raft_retry_join.hcl b/command/server/test-fixtures/raft_retry_join.hcl index 844dd744e40c..e39b678b1512 100644 --- a/command/server/test-fixtures/raft_retry_join.hcl +++ b/command/server/test-fixtures/raft_retry_join.hcl @@ -2,21 +2,33 @@ # SPDX-License-Identifier: BUSL-1.1 storage "raft" { - path = "/storage/path/raft" - node_id = "raft1" - retry_join = [ - { - "leader_api_addr" = "http://127.0.0.1:8200" - }, - { - "leader_api_addr" = "http://127.0.0.2:8200" - }, - { - "leader_api_addr" = "http://127.0.0.3:8200" - } + path = "/storage/path/raft" + node_id = "raft1" + retry_join = [ + { "leader_api_addr" = "http://127.0.0.1:8200" }, + ] + retry_join = [ + { "leader_api_addr" = "http://[2001:db8:0:0:0:0:2:1]:8200" } + ] + retry_join = [ + { "auto_join" = "provider=mdns service=consul domain=2001:db8:0:0:0:0:2:1" } + ] + retry_join = [ + { "auto_join" = "provider=os tag_key=consul tag_value=server username=foo password=bar auth_url=https://[2001:db8:0:0:0:0:2:1]/auth" } + ] + retry_join = [ + { "auto_join" = "provider=triton account=testaccount url=https://[2001:db8:0:0:0:0:2:1] key_id=1234 tag_key=consul-role tag_value=server" } + ] + retry_join = [ + { "auto_join" = "provider=packet auth_token=token project=uuid url=https://[2001:db8:0:0:0:0:2:1] address_type=public_v6" } + ] + retry_join = [ + { "auto_join" = "provider=vsphere category_name=consul-role tag_name=consul-server host=https://[2001:db8:0:0:0:0:2:1] user=foo password=bar insecure_ssl=false" } ] } + listener "tcp" { - address = "127.0.0.1:8200" + address = "127.0.0.1:8200" } + disable_mlock = true diff --git a/internalshared/configutil/kms.go b/internalshared/configutil/kms.go index 7fdcdcf11dec..6e7d19bf4b2a 100644 --- a/internalshared/configutil/kms.go +++ b/internalshared/configutil/kms.go @@ -230,6 +230,7 @@ var kmsSealAddressKeys = map[string][]string{ wrapping.WrapperTypeOciKms.String(): {"key_id", "crypto_endpoint", "management_endpoint"}, wrapping.WrapperTypePkcs11.String(): {}, wrapping.WrapperTypeTransit.String(): {"address"}, + "pkcs11-disabled": {}, // only used in tests } // normalizeKMSSealConfigAddrs takes a kms seal type, a config key, and config From 92698bdbb4594f49ee2da82f0bcb229316232d64 Mon Sep 17 00:00:00 2001 From: Ryan Cragun Date: Wed, 18 Dec 2024 14:21:45 -0700 Subject: [PATCH 08/12] ipv6: cleanup pass before review Signed-off-by: Ryan Cragun --- command/server/config.go | 53 +++++------ command/server/config_test_helpers.go | 6 +- internalshared/configutil/kms.go | 18 ++-- internalshared/configutil/listener.go | 94 ------------------- internalshared/configutil/listener_test.go | 86 ------------------ internalshared/configutil/normalize.go | 99 +++++++++++++++++++++ internalshared/configutil/normalize_test.go | 93 +++++++++++++++++++ internalshared/configutil/telemetry.go | 9 +- internalshared/configutil/telemetry_test.go | 7 +- 9 files changed, 245 insertions(+), 220 deletions(-) create mode 100644 internalshared/configutil/normalize.go create mode 100644 internalshared/configutil/normalize_test.go diff --git a/command/server/config.go b/command/server/config.go index 3031decef29c..6e2ff9adb2b4 100644 --- a/command/server/config.go +++ b/command/server/config.go @@ -936,33 +936,33 @@ func ParseStorage(result *Config, list *ast.ObjectList, name string) error { } m := make(map[string]string) - for k, val := range config { - valStr, ok := val.(string) + for k, v := range config { + vStr, ok := v.(string) if ok { var err error - m[k], err = normalizeStorageConfigAddresses(key, k, valStr) + m[k], err = normalizeStorageConfigAddresses(key, k, vStr) if err != nil { return err } continue } - var valBytes []byte var err error + var vBytes []byte // Raft's retry_join requires special normalization due to its complexity if key == "raft" && k == "retry_join" { - valBytes, err = normalizeRaftRetryJoin(val) + vBytes, err = normalizeRaftRetryJoin(v) if err != nil { return err } } else { - valBytes, err = json.Marshal(val) + vBytes, err = json.Marshal(v) if err != nil { return err } } - m[k] = string(valBytes) + m[k] = string(vBytes) } // Pull out the redirect address since it's common to all backends @@ -1015,10 +1015,12 @@ func ParseStorage(result *Config, list *ast.ObjectList, name string) error { return nil } -// storageAddressKeys is a map to physical storage configuration keys that might -// contain URLs, IP addresses, or IP:port style addresses. All physical storage -// types must have an entry in this map, otherwise our normalization check will -// fail when parsing the storage entry config. +// storageAddressKeys is a maps a storage backend type to its associated +// that may configuration whose values are URLs, IP addresses, or host:port +// style addresses. All physical storage types must have an entry in this map, +// otherwise our normalization check will fail when parsing the storage entry +// config. Physical storage types which don't contain such keys should include +// an empty array. var storageAddressKeys = map[string][]string{ "aerospike": {"hostname"}, "alicloudoss": {"endpoint"}, @@ -1038,15 +1040,16 @@ var storageAddressKeys = map[string][]string{ "mysql": {"address"}, "oci": {}, "postgresql": {"connection_url"}, - "raft": {}, // handled separately in normalizeRaftRetryJoin() + "raft": {}, // retry_join is handled separately in normalizeRaftRetryJoin() "s3": {"endpoint"}, "spanner": {}, "swift": {"auth_url", "storage_url"}, "zookeeper": {"address"}, } -// normalizeStorageConfigAddresses takes a storage name, config key, and config -// value and will normalize any URLs, IP addresses, or IP:port style addresses. +// normalizeStorageConfigAddresses 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 normalizeStorageConfigAddresses(storage string, key string, value string) (string, error) { keys, ok := storageAddressKeys[storage] if !ok { @@ -1060,9 +1063,9 @@ func normalizeStorageConfigAddresses(storage string, key string, value string) ( return value, nil } -// normalizeRaftRetryJoin takes the hcl value representation of a retry_join -// stanza and normalizes any URLs, IP addresses, or IP:port style addresses, -// and returns the value encoded as JSON. +// normalizeRaftRetryJoin takes the hcl decoded value representation of a +// retry_join stanza and normalizes any URLs, IP addresses, or host:port style +// addresses, and returns the value encoded as JSON. func normalizeRaftRetryJoin(val any) ([]byte, error) { res := []map[string]any{} @@ -1072,7 +1075,7 @@ func normalizeRaftRetryJoin(val any) ([]byte, error) { } for _, rj := range retryJoin { - rjMap := rj.(map[string]any) + rjMap, ok := rj.(map[string]any) if !ok { return nil, fmt.Errorf("malformed retry_join stanza: %v+", rj) } @@ -1127,11 +1130,11 @@ func parseHAStorage(result *Config, list *ast.ObjectList, name string) error { } m := make(map[string]string) - for k, val := range config { - valStr, ok := val.(string) + for k, v := range config { + vStr, ok := v.(string) if ok { var err error - m[k], err = normalizeStorageConfigAddresses(key, k, valStr) + m[k], err = normalizeStorageConfigAddresses(key, k, vStr) if err != nil { return err } @@ -1139,21 +1142,21 @@ func parseHAStorage(result *Config, list *ast.ObjectList, name string) error { } var err error - var valBytes []byte + var vBytes []byte // Raft's retry_join requires special normalization due to its complexity if key == "raft" && k == "retry_join" { - valBytes, err = normalizeRaftRetryJoin(val) + vBytes, err = normalizeRaftRetryJoin(v) if err != nil { return err } } else { - valBytes, err = json.Marshal(val) + vBytes, err = json.Marshal(v) if err != nil { return err } } - m[k] = string(valBytes) + m[k] = string(vBytes) } // Pull out the redirect address since it's common to all backends diff --git a/command/server/config_test_helpers.go b/command/server/config_test_helpers.go index d3add9a4f0bc..3d71364b8d09 100644 --- a/command/server/config_test_helpers.go +++ b/command/server/config_test_helpers.go @@ -31,8 +31,7 @@ func boolPointer(x bool) *bool { return &x } -// testConfigRaftRetryJoin ensures that we properly decode and normalize -// config which contains IPv6 addresses. +// testConfigRaftRetryJoin decodes and normalizes retry_join stanzas. func testConfigRaftRetryJoin(t *testing.T) { t.Helper() config, err := LoadConfigFile("./test-fixtures/raft_retry_join.hcl") @@ -1140,6 +1139,9 @@ ha_storage "consul" { } } +// testParseStorageURLConformance verifies that any storage configuration that +// takes a URL, IP Address, or host:port address conforms to RFC-5942 §4 when +// configured with an IPv6 address. See: https://rfc-editor.org/rfc/rfc5952.html func testParseStorageURLConformance(t *testing.T) { t.Parallel() diff --git a/internalshared/configutil/kms.go b/internalshared/configutil/kms.go index 6e7d19bf4b2a..36e6539a7556 100644 --- a/internalshared/configutil/kms.go +++ b/internalshared/configutil/kms.go @@ -218,10 +218,11 @@ func ParseKMSes(d string) ([]*KMS, error) { return result.Seals, nil } -// kmsSealAddressKeys is a map to seal keys that might contain URLs, IP -// addresses, or IP:port style addresses. All physical storage types must have -// an entry in this map, otherwise our normalization check will fail when -// parsing the storage entry config. +// kmsSealAddressKeys maps seal key types to corresponding config keys whose +// values might contain URLs, IP addresses, or host:port addresses. All seal +// types must contain an entry here, otherwise our normalization check will fail +// when parsing the seal config. Seal types which do not contain such +// configurations ought to have an empty array as the value in the map. var kmsSealAddressKeys = map[string][]string{ wrapping.WrapperTypeAliCloudKms.String(): {"domain"}, wrapping.WrapperTypeAwsKms.String(): {"endpoint"}, @@ -233,8 +234,10 @@ var kmsSealAddressKeys = map[string][]string{ "pkcs11-disabled": {}, // only used in tests } -// normalizeKMSSealConfigAddrs takes a kms seal type, a config key, and config -// value and will normalize any URLs, IP addresses, or IP:port style addresses. +// normalizeKMSSealConfigAddrs takes a kms seal type, a config key, and its +// associated value and will normalize any URLs, IP addresses, or host:port +// addresses contained in the value if the config key is known in the +// kmsSealAddressKeys. func normalizeKMSSealConfigAddrs(seal string, key string, value string) (string, error) { keys, ok := kmsSealAddressKeys[seal] if !ok { @@ -248,6 +251,8 @@ func normalizeKMSSealConfigAddrs(seal string, key string, value string) (string, return value, nil } +// mergeKMSEnvConfig takes a KMS and merges any normalized values set via +// environment variables. func mergeKMSEnvConfig(configKMS *KMS) error { envConfig := GetEnvConfigFunc(configKMS) if len(envConfig) > 0 && configKMS.Config == nil { @@ -276,6 +281,7 @@ func configureWrapper(configKMS *KMS, infoKeys *[]string, info *map[string]strin var kmsInfo map[string]string var err error + // Get the any seal config set as env variables and merge it into the KMS. if err = mergeKMSEnvConfig(configKMS); err != nil { return nil, err } diff --git a/internalshared/configutil/listener.go b/internalshared/configutil/listener.go index 5564f6fa98f2..d06a70bdcc70 100644 --- a/internalshared/configutil/listener.go +++ b/internalshared/configutil/listener.go @@ -6,9 +6,7 @@ package configutil import ( "errors" "fmt" - "net" "net/textproto" - "net/url" "regexp" "strings" "time" @@ -198,98 +196,6 @@ func ParseSingleIPTemplate(ipTmpl string) (string, error) { } } -// NormalizeAddr takes a string and returns a conformant copy of it. If the given -// string is not a URL, or if the URL's Hostname is either a DNS hostname or -// IPv4 address, the given URL will be returned unchanged. If the URL hostname -// is an IPv6 address then the address will normalized to conform to RFC-5952. -// See: https://rfc-editor.org/rfc/rfc5952.html -func NormalizeAddr(u string) string { - if u == "" { - return "" - } - - var ip net.IP - var port string - bracketedIPv6 := false - - // Try parsing it as a URL - pu, err := url.Parse(u) - 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(u) - if ip == nil { - // We haven't been given a URL or IP address, try parsing an IP:Port - // combination. - idx := strings.LastIndex(u, ":") - if idx > 0 { - // We've perhaps received an IP:Port address - addr := u[:idx] - port = u[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 u - } - - if v4 := ip.To4(); v4 != nil { - // We don't need to normalize IPv4 addresses. - if pu != nil && port == "" { - return pu.String() - } - - if port != "" { - // Return the address:port - return fmt.Sprintf("%s:%s", v4.String(), port) - } - - // Return the ip addres - return v4.String() - } - - 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() - } - - // 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() - } - - // It shouldn't be possible to get to this point. If we somehow we manage - // to, return the string unchanged. - return u -} - // ParseListeners attempts to parse the AST list of objects into listeners. func ParseListeners(list *ast.ObjectList) ([]*Listener, error) { listeners := make([]*Listener, len(list.Items)) diff --git a/internalshared/configutil/listener_test.go b/internalshared/configutil/listener_test.go index 4d8cb63c267c..02237ca45b7f 100644 --- a/internalshared/configutil/listener_test.go +++ b/internalshared/configutil/listener_test.go @@ -90,92 +90,6 @@ func TestListener_ParseSingleIPTemplate(t *testing.T) { } } -// TestNormalizeAddr ensures that strings that match either an IP address or URL -// and contain an IPv6 address conform to RFC-5952 -// See: https://rfc-editor.org/rfc/rfc5952.html -func TestNormalizeAddr(t *testing.T) { - t.Parallel() - - tests := map[string]struct { - addr string - expected string - isErrorExpected bool - }{ - "hostname": { - addr: "https://vaultproject.io:8200", - expected: "https://vaultproject.io:8200", - }, - "ipv4": { - 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 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", - }, - "ipv6 IP:Port addr with brackets": { - addr: "[2001:0db8::0001]:8500", - expected: "[2001:db8::1]:8500", - }, - "ipv6 RFC-5952 4.1 conformance leading zeroes": { - addr: "2001:0db8::0001", - expected: "2001:db8::1", - }, - "ipv6 URL RFC-5952 4.1 conformance leading zeroes": { - addr: "https://[2001:0db8::0001]:8200", - expected: "https://[2001:db8::1]:8200", - }, - "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", - }, - "ipv6 URL RFC-5952 4.2.2 conformance one 16-bit 0 field": { - addr: "https://[2001:db8:0:1:1:1:1:1]:8200", - expected: "https://[2001:db8:0:1:1:1:1:1]:8200", - }, - "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", - }, - "ipv6 URL RFC-5952 4.2.3 conformance longest run of 0 bits shortened": { - addr: "https://[2001:0:0:1:0:0:0:1]:8200", - expected: "https://[2001:0:0:1::1]:8200", - }, - "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": { - addr: "https://[2001:db8:0:0:1:0:0:1]:8200", - expected: "https://[2001:db8::1:0:0:1]:8200", - }, - "ipv6 RFC-5952 4.3 conformance downcase hex letters": { - addr: "2001:DB8:AC3:FE4::1", - expected: "2001:db8:ac3:fe4::1", - }, - "ipv6 URL RFC-5952 4.3 conformance downcase hex letters": { - addr: "https://[2001:DB8:AC3:FE4::1]:8200", - expected: "https://[2001:db8:ac3:fe4::1]:8200", - }, - } - for name, tc := range tests { - name := name - tc := tc - t.Run(name, func(t *testing.T) { - t.Parallel() - require.Equal(t, tc.expected, NormalizeAddr(tc.addr)) - }) - } -} - // TestListener_parseType exercises the listener receiver parseType. // We check various inputs to ensure we can parse the values as expected and // assign the relevant value on the SharedConfig struct. diff --git a/internalshared/configutil/normalize.go b/internalshared/configutil/normalize.go new file mode 100644 index 000000000000..9b1e7db02f50 --- /dev/null +++ b/internalshared/configutil/normalize.go @@ -0,0 +1,99 @@ +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(u string) string { + if u == "" { + return "" + } + + var ip net.IP + var port string + bracketedIPv6 := false + + // Try parsing it as a URL + pu, err := url.Parse(u) + 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(u) + if ip == nil { + // We haven't been given a URL or IP address, try parsing an IP:Port + // combination. + idx := strings.LastIndex(u, ":") + if idx > 0 { + // We've perhaps received an IP:Port address + addr := u[:idx] + port = u[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 u + } + + if v4 := ip.To4(); v4 != nil { + // We don't need to normalize IPv4 addresses. + if pu != nil && port == "" { + return pu.String() + } + + if port != "" { + // Return the address:port + return fmt.Sprintf("%s:%s", v4.String(), port) + } + + // Return the ip addres + return v4.String() + } + + 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() + } + + // 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() + } + + // It shouldn't be possible to get to this point. If we somehow we manage + // to, return the string unchanged. + return u +} diff --git a/internalshared/configutil/normalize_test.go b/internalshared/configutil/normalize_test.go new file mode 100644 index 000000000000..cce21ada68f6 --- /dev/null +++ b/internalshared/configutil/normalize_test.go @@ -0,0 +1,93 @@ +package configutil + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// TestNormalizeAddr ensures that strings that match either an IP address or URL +// and contain an IPv6 address conform to RFC-5942 §4 +// See: https://rfc-editor.org/rfc/rfc5952.html +func TestNormalizeAddr(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + addr string + expected string + isErrorExpected bool + }{ + "hostname": { + addr: "https://vaultproject.io:8200", + expected: "https://vaultproject.io:8200", + }, + "ipv4": { + 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 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", + }, + "ipv6 IP:Port addr with brackets": { + addr: "[2001:0db8::0001]:8500", + expected: "[2001:db8::1]:8500", + }, + "ipv6 RFC-5952 4.1 conformance leading zeroes": { + addr: "2001:0db8::0001", + expected: "2001:db8::1", + }, + "ipv6 URL RFC-5952 4.1 conformance leading zeroes": { + addr: "https://[2001:0db8::0001]:8200", + expected: "https://[2001:db8::1]:8200", + }, + "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", + }, + "ipv6 URL RFC-5952 4.2.2 conformance one 16-bit 0 field": { + addr: "https://[2001:db8:0:1:1:1:1:1]:8200", + expected: "https://[2001:db8:0:1:1:1:1:1]:8200", + }, + "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", + }, + "ipv6 URL RFC-5952 4.2.3 conformance longest run of 0 bits shortened": { + addr: "https://[2001:0:0:1:0:0:0:1]:8200", + expected: "https://[2001:0:0:1::1]:8200", + }, + "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": { + addr: "https://[2001:db8:0:0:1:0:0:1]:8200", + expected: "https://[2001:db8::1:0:0:1]:8200", + }, + "ipv6 RFC-5952 4.3 conformance downcase hex letters": { + addr: "2001:DB8:AC3:FE4::1", + expected: "2001:db8:ac3:fe4::1", + }, + "ipv6 URL RFC-5952 4.3 conformance downcase hex letters": { + addr: "https://[2001:DB8:AC3:FE4::1]:8200", + expected: "https://[2001:db8:ac3:fe4::1]:8200", + }, + } + for name, tc := range tests { + name := name + tc := tc + t.Run(name, func(t *testing.T) { + t.Parallel() + require.Equal(t, tc.expected, NormalizeAddr(tc.addr)) + }) + } +} diff --git a/internalshared/configutil/telemetry.go b/internalshared/configutil/telemetry.go index 88ffc7e54118..493e91d303d8 100644 --- a/internalshared/configutil/telemetry.go +++ b/internalshared/configutil/telemetry.go @@ -36,7 +36,6 @@ const ( ) // Telemetry is the telemetry configuration for the server. -// NOTE: When adding new address or URL fields be sure to update normalizeTelemetryAddresses(). type Telemetry struct { FoundKeys []string `hcl:",decodedFields"` UnusedKeys UnusedKeyMap `hcl:",unusedKeyPositions"` @@ -194,7 +193,9 @@ func parseTelemetry(result *SharedConfig, list *ast.ObjectList) error { return multierror.Prefix(err, "telemetry:") } - // Make sure addresses conform to RFC-5952 + // Make sure addresses conform to RFC-5942 §4. If you've added new fields that + // are an address or URL be sure to update normalizeTelemetryAddresses(). + // See: https://rfc-editor.org/rfc/rfc5952.html normalizeTelemetryAddresses(result.Telemetry) if result.Telemetry.PrometheusRetentionTimeRaw != nil { @@ -246,8 +247,8 @@ func parseTelemetry(result *SharedConfig, list *ast.ObjectList) error { return nil } -// normalizeTelemetryAddresses takes a reference to Telemetry and ensures that -// any address and URL configuration options that are IPv6 conform to RFC-5952. +// normalizeTelemetryAddresses ensures that any telemetry configuration that can +// be a URL, IP Address, or host:port address is conformant with RFC-5942 §4 // See: https://rfc-editor.org/rfc/rfc5952.html func normalizeTelemetryAddresses(in *Telemetry) { if in == nil { diff --git a/internalshared/configutil/telemetry_test.go b/internalshared/configutil/telemetry_test.go index 8b6ecb255e86..ae2fd1f33cb9 100644 --- a/internalshared/configutil/telemetry_test.go +++ b/internalshared/configutil/telemetry_test.go @@ -56,9 +56,10 @@ func TestParsePrefixFilters(t *testing.T) { }) } -// Test_normalizeTelemetryAddresses verifies that addr and URL configuration is -// normalized to conform to RFC-5952. -func Test_normalizeTelemetryAddresses(t *testing.T) { +// TestNormalizeTelemetryAddresses ensures that any telemetry configuration that +// can be a URL, IP Address, or host:port address is conformant with RFC-5942 §4 +// See: https://rfc-editor.org/rfc/rfc5952.html +func TestNormalizeTelemetryAddresses(t *testing.T) { t.Parallel() tests := map[string]struct { From afddd326f9bb22be24a82c7d5523724f01ea1a24 Mon Sep 17 00:00:00 2001 From: Ryan Cragun Date: Wed, 18 Dec 2024 14:31:43 -0700 Subject: [PATCH 09/12] add changelog Signed-off-by: Ryan Cragun --- changelog/29228.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/29228.txt diff --git a/changelog/29228.txt b/changelog/29228.txt new file mode 100644 index 000000000000..2bcb35ef5d56 --- /dev/null +++ b/changelog/29228.txt @@ -0,0 +1,3 @@ +```release-note:change +server/config: Configuration values which include IPv6 addresses as described in RFC-4241 will be automatically translated and displayed conformant to RFC-5952 §4. +``` From fdb05f92803679bf6f9394e87c32ba5292e0922a Mon Sep 17 00:00:00 2001 From: Ryan Cragun Date: Wed, 18 Dec 2024 14:32:17 -0700 Subject: [PATCH 10/12] fix copywrite headers Signed-off-by: Ryan Cragun --- internalshared/configutil/normalize.go | 3 +++ internalshared/configutil/normalize_test.go | 3 +++ 2 files changed, 6 insertions(+) diff --git a/internalshared/configutil/normalize.go b/internalshared/configutil/normalize.go index 9b1e7db02f50..47feef8f6e65 100644 --- a/internalshared/configutil/normalize.go +++ b/internalshared/configutil/normalize.go @@ -1,3 +1,6 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + package configutil import ( diff --git a/internalshared/configutil/normalize_test.go b/internalshared/configutil/normalize_test.go index cce21ada68f6..d1aec31e621b 100644 --- a/internalshared/configutil/normalize_test.go +++ b/internalshared/configutil/normalize_test.go @@ -1,3 +1,6 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + package configutil import ( From 477e93b338651b55aea7834a1bfff71dd89e71f7 Mon Sep 17 00:00:00 2001 From: Ryan Cragun Date: Wed, 18 Dec 2024 15:32:08 -0700 Subject: [PATCH 11/12] fix tests Always use the type when verifying storage and seals. We'll also update an older test that didn't use real storage types during migration to use real storage types. Signed-off-by: Ryan Cragun --- command/operator_migrate_test.go | 25 ++++++++-------- command/server/config.go | 50 +++++++++++++++++--------------- internalshared/configutil/kms.go | 3 +- 3 files changed, 41 insertions(+), 37 deletions(-) diff --git a/command/operator_migrate_test.go b/command/operator_migrate_test.go index 15190b2640f5..2de2692d144e 100644 --- a/command/operator_migrate_test.go +++ b/command/operator_migrate_test.go @@ -18,6 +18,7 @@ import ( "time" "github.com/go-test/deep" + log "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-secure-stdlib/base62" "github.com/hashicorp/vault/command/server" @@ -190,23 +191,23 @@ func TestMigration(t *testing.T) { cmd := new(OperatorMigrateCommand) cfgName := filepath.Join(t.TempDir(), "migrator") os.WriteFile(cfgName, []byte(` -storage_source "src_type" { +storage_source "consul" { path = "src_path" } -storage_destination "dest_type" { +storage_destination "raft" { path = "dest_path" }`), 0o644) expCfg := &migratorConfig{ StorageSource: &server.Storage{ - Type: "src_type", + Type: "consul", Config: map[string]string{ "path": "src_path", }, }, StorageDestination: &server.Storage{ - Type: "dest_type", + Type: "raft", Config: map[string]string{ "path": "dest_path", }, @@ -230,41 +231,41 @@ storage_destination "dest_type" { // missing source verifyBad(` -storage_destination "dest_type" { +storage_destination "raft" { path = "dest_path" }`) // missing destination verifyBad(` -storage_source "src_type" { +storage_source "consul" { path = "src_path" }`) // duplicate source verifyBad(` -storage_source "src_type" { +storage_source "consul" { path = "src_path" } -storage_source "src_type2" { +storage_source "raft" { path = "src_path" } -storage_destination "dest_type" { +storage_destination "raft" { path = "dest_path" }`) // duplicate destination verifyBad(` -storage_source "src_type" { +storage_source "consul" { path = "src_path" } -storage_destination "dest_type" { +storage_destination "raft" { path = "dest_path" } -storage_destination "dest_type2" { +storage_destination "consul" { path = "dest_path" }`) }) diff --git a/command/server/config.go b/command/server/config.go index 6e2ff9adb2b4..6840c8770fe2 100644 --- a/command/server/config.go +++ b/command/server/config.go @@ -1022,29 +1022,33 @@ func ParseStorage(result *Config, list *ast.ObjectList, name string) error { // config. Physical storage types which don't contain such keys should include // an empty array. var storageAddressKeys = map[string][]string{ - "aerospike": {"hostname"}, - "alicloudoss": {"endpoint"}, - "azure": {"arm_endpoint"}, - "cassandra": {"hosts"}, - "cockroachdb": {"connection_url"}, - "consul": {"address", "service_address"}, - "couchdb": {"endpoint"}, - "dynamodb": {"endpoint"}, - "etcd": {"address", "discovery_srv"}, - "filesystem": {}, - "foundationdb": {}, - "gcs": {}, - "inmem": {}, - "manta": {"url"}, - "mssql": {"server"}, - "mysql": {"address"}, - "oci": {}, - "postgresql": {"connection_url"}, - "raft": {}, // retry_join is handled separately in normalizeRaftRetryJoin() - "s3": {"endpoint"}, - "spanner": {}, - "swift": {"auth_url", "storage_url"}, - "zookeeper": {"address"}, + "aerospike": {"hostname"}, + "alicloudoss": {"endpoint"}, + "azure": {"arm_endpoint"}, + "cassandra": {"hosts"}, + "cockroachdb": {"connection_url"}, + "consul": {"address", "service_address"}, + "couchdb": {"endpoint"}, + "dynamodb": {"endpoint"}, + "etcd": {"address", "discovery_srv"}, + "file": {}, + "filesystem": {}, + "foundationdb": {}, + "gcs": {}, + "inmem": {}, + "inmem_ha": {}, + "inmem_transactional": {}, + "inmem_transactional_ha": {}, + "manta": {"url"}, + "mssql": {"server"}, + "mysql": {"address"}, + "oci": {}, + "postgresql": {"connection_url"}, + "raft": {}, // retry_join is handled separately in normalizeRaftRetryJoin() + "s3": {"endpoint"}, + "spanner": {}, + "swift": {"auth_url", "storage_url"}, + "zookeeper": {"address"}, } // normalizeStorageConfigAddresses takes a storage name, a configuration key diff --git a/internalshared/configutil/kms.go b/internalshared/configutil/kms.go index 36e6539a7556..fbddde17fda8 100644 --- a/internalshared/configutil/kms.go +++ b/internalshared/configutil/kms.go @@ -158,7 +158,7 @@ func parseKMS(result *[]*KMS, list *ast.ObjectList, blockName string, maxKMS int if err != nil { return multierror.Prefix(err, fmt.Sprintf("%s.%s:", blockName, key)) } - strMap[k], err = normalizeKMSSealConfigAddrs(name, k, s) + strMap[k], err = normalizeKMSSealConfigAddrs(key, k, s) if err != nil { return multierror.Prefix(err, fmt.Sprintf("%s.%s:", blockName, key)) } @@ -231,7 +231,6 @@ var kmsSealAddressKeys = map[string][]string{ wrapping.WrapperTypeOciKms.String(): {"key_id", "crypto_endpoint", "management_endpoint"}, wrapping.WrapperTypePkcs11.String(): {}, wrapping.WrapperTypeTransit.String(): {"address"}, - "pkcs11-disabled": {}, // only used in tests } // normalizeKMSSealConfigAddrs takes a kms seal type, a config key, and its From 377db0c1feb56a024188562add66ab438788ce8d Mon Sep 17 00:00:00 2001 From: Ryan Cragun Date: Wed, 18 Dec 2024 16:27:41 -0700 Subject: [PATCH 12/12] vet: fix vet issues Signed-off-by: Ryan Cragun --- command/server/config_test.go | 4 ++++ internalshared/configutil/kms.go | 2 +- internalshared/configutil/kms_test.go | 8 ++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/command/server/config_test.go b/command/server/config_test.go index 036409c2dbe0..a5ac858e6a3c 100644 --- a/command/server/config_test.go +++ b/command/server/config_test.go @@ -66,6 +66,10 @@ func TestParseStorage(t *testing.T) { testParseStorageTemplate(t) } +// TestParseStorageURLConformance tests that all config attrs whose values can be +// URLs, IP addresses, or host:port addresses, when configured with an IPv6 +// address, the normalized to be conformant with RFC-5942 §4 +// See: https://rfc-editor.org/rfc/rfc5952.html func TestParseStorageURLConformance(t *testing.T) { testParseStorageURLConformance(t) } diff --git a/internalshared/configutil/kms.go b/internalshared/configutil/kms.go index fbddde17fda8..575ce4ac60d1 100644 --- a/internalshared/configutil/kms.go +++ b/internalshared/configutil/kms.go @@ -287,7 +287,7 @@ func configureWrapper(configKMS *KMS, infoKeys *[]string, info *map[string]strin switch wrapping.WrapperType(configKMS.Type) { case wrapping.WrapperTypeShamir: - return nil, nil + return wrapper, nil case wrapping.WrapperTypeAead: wrapper, kmsInfo, err = GetAEADKMSFunc(configKMS, opts...) diff --git a/internalshared/configutil/kms_test.go b/internalshared/configutil/kms_test.go index 969ac6f1ae3f..18a4a03f6f90 100644 --- a/internalshared/configutil/kms_test.go +++ b/internalshared/configutil/kms_test.go @@ -96,6 +96,10 @@ func Test_getEnvConfig(t *testing.T) { } } +// TestParseKMSesURLConformance tests that all config attrs whose values can be +// URLs, IP addresses, or host:port addresses, when configured with an IPv6 +// address, the normalized to be conformant with RFC-5942 §4 +// See: https://rfc-editor.org/rfc/rfc5952.html func TestParseKMSesURLConformance(t *testing.T) { t.Parallel() @@ -291,6 +295,10 @@ seal "transit" { } } +// TestMergeKMSEnvConfigAddrConformance tests that all env config whose values +// can be URLs, IP addresses, or host:port addresses, when configured with an +// an IPv6 address, the normalized to be conformant with RFC-5942 §4 +// See: https://rfc-editor.org/rfc/rfc5952.html func TestMergeKMSEnvConfigAddrConformance(t *testing.T) { for name, tc := range map[string]struct { sealType string // default to name if none given