Diagnoses Consul Direct Access Check (#11505)

* Create helpers which integrate with OpenTelemetry for diagnose collection

* Go mod vendor

* consul tls checks

* draft for storage end to end check

* Comments

* Update vault/diagnose/helpers.go

Co-authored-by: swayne275 <swayne275@gmail.com>

* Add unit test/example

* tweak output

* More comments

* add spot check concept

* Get unit tests working on Result structs

* Fix unit test

* Get unit tests working, and make diagnose sessions local rather than global

* Comments

* Last comments

* No need for init

* :|

* Fix helpers_test

* cleaned up chan logic. Tests next.

* fix tests

* remove a comment

* tests

* remove a comment

* run direct access checks in diagnose command

* review comments

Co-authored-by: Scott G. Miller <smiller@hashicorp.com>
Co-authored-by: swayne275 <swayne275@gmail.com>

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
 			}

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,
 				},
 			},
 		},

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"

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 {

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"
 }
 

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"
 }

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"
+}

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 {

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 ""
+}