diff --git a/command/operator_diagnose.go b/command/operator_diagnose.go index b66dea73eb..8b8511c4df 100644 --- a/command/operator_diagnose.go +++ b/command/operator_diagnose.go @@ -231,11 +231,21 @@ func (c *OperatorDiagnoseCommand) offlineDiagnostics(ctx context.Context) error return err } + dirAccess := diagnose.ConsulDirectAccess(config.HAStorage.Config) + if dirAccess != "" { + diagnose.Warn(ctx, dirAccess) + } + if config.Storage != nil && config.Storage.Type == storageTypeConsul { err = physconsul.SetupSecureTLS(api.DefaultConfig(), config.Storage.Config, server.logger, true) if err != nil { return err } + + dirAccess := diagnose.ConsulDirectAccess(config.Storage.Config) + if dirAccess != "" { + diagnose.Warn(ctx, dirAccess) + } } if config.HAStorage != nil && config.HAStorage.Type == storageTypeConsul { @@ -259,11 +269,18 @@ func (c *OperatorDiagnoseCommand) offlineDiagnostics(ctx context.Context) error } return diagnose.Test(ctx, "service-discovery", func(ctx context.Context) error { + srConfig := config.ServiceRegistration.Config // Initialize the Service Discovery, if there is one if config.ServiceRegistration != nil && config.ServiceRegistration.Type == "consul" { + // setupStorage populates the srConfig, so no nil checks are necessary. + dirAccess := diagnose.ConsulDirectAccess(config.ServiceRegistration.Config) + if dirAccess != "" { + diagnose.Warn(ctx, dirAccess) + } + // SetupSecureTLS for service discovery uses the same cert and key to set up physical // storage. See the consul package in physical for details. - err = srconsul.SetupSecureTLS(api.DefaultConfig(), config.ServiceRegistration.Config, server.logger, true) + err = srconsul.SetupSecureTLS(api.DefaultConfig(), srConfig, server.logger, true) if err != nil { return err } diff --git a/command/operator_diagnose_test.go b/command/operator_diagnose_test.go index 35c2e9d395..42f58e65a6 100644 --- a/command/operator_diagnose_test.go +++ b/command/operator_diagnose_test.go @@ -150,6 +150,9 @@ func TestOperatorDiagnoseCommand_Run(t *testing.T) { { Name: "storage", Status: diagnose.ErrorStatus, + Warnings: []string{ + diagnose.AddrDNExistErr, + }, }, }, }, @@ -178,6 +181,39 @@ func TestOperatorDiagnoseCommand_Run(t *testing.T) { Name: "service-discovery", Status: diagnose.ErrorStatus, Message: "failed to verify certificate: x509: certificate has expired or is not yet valid:", + Warnings: []string{ + diagnose.DirAccessErr, + }, + }, + }, + }, + { + "diagnose_direct_storage_access", + []string{ + "-config", "./server/test-fixtures/diagnose_ok_storage_direct_access.hcl", + }, + []*diagnose.Result{ + { + Name: "parse-config", + Status: diagnose.OkStatus, + }, + { + Name: "init-listeners", + Status: diagnose.WarningStatus, + Warnings: []string{ + "TLS is disabled in a Listener config stanza.", + }, + }, + { + Name: "storage", + Status: diagnose.WarningStatus, + Warnings: []string{ + diagnose.DirAccessErr, + }, + }, + { + Name: "service-discovery", + Status: diagnose.OkStatus, }, }, }, diff --git a/command/server/test-fixtures/config_bad_https_storage.hcl b/command/server/test-fixtures/config_bad_https_storage.hcl index 20cc4e2c74..27911d755d 100644 --- a/command/server/test-fixtures/config_bad_https_storage.hcl +++ b/command/server/test-fixtures/config_bad_https_storage.hcl @@ -9,6 +9,7 @@ listener "tcp" { } backend "consul" { + address = "127.0.0.1:8500" foo = "bar" advertise_addr = "foo" scheme = "https" @@ -17,6 +18,7 @@ backend "consul" { } ha_backend "consul" { + address = "127.0.0.1:8500" bar = "baz" advertise_addr = "snafu" disable_clustering = "true" diff --git a/command/server/test-fixtures/config_diagnose_hastorage_bad_https.hcl b/command/server/test-fixtures/config_diagnose_hastorage_bad_https.hcl index 1ffdba05e6..c5187f5615 100644 --- a/command/server/test-fixtures/config_diagnose_hastorage_bad_https.hcl +++ b/command/server/test-fixtures/config_diagnose_hastorage_bad_https.hcl @@ -11,6 +11,8 @@ listener "tcp" { backend "consul" { foo = "bar" advertise_addr = "foo" + address = "127.0.0.1:1028" + } ha_backend "consul" { @@ -24,6 +26,8 @@ ha_backend "consul" { service_registration "consul" { foo = "bar" + address = "127.0.0.1:1028" + } telemetry { diff --git a/command/server/test-fixtures/config_diagnose_ok.hcl b/command/server/test-fixtures/config_diagnose_ok.hcl index 053ad635df..9c1e76d975 100644 --- a/command/server/test-fixtures/config_diagnose_ok.hcl +++ b/command/server/test-fixtures/config_diagnose_ok.hcl @@ -9,17 +9,20 @@ listener "tcp" { } backend "consul" { + address = "127.0.0.1:8500" foo = "bar" advertise_addr = "foo" } ha_backend "consul" { + address = "127.0.0.1:8500" bar = "baz" advertise_addr = "snafu" disable_clustering = "true" } service_registration "consul" { + address = "127.0.0.1:8500" foo = "bar" } diff --git a/command/server/test-fixtures/diagnose_bad_https_consul_sr.hcl b/command/server/test-fixtures/diagnose_bad_https_consul_sr.hcl index 29ddc6fab3..49d1de056e 100644 --- a/command/server/test-fixtures/diagnose_bad_https_consul_sr.hcl +++ b/command/server/test-fixtures/diagnose_bad_https_consul_sr.hcl @@ -11,17 +11,19 @@ listener "tcp" { backend "consul" { foo = "bar" advertise_addr = "foo" + address = "127.0.0.1:8500" } ha_backend "consul" { bar = "baz" advertise_addr = "snafu" disable_clustering = "true" + address = "127.0.0.1:8500" } service_registration "consul" { + address = "https://consulserverIP:8500" foo = "bar" - address = "https://127.0.0.1:8200" tls_cert_file = "./../vault/diagnose/test-fixtures/expiredcert.pem" tls_key_file = "./../vault/diagnose/test-fixtures/expiredprivatekey.pem" } diff --git a/command/server/test-fixtures/diagnose_ok_storage_direct_access.hcl b/command/server/test-fixtures/diagnose_ok_storage_direct_access.hcl new file mode 100644 index 0000000000..4a71fb6065 --- /dev/null +++ b/command/server/test-fixtures/diagnose_ok_storage_direct_access.hcl @@ -0,0 +1,27 @@ +disable_cache = true +disable_mlock = true + +ui = true + +listener "tcp" { + address = "127.0.0.1:1024" + tls_disable = true +} + +backend "consul" { + address = "consulserver:8500" + foo = "bar" + advertise_addr = "foo" +} + +ha_backend "consul" { + address = "127.0.0.1:1024" + bar = "baz" + advertise_addr = "snafu" + disable_clustering = "true" +} + +service_registration "consul" { + address = "127.0.0.1:8500" + foo = "bar" +} \ No newline at end of file diff --git a/command/server/test-fixtures/tls_config_ok.hcl b/command/server/test-fixtures/tls_config_ok.hcl index 83c8d1159b..44be00930e 100644 --- a/command/server/test-fixtures/tls_config_ok.hcl +++ b/command/server/test-fixtures/tls_config_ok.hcl @@ -12,16 +12,19 @@ listener "tcp" { backend "consul" { foo = "bar" advertise_addr = "foo" + address = "127.0.0.1:8500" } ha_backend "consul" { bar = "baz" advertise_addr = "snafu" disable_clustering = "true" + address = "127.0.0.1:8500" } service_registration "consul" { foo = "bar" + address = "127.0.0.1:8500" } telemetry { diff --git a/vault/diagnose/storage_checks.go b/vault/diagnose/storage_checks.go index 5fdf0de2ee..76f6fa4fe2 100644 --- a/vault/diagnose/storage_checks.go +++ b/vault/diagnose/storage_checks.go @@ -3,6 +3,7 @@ package diagnose import ( "context" "fmt" + "strings" "time" "github.com/hashicorp/vault/sdk/physical" @@ -14,6 +15,8 @@ const ( secretVal string = "diagnoseSecret" timeOutErr string = "storage call timed out after 20 seconds: " + DirAccessErr string = "consul storage does not connect to local agent, but directly to server" + AddrDNExistErr string = "config address does not exist: 127.0.0.1:8500 will be used" wrongRWValsPrefix string = "Storage get and put gave wrong values: " ) @@ -74,3 +77,17 @@ func StorageEndToEndLatencyCheck(ctx context.Context, b physical.Backend) error } return nil } + +// ConsulDirectAccess verifies that consul is connecting to local agent, +// versus directly to a remote server. We can only assume that the local address +// is a server, not a client. +func ConsulDirectAccess(config map[string]string) string { + configAddr, ok := config["address"] + if !ok { + return AddrDNExistErr + } + if !strings.Contains(configAddr, "localhost") && !strings.Contains(configAddr, "127.0.0.1") { + return DirAccessErr + } + return "" +}