diff --git a/command/operator_diagnose.go b/command/operator_diagnose.go index ba10feab4f26..246142b954a9 100644 --- a/command/operator_diagnose.go +++ b/command/operator_diagnose.go @@ -426,7 +426,7 @@ func (c *OperatorDiagnoseCommand) offlineDiagnostics(ctx context.Context) error SEALFAIL: sealspan.End() var coreConfig vault.CoreConfig - if err := diagnose.Test(ctx, "setup-core", func(ctx context.Context) error { + diagnose.Test(ctx, "setup-core", func(ctx context.Context) error { var secureRandomReader io.Reader // prepare a secure random reader for core secureRandomReader, err = configutil.CreateSecureRandomReaderFunc(config.SharedConfig, barrierWrapper) @@ -436,9 +436,7 @@ SEALFAIL: diagnose.SpotOk(ctx, "init-randreader", "") coreConfig = createCoreConfig(server, config, *backend, configSR, barrierSeal, unwrapSeal, metricsHelper, metricSink, secureRandomReader) return nil - }); err != nil { - diagnose.Error(ctx, err) - } + }) var disableClustering bool diagnose.Test(ctx, "setup-ha-storage", func(ctx context.Context) error { @@ -514,6 +512,9 @@ SEALFAIL: info := make(map[string]string) var listeners []listenerutil.Listener var status int + + diagnose.ListenerChecks(ctx, config.Listeners) + diagnose.Test(ctx, "create-listeners", func(ctx context.Context) error { status, listeners, _, err = server.InitListeners(config, disableClustering, &infoKeys, &info) if status != 0 { @@ -531,32 +532,7 @@ SEALFAIL: } } - defer c.cleanupGuard.Do(listenerCloseFunc) - - listenerTLSContext, listenerTLSSpan := diagnose.StartSpan(ctx, "check-listener-tls") - sanitizedListeners := make([]listenerutil.Listener, 0, len(config.Listeners)) - for _, ln := range lns { - if ln.Config.TLSDisable { - diagnose.Warn(listenerTLSContext, "TLS is disabled in a Listener config stanza.") - continue - } - if ln.Config.TLSDisableClientCerts { - diagnose.Warn(listenerTLSContext, "TLS for a listener is turned on without requiring client certs.") - - } - err = diagnose.TLSMutualExclusionCertCheck(ln.Config) - if err != nil { - diagnose.Warn(listenerTLSContext, fmt.Sprintf("TLSDisableClientCerts and TLSRequireAndVerifyClientCert should not both be set. %s", err)) - } - - sanitizedListeners = append(sanitizedListeners, listenerutil.Listener{ - Listener: ln.Listener, - Config: ln.Config, - }) - } - diagnose.ListenerChecks(listenerTLSContext, sanitizedListeners) - - listenerTLSSpan.End() + c.cleanupGuard.Do(listenerCloseFunc) return nil }) diff --git a/vault/diagnose/helpers_test.go b/vault/diagnose/helpers_test.go index 346d55f990c3..2b0d1ec9e142 100644 --- a/vault/diagnose/helpers_test.go +++ b/vault/diagnose/helpers_test.go @@ -3,11 +3,12 @@ package diagnose import ( "context" "errors" - "github.com/go-test/deep" "os" "reflect" "strings" "testing" + + "github.com/go-test/deep" ) const getMoreCoffee = "You'll find more coffee in the freezer door, or consider buying more for the office." @@ -21,6 +22,26 @@ func TestDiagnoseOtelResults(t *testing.T) { }, Advice: getMoreCoffee, Children: []*Result{ + { + Name: "prepare-kitchen", + Status: ErrorStatus, + Children: []*Result{ + { + Name: "build-microwave", + Status: ErrorStatus, + Children: []*Result{ + { + Name: "buy-parts", + Status: ErrorStatus, + Message: "no stores sell microwave parts, please buy a microwave instead.", + Warnings: []string{ + "warning: you are about to try to build a microwave from scratch.", + }, + }, + }, + }, + }, + }, { Name: "warm-milk", Status: OkStatus, @@ -54,6 +75,7 @@ func TestDiagnoseOtelResults(t *testing.T) { results := sess.Finalize(ctx) results.ZeroTimes() + if !reflect.DeepEqual(results, expected) { t.Fatalf("results mismatch: %s", strings.Join(deep.Equal(results, expected), "\n")) } @@ -63,20 +85,25 @@ func TestDiagnoseOtelResults(t *testing.T) { const coffeeLeft = 3 func makeCoffee(ctx context.Context) error { + if coffeeLeft < 5 { Warn(ctx, "coffee getting low") Advise(ctx, getMoreCoffee) } - err := Test(ctx, "warm-milk", warmMilk) - if err != nil { - return err - } + // To mimic listener TLS checks, we'll see if we can nest a Test and add errors in the function + Test(ctx, "prepare-kitchen", func(ctx context.Context) error { + return Test(ctx, "build-microwave", func(ctx context.Context) error { + buildMicrowave(ctx) + return nil + }) + }) - err = brewCoffee(ctx) - if err != nil { - return err - } + Test(ctx, "warm-milk", func(ctx context.Context) error { + return warmMilk(ctx) + }) + + brewCoffee(ctx) SpotCheck(ctx, "pick-scone", pickScone) @@ -84,6 +111,25 @@ func makeCoffee(ctx context.Context) error { return nil } +// buildMicrowave will throw an error in the function itself to fail the span, +// but will return nil so the caller test doesn't necessarily throw an error. +// The intended behavior is that the superspan will detect the failed subspan +// and fail regardless. This happens when Fail is used to fail the span, but not +// when Error is used. See the comment in the function itself. +func buildMicrowave(ctx context.Context) error { + ctx, span := StartSpan(ctx, "buy-parts") + + Fail(ctx, "no stores sell microwave parts, please buy a microwave instead.") + + // The error line here does not actually yield an error in the output. + // TODO: Debug this. In the meantime, always use Fail over Error. + // Error(ctx, errors.New("no stores sell microwave parts, please buy a microwave instead.")) + + Warn(ctx, "warning: you are about to try to build a microwave from scratch.") + span.End() + return nil +} + func warmMilk(ctx context.Context) error { // Always succeeds return nil diff --git a/vault/diagnose/output.go b/vault/diagnose/output.go index 963fece8ab62..a626587b4433 100644 --- a/vault/diagnose/output.go +++ b/vault/diagnose/output.go @@ -4,13 +4,14 @@ import ( "context" "errors" "fmt" - "go.opentelemetry.io/otel/attribute" "io" "sort" "strings" "sync" "time" + "go.opentelemetry.io/otel/attribute" + wordwrap "github.com/mitchellh/go-wordwrap" "go.opentelemetry.io/otel/codes" sdktrace "go.opentelemetry.io/otel/sdk/trace" @@ -318,22 +319,20 @@ func (r *Result) StringWrapped(wrapLimit int) string { func (r *Result) write(sb *strings.Builder, depth int, limit int) { indent(sb, depth) var prelude string - if len(r.Warnings) == 0 { - switch r.Status { - case OkStatus: - prelude = status_ok - case WarningStatus: - prelude = status_warn - case ErrorStatus: - prelude = status_failed - case SkippedStatus: - prelude = status_skipped - } - prelude = prelude + r.Name + switch r.Status { + case OkStatus: + prelude = status_ok + case WarningStatus: + prelude = status_warn + case ErrorStatus: + prelude = status_failed + case SkippedStatus: + prelude = status_skipped + } + prelude = prelude + r.Name - if r.Message != "" { - prelude = prelude + ": " + r.Message - } + if r.Message != "" { + prelude = prelude + ": " + r.Message } warnings := r.Warnings if r.Message == "" && len(warnings) > 0 { @@ -343,6 +342,7 @@ func (r *Result) write(sb *strings.Builder, depth int, limit int) { warnings = warnings[1:] } } + writeWrapped(sb, prelude, depth+1, limit) for _, w := range warnings { sb.WriteRune('\n') diff --git a/vault/diagnose/tls_verification.go b/vault/diagnose/tls_verification.go index f633d867a2f6..d273fe76f4dc 100644 --- a/vault/diagnose/tls_verification.go +++ b/vault/diagnose/tls_verification.go @@ -12,7 +12,6 @@ import ( "time" "github.com/hashicorp/vault/internalshared/configutil" - "github.com/hashicorp/vault/internalshared/listenerutil" "github.com/hashicorp/vault/sdk/helper/tlsutil" ) @@ -21,17 +20,32 @@ const maxVersionError = "'tls_max_version' value %q not supported, please specif // ListenerChecks diagnoses warnings and the first encountered error for the listener // configuration stanzas. -func ListenerChecks(ctx context.Context, listeners []listenerutil.Listener) ([]string, []error) { +func ListenerChecks(ctx context.Context, listeners []*configutil.Listener) ([]string, []error) { + testName := "check-listener-tls" + ctx, span := StartSpan(ctx, testName) + defer span.End() // These aggregated warnings and errors are returned purely for testing purposes. // The errors and warnings will report in this function itself. var listenerWarnings []string var listenerErrors []error - for _, listener := range listeners { - l := listener.Config + for _, l := range listeners { listenerID := l.Address + if l.TLSDisable { + Warn(ctx, fmt.Sprintf("listener at address: %s has error: TLS is disabled in a Listener config stanza.", listenerID)) + continue + } + if l.TLSDisableClientCerts { + Warn(ctx, fmt.Sprintf("listener at address: %s has error: TLS for a listener is turned on without requiring client certs.", listenerID)) + + } + status, warning := TLSMutualExclusionCertCheck(l) + if status == 1 { + Warn(ctx, warning) + } + // Perform the TLS version check for listeners. if l.TLSMinVersion == "" { l.TLSMinVersion = "tls12" @@ -43,13 +57,13 @@ func ListenerChecks(ctx context.Context, listeners []listenerutil.Listener) ([]s if !ok { err := fmt.Errorf("listener at address: %s has error %s: ", listenerID, fmt.Sprintf(minVersionError, l.TLSMinVersion)) listenerErrors = append(listenerErrors, err) - Error(ctx, err) + Fail(ctx, err.Error()) } _, ok = tlsutil.TLSLookup[l.TLSMaxVersion] if !ok { err := fmt.Errorf("listener at address: %s has error %s: ", listenerID, fmt.Sprintf(maxVersionError, l.TLSMaxVersion)) listenerErrors = append(listenerErrors, err) - Error(ctx, err) + Fail(ctx, err.Error()) } // Perform checks on the TLS Cryptographic Information. @@ -74,8 +88,7 @@ func outputError(ctx context.Context, newWarnings, listenerWarnings []string, ne if newErr != nil { errMsg := listenerID + ": " + newErr.Error() listenerErrors = append(listenerErrors, fmt.Errorf(errMsg)) - Error(ctx, fmt.Errorf(errMsg)) - + Fail(ctx, errMsg) } return listenerWarnings, listenerErrors } @@ -256,15 +269,14 @@ func NearExpiration(c *x509.Certificate) (bool, time.Duration) { } // TLSMutualExclusionCertCheck returns error if both TLSDisableClientCerts and TLSRequireAndVerifyClientCert are set -func TLSMutualExclusionCertCheck(l *configutil.Listener) error { +func TLSMutualExclusionCertCheck(l *configutil.Listener) (int, string) { if l.TLSDisableClientCerts { if l.TLSRequireAndVerifyClientCert { - return fmt.Errorf("the tls_disable_client_certs and tls_require_and_verify_client_cert fields in the " + - "listener stanza of the vault server config are mutually exclusive fields. Please ensure they are not both set to true.") + return 1, "the tls_disable_client_certs and tls_require_and_verify_client_cert fields in the listener stanza of the vault server config are mutually exclusive fields. Please ensure they are not both set to true." } } - return nil + return 0, "" } // TLSClientCAFileCheck Checks the validity of a client CA file diff --git a/vault/diagnose/tls_verification_test.go b/vault/diagnose/tls_verification_test.go index ffa9e8946603..0017d9b6e27e 100644 --- a/vault/diagnose/tls_verification_test.go +++ b/vault/diagnose/tls_verification_test.go @@ -7,23 +7,20 @@ import ( "testing" "github.com/hashicorp/vault/internalshared/configutil" - "github.com/hashicorp/vault/internalshared/listenerutil" ) // TestTLSValidCert is the positive test case to show that specifying a valid cert and key // passes all checks. func TestTLSValidCert(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./test-fixtures/goodcertwithroot.pem", - TLSKeyFile: "./test-fixtures/goodkey.pem", - TLSMinVersion: "tls10", - TLSDisableClientCerts: true, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./test-fixtures/goodcertwithroot.pem", + TLSKeyFile: "./test-fixtures/goodkey.pem", + TLSMinVersion: "tls10", + TLSDisableClientCerts: true, }, } warnings, errs := ListenerChecks(context.Background(), listeners) @@ -38,17 +35,15 @@ func TestTLSValidCert(t *testing.T) { // TestTLSFakeCert simply ensures that the certificate file must contain PEM data. func TestTLSFakeCert(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./test-fixtures/fakecert.pem", - TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", - TLSMinVersion: "tls10", - TLSDisableClientCerts: true, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./test-fixtures/fakecert.pem", + TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", + TLSMinVersion: "tls10", + TLSDisableClientCerts: true, }, } _, errs := ListenerChecks(context.Background(), listeners) @@ -68,17 +63,15 @@ func TestTLSFakeCert(t *testing.T) { // an extra DER sequence, and makes sure a trailing data error // is returned. func TestTLSTrailingData(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./test-fixtures/trailingdatacert.pem", - TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", - TLSMinVersion: "tls10", - TLSDisableClientCerts: true, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./test-fixtures/trailingdatacert.pem", + TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", + TLSMinVersion: "tls10", + TLSDisableClientCerts: true, }, } _, errs := ListenerChecks(context.Background(), listeners) @@ -93,17 +86,15 @@ func TestTLSTrailingData(t *testing.T) { // TestTLSExpiredCert checks that an expired certificate fails TLS checks // with an appropriate error. func TestTLSExpiredCert(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./test-fixtures/expiredcert.pem", - TLSKeyFile: "./test-fixtures/expiredprivatekey.pem", - TLSMinVersion: "tls10", - TLSDisableClientCerts: true, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./test-fixtures/expiredcert.pem", + TLSKeyFile: "./test-fixtures/expiredprivatekey.pem", + TLSMinVersion: "tls10", + TLSDisableClientCerts: true, }, } warnings, errs := ListenerChecks(context.Background(), listeners) @@ -124,17 +115,15 @@ func TestTLSExpiredCert(t *testing.T) { // TestTLSMismatchedCryptographicInfo verifies that a cert and key of differing cryptographic // types, when specified together, is met with a unique error message. func TestTLSMismatchedCryptographicInfo(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", - TLSKeyFile: "./test-fixtures/ecdsa.key", - TLSMinVersion: "tls10", - TLSDisableClientCerts: true, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", + TLSKeyFile: "./test-fixtures/ecdsa.key", + TLSMinVersion: "tls10", + TLSDisableClientCerts: true, }, } _, errs := ListenerChecks(context.Background(), listeners) @@ -145,18 +134,16 @@ func TestTLSMismatchedCryptographicInfo(t *testing.T) { t.Fatalf("Bad error message: %s", errs[0]) } - listeners = []listenerutil.Listener{ + listeners = []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./test-fixtures/ecdsa.crt", - TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", - TLSClientCAFile: "./../../api/test-fixtures/root/rootcacert.pem", - TLSMinVersion: "tls10", - TLSDisableClientCerts: true, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./test-fixtures/ecdsa.crt", + TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", + TLSClientCAFile: "./../../api/test-fixtures/root/rootcacert.pem", + TLSMinVersion: "tls10", + TLSDisableClientCerts: true, }, } _, errs = ListenerChecks(context.Background(), listeners) @@ -170,18 +157,16 @@ func TestTLSMismatchedCryptographicInfo(t *testing.T) { // TestTLSMultiKeys verifies that a unique error message is thrown when a key is specified twice. func TestTLSMultiKeys(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./../../api/test-fixtures/keys/key.pem", - TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", - TLSClientCAFile: "./../../api/test-fixtures/root/rootcacert.pem", - TLSMinVersion: "tls10", - TLSDisableClientCerts: true, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./../../api/test-fixtures/keys/key.pem", + TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", + TLSClientCAFile: "./../../api/test-fixtures/root/rootcacert.pem", + TLSMinVersion: "tls10", + TLSDisableClientCerts: true, }, } _, errs := ListenerChecks(context.Background(), listeners) @@ -195,17 +180,15 @@ func TestTLSMultiKeys(t *testing.T) { // TestTLSCertAsKey verifies that a unique error message is thrown when a cert is specified twice. func TestTLSCertAsKey(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", - TLSKeyFile: "./../../api/test-fixtures/keys/cert.pem", - TLSMinVersion: "tls10", - TLSDisableClientCerts: true, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", + TLSKeyFile: "./../../api/test-fixtures/keys/cert.pem", + TLSMinVersion: "tls10", + TLSDisableClientCerts: true, }, } _, errs := ListenerChecks(context.Background(), listeners) @@ -221,17 +204,15 @@ func TestTLSCertAsKey(t *testing.T) { // the root. The root certificate used in this test is the Baltimore Cyber Trust root // certificate, downloaded from: https://www.digicert.com/kb/digicert-root-certificates.htm func TestTLSInvalidRoot(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./test-fixtures/goodcertbadroot.pem", - TLSKeyFile: "./test-fixtures/goodkey.pem", - TLSMinVersion: "tls10", - TLSDisableClientCerts: true, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./test-fixtures/goodcertbadroot.pem", + TLSKeyFile: "./test-fixtures/goodkey.pem", + TLSMinVersion: "tls10", + TLSDisableClientCerts: true, }, } _, errs := ListenerChecks(context.Background(), listeners) @@ -247,17 +228,15 @@ func TestTLSInvalidRoot(t *testing.T) { // is still accepted by diagnose as valid. This is an acceptable, though less secure, // server configuration. func TestTLSNoRoot(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", - TLSKeyFile: "./test-fixtures/goodkey.pem", - TLSMinVersion: "tls10", - TLSDisableClientCerts: true, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", + TLSKeyFile: "./test-fixtures/goodkey.pem", + TLSMinVersion: "tls10", + TLSDisableClientCerts: true, }, } _, errs := ListenerChecks(context.Background(), listeners) @@ -270,18 +249,16 @@ func TestTLSNoRoot(t *testing.T) { // TestTLSInvalidMinVersion checks that a listener with an invalid minimum configured // version errors appropriately. func TestTLSInvalidMinVersion(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", - TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", - TLSClientCAFile: "./../../api/test-fixtures/root/rootcacert.pem", - TLSMinVersion: "0", - TLSDisableClientCerts: true, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", + TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", + TLSClientCAFile: "./../../api/test-fixtures/root/rootcacert.pem", + TLSMinVersion: "0", + TLSDisableClientCerts: true, }, } _, errs := ListenerChecks(context.Background(), listeners) @@ -296,18 +273,16 @@ func TestTLSInvalidMinVersion(t *testing.T) { // TestTLSInvalidMaxVersion checks that a listener with an invalid maximum configured // version errors appropriately. func TestTLSInvalidMaxVersion(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", - TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", - TLSClientCAFile: "./../../api/test-fixtures/root/rootcacert.pem", - TLSMaxVersion: "0", - TLSDisableClientCerts: true, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", + TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", + TLSClientCAFile: "./../../api/test-fixtures/root/rootcacert.pem", + TLSMaxVersion: "0", + TLSDisableClientCerts: true, }, } _, errs := ListenerChecks(context.Background(), listeners) @@ -322,46 +297,42 @@ func TestTLSInvalidMaxVersion(t *testing.T) { // TestDisabledClientCertsAndDisabledTLSClientCAVerfiy checks that a listener works properly when both // TLSRequireAndVerifyClientCert and TLSDisableClientCerts are false func TestDisabledClientCertsAndDisabledTLSClientCAVerfiy(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", - TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", - TLSClientCAFile: "./../../api/test-fixtures/root/rootcacert.pem", - TLSMaxVersion: "tls10", - TLSRequireAndVerifyClientCert: false, - TLSDisableClientCerts: false, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", + TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", + TLSClientCAFile: "./../../api/test-fixtures/root/rootcacert.pem", + TLSMaxVersion: "tls10", + TLSRequireAndVerifyClientCert: false, + TLSDisableClientCerts: false, }, } - err := TLSMutualExclusionCertCheck(listeners[0].Config) - if err != nil { + status, _ := TLSMutualExclusionCertCheck(listeners[0]) + if status != 0 { t.Fatalf("TLS config failed when both TLSRequireAndVerifyClientCert and TLSDisableClientCerts are false") } } // TestTLSClientCAVerfiy checks that a listener which has TLS client certs checks enabled works as expected func TestTLSClientCAVerfiy(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", - TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", - TLSClientCAFile: "./../../api/test-fixtures/root/rootcacert.pem", - TLSMaxVersion: "tls10", - TLSRequireAndVerifyClientCert: true, - TLSDisableClientCerts: false, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", + TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", + TLSClientCAFile: "./../../api/test-fixtures/root/rootcacert.pem", + TLSMaxVersion: "tls10", + TLSRequireAndVerifyClientCert: true, + TLSDisableClientCerts: false, }, } - err := TLSMutualExclusionCertCheck(listeners[0].Config) - if err != nil { + status, err := TLSMutualExclusionCertCheck(listeners[0]) + if status != 0 { t.Fatalf("TLS config check failed with %s", err) } } @@ -369,23 +340,21 @@ func TestTLSClientCAVerfiy(t *testing.T) { // TestTLSClientCAVerfiySkip checks that TLS client cert checks are skipped if TLSDisableClientCerts is true // regardless of the value for TLSRequireAndVerifyClientCert func TestTLSClientCAVerfiySkip(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", - TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", - TLSClientCAFile: "./../../api/test-fixtures/root/rootcacert.pem", - TLSMaxVersion: "tls10", - TLSRequireAndVerifyClientCert: false, - TLSDisableClientCerts: true, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", + TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", + TLSClientCAFile: "./../../api/test-fixtures/root/rootcacert.pem", + TLSMaxVersion: "tls10", + TLSRequireAndVerifyClientCert: false, + TLSDisableClientCerts: true, }, } - err := TLSMutualExclusionCertCheck(listeners[0].Config) - if err != nil { + status, err := TLSMutualExclusionCertCheck(listeners[0]) + if status != 0 { t.Fatalf("TLS config check did not skip verification and failed with %s", err) } } @@ -393,26 +362,24 @@ func TestTLSClientCAVerfiySkip(t *testing.T) { // TestTLSClientCAVerfiyMutualExclusion checks that TLS client cert checks are skipped if TLSDisableClientCerts is true // regardless of the value for TLSRequireAndVerifyClientCert func TestTLSClientCAVerfiyMutualExclusion(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", - TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", - TLSClientCAFile: "./../../api/test-fixtures/root/rootcacert.pem", - TLSMaxVersion: "tls10", - TLSRequireAndVerifyClientCert: true, - TLSDisableClientCerts: true, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", + TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", + TLSClientCAFile: "./../../api/test-fixtures/root/rootcacert.pem", + TLSMaxVersion: "tls10", + TLSRequireAndVerifyClientCert: true, + TLSDisableClientCerts: true, }, } - err := TLSMutualExclusionCertCheck(listeners[0].Config) - if err == nil { + status, err := TLSMutualExclusionCertCheck(listeners[0]) + if status == 0 { t.Fatalf("TLS config check should have failed when both 'tls_disable_client_certs' and 'tls_require_and_verify_client_cert' are true") } - if !strings.Contains(err.Error(), "the tls_disable_client_certs and tls_require_and_verify_client_cert fields in the "+ + if !strings.Contains(err, "the tls_disable_client_certs and tls_require_and_verify_client_cert fields in the "+ "listener stanza of the vault server config are mutually exclusive fields") { t.Fatalf("Bad error message: %s", err) } @@ -420,19 +387,17 @@ func TestTLSClientCAVerfiyMutualExclusion(t *testing.T) { // TestTLSClientCAVerfiy checks that a listener which has TLS client certs checks enabled works as expected func TestTLSClientCAFileCheck(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", - TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", - TLSClientCAFile: "./../../api/test-fixtures/root/rootcacert.pem", - TLSMaxVersion: "tls10", - TLSRequireAndVerifyClientCert: true, - TLSDisableClientCerts: false, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", + TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", + TLSClientCAFile: "./../../api/test-fixtures/root/rootcacert.pem", + TLSMaxVersion: "tls10", + TLSRequireAndVerifyClientCert: true, + TLSDisableClientCerts: false, }, } warnings, errs := ListenerChecks(context.Background(), listeners) @@ -446,19 +411,17 @@ func TestTLSClientCAFileCheck(t *testing.T) { // TestTLSLeafCertInClientCAFile checks if a leafCert exist in TLSClientCAFile func TestTLSLeafCertInClientCAFile(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", - TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", - TLSClientCAFile: "./test-fixtures/goodcertbadroot.pem", - TLSMaxVersion: "tls10", - TLSRequireAndVerifyClientCert: true, - TLSDisableClientCerts: false, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", + TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", + TLSClientCAFile: "./test-fixtures/goodcertbadroot.pem", + TLSMaxVersion: "tls10", + TLSRequireAndVerifyClientCert: true, + TLSDisableClientCerts: false, }, } _, errs := ListenerChecks(context.Background(), listeners) @@ -472,19 +435,17 @@ func TestTLSLeafCertInClientCAFile(t *testing.T) { // TestTLSNoRootInClientCAFile checks if no Root cert exist in TLSClientCAFile func TestTLSNoRootInClientCAFile(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", - TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", - TLSClientCAFile: "./test-fixtures/intermediateCert.pem", - TLSMaxVersion: "tls10", - TLSRequireAndVerifyClientCert: true, - TLSDisableClientCerts: false, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", + TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", + TLSClientCAFile: "./test-fixtures/intermediateCert.pem", + TLSMaxVersion: "tls10", + TLSRequireAndVerifyClientCert: true, + TLSDisableClientCerts: false, }, } _, errs := ListenerChecks(context.Background(), listeners) @@ -498,19 +459,17 @@ func TestTLSNoRootInClientCAFile(t *testing.T) { // TestTLSIntermediateCertInClientCAFile checks if an intermediate cert is included in TLSClientCAFile func TestTLSIntermediateCertInClientCAFile(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", - TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", - TLSClientCAFile: "./test-fixtures/chain.crt.pem", - TLSMaxVersion: "tls10", - TLSRequireAndVerifyClientCert: true, - TLSDisableClientCerts: false, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", + TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", + TLSClientCAFile: "./test-fixtures/chain.crt.pem", + TLSMaxVersion: "tls10", + TLSRequireAndVerifyClientCert: true, + TLSDisableClientCerts: false, }, } _, errs := ListenerChecks(context.Background(), listeners) @@ -524,19 +483,17 @@ func TestTLSIntermediateCertInClientCAFile(t *testing.T) { // TestTLSMultipleRootInClietCACert checks if multiple roots included in TLSClientCAFile func TestTLSMultipleRootInClietCACert(t *testing.T) { - listeners := []listenerutil.Listener{ + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", - TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", - TLSClientCAFile: "./test-fixtures/twoRootCA.pem", - TLSMinVersion: "tls10", - TLSRequireAndVerifyClientCert: true, - TLSDisableClientCerts: false, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", + TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", + TLSClientCAFile: "./test-fixtures/twoRootCA.pem", + TLSMinVersion: "tls10", + TLSRequireAndVerifyClientCert: true, + TLSDisableClientCerts: false, }, } warnings, errs := ListenerChecks(context.Background(), listeners) @@ -552,20 +509,18 @@ func TestTLSMultipleRootInClietCACert(t *testing.T) { } // TestTLSSelfSignedCerts tests invalid self-signed cert as TLSClientCAFile -func TestTLSSelfSignedCerts(t *testing.T) { - listeners := []listenerutil.Listener{ +func TestTLSSelfSignedCert(t *testing.T) { + listeners := []*configutil.Listener{ { - Config: &configutil.Listener{ - Type: "tcp", - Address: "127.0.0.1:443", - ClusterAddress: "127.0.0.1:8201", - TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", - TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", - TLSClientCAFile: "test-fixtures/selfSignedCert.pem", - TLSMinVersion: "tls10", - TLSRequireAndVerifyClientCert: true, - TLSDisableClientCerts: false, - }, + Type: "tcp", + Address: "127.0.0.1:443", + ClusterAddress: "127.0.0.1:8201", + TLSCertFile: "./../../api/test-fixtures/keys/cert.pem", + TLSKeyFile: "./../../api/test-fixtures/keys/key.pem", + TLSClientCAFile: "test-fixtures/selfSignedCert.pem", + TLSMinVersion: "tls10", + TLSRequireAndVerifyClientCert: true, + TLSDisableClientCerts: false, }, } _, errs := ListenerChecks(context.Background(), listeners) diff --git a/vault/init.go b/vault/init.go index c7662524e41e..24ea18120d0b 100644 --- a/vault/init.go +++ b/vault/init.go @@ -11,7 +11,6 @@ import ( wrapping "github.com/hashicorp/go-kms-wrapping" "github.com/hashicorp/vault/physical/raft" - "github.com/hashicorp/vault/vault/diagnose" "github.com/hashicorp/vault/vault/seal" aeadwrapper "github.com/hashicorp/go-kms-wrapping/wrappers/aead" @@ -467,11 +466,9 @@ func (c *Core) UnsealWithStoredKeys(ctx context.Context) error { // This usually happens when auto-unseal is configured, but the servers have // not been initialized yet. if len(keys) == 0 { - diagnose.Error(ctx, errors.New("stored unseal keys are supported, but none were found")) return NewNonFatalError(errors.New("stored unseal keys are supported, but none were found")) } if len(keys) != 1 { - diagnose.Error(ctx, errors.New("expected exactly one stored key")) return NewNonFatalError(errors.New("expected exactly one stored key")) } @@ -485,7 +482,6 @@ func (c *Core) UnsealWithStoredKeys(ctx context.Context) error { // subset of the required threshold of keys. We still consider this a // "success", since trying again would yield the same result. c.Logger().Warn("vault still sealed after using stored unseal key") - diagnose.Warn(ctx, "vault still sealed after using stored unseal key") } else { c.Logger().Info("unsealed with stored key") }