From 486df81934bc7b6147d47864815f66ebd2ffb538 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Wed, 13 Dec 2023 09:08:02 +0000 Subject: [PATCH] Audit related foibles (#24493) * update node and pipeline registration to prevent overwriting, strip some unused bits of NewTestCluster, tweak to prevent auditing on a test that is flaking * tidy imports --- builtin/audit/file/backend.go | 6 +++--- builtin/audit/socket/backend.go | 9 ++++----- builtin/audit/syslog/backend.go | 6 +++--- helper/builtinplugins/builtinplugins_test.go | 7 +++++++ helper/testhelpers/corehelpers/corehelpers.go | 15 +++++++-------- vault/testing.go | 2 -- 6 files changed, 24 insertions(+), 21 deletions(-) diff --git a/builtin/audit/file/backend.go b/builtin/audit/file/backend.go index 4cc82c15ce85..fc6a44a58719 100644 --- a/builtin/audit/file/backend.go +++ b/builtin/audit/file/backend.go @@ -436,16 +436,16 @@ func (b *Backend) Invalidate(_ context.Context) { // the audit.Backend interface. func (b *Backend) RegisterNodesAndPipeline(broker *eventlogger.Broker, name string) error { for id, node := range b.nodeMap { - if err := broker.RegisterNode(id, node); err != nil { + if err := broker.RegisterNode(id, node, eventlogger.WithNodeRegistrationPolicy(eventlogger.DenyOverwrite)); err != nil { return err } } pipeline := eventlogger.Pipeline{ PipelineID: eventlogger.PipelineID(name), - EventType: eventlogger.EventType("audit"), + EventType: eventlogger.EventType(event.AuditType.String()), NodeIDs: b.nodeIDList, } - return broker.RegisterPipeline(pipeline) + return broker.RegisterPipeline(pipeline, eventlogger.WithPipelineRegistrationPolicy(eventlogger.DenyOverwrite)) } diff --git a/builtin/audit/socket/backend.go b/builtin/audit/socket/backend.go index 85370a3506f4..1e906468c7f8 100644 --- a/builtin/audit/socket/backend.go +++ b/builtin/audit/socket/backend.go @@ -12,10 +12,9 @@ import ( "sync" "time" - "github.com/hashicorp/go-secure-stdlib/parseutil" - "github.com/hashicorp/eventlogger" "github.com/hashicorp/go-multierror" + "github.com/hashicorp/go-secure-stdlib/parseutil" "github.com/hashicorp/vault/audit" "github.com/hashicorp/vault/internal/observability/event" "github.com/hashicorp/vault/sdk/helper/salt" @@ -336,16 +335,16 @@ func (b *Backend) Invalidate(_ context.Context) { // the audit.Backend interface. func (b *Backend) RegisterNodesAndPipeline(broker *eventlogger.Broker, name string) error { for id, node := range b.nodeMap { - if err := broker.RegisterNode(id, node); err != nil { + if err := broker.RegisterNode(id, node, eventlogger.WithNodeRegistrationPolicy(eventlogger.DenyOverwrite)); err != nil { return err } } pipeline := eventlogger.Pipeline{ PipelineID: eventlogger.PipelineID(name), - EventType: eventlogger.EventType("audit"), + EventType: eventlogger.EventType(event.AuditType.String()), NodeIDs: b.nodeIDList, } - return broker.RegisterPipeline(pipeline) + return broker.RegisterPipeline(pipeline, eventlogger.WithPipelineRegistrationPolicy(eventlogger.DenyOverwrite)) } diff --git a/builtin/audit/syslog/backend.go b/builtin/audit/syslog/backend.go index f1b7f8179045..9dc0298f64f6 100644 --- a/builtin/audit/syslog/backend.go +++ b/builtin/audit/syslog/backend.go @@ -245,16 +245,16 @@ func (b *Backend) Invalidate(_ context.Context) { // the audit.Backend interface. func (b *Backend) RegisterNodesAndPipeline(broker *eventlogger.Broker, name string) error { for id, node := range b.nodeMap { - if err := broker.RegisterNode(id, node); err != nil { + if err := broker.RegisterNode(id, node, eventlogger.WithNodeRegistrationPolicy(eventlogger.DenyOverwrite)); err != nil { return err } } pipeline := eventlogger.Pipeline{ PipelineID: eventlogger.PipelineID(name), - EventType: eventlogger.EventType("audit"), + EventType: eventlogger.EventType(event.AuditType.String()), NodeIDs: b.nodeIDList, } - return broker.RegisterPipeline(pipeline) + return broker.RegisterPipeline(pipeline, eventlogger.WithPipelineRegistrationPolicy(eventlogger.DenyOverwrite)) } diff --git a/helper/builtinplugins/builtinplugins_test.go b/helper/builtinplugins/builtinplugins_test.go index 9587960436ed..8df20209c1d7 100644 --- a/helper/builtinplugins/builtinplugins_test.go +++ b/helper/builtinplugins/builtinplugins_test.go @@ -8,7 +8,9 @@ import ( logicalKv "github.com/hashicorp/vault-plugin-secrets-kv" "github.com/hashicorp/vault/api" + "github.com/hashicorp/vault/audit" logicalDb "github.com/hashicorp/vault/builtin/logical/database" + "github.com/hashicorp/vault/helper/testhelpers/corehelpers" vaulthttp "github.com/hashicorp/vault/http" "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/sdk/logical" @@ -43,6 +45,11 @@ func TestBuiltinPluginsWork(t *testing.T) { "database": logicalDb.Factory, }, PendingRemovalMountsAllowed: true, + // Specifying at least one audit backend factory will prevent NewTestCluster + // from attempting to enable a noop audit, and audit isn't required for this test. + AuditBackends: map[string]audit.Factory{ + "noop": corehelpers.NoopAuditFactory(nil), + }, }, &vault.TestClusterOptions{ HandlerFunc: vaulthttp.Handler, diff --git a/helper/testhelpers/corehelpers/corehelpers.go b/helper/testhelpers/corehelpers/corehelpers.go index 582a4d9ef660..8e0e449ad5d8 100644 --- a/helper/testhelpers/corehelpers/corehelpers.go +++ b/helper/testhelpers/corehelpers/corehelpers.go @@ -16,12 +16,11 @@ import ( "sync" "time" - "github.com/hashicorp/vault/internal/observability/event" - "github.com/hashicorp/eventlogger" "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/audit" "github.com/hashicorp/vault/builtin/credential/approle" + "github.com/hashicorp/vault/internal/observability/event" "github.com/hashicorp/vault/plugins/database/mysql" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/helper/consts" @@ -429,20 +428,20 @@ func (n *NoopAudit) Invalidate(_ context.Context) { // RegisterNodesAndPipeline registers the nodes and a pipeline as required by // the audit.Backend interface. -func (b *NoopAudit) RegisterNodesAndPipeline(broker *eventlogger.Broker, name string) error { - for id, node := range b.nodeMap { - if err := broker.RegisterNode(id, node); err != nil { +func (n *NoopAudit) RegisterNodesAndPipeline(broker *eventlogger.Broker, name string) error { + for id, node := range n.nodeMap { + if err := broker.RegisterNode(id, node, eventlogger.WithNodeRegistrationPolicy(eventlogger.DenyOverwrite)); err != nil { return err } } pipeline := eventlogger.Pipeline{ PipelineID: eventlogger.PipelineID(name), - EventType: eventlogger.EventType("audit"), - NodeIDs: b.nodeIDList, + EventType: eventlogger.EventType(event.AuditType.String()), + NodeIDs: n.nodeIDList, } - return broker.RegisterPipeline(pipeline) + return broker.RegisterPipeline(pipeline, eventlogger.WithPipelineRegistrationPolicy(eventlogger.DenyOverwrite)) } type TestLogger struct { diff --git a/vault/testing.go b/vault/testing.go index caf83a98ab1f..14f000a2c520 100644 --- a/vault/testing.go +++ b/vault/testing.go @@ -709,7 +709,6 @@ type TestCluster struct { SetupFunc func() cleanupFuncs []func() - base *CoreConfig LicensePublicKey ed25519.PublicKey LicensePrivateKey ed25519.PrivateKey opts *TestClusterOptions @@ -1189,7 +1188,6 @@ func NewTestCluster(t testing.T, base *CoreConfig, opts *TestClusterOptions) *Te baseAddr, certIPs := GenerateListenerAddr(t, opts, certIPs) var testCluster TestCluster - testCluster.base = base switch { case opts != nil && opts.Logger != nil && !reflect.ValueOf(opts.Logger).IsNil():