diff --git a/audit/entry_formatter.go b/audit/entry_formatter.go index 65f48fe4f8b1..81113c9a18b5 100644 --- a/audit/entry_formatter.go +++ b/audit/entry_formatter.go @@ -153,10 +153,26 @@ func (f *EntryFormatter) Process(ctx context.Context, e *eventlogger.Event) (*ev result = append([]byte(f.prefix), result...) } - // Store the final format. - e.FormattedAs(f.config.RequiredFormat.String(), result) + // Copy some properties from the event (and audit event) and store the + // format for the next (sink) node to Process. + a2 := &AuditEvent{ + ID: a.ID, + Version: a.Version, + Subtype: a.Subtype, + Timestamp: a.Timestamp, + Data: data, // Use the cloned data here rather than a pointer to the original. + } + + e2 := &eventlogger.Event{ + Type: e.Type, + CreatedAt: e.CreatedAt, + Formatted: make(map[string][]byte), // we are about to set this ourselves. + Payload: a2, + } + + e2.FormattedAs(f.config.RequiredFormat.String(), result) - return e, nil + return e2, nil } // FormatRequest attempts to format the specified logical.LogInput into a RequestEntry. diff --git a/audit/entry_formatter_test.go b/audit/entry_formatter_test.go index 56e9af4da376..6e2583da5c5c 100644 --- a/audit/entry_formatter_test.go +++ b/audit/entry_formatter_test.go @@ -306,7 +306,7 @@ func TestEntryFormatter_Process(t *testing.T) { tc := tc t.Run(name, func(t *testing.T) { t.Parallel() - e := fakeEvent(t, tc.Subtype, tc.RequiredFormat, tc.Data) + e := fakeEvent(t, tc.Subtype, tc.Data) require.NotNil(t, e) ss := newStaticSalt(t) @@ -326,18 +326,16 @@ func TestEntryFormatter_Process(t *testing.T) { } processed, err := f.Process(ctx, e) - b, found := e.Format(string(tc.RequiredFormat)) switch { case tc.IsErrorExpected: require.Error(t, err) require.EqualError(t, err, tc.ExpectedErrorMessage) require.Nil(t, processed) - require.False(t, found) - require.Nil(t, b) default: require.NoError(t, err) require.NotNil(t, processed) + b, found := processed.Format(string(tc.RequiredFormat)) require.True(t, found) require.NotNil(t, b) } @@ -390,7 +388,7 @@ func BenchmarkAuditFileSink_Process(b *testing.B) { require.NotNil(b, sink) // Generate the event - e := fakeEvent(b, RequestType, JSONFormat, in) + e := fakeEvent(b, RequestType, in) require.NotNil(b, e) b.ResetTimer() @@ -956,6 +954,63 @@ func TestEntryFormatter_FormatResponse_ElideListResponses(t *testing.T) { }) } +// TestEntryFormatter_Process_NoMutation tests that the event returned by an +// EntryFormatter.Process method is not the same as the one that it accepted. +func TestEntryFormatter_Process_NoMutation(t *testing.T) { + // Create the formatter node. + cfg, err := NewFormatterConfig() + require.NoError(t, err) + ss := newStaticSalt(t) + formatter, err := NewEntryFormatter(cfg, ss) + require.NoError(t, err) + require.NotNil(t, formatter) + + in := &logical.LogInput{ + Auth: &logical.Auth{ + ClientToken: "foo", + Accessor: "bar", + EntityID: "foobarentity", + DisplayName: "testtoken", + NoDefaultPolicy: true, + Policies: []string{"root"}, + TokenType: logical.TokenTypeService, + }, + Request: &logical.Request{ + Operation: logical.UpdateOperation, + Path: "/foo", + Connection: &logical.Connection{ + RemoteAddr: "127.0.0.1", + }, + WrapInfo: &logical.RequestWrapInfo{ + TTL: 60 * time.Second, + }, + Headers: map[string][]string{ + "foo": {"bar"}, + }, + }, + } + + e := fakeEvent(t, RequestType, in) + + e2, err := formatter.Process(namespace.RootContext(nil), e) + require.NoError(t, err) + require.NotNil(t, e2) + + // Ensure the pointers are different. + require.NotEqual(t, e2, e) + + // Do the same for the audit event in the payload. + a, ok := e.Payload.(*AuditEvent) + require.True(t, ok) + require.NotNil(t, a) + + a2, ok := e2.Payload.(*AuditEvent) + require.True(t, ok) + require.NotNil(t, a2) + + require.NotEqual(t, a2, a) +} + // hashExpectedValueForComparison replicates enough of the audit HMAC process on a piece of expected data in a test, // so that we can use assert.Equal to compare the expected and output values. func (f *EntryFormatter) hashExpectedValueForComparison(input map[string]any) map[string]any { @@ -981,7 +1036,7 @@ func (f *EntryFormatter) hashExpectedValueForComparison(input map[string]any) ma // fakeEvent will return a new fake event containing audit data based on the // specified subtype, format and logical.LogInput. -func fakeEvent(tb testing.TB, subtype subtype, format format, input *logical.LogInput) *eventlogger.Event { +func fakeEvent(tb testing.TB, subtype subtype, input *logical.LogInput) *eventlogger.Event { tb.Helper() date := time.Date(2023, time.July, 11, 15, 49, 10, 0o0, time.Local) diff --git a/audit/event.go b/audit/event.go index 6ea3f184914d..353203721481 100644 --- a/audit/event.go +++ b/audit/event.go @@ -13,7 +13,7 @@ import ( // for audit events. It will generate an ID if no ID is supplied. Supported // options: WithID, WithNow. func NewEvent(s subtype, opt ...Option) (*AuditEvent, error) { - const op = "audit.newEvent" + const op = "audit.NewEvent" // Get the default options opts, err := getOpts(opt...) diff --git a/audit/event_test.go b/audit/event_test.go index 7a520e3483d8..acd586ec94b0 100644 --- a/audit/event_test.go +++ b/audit/event_test.go @@ -29,21 +29,21 @@ func TestAuditEvent_new(t *testing.T) { Subtype: subtype(""), Format: format(""), IsErrorExpected: true, - ExpectedErrorMessage: "audit.newEvent: audit.(AuditEvent).validate: audit.(subtype).validate: '' is not a valid event subtype: invalid parameter", + ExpectedErrorMessage: "audit.NewEvent: audit.(AuditEvent).validate: audit.(subtype).validate: '' is not a valid event subtype: invalid parameter", }, "empty-Option": { Options: []Option{}, Subtype: subtype(""), Format: format(""), IsErrorExpected: true, - ExpectedErrorMessage: "audit.newEvent: audit.(AuditEvent).validate: audit.(subtype).validate: '' is not a valid event subtype: invalid parameter", + ExpectedErrorMessage: "audit.NewEvent: audit.(AuditEvent).validate: audit.(subtype).validate: '' is not a valid event subtype: invalid parameter", }, "bad-id": { Options: []Option{WithID("")}, Subtype: ResponseType, Format: JSONFormat, IsErrorExpected: true, - ExpectedErrorMessage: "audit.newEvent: error applying options: id cannot be empty", + ExpectedErrorMessage: "audit.NewEvent: error applying options: id cannot be empty", }, "good": { Options: []Option{ diff --git a/builtin/audit/file/backend.go b/builtin/audit/file/backend.go index f1d04bfa5362..b9f5288bb816 100644 --- a/builtin/audit/file/backend.go +++ b/builtin/audit/file/backend.go @@ -36,7 +36,6 @@ type Backend struct { name string nodeIDList []eventlogger.NodeID nodeMap map[eventlogger.NodeID]eventlogger.Node - filePath string salt *atomic.Value saltConfig *salt.Config saltMutex sync.RWMutex @@ -89,7 +88,6 @@ func Factory(_ context.Context, conf *audit.BackendConfig, headersConfig audit.H b := &Backend{ fallback: fallback, - filePath: filePath, name: conf.MountPath, saltConfig: conf.SaltConfig, saltView: conf.SaltView, diff --git a/builtin/audit/socket/backend.go b/builtin/audit/socket/backend.go index c35f512e04b8..6e572e408be7 100644 --- a/builtin/audit/socket/backend.go +++ b/builtin/audit/socket/backend.go @@ -6,11 +6,9 @@ package socket import ( "context" "fmt" - "net" "strconv" "strings" "sync" - "time" "github.com/hashicorp/eventlogger" "github.com/hashicorp/go-secure-stdlib/parseutil" @@ -24,19 +22,14 @@ var _ audit.Backend = (*Backend)(nil) // Backend is the audit backend for the socket audit transport. type Backend struct { - sync.Mutex - address string - connection net.Conn - fallback bool - name string - nodeIDList []eventlogger.NodeID - nodeMap map[eventlogger.NodeID]eventlogger.Node - salt *salt.Salt - saltConfig *salt.Config - saltMutex sync.RWMutex - saltView logical.Storage - socketType string - writeDuration time.Duration + fallback bool + name string + nodeIDList []eventlogger.NodeID + nodeMap map[eventlogger.NodeID]eventlogger.Node + salt *salt.Salt + saltConfig *salt.Config + saltMutex sync.RWMutex + saltView logical.Storage } func Factory(_ context.Context, conf *audit.BackendConfig, headersConfig audit.HeaderFormatter) (audit.Backend, error) { @@ -65,14 +58,10 @@ func Factory(_ context.Context, conf *audit.BackendConfig, headersConfig audit.H writeDeadline = "2s" } - writeDuration, err := parseutil.ParseDurationSecond(writeDeadline) - if err != nil { - return nil, fmt.Errorf("%s: failed to parse 'write_timeout': %w", op, err) - } - // The config options 'fallback' and 'filter' are mutually exclusive, a fallback // device catches everything, so it cannot be allowed to filter. var fallback bool + var err error if fallbackRaw, ok := conf.Config["fallback"]; ok { fallback, err = parseutil.ParseBool(fallbackRaw) if err != nil { @@ -85,15 +74,12 @@ func Factory(_ context.Context, conf *audit.BackendConfig, headersConfig audit.H } b := &Backend{ - fallback: fallback, - address: address, - name: conf.MountPath, - saltConfig: conf.SaltConfig, - saltView: conf.SaltView, - socketType: socketType, - writeDuration: writeDuration, - nodeIDList: []eventlogger.NodeID{}, - nodeMap: make(map[eventlogger.NodeID]eventlogger.Node), + fallback: fallback, + name: conf.MountPath, + saltConfig: conf.SaltConfig, + saltView: conf.SaltView, + nodeIDList: []eventlogger.NodeID{}, + nodeMap: make(map[eventlogger.NodeID]eventlogger.Node), } err = b.configureFilterNode(conf.Config["filter"]) diff --git a/builtin/audit/socket/backend_test.go b/builtin/audit/socket/backend_test.go index c118df6093bd..85c339dc846b 100644 --- a/builtin/audit/socket/backend_test.go +++ b/builtin/audit/socket/backend_test.go @@ -417,7 +417,7 @@ func TestBackend_Factory_Conf(t *testing.T) { }, }, isErrorExpected: true, - expectedErrorMessage: "socket.Factory: failed to parse 'write_timeout': time: invalid duration \"qwerty\"", + expectedErrorMessage: "socket.Factory: error configuring sink node: socket.(Backend).configureSinkNode: error creating socket sink node: event.NewSocketSink: error applying options: unable to parse max duration: time: invalid duration \"qwerty\"", }, "non-fallback-device-with-filter": { backendConfig: &audit.BackendConfig{ diff --git a/changelog/24968.txt b/changelog/24968.txt new file mode 100644 index 000000000000..47c71eaa82e5 --- /dev/null +++ b/changelog/24968.txt @@ -0,0 +1,3 @@ +```release-note:bug +audit: Fix bug where use of 'log_raw' option could result in other devices logging raw audit data +``` diff --git a/internal/observability/event/options.go b/internal/observability/event/options.go index 30d667740b0d..e788cecb6de3 100644 --- a/internal/observability/event/options.go +++ b/internal/observability/event/options.go @@ -12,7 +12,6 @@ import ( "time" "github.com/hashicorp/go-secure-stdlib/parseutil" - "github.com/hashicorp/go-uuid" ) @@ -160,7 +159,7 @@ func WithMaxDuration(duration string) Option { parsed, err := parseutil.ParseDurationSecond(duration) if err != nil { - return err + return fmt.Errorf("unable to parse max duration: %w", err) } o.withMaxDuration = parsed diff --git a/internal/observability/event/options_test.go b/internal/observability/event/options_test.go index 0f36014740cf..95f1193f1b7e 100644 --- a/internal/observability/event/options_test.go +++ b/internal/observability/event/options_test.go @@ -324,12 +324,12 @@ func TestOptions_WithMaxDuration(t *testing.T) { "bad-value": { Value: "juan", IsErrorExpected: true, - ExpectedErrorMessage: "time: invalid duration \"juan\"", + ExpectedErrorMessage: "unable to parse max duration: time: invalid duration \"juan\"", }, "bad-spacey-value": { Value: " juan ", IsErrorExpected: true, - ExpectedErrorMessage: "time: invalid duration \"juan\"", + ExpectedErrorMessage: "unable to parse max duration: time: invalid duration \"juan\"", }, "duration-2s": { Value: "2s", diff --git a/internal/observability/event/sink_socket.go b/internal/observability/event/sink_socket.go index e9cb00c19662..861da0a20121 100644 --- a/internal/observability/event/sink_socket.go +++ b/internal/observability/event/sink_socket.go @@ -11,9 +11,8 @@ import ( "sync" "time" - "github.com/hashicorp/go-multierror" - "github.com/hashicorp/eventlogger" + "github.com/hashicorp/go-multierror" ) var _ eventlogger.Node = (*SocketSink)(nil) diff --git a/internal/observability/event/sink_socket_test.go b/internal/observability/event/sink_socket_test.go index 3c647f7b3ea1..c44766780dc3 100644 --- a/internal/observability/event/sink_socket_test.go +++ b/internal/observability/event/sink_socket_test.go @@ -50,7 +50,7 @@ func TestNewSocketSink(t *testing.T) { format: "json", opts: []Option{WithMaxDuration("bar")}, wantErr: true, - expectedErrMsg: "event.NewSocketSink: error applying options: time: invalid duration \"bar\"", + expectedErrMsg: "event.NewSocketSink: error applying options: unable to parse max duration: time: invalid duration \"bar\"", }, "happy": { address: "wss://foo", diff --git a/internal/observability/event/sink_stdout.go b/internal/observability/event/sink_stdout.go index 6b1f43dace8f..71782695c7a2 100644 --- a/internal/observability/event/sink_stdout.go +++ b/internal/observability/event/sink_stdout.go @@ -36,7 +36,7 @@ func NewStdoutSinkNode(format string) (*StdoutSink, error) { } // Process persists the provided eventlogger.Event to the standard output stream. -func (s *StdoutSink) Process(ctx context.Context, event *eventlogger.Event) (*eventlogger.Event, error) { +func (s *StdoutSink) Process(ctx context.Context, e *eventlogger.Event) (*eventlogger.Event, error) { const op = "event.(StdoutSink).Process" select { @@ -45,11 +45,11 @@ func (s *StdoutSink) Process(ctx context.Context, event *eventlogger.Event) (*ev default: } - if event == nil { + if e == nil { return nil, fmt.Errorf("%s: event is nil: %w", op, ErrInvalidParameter) } - formattedBytes, found := event.Format(s.requiredFormat) + formattedBytes, found := e.Format(s.requiredFormat) if !found { return nil, fmt.Errorf("%s: unable to retrieve event formatted as %q", op, s.requiredFormat) } diff --git a/internal/observability/event/sink_syslog.go b/internal/observability/event/sink_syslog.go index d099ed5c7349..7d0e359ffcef 100644 --- a/internal/observability/event/sink_syslog.go +++ b/internal/observability/event/sink_syslog.go @@ -8,9 +8,8 @@ import ( "fmt" "strings" - gsyslog "github.com/hashicorp/go-syslog" - "github.com/hashicorp/eventlogger" + gsyslog "github.com/hashicorp/go-syslog" ) var _ eventlogger.Node = (*SyslogSink)(nil)