diff --git a/audit/entry_filter.go b/audit/entry_filter.go index 4b726153563a..ca08ee48a2a7 100644 --- a/audit/entry_filter.go +++ b/audit/entry_filter.go @@ -11,7 +11,6 @@ import ( "github.com/hashicorp/eventlogger" "github.com/hashicorp/go-bexpr" "github.com/hashicorp/vault/helper/namespace" - "github.com/hashicorp/vault/internal/observability/event" "github.com/hashicorp/vault/sdk/logical" ) @@ -27,16 +26,14 @@ type EntryFilter struct { // NewEntryFilter should be used to create an EntryFilter node. // The filter supplied should be in bexpr format and reference fields from logical.LogInputBexpr. func NewEntryFilter(filter string) (*EntryFilter, error) { - const op = "audit.NewEntryFilter" - filter = strings.TrimSpace(filter) if filter == "" { - return nil, fmt.Errorf("%s: cannot create new audit filter with empty filter expression: %w", op, event.ErrInvalidParameter) + return nil, fmt.Errorf("cannot create new audit filter with empty filter expression: %w", ErrExternalOptions) } eval, err := bexpr.CreateEvaluator(filter) if err != nil { - return nil, fmt.Errorf("%s: cannot create new audit filter: %w", op, err) + return nil, fmt.Errorf("cannot create new audit filter: %w: %w", ErrExternalOptions, err) } // Validate the filter by attempting to evaluate it with an empty input. @@ -45,7 +42,7 @@ func NewEntryFilter(filter string) (*EntryFilter, error) { li := logical.LogInputBexpr{} _, err = eval.Evaluate(li) if err != nil { - return nil, fmt.Errorf("%s: filter references an unsupported field: %s", op, filter) + return nil, fmt.Errorf("filter references an unsupported field: %s: %w: %w", filter, ErrExternalOptions, err) } return &EntryFilter{evaluator: eval}, nil @@ -64,8 +61,6 @@ func (*EntryFilter) Type() eventlogger.NodeType { // Process will attempt to parse the incoming event data and decide whether it // should be filtered or remain in the pipeline and passed to the next node. func (f *EntryFilter) Process(ctx context.Context, e *eventlogger.Event) (*eventlogger.Event, error) { - const op = "audit.(EntryFilter).Process" - select { case <-ctx.Done(): return nil, ctx.Err() @@ -73,12 +68,12 @@ func (f *EntryFilter) Process(ctx context.Context, e *eventlogger.Event) (*event } if e == nil { - return nil, fmt.Errorf("%s: event is nil: %w", op, event.ErrInvalidParameter) + return nil, fmt.Errorf("event is nil: %w", ErrInvalidParameter) } a, ok := e.Payload.(*AuditEvent) if !ok { - return nil, fmt.Errorf("%s: cannot parse event payload: %w", op, event.ErrInvalidParameter) + return nil, fmt.Errorf("cannot parse event payload: %w", ErrInvalidParameter) } // If we don't have data to process, then we're done. @@ -88,14 +83,14 @@ func (f *EntryFilter) Process(ctx context.Context, e *eventlogger.Event) (*event ns, err := namespace.FromContext(ctx) if err != nil { - return nil, fmt.Errorf("%s: cannot obtain namespace: %w", op, err) + return nil, fmt.Errorf("cannot obtain namespace: %w", err) } datum := a.Data.BexprDatum(ns.Path) result, err := f.evaluator.Evaluate(datum) if err != nil { - return nil, fmt.Errorf("%s: unable to evaluate filter: %w", op, err) + return nil, fmt.Errorf("unable to evaluate filter: %w", err) } if result { diff --git a/audit/entry_filter_test.go b/audit/entry_filter_test.go index 569e37ee7c38..99230d055959 100644 --- a/audit/entry_filter_test.go +++ b/audit/entry_filter_test.go @@ -27,22 +27,22 @@ func TestEntryFilter_NewEntryFilter(t *testing.T) { "empty-filter": { Filter: "", IsErrorExpected: true, - ExpectedErrorMessage: "audit.NewEntryFilter: cannot create new audit filter with empty filter expression: invalid parameter", + ExpectedErrorMessage: "cannot create new audit filter with empty filter expression: invalid configuration", }, "spacey-filter": { Filter: " ", IsErrorExpected: true, - ExpectedErrorMessage: "audit.NewEntryFilter: cannot create new audit filter with empty filter expression: invalid parameter", + ExpectedErrorMessage: "cannot create new audit filter with empty filter expression: invalid configuration", }, "bad-filter": { Filter: "____", IsErrorExpected: true, - ExpectedErrorMessage: "audit.NewEntryFilter: cannot create new audit filter", + ExpectedErrorMessage: "cannot create new audit filter", }, "unsupported-field-filter": { Filter: "foo == bar", IsErrorExpected: true, - ExpectedErrorMessage: "audit.NewEntryFilter: filter references an unsupported field: foo == bar", + ExpectedErrorMessage: "filter references an unsupported field: foo == bar", }, "good-filter-operation": { Filter: "operation == create", @@ -122,7 +122,7 @@ func TestEntryFilter_Process_ContextDone(t *testing.T) { // Fake event logger event e := &eventlogger.Event{ - Type: eventlogger.EventType(event.AuditType.String()), + Type: event.AuditType.AsEventType(), CreatedAt: time.Now(), Formatted: make(map[string][]byte), Payload: a, @@ -146,7 +146,7 @@ func TestEntryFilter_Process_NilEvent(t *testing.T) { require.NoError(t, err) e, err := l.Process(context.Background(), nil) require.Error(t, err) - require.EqualError(t, err, "audit.(EntryFilter).Process: event is nil: invalid parameter") + require.EqualError(t, err, "event is nil: invalid internal parameter") // Ensure that the pipeline won't continue. require.Nil(t, e) @@ -162,7 +162,7 @@ func TestEntryFilter_Process_BadPayload(t *testing.T) { require.NoError(t, err) e := &eventlogger.Event{ - Type: eventlogger.EventType(event.AuditType.String()), + Type: event.AuditType.AsEventType(), CreatedAt: time.Now(), Formatted: make(map[string][]byte), Payload: nil, @@ -170,7 +170,7 @@ func TestEntryFilter_Process_BadPayload(t *testing.T) { e2, err := l.Process(context.Background(), e) require.Error(t, err) - require.EqualError(t, err, "audit.(EntryFilter).Process: cannot parse event payload: invalid parameter") + require.EqualError(t, err, "cannot parse event payload: invalid internal parameter") // Ensure that the pipeline won't continue. require.Nil(t, e2) @@ -191,7 +191,7 @@ func TestEntryFilter_Process_NoAuditDataInPayload(t *testing.T) { a.Data = nil e := &eventlogger.Event{ - Type: eventlogger.EventType(event.AuditType.String()), + Type: event.AuditType.AsEventType(), CreatedAt: time.Now(), Formatted: make(map[string][]byte), Payload: a, @@ -223,7 +223,7 @@ func TestEntryFilter_Process_FilterSuccess(t *testing.T) { } e := &eventlogger.Event{ - Type: eventlogger.EventType(event.AuditType.String()), + Type: event.AuditType.AsEventType(), CreatedAt: time.Now(), Formatted: make(map[string][]byte), Payload: a, @@ -256,7 +256,7 @@ func TestEntryFilter_Process_FilterFail(t *testing.T) { } e := &eventlogger.Event{ - Type: eventlogger.EventType(event.AuditType.String()), + Type: event.AuditType.AsEventType(), CreatedAt: time.Now(), Formatted: make(map[string][]byte), Payload: a, diff --git a/audit/entry_formatter.go b/audit/entry_formatter.go index 289fde83fa7d..1d3ccc59c2d4 100644 --- a/audit/entry_formatter.go +++ b/audit/entry_formatter.go @@ -18,7 +18,6 @@ import ( "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-multierror" "github.com/hashicorp/vault/helper/namespace" - "github.com/hashicorp/vault/internal/observability/event" "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/helper/salt" "github.com/hashicorp/vault/sdk/logical" @@ -36,7 +35,49 @@ type timeProvider interface { formattedTime() string } +// FormatterConfig is used to provide basic configuration to a formatter. +// Use NewFormatterConfig to initialize the FormatterConfig struct. +type FormatterConfig struct { + Raw bool + HMACAccessor bool + + // Vault lacks pagination in its APIs. As a result, certain list operations can return **very** large responses. + // The user's chosen audit sinks may experience difficulty consuming audit records that swell to tens of megabytes + // of JSON. The responses of list operations are typically not very interesting, as they are mostly lists of keys, + // or, even when they include a "key_info" field, are not returning confidential information. They become even less + // interesting once HMAC-ed by the audit system. + // + // Some example Vault "list" operations that are prone to becoming very large in an active Vault installation are: + // auth/token/accessors/ + // identity/entity/id/ + // identity/entity-alias/id/ + // pki/certs/ + // + // This option exists to provide such users with the option to have response data elided from audit logs, only when + // the operation type is "list". For added safety, the elision only applies to the "keys" and "key_info" fields + // within the response data - these are conventionally the only fields present in a list response - see + // logical.ListResponse, and logical.ListResponseWithInfo. However, other fields are technically possible if a + // plugin author writes unusual code, and these will be preserved in the audit log even with this option enabled. + // The elision replaces the values of the "keys" and "key_info" fields with an integer count of the number of + // entries. This allows even the elided audit logs to still be useful for answering questions like + // "Was any data returned?" or "How many records were listed?". + ElideListResponses bool + + // This should only ever be used in a testing context + OmitTime bool + + // The required/target format for the event (supported: JSONFormat and JSONxFormat). + RequiredFormat format + + // headerFormatter specifies the formatter used for headers that existing in any incoming audit request. + headerFormatter HeaderFormatter + + // Prefix specifies a Prefix that should be prepended to any formatted request or response before serialization. + Prefix string +} + // EntryFormatter should be used to format audit requests and responses. +// NOTE: Use NewEntryFormatter to initialize the EntryFormatter struct. type EntryFormatter struct { config FormatterConfig salter Salter @@ -44,26 +85,42 @@ type EntryFormatter struct { name string } +// NewFormatterConfig should be used to create a FormatterConfig. +// Accepted options: WithElision, WithFormat, WithHMACAccessor, WithOmitTime, WithPrefix, WithRaw. +func NewFormatterConfig(headerFormatter HeaderFormatter, opt ...Option) (FormatterConfig, error) { + if headerFormatter == nil || reflect.ValueOf(headerFormatter).IsNil() { + return FormatterConfig{}, fmt.Errorf("header formatter is required: %w", ErrInvalidParameter) + } + + opts, err := getOpts(opt...) + if err != nil { + return FormatterConfig{}, err + } + + return FormatterConfig{ + headerFormatter: headerFormatter, + ElideListResponses: opts.withElision, + HMACAccessor: opts.withHMACAccessor, + OmitTime: opts.withOmitTime, + Prefix: opts.withPrefix, + Raw: opts.withRaw, + RequiredFormat: opts.withFormat, + }, nil +} + // NewEntryFormatter should be used to create an EntryFormatter. func NewEntryFormatter(name string, config FormatterConfig, salter Salter, logger hclog.Logger) (*EntryFormatter, error) { - const op = "audit.NewEntryFormatter" - name = strings.TrimSpace(name) if name == "" { - return nil, fmt.Errorf("%s: name is required: %w", op, event.ErrInvalidParameter) + return nil, fmt.Errorf("name is required: %w", ErrInvalidParameter) } if salter == nil { - return nil, fmt.Errorf("%s: cannot create a new audit formatter with nil salter: %w", op, event.ErrInvalidParameter) + return nil, fmt.Errorf("cannot create a new audit formatter with nil salter: %w", ErrInvalidParameter) } if logger == nil || reflect.ValueOf(logger).IsNil() { - return nil, fmt.Errorf("%s: cannot create a new audit formatter with nil logger: %w", op, event.ErrInvalidParameter) - } - - // We need to ensure that the format isn't just some default empty string. - if err := config.RequiredFormat.validate(); err != nil { - return nil, fmt.Errorf("%s: format not valid: %w", op, err) + return nil, fmt.Errorf("cannot create a new audit formatter with nil logger: %w", ErrInvalidParameter) } return &EntryFormatter{ @@ -87,8 +144,6 @@ func (*EntryFormatter) Type() eventlogger.NodeType { // Process will attempt to parse the incoming event data into a corresponding // audit Request/Response which is serialized to JSON/JSONx and stored within the event. func (f *EntryFormatter) Process(ctx context.Context, e *eventlogger.Event) (_ *eventlogger.Event, retErr error) { - const op = "audit.(EntryFormatter).Process" - // Return early if the context was cancelled, eventlogger will not carry on // asking nodes to process, so any sink node in the pipeline won't be called. select { @@ -100,16 +155,16 @@ func (f *EntryFormatter) Process(ctx context.Context, e *eventlogger.Event) (_ * // Perform validation on the event, then retrieve the underlying AuditEvent // and LogInput (from the AuditEvent Data). if e == nil { - return nil, fmt.Errorf("%s: event is nil: %w", op, event.ErrInvalidParameter) + return nil, fmt.Errorf("event is nil: %w", ErrInvalidParameter) } a, ok := e.Payload.(*AuditEvent) if !ok { - return nil, fmt.Errorf("%s: cannot parse event payload: %w", op, event.ErrInvalidParameter) + return nil, fmt.Errorf("cannot parse event payload: %w", ErrInvalidParameter) } if a.Data == nil { - return nil, fmt.Errorf("%s: cannot audit event (%s) with no data: %w", op, a.Subtype, event.ErrInvalidParameter) + return nil, fmt.Errorf("cannot audit event (%s) with no data: %w", a.Subtype, ErrInvalidParameter) } // Handle panics @@ -126,13 +181,13 @@ func (f *EntryFormatter) Process(ctx context.Context, e *eventlogger.Event) (_ * "stacktrace", string(debug.Stack())) // Ensure that we add this error onto any pre-existing error that was being returned. - retErr = multierror.Append(retErr, fmt.Errorf("%s: panic generating audit log: %q", op, f.name)).ErrorOrNil() + retErr = multierror.Append(retErr, fmt.Errorf("panic generating audit log: %q", f.name)).ErrorOrNil() }() // Take a copy of the event data before we modify anything. data, err := a.Data.Clone() if err != nil { - return nil, fmt.Errorf("%s: unable to copy audit event data: %w", op, err) + return nil, fmt.Errorf("unable to clone audit event data: %w", err) } // If the request is present in the input data, apply header configuration @@ -144,7 +199,7 @@ func (f *EntryFormatter) Process(ctx context.Context, e *eventlogger.Event) (_ * // e.g. via: /sys/config/auditing/request-headers/:name data.Request.Headers, err = f.config.headerFormatter.ApplyConfig(ctx, data.Request.Headers, f.salter) if err != nil { - return nil, fmt.Errorf("%s: unable to transform headers for auditing: %w", op, err) + return nil, fmt.Errorf("unable to transform headers for auditing: %w", err) } } @@ -165,25 +220,25 @@ func (f *EntryFormatter) Process(ctx context.Context, e *eventlogger.Event) (_ * case ResponseType: entry, err = f.FormatResponse(ctx, data, a) default: - return nil, fmt.Errorf("%s: unknown audit event subtype: %q", op, a.Subtype) + return nil, fmt.Errorf("unknown audit event subtype: %q", a.Subtype) } if err != nil { - return nil, fmt.Errorf("%s: unable to parse %s from audit event: %w", op, a.Subtype.String(), err) + return nil, fmt.Errorf("unable to parse %s from audit event: %w", a.Subtype, err) } result, err := jsonutil.EncodeJSON(entry) if err != nil { - return nil, fmt.Errorf("%s: unable to format %s: %w", op, a.Subtype.String(), err) + return nil, fmt.Errorf("unable to format %s: %w", a.Subtype, err) } if f.config.RequiredFormat == JSONxFormat { var err error result, err = jsonx.EncodeJSONBytes(result) if err != nil { - return nil, fmt.Errorf("%s: unable to encode JSONx using JSON data: %w", op, err) + return nil, fmt.Errorf("unable to encode JSONx using JSON data: %w", err) } if result == nil { - return nil, fmt.Errorf("%s: encoded JSONx was nil: %w", op, err) + return nil, fmt.Errorf("encoded JSONx was nil: %w", err) } } @@ -569,31 +624,6 @@ func (f *EntryFormatter) FormatResponse(ctx context.Context, in *logical.LogInpu return respEntry, nil } -// NewFormatterConfig should be used to create a FormatterConfig. -// Accepted options: WithElision, WithFormat, WithHMACAccessor, WithOmitTime, WithPrefix, WithRaw. -func NewFormatterConfig(headerFormatter HeaderFormatter, opt ...Option) (FormatterConfig, error) { - const op = "audit.NewFormatterConfig" - - if headerFormatter == nil || reflect.ValueOf(headerFormatter).IsNil() { - return FormatterConfig{}, fmt.Errorf("%s: header formatter is required: %w", op, event.ErrInvalidParameter) - } - - opts, err := getOpts(opt...) - if err != nil { - return FormatterConfig{}, fmt.Errorf("%s: error applying options: %w", op, err) - } - - return FormatterConfig{ - headerFormatter: headerFormatter, - ElideListResponses: opts.withElision, - HMACAccessor: opts.withHMACAccessor, - OmitTime: opts.withOmitTime, - Prefix: opts.withPrefix, - Raw: opts.withRaw, - RequiredFormat: opts.withFormat, - }, nil -} - // getRemoteAddr safely gets the remote address avoiding a nil pointer func getRemoteAddr(req *logical.Request) string { if req != nil && req.Connection != nil { diff --git a/audit/entry_formatter_test.go b/audit/entry_formatter_test.go index b4871a903f1a..349a3b1911e8 100644 --- a/audit/entry_formatter_test.go +++ b/audit/entry_formatter_test.go @@ -103,25 +103,25 @@ func TestNewEntryFormatter(t *testing.T) { "empty-name": { Name: "", IsErrorExpected: true, - ExpectedErrorMessage: "audit.NewEntryFormatter: name is required: invalid parameter", + ExpectedErrorMessage: "name is required: invalid internal parameter", }, "spacey-name": { Name: " ", IsErrorExpected: true, - ExpectedErrorMessage: "audit.NewEntryFormatter: name is required: invalid parameter", + ExpectedErrorMessage: "name is required: invalid internal parameter", }, "nil-salter": { Name: "juan", UseStaticSalt: false, IsErrorExpected: true, - ExpectedErrorMessage: "audit.NewEntryFormatter: cannot create a new audit formatter with nil salter: invalid parameter", + ExpectedErrorMessage: "cannot create a new audit formatter with nil salter: invalid internal parameter", }, "nil-logger": { Name: "juan", UseStaticSalt: true, Logger: nil, IsErrorExpected: true, - ExpectedErrorMessage: "audit.NewEntryFormatter: cannot create a new audit formatter with nil logger: invalid parameter", + ExpectedErrorMessage: "cannot create a new audit formatter with nil logger: invalid internal parameter", }, "static-salter": { Name: "juan", @@ -258,42 +258,42 @@ func TestEntryFormatter_Process(t *testing.T) { }{ "json-request-no-data": { IsErrorExpected: true, - ExpectedErrorMessage: "audit.(EntryFormatter).Process: cannot audit event (request) with no data: invalid parameter", + ExpectedErrorMessage: "cannot audit event (request) with no data: invalid internal parameter", Subtype: RequestType, RequiredFormat: JSONFormat, Data: nil, }, "json-response-no-data": { IsErrorExpected: true, - ExpectedErrorMessage: "audit.(EntryFormatter).Process: cannot audit event (response) with no data: invalid parameter", + ExpectedErrorMessage: "cannot audit event (response) with no data: invalid internal parameter", Subtype: ResponseType, RequiredFormat: JSONFormat, Data: nil, }, "json-request-basic-input": { IsErrorExpected: true, - ExpectedErrorMessage: "audit.(EntryFormatter).Process: unable to parse request from audit event: request to request-audit a nil request", + ExpectedErrorMessage: "unable to parse request from audit event: request to request-audit a nil request", Subtype: RequestType, RequiredFormat: JSONFormat, Data: &logical.LogInput{Type: "magic"}, }, "json-response-basic-input": { IsErrorExpected: true, - ExpectedErrorMessage: "audit.(EntryFormatter).Process: unable to parse response from audit event: request to response-audit a nil request", + ExpectedErrorMessage: "unable to parse response from audit event: request to response-audit a nil request", Subtype: ResponseType, RequiredFormat: JSONFormat, Data: &logical.LogInput{Type: "magic"}, }, "json-request-basic-input-and-request-no-ns": { IsErrorExpected: true, - ExpectedErrorMessage: "audit.(EntryFormatter).Process: unable to parse request from audit event: no namespace", + ExpectedErrorMessage: "unable to parse request from audit event: no namespace", Subtype: RequestType, RequiredFormat: JSONFormat, Data: &logical.LogInput{Request: &logical.Request{ID: "123"}}, }, "json-response-basic-input-and-request-no-ns": { IsErrorExpected: true, - ExpectedErrorMessage: "audit.(EntryFormatter).Process: unable to parse response from audit event: no namespace", + ExpectedErrorMessage: "unable to parse response from audit event: no namespace", Subtype: ResponseType, RequiredFormat: JSONFormat, Data: &logical.LogInput{Request: &logical.Request{ID: "123"}}, @@ -314,42 +314,42 @@ func TestEntryFormatter_Process(t *testing.T) { }, "jsonx-request-no-data": { IsErrorExpected: true, - ExpectedErrorMessage: "audit.(EntryFormatter).Process: cannot audit event (request) with no data: invalid parameter", + ExpectedErrorMessage: "cannot audit event (request) with no data: invalid internal parameter", Subtype: RequestType, RequiredFormat: JSONxFormat, Data: nil, }, "jsonx-response-no-data": { IsErrorExpected: true, - ExpectedErrorMessage: "audit.(EntryFormatter).Process: cannot audit event (response) with no data: invalid parameter", + ExpectedErrorMessage: "cannot audit event (response) with no data: invalid internal parameter", Subtype: ResponseType, RequiredFormat: JSONxFormat, Data: nil, }, "jsonx-request-basic-input": { IsErrorExpected: true, - ExpectedErrorMessage: "audit.(EntryFormatter).Process: unable to parse request from audit event: request to request-audit a nil request", + ExpectedErrorMessage: "unable to parse request from audit event: request to request-audit a nil request", Subtype: RequestType, RequiredFormat: JSONxFormat, Data: &logical.LogInput{Type: "magic"}, }, "jsonx-response-basic-input": { IsErrorExpected: true, - ExpectedErrorMessage: "audit.(EntryFormatter).Process: unable to parse response from audit event: request to response-audit a nil request", + ExpectedErrorMessage: "unable to parse response from audit event: request to response-audit a nil request", Subtype: ResponseType, RequiredFormat: JSONxFormat, Data: &logical.LogInput{Type: "magic"}, }, "jsonx-request-basic-input-and-request-no-ns": { IsErrorExpected: true, - ExpectedErrorMessage: "audit.(EntryFormatter).Process: unable to parse request from audit event: no namespace", + ExpectedErrorMessage: "unable to parse request from audit event: no namespace", Subtype: RequestType, RequiredFormat: JSONxFormat, Data: &logical.LogInput{Request: &logical.Request{ID: "123"}}, }, "jsonx-response-basic-input-and-request-no-ns": { IsErrorExpected: true, - ExpectedErrorMessage: "audit.(EntryFormatter).Process: unable to parse response from audit event: no namespace", + ExpectedErrorMessage: "unable to parse response from audit event: no namespace", Subtype: ResponseType, RequiredFormat: JSONxFormat, Data: &logical.LogInput{Request: &logical.Request{ID: "123"}}, @@ -738,7 +738,7 @@ func TestEntryFormatter_Process_JSON(t *testing.T) { auditEvent.Data = in e := &eventlogger.Event{ - Type: eventlogger.EventType(event.AuditType.String()), + Type: event.AuditType.AsEventType(), CreatedAt: time.Now(), Formatted: make(map[string][]byte), Payload: auditEvent, @@ -902,7 +902,7 @@ func TestEntryFormatter_Process_JSONx(t *testing.T) { auditEvent.Data = in e := &eventlogger.Event{ - Type: eventlogger.EventType(event.AuditType.String()), + Type: event.AuditType.AsEventType(), CreatedAt: time.Now(), Formatted: make(map[string][]byte), Payload: auditEvent, @@ -1169,7 +1169,7 @@ func TestEntryFormatter_Process_Panic(t *testing.T) { e2, err := formatter.Process(namespace.RootContext(nil), e) require.Error(t, err) - require.Contains(t, err.Error(), "audit.(EntryFormatter).Process: panic generating audit log: \"juan\"") + require.Contains(t, err.Error(), "panic generating audit log: \"juan\"") require.Nil(t, e2) } diff --git a/audit/errors.go b/audit/errors.go new file mode 100644 index 000000000000..be4e879fd2f1 --- /dev/null +++ b/audit/errors.go @@ -0,0 +1,34 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package audit + +import "errors" + +var ( + // ErrInternal should be used to represent an unexpected error that occurred + // within the audit system. + ErrInternal = errors.New("audit system internal error") + + // ErrInvalidParameter should be used to represent an error in which the + // internal audit system is receiving invalid parameters from other parts of + // Vault which should have already been validated. + ErrInvalidParameter = errors.New("invalid internal parameter") + + // ErrExternalOptions should be used to represent an error related to + // invalid configuration provided to Vault (i.e. by the Vault Operator). + ErrExternalOptions = errors.New("invalid configuration") +) + +// ConvertToExternalError handles converting an error that was generated in Vault +// and should appear as-is in the server logs, to an error that can be returned to +// calling clients (via the API/CLI). +func ConvertToExternalError(err error) error { + // If the error is an internal error, the contents will have been logged, and + // we should probably shield the caller from the details. + if errors.Is(err, ErrInternal) { + return ErrInternal + } + + return err +} diff --git a/audit/errors_test.go b/audit/errors_test.go new file mode 100644 index 000000000000..2d6314843402 --- /dev/null +++ b/audit/errors_test.go @@ -0,0 +1,26 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package audit + +import ( + "errors" + "fmt" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestErrors_ConvertToExternalError is used to check that we 'mute' errors which +// have an internal error in their tree. +func TestErrors_ConvertToExternalError(t *testing.T) { + t.Parallel() + + err := fmt.Errorf("wrap this error: %w", ErrInternal) + res := ConvertToExternalError(err) + require.EqualError(t, res, "audit system internal error") + + err = fmt.Errorf("test: %w", errors.New("this is just an error")) + res = ConvertToExternalError(err) + require.Equal(t, "test: this is just an error", res.Error()) +} diff --git a/audit/event.go b/audit/event.go index a68055bb0f9a..f3d0d3cbcd0e 100644 --- a/audit/event.go +++ b/audit/event.go @@ -5,6 +5,7 @@ package audit import ( "fmt" + "strings" "time" "github.com/hashicorp/vault/internal/observability/event" @@ -48,12 +49,10 @@ type subtype string // 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" - // Get the default options opts, err := getOpts(opt...) if err != nil { - return nil, fmt.Errorf("%s: error applying options: %w", op, err) + return nil, err } if opts.withID == "" { @@ -61,7 +60,7 @@ func NewEvent(s subtype, opt ...Option) (*AuditEvent, error) { opts.withID, err = event.NewID(string(event.AuditType)) if err != nil { - return nil, fmt.Errorf("%s: error creating ID for event: %w", op, err) + return nil, fmt.Errorf("error creating ID for event: %w", err) } } @@ -73,34 +72,32 @@ func NewEvent(s subtype, opt ...Option) (*AuditEvent, error) { } if err := audit.validate(); err != nil { - return nil, fmt.Errorf("%s: %w", op, err) + return nil, err } return audit, nil } // validate attempts to ensure the audit event in its present state is valid. func (a *AuditEvent) validate() error { - const op = "audit.(AuditEvent).validate" - if a == nil { - return fmt.Errorf("%s: event is nil: %w", op, event.ErrInvalidParameter) + return fmt.Errorf("event is nil: %w", ErrInvalidParameter) } if a.ID == "" { - return fmt.Errorf("%s: missing ID: %w", op, event.ErrInvalidParameter) + return fmt.Errorf("missing ID: %w", ErrInvalidParameter) } if a.Version != version { - return fmt.Errorf("%s: event version unsupported: %w", op, event.ErrInvalidParameter) + return fmt.Errorf("event version unsupported: %w", ErrInvalidParameter) } if a.Timestamp.IsZero() { - return fmt.Errorf("%s: event timestamp cannot be the zero time instant: %w", op, event.ErrInvalidParameter) + return fmt.Errorf("event timestamp cannot be the zero time instant: %w", ErrInvalidParameter) } err := a.Subtype.validate() if err != nil { - return fmt.Errorf("%s: %w", op, err) + return err } return nil @@ -108,23 +105,21 @@ func (a *AuditEvent) validate() error { // validate ensures that subtype is one of the set of allowed event subtypes. func (t subtype) validate() error { - const op = "audit.(subtype).validate" switch t { case RequestType, ResponseType: return nil default: - return fmt.Errorf("%s: '%s' is not a valid event subtype: %w", op, t, event.ErrInvalidParameter) + return fmt.Errorf("invalid event subtype %q: %w", t, ErrInvalidParameter) } } // validate ensures that format is one of the set of allowed event formats. func (f format) validate() error { - const op = "audit.(format).validate" switch f { case JSONFormat, JSONxFormat: return nil default: - return fmt.Errorf("%s: '%s' is not a valid format: %w", op, f, event.ErrInvalidParameter) + return fmt.Errorf("invalid format %q: %w", f, ErrInvalidParameter) } } @@ -163,3 +158,10 @@ func (t subtype) String() string { func (a *AuditEvent) formattedTime() string { return a.Timestamp.UTC().Format(time.RFC3339Nano) } + +// IsValidFormat provides a means to validate whether the supplied format is valid. +// Examples of valid formats are JSON and JSONx. +func IsValidFormat(v string) bool { + err := format(strings.TrimSpace(strings.ToLower(v))).validate() + return err == nil +} diff --git a/audit/event_test.go b/audit/event_test.go index e5b98fdc1703..9dc8d6b6a726 100644 --- a/audit/event_test.go +++ b/audit/event_test.go @@ -31,21 +31,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: "invalid event subtype \"\": invalid internal 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: "invalid event subtype \"\": invalid internal parameter", }, "bad-id": { Options: []Option{WithID("")}, Subtype: ResponseType, Format: JSONFormat, IsErrorExpected: true, - ExpectedErrorMessage: "audit.NewEvent: error applying options: id cannot be empty", + ExpectedErrorMessage: "id cannot be empty", }, "good": { Options: []Option{ @@ -119,12 +119,12 @@ func TestAuditEvent_Validate(t *testing.T) { "nil": { Value: nil, IsErrorExpected: true, - ExpectedErrorMessage: "audit.(AuditEvent).validate: event is nil: invalid parameter", + ExpectedErrorMessage: "event is nil: invalid internal parameter", }, "default": { Value: &AuditEvent{}, IsErrorExpected: true, - ExpectedErrorMessage: "audit.(AuditEvent).validate: missing ID: invalid parameter", + ExpectedErrorMessage: "missing ID: invalid internal parameter", }, "id-empty": { Value: &AuditEvent{ @@ -135,7 +135,7 @@ func TestAuditEvent_Validate(t *testing.T) { Data: nil, }, IsErrorExpected: true, - ExpectedErrorMessage: "audit.(AuditEvent).validate: missing ID: invalid parameter", + ExpectedErrorMessage: "missing ID: invalid internal parameter", }, "version-fiddled": { Value: &AuditEvent{ @@ -146,7 +146,7 @@ func TestAuditEvent_Validate(t *testing.T) { Data: nil, }, IsErrorExpected: true, - ExpectedErrorMessage: "audit.(AuditEvent).validate: event version unsupported: invalid parameter", + ExpectedErrorMessage: "event version unsupported: invalid internal parameter", }, "subtype-fiddled": { Value: &AuditEvent{ @@ -157,7 +157,7 @@ func TestAuditEvent_Validate(t *testing.T) { Data: nil, }, IsErrorExpected: true, - ExpectedErrorMessage: "audit.(AuditEvent).validate: audit.(subtype).validate: 'moon' is not a valid event subtype: invalid parameter", + ExpectedErrorMessage: "invalid event subtype \"moon\": invalid internal parameter", }, "default-time": { Value: &AuditEvent{ @@ -168,7 +168,7 @@ func TestAuditEvent_Validate(t *testing.T) { Data: nil, }, IsErrorExpected: true, - ExpectedErrorMessage: "audit.(AuditEvent).validate: event timestamp cannot be the zero time instant: invalid parameter", + ExpectedErrorMessage: "event timestamp cannot be the zero time instant: invalid internal parameter", }, "valid": { Value: &AuditEvent{ @@ -212,12 +212,12 @@ func TestAuditEvent_Validate_Subtype(t *testing.T) { "empty": { Value: "", IsErrorExpected: true, - ExpectedErrorMessage: "audit.(subtype).validate: '' is not a valid event subtype: invalid parameter", + ExpectedErrorMessage: "invalid event subtype \"\": invalid internal parameter", }, "unsupported": { Value: "foo", IsErrorExpected: true, - ExpectedErrorMessage: "audit.(subtype).validate: 'foo' is not a valid event subtype: invalid parameter", + ExpectedErrorMessage: "invalid event subtype \"foo\": invalid internal parameter", }, "request": { Value: "AuditRequest", @@ -259,12 +259,12 @@ func TestAuditEvent_Validate_Format(t *testing.T) { "empty": { Value: "", IsErrorExpected: true, - ExpectedErrorMessage: "audit.(format).validate: '' is not a valid format: invalid parameter", + ExpectedErrorMessage: "invalid format \"\": invalid internal parameter", }, "unsupported": { Value: "foo", IsErrorExpected: true, - ExpectedErrorMessage: "audit.(format).validate: 'foo' is not a valid format: invalid parameter", + ExpectedErrorMessage: "invalid format \"foo\": invalid internal parameter", }, "json": { Value: "json", @@ -378,3 +378,69 @@ func TestAuditEvent_formattedTime(t *testing.T) { require.NotNil(t, a) require.Equal(t, "2024-03-22T10:00:05.00000001Z", a.formattedTime()) } + +// TestEvent_IsValidFormat ensures that we can correctly determine valid and +// invalid formats. +func TestEvent_IsValidFormat(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + input string + expected bool + }{ + "empty": { + input: "", + expected: false, + }, + "whitespace": { + input: " ", + expected: false, + }, + "invalid-test": { + input: "test", + expected: false, + }, + "valid-json": { + input: "json", + expected: true, + }, + "upper-json": { + input: "JSON", + expected: true, + }, + "mixed-json": { + input: "Json", + expected: true, + }, + "spacey-json": { + input: " json ", + expected: true, + }, + "valid-jsonx": { + input: "jsonx", + expected: true, + }, + "upper-jsonx": { + input: "JSONX", + expected: true, + }, + "mixed-jsonx": { + input: "JsonX", + expected: true, + }, + "spacey-jsonx": { + input: " jsonx ", + expected: true, + }, + } + + for name, tc := range tests { + name := name + tc := tc + t.Run(name, func(t *testing.T) { + t.Parallel() + res := IsValidFormat(tc.input) + require.Equal(t, tc.expected, res) + }) + } +} diff --git a/audit/nodes.go b/audit/nodes.go index 624777e72515..6b6b0b845871 100644 --- a/audit/nodes.go +++ b/audit/nodes.go @@ -44,7 +44,7 @@ func ProcessManual(ctx context.Context, data *logical.LogInput, ids []eventlogge // Create an eventlogger event with the audit event as the payload. e := &eventlogger.Event{ - Type: eventlogger.EventType(event.AuditType.String()), + Type: event.AuditType.AsEventType(), CreatedAt: time.Now(), Formatted: make(map[string][]byte), Payload: a, diff --git a/audit/options.go b/audit/options.go index bf3458fa1e09..a48d76cd0227 100644 --- a/audit/options.go +++ b/audit/options.go @@ -104,7 +104,7 @@ func WithSubtype(s string) Option { // WithFormat provides an Option to represent event format. func WithFormat(f string) Option { return func(o *options) error { - f := strings.TrimSpace(f) + f := strings.TrimSpace(strings.ToLower(f)) if f == "" { // Return early, we won't attempt to apply this option if its empty. return nil diff --git a/audit/options_test.go b/audit/options_test.go index e94ed52d9ed8..33de069faeee 100644 --- a/audit/options_test.go +++ b/audit/options_test.go @@ -33,7 +33,7 @@ func TestOptions_WithFormat(t *testing.T) { "invalid-test": { Value: "test", IsErrorExpected: true, - ExpectedErrorMessage: "audit.(format).validate: 'test' is not a valid format: invalid parameter", + ExpectedErrorMessage: "invalid format \"test\": invalid internal parameter", }, "valid-json": { Value: "json", diff --git a/audit/sink_metric_timer.go b/audit/sink_metric_timer.go index 4bfbda71c455..57a282ae04ea 100644 --- a/audit/sink_metric_timer.go +++ b/audit/sink_metric_timer.go @@ -11,7 +11,6 @@ import ( "github.com/armon/go-metrics" "github.com/hashicorp/eventlogger" - "github.com/hashicorp/vault/internal/observability/event" ) var _ eventlogger.Node = (*SinkMetricTimer)(nil) @@ -29,19 +28,17 @@ type SinkMetricTimer struct { // NewSinkMetricTimer should be used to create the SinkMetricTimer. // It expects that an eventlogger.NodeTypeSink should be supplied as the sink. func NewSinkMetricTimer(name string, sink eventlogger.Node) (*SinkMetricTimer, error) { - const op = "audit.NewSinkMetricTimer" - name = strings.TrimSpace(name) if name == "" { - return nil, fmt.Errorf("%s: name is required: %w", op, event.ErrInvalidParameter) + return nil, fmt.Errorf("name is required: %w", ErrInvalidParameter) } if sink == nil || reflect.ValueOf(sink).IsNil() { - return nil, fmt.Errorf("%s: sink node is required: %w", op, event.ErrInvalidParameter) + return nil, fmt.Errorf("sink node is required: %w", ErrInvalidParameter) } if sink.Type() != eventlogger.NodeTypeSink { - return nil, fmt.Errorf("%s: sink node must be of type 'sink': %w", op, event.ErrInvalidParameter) + return nil, fmt.Errorf("sink node must be of type 'sink': %w", ErrInvalidParameter) } return &SinkMetricTimer{ diff --git a/audit/sink_metric_timer_test.go b/audit/sink_metric_timer_test.go index 7d4ef0731721..f65bbb9b52a5 100644 --- a/audit/sink_metric_timer_test.go +++ b/audit/sink_metric_timer_test.go @@ -30,19 +30,19 @@ func TestNewSinkMetricTimer(t *testing.T) { "no-name": { name: "", isErrorExpected: true, - expectedErrorMessage: "audit.NewSinkMetricTimer: name is required: invalid parameter", + expectedErrorMessage: "name is required: invalid internal parameter", }, "no-node": { name: "foo", node: nil, isErrorExpected: true, - expectedErrorMessage: "audit.NewSinkMetricTimer: sink node is required: invalid parameter", + expectedErrorMessage: "sink node is required: invalid internal parameter", }, "bad-node": { name: "foo", node: &EntryFormatter{}, isErrorExpected: true, - expectedErrorMessage: "audit.NewSinkMetricTimer: sink node must be of type 'sink': invalid parameter", + expectedErrorMessage: "sink node must be of type 'sink': invalid internal parameter", }, } diff --git a/audit/types.go b/audit/types.go index d64cbf174864..448a17393e8a 100644 --- a/audit/types.go +++ b/audit/types.go @@ -66,47 +66,6 @@ type HeaderFormatter interface { ApplyConfig(context.Context, map[string][]string, Salter) (map[string][]string, error) } -// FormatterConfig is used to provide basic configuration to a formatter. -// Use NewFormatterConfig to initialize the FormatterConfig struct. -type FormatterConfig struct { - Raw bool - HMACAccessor bool - - // Vault lacks pagination in its APIs. As a result, certain list operations can return **very** large responses. - // The user's chosen audit sinks may experience difficulty consuming audit records that swell to tens of megabytes - // of JSON. The responses of list operations are typically not very interesting, as they are mostly lists of keys, - // or, even when they include a "key_info" field, are not returning confidential information. They become even less - // interesting once HMAC-ed by the audit system. - // - // Some example Vault "list" operations that are prone to becoming very large in an active Vault installation are: - // auth/token/accessors/ - // identity/entity/id/ - // identity/entity-alias/id/ - // pki/certs/ - // - // This option exists to provide such users with the option to have response data elided from audit logs, only when - // the operation type is "list". For added safety, the elision only applies to the "keys" and "key_info" fields - // within the response data - these are conventionally the only fields present in a list response - see - // logical.ListResponse, and logical.ListResponseWithInfo. However, other fields are technically possible if a - // plugin author writes unusual code, and these will be preserved in the audit log even with this option enabled. - // The elision replaces the values of the "keys" and "key_info" fields with an integer count of the number of - // entries. This allows even the elided audit logs to still be useful for answering questions like - // "Was any data returned?" or "How many records were listed?". - ElideListResponses bool - - // This should only ever be used in a testing context - OmitTime bool - - // The required/target format for the event (supported: JSONFormat and JSONxFormat). - RequiredFormat format - - // headerFormatter specifies the formatter used for headers that existing in any incoming audit request. - headerFormatter HeaderFormatter - - // Prefix specifies a Prefix that should be prepended to any formatted request or response before serialization. - Prefix string -} - // RequestEntry is the structure of a request audit log entry. type RequestEntry struct { Auth *Auth `json:"auth,omitempty"` diff --git a/builtin/audit/file/backend.go b/builtin/audit/file/backend.go index 78a9d454feef..7bede8f564cf 100644 --- a/builtin/audit/file/backend.go +++ b/builtin/audit/file/backend.go @@ -45,18 +45,20 @@ type Backend struct { } func Factory(_ context.Context, conf *audit.BackendConfig, headersConfig audit.HeaderFormatter) (audit.Backend, error) { - const op = "file.Factory" - if conf.SaltConfig == nil { - return nil, fmt.Errorf("%s: nil salt config", op) + return nil, fmt.Errorf("nil salt config: %w", audit.ErrInvalidParameter) } if conf.SaltView == nil { - return nil, fmt.Errorf("%s: nil salt view", op) + return nil, fmt.Errorf("nil salt view: %w", audit.ErrInvalidParameter) } if conf.Logger == nil || reflect.ValueOf(conf.Logger).IsNil() { - return nil, fmt.Errorf("%s: nil logger", op) + return nil, fmt.Errorf("nil logger: %w", audit.ErrInvalidParameter) + } + + if conf.MountPath == "" { + return nil, fmt.Errorf("mount path cannot be empty: %w", audit.ErrInvalidParameter) } // The config options 'fallback' and 'filter' are mutually exclusive, a fallback @@ -66,12 +68,12 @@ func Factory(_ context.Context, conf *audit.BackendConfig, headersConfig audit.H if fallbackRaw, ok := conf.Config["fallback"]; ok { fallback, err = parseutil.ParseBool(fallbackRaw) if err != nil { - return nil, fmt.Errorf("%s: unable to parse 'fallback': %w", op, err) + return nil, fmt.Errorf("unable to parse 'fallback': %w", audit.ErrExternalOptions) } } if _, ok := conf.Config["filter"]; ok && fallback { - return nil, fmt.Errorf("%s: cannot configure a fallback device with a filter: %w", op, event.ErrInvalidParameter) + return nil, fmt.Errorf("cannot configure a fallback device with a filter: %w", audit.ErrExternalOptions) } // Get file path from config or fall back to the old option name ('path') for compatibility @@ -82,7 +84,7 @@ func Factory(_ context.Context, conf *audit.BackendConfig, headersConfig audit.H } else if p, ok = conf.Config["path"]; ok { filePath = p } else { - return nil, fmt.Errorf("%s: file_path is required", op) + return nil, fmt.Errorf("file_path is required: %w", audit.ErrExternalOptions) } // normalize file path if configured for stdout @@ -93,6 +95,11 @@ func Factory(_ context.Context, conf *audit.BackendConfig, headersConfig audit.H filePath = discard } + cfg, err := newFormatterConfig(headersConfig, conf.Config) + if err != nil { + return nil, err + } + b := &Backend{ fallback: fallback, name: conf.MountPath, @@ -109,22 +116,17 @@ func Factory(_ context.Context, conf *audit.BackendConfig, headersConfig audit.H err = b.configureFilterNode(conf.Config["filter"]) if err != nil { - return nil, fmt.Errorf("%s: error configuring filter node: %w", op, err) - } - - cfg, err := newFormatterConfig(headersConfig, conf.Config) - if err != nil { - return nil, fmt.Errorf("%s: failed to create formatter config: %w", op, err) + return nil, err } err = b.configureFormatterNode(conf.MountPath, cfg, conf.Logger) if err != nil { - return nil, fmt.Errorf("%s: error configuring formatter node: %w", op, err) + return nil, err } err = b.configureSinkNode(conf.MountPath, filePath, conf.Config["mode"], cfg.RequiredFormat.String()) if err != nil { - return nil, fmt.Errorf("%s: error configuring sink node: %w", op, err) + return nil, fmt.Errorf("error configuring sink node: %w", err) } return b, nil @@ -181,11 +183,13 @@ func (b *Backend) Invalidate(_ context.Context) { // newFormatterConfig creates the configuration required by a formatter node using // the config map supplied to the factory. func newFormatterConfig(headerFormatter audit.HeaderFormatter, config map[string]string) (audit.FormatterConfig, error) { - const op = "file.newFormatterConfig" - var opts []audit.Option if format, ok := config["format"]; ok { + if !audit.IsValidFormat(format) { + return audit.FormatterConfig{}, fmt.Errorf("unsupported 'format': %w", audit.ErrExternalOptions) + } + opts = append(opts, audit.WithFormat(format)) } @@ -193,7 +197,7 @@ func newFormatterConfig(headerFormatter audit.HeaderFormatter, config map[string if hmacAccessorRaw, ok := config["hmac_accessor"]; ok { v, err := strconv.ParseBool(hmacAccessorRaw) if err != nil { - return audit.FormatterConfig{}, fmt.Errorf("%s: unable to parse 'hmac_accessor': %w", op, err) + return audit.FormatterConfig{}, fmt.Errorf("unable to parse 'hmac_accessor': %w", audit.ErrExternalOptions) } opts = append(opts, audit.WithHMACAccessor(v)) } @@ -202,7 +206,7 @@ func newFormatterConfig(headerFormatter audit.HeaderFormatter, config map[string if raw, ok := config["log_raw"]; ok { v, err := strconv.ParseBool(raw) if err != nil { - return audit.FormatterConfig{}, fmt.Errorf("%s: unable to parse 'log_raw': %w", op, err) + return audit.FormatterConfig{}, fmt.Errorf("unable to parse 'log_raw: %w", audit.ErrExternalOptions) } opts = append(opts, audit.WithRaw(v)) } @@ -210,7 +214,7 @@ func newFormatterConfig(headerFormatter audit.HeaderFormatter, config map[string if elideListResponsesRaw, ok := config["elide_list_responses"]; ok { v, err := strconv.ParseBool(elideListResponsesRaw) if err != nil { - return audit.FormatterConfig{}, fmt.Errorf("%s: unable to parse 'elide_list_responses': %w", op, err) + return audit.FormatterConfig{}, fmt.Errorf("unable to parse 'elide_list_responses': %w", audit.ErrExternalOptions) } opts = append(opts, audit.WithElision(v)) } @@ -224,16 +228,14 @@ func newFormatterConfig(headerFormatter audit.HeaderFormatter, config map[string // configureFormatterNode is used to configure a formatter node and associated ID on the Backend. func (b *Backend) configureFormatterNode(name string, formatConfig audit.FormatterConfig, logger hclog.Logger) error { - const op = "file.(Backend).configureFormatterNode" - formatterNodeID, err := event.GenerateNodeID() if err != nil { - return fmt.Errorf("%s: error generating random NodeID for formatter node: %w", op, err) + return fmt.Errorf("error generating random NodeID for formatter node: %w: %w", audit.ErrInternal, err) } formatterNode, err := audit.NewEntryFormatter(name, formatConfig, b, logger) if err != nil { - return fmt.Errorf("%s: error creating formatter: %w", op, err) + return fmt.Errorf("error creating formatter: %w", err) } b.nodeIDList = append(b.nodeIDList, formatterNodeID) @@ -244,26 +246,24 @@ func (b *Backend) configureFormatterNode(name string, formatConfig audit.Formatt // configureSinkNode is used to configure a sink node and associated ID on the Backend. func (b *Backend) configureSinkNode(name string, filePath string, mode string, format string) error { - const op = "file.(Backend).configureSinkNode" - name = strings.TrimSpace(name) if name == "" { - return fmt.Errorf("%s: name is required: %w", op, event.ErrInvalidParameter) + return fmt.Errorf("name is required: %w", audit.ErrExternalOptions) } filePath = strings.TrimSpace(filePath) if filePath == "" { - return fmt.Errorf("%s: file path is required: %w", op, event.ErrInvalidParameter) + return fmt.Errorf("file path is required: %w", audit.ErrExternalOptions) } format = strings.TrimSpace(format) if format == "" { - return fmt.Errorf("%s: format is required: %w", op, event.ErrInvalidParameter) + return fmt.Errorf("format is required: %w", audit.ErrInvalidParameter) } sinkNodeID, err := event.GenerateNodeID() if err != nil { - return fmt.Errorf("%s: error generating random NodeID for sink node: %w", op, err) + return fmt.Errorf("error generating random NodeID for sink node: %w: %w", audit.ErrInternal, err) } // normalize file path if configured for stdout or discard @@ -290,13 +290,13 @@ func (b *Backend) configureSinkNode(name string, filePath string, mode string, f } if err != nil { - return fmt.Errorf("%s: file sink creation failed for path %q: %w", op, filePath, err) + return fmt.Errorf("file sink creation failed for path %q: %w", filePath, err) } // Wrap the sink node with metrics middleware sinkMetricTimer, err := audit.NewSinkMetricTimer(sinkName, sinkNode) if err != nil { - return fmt.Errorf("%s: unable to add timing metrics to sink for path %q: %w", op, filePath, err) + return fmt.Errorf("unable to add timing metrics to sink for path %q: %w", filePath, err) } // Decide what kind of labels we want and wrap the sink node inside a metrics counter. @@ -310,7 +310,7 @@ func (b *Backend) configureSinkNode(name string, filePath string, mode string, f sinkMetricCounter, err := event.NewMetricsCounter(sinkName, sinkMetricTimer, metricLabeler) if err != nil { - return fmt.Errorf("%s: unable to add counting metrics to sink for path %q: %w", op, filePath, err) + return fmt.Errorf("unable to add counting metrics to sink for path %q: %w", filePath, err) } b.nodeIDList = append(b.nodeIDList, sinkNodeID) @@ -336,7 +336,7 @@ func (b *Backend) NodeIDs() []eventlogger.NodeID { // EventType returns the event type for the backend. func (b *Backend) EventType() eventlogger.EventType { - return eventlogger.EventType(event.AuditType.String()) + return event.AuditType.AsEventType() } // HasFiltering determines if the first node for the pipeline is an eventlogger.NodeTypeFilter. diff --git a/builtin/audit/file/backend_test.go b/builtin/audit/file/backend_test.go index b67162a0a740..004513f1adc1 100644 --- a/builtin/audit/file/backend_test.go +++ b/builtin/audit/file/backend_test.go @@ -193,7 +193,7 @@ func TestBackend_newFormatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedMessage: "audit.NewFormatterConfig: error applying options: audit.(format).validate: 'squiggly' is not a valid format: invalid parameter", + expectedMessage: "unsupported 'format': invalid configuration", }, "invalid-hmac-accessor": { config: map[string]string{ @@ -202,7 +202,7 @@ func TestBackend_newFormatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedMessage: "file.newFormatterConfig: unable to parse 'hmac_accessor': strconv.ParseBool: parsing \"maybe\": invalid syntax", + expectedMessage: "unable to parse 'hmac_accessor': invalid configuration", }, "invalid-log-raw": { config: map[string]string{ @@ -212,7 +212,7 @@ func TestBackend_newFormatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedMessage: "file.newFormatterConfig: unable to parse 'log_raw': strconv.ParseBool: parsing \"maybe\": invalid syntax", + expectedMessage: "unable to parse 'log_raw: invalid configuration", }, "invalid-elide-bool": { config: map[string]string{ @@ -223,7 +223,7 @@ func TestBackend_newFormatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedMessage: "file.newFormatterConfig: unable to parse 'elide_list_responses': strconv.ParseBool: parsing \"maybe\": invalid syntax", + expectedMessage: "unable to parse 'elide_list_responses': invalid configuration", }, "prefix": { config: map[string]string{ @@ -300,24 +300,24 @@ func TestBackend_configureSinkNode(t *testing.T) { "name-empty": { name: "", wantErr: true, - expectedErrMsg: "file.(Backend).configureSinkNode: name is required: invalid parameter", + expectedErrMsg: "name is required: invalid configuration", }, "name-whitespace": { name: " ", wantErr: true, - expectedErrMsg: "file.(Backend).configureSinkNode: name is required: invalid parameter", + expectedErrMsg: "name is required: invalid configuration", }, "filePath-empty": { name: "foo", filePath: "", wantErr: true, - expectedErrMsg: "file.(Backend).configureSinkNode: file path is required: invalid parameter", + expectedErrMsg: "file path is required: invalid configuration", }, "filePath-whitespace": { name: "foo", filePath: " ", wantErr: true, - expectedErrMsg: "file.(Backend).configureSinkNode: file path is required: invalid parameter", + expectedErrMsg: "file path is required: invalid configuration", }, "filePath-stdout-lower": { name: "foo", @@ -360,14 +360,14 @@ func TestBackend_configureSinkNode(t *testing.T) { filePath: "/tmp/", format: "", wantErr: true, - expectedErrMsg: "file.(Backend).configureSinkNode: format is required: invalid parameter", + expectedErrMsg: "format is required: invalid internal parameter", }, "format-whitespace": { name: "foo", filePath: "/tmp/", format: " ", wantErr: true, - expectedErrMsg: "file.(Backend).configureSinkNode: format is required: invalid parameter", + expectedErrMsg: "format is required: invalid internal parameter", }, "filePath-weird-with-mode-zero": { name: "foo", @@ -375,7 +375,7 @@ func TestBackend_configureSinkNode(t *testing.T) { format: "json", mode: "0", wantErr: true, - expectedErrMsg: "file.(Backend).configureSinkNode: file sink creation failed for path \"/tmp/qwerty\": event.NewFileSink: unable to determine existing file mode: stat /tmp/qwerty: no such file or directory", + expectedErrMsg: "file sink creation failed for path \"/tmp/qwerty\": unable to determine existing file mode: stat /tmp/qwerty: no such file or directory", }, "happy": { name: "foo", @@ -437,14 +437,14 @@ func TestBackend_Factory_Conf(t *testing.T) { SaltConfig: nil, }, isErrorExpected: true, - expectedErrorMessage: "file.Factory: nil salt config", + expectedErrorMessage: "nil salt config: invalid internal parameter", }, "nil-salt-view": { backendConfig: &audit.BackendConfig{ SaltConfig: &salt.Config{}, }, isErrorExpected: true, - expectedErrorMessage: "file.Factory: nil salt view", + expectedErrorMessage: "nil salt view: invalid internal parameter", }, "nil-logger": { backendConfig: &audit.BackendConfig{ @@ -454,7 +454,7 @@ func TestBackend_Factory_Conf(t *testing.T) { Logger: nil, }, isErrorExpected: true, - expectedErrorMessage: "file.Factory: nil logger", + expectedErrorMessage: "nil logger: invalid internal parameter", }, "fallback-device-with-filter": { backendConfig: &audit.BackendConfig{ @@ -469,7 +469,7 @@ func TestBackend_Factory_Conf(t *testing.T) { }, }, isErrorExpected: true, - expectedErrorMessage: "file.Factory: cannot configure a fallback device with a filter: invalid parameter", + expectedErrorMessage: "cannot configure a fallback device with a filter: invalid configuration", }, "non-fallback-device-with-filter": { backendConfig: &audit.BackendConfig{ diff --git a/builtin/audit/socket/backend.go b/builtin/audit/socket/backend.go index 7eaf1a29f0a5..96c788df60e2 100644 --- a/builtin/audit/socket/backend.go +++ b/builtin/audit/socket/backend.go @@ -35,23 +35,24 @@ type Backend struct { } func Factory(_ context.Context, conf *audit.BackendConfig, headersConfig audit.HeaderFormatter) (audit.Backend, error) { - const op = "socket.Factory" - if conf.SaltConfig == nil { - return nil, fmt.Errorf("%s: nil salt config", op) + return nil, fmt.Errorf("nil salt config: %w", audit.ErrInvalidParameter) } if conf.SaltView == nil { - return nil, fmt.Errorf("%s: nil salt view", op) + return nil, fmt.Errorf("nil salt view: %w", audit.ErrInvalidParameter) } if conf.Logger == nil || reflect.ValueOf(conf.Logger).IsNil() { - return nil, fmt.Errorf("%s: nil logger", op) + return nil, fmt.Errorf("nil logger: %w", audit.ErrInvalidParameter) + } + if conf.MountPath == "" { + return nil, fmt.Errorf("mount path cannot be empty: %w", audit.ErrInvalidParameter) } address, ok := conf.Config["address"] if !ok { - return nil, fmt.Errorf("%s: address is required", op) + return nil, fmt.Errorf("address is required: %w", audit.ErrExternalOptions) } socketType, ok := conf.Config["socket_type"] @@ -64,19 +65,33 @@ func Factory(_ context.Context, conf *audit.BackendConfig, headersConfig audit.H writeDeadline = "2s" } + sinkOpts := []event.Option{ + event.WithSocketType(socketType), + event.WithMaxDuration(writeDeadline), + } + + err := event.ValidateOptions(sinkOpts...) + if err != nil { + return nil, 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 { - return nil, fmt.Errorf("%s: unable to parse 'fallback': %w", op, err) + return nil, fmt.Errorf("unable to parse 'fallback': %w", audit.ErrExternalOptions) } } if _, ok := conf.Config["filter"]; ok && fallback { - return nil, fmt.Errorf("%s: cannot configure a fallback device with a filter: %w", op, event.ErrInvalidParameter) + return nil, fmt.Errorf("cannot configure a fallback device with a filter: %w", audit.ErrExternalOptions) + } + + cfg, err := newFormatterConfig(headersConfig, conf.Config) + if err != nil { + return nil, err } b := &Backend{ @@ -90,27 +105,17 @@ func Factory(_ context.Context, conf *audit.BackendConfig, headersConfig audit.H err = b.configureFilterNode(conf.Config["filter"]) if err != nil { - return nil, fmt.Errorf("%s: error configuring filter node: %w", op, err) - } - - cfg, err := newFormatterConfig(headersConfig, conf.Config) - if err != nil { - return nil, fmt.Errorf("%s: failed to create formatter config: %w", op, err) + return nil, err } err = b.configureFormatterNode(conf.MountPath, cfg, conf.Logger) if err != nil { - return nil, fmt.Errorf("%s: error configuring formatter node: %w", op, err) - } - - sinkOpts := []event.Option{ - event.WithSocketType(socketType), - event.WithMaxDuration(writeDeadline), + return nil, err } err = b.configureSinkNode(conf.MountPath, address, cfg.RequiredFormat.String(), sinkOpts...) if err != nil { - return nil, fmt.Errorf("%s: error configuring sink node: %w", op, err) + return nil, err } return b, nil @@ -163,11 +168,13 @@ func (b *Backend) Invalidate(_ context.Context) { // newFormatterConfig creates the configuration required by a formatter node using // the config map supplied to the factory. func newFormatterConfig(headerFormatter audit.HeaderFormatter, config map[string]string) (audit.FormatterConfig, error) { - const op = "socket.newFormatterConfig" - var opts []audit.Option if format, ok := config["format"]; ok { + if !audit.IsValidFormat(format) { + return audit.FormatterConfig{}, fmt.Errorf("unsupported 'format': %w", audit.ErrExternalOptions) + } + opts = append(opts, audit.WithFormat(format)) } @@ -175,7 +182,7 @@ func newFormatterConfig(headerFormatter audit.HeaderFormatter, config map[string if hmacAccessorRaw, ok := config["hmac_accessor"]; ok { v, err := strconv.ParseBool(hmacAccessorRaw) if err != nil { - return audit.FormatterConfig{}, fmt.Errorf("%s: unable to parse 'hmac_accessor': %w", op, err) + return audit.FormatterConfig{}, fmt.Errorf("unable to parse 'hmac_accessor': %w", audit.ErrExternalOptions) } opts = append(opts, audit.WithHMACAccessor(v)) } @@ -184,7 +191,7 @@ func newFormatterConfig(headerFormatter audit.HeaderFormatter, config map[string if raw, ok := config["log_raw"]; ok { v, err := strconv.ParseBool(raw) if err != nil { - return audit.FormatterConfig{}, fmt.Errorf("%s: unable to parse 'log_raw': %w", op, err) + return audit.FormatterConfig{}, fmt.Errorf("unable to parse 'log_raw: %w", audit.ErrExternalOptions) } opts = append(opts, audit.WithRaw(v)) } @@ -192,7 +199,7 @@ func newFormatterConfig(headerFormatter audit.HeaderFormatter, config map[string if elideListResponsesRaw, ok := config["elide_list_responses"]; ok { v, err := strconv.ParseBool(elideListResponsesRaw) if err != nil { - return audit.FormatterConfig{}, fmt.Errorf("%s: unable to parse 'elide_list_responses': %w", op, err) + return audit.FormatterConfig{}, fmt.Errorf("unable to parse 'elide_list_responses': %w", audit.ErrExternalOptions) } opts = append(opts, audit.WithElision(v)) } @@ -206,16 +213,14 @@ func newFormatterConfig(headerFormatter audit.HeaderFormatter, config map[string // configureFormatterNode is used to configure a formatter node and associated ID on the Backend. func (b *Backend) configureFormatterNode(name string, formatConfig audit.FormatterConfig, logger hclog.Logger) error { - const op = "socket.(Backend).configureFormatterNode" - formatterNodeID, err := event.GenerateNodeID() if err != nil { - return fmt.Errorf("%s: error generating random NodeID for formatter node: %w", op, err) + return fmt.Errorf("error generating random NodeID for formatter node: %w: %w", audit.ErrInternal, err) } formatterNode, err := audit.NewEntryFormatter(name, formatConfig, b, logger) if err != nil { - return fmt.Errorf("%s: error creating formatter: %w", op, err) + return fmt.Errorf("error creating formatter: %w", err) } b.nodeIDList = append(b.nodeIDList, formatterNodeID) @@ -226,37 +231,35 @@ func (b *Backend) configureFormatterNode(name string, formatConfig audit.Formatt // configureSinkNode is used to configure a sink node and associated ID on the Backend. func (b *Backend) configureSinkNode(name string, address string, format string, opts ...event.Option) error { - const op = "socket.(Backend).configureSinkNode" - name = strings.TrimSpace(name) if name == "" { - return fmt.Errorf("%s: name is required: %w", op, event.ErrInvalidParameter) + return fmt.Errorf("name is required: %w", audit.ErrInvalidParameter) } address = strings.TrimSpace(address) if address == "" { - return fmt.Errorf("%s: address is required: %w", op, event.ErrInvalidParameter) + return fmt.Errorf("address is required: %w", audit.ErrInvalidParameter) } format = strings.TrimSpace(format) if format == "" { - return fmt.Errorf("%s: format is required: %w", op, event.ErrInvalidParameter) + return fmt.Errorf("format is required: %w", audit.ErrInvalidParameter) } sinkNodeID, err := event.GenerateNodeID() if err != nil { - return fmt.Errorf("%s: error generating random NodeID for sink node: %w", op, err) + return fmt.Errorf("error generating random NodeID for sink node: %w", err) } n, err := event.NewSocketSink(address, format, opts...) if err != nil { - return fmt.Errorf("%s: error creating socket sink node: %w", op, err) + return err } // Wrap the sink node with metrics middleware sinkMetricTimer, err := audit.NewSinkMetricTimer(name, n) if err != nil { - return fmt.Errorf("%s: unable to add timing metrics to sink for path %q: %w", op, name, err) + return fmt.Errorf("unable to add timing metrics to sink for path %q: %w", name, err) } // Decide what kind of labels we want and wrap the sink node inside a metrics counter. @@ -270,7 +273,7 @@ func (b *Backend) configureSinkNode(name string, address string, format string, sinkMetricCounter, err := event.NewMetricsCounter(name, sinkMetricTimer, metricLabeler) if err != nil { - return fmt.Errorf("%s: unable to add counting metrics to sink for path %q: %w", op, name, err) + return fmt.Errorf("unable to add counting metrics to sink for path %q: %w", name, err) } b.nodeIDList = append(b.nodeIDList, sinkNodeID) @@ -296,7 +299,7 @@ func (b *Backend) NodeIDs() []eventlogger.NodeID { // EventType returns the event type for the backend. func (b *Backend) EventType() eventlogger.EventType { - return eventlogger.EventType(event.AuditType.String()) + return event.AuditType.AsEventType() } // HasFiltering determines if the first node for the pipeline is an eventlogger.NodeTypeFilter. diff --git a/builtin/audit/socket/backend_test.go b/builtin/audit/socket/backend_test.go index 700fee9bbdea..b800374febd0 100644 --- a/builtin/audit/socket/backend_test.go +++ b/builtin/audit/socket/backend_test.go @@ -65,7 +65,7 @@ func TestBackend_newFormatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedErrMsg: "audit.NewFormatterConfig: error applying options: audit.(format).validate: 'squiggly' is not a valid format: invalid parameter", + expectedErrMsg: "unsupported 'format': invalid configuration", }, "invalid-hmac-accessor": { config: map[string]string{ @@ -74,7 +74,7 @@ func TestBackend_newFormatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedErrMsg: "socket.newFormatterConfig: unable to parse 'hmac_accessor': strconv.ParseBool: parsing \"maybe\": invalid syntax", + expectedErrMsg: "unable to parse 'hmac_accessor': invalid configuration", }, "invalid-log-raw": { config: map[string]string{ @@ -84,7 +84,7 @@ func TestBackend_newFormatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedErrMsg: "socket.newFormatterConfig: unable to parse 'log_raw': strconv.ParseBool: parsing \"maybe\": invalid syntax", + expectedErrMsg: "unable to parse 'log_raw: invalid configuration", }, "invalid-elide-bool": { config: map[string]string{ @@ -95,7 +95,7 @@ func TestBackend_newFormatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedErrMsg: "socket.newFormatterConfig: unable to parse 'elide_list_responses': strconv.ParseBool: parsing \"maybe\": invalid syntax", + expectedErrMsg: "unable to parse 'elide_list_responses': invalid configuration", }, "prefix": { config: map[string]string{ @@ -172,39 +172,39 @@ func TestBackend_configureSinkNode(t *testing.T) { name: "", address: "wss://foo", wantErr: true, - expectedErrMsg: "socket.(Backend).configureSinkNode: name is required: invalid parameter", + expectedErrMsg: "name is required: invalid internal parameter", }, "name-whitespace": { name: " ", address: "wss://foo", wantErr: true, - expectedErrMsg: "socket.(Backend).configureSinkNode: name is required: invalid parameter", + expectedErrMsg: "name is required: invalid internal parameter", }, "address-empty": { name: "foo", address: "", wantErr: true, - expectedErrMsg: "socket.(Backend).configureSinkNode: address is required: invalid parameter", + expectedErrMsg: "address is required: invalid internal parameter", }, "address-whitespace": { name: "foo", address: " ", wantErr: true, - expectedErrMsg: "socket.(Backend).configureSinkNode: address is required: invalid parameter", + expectedErrMsg: "address is required: invalid internal parameter", }, "format-empty": { name: "foo", address: "wss://foo", format: "", wantErr: true, - expectedErrMsg: "socket.(Backend).configureSinkNode: format is required: invalid parameter", + expectedErrMsg: "format is required: invalid internal parameter", }, "format-whitespace": { name: "foo", address: "wss://foo", format: " ", wantErr: true, - expectedErrMsg: "socket.(Backend).configureSinkNode: format is required: invalid parameter", + expectedErrMsg: "format is required: invalid internal parameter", }, "happy": { name: "foo", @@ -265,14 +265,14 @@ func TestBackend_Factory_Conf(t *testing.T) { SaltConfig: nil, }, isErrorExpected: true, - expectedErrorMessage: "socket.Factory: nil salt config", + expectedErrorMessage: "nil salt config: invalid internal parameter", }, "nil-salt-view": { backendConfig: &audit.BackendConfig{ SaltConfig: &salt.Config{}, }, isErrorExpected: true, - expectedErrorMessage: "socket.Factory: nil salt view", + expectedErrorMessage: "nil salt view: invalid internal parameter", }, "nil-logger": { backendConfig: &audit.BackendConfig{ @@ -282,7 +282,7 @@ func TestBackend_Factory_Conf(t *testing.T) { Logger: nil, }, isErrorExpected: true, - expectedErrorMessage: "socket.Factory: nil logger", + expectedErrorMessage: "nil logger: invalid internal parameter", }, "no-address": { backendConfig: &audit.BackendConfig{ @@ -293,7 +293,7 @@ func TestBackend_Factory_Conf(t *testing.T) { Config: map[string]string{}, }, isErrorExpected: true, - expectedErrorMessage: "socket.Factory: address is required", + expectedErrorMessage: "address is required: invalid configuration", }, "empty-address": { backendConfig: &audit.BackendConfig{ @@ -306,7 +306,7 @@ func TestBackend_Factory_Conf(t *testing.T) { }, }, isErrorExpected: true, - expectedErrorMessage: "socket.Factory: error configuring sink node: socket.(Backend).configureSinkNode: address is required: invalid parameter", + expectedErrorMessage: "address is required: invalid internal parameter", }, "whitespace-address": { backendConfig: &audit.BackendConfig{ @@ -319,7 +319,7 @@ func TestBackend_Factory_Conf(t *testing.T) { }, }, isErrorExpected: true, - expectedErrorMessage: "socket.Factory: error configuring sink node: socket.(Backend).configureSinkNode: address is required: invalid parameter", + expectedErrorMessage: "address is required: invalid internal parameter", }, "write-duration-valid": { backendConfig: &audit.BackendConfig{ @@ -346,7 +346,7 @@ func TestBackend_Factory_Conf(t *testing.T) { }, }, isErrorExpected: true, - 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\"", + expectedErrorMessage: "unable to parse max duration: invalid parameter: time: invalid duration \"qwerty\"", }, "non-fallback-device-with-filter": { backendConfig: &audit.BackendConfig{ @@ -377,7 +377,7 @@ func TestBackend_Factory_Conf(t *testing.T) { }, }, isErrorExpected: true, - expectedErrorMessage: "socket.Factory: cannot configure a fallback device with a filter: invalid parameter", + expectedErrorMessage: "cannot configure a fallback device with a filter: invalid configuration", }, } diff --git a/builtin/audit/syslog/backend.go b/builtin/audit/syslog/backend.go index 22471c33e73d..19f541e632af 100644 --- a/builtin/audit/syslog/backend.go +++ b/builtin/audit/syslog/backend.go @@ -35,18 +35,20 @@ type Backend struct { } func Factory(_ context.Context, conf *audit.BackendConfig, headersConfig audit.HeaderFormatter) (audit.Backend, error) { - const op = "syslog.Factory" - if conf.SaltConfig == nil { - return nil, fmt.Errorf("%s: nil salt config", op) + return nil, fmt.Errorf("nil salt config: %w", audit.ErrInvalidParameter) } if conf.SaltView == nil { - return nil, fmt.Errorf("%s: nil salt view", op) + return nil, fmt.Errorf("nil salt view: %w", audit.ErrInvalidParameter) } if conf.Logger == nil || reflect.ValueOf(conf.Logger).IsNil() { - return nil, fmt.Errorf("%s: nil logger", op) + return nil, fmt.Errorf("nil logger: %w", audit.ErrInvalidParameter) + } + + if conf.MountPath == "" { + return nil, fmt.Errorf("mount path cannot be empty: %w", audit.ErrInvalidParameter) } // Get facility or default to AUTH @@ -61,19 +63,33 @@ func Factory(_ context.Context, conf *audit.BackendConfig, headersConfig audit.H tag = "vault" } + sinkOpts := []event.Option{ + event.WithFacility(facility), + event.WithTag(tag), + } + + err := event.ValidateOptions(sinkOpts...) + if err != nil { + return nil, 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 { - return nil, fmt.Errorf("%s: unable to parse 'fallback': %w", op, err) + return nil, fmt.Errorf("unable to parse 'fallback': %w", audit.ErrExternalOptions) } } if _, ok := conf.Config["filter"]; ok && fallback { - return nil, fmt.Errorf("%s: cannot configure a fallback device with a filter: %w", op, event.ErrInvalidParameter) + return nil, fmt.Errorf("cannot configure a fallback device with a filter: %w", audit.ErrExternalOptions) + } + + cfg, err := newFormatterConfig(headersConfig, conf.Config) + if err != nil { + return nil, err } b := &Backend{ @@ -87,27 +103,17 @@ func Factory(_ context.Context, conf *audit.BackendConfig, headersConfig audit.H err = b.configureFilterNode(conf.Config["filter"]) if err != nil { - return nil, fmt.Errorf("%s: error configuring filter node: %w", op, err) - } - - cfg, err := newFormatterConfig(headersConfig, conf.Config) - if err != nil { - return nil, fmt.Errorf("%s: failed to create formatter config: %w", op, err) + return nil, err } err = b.configureFormatterNode(conf.MountPath, cfg, conf.Logger) if err != nil { - return nil, fmt.Errorf("%s: error configuring formatter node: %w", op, err) - } - - sinkOpts := []event.Option{ - event.WithFacility(facility), - event.WithTag(tag), + return nil, err } err = b.configureSinkNode(conf.MountPath, cfg.RequiredFormat.String(), sinkOpts...) if err != nil { - return nil, fmt.Errorf("%s: error configuring sink node: %w", op, err) + return nil, err } return b, nil @@ -154,11 +160,13 @@ func (b *Backend) Invalidate(_ context.Context) { // newFormatterConfig creates the configuration required by a formatter node using // the config map supplied to the factory. func newFormatterConfig(headerFormatter audit.HeaderFormatter, config map[string]string) (audit.FormatterConfig, error) { - const op = "syslog.newFormatterConfig" - var opts []audit.Option if format, ok := config["format"]; ok { + if !audit.IsValidFormat(format) { + return audit.FormatterConfig{}, fmt.Errorf("unsupported 'format': %w", audit.ErrExternalOptions) + } + opts = append(opts, audit.WithFormat(format)) } @@ -166,7 +174,7 @@ func newFormatterConfig(headerFormatter audit.HeaderFormatter, config map[string if hmacAccessorRaw, ok := config["hmac_accessor"]; ok { v, err := strconv.ParseBool(hmacAccessorRaw) if err != nil { - return audit.FormatterConfig{}, fmt.Errorf("%s: unable to parse 'hmac_accessor': %w", op, err) + return audit.FormatterConfig{}, fmt.Errorf("unable to parse 'hmac_accessor': %w", audit.ErrExternalOptions) } opts = append(opts, audit.WithHMACAccessor(v)) } @@ -175,7 +183,7 @@ func newFormatterConfig(headerFormatter audit.HeaderFormatter, config map[string if raw, ok := config["log_raw"]; ok { v, err := strconv.ParseBool(raw) if err != nil { - return audit.FormatterConfig{}, fmt.Errorf("%s: unable to parse 'log_raw': %w", op, err) + return audit.FormatterConfig{}, fmt.Errorf("unable to parse 'log_raw: %w", audit.ErrExternalOptions) } opts = append(opts, audit.WithRaw(v)) } @@ -183,7 +191,7 @@ func newFormatterConfig(headerFormatter audit.HeaderFormatter, config map[string if elideListResponsesRaw, ok := config["elide_list_responses"]; ok { v, err := strconv.ParseBool(elideListResponsesRaw) if err != nil { - return audit.FormatterConfig{}, fmt.Errorf("%s: unable to parse 'elide_list_responses': %w", op, err) + return audit.FormatterConfig{}, fmt.Errorf("unable to parse 'elide_list_responses': %w", audit.ErrExternalOptions) } opts = append(opts, audit.WithElision(v)) } @@ -197,16 +205,14 @@ func newFormatterConfig(headerFormatter audit.HeaderFormatter, config map[string // configureFormatterNode is used to configure a formatter node and associated ID on the Backend. func (b *Backend) configureFormatterNode(name string, formatConfig audit.FormatterConfig, logger hclog.Logger) error { - const op = "syslog.(Backend).configureFormatterNode" - formatterNodeID, err := event.GenerateNodeID() if err != nil { - return fmt.Errorf("%s: error generating random NodeID for formatter node: %w", op, err) + return fmt.Errorf("error generating random NodeID for formatter node: %w: %w", audit.ErrInternal, err) } formatterNode, err := audit.NewEntryFormatter(name, formatConfig, b, logger) if err != nil { - return fmt.Errorf("%s: error creating formatter: %w", op, err) + return fmt.Errorf("error creating formatter: %w", err) } b.nodeIDList = append(b.nodeIDList, formatterNodeID) @@ -217,32 +223,30 @@ func (b *Backend) configureFormatterNode(name string, formatConfig audit.Formatt // configureSinkNode is used to configure a sink node and associated ID on the Backend. func (b *Backend) configureSinkNode(name string, format string, opts ...event.Option) error { - const op = "syslog.(Backend).configureSinkNode" - name = strings.TrimSpace(name) if name == "" { - return fmt.Errorf("%s: name is required: %w", op, event.ErrInvalidParameter) + return fmt.Errorf("name is required: %w", audit.ErrInvalidParameter) } format = strings.TrimSpace(format) if format == "" { - return fmt.Errorf("%s: format is required: %w", op, event.ErrInvalidParameter) + return fmt.Errorf("format is required: %w", audit.ErrInvalidParameter) } sinkNodeID, err := event.GenerateNodeID() if err != nil { - return fmt.Errorf("%s: error generating random NodeID for sink node: %w", op, err) + return fmt.Errorf("error generating random NodeID for sink node: %w: %w", audit.ErrInternal, err) } n, err := event.NewSyslogSink(format, opts...) if err != nil { - return fmt.Errorf("%s: error creating syslog sink node: %w", op, err) + return fmt.Errorf("error creating syslog sink node: %w", err) } // Wrap the sink node with metrics middleware sinkMetricTimer, err := audit.NewSinkMetricTimer(name, n) if err != nil { - return fmt.Errorf("%s: unable to add timing metrics to sink for path %q: %w", op, name, err) + return fmt.Errorf("unable to add timing metrics to sink for path %q: %w", name, err) } // Decide what kind of labels we want and wrap the sink node inside a metrics counter. @@ -256,7 +260,7 @@ func (b *Backend) configureSinkNode(name string, format string, opts ...event.Op sinkMetricCounter, err := event.NewMetricsCounter(name, sinkMetricTimer, metricLabeler) if err != nil { - return fmt.Errorf("%s: unable to add counting metrics to sink for path %q: %w", op, name, err) + return fmt.Errorf("unable to add counting metrics to sink for path %q: %w", name, err) } b.nodeIDList = append(b.nodeIDList, sinkNodeID) @@ -282,7 +286,7 @@ func (b *Backend) NodeIDs() []eventlogger.NodeID { // EventType returns the event type for the backend. func (b *Backend) EventType() eventlogger.EventType { - return eventlogger.EventType(event.AuditType.String()) + return event.AuditType.AsEventType() } // HasFiltering determines if the first node for the pipeline is an eventlogger.NodeTypeFilter. diff --git a/builtin/audit/syslog/backend_test.go b/builtin/audit/syslog/backend_test.go index 906734343601..1ee304a210b4 100644 --- a/builtin/audit/syslog/backend_test.go +++ b/builtin/audit/syslog/backend_test.go @@ -65,7 +65,7 @@ func TestBackend_newFormatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedErrMsg: "audit.NewFormatterConfig: error applying options: audit.(format).validate: 'squiggly' is not a valid format: invalid parameter", + expectedErrMsg: "unsupported 'format': invalid configuration", }, "invalid-hmac-accessor": { config: map[string]string{ @@ -74,7 +74,7 @@ func TestBackend_newFormatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedErrMsg: "syslog.newFormatterConfig: unable to parse 'hmac_accessor': strconv.ParseBool: parsing \"maybe\": invalid syntax", + expectedErrMsg: "unable to parse 'hmac_accessor': invalid configuration", }, "invalid-log-raw": { config: map[string]string{ @@ -84,7 +84,7 @@ func TestBackend_newFormatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedErrMsg: "syslog.newFormatterConfig: unable to parse 'log_raw': strconv.ParseBool: parsing \"maybe\": invalid syntax", + expectedErrMsg: "unable to parse 'log_raw: invalid configuration", }, "invalid-elide-bool": { config: map[string]string{ @@ -95,7 +95,7 @@ func TestBackend_newFormatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedErrMsg: "syslog.newFormatterConfig: unable to parse 'elide_list_responses': strconv.ParseBool: parsing \"maybe\": invalid syntax", + expectedErrMsg: "unable to parse 'elide_list_responses': invalid configuration", }, "prefix": { config: map[string]string{ @@ -170,24 +170,24 @@ func TestBackend_configureSinkNode(t *testing.T) { "name-empty": { name: "", wantErr: true, - expectedErrMsg: "syslog.(Backend).configureSinkNode: name is required: invalid parameter", + expectedErrMsg: "name is required: invalid internal parameter", }, "name-whitespace": { name: " ", wantErr: true, - expectedErrMsg: "syslog.(Backend).configureSinkNode: name is required: invalid parameter", + expectedErrMsg: "name is required: invalid internal parameter", }, "format-empty": { name: "foo", format: "", wantErr: true, - expectedErrMsg: "syslog.(Backend).configureSinkNode: format is required: invalid parameter", + expectedErrMsg: "format is required: invalid internal parameter", }, "format-whitespace": { name: "foo", format: " ", wantErr: true, - expectedErrMsg: "syslog.(Backend).configureSinkNode: format is required: invalid parameter", + expectedErrMsg: "format is required: invalid internal parameter", }, "happy": { name: "foo", @@ -247,14 +247,14 @@ func TestBackend_Factory_Conf(t *testing.T) { SaltConfig: nil, }, isErrorExpected: true, - expectedErrorMessage: "syslog.Factory: nil salt config", + expectedErrorMessage: "nil salt config: invalid internal parameter", }, "nil-salt-view": { backendConfig: &audit.BackendConfig{ SaltConfig: &salt.Config{}, }, isErrorExpected: true, - expectedErrorMessage: "syslog.Factory: nil salt view", + expectedErrorMessage: "nil salt view: invalid internal parameter", }, "non-fallback-device-with-filter": { backendConfig: &audit.BackendConfig{ @@ -281,7 +281,7 @@ func TestBackend_Factory_Conf(t *testing.T) { }, }, isErrorExpected: true, - expectedErrorMessage: "syslog.Factory: cannot configure a fallback device with a filter: invalid parameter", + expectedErrorMessage: "cannot configure a fallback device with a filter: invalid configuration", }, } diff --git a/helper/testhelpers/corehelpers/corehelpers.go b/helper/testhelpers/corehelpers/corehelpers.go index 941a88394f01..cae4d5b019d4 100644 --- a/helper/testhelpers/corehelpers/corehelpers.go +++ b/helper/testhelpers/corehelpers/corehelpers.go @@ -524,7 +524,7 @@ func (n *NoopAudit) RegisterNodesAndPipeline(broker *eventlogger.Broker, name st pipeline := eventlogger.Pipeline{ PipelineID: eventlogger.PipelineID(name), - EventType: eventlogger.EventType(event.AuditType.String()), + EventType: event.AuditType.AsEventType(), NodeIDs: n.nodeIDList, } @@ -600,7 +600,7 @@ func (tl *TestLogger) StopLogging() { } func (n *NoopAudit) EventType() eventlogger.EventType { - return eventlogger.EventType(event.AuditType.String()) + return event.AuditType.AsEventType() } func (n *NoopAudit) HasFiltering() bool { diff --git a/internal/observability/event/event_type.go b/internal/observability/event/event_type.go index a6e10e4903c3..16a2f7674bb8 100644 --- a/internal/observability/event/event_type.go +++ b/internal/observability/event/event_type.go @@ -19,12 +19,11 @@ const ( // Validate ensures that EventType is one of the set of allowed event types. func (t EventType) Validate() error { - const op = "event.(EventType).Validate" switch t { case AuditType: return nil default: - return fmt.Errorf("%s: '%s' is not a valid event type: %w", op, t, ErrInvalidParameter) + return fmt.Errorf("invalid event type %q: %w", t, ErrInvalidParameter) } } @@ -40,3 +39,8 @@ func GenerateNodeID() (eventlogger.NodeID, error) { func (t EventType) String() string { return string(t) } + +// AsEventType returns the EventType in a format for eventlogger. +func (t EventType) AsEventType() eventlogger.EventType { + return eventlogger.EventType(t.String()) +} diff --git a/internal/observability/event/event_type_test.go b/internal/observability/event/event_type_test.go index 9bc36f995110..ce8e238dfec0 100644 --- a/internal/observability/event/event_type_test.go +++ b/internal/observability/event/event_type_test.go @@ -23,12 +23,12 @@ func TestEventType_Validate(t *testing.T) { "empty": { Value: "", IsValid: false, - ExpectedError: "event.(EventType).Validate: '' is not a valid event type: invalid parameter", + ExpectedError: "invalid event type \"\": invalid parameter", }, "random": { Value: "random", IsValid: false, - ExpectedError: "event.(EventType).Validate: 'random' is not a valid event type: invalid parameter", + ExpectedError: "invalid event type \"random\": invalid parameter", }, } diff --git a/internal/observability/event/node_metrics_counter.go b/internal/observability/event/node_metrics_counter.go index 1d94ade648cc..980906137634 100644 --- a/internal/observability/event/node_metrics_counter.go +++ b/internal/observability/event/node_metrics_counter.go @@ -31,19 +31,17 @@ type Labeler interface { // NewMetricsCounter should be used to create the MetricsCounter. func NewMetricsCounter(name string, node eventlogger.Node, labeler Labeler) (*MetricsCounter, error) { - const op = "event.NewMetricsCounter" - name = strings.TrimSpace(name) if name == "" { - return nil, fmt.Errorf("%s: name is required: %w", op, ErrInvalidParameter) + return nil, fmt.Errorf("name is required: %w", ErrInvalidParameter) } if node == nil || reflect.ValueOf(node).IsNil() { - return nil, fmt.Errorf("%s: node is required: %w", op, ErrInvalidParameter) + return nil, fmt.Errorf("node is required: %w", ErrInvalidParameter) } if labeler == nil || reflect.ValueOf(labeler).IsNil() { - return nil, fmt.Errorf("%s: labeler is required: %w", op, ErrInvalidParameter) + return nil, fmt.Errorf("labeler is required: %w", ErrInvalidParameter) } return &MetricsCounter{ diff --git a/internal/observability/event/node_metrics_counter_test.go b/internal/observability/event/node_metrics_counter_test.go index dd6d56db5842..ac1679723123 100644 --- a/internal/observability/event/node_metrics_counter_test.go +++ b/internal/observability/event/node_metrics_counter_test.go @@ -39,20 +39,20 @@ func TestNewMetricsCounter(t *testing.T) { node: nil, labeler: nil, isErrorExpected: true, - expectedErrorMessage: "event.NewMetricsCounter: name is required: invalid parameter", + expectedErrorMessage: "name is required: invalid parameter", }, "no-node": { name: "foo", node: nil, isErrorExpected: true, - expectedErrorMessage: "event.NewMetricsCounter: node is required: invalid parameter", + expectedErrorMessage: "node is required: invalid parameter", }, "no-labeler": { name: "foo", node: &testEventLoggerNode{}, labeler: nil, isErrorExpected: true, - expectedErrorMessage: "event.NewMetricsCounter: labeler is required: invalid parameter", + expectedErrorMessage: "labeler is required: invalid parameter", }, } diff --git a/internal/observability/event/options.go b/internal/observability/event/options.go index e788cecb6de3..62fb4265954e 100644 --- a/internal/observability/event/options.go +++ b/internal/observability/event/options.go @@ -4,7 +4,6 @@ package event import ( - "errors" "fmt" "os" "strconv" @@ -59,18 +58,24 @@ func getOpts(opt ...Option) (options, error) { return opts, nil } +// ValidateOptions can be used to validate options before they are required. +func ValidateOptions(opt ...Option) error { + _, err := getOpts(opt...) + + return err +} + // NewID is a bit of a modified NewID has been done to stop a circular // dependency with the errors package that is caused by importing // boundary/internal/db func NewID(prefix string) (string, error) { - const op = "event.NewID" if prefix == "" { - return "", fmt.Errorf("%s: missing prefix: %w", op, ErrInvalidParameter) + return "", fmt.Errorf("missing prefix: %w", ErrInvalidParameter) } id, err := uuid.GenerateUUID() if err != nil { - return "", fmt.Errorf("%s: unable to generate ID: %w", op, err) + return "", fmt.Errorf("unable to generate ID: %w", err) } return fmt.Sprintf("%s_%s", prefix, id), nil @@ -84,7 +89,7 @@ func WithID(id string) Option { id := strings.TrimSpace(id) switch { case id == "": - err = errors.New("id cannot be empty") + err = fmt.Errorf("id cannot be empty: %w", ErrInvalidParameter) default: o.withID = id } @@ -100,7 +105,7 @@ func WithNow(now time.Time) Option { switch { case now.IsZero(): - err = errors.New("cannot specify 'now' to be the zero time instant") + err = fmt.Errorf("cannot specify 'now' to be the zero time instant: %w", ErrInvalidParameter) default: o.withNow = now } @@ -159,7 +164,7 @@ func WithMaxDuration(duration string) Option { parsed, err := parseutil.ParseDurationSecond(duration) if err != nil { - return fmt.Errorf("unable to parse max duration: %w", err) + return fmt.Errorf("unable to parse max duration: %w: %w", ErrInvalidParameter, err) } o.withMaxDuration = parsed @@ -187,7 +192,7 @@ func WithFileMode(mode string) Option { switch { case err != nil: - return fmt.Errorf("unable to parse file mode: %w", err) + return fmt.Errorf("unable to parse file mode: %w: %w", ErrInvalidParameter, err) default: m := os.FileMode(raw) o.withFileMode = &m diff --git a/internal/observability/event/options_test.go b/internal/observability/event/options_test.go index 95f1193f1b7e..a3e47a2c487c 100644 --- a/internal/observability/event/options_test.go +++ b/internal/observability/event/options_test.go @@ -22,7 +22,7 @@ func TestOptions_WithNow(t *testing.T) { "default-time": { Value: time.Time{}, IsErrorExpected: true, - ExpectedErrorMessage: "cannot specify 'now' to be the zero time instant", + ExpectedErrorMessage: "cannot specify 'now' to be the zero time instant: invalid parameter", }, "valid-time": { Value: time.Date(2023, time.July, 4, 12, 3, 0, 0, time.Local), @@ -63,12 +63,12 @@ func TestOptions_WithID(t *testing.T) { "empty": { Value: "", IsErrorExpected: true, - ExpectedErrorMessage: "id cannot be empty", + ExpectedErrorMessage: "id cannot be empty: invalid parameter", }, "whitespace": { Value: " ", IsErrorExpected: true, - ExpectedErrorMessage: "id cannot be empty", + ExpectedErrorMessage: "id cannot be empty: invalid parameter", }, "valid": { Value: "test", @@ -152,7 +152,7 @@ func TestOptions_Opts(t *testing.T) { WithNow(time.Time{}), }, IsErrorExpected: true, - ExpectedErrorMessage: "cannot specify 'now' to be the zero time instant", + ExpectedErrorMessage: "cannot specify 'now' to be the zero time instant: invalid parameter", }, "with-multiple-valid-options": { opts: []Option{ @@ -324,12 +324,12 @@ func TestOptions_WithMaxDuration(t *testing.T) { "bad-value": { Value: "juan", IsErrorExpected: true, - ExpectedErrorMessage: "unable to parse max duration: time: invalid duration \"juan\"", + ExpectedErrorMessage: "unable to parse max duration: invalid parameter: time: invalid duration \"juan\"", }, "bad-spacey-value": { Value: " juan ", IsErrorExpected: true, - ExpectedErrorMessage: "unable to parse max duration: time: invalid duration \"juan\"", + ExpectedErrorMessage: "unable to parse max duration: invalid parameter: time: invalid duration \"juan\"", }, "duration-2s": { Value: "2s", @@ -383,7 +383,7 @@ func TestOptions_WithFileMode(t *testing.T) { "nonsense": { Value: "juan", IsErrorExpected: true, - ExpectedErrorMessage: "unable to parse file mode: strconv.ParseUint: parsing \"juan\": invalid syntax", + ExpectedErrorMessage: "unable to parse file mode: invalid parameter: strconv.ParseUint: parsing \"juan\": invalid syntax", }, "zero": { Value: "0000", diff --git a/internal/observability/event/sink_file.go b/internal/observability/event/sink_file.go index bd997b70721b..0f5e22e4c8de 100644 --- a/internal/observability/event/sink_file.go +++ b/internal/observability/event/sink_file.go @@ -36,17 +36,15 @@ type FileSink struct { // NewFileSink should be used to create a new FileSink. // Accepted options: WithFileMode. func NewFileSink(path string, format string, opt ...Option) (*FileSink, error) { - const op = "event.NewFileSink" - // Parse and check path p := strings.TrimSpace(path) if p == "" { - return nil, fmt.Errorf("%s: path is required", op) + return nil, fmt.Errorf("path is required: %w", ErrInvalidParameter) } opts, err := getOpts(opt...) if err != nil { - return nil, fmt.Errorf("%s: error applying options: %w", op, err) + return nil, err } mode := os.FileMode(defaultFileMode) @@ -58,7 +56,7 @@ func NewFileSink(path string, format string, opt ...Option) (*FileSink, error) { case *opts.withFileMode == 0: // Maintain the existing file's mode when set to "0000". fileInfo, err := os.Stat(path) if err != nil { - return nil, fmt.Errorf("%s: unable to determine existing file mode: %w", op, err) + return nil, fmt.Errorf("unable to determine existing file mode: %w", err) } mode = fileInfo.Mode() default: @@ -77,7 +75,7 @@ func NewFileSink(path string, format string, opt ...Option) (*FileSink, error) { // otherwise it will be too late to catch later without problems // (ref: https://github.com/hashicorp/vault/issues/550) if err := sink.open(); err != nil { - return nil, fmt.Errorf("%s: sanity check failed; unable to open %q for writing: %w", op, path, err) + return nil, fmt.Errorf("sanity check failed; unable to open %q for writing: %w", sink.path, err) } return sink, nil @@ -85,8 +83,6 @@ func NewFileSink(path string, format string, opt ...Option) (*FileSink, error) { // Process handles writing the event to the file sink. func (s *FileSink) Process(ctx context.Context, e *eventlogger.Event) (*eventlogger.Event, error) { - const op = "event.(FileSink).Process" - select { case <-ctx.Done(): return nil, ctx.Err() @@ -94,7 +90,7 @@ func (s *FileSink) Process(ctx context.Context, e *eventlogger.Event) (*eventlog } if e == nil { - return nil, fmt.Errorf("%s: event is nil: %w", op, ErrInvalidParameter) + return nil, fmt.Errorf("event is nil: %w", ErrInvalidParameter) } // '/dev/null' path means we just do nothing and pretend we're done. @@ -104,12 +100,12 @@ func (s *FileSink) Process(ctx context.Context, e *eventlogger.Event) (*eventlog formatted, found := e.Format(s.requiredFormat) if !found { - return nil, fmt.Errorf("%s: unable to retrieve event formatted as %q", op, s.requiredFormat) + return nil, fmt.Errorf("unable to retrieve event formatted as %q: %w", s.requiredFormat, ErrInvalidParameter) } err := s.log(formatted) if err != nil { - return nil, fmt.Errorf("%s: error writing file for sink: %w", op, err) + return nil, fmt.Errorf("error writing file for sink %q: %w", s.path, err) } // return nil for the event to indicate the pipeline is complete. @@ -118,8 +114,6 @@ func (s *FileSink) Process(ctx context.Context, e *eventlogger.Event) (*eventlog // Reopen handles closing and reopening the file. func (s *FileSink) Reopen() error { - const op = "event.(FileSink).Reopen" - // '/dev/null' path means we just do nothing and pretend we're done. if s.path == devnull { return nil @@ -136,7 +130,7 @@ func (s *FileSink) Reopen() error { // Set to nil here so that even if we error out, on the next access open() will be tried. s.file = nil if err != nil { - return fmt.Errorf("%s: unable to close file for re-opening on sink: %w", op, err) + return fmt.Errorf("unable to close file for re-opening on sink %q: %w", s.path, err) } return s.open() @@ -152,20 +146,18 @@ func (s *FileSink) Type() eventlogger.NodeType { // It doesn't have any locking and relies on calling functions of FileSink to // handle this (e.g. log and Reopen methods). func (s *FileSink) open() error { - const op = "event.(FileSink).open" - if s.file != nil { return nil } if err := os.MkdirAll(filepath.Dir(s.path), s.fileMode); err != nil { - return fmt.Errorf("%s: unable to create file %q: %w", op, s.path, err) + return fmt.Errorf("unable to create file %q: %w", s.path, err) } var err error s.file, err = os.OpenFile(s.path, os.O_APPEND|os.O_WRONLY|os.O_CREATE, s.fileMode) if err != nil { - return fmt.Errorf("%s: unable to open file for sink: %w", op, err) + return fmt.Errorf("unable to open file for sink %q: %w", s.path, err) } // Change the file mode in case the log file already existed. @@ -176,7 +168,7 @@ func (s *FileSink) open() error { if s.fileMode != 0 { err = os.Chmod(s.path, s.fileMode) if err != nil { - return fmt.Errorf("%s: unable to change file %q permissions '%v' for sink: %w", op, s.path, s.fileMode, err) + return fmt.Errorf("unable to change file permissions '%v' for sink %q: %w", s.fileMode, s.path, err) } } } @@ -187,15 +179,13 @@ func (s *FileSink) open() error { // log writes the buffer to the file. // It acquires a lock on the file to do this. func (s *FileSink) log(data []byte) error { - const op = "event.(FileSink).log" - s.fileLock.Lock() defer s.fileLock.Unlock() reader := bytes.NewReader(data) if err := s.open(); err != nil { - return fmt.Errorf("%s: unable to open file for sink: %w", op, err) + return fmt.Errorf("unable to open file for sink %q: %w", s.path, err) } if _, err := reader.WriteTo(s.file); err == nil { @@ -205,23 +195,23 @@ func (s *FileSink) log(data []byte) error { // Otherwise, opportunistically try to re-open the FD, once per call (1 retry attempt). err := s.file.Close() if err != nil { - return fmt.Errorf("%s: unable to close file for sink: %w", op, err) + return fmt.Errorf("unable to close file for sink %q: %w", s.path, err) } s.file = nil if err := s.open(); err != nil { - return fmt.Errorf("%s: unable to re-open file for sink: %w", op, err) + return fmt.Errorf("unable to re-open file for sink %q: %w", s.path, err) } _, err = reader.Seek(0, io.SeekStart) if err != nil { - return fmt.Errorf("%s: unable to seek to start of file for sink: %w", op, err) + return fmt.Errorf("unable to seek to start of file for sink %q: %w", s.path, err) } _, err = reader.WriteTo(s.file) if err != nil { - return fmt.Errorf("%s: unable to re-write to file for sink: %w", op, err) + return fmt.Errorf("unable to re-write to file for sink %q: %w", s.path, err) } return nil diff --git a/internal/observability/event/sink_file_test.go b/internal/observability/event/sink_file_test.go index 17f3b25a9414..4018bfd474a9 100644 --- a/internal/observability/event/sink_file_test.go +++ b/internal/observability/event/sink_file_test.go @@ -40,14 +40,14 @@ func TestNewFileSink(t *testing.T) { "default-values": { ShouldUseAbsolutePath: true, IsErrorExpected: true, - ExpectedErrorMessage: "event.NewFileSink: path is required", + ExpectedErrorMessage: "path is required: invalid parameter", }, "spacey-path": { ShouldUseAbsolutePath: true, Path: " ", Format: "json", IsErrorExpected: true, - ExpectedErrorMessage: "event.NewFileSink: path is required", + ExpectedErrorMessage: "path is required: invalid parameter", }, "valid-path-and-format": { Path: "vault.log", @@ -226,7 +226,7 @@ func TestFileSink_Process(t *testing.T) { Data: "foo", ShouldIgnoreFormat: true, IsErrorExpected: true, - ExpectedErrorMessage: "event.(FileSink).Process: unable to retrieve event formatted as \"json\"", + ExpectedErrorMessage: "unable to retrieve event formatted as \"json\": invalid parameter", }, "nil": { Path: "foo.log", @@ -234,7 +234,7 @@ func TestFileSink_Process(t *testing.T) { Data: "foo", ShouldUseNilEvent: true, IsErrorExpected: true, - ExpectedErrorMessage: "event.(FileSink).Process: event is nil: invalid parameter", + ExpectedErrorMessage: "event is nil: invalid parameter", }, } diff --git a/internal/observability/event/sink_socket.go b/internal/observability/event/sink_socket.go index 861da0a20121..7d7502306086 100644 --- a/internal/observability/event/sink_socket.go +++ b/internal/observability/event/sink_socket.go @@ -30,21 +30,19 @@ type SocketSink struct { // NewSocketSink should be used to create a new SocketSink. // Accepted options: WithMaxDuration and WithSocketType. func NewSocketSink(address string, format string, opt ...Option) (*SocketSink, error) { - const op = "event.NewSocketSink" - address = strings.TrimSpace(address) if address == "" { - return nil, fmt.Errorf("%s: address is required: %w", op, ErrInvalidParameter) + return nil, fmt.Errorf("address is required: %w", ErrInvalidParameter) } format = strings.TrimSpace(format) if format == "" { - return nil, fmt.Errorf("%s: format is required: %w", op, ErrInvalidParameter) + return nil, fmt.Errorf("format is required: %w", ErrInvalidParameter) } opts, err := getOpts(opt...) if err != nil { - return nil, fmt.Errorf("%s: error applying options: %w", op, err) + return nil, err } sink := &SocketSink{ @@ -61,8 +59,6 @@ func NewSocketSink(address string, format string, opt ...Option) (*SocketSink, e // Process handles writing the event to the socket. func (s *SocketSink) Process(ctx context.Context, e *eventlogger.Event) (*eventlogger.Event, error) { - const op = "event.(SocketSink).Process" - select { case <-ctx.Done(): return nil, ctx.Err() @@ -73,12 +69,12 @@ func (s *SocketSink) Process(ctx context.Context, e *eventlogger.Event) (*eventl defer s.socketLock.Unlock() if e == nil { - return nil, fmt.Errorf("%s: event is nil: %w", op, ErrInvalidParameter) + return nil, fmt.Errorf("event is nil: %w", ErrInvalidParameter) } formatted, found := e.Format(s.requiredFormat) if !found { - return nil, fmt.Errorf("%s: unable to retrieve event formatted as %q", op, s.requiredFormat) + return nil, fmt.Errorf("unable to retrieve event formatted as %q: %w", s.requiredFormat, ErrInvalidParameter) } // Try writing and return early if successful. @@ -99,7 +95,7 @@ func (s *SocketSink) Process(ctx context.Context, e *eventlogger.Event) (*eventl // Format the error nicely if we need to return one. if err != nil { - err = fmt.Errorf("%s: error writing to socket: %w", op, err) + err = fmt.Errorf("error writing to socket %q: %w", s.address, err) } // return nil for the event to indicate the pipeline is complete. @@ -108,14 +104,12 @@ func (s *SocketSink) Process(ctx context.Context, e *eventlogger.Event) (*eventl // Reopen handles reopening the connection for the socket sink. func (s *SocketSink) Reopen() error { - const op = "event.(SocketSink).Reopen" - s.socketLock.Lock() defer s.socketLock.Unlock() err := s.reconnect(nil) if err != nil { - return fmt.Errorf("%s: error reconnecting: %w", op, err) + return fmt.Errorf("error reconnecting %q: %w", s.address, err) } return nil @@ -128,8 +122,6 @@ func (_ *SocketSink) Type() eventlogger.NodeType { // connect attempts to establish a connection using the socketType and address. func (s *SocketSink) connect(ctx context.Context) error { - const op = "event.(SocketSink).connect" - // If we're already connected, we should have disconnected first. if s.connection != nil { return nil @@ -141,7 +133,7 @@ func (s *SocketSink) connect(ctx context.Context) error { dialer := net.Dialer{} conn, err := dialer.DialContext(timeoutContext, s.socketType, s.address) if err != nil { - return fmt.Errorf("%s: error connecting to %q address %q: %w", op, s.socketType, s.address, err) + return fmt.Errorf("error connecting to %q address %q: %w", s.socketType, s.address, err) } s.connection = conn @@ -151,8 +143,6 @@ func (s *SocketSink) connect(ctx context.Context) error { // disconnect attempts to close and clear an existing connection. func (s *SocketSink) disconnect() error { - const op = "event.(SocketSink).disconnect" - // If we're already disconnected, we can return early. if s.connection == nil { return nil @@ -160,7 +150,7 @@ func (s *SocketSink) disconnect() error { err := s.connection.Close() if err != nil { - return fmt.Errorf("%s: error closing connection: %w", op, err) + return fmt.Errorf("error closing connection to %q address %q: %w", s.socketType, s.address, err) } s.connection = nil @@ -169,16 +159,14 @@ func (s *SocketSink) disconnect() error { // reconnect attempts to disconnect and then connect to the configured socketType and address. func (s *SocketSink) reconnect(ctx context.Context) error { - const op = "event.(SocketSink).reconnect" - err := s.disconnect() if err != nil { - return fmt.Errorf("%s: error disconnecting: %w", op, err) + return err } err = s.connect(ctx) if err != nil { - return fmt.Errorf("%s: error connecting: %w", op, err) + return err } return nil @@ -186,22 +174,20 @@ func (s *SocketSink) reconnect(ctx context.Context) error { // write attempts to write the specified data using the established connection. func (s *SocketSink) write(ctx context.Context, data []byte) error { - const op = "event.(SocketSink).write" - // Ensure we're connected. err := s.connect(ctx) if err != nil { - return fmt.Errorf("%s: connection error: %w", op, err) + return err } err = s.connection.SetWriteDeadline(time.Now().Add(s.maxDuration)) if err != nil { - return fmt.Errorf("%s: unable to set write deadline: %w", op, err) + return fmt.Errorf("unable to set write deadline: %w", err) } _, err = s.connection.Write(data) if err != nil { - return fmt.Errorf("%s: unable to write to socket: %w", op, err) + return fmt.Errorf("unable to write to socket: %w", err) } return nil diff --git a/internal/observability/event/sink_socket_test.go b/internal/observability/event/sink_socket_test.go index c44766780dc3..f0685e52e461 100644 --- a/internal/observability/event/sink_socket_test.go +++ b/internal/observability/event/sink_socket_test.go @@ -26,31 +26,31 @@ func TestNewSocketSink(t *testing.T) { "address-empty": { address: "", wantErr: true, - expectedErrMsg: "event.NewSocketSink: address is required: invalid parameter", + expectedErrMsg: "address is required: invalid parameter", }, "address-whitespace": { address: " ", wantErr: true, - expectedErrMsg: "event.NewSocketSink: address is required: invalid parameter", + expectedErrMsg: "address is required: invalid parameter", }, "format-empty": { address: "addr", format: "", wantErr: true, - expectedErrMsg: "event.NewSocketSink: format is required: invalid parameter", + expectedErrMsg: "format is required: invalid parameter", }, "format-whitespace": { address: "addr", format: " ", wantErr: true, - expectedErrMsg: "event.NewSocketSink: format is required: invalid parameter", + expectedErrMsg: "format is required: invalid parameter", }, "bad-max-duration": { address: "addr", format: "json", opts: []Option{WithMaxDuration("bar")}, wantErr: true, - expectedErrMsg: "event.NewSocketSink: error applying options: unable to parse max duration: time: invalid duration \"bar\"", + expectedErrMsg: "unable to parse max duration: invalid parameter: 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 71782695c7a2..1c0508f80da6 100644 --- a/internal/observability/event/sink_stdout.go +++ b/internal/observability/event/sink_stdout.go @@ -23,11 +23,9 @@ type StdoutSink struct { // NewStdoutSinkNode creates a new StdoutSink that will persist the events // it processes using the specified expected format. func NewStdoutSinkNode(format string) (*StdoutSink, error) { - const op = "event.NewStdoutSinkNode" - format = strings.TrimSpace(format) if format == "" { - return nil, fmt.Errorf("%s: format is required: %w", op, ErrInvalidParameter) + return nil, fmt.Errorf("format is required: %w", ErrInvalidParameter) } return &StdoutSink{ @@ -37,8 +35,6 @@ func NewStdoutSinkNode(format string) (*StdoutSink, error) { // Process persists the provided eventlogger.Event to the standard output stream. func (s *StdoutSink) Process(ctx context.Context, e *eventlogger.Event) (*eventlogger.Event, error) { - const op = "event.(StdoutSink).Process" - select { case <-ctx.Done(): return nil, ctx.Err() @@ -46,17 +42,17 @@ func (s *StdoutSink) Process(ctx context.Context, e *eventlogger.Event) (*eventl } if e == nil { - return nil, fmt.Errorf("%s: event is nil: %w", op, ErrInvalidParameter) + return nil, fmt.Errorf("event is nil: %w", ErrInvalidParameter) } - formattedBytes, found := e.Format(s.requiredFormat) + formatted, found := e.Format(s.requiredFormat) if !found { - return nil, fmt.Errorf("%s: unable to retrieve event formatted as %q", op, s.requiredFormat) + return nil, fmt.Errorf("unable to retrieve event formatted as %q: %w", s.requiredFormat, ErrInvalidParameter) } - _, err := os.Stdout.Write(formattedBytes) + _, err := os.Stdout.Write(formatted) if err != nil { - return nil, fmt.Errorf("%s: error writing to stdout: %w", op, err) + return nil, fmt.Errorf("error writing to stdout: %w", err) } // Return nil, nil to indicate the pipeline is complete. diff --git a/internal/observability/event/sink_syslog.go b/internal/observability/event/sink_syslog.go index 7d0e359ffcef..6d6b6b6aee2f 100644 --- a/internal/observability/event/sink_syslog.go +++ b/internal/observability/event/sink_syslog.go @@ -23,21 +23,19 @@ type SyslogSink struct { // NewSyslogSink should be used to create a new SyslogSink. // Accepted options: WithFacility and WithTag. func NewSyslogSink(format string, opt ...Option) (*SyslogSink, error) { - const op = "event.NewSyslogSink" - format = strings.TrimSpace(format) if format == "" { - return nil, fmt.Errorf("%s: format is required: %w", op, ErrInvalidParameter) + return nil, fmt.Errorf("format is required: %w", ErrInvalidParameter) } opts, err := getOpts(opt...) if err != nil { - return nil, fmt.Errorf("%s: error applying options: %w", op, err) + return nil, err } logger, err := gsyslog.NewLogger(gsyslog.LOG_INFO, opts.withFacility, opts.withTag) if err != nil { - return nil, fmt.Errorf("%s: error creating syslogger: %w", op, err) + return nil, fmt.Errorf("error creating syslogger: %w", err) } return &SyslogSink{requiredFormat: format, logger: logger}, nil @@ -45,8 +43,6 @@ func NewSyslogSink(format string, opt ...Option) (*SyslogSink, error) { // Process handles writing the event to the syslog. func (s *SyslogSink) Process(ctx context.Context, e *eventlogger.Event) (*eventlogger.Event, error) { - const op = "event.(SyslogSink).Process" - select { case <-ctx.Done(): return nil, ctx.Err() @@ -54,17 +50,17 @@ func (s *SyslogSink) Process(ctx context.Context, e *eventlogger.Event) (*eventl } if e == nil { - return nil, fmt.Errorf("%s: event is nil: %w", op, ErrInvalidParameter) + return nil, fmt.Errorf("event is nil: %w", ErrInvalidParameter) } formatted, found := e.Format(s.requiredFormat) if !found { - return nil, fmt.Errorf("%s: unable to retrieve event formatted as %q", op, s.requiredFormat) + return nil, fmt.Errorf("unable to retrieve event formatted as %q: %w", s.requiredFormat, ErrInvalidParameter) } _, err := s.logger.Write(formatted) if err != nil { - return nil, fmt.Errorf("%s: error writing to syslog: %w", op, err) + return nil, fmt.Errorf("error writing to syslog: %w", err) } // return nil for the event to indicate the pipeline is complete. diff --git a/internal/observability/event/sink_syslog_test.go b/internal/observability/event/sink_syslog_test.go index f977a4a50538..519ae5197c6d 100644 --- a/internal/observability/event/sink_syslog_test.go +++ b/internal/observability/event/sink_syslog_test.go @@ -24,12 +24,12 @@ func TestNewSyslogSink(t *testing.T) { "format-empty": { format: "", wantErr: true, - expectedErrMsg: "event.NewSyslogSink: format is required: invalid parameter", + expectedErrMsg: "format is required: invalid parameter", }, "format-whitespace": { format: " ", wantErr: true, - expectedErrMsg: "event.NewSyslogSink: format is required: invalid parameter", + expectedErrMsg: "format is required: invalid parameter", }, "happy": { format: "json", diff --git a/sdk/go.sum b/sdk/go.sum index 00cd1eda0659..11d5a01744c2 100644 --- a/sdk/go.sum +++ b/sdk/go.sum @@ -473,6 +473,8 @@ github.com/rs/zerolog v1.15.0/go.mod h1:xYTKnLHcpfU2225ny5qZjxnj9NvkumZYjJHlAThC github.com/ryanuber/columnize v2.1.0+incompatible/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= github.com/ryanuber/go-glob v1.0.0 h1:iQh3xXAumdQ+4Ufa5b25cRpC5TYKlno6hsv6Cb3pkBk= github.com/ryanuber/go-glob v1.0.0/go.mod h1:807d1WSdnB0XRJzKNil9Om6lcp/3a0v4qIHxIXzX/Yc= +github.com/sasha-s/go-deadlock v0.2.0 h1:lMqc+fUb7RrFS3gQLtoQsJ7/6TV/pAIFvBsqX73DK8Y= +github.com/sasha-s/go-deadlock v0.2.0/go.mod h1:StQn567HiB1fF2yJ44N9au7wOhrPS3iZqiDbRupzT10= github.com/satori/go.uuid v1.2.0/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdhQKdks0= github.com/shopspring/decimal v0.0.0-20180709203117-cd690d0c9e24/go.mod h1:M+9NzErvs504Cn4c5DxATwIqPbtswREoFCre64PpcG4= github.com/shopspring/decimal v1.2.0 h1:abSATXmQEYyShuxI4/vyW3tV1MrKAJzCZ/0zLUXYbsQ= diff --git a/vault/audit.go b/vault/audit.go index f4429c592181..a039b3421ee9 100644 --- a/vault/audit.go +++ b/vault/audit.go @@ -65,6 +65,16 @@ func (c *Core) generateAuditTestProbe() (*logical.LogInput, error) { // enableAudit is used to enable a new audit backend func (c *Core) enableAudit(ctx context.Context, entry *MountEntry, updateStorage bool) error { + // Check ahead of time if the type of audit device we're trying to enable is configured in Vault. + if _, ok := c.auditBackends[entry.Type]; !ok { + return fmt.Errorf("unknown backend type: %q: %w", entry.Type, audit.ErrExternalOptions) + } + + // We can check early to ensure that non-Enterprise versions aren't trying to supply Enterprise only options. + if hasInvalidAuditOptions(entry.Options) { + return fmt.Errorf("enterprise-only options supplied: %w", audit.ErrExternalOptions) + } + // Ensure we end the path in a slash if !strings.HasSuffix(entry.Path, "/") { entry.Path += "/" @@ -72,18 +82,13 @@ func (c *Core) enableAudit(ctx context.Context, entry *MountEntry, updateStorage // Ensure there is a name if entry.Path == "/" { - return fmt.Errorf("backend path must be specified") - } - - // We can check early to ensure that non-Enterprise versions aren't trying to supply Enterprise only options. - if hasInvalidAuditOptions(entry.Options) { - return fmt.Errorf("enterprise-only options supplied") + return fmt.Errorf("backend path must be specified: %w", audit.ErrExternalOptions) } if fallbackRaw, ok := entry.Options["fallback"]; ok { fallback, err := parseutil.ParseBool(fallbackRaw) if err != nil { - return fmt.Errorf("unable to enable audit device '%s', cannot parse supplied 'fallback' setting: %w", entry.Path, err) + return fmt.Errorf("cannot parse supplied 'fallback' setting: %w", audit.ErrExternalOptions) } // Reassigning the fallback value means we can ensure that the formatting @@ -91,6 +96,17 @@ func (c *Core) enableAudit(ctx context.Context, entry *MountEntry, updateStorage entry.Options["fallback"] = strconv.FormatBool(fallback) } + if skipTestRaw, ok := entry.Options["skip_test"]; ok { + skipTest, err := parseutil.ParseBool(skipTestRaw) + if err != nil { + return fmt.Errorf("cannot parse supplied 'skip_test' setting: %w", audit.ErrExternalOptions) + } + + // Reassigning the value means we can ensure that the formatting + // of it as a string is consistent for future comparisons. + entry.Options["skip_test"] = strconv.FormatBool(skipTest) + } + // Update the audit table c.auditLock.Lock() defer c.auditLock.Unlock() @@ -99,13 +115,13 @@ func (c *Core) enableAudit(ctx context.Context, entry *MountEntry, updateStorage for _, ent := range c.audit.Entries { switch { case entry.Options["fallback"] == "true" && ent.Options["fallback"] == "true": - return fmt.Errorf("unable to enable audit device '%s', a fallback device already exists '%s'", entry.Path, ent.Path) + return fmt.Errorf("fallback device already exists '%s': %w", ent.Path, audit.ErrExternalOptions) // Existing is sql/mysql/ new is sql/ or // existing is sql/ and new is sql/mysql/ case strings.HasPrefix(ent.Path, entry.Path): fallthrough case strings.HasPrefix(entry.Path, ent.Path): - return fmt.Errorf("path already in use") + return fmt.Errorf("path already in use: %w", audit.ErrExternalOptions) } } @@ -113,14 +129,14 @@ func (c *Core) enableAudit(ctx context.Context, entry *MountEntry, updateStorage if entry.UUID == "" { entryUUID, err := uuid.GenerateUUID() if err != nil { - return err + return fmt.Errorf("%w: %w", audit.ErrInternal, err) } entry.UUID = entryUUID } if entry.Accessor == "" { accessor, err := c.generateMountAccessor("audit_" + entry.Type) if err != nil { - return err + return fmt.Errorf("%w: %w", audit.ErrInternal, err) } entry.Accessor = accessor } @@ -141,14 +157,14 @@ func (c *Core) enableAudit(ctx context.Context, entry *MountEntry, updateStorage return err } if backend == nil { - return fmt.Errorf("nil audit backend of type %q returned from factory", entry.Type) + return fmt.Errorf("nil audit backend of type %q returned from factory: %w", entry.Type, audit.ErrInternal) } if entry.Options["skip_test"] != "true" { // Test the new audit device and report failure if it doesn't work. testProbe, err := c.generateAuditTestProbe() if err != nil { - return err + return fmt.Errorf("error generating test probe: %w: %w", audit.ErrInternal, err) } err = backend.LogTestMessage(ctx, testProbe) if err != nil { @@ -163,14 +179,14 @@ func (c *Core) enableAudit(ctx context.Context, entry *MountEntry, updateStorage ns, err := namespace.FromContext(ctx) if err != nil { - return err + return fmt.Errorf("%w: %w", audit.ErrInternal, err) } entry.NamespaceID = ns.ID entry.namespace = ns if updateStorage { if err := c.persistAudit(ctx, newTable, entry.Local); err != nil { - return errors.New("failed to update audit table") + return fmt.Errorf("failed to update audit table: %w: %w", audit.ErrInternal, err) } } @@ -515,12 +531,12 @@ func (c *Core) removeAuditReloadFunc(entry *MountEntry) { func (c *Core) newAuditBackend(ctx context.Context, entry *MountEntry, view logical.Storage, conf map[string]string) (audit.Backend, error) { // Ensure that non-Enterprise versions aren't trying to supply Enterprise only options. if hasInvalidAuditOptions(entry.Options) { - return nil, fmt.Errorf("enterprise-only options supplied") + return nil, fmt.Errorf("enterprise-only options supplied: %w", audit.ErrInvalidParameter) } f, ok := c.auditBackends[entry.Type] if !ok { - return nil, fmt.Errorf("unknown backend type: %q", entry.Type) + return nil, fmt.Errorf("unknown backend type: %q: %w", entry.Type, audit.ErrInvalidParameter) } saltConfig := &salt.Config{ HMAC: sha256.New, @@ -542,7 +558,7 @@ func (c *Core) newAuditBackend(ctx context.Context, entry *MountEntry, view logi return nil, fmt.Errorf("unable to create new audit backend: %w", err) } if be == nil { - return nil, fmt.Errorf("nil backend returned from %q factory function", entry.Type) + return nil, fmt.Errorf("nil backend returned from %q factory function: %w", entry.Type, audit.ErrInternal) } switch entry.Type { diff --git a/vault/audit_broker.go b/vault/audit_broker.go index 4992a2410505..6d8ee6cd6808 100644 --- a/vault/audit_broker.go +++ b/vault/audit_broker.go @@ -7,6 +7,7 @@ import ( "context" "errors" "fmt" + "reflect" "strings" "sync" "time" @@ -50,17 +51,15 @@ type AuditBroker struct { // NewAuditBroker creates a new audit broker func NewAuditBroker(log hclog.Logger) (*AuditBroker, error) { - const op = "vault.NewAuditBroker" - eventBroker, err := eventlogger.NewBroker() if err != nil { - return nil, fmt.Errorf("%s: error creating event broker for audit events: %w", op, err) + return nil, fmt.Errorf("error creating event broker for audit events: %w", err) } // Set up the broker that will support a single fallback device. fallbackEventBroker, err := eventlogger.NewBroker() if err != nil { - return nil, fmt.Errorf("%s: error creating event fallback broker for audit event: %w", op, err) + return nil, fmt.Errorf("error creating event fallback broker for audit event: %w", err) } broker := &AuditBroker{ @@ -75,45 +74,54 @@ func NewAuditBroker(log hclog.Logger) (*AuditBroker, error) { // Register is used to add new audit backend to the broker func (a *AuditBroker) Register(name string, backend audit.Backend, local bool) error { - const op = "vault.(AuditBroker).Register" - a.Lock() defer a.Unlock() name = strings.TrimSpace(name) if name == "" { - return fmt.Errorf("%s: name is required: %w", op, event.ErrInvalidParameter) + return fmt.Errorf("name is required: %w", audit.ErrInvalidParameter) + } + + if backend == nil || reflect.ValueOf(backend).IsNil() { + return fmt.Errorf("backend cannot be nil: %w", audit.ErrInvalidParameter) } // If the backend is already registered, we cannot re-register it. if a.isRegistered(name) { - return fmt.Errorf("%s: backend already registered '%s'", op, name) + return fmt.Errorf("backend already registered '%s': %w", name, audit.ErrExternalOptions) } // Fallback devices are singleton instances, we cannot register more than one or overwrite the existing one. - if backend.IsFallback() && a.fallbackBroker.IsAnyPipelineRegistered(eventlogger.EventType(event.AuditType.String())) { - existing, err := a.existingFallbackName() - if err != nil { - return fmt.Errorf("%s: existing fallback device already registered: %w", op, err) + if backend.IsFallback() && hasAuditPipelines(a.fallbackBroker) { + // Get the name of the fallback device which is registered with the broker. + var existing string + for _, be := range a.backends { + if be.backend.IsFallback() { + existing = be.backend.Name() + } + } + if existing == "" { + // We expected an existing fallback device but didn't find it. + return fmt.Errorf("cannot determine name of existing registered fallback device: %w", audit.ErrInternal) } - return fmt.Errorf("%s: existing fallback device already registered: %q", op, existing) + return fmt.Errorf("existing fallback device already registered %q: %w", existing, audit.ErrInvalidParameter) } if name != backend.Name() { - return fmt.Errorf("%s: audit registration failed due to device name mismatch: %q, %q", op, name, backend.Name()) + return fmt.Errorf("audit registration failed due to device name mismatch: %q, %q: %w", name, backend.Name(), audit.ErrInternal) } switch { case backend.IsFallback(): err := a.registerFallback(name, backend) if err != nil { - return fmt.Errorf("%s: unable to register fallback device for %q: %w", op, name, err) + return fmt.Errorf("unable to register fallback device for %q: %w: %w", name, err, audit.ErrInternal) } default: err := a.register(name, backend) if err != nil { - return fmt.Errorf("%s: unable to register device for %q: %w", op, name, err) + return fmt.Errorf("unable to register device for %q: %w", name, err) } } @@ -127,14 +135,12 @@ func (a *AuditBroker) Register(name string, backend audit.Backend, local bool) e // Deregister is used to remove an audit backend from the broker func (a *AuditBroker) Deregister(ctx context.Context, name string) error { - const op = "vault.(AuditBroker).Deregister" - a.Lock() defer a.Unlock() name = strings.TrimSpace(name) if name == "" { - return fmt.Errorf("%s: name is required: %w", op, event.ErrInvalidParameter) + return fmt.Errorf("name is required: %w", audit.ErrInvalidParameter) } // If the backend isn't actually registered, then there's nothing to do. @@ -148,17 +154,16 @@ func (a *AuditBroker) Deregister(ctx context.Context, name string) error { // the error. delete(a.backends, name) + var err error switch { case name == a.fallbackName: - err := a.deregisterFallback(ctx, name) - if err != nil { - return fmt.Errorf("%s: deregistration failed for fallback audit device %q: %w", op, name, err) - } + err = a.deregisterFallback(ctx, name) default: - err := a.deregister(ctx, name) - if err != nil { - return fmt.Errorf("%s: deregistration failed for audit device %q: %w", op, name, err) - } + err = a.deregister(ctx, name) + } + + if err != nil { + return fmt.Errorf("deregistration failed for audit device %q: %w: %w", name, err, audit.ErrInternal) } return nil @@ -239,8 +244,8 @@ func (a *AuditBroker) LogRequest(ctx context.Context, in *logical.LogInput) (ret // normal audit devices, so check if the broker had an audit based pipeline // registered before trying to send to it. var status eventlogger.Status - if a.broker.IsAnyPipelineRegistered(eventlogger.EventType(event.AuditType.String())) { - status, err = a.broker.Send(ctx, eventlogger.EventType(event.AuditType.String()), e) + if hasAuditPipelines(a.broker) { + status, err = a.broker.Send(ctx, event.AuditType.AsEventType(), e) if err != nil { retErr = multierror.Append(retErr, multierror.Append(err, status.Warnings...)) return retErr.ErrorOrNil() @@ -260,8 +265,8 @@ func (a *AuditBroker) LogRequest(ctx context.Context, in *logical.LogInput) (ret // If a fallback device is registered we can rely on that to 'catch all' // and also the broker level guarantee for completed sinks. - if a.fallbackBroker.IsAnyPipelineRegistered(eventlogger.EventType(event.AuditType.String())) { - status, err = a.fallbackBroker.Send(ctx, eventlogger.EventType(event.AuditType.String()), e) + if a.fallbackBroker.IsAnyPipelineRegistered(event.AuditType.AsEventType()) { + status, err = a.fallbackBroker.Send(ctx, event.AuditType.AsEventType(), e) if err != nil { retErr = multierror.Append(retErr, multierror.Append(fmt.Errorf("auditing request to fallback device failed: %w", err), status.Warnings...)) } @@ -323,8 +328,8 @@ func (a *AuditBroker) LogResponse(ctx context.Context, in *logical.LogInput) (re // normal audit devices, so check if the broker had an audit based pipeline // registered before trying to send to it. var status eventlogger.Status - if a.broker.IsAnyPipelineRegistered(eventlogger.EventType(event.AuditType.String())) { - status, err = a.broker.Send(auditContext, eventlogger.EventType(event.AuditType.String()), e) + if a.broker.IsAnyPipelineRegistered(event.AuditType.AsEventType()) { + status, err = a.broker.Send(auditContext, event.AuditType.AsEventType(), e) if err != nil { retErr = multierror.Append(retErr, multierror.Append(err, status.Warnings...)) return retErr.ErrorOrNil() @@ -344,8 +349,8 @@ func (a *AuditBroker) LogResponse(ctx context.Context, in *logical.LogInput) (re // If a fallback device is registered we can rely on that to 'catch all' // and also the broker level guarantee for completed sinks. - if a.fallbackBroker.IsAnyPipelineRegistered(eventlogger.EventType(event.AuditType.String())) { - status, err = a.fallbackBroker.Send(auditContext, eventlogger.EventType(event.AuditType.String()), e) + if a.fallbackBroker.IsAnyPipelineRegistered(event.AuditType.AsEventType()) { + status, err = a.fallbackBroker.Send(auditContext, event.AuditType.AsEventType(), e) if err != nil { retErr = multierror.Append(retErr, multierror.Append(fmt.Errorf("auditing response to fallback device failed: %w", err), status.Warnings...)) } @@ -400,12 +405,10 @@ func (a *AuditBroker) requiredSuccessThresholdSinks() int { // backend's name, on the specified eventlogger.Broker using the audit.Backend // to supply them. func registerNodesAndPipeline(broker *eventlogger.Broker, b audit.Backend) error { - const op = "vault.registerNodesAndPipeline" - for id, node := range b.Nodes() { err := broker.RegisterNode(id, node) if err != nil { - return fmt.Errorf("%s: unable to register nodes for %q: %w", op, b.Name(), err) + return fmt.Errorf("unable to register nodes for %q: %w", b.Name(), err) } } @@ -417,44 +420,27 @@ func registerNodesAndPipeline(broker *eventlogger.Broker, b audit.Backend) error err := broker.RegisterPipeline(pipeline) if err != nil { - return fmt.Errorf("%s: unable to register pipeline for %q: %w", op, b.Name(), err) + return fmt.Errorf("unable to register pipeline for %q: %w", b.Name(), err) } return nil } -// existingFallbackName returns the name of the fallback device which is registered -// with the AuditBroker. -func (a *AuditBroker) existingFallbackName() (string, error) { - const op = "vault.(AuditBroker).existingFallbackName" - - for _, be := range a.backends { - if be.backend.IsFallback() { - return be.backend.Name(), nil - } - } - - return "", fmt.Errorf("%s: existing fallback device name is missing", op) -} - // registerFallback can be used to register a fallback device, it will also // configure the success threshold required for sinks. func (a *AuditBroker) registerFallback(name string, backend audit.Backend) error { - const op = "vault.(AuditBroker).registerFallback" - err := registerNodesAndPipeline(a.fallbackBroker, backend) if err != nil { - return fmt.Errorf("%s: fallback device pipeline registration error: %w", op, err) + return fmt.Errorf("pipeline registration error for fallback device: %w", err) } // Store the name of the fallback audit device so that we can check when // deregistering if the device is the single fallback one. a.fallbackName = backend.Name() - // We need to turn on the threshold for the fallback broker, so we can - // guarantee it ends up somewhere - err = a.fallbackBroker.SetSuccessThresholdSinks(eventlogger.EventType(event.AuditType.String()), 1) + // We need to turn on the threshold for the fallback broker, so we can guarantee it ends up somewhere + err = a.fallbackBroker.SetSuccessThresholdSinks(event.AuditType.AsEventType(), 1) if err != nil { - return fmt.Errorf("%s: unable to configure fallback sink success threshold (1) for %q: %w", op, name, err) + return fmt.Errorf("unable to configure fallback sink success threshold (1): %w", err) } return nil @@ -463,16 +449,14 @@ func (a *AuditBroker) registerFallback(name string, backend audit.Backend) error // deregisterFallback can be used to deregister a fallback audit device, it will // also configure the success threshold required for sinks. func (a *AuditBroker) deregisterFallback(ctx context.Context, name string) error { - const op = "vault.(AuditBroker).deregisterFallback" - - err := a.fallbackBroker.SetSuccessThresholdSinks(eventlogger.EventType(event.AuditType.String()), 0) + err := a.fallbackBroker.SetSuccessThresholdSinks(event.AuditType.AsEventType(), 0) if err != nil { - return fmt.Errorf("%s: unable to configure fallback sink success threshold (0) for %q: %w", op, name, err) + return fmt.Errorf("unable to reconfigure fallback sink success threshold (0): %w", err) } - _, err = a.fallbackBroker.RemovePipelineAndNodes(ctx, eventlogger.EventType(event.AuditType.String()), eventlogger.PipelineID(name)) + _, err = a.fallbackBroker.RemovePipelineAndNodes(ctx, event.AuditType.AsEventType(), eventlogger.PipelineID(name)) if err != nil { - return fmt.Errorf("%s: unable to deregister fallback device %q: %w", op, name, err) + return fmt.Errorf("unable to deregister fallback device: %w", err) } // Clear the fallback device name now we've deregistered. @@ -484,11 +468,9 @@ func (a *AuditBroker) deregisterFallback(ctx context.Context, name string) error // register can be used to register a normal audit device, it will also calculate // and configure the success threshold required for sinks. func (a *AuditBroker) register(name string, backend audit.Backend) error { - const op = "vault.(AuditBroker).register" - err := registerNodesAndPipeline(a.broker, backend) if err != nil { - return fmt.Errorf("%s: audit pipeline registration error: %w", op, err) + return fmt.Errorf("audit pipeline registration error: %w", err) } // Establish if we ONLY have pipelines that include filter nodes. @@ -503,9 +485,9 @@ func (a *AuditBroker) register(name string, backend audit.Backend) error { } // Update the success threshold now that the pipeline is registered. - err = a.broker.SetSuccessThresholdSinks(eventlogger.EventType(event.AuditType.String()), threshold) + err = a.broker.SetSuccessThresholdSinks(event.AuditType.AsEventType(), threshold) if err != nil { - return fmt.Errorf("%s: unable to configure sink success threshold (%d) for %q: %w", op, threshold, name, err) + return fmt.Errorf("unable to configure sink success threshold (%d): %w", threshold, err) } return nil @@ -514,25 +496,29 @@ func (a *AuditBroker) register(name string, backend audit.Backend) error { // deregister can be used to deregister a normal audit device, it will also // calculate and configure the success threshold required for sinks. func (a *AuditBroker) deregister(ctx context.Context, name string) error { - const op = "vault.(AuditBroker).deregister" - // Establish if we ONLY have pipelines that include filter nodes. // Otherwise, we can rely on the eventlogger broker guarantee. threshold := a.requiredSuccessThresholdSinks() - err := a.broker.SetSuccessThresholdSinks(eventlogger.EventType(event.AuditType.String()), threshold) + err := a.broker.SetSuccessThresholdSinks(event.AuditType.AsEventType(), threshold) if err != nil { - return fmt.Errorf("%s: unable to configure sink success threshold (%d) for %q: %w", op, threshold, name, err) + return fmt.Errorf("unable to reconfigure sink success threshold (%d): %w", threshold, err) } // The first return value, a bool, indicates whether // RemovePipelineAndNodes encountered the error while evaluating // pre-conditions (false) or once it started removing the pipeline and // the nodes (true). This code doesn't care either way. - _, err = a.broker.RemovePipelineAndNodes(ctx, eventlogger.EventType(event.AuditType.String()), eventlogger.PipelineID(name)) + _, err = a.broker.RemovePipelineAndNodes(ctx, event.AuditType.AsEventType(), eventlogger.PipelineID(name)) if err != nil { - return fmt.Errorf("%s: unable to remove pipeline and nodes for %q: %w", op, name, err) + return fmt.Errorf("unable to remove pipeline and nodes: %w", err) } return nil } + +// hasAuditPipelines can be used as a shorthand to check if a broker has any +// registered pipelines that are for the audit event type. +func hasAuditPipelines(broker *eventlogger.Broker) bool { + return broker.IsAnyPipelineRegistered(event.AuditType.AsEventType()) +} diff --git a/vault/audit_broker_test.go b/vault/audit_broker_test.go index 289fb246265c..4626353b4115 100644 --- a/vault/audit_broker_test.go +++ b/vault/audit_broker_test.go @@ -89,7 +89,7 @@ func TestAuditBroker_Register_MultipleFails(t *testing.T) { err = a.Register(path, noFilterBackend, false) require.Error(t, err) - require.EqualError(t, err, "vault.(AuditBroker).Register: backend already registered 'b2-no-filter'") + require.EqualError(t, err, "backend already registered 'b2-no-filter': invalid configuration") } // BenchmarkAuditBroker_File_Request_DevNull Attempts to register a single `file` diff --git a/vault/logical_system.go b/vault/logical_system.go index a2235ad94312..87595083b2c6 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -32,6 +32,7 @@ import ( "github.com/hashicorp/go-secure-stdlib/parseutil" "github.com/hashicorp/go-secure-stdlib/strutil" semver "github.com/hashicorp/go-version" + "github.com/hashicorp/vault/audit" "github.com/hashicorp/vault/helper/experiments" "github.com/hashicorp/vault/helper/hostutil" "github.com/hashicorp/vault/helper/identity" @@ -3865,7 +3866,7 @@ func (b *SystemBackend) handleAuditHash(ctx context.Context, req *logical.Reques } // handleEnableAudit is used to enable a new audit backend -func (b *SystemBackend) handleEnableAudit(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { +func (b *SystemBackend) handleEnableAudit(ctx context.Context, _ *logical.Request, data *framework.FieldData) (*logical.Response, error) { repState := b.Core.ReplicationState() local := data.Get("local").(bool) @@ -3895,13 +3896,14 @@ func (b *SystemBackend) handleEnableAudit(ctx context.Context, req *logical.Requ // Attempt enabling if err := b.Core.enableAudit(ctx, me, true); err != nil { b.Backend.Logger().Error("enable audit mount failed", "path", me.Path, "error", err) - return handleError(err) + + return handleError(audit.ConvertToExternalError(err)) } return nil, nil } // handleDisableAudit is used to disable an audit backend -func (b *SystemBackend) handleDisableAudit(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { +func (b *SystemBackend) handleDisableAudit(ctx context.Context, _ *logical.Request, data *framework.FieldData) (*logical.Response, error) { path := data.Get("path").(string) if !strings.HasSuffix(path, "/") { @@ -3936,7 +3938,8 @@ func (b *SystemBackend) handleDisableAudit(ctx context.Context, req *logical.Req // Attempt disable if existed, err := b.Core.disableAudit(ctx, path, true); existed && err != nil { b.Backend.Logger().Error("disable audit mount failed", "path", path, "error", err) - return handleError(err) + + return handleError(audit.ConvertToExternalError(err)) } return nil, nil } diff --git a/vault/logical_system_test.go b/vault/logical_system_test.go index 9a32d4029631..8acf4d7799cc 100644 --- a/vault/logical_system_test.go +++ b/vault/logical_system_test.go @@ -2792,7 +2792,7 @@ func TestSystemBackend_enableAudit_invalid(t *testing.T) { if err != logical.ErrInvalidRequest { t.Fatalf("err: %v", err) } - if resp.Data["error"] != `unknown backend type: "nope"` { + if resp.Data["error"] != "unknown backend type: \"nope\": invalid configuration" { t.Fatalf("bad: %v", resp) } }