Skip to content

Commit

Permalink
Audit related foibles (#24493)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Peter Wilson authored Dec 13, 2023
1 parent b468950 commit 486df81
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 21 deletions.
6 changes: 3 additions & 3 deletions builtin/audit/file/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
9 changes: 4 additions & 5 deletions builtin/audit/socket/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
}
6 changes: 3 additions & 3 deletions builtin/audit/syslog/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
7 changes: 7 additions & 0 deletions helper/builtinplugins/builtinplugins_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down
15 changes: 7 additions & 8 deletions helper/testhelpers/corehelpers/corehelpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 0 additions & 2 deletions vault/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,6 @@ type TestCluster struct {
SetupFunc func()

cleanupFuncs []func()
base *CoreConfig
LicensePublicKey ed25519.PublicKey
LicensePrivateKey ed25519.PrivateKey
opts *TestClusterOptions
Expand Down Expand Up @@ -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():
Expand Down

0 comments on commit 486df81

Please sign in to comment.