From 5fc9456f9c73c613a65a8e6343f2411321f71259 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Tue, 2 Apr 2024 19:35:18 +0100 Subject: [PATCH 1/6] Check Enterprise unseal order for audit funcs, enforce header formatter in audit entry formatter node --- audit/entry_formatter.go | 61 +++++----- audit/entry_formatter_test.go | 107 +++++++++++++++--- audit/options.go | 32 ++---- audit/options_test.go | 51 --------- audit/types.go | 6 + builtin/audit/file/backend.go | 25 ++-- .../audit/file/backend_filter_node_test.go | 2 +- builtin/audit/file/backend_test.go | 62 +++++++--- builtin/audit/socket/backend.go | 35 +++--- .../audit/socket/backend_filter_node_test.go | 2 +- builtin/audit/socket/backend_test.go | 54 +++++++-- builtin/audit/syslog/backend.go | 25 ++-- .../audit/syslog/backend_filter_node_test.go | 2 +- builtin/audit/syslog/backend_test.go | 54 +++++++-- helper/testhelpers/corehelpers/corehelpers.go | 12 +- http/logical_test.go | 5 +- vault/audit_test.go | 29 +++-- vault/audited_headers.go | 6 + vault/core_test.go | 6 +- 19 files changed, 351 insertions(+), 225 deletions(-) diff --git a/audit/entry_formatter.go b/audit/entry_formatter.go index 607b5bf6f03b..289fde83fa7d 100644 --- a/audit/entry_formatter.go +++ b/audit/entry_formatter.go @@ -38,17 +38,14 @@ type timeProvider interface { // EntryFormatter should be used to format audit requests and responses. type EntryFormatter struct { - config FormatterConfig - salter Salter - logger hclog.Logger - headerFormatter HeaderFormatter - name string - prefix string + config FormatterConfig + salter Salter + logger hclog.Logger + name string } // NewEntryFormatter should be used to create an EntryFormatter. -// Accepted options: WithHeaderFormatter, WithPrefix. -func NewEntryFormatter(name string, config FormatterConfig, salter Salter, logger hclog.Logger, opt ...Option) (*EntryFormatter, error) { +func NewEntryFormatter(name string, config FormatterConfig, salter Salter, logger hclog.Logger) (*EntryFormatter, error) { const op = "audit.NewEntryFormatter" name = strings.TrimSpace(name) @@ -69,18 +66,11 @@ func NewEntryFormatter(name string, config FormatterConfig, salter Salter, logge return nil, fmt.Errorf("%s: format not valid: %w", op, err) } - opts, err := getOpts(opt...) - if err != nil { - return nil, fmt.Errorf("%s: error applying options: %w", op, err) - } - return &EntryFormatter{ - config: config, - salter: salter, - logger: logger, - headerFormatter: opts.withHeaderFormatter, - name: name, - prefix: opts.withPrefix, + config: config, + salter: salter, + logger: logger, + name: name, }, nil } @@ -145,11 +135,14 @@ func (f *EntryFormatter) Process(ctx context.Context, e *eventlogger.Event) (_ * return nil, fmt.Errorf("%s: unable to copy audit event data: %w", op, err) } - // Ensure that any headers in the request, are formatted as required, and are - // only present if they have been configured to appear in the audit log. - // e.g. via: /sys/config/auditing/request-headers/:name - if f.headerFormatter != nil && data.Request != nil && data.Request.Headers != nil { - data.Request.Headers, err = f.headerFormatter.ApplyConfig(ctx, data.Request.Headers, f.salter) + // If the request is present in the input data, apply header configuration + // regardless. We shouldn't be in a situation where the header formatter isn't + // present as it's required. + if data.Request != nil { + // Ensure that any headers in the request, are formatted as required, and are + // only present if they have been configured to appear in the audit log. + // 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) } @@ -198,8 +191,8 @@ func (f *EntryFormatter) Process(ctx context.Context, e *eventlogger.Event) (_ * // don't support a prefix just sitting there. // However, this would be a breaking change to how Vault currently works to // include the prefix as part of the JSON object or XML document. - if f.prefix != "" { - result = append([]byte(f.prefix), result...) + if f.config.Prefix != "" { + result = append([]byte(f.config.Prefix), result...) } // Copy some properties from the event (and audit event) and store the @@ -577,19 +570,25 @@ func (f *EntryFormatter) FormatResponse(ctx context.Context, in *logical.LogInpu } // NewFormatterConfig should be used to create a FormatterConfig. -// Accepted options: WithElision, WithHMACAccessor, WithOmitTime, WithRaw, WithFormat. -func NewFormatterConfig(opt ...Option) (FormatterConfig, error) { +// 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 @@ -663,10 +662,8 @@ func doElideListResponseData(data map[string]interface{}) { // newTemporaryEntryFormatter creates a cloned EntryFormatter instance with a non-persistent Salter. func newTemporaryEntryFormatter(n *EntryFormatter) *EntryFormatter { return &EntryFormatter{ - salter: &nonPersistentSalt{}, - headerFormatter: n.headerFormatter, - config: n.config, - prefix: n.prefix, + salter: &nonPersistentSalt{}, + config: n.config, } } diff --git a/audit/entry_formatter_test.go b/audit/entry_formatter_test.go index b8b937d6555a..2b90a4409dc4 100644 --- a/audit/entry_formatter_test.go +++ b/audit/entry_formatter_test.go @@ -59,6 +59,24 @@ const testFormatJSONReqBasicStrFmt = ` } ` +// testHeaderFormatter is a stub to prevent the need to import the vault package +// to bring in vault.AuditedHeadersConfig for testing. +type testHeaderFormatter struct { + shouldReturnEmpty bool +} + +// ApplyConfig satisfies the HeaderFormatter interface for testing. +// It will either return the headers it was supplied or empty headers depending +// on how it is configured. +// ignore-nil-nil-function-check. +func (f *testHeaderFormatter) ApplyConfig(_ context.Context, headers map[string][]string, salter Salter) (result map[string][]string, retErr error) { + if f.shouldReturnEmpty { + return make(map[string][]string), nil + } + + return headers, nil +} + // testTimeProvider is just a test struct used to imitate an AuditEvent's ability // to provide a formatted time. type testTimeProvider struct{} @@ -178,9 +196,9 @@ func TestNewEntryFormatter(t *testing.T) { ss = newStaticSalt(t) } - cfg, err := NewFormatterConfig(tc.Options...) + cfg, err := NewFormatterConfig(&testHeaderFormatter{}, tc.Options...) require.NoError(t, err) - f, err := NewEntryFormatter(tc.Name, cfg, ss, tc.Logger, tc.Options...) + f, err := NewEntryFormatter(tc.Name, cfg, ss, tc.Logger /*, tc.Options...*/) switch { case tc.IsErrorExpected: @@ -191,7 +209,7 @@ func TestNewEntryFormatter(t *testing.T) { require.NoError(t, err) require.NotNil(t, f) require.Equal(t, tc.ExpectedFormat, f.config.RequiredFormat) - require.Equal(t, tc.ExpectedPrefix, f.prefix) + require.Equal(t, tc.ExpectedPrefix, f.config.Prefix) } }) } @@ -202,7 +220,7 @@ func TestEntryFormatter_Reopen(t *testing.T) { t.Parallel() ss := newStaticSalt(t) - cfg, err := NewFormatterConfig() + cfg, err := NewFormatterConfig(&testHeaderFormatter{}) require.NoError(t, err) f, err := NewEntryFormatter("juan", cfg, ss, hclog.NewNullLogger()) @@ -216,7 +234,7 @@ func TestEntryFormatter_Type(t *testing.T) { t.Parallel() ss := newStaticSalt(t) - cfg, err := NewFormatterConfig() + cfg, err := NewFormatterConfig(&testHeaderFormatter{}) require.NoError(t, err) f, err := NewEntryFormatter("juan", cfg, ss, hclog.NewNullLogger()) @@ -361,7 +379,7 @@ func TestEntryFormatter_Process(t *testing.T) { require.NotNil(t, e) ss := newStaticSalt(t) - cfg, err := NewFormatterConfig(WithFormat(tc.RequiredFormat.String())) + cfg, err := NewFormatterConfig(&testHeaderFormatter{}, WithFormat(tc.RequiredFormat.String())) require.NoError(t, err) f, err := NewEntryFormatter("juan", cfg, ss, hclog.NewNullLogger()) @@ -426,7 +444,7 @@ func BenchmarkAuditFileSink_Process(b *testing.B) { ctx := namespace.RootContext(context.Background()) // Create the formatter node. - cfg, err := NewFormatterConfig() + cfg, err := NewFormatterConfig(&testHeaderFormatter{}) require.NoError(b, err) ss := newStaticSalt(b) formatter, err := NewEntryFormatter("juan", cfg, ss, hclog.NewNullLogger()) @@ -504,7 +522,7 @@ func TestEntryFormatter_FormatRequest(t *testing.T) { t.Parallel() ss := newStaticSalt(t) - cfg, err := NewFormatterConfig(WithOmitTime(tc.ShouldOmitTime)) + cfg, err := NewFormatterConfig(&testHeaderFormatter{}, WithOmitTime(tc.ShouldOmitTime)) require.NoError(t, err) f, err := NewEntryFormatter("juan", cfg, ss, hclog.NewNullLogger()) require.NoError(t, err) @@ -586,7 +604,7 @@ func TestEntryFormatter_FormatResponse(t *testing.T) { t.Parallel() ss := newStaticSalt(t) - cfg, err := NewFormatterConfig(WithOmitTime(tc.ShouldOmitTime)) + cfg, err := NewFormatterConfig(&testHeaderFormatter{}, WithOmitTime(tc.ShouldOmitTime)) require.NoError(t, err) f, err := NewEntryFormatter("juan", cfg, ss, hclog.NewNullLogger()) require.NoError(t, err) @@ -702,9 +720,9 @@ func TestEntryFormatter_Process_JSON(t *testing.T) { } for name, tc := range cases { - cfg, err := NewFormatterConfig(WithHMACAccessor(false)) + cfg, err := NewFormatterConfig(&testHeaderFormatter{}, WithHMACAccessor(false), WithPrefix(tc.Prefix)) require.NoError(t, err) - formatter, err := NewEntryFormatter("juan", cfg, ss, hclog.NewNullLogger(), WithPrefix(tc.Prefix)) + formatter, err := NewEntryFormatter("juan", cfg, ss, hclog.NewNullLogger()) require.NoError(t, err) in := &logical.LogInput{ @@ -860,12 +878,14 @@ func TestEntryFormatter_Process_JSONx(t *testing.T) { for name, tc := range cases { cfg, err := NewFormatterConfig( + &testHeaderFormatter{}, WithOmitTime(true), WithHMACAccessor(false), WithFormat(JSONxFormat.String()), + WithPrefix(tc.Prefix), ) require.NoError(t, err) - formatter, err := NewEntryFormatter("juan", cfg, tempStaticSalt, hclog.NewNullLogger(), WithPrefix(tc.Prefix)) + formatter, err := NewEntryFormatter("juan", cfg, tempStaticSalt, hclog.NewNullLogger()) require.NoError(t, err) require.NotNil(t, formatter) @@ -997,7 +1017,7 @@ func TestEntryFormatter_FormatResponse_ElideListResponses(t *testing.T) { } t.Run("Default case", func(t *testing.T) { - config, err := NewFormatterConfig(WithElision(true)) + config, err := NewFormatterConfig(&testHeaderFormatter{}, WithElision(true)) require.NoError(t, err) for name, tc := range tests { name := name @@ -1010,7 +1030,7 @@ func TestEntryFormatter_FormatResponse_ElideListResponses(t *testing.T) { }) t.Run("When Operation is not list, eliding does not happen", func(t *testing.T) { - config, err := NewFormatterConfig(WithElision(true)) + config, err := NewFormatterConfig(&testHeaderFormatter{}, WithElision(true)) require.NoError(t, err) tc := oneInterestingTestCase entry := format(t, config, logical.ReadOperation, tc.inputData) @@ -1018,7 +1038,7 @@ func TestEntryFormatter_FormatResponse_ElideListResponses(t *testing.T) { }) t.Run("When ElideListResponses is false, eliding does not happen", func(t *testing.T) { - config, err := NewFormatterConfig(WithElision(false), WithFormat(JSONFormat.String())) + config, err := NewFormatterConfig(&testHeaderFormatter{}, WithElision(false), WithFormat(JSONFormat.String())) require.NoError(t, err) tc := oneInterestingTestCase entry := format(t, config, logical.ListOperation, tc.inputData) @@ -1026,7 +1046,7 @@ func TestEntryFormatter_FormatResponse_ElideListResponses(t *testing.T) { }) t.Run("When Raw is true, eliding still happens", func(t *testing.T) { - config, err := NewFormatterConfig(WithElision(true), WithRaw(true), WithFormat(JSONFormat.String())) + config, err := NewFormatterConfig(&testHeaderFormatter{}, WithElision(true), WithRaw(true), WithFormat(JSONFormat.String())) require.NoError(t, err) tc := oneInterestingTestCase entry := format(t, config, logical.ListOperation, tc.inputData) @@ -1040,7 +1060,7 @@ func TestEntryFormatter_Process_NoMutation(t *testing.T) { t.Parallel() // Create the formatter node. - cfg, err := NewFormatterConfig() + cfg, err := NewFormatterConfig(&testHeaderFormatter{}) require.NoError(t, err) ss := newStaticSalt(t) formatter, err := NewEntryFormatter("juan", cfg, ss, hclog.NewNullLogger()) @@ -1100,7 +1120,7 @@ func TestEntryFormatter_Process_Panic(t *testing.T) { t.Parallel() // Create the formatter node. - cfg, err := NewFormatterConfig() + cfg, err := NewFormatterConfig(&testHeaderFormatter{}) require.NoError(t, err) ss := newStaticSalt(t) formatter, err := NewEntryFormatter("juan", cfg, ss, hclog.NewNullLogger()) @@ -1153,6 +1173,57 @@ func TestEntryFormatter_Process_Panic(t *testing.T) { require.Nil(t, e2) } +// TODO: PW: DOCS +func TestEntryFormatter_NewFormatterConfig_NilHeaderFormatter(t *testing.T) { + _, err := NewFormatterConfig(nil) + require.Error(t, err) + + // TODO: PW: Test applying various options. +} + +// TODO: PW: DOCS +func TestEntryFormatter_Process_NeverLeaksHeaders(t *testing.T) { + t.Parallel() + + // Create the formatter node. + cfg, err := NewFormatterConfig(&testHeaderFormatter{shouldReturnEmpty: true}) + require.NoError(t, err) + ss := newStaticSalt(t) + + // Create a formatter with no header formatting config. + formatter, err := NewEntryFormatter("juan", cfg, ss, hclog.NewNullLogger()) + require.NoError(t, err) + require.NotNil(t, formatter) + + // Set up the input and verify we have a single foo:bar header. + var input *logical.LogInput + err = json.Unmarshal([]byte(testFormatJSONReqBasicStrFmt), &input) + require.NoError(t, err) + require.NotNil(t, input) + require.Len(t, input.Request.Headers, 1) + require.Len(t, input.Request.Headers["foo"], 1) + require.Equal(t, "bar", input.Request.Headers["foo"][0]) + + e := fakeEvent(t, RequestType, input) + + // Process .... + ctx := namespace.RootContext(context.Background()) + e2, err := formatter.Process(ctx, e) + require.NoError(t, err) + require.NotNil(t, e2) + + // now check the formatted as ... (JSON). + x2, b2 := e2.Format(JSONFormat.String()) + require.True(t, b2) + require.NotNil(t, x2) + + var input2 *logical.LogInput + err = json.Unmarshal(x2, &input2) + require.NoError(t, err) + require.NotNil(t, input2) + require.Len(t, input2.Request.Headers, 0) +} + // hashExpectedValueForComparison replicates enough of the audit HMAC process on a piece of expected data in a test, // so that we can use assert.Equal to compare the expected and output values. func (f *EntryFormatter) hashExpectedValueForComparison(input map[string]any) map[string]any { diff --git a/audit/options.go b/audit/options.go index 109b721e980d..bf3458fa1e09 100644 --- a/audit/options.go +++ b/audit/options.go @@ -5,7 +5,6 @@ package audit import ( "errors" - "reflect" "strings" "time" ) @@ -15,16 +14,15 @@ type Option func(*options) error // options are used to represent configuration for a audit related nodes. type options struct { - withID string - withNow time.Time - withSubtype subtype - withFormat format - withPrefix string - withRaw bool - withElision bool - withOmitTime bool - withHMACAccessor bool - withHeaderFormatter HeaderFormatter + withID string + withNow time.Time + withSubtype subtype + withFormat format + withPrefix string + withRaw bool + withElision bool + withOmitTime bool + withHMACAccessor bool } // getDefaultOptions returns options with their default values. @@ -163,15 +161,3 @@ func WithHMACAccessor(h bool) Option { return nil } } - -// WithHeaderFormatter provides an Option to supply a HeaderFormatter. -// If the HeaderFormatter interface supplied is nil (type or value), the option will not be applied. -func WithHeaderFormatter(f HeaderFormatter) Option { - return func(o *options) error { - if f != nil && !reflect.ValueOf(f).IsNil() { - o.withHeaderFormatter = f - } - - return nil - } -} diff --git a/audit/options_test.go b/audit/options_test.go index 3bbe05f8e684..e94ed52d9ed8 100644 --- a/audit/options_test.go +++ b/audit/options_test.go @@ -4,7 +4,6 @@ package audit import ( - "context" "testing" "time" @@ -380,47 +379,6 @@ func TestOptions_WithOmitTime(t *testing.T) { } } -// TestOptions_WithHeaderFormatter exercises the WithHeaderFormatter Option to -// ensure it applies the option as expected under various circumstances. -func TestOptions_WithHeaderFormatter(t *testing.T) { - t.Parallel() - - tests := map[string]struct { - Value HeaderFormatter - ExpectedValue HeaderFormatter - ShouldLeaveUninitialized bool - }{ - "nil": { - Value: nil, - ExpectedValue: nil, - }, - "unassigned-interface": { - ShouldLeaveUninitialized: true, - }, - "happy-path": { - Value: &testHeaderFormatter{}, - ExpectedValue: &testHeaderFormatter{}, - }, - } - - for name, tc := range tests { - name := name - tc := tc - t.Run(name, func(t *testing.T) { - t.Parallel() - opts := &options{} - var f HeaderFormatter - if !tc.ShouldLeaveUninitialized { - f = tc.Value - } - applyOption := WithHeaderFormatter(f) - err := applyOption(opts) - require.NoError(t, err) - require.Equal(t, tc.ExpectedValue, opts.withHeaderFormatter) - }) - } -} - // TestOptions_Default exercises getDefaultOptions to assert the default values. func TestOptions_Default(t *testing.T) { t.Parallel() @@ -549,12 +507,3 @@ func TestOptions_Opts(t *testing.T) { }) } } - -// testHeaderFormatter is a stub to prevent the need to import the vault package -// to bring in vault.AuditedHeadersConfig for testing. -type testHeaderFormatter struct{} - -// ApplyConfig satisfied the HeaderFormatter interface for testing. -func (f *testHeaderFormatter) ApplyConfig(ctx context.Context, headers map[string][]string, salter Salter) (result map[string][]string, retErr error) { - return nil, nil -} diff --git a/audit/types.go b/audit/types.go index 072887bd719c..d64cbf174864 100644 --- a/audit/types.go +++ b/audit/types.go @@ -99,6 +99,12 @@ type FormatterConfig struct { // 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. diff --git a/builtin/audit/file/backend.go b/builtin/audit/file/backend.go index 86d0a96eb137..78a9d454feef 100644 --- a/builtin/audit/file/backend.go +++ b/builtin/audit/file/backend.go @@ -112,17 +112,12 @@ func Factory(_ context.Context, conf *audit.BackendConfig, headersConfig audit.H return nil, fmt.Errorf("%s: error configuring filter node: %w", op, err) } - cfg, err := formatterConfig(conf.Config) + cfg, err := newFormatterConfig(headersConfig, conf.Config) if err != nil { return nil, fmt.Errorf("%s: failed to create formatter config: %w", op, err) } - formatterOpts := []audit.Option{ - audit.WithHeaderFormatter(headersConfig), - audit.WithPrefix(conf.Config["prefix"]), - } - - err = b.configureFormatterNode(conf.MountPath, cfg, conf.Logger, formatterOpts...) + err = b.configureFormatterNode(conf.MountPath, cfg, conf.Logger) if err != nil { return nil, fmt.Errorf("%s: error configuring formatter node: %w", op, err) } @@ -183,10 +178,10 @@ func (b *Backend) Invalidate(_ context.Context) { b.salt.Store((*salt.Salt)(nil)) } -// formatterConfig creates the configuration required by a formatter node using +// newFormatterConfig creates the configuration required by a formatter node using // the config map supplied to the factory. -func formatterConfig(config map[string]string) (audit.FormatterConfig, error) { - const op = "file.formatterConfig" +func newFormatterConfig(headerFormatter audit.HeaderFormatter, config map[string]string) (audit.FormatterConfig, error) { + const op = "file.newFormatterConfig" var opts []audit.Option @@ -220,11 +215,15 @@ func formatterConfig(config map[string]string) (audit.FormatterConfig, error) { opts = append(opts, audit.WithElision(v)) } - return audit.NewFormatterConfig(opts...) + if prefix, ok := config["prefix"]; ok { + opts = append(opts, audit.WithPrefix(prefix)) + } + + return audit.NewFormatterConfig(headerFormatter, opts...) } // 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, opts ...audit.Option) error { +func (b *Backend) configureFormatterNode(name string, formatConfig audit.FormatterConfig, logger hclog.Logger) error { const op = "file.(Backend).configureFormatterNode" formatterNodeID, err := event.GenerateNodeID() @@ -232,7 +231,7 @@ func (b *Backend) configureFormatterNode(name string, formatConfig audit.Formatt return fmt.Errorf("%s: error generating random NodeID for formatter node: %w", op, err) } - formatterNode, err := audit.NewEntryFormatter(name, formatConfig, b, logger, opts...) + formatterNode, err := audit.NewEntryFormatter(name, formatConfig, b, logger) if err != nil { return fmt.Errorf("%s: error creating formatter: %w", op, err) } diff --git a/builtin/audit/file/backend_filter_node_test.go b/builtin/audit/file/backend_filter_node_test.go index 3c821393eedc..94dbf354e72d 100644 --- a/builtin/audit/file/backend_filter_node_test.go +++ b/builtin/audit/file/backend_filter_node_test.go @@ -74,7 +74,7 @@ func TestBackend_configureFilterFormatterSink(t *testing.T) { nodeMap: map[eventlogger.NodeID]eventlogger.Node{}, } - formatConfig, err := audit.NewFormatterConfig() + formatConfig, err := audit.NewFormatterConfig(&testHeaderFormatter{}) require.NoError(t, err) err = b.configureFilterNode("path == bar") diff --git a/builtin/audit/file/backend_test.go b/builtin/audit/file/backend_test.go index 3f1e118c9899..2f4692b001de 100644 --- a/builtin/audit/file/backend_test.go +++ b/builtin/audit/file/backend_test.go @@ -19,6 +19,24 @@ import ( "github.com/stretchr/testify/require" ) +// testHeaderFormatter is a stub to prevent the need to import the vault package +// to bring in vault.AuditedHeadersConfig for testing. +type testHeaderFormatter struct { + shouldReturnEmpty bool +} + +// ApplyConfig satisfies the HeaderFormatter interface for testing. +// It will either return the headers it was supplied or empty headers depending +// on how it is configured. +// ignore-nil-nil-function-check. +func (f *testHeaderFormatter) ApplyConfig(_ context.Context, headers map[string][]string, salter audit.Salter) (result map[string][]string, retErr error) { + if f.shouldReturnEmpty { + return make(map[string][]string), nil + } + + return headers, nil +} + // TestAuditFile_fileModeNew verifies that the backend Factory correctly sets // the file mode when the mode argument is set. func TestAuditFile_fileModeNew(t *testing.T) { @@ -40,7 +58,7 @@ func TestAuditFile_fileModeNew(t *testing.T) { SaltView: &logical.InmemStorage{}, Logger: hclog.NewNullLogger(), } - _, err = Factory(context.Background(), backendConfig, nil) + _, err = Factory(context.Background(), backendConfig, &testHeaderFormatter{}) require.NoError(t, err) info, err := os.Stat(file) @@ -73,7 +91,7 @@ func TestAuditFile_fileModeExisting(t *testing.T) { Logger: hclog.NewNullLogger(), } - _, err = Factory(context.Background(), backendConfig, nil) + _, err = Factory(context.Background(), backendConfig, &testHeaderFormatter{}) require.NoError(t, err) info, err := os.Stat(f.Name()) @@ -107,7 +125,7 @@ func TestAuditFile_fileMode0000(t *testing.T) { Logger: hclog.NewNullLogger(), } - _, err = Factory(context.Background(), backendConfig, nil) + _, err = Factory(context.Background(), backendConfig, &testHeaderFormatter{}) require.NoError(t, err) info, err := os.Stat(f.Name()) @@ -136,7 +154,7 @@ func TestAuditFile_EventLogger_fileModeNew(t *testing.T) { Logger: hclog.NewNullLogger(), } - _, err = Factory(context.Background(), backendConfig, nil) + _, err = Factory(context.Background(), backendConfig, &testHeaderFormatter{}) require.NoError(t, err) info, err := os.Stat(file) @@ -144,8 +162,8 @@ func TestAuditFile_EventLogger_fileModeNew(t *testing.T) { require.Equalf(t, os.FileMode(mode), info.Mode(), "File mode does not match.") } -// TestBackend_formatterConfig ensures that all the configuration values are parsed correctly. -func TestBackend_formatterConfig(t *testing.T) { +// TestBackend_newFormatterConfig ensures that all the configuration values are parsed correctly. +func TestBackend_newFormatterConfig(t *testing.T) { t.Parallel() tests := map[string]struct { @@ -201,7 +219,7 @@ func TestBackend_formatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedMessage: "file.formatterConfig: unable to parse 'hmac_accessor': strconv.ParseBool: parsing \"maybe\": invalid syntax", + expectedMessage: "file.newFormatterConfig: unable to parse 'hmac_accessor': strconv.ParseBool: parsing \"maybe\": invalid syntax", }, "invalid-log-raw": { config: map[string]string{ @@ -211,7 +229,7 @@ func TestBackend_formatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedMessage: "file.formatterConfig: unable to parse 'log_raw': strconv.ParseBool: parsing \"maybe\": invalid syntax", + expectedMessage: "file.newFormatterConfig: unable to parse 'log_raw': strconv.ParseBool: parsing \"maybe\": invalid syntax", }, "invalid-elide-bool": { config: map[string]string{ @@ -222,7 +240,18 @@ func TestBackend_formatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedMessage: "file.formatterConfig: unable to parse 'elide_list_responses': strconv.ParseBool: parsing \"maybe\": invalid syntax", + expectedMessage: "file.newFormatterConfig: unable to parse 'elide_list_responses': strconv.ParseBool: parsing \"maybe\": invalid syntax", + }, + "prefix": { + config: map[string]string{ + "format": audit.JSONFormat.String(), + "prefix": "foo", + }, + want: audit.FormatterConfig{ + RequiredFormat: audit.JSONFormat, + Prefix: "foo", + HMACAccessor: true, + }, }, } for name, tc := range tests { @@ -231,14 +260,19 @@ func TestBackend_formatterConfig(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - got, err := formatterConfig(tc.config) + got, err := newFormatterConfig(&testHeaderFormatter{}, tc.config) if tc.wantErr { require.Error(t, err) require.EqualError(t, err, tc.expectedMessage) } else { require.NoError(t, err) } - require.Equal(t, tc.want, got) + require.Equal(t, tc.want.RequiredFormat, got.RequiredFormat) + require.Equal(t, tc.want.Raw, got.Raw) + require.Equal(t, tc.want.ElideListResponses, got.ElideListResponses) + require.Equal(t, tc.want.HMACAccessor, got.HMACAccessor) + require.Equal(t, tc.want.OmitTime, got.OmitTime) + require.Equal(t, tc.want.Prefix, got.Prefix) }) } } @@ -253,7 +287,7 @@ func TestBackend_configureFormatterNode(t *testing.T) { nodeMap: map[eventlogger.NodeID]eventlogger.Node{}, } - formatConfig, err := audit.NewFormatterConfig() + formatConfig, err := audit.NewFormatterConfig(&testHeaderFormatter{}) require.NoError(t, err) err = b.configureFormatterNode("juan", formatConfig, hclog.NewNullLogger()) @@ -476,7 +510,7 @@ func TestBackend_Factory_Conf(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - be, err := Factory(ctx, tc.backendConfig, nil) + be, err := Factory(ctx, tc.backendConfig, &testHeaderFormatter{}) switch { case tc.isErrorExpected: @@ -535,7 +569,7 @@ func TestBackend_IsFallback(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - be, err := Factory(ctx, tc.backendConfig, nil) + be, err := Factory(ctx, tc.backendConfig, &testHeaderFormatter{}) require.NoError(t, err) require.NotNil(t, be) require.Equal(t, tc.isFallbackExpected, be.IsFallback()) diff --git a/builtin/audit/socket/backend.go b/builtin/audit/socket/backend.go index d9f96b2d7661..7eaf1a29f0a5 100644 --- a/builtin/audit/socket/backend.go +++ b/builtin/audit/socket/backend.go @@ -93,17 +93,12 @@ func Factory(_ context.Context, conf *audit.BackendConfig, headersConfig audit.H return nil, fmt.Errorf("%s: error configuring filter node: %w", op, err) } - cfg, err := formatterConfig(conf.Config) + cfg, err := newFormatterConfig(headersConfig, conf.Config) if err != nil { return nil, fmt.Errorf("%s: failed to create formatter config: %w", op, err) } - opts := []audit.Option{ - audit.WithHeaderFormatter(headersConfig), - audit.WithPrefix(conf.Config["prefix"]), - } - - err = b.configureFormatterNode(conf.MountPath, cfg, conf.Logger, opts...) + err = b.configureFormatterNode(conf.MountPath, cfg, conf.Logger) if err != nil { return nil, fmt.Errorf("%s: error configuring formatter node: %w", op, err) } @@ -165,15 +160,15 @@ func (b *Backend) Invalidate(_ context.Context) { b.salt = nil } -// formatterConfig creates the configuration required by a formatter node using +// newFormatterConfig creates the configuration required by a formatter node using // the config map supplied to the factory. -func formatterConfig(config map[string]string) (audit.FormatterConfig, error) { - const op = "socket.formatterConfig" +func newFormatterConfig(headerFormatter audit.HeaderFormatter, config map[string]string) (audit.FormatterConfig, error) { + const op = "socket.newFormatterConfig" - var cfgOpts []audit.Option + var opts []audit.Option if format, ok := config["format"]; ok { - cfgOpts = append(cfgOpts, audit.WithFormat(format)) + opts = append(opts, audit.WithFormat(format)) } // Check if hashing of accessor is disabled @@ -182,7 +177,7 @@ func formatterConfig(config map[string]string) (audit.FormatterConfig, error) { if err != nil { return audit.FormatterConfig{}, fmt.Errorf("%s: unable to parse 'hmac_accessor': %w", op, err) } - cfgOpts = append(cfgOpts, audit.WithHMACAccessor(v)) + opts = append(opts, audit.WithHMACAccessor(v)) } // Check if raw logging is enabled @@ -191,7 +186,7 @@ func formatterConfig(config map[string]string) (audit.FormatterConfig, error) { if err != nil { return audit.FormatterConfig{}, fmt.Errorf("%s: unable to parse 'log_raw': %w", op, err) } - cfgOpts = append(cfgOpts, audit.WithRaw(v)) + opts = append(opts, audit.WithRaw(v)) } if elideListResponsesRaw, ok := config["elide_list_responses"]; ok { @@ -199,14 +194,18 @@ func formatterConfig(config map[string]string) (audit.FormatterConfig, error) { if err != nil { return audit.FormatterConfig{}, fmt.Errorf("%s: unable to parse 'elide_list_responses': %w", op, err) } - cfgOpts = append(cfgOpts, audit.WithElision(v)) + opts = append(opts, audit.WithElision(v)) + } + + if prefix, ok := config["prefix"]; ok { + opts = append(opts, audit.WithPrefix(prefix)) } - return audit.NewFormatterConfig(cfgOpts...) + return audit.NewFormatterConfig(headerFormatter, opts...) } // 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, opts ...audit.Option) error { +func (b *Backend) configureFormatterNode(name string, formatConfig audit.FormatterConfig, logger hclog.Logger) error { const op = "socket.(Backend).configureFormatterNode" formatterNodeID, err := event.GenerateNodeID() @@ -214,7 +213,7 @@ func (b *Backend) configureFormatterNode(name string, formatConfig audit.Formatt return fmt.Errorf("%s: error generating random NodeID for formatter node: %w", op, err) } - formatterNode, err := audit.NewEntryFormatter(name, formatConfig, b, logger, opts...) + formatterNode, err := audit.NewEntryFormatter(name, formatConfig, b, logger) if err != nil { return fmt.Errorf("%s: error creating formatter: %w", op, err) } diff --git a/builtin/audit/socket/backend_filter_node_test.go b/builtin/audit/socket/backend_filter_node_test.go index d8c26c2cc13c..a42e4fdcd36a 100644 --- a/builtin/audit/socket/backend_filter_node_test.go +++ b/builtin/audit/socket/backend_filter_node_test.go @@ -74,7 +74,7 @@ func TestBackend_configureFilterFormatterSink(t *testing.T) { nodeMap: map[eventlogger.NodeID]eventlogger.Node{}, } - formatConfig, err := audit.NewFormatterConfig() + formatConfig, err := audit.NewFormatterConfig(&testHeaderFormatter{}) require.NoError(t, err) err = b.configureFilterNode("path == bar") diff --git a/builtin/audit/socket/backend_test.go b/builtin/audit/socket/backend_test.go index 2a2dcee2bf02..02aedd317c2d 100644 --- a/builtin/audit/socket/backend_test.go +++ b/builtin/audit/socket/backend_test.go @@ -16,8 +16,26 @@ import ( "github.com/stretchr/testify/require" ) -// TestBackend_formatterConfig ensures that all the configuration values are parsed correctly. -func TestBackend_formatterConfig(t *testing.T) { +// testHeaderFormatter is a stub to prevent the need to import the vault package +// to bring in vault.AuditedHeadersConfig for testing. +type testHeaderFormatter struct { + shouldReturnEmpty bool +} + +// ApplyConfig satisfies the HeaderFormatter interface for testing. +// It will either return the headers it was supplied or empty headers depending +// on how it is configured. +// ignore-nil-nil-function-check. +func (f *testHeaderFormatter) ApplyConfig(_ context.Context, headers map[string][]string, salter audit.Salter) (result map[string][]string, retErr error) { + if f.shouldReturnEmpty { + return make(map[string][]string), nil + } + + return headers, nil +} + +// TestBackend_newFormatterConfig ensures that all the configuration values are parsed correctly. +func TestBackend_newFormatterConfig(t *testing.T) { t.Parallel() tests := map[string]struct { @@ -73,7 +91,7 @@ func TestBackend_formatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedErrMsg: "socket.formatterConfig: unable to parse 'hmac_accessor': strconv.ParseBool: parsing \"maybe\": invalid syntax", + expectedErrMsg: "socket.newFormatterConfig: unable to parse 'hmac_accessor': strconv.ParseBool: parsing \"maybe\": invalid syntax", }, "invalid-log-raw": { config: map[string]string{ @@ -83,7 +101,7 @@ func TestBackend_formatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedErrMsg: "socket.formatterConfig: unable to parse 'log_raw': strconv.ParseBool: parsing \"maybe\": invalid syntax", + expectedErrMsg: "socket.newFormatterConfig: unable to parse 'log_raw': strconv.ParseBool: parsing \"maybe\": invalid syntax", }, "invalid-elide-bool": { config: map[string]string{ @@ -94,7 +112,18 @@ func TestBackend_formatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedErrMsg: "socket.formatterConfig: unable to parse 'elide_list_responses': strconv.ParseBool: parsing \"maybe\": invalid syntax", + expectedErrMsg: "socket.newFormatterConfig: unable to parse 'elide_list_responses': strconv.ParseBool: parsing \"maybe\": invalid syntax", + }, + "prefix": { + config: map[string]string{ + "format": audit.JSONFormat.String(), + "prefix": "foo", + }, + want: audit.FormatterConfig{ + RequiredFormat: audit.JSONFormat, + Prefix: "foo", + HMACAccessor: true, + }, }, } for name, tc := range tests { @@ -103,14 +132,19 @@ func TestBackend_formatterConfig(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - got, err := formatterConfig(tc.config) + got, err := newFormatterConfig(&testHeaderFormatter{}, tc.config) if tc.wantErr { require.Error(t, err) require.EqualError(t, err, tc.expectedErrMsg) } else { require.NoError(t, err) } - require.Equal(t, tc.want, got) + require.Equal(t, tc.want.RequiredFormat, got.RequiredFormat) + require.Equal(t, tc.want.Raw, got.Raw) + require.Equal(t, tc.want.ElideListResponses, got.ElideListResponses) + require.Equal(t, tc.want.HMACAccessor, got.HMACAccessor) + require.Equal(t, tc.want.OmitTime, got.OmitTime) + require.Equal(t, tc.want.Prefix, got.Prefix) }) } } @@ -125,7 +159,7 @@ func TestBackend_configureFormatterNode(t *testing.T) { nodeMap: map[eventlogger.NodeID]eventlogger.Node{}, } - formatConfig, err := audit.NewFormatterConfig() + formatConfig, err := audit.NewFormatterConfig(&testHeaderFormatter{}) require.NoError(t, err) err = b.configureFormatterNode("juan", formatConfig, hclog.NewNullLogger()) @@ -370,7 +404,7 @@ func TestBackend_Factory_Conf(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - be, err := Factory(ctx, tc.backendConfig, nil) + be, err := Factory(ctx, tc.backendConfig, &testHeaderFormatter{}) switch { case tc.isErrorExpected: @@ -431,7 +465,7 @@ func TestBackend_IsFallback(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - be, err := Factory(ctx, tc.backendConfig, nil) + be, err := Factory(ctx, tc.backendConfig, &testHeaderFormatter{}) require.NoError(t, err) require.NotNil(t, be) require.Equal(t, tc.isFallbackExpected, be.IsFallback()) diff --git a/builtin/audit/syslog/backend.go b/builtin/audit/syslog/backend.go index 41a6607b68f7..22471c33e73d 100644 --- a/builtin/audit/syslog/backend.go +++ b/builtin/audit/syslog/backend.go @@ -90,17 +90,12 @@ func Factory(_ context.Context, conf *audit.BackendConfig, headersConfig audit.H return nil, fmt.Errorf("%s: error configuring filter node: %w", op, err) } - cfg, err := formatterConfig(conf.Config) + cfg, err := newFormatterConfig(headersConfig, conf.Config) if err != nil { return nil, fmt.Errorf("%s: failed to create formatter config: %w", op, err) } - formatterOpts := []audit.Option{ - audit.WithHeaderFormatter(headersConfig), - audit.WithPrefix(conf.Config["prefix"]), - } - - err = b.configureFormatterNode(conf.MountPath, cfg, conf.Logger, formatterOpts...) + err = b.configureFormatterNode(conf.MountPath, cfg, conf.Logger) if err != nil { return nil, fmt.Errorf("%s: error configuring formatter node: %w", op, err) } @@ -156,10 +151,10 @@ func (b *Backend) Invalidate(_ context.Context) { b.salt = nil } -// formatterConfig creates the configuration required by a formatter node using +// newFormatterConfig creates the configuration required by a formatter node using // the config map supplied to the factory. -func formatterConfig(config map[string]string) (audit.FormatterConfig, error) { - const op = "syslog.formatterConfig" +func newFormatterConfig(headerFormatter audit.HeaderFormatter, config map[string]string) (audit.FormatterConfig, error) { + const op = "syslog.newFormatterConfig" var opts []audit.Option @@ -193,11 +188,15 @@ func formatterConfig(config map[string]string) (audit.FormatterConfig, error) { opts = append(opts, audit.WithElision(v)) } - return audit.NewFormatterConfig(opts...) + if prefix, ok := config["prefix"]; ok { + opts = append(opts, audit.WithPrefix(prefix)) + } + + return audit.NewFormatterConfig(headerFormatter, opts...) } // 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, opts ...audit.Option) error { +func (b *Backend) configureFormatterNode(name string, formatConfig audit.FormatterConfig, logger hclog.Logger) error { const op = "syslog.(Backend).configureFormatterNode" formatterNodeID, err := event.GenerateNodeID() @@ -205,7 +204,7 @@ func (b *Backend) configureFormatterNode(name string, formatConfig audit.Formatt return fmt.Errorf("%s: error generating random NodeID for formatter node: %w", op, err) } - formatterNode, err := audit.NewEntryFormatter(name, formatConfig, b, logger, opts...) + formatterNode, err := audit.NewEntryFormatter(name, formatConfig, b, logger) if err != nil { return fmt.Errorf("%s: error creating formatter: %w", op, err) } diff --git a/builtin/audit/syslog/backend_filter_node_test.go b/builtin/audit/syslog/backend_filter_node_test.go index 402212dbbf03..62fabaeebddf 100644 --- a/builtin/audit/syslog/backend_filter_node_test.go +++ b/builtin/audit/syslog/backend_filter_node_test.go @@ -74,7 +74,7 @@ func TestBackend_configureFilterFormatterSink(t *testing.T) { nodeMap: map[eventlogger.NodeID]eventlogger.Node{}, } - formatConfig, err := audit.NewFormatterConfig() + formatConfig, err := audit.NewFormatterConfig(&testHeaderFormatter{}) require.NoError(t, err) err = b.configureFilterNode("path == bar") diff --git a/builtin/audit/syslog/backend_test.go b/builtin/audit/syslog/backend_test.go index 72765e3d6b18..2417f7de89ba 100644 --- a/builtin/audit/syslog/backend_test.go +++ b/builtin/audit/syslog/backend_test.go @@ -16,8 +16,26 @@ import ( "github.com/stretchr/testify/require" ) -// TestBackend_formatterConfig ensures that all the configuration values are parsed correctly. -func TestBackend_formatterConfig(t *testing.T) { +// testHeaderFormatter is a stub to prevent the need to import the vault package +// to bring in vault.AuditedHeadersConfig for testing. +type testHeaderFormatter struct { + shouldReturnEmpty bool +} + +// ApplyConfig satisfies the HeaderFormatter interface for testing. +// It will either return the headers it was supplied or empty headers depending +// on how it is configured. +// ignore-nil-nil-function-check. +func (f *testHeaderFormatter) ApplyConfig(_ context.Context, headers map[string][]string, salter audit.Salter) (result map[string][]string, retErr error) { + if f.shouldReturnEmpty { + return make(map[string][]string), nil + } + + return headers, nil +} + +// TestBackend_newFormatterConfig ensures that all the configuration values are parsed correctly. +func TestBackend_newFormatterConfig(t *testing.T) { t.Parallel() tests := map[string]struct { @@ -73,7 +91,7 @@ func TestBackend_formatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedErrMsg: "syslog.formatterConfig: unable to parse 'hmac_accessor': strconv.ParseBool: parsing \"maybe\": invalid syntax", + expectedErrMsg: "syslog.newFormatterConfig: unable to parse 'hmac_accessor': strconv.ParseBool: parsing \"maybe\": invalid syntax", }, "invalid-log-raw": { config: map[string]string{ @@ -83,7 +101,7 @@ func TestBackend_formatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedErrMsg: "syslog.formatterConfig: unable to parse 'log_raw': strconv.ParseBool: parsing \"maybe\": invalid syntax", + expectedErrMsg: "syslog.newFormatterConfig: unable to parse 'log_raw': strconv.ParseBool: parsing \"maybe\": invalid syntax", }, "invalid-elide-bool": { config: map[string]string{ @@ -94,7 +112,18 @@ func TestBackend_formatterConfig(t *testing.T) { }, want: audit.FormatterConfig{}, wantErr: true, - expectedErrMsg: "syslog.formatterConfig: unable to parse 'elide_list_responses': strconv.ParseBool: parsing \"maybe\": invalid syntax", + expectedErrMsg: "syslog.newFormatterConfig: unable to parse 'elide_list_responses': strconv.ParseBool: parsing \"maybe\": invalid syntax", + }, + "prefix": { + config: map[string]string{ + "format": audit.JSONFormat.String(), + "prefix": "foo", + }, + want: audit.FormatterConfig{ + RequiredFormat: audit.JSONFormat, + Prefix: "foo", + HMACAccessor: true, + }, }, } for name, tc := range tests { @@ -103,14 +132,19 @@ func TestBackend_formatterConfig(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - got, err := formatterConfig(tc.config) + got, err := newFormatterConfig(&testHeaderFormatter{}, tc.config) if tc.wantErr { require.Error(t, err) require.EqualError(t, err, tc.expectedErrMsg) } else { require.NoError(t, err) } - require.Equal(t, tc.want, got) + require.Equal(t, tc.want.RequiredFormat, got.RequiredFormat) + require.Equal(t, tc.want.Raw, got.Raw) + require.Equal(t, tc.want.ElideListResponses, got.ElideListResponses) + require.Equal(t, tc.want.HMACAccessor, got.HMACAccessor) + require.Equal(t, tc.want.OmitTime, got.OmitTime) + require.Equal(t, tc.want.Prefix, got.Prefix) }) } } @@ -125,7 +159,7 @@ func TestBackend_configureFormatterNode(t *testing.T) { nodeMap: map[eventlogger.NodeID]eventlogger.Node{}, } - formatConfig, err := audit.NewFormatterConfig() + formatConfig, err := audit.NewFormatterConfig(&testHeaderFormatter{}) require.NoError(t, err) err = b.configureFormatterNode("juan", formatConfig, hclog.NewNullLogger()) @@ -274,7 +308,7 @@ func TestBackend_Factory_Conf(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - be, err := Factory(ctx, tc.backendConfig, nil) + be, err := Factory(ctx, tc.backendConfig, &testHeaderFormatter{}) switch { case tc.isErrorExpected: @@ -331,7 +365,7 @@ func TestBackend_IsFallback(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - be, err := Factory(ctx, tc.backendConfig, nil) + be, err := Factory(ctx, tc.backendConfig, &testHeaderFormatter{}) require.NoError(t, err) require.NotNil(t, be) require.Equal(t, tc.isFallbackExpected, be.IsFallback()) diff --git a/helper/testhelpers/corehelpers/corehelpers.go b/helper/testhelpers/corehelpers/corehelpers.go index 8e252ac2d4f9..158c9f0309d3 100644 --- a/helper/testhelpers/corehelpers/corehelpers.go +++ b/helper/testhelpers/corehelpers/corehelpers.go @@ -216,13 +216,13 @@ func (m *mockBuiltinRegistry) DeprecationStatus(name string, pluginType consts.P return consts.Unknown, false } -func TestNoopAudit(t testing.T, path string, config map[string]string, opts ...audit.Option) *NoopAudit { +func TestNoopAudit(t testing.T, path string, config map[string]string, formatter audit.HeaderFormatter) *NoopAudit { cfg := &audit.BackendConfig{ Config: config, MountPath: path, Logger: NewTestLogger(t), } - n, err := NewNoopAudit(cfg, opts...) + n, err := NewNoopAudit(cfg, formatter) if err != nil { t.Fatal(err) } @@ -232,7 +232,7 @@ func TestNoopAudit(t testing.T, path string, config map[string]string, opts ...a // NewNoopAudit should be used to create a NoopAudit as it handles creation of a // predictable salt and wraps eventlogger nodes so information can be retrieved on // what they've seen or formatted. -func NewNoopAudit(config *audit.BackendConfig, opts ...audit.Option) (*NoopAudit, error) { +func NewNoopAudit(config *audit.BackendConfig, headerFormatter audit.HeaderFormatter) (*NoopAudit, error) { view := &logical.InmemStorage{} // Create the salt with a known key for predictable hmac values. @@ -259,7 +259,7 @@ func NewNoopAudit(config *audit.BackendConfig, opts ...audit.Option) (*NoopAudit nodeMap: make(map[eventlogger.NodeID]eventlogger.Node, 2), } - cfg, err := audit.NewFormatterConfig() + cfg, err := audit.NewFormatterConfig(headerFormatter) if err != nil { return nil, err } @@ -269,7 +269,7 @@ func NewNoopAudit(config *audit.BackendConfig, opts ...audit.Option) (*NoopAudit return nil, fmt.Errorf("error generating random NodeID for formatter node: %w", err) } - formatterNode, err := audit.NewEntryFormatter(config.MountPath, cfg, noopBackend, config.Logger, opts...) + formatterNode, err := audit.NewEntryFormatter(config.MountPath, cfg, noopBackend, config.Logger) if err != nil { return nil, fmt.Errorf("error creating formatter: %w", err) } @@ -297,7 +297,7 @@ func NewNoopAudit(config *audit.BackendConfig, opts ...audit.Option) (*NoopAudit // The records parameter will be repointed to the one used within the pipeline. func NoopAuditFactory(records **[][]byte) audit.Factory { return func(_ context.Context, config *audit.BackendConfig, headerFormatter audit.HeaderFormatter) (audit.Backend, error) { - n, err := NewNoopAudit(config, audit.WithHeaderFormatter(headerFormatter)) + n, err := NewNoopAudit(config, headerFormatter) if err != nil { return nil, err } diff --git a/http/logical_test.go b/http/logical_test.go index e07815896d83..677893589b5c 100644 --- a/http/logical_test.go +++ b/http/logical_test.go @@ -35,6 +35,7 @@ import ( "github.com/hashicorp/vault/sdk/physical" "github.com/hashicorp/vault/sdk/physical/inmem" "github.com/hashicorp/vault/vault" + "github.com/stretchr/testify/require" ) func TestLogical(t *testing.T) { @@ -569,7 +570,9 @@ func TestLogical_RespondWithStatusCode(t *testing.T) { func TestLogical_Audit_invalidWrappingToken(t *testing.T) { // Create a noop audit backend - noop := corehelpers.TestNoopAudit(t, "noop/", nil) + ahc, err := vault.NewAuditedHeadersConfig(&vault.BarrierView{}) + require.NoError(t, err) + noop := corehelpers.TestNoopAudit(t, "noop/", nil, ahc) c, _, root := vault.TestCoreUnsealedWithConfig(t, &vault.CoreConfig{ AuditBackends: map[string]audit.Factory{ "noop": func(ctx context.Context, config *audit.BackendConfig, _ audit.HeaderFormatter) (audit.Backend, error) { diff --git a/vault/audit_test.go b/vault/audit_test.go index cd25a1e1c6a6..32c3b1cf4e87 100644 --- a/vault/audit_test.go +++ b/vault/audit_test.go @@ -37,7 +37,12 @@ func TestAudit_ReadOnlyViewDuringMount(t *testing.T) { t.Fatalf("expected a read-only error") } factory := corehelpers.NoopAuditFactory(nil) - return factory(ctx, config, nil) + _, barrier, _ := mockBarrier(t) + view := NewBarrierView(barrier, auditedHeadersSubPath) + ahc, err := NewAuditedHeadersConfig(view) + require.NoError(t, err) + require.Len(t, ahc.headerSettings, 0) + return factory(ctx, config, ahc) } me := &MountEntry{ @@ -345,8 +350,10 @@ func TestAuditBroker_LogRequest(t *testing.T) { if err != nil { t.Fatal(err) } - a1 := corehelpers.TestNoopAudit(t, "foo", nil) - a2 := corehelpers.TestNoopAudit(t, "bar", nil) + ahc, err := NewAuditedHeadersConfig(&BarrierView{}) + require.NoError(t, err) + a1 := corehelpers.TestNoopAudit(t, "foo", nil, ahc) + a2 := corehelpers.TestNoopAudit(t, "bar", nil, ahc) err = b.Register("foo", a1, false) require.NoError(t, err) err = b.Register("bar", a2, false) @@ -433,8 +440,11 @@ func TestAuditBroker_LogResponse(t *testing.T) { if err != nil { t.Fatal(err) } - a1 := corehelpers.TestNoopAudit(t, "foo", nil) - a2 := corehelpers.TestNoopAudit(t, "bar", nil) + + ahc, err := NewAuditedHeadersConfig(&BarrierView{}) + require.NoError(t, err) + a1 := corehelpers.TestNoopAudit(t, "foo", nil, ahc) + a2 := corehelpers.TestNoopAudit(t, "bar", nil, ahc) err = b.Register("foo", a1, false) require.NoError(t, err) err = b.Register("bar", a2, false) @@ -539,17 +549,16 @@ func TestAuditBroker_AuditHeaders(t *testing.T) { } _, barrier, _ := mockBarrier(t) view := NewBarrierView(barrier, "headers/") + headersConf, err := NewAuditedHeadersConfig(view) + require.NoError(t, err) - headersConf := &AuditedHeadersConfig{ - view: view, - } err = headersConf.add(context.Background(), "X-Test-Header", false) require.NoError(t, err) err = headersConf.add(context.Background(), "X-Vault-Header", false) require.NoError(t, err) - a1 := corehelpers.TestNoopAudit(t, "foo", nil, audit.WithHeaderFormatter(headersConf)) - a2 := corehelpers.TestNoopAudit(t, "bar", nil, audit.WithHeaderFormatter(headersConf)) + a1 := corehelpers.TestNoopAudit(t, "foo", nil, headersConf) + a2 := corehelpers.TestNoopAudit(t, "bar", nil, headersConf) err = b.Register("foo", a1, false) require.NoError(t, err) diff --git a/vault/audited_headers.go b/vault/audited_headers.go index f66881dfd536..08367003a813 100644 --- a/vault/audited_headers.go +++ b/vault/audited_headers.go @@ -183,7 +183,13 @@ func (a *AuditedHeadersConfig) invalidate(ctx context.Context) error { } // ApplyConfig returns a map of approved headers and their values, either hmac'ed or plaintext. +// If the supplied headers are empty or nil, the input value is returned as-is. func (a *AuditedHeadersConfig) ApplyConfig(ctx context.Context, headers map[string][]string, salter audit.Salter) (result map[string][]string, retErr error) { + // Return early if we don't have headers. + if headers == nil || len(headers) < 1 { + return headers, nil + } + // Grab a read lock a.RLock() defer a.RUnlock() diff --git a/vault/core_test.go b/vault/core_test.go index 0314ce65ff81..092e57341bae 100644 --- a/vault/core_test.go +++ b/vault/core_test.go @@ -1547,7 +1547,7 @@ func TestCore_HandleRequest_AuditTrail(t *testing.T) { c, _, root := TestCoreUnsealed(t) c.auditBackends["noop"] = func(ctx context.Context, config *audit.BackendConfig, headerFormatter audit.HeaderFormatter) (audit.Backend, error) { var err error - noop, err = corehelpers.NewNoopAudit(config, audit.WithHeaderFormatter(headerFormatter)) + noop, err = corehelpers.NewNoopAudit(config, headerFormatter) return noop, err } @@ -1610,7 +1610,7 @@ func TestCore_HandleRequest_AuditTrail_noHMACKeys(t *testing.T) { c, _, root := TestCoreUnsealed(t) c.auditBackends["noop"] = func(ctx context.Context, config *audit.BackendConfig, headerFormatter audit.HeaderFormatter) (audit.Backend, error) { var err error - noop, err = corehelpers.NewNoopAudit(config, audit.WithHeaderFormatter(headerFormatter)) + noop, err = corehelpers.NewNoopAudit(config, headerFormatter) return noop, err } @@ -1731,7 +1731,7 @@ func TestCore_HandleLogin_AuditTrail(t *testing.T) { } c.auditBackends["noop"] = func(ctx context.Context, config *audit.BackendConfig, headerFormatter audit.HeaderFormatter) (audit.Backend, error) { var err error - noop, err = corehelpers.NewNoopAudit(config, audit.WithHeaderFormatter(headerFormatter)) + noop, err = corehelpers.NewNoopAudit(config, headerFormatter) return noop, err } From 5196eb9bbadbca93184989e8cf704e218113217b Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Tue, 2 Apr 2024 19:45:02 +0100 Subject: [PATCH 2/6] fix up test comment --- audit/entry_formatter_test.go | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/audit/entry_formatter_test.go b/audit/entry_formatter_test.go index 2b90a4409dc4..0b9c66cfc9a6 100644 --- a/audit/entry_formatter_test.go +++ b/audit/entry_formatter_test.go @@ -1173,15 +1173,16 @@ func TestEntryFormatter_Process_Panic(t *testing.T) { require.Nil(t, e2) } -// TODO: PW: DOCS +// TestEntryFormatter_NewFormatterConfig_NilHeaderFormatter ensures we cannot +// create a FormatterConfig using NewFormatterConfig if we supply a nil formatter. func TestEntryFormatter_NewFormatterConfig_NilHeaderFormatter(t *testing.T) { _, err := NewFormatterConfig(nil) require.Error(t, err) - - // TODO: PW: Test applying various options. } -// TODO: PW: DOCS +// TestEntryFormatter_Process_NeverLeaksHeaders ensures that if we never accidentally +// leak headers if applying them means we don't have any. This is more like a sense +// check to ensure the returned event doesn't somehow end up with the headers 'back'. func TestEntryFormatter_Process_NeverLeaksHeaders(t *testing.T) { t.Parallel() @@ -1189,8 +1190,6 @@ func TestEntryFormatter_Process_NeverLeaksHeaders(t *testing.T) { cfg, err := NewFormatterConfig(&testHeaderFormatter{shouldReturnEmpty: true}) require.NoError(t, err) ss := newStaticSalt(t) - - // Create a formatter with no header formatting config. formatter, err := NewEntryFormatter("juan", cfg, ss, hclog.NewNullLogger()) require.NoError(t, err) require.NotNil(t, formatter) @@ -1206,19 +1205,18 @@ func TestEntryFormatter_Process_NeverLeaksHeaders(t *testing.T) { e := fakeEvent(t, RequestType, input) - // Process .... + // Process the node. ctx := namespace.RootContext(context.Background()) e2, err := formatter.Process(ctx, e) require.NoError(t, err) require.NotNil(t, e2) - // now check the formatted as ... (JSON). - x2, b2 := e2.Format(JSONFormat.String()) + // Now check we can retrieve the formatted JSON. + jsonFormatted, b2 := e2.Format(JSONFormat.String()) require.True(t, b2) - require.NotNil(t, x2) - + require.NotNil(t, jsonFormatted) var input2 *logical.LogInput - err = json.Unmarshal(x2, &input2) + err = json.Unmarshal(jsonFormatted, &input2) require.NoError(t, err) require.NotNil(t, input2) require.Len(t, input2.Request.Headers, 0) From a1df5d33d40b828a17e83610d3bd4ed574d31465 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Tue, 2 Apr 2024 20:32:01 +0100 Subject: [PATCH 3/6] clean up commented out code --- audit/entry_formatter_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/audit/entry_formatter_test.go b/audit/entry_formatter_test.go index 0b9c66cfc9a6..96afb654d005 100644 --- a/audit/entry_formatter_test.go +++ b/audit/entry_formatter_test.go @@ -198,7 +198,7 @@ func TestNewEntryFormatter(t *testing.T) { cfg, err := NewFormatterConfig(&testHeaderFormatter{}, tc.Options...) require.NoError(t, err) - f, err := NewEntryFormatter(tc.Name, cfg, ss, tc.Logger /*, tc.Options...*/) + f, err := NewEntryFormatter(tc.Name, cfg, ss, tc.Logger) switch { case tc.IsErrorExpected: From 73f3ab42ce87ca4c767eb5c68f0ece18f70343cb Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Wed, 3 Apr 2024 09:20:17 +0100 Subject: [PATCH 4/6] ApplyConfig return empty headers (but never nil) when nil/empty supplied --- vault/audited_headers.go | 8 ++++---- vault/audited_headers_test.go | 23 ++++++++++++----------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/vault/audited_headers.go b/vault/audited_headers.go index 08367003a813..2c24a38c06c9 100644 --- a/vault/audited_headers.go +++ b/vault/audited_headers.go @@ -182,12 +182,12 @@ func (a *AuditedHeadersConfig) invalidate(ctx context.Context) error { return nil } -// ApplyConfig returns a map of approved headers and their values, either hmac'ed or plaintext. -// If the supplied headers are empty or nil, the input value is returned as-is. +// ApplyConfig returns a map of approved headers and their values, either HMAC'd or plaintext. +// If the supplied headers are empty or nil, an empty set of headers will be returned. func (a *AuditedHeadersConfig) ApplyConfig(ctx context.Context, headers map[string][]string, salter audit.Salter) (result map[string][]string, retErr error) { // Return early if we don't have headers. - if headers == nil || len(headers) < 1 { - return headers, nil + if len(headers) < 1 { + return map[string][]string{}, nil } // Grab a read lock diff --git a/vault/audited_headers_test.go b/vault/audited_headers_test.go index 0b64577a8942..8a545a045ab8 100644 --- a/vault/audited_headers_test.go +++ b/vault/audited_headers_test.go @@ -273,21 +273,22 @@ func TestAuditedHeadersConfig_ApplyConfig(t *testing.T) { func TestAuditedHeadersConfig_ApplyConfig_NoRequestHeaders(t *testing.T) { conf := mockAuditedHeadersConfig(t) - conf.add(context.Background(), "X-TesT-Header", false) - conf.add(context.Background(), "X-Vault-HeAdEr", true) - - reqHeaders := map[string][]string{} + err := conf.add(context.Background(), "X-TesT-Header", false) + require.NoError(t, err) + err = conf.add(context.Background(), "X-Vault-HeAdEr", true) + require.NoError(t, err) salter := &TestSalter{} - result, err := conf.ApplyConfig(context.Background(), reqHeaders, salter) - if err != nil { - t.Fatal(err) - } + // Test sending in nil headers first. + result, err := conf.ApplyConfig(context.Background(), nil, salter) + require.NoError(t, err) + require.NotNil(t, result) - if len(result) != 0 { - t.Fatalf("Expected no headers but actually got: %d\n", len(result)) - } + result, err = conf.ApplyConfig(context.Background(), map[string][]string{}, salter) + require.NoError(t, err) + require.NotNil(t, result) + require.Len(t, result, 0) } func TestAuditedHeadersConfig_ApplyConfig_NoConfiguredHeaders(t *testing.T) { From f42322aeea1091ae32deb6129a1d08e274781f08 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Wed, 3 Apr 2024 09:55:14 +0100 Subject: [PATCH 5/6] Add NoopHeaderFormatter and remove builtin audit testHeaderFormatters --- .../audit/file/backend_filter_node_test.go | 3 +- builtin/audit/file/backend_test.go | 35 +++++-------------- .../audit/socket/backend_filter_node_test.go | 3 +- builtin/audit/socket/backend_test.go | 27 +++----------- .../audit/syslog/backend_filter_node_test.go | 3 +- builtin/audit/syslog/backend_test.go | 27 +++----------- helper/testhelpers/corehelpers/corehelpers.go | 34 ++++++++++++++---- http/logical_test.go | 5 +-- vault/audit_test.go | 34 +++++------------- vault/core_test.go | 12 +++---- 10 files changed, 68 insertions(+), 115 deletions(-) diff --git a/builtin/audit/file/backend_filter_node_test.go b/builtin/audit/file/backend_filter_node_test.go index 94dbf354e72d..3b05b6702f0d 100644 --- a/builtin/audit/file/backend_filter_node_test.go +++ b/builtin/audit/file/backend_filter_node_test.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/eventlogger" "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/audit" + "github.com/hashicorp/vault/helper/testhelpers/corehelpers" "github.com/stretchr/testify/require" ) @@ -74,7 +75,7 @@ func TestBackend_configureFilterFormatterSink(t *testing.T) { nodeMap: map[eventlogger.NodeID]eventlogger.Node{}, } - formatConfig, err := audit.NewFormatterConfig(&testHeaderFormatter{}) + formatConfig, err := audit.NewFormatterConfig(&corehelpers.NoopHeaderFormatter{}) require.NoError(t, err) err = b.configureFilterNode("path == bar") diff --git a/builtin/audit/file/backend_test.go b/builtin/audit/file/backend_test.go index 2f4692b001de..b67162a0a740 100644 --- a/builtin/audit/file/backend_test.go +++ b/builtin/audit/file/backend_test.go @@ -13,30 +13,13 @@ import ( "github.com/hashicorp/eventlogger" "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/audit" + "github.com/hashicorp/vault/helper/testhelpers/corehelpers" "github.com/hashicorp/vault/internal/observability/event" "github.com/hashicorp/vault/sdk/helper/salt" "github.com/hashicorp/vault/sdk/logical" "github.com/stretchr/testify/require" ) -// testHeaderFormatter is a stub to prevent the need to import the vault package -// to bring in vault.AuditedHeadersConfig for testing. -type testHeaderFormatter struct { - shouldReturnEmpty bool -} - -// ApplyConfig satisfies the HeaderFormatter interface for testing. -// It will either return the headers it was supplied or empty headers depending -// on how it is configured. -// ignore-nil-nil-function-check. -func (f *testHeaderFormatter) ApplyConfig(_ context.Context, headers map[string][]string, salter audit.Salter) (result map[string][]string, retErr error) { - if f.shouldReturnEmpty { - return make(map[string][]string), nil - } - - return headers, nil -} - // TestAuditFile_fileModeNew verifies that the backend Factory correctly sets // the file mode when the mode argument is set. func TestAuditFile_fileModeNew(t *testing.T) { @@ -58,7 +41,7 @@ func TestAuditFile_fileModeNew(t *testing.T) { SaltView: &logical.InmemStorage{}, Logger: hclog.NewNullLogger(), } - _, err = Factory(context.Background(), backendConfig, &testHeaderFormatter{}) + _, err = Factory(context.Background(), backendConfig, &corehelpers.NoopHeaderFormatter{}) require.NoError(t, err) info, err := os.Stat(file) @@ -91,7 +74,7 @@ func TestAuditFile_fileModeExisting(t *testing.T) { Logger: hclog.NewNullLogger(), } - _, err = Factory(context.Background(), backendConfig, &testHeaderFormatter{}) + _, err = Factory(context.Background(), backendConfig, &corehelpers.NoopHeaderFormatter{}) require.NoError(t, err) info, err := os.Stat(f.Name()) @@ -125,7 +108,7 @@ func TestAuditFile_fileMode0000(t *testing.T) { Logger: hclog.NewNullLogger(), } - _, err = Factory(context.Background(), backendConfig, &testHeaderFormatter{}) + _, err = Factory(context.Background(), backendConfig, &corehelpers.NoopHeaderFormatter{}) require.NoError(t, err) info, err := os.Stat(f.Name()) @@ -154,7 +137,7 @@ func TestAuditFile_EventLogger_fileModeNew(t *testing.T) { Logger: hclog.NewNullLogger(), } - _, err = Factory(context.Background(), backendConfig, &testHeaderFormatter{}) + _, err = Factory(context.Background(), backendConfig, &corehelpers.NoopHeaderFormatter{}) require.NoError(t, err) info, err := os.Stat(file) @@ -260,7 +243,7 @@ func TestBackend_newFormatterConfig(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - got, err := newFormatterConfig(&testHeaderFormatter{}, tc.config) + got, err := newFormatterConfig(&corehelpers.NoopHeaderFormatter{}, tc.config) if tc.wantErr { require.Error(t, err) require.EqualError(t, err, tc.expectedMessage) @@ -287,7 +270,7 @@ func TestBackend_configureFormatterNode(t *testing.T) { nodeMap: map[eventlogger.NodeID]eventlogger.Node{}, } - formatConfig, err := audit.NewFormatterConfig(&testHeaderFormatter{}) + formatConfig, err := audit.NewFormatterConfig(&corehelpers.NoopHeaderFormatter{}) require.NoError(t, err) err = b.configureFormatterNode("juan", formatConfig, hclog.NewNullLogger()) @@ -510,7 +493,7 @@ func TestBackend_Factory_Conf(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - be, err := Factory(ctx, tc.backendConfig, &testHeaderFormatter{}) + be, err := Factory(ctx, tc.backendConfig, &corehelpers.NoopHeaderFormatter{}) switch { case tc.isErrorExpected: @@ -569,7 +552,7 @@ func TestBackend_IsFallback(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - be, err := Factory(ctx, tc.backendConfig, &testHeaderFormatter{}) + be, err := Factory(ctx, tc.backendConfig, &corehelpers.NoopHeaderFormatter{}) require.NoError(t, err) require.NotNil(t, be) require.Equal(t, tc.isFallbackExpected, be.IsFallback()) diff --git a/builtin/audit/socket/backend_filter_node_test.go b/builtin/audit/socket/backend_filter_node_test.go index a42e4fdcd36a..c3b9112e7c82 100644 --- a/builtin/audit/socket/backend_filter_node_test.go +++ b/builtin/audit/socket/backend_filter_node_test.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/eventlogger" "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/audit" + "github.com/hashicorp/vault/helper/testhelpers/corehelpers" "github.com/stretchr/testify/require" ) @@ -74,7 +75,7 @@ func TestBackend_configureFilterFormatterSink(t *testing.T) { nodeMap: map[eventlogger.NodeID]eventlogger.Node{}, } - formatConfig, err := audit.NewFormatterConfig(&testHeaderFormatter{}) + formatConfig, err := audit.NewFormatterConfig(&corehelpers.NoopHeaderFormatter{}) require.NoError(t, err) err = b.configureFilterNode("path == bar") diff --git a/builtin/audit/socket/backend_test.go b/builtin/audit/socket/backend_test.go index 02aedd317c2d..700fee9bbdea 100644 --- a/builtin/audit/socket/backend_test.go +++ b/builtin/audit/socket/backend_test.go @@ -10,30 +10,13 @@ import ( "github.com/hashicorp/eventlogger" "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/audit" + "github.com/hashicorp/vault/helper/testhelpers/corehelpers" "github.com/hashicorp/vault/internal/observability/event" "github.com/hashicorp/vault/sdk/helper/salt" "github.com/hashicorp/vault/sdk/logical" "github.com/stretchr/testify/require" ) -// testHeaderFormatter is a stub to prevent the need to import the vault package -// to bring in vault.AuditedHeadersConfig for testing. -type testHeaderFormatter struct { - shouldReturnEmpty bool -} - -// ApplyConfig satisfies the HeaderFormatter interface for testing. -// It will either return the headers it was supplied or empty headers depending -// on how it is configured. -// ignore-nil-nil-function-check. -func (f *testHeaderFormatter) ApplyConfig(_ context.Context, headers map[string][]string, salter audit.Salter) (result map[string][]string, retErr error) { - if f.shouldReturnEmpty { - return make(map[string][]string), nil - } - - return headers, nil -} - // TestBackend_newFormatterConfig ensures that all the configuration values are parsed correctly. func TestBackend_newFormatterConfig(t *testing.T) { t.Parallel() @@ -132,7 +115,7 @@ func TestBackend_newFormatterConfig(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - got, err := newFormatterConfig(&testHeaderFormatter{}, tc.config) + got, err := newFormatterConfig(&corehelpers.NoopHeaderFormatter{}, tc.config) if tc.wantErr { require.Error(t, err) require.EqualError(t, err, tc.expectedErrMsg) @@ -159,7 +142,7 @@ func TestBackend_configureFormatterNode(t *testing.T) { nodeMap: map[eventlogger.NodeID]eventlogger.Node{}, } - formatConfig, err := audit.NewFormatterConfig(&testHeaderFormatter{}) + formatConfig, err := audit.NewFormatterConfig(&corehelpers.NoopHeaderFormatter{}) require.NoError(t, err) err = b.configureFormatterNode("juan", formatConfig, hclog.NewNullLogger()) @@ -404,7 +387,7 @@ func TestBackend_Factory_Conf(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - be, err := Factory(ctx, tc.backendConfig, &testHeaderFormatter{}) + be, err := Factory(ctx, tc.backendConfig, &corehelpers.NoopHeaderFormatter{}) switch { case tc.isErrorExpected: @@ -465,7 +448,7 @@ func TestBackend_IsFallback(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - be, err := Factory(ctx, tc.backendConfig, &testHeaderFormatter{}) + be, err := Factory(ctx, tc.backendConfig, &corehelpers.NoopHeaderFormatter{}) require.NoError(t, err) require.NotNil(t, be) require.Equal(t, tc.isFallbackExpected, be.IsFallback()) diff --git a/builtin/audit/syslog/backend_filter_node_test.go b/builtin/audit/syslog/backend_filter_node_test.go index 62fabaeebddf..8c4903734019 100644 --- a/builtin/audit/syslog/backend_filter_node_test.go +++ b/builtin/audit/syslog/backend_filter_node_test.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/eventlogger" "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/audit" + "github.com/hashicorp/vault/helper/testhelpers/corehelpers" "github.com/stretchr/testify/require" ) @@ -74,7 +75,7 @@ func TestBackend_configureFilterFormatterSink(t *testing.T) { nodeMap: map[eventlogger.NodeID]eventlogger.Node{}, } - formatConfig, err := audit.NewFormatterConfig(&testHeaderFormatter{}) + formatConfig, err := audit.NewFormatterConfig(&corehelpers.NoopHeaderFormatter{}) require.NoError(t, err) err = b.configureFilterNode("path == bar") diff --git a/builtin/audit/syslog/backend_test.go b/builtin/audit/syslog/backend_test.go index 2417f7de89ba..906734343601 100644 --- a/builtin/audit/syslog/backend_test.go +++ b/builtin/audit/syslog/backend_test.go @@ -10,30 +10,13 @@ import ( "github.com/hashicorp/eventlogger" "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/audit" + "github.com/hashicorp/vault/helper/testhelpers/corehelpers" "github.com/hashicorp/vault/internal/observability/event" "github.com/hashicorp/vault/sdk/helper/salt" "github.com/hashicorp/vault/sdk/logical" "github.com/stretchr/testify/require" ) -// testHeaderFormatter is a stub to prevent the need to import the vault package -// to bring in vault.AuditedHeadersConfig for testing. -type testHeaderFormatter struct { - shouldReturnEmpty bool -} - -// ApplyConfig satisfies the HeaderFormatter interface for testing. -// It will either return the headers it was supplied or empty headers depending -// on how it is configured. -// ignore-nil-nil-function-check. -func (f *testHeaderFormatter) ApplyConfig(_ context.Context, headers map[string][]string, salter audit.Salter) (result map[string][]string, retErr error) { - if f.shouldReturnEmpty { - return make(map[string][]string), nil - } - - return headers, nil -} - // TestBackend_newFormatterConfig ensures that all the configuration values are parsed correctly. func TestBackend_newFormatterConfig(t *testing.T) { t.Parallel() @@ -132,7 +115,7 @@ func TestBackend_newFormatterConfig(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - got, err := newFormatterConfig(&testHeaderFormatter{}, tc.config) + got, err := newFormatterConfig(&corehelpers.NoopHeaderFormatter{}, tc.config) if tc.wantErr { require.Error(t, err) require.EqualError(t, err, tc.expectedErrMsg) @@ -159,7 +142,7 @@ func TestBackend_configureFormatterNode(t *testing.T) { nodeMap: map[eventlogger.NodeID]eventlogger.Node{}, } - formatConfig, err := audit.NewFormatterConfig(&testHeaderFormatter{}) + formatConfig, err := audit.NewFormatterConfig(&corehelpers.NoopHeaderFormatter{}) require.NoError(t, err) err = b.configureFormatterNode("juan", formatConfig, hclog.NewNullLogger()) @@ -308,7 +291,7 @@ func TestBackend_Factory_Conf(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - be, err := Factory(ctx, tc.backendConfig, &testHeaderFormatter{}) + be, err := Factory(ctx, tc.backendConfig, &corehelpers.NoopHeaderFormatter{}) switch { case tc.isErrorExpected: @@ -365,7 +348,7 @@ func TestBackend_IsFallback(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - be, err := Factory(ctx, tc.backendConfig, &testHeaderFormatter{}) + be, err := Factory(ctx, tc.backendConfig, &corehelpers.NoopHeaderFormatter{}) require.NoError(t, err) require.NotNil(t, be) require.Equal(t, tc.isFallbackExpected, be.IsFallback()) diff --git a/helper/testhelpers/corehelpers/corehelpers.go b/helper/testhelpers/corehelpers/corehelpers.go index 158c9f0309d3..941a88394f01 100644 --- a/helper/testhelpers/corehelpers/corehelpers.go +++ b/helper/testhelpers/corehelpers/corehelpers.go @@ -14,6 +14,7 @@ import ( "io" "os" "path/filepath" + "strings" "sync" "time" @@ -216,23 +217,44 @@ func (m *mockBuiltinRegistry) DeprecationStatus(name string, pluginType consts.P return consts.Unknown, false } -func TestNoopAudit(t testing.T, path string, config map[string]string, formatter audit.HeaderFormatter) *NoopAudit { +func TestNoopAudit(t testing.T, path string, config map[string]string) *NoopAudit { cfg := &audit.BackendConfig{ Config: config, MountPath: path, Logger: NewTestLogger(t), } - n, err := NewNoopAudit(cfg, formatter) + n, err := NewNoopAudit(cfg) if err != nil { t.Fatal(err) } return n } +// NoopHeaderFormatter can be used within no-op audit devices to do nothing when +// it comes to only allow configured headers to appear in the result. +// Whatever is passed in will be returned (nil becomes an empty map) in lowercase. +type NoopHeaderFormatter struct{} + +// ApplyConfig implements the relevant interface to make NoopHeaderFormatter an audit.HeaderFormatter. +func (f *NoopHeaderFormatter) ApplyConfig(_ context.Context, headers map[string][]string, _ audit.Salter) (result map[string][]string, retErr error) { + if len(headers) < 1 { + return map[string][]string{}, nil + } + + // Make a copy of the incoming headers with everything lower so we can + // case-insensitively compare + lowerHeaders := make(map[string][]string, len(headers)) + for k, v := range headers { + lowerHeaders[strings.ToLower(k)] = v + } + + return lowerHeaders, nil +} + // NewNoopAudit should be used to create a NoopAudit as it handles creation of a // predictable salt and wraps eventlogger nodes so information can be retrieved on // what they've seen or formatted. -func NewNoopAudit(config *audit.BackendConfig, headerFormatter audit.HeaderFormatter) (*NoopAudit, error) { +func NewNoopAudit(config *audit.BackendConfig) (*NoopAudit, error) { view := &logical.InmemStorage{} // Create the salt with a known key for predictable hmac values. @@ -259,7 +281,7 @@ func NewNoopAudit(config *audit.BackendConfig, headerFormatter audit.HeaderForma nodeMap: make(map[eventlogger.NodeID]eventlogger.Node, 2), } - cfg, err := audit.NewFormatterConfig(headerFormatter) + cfg, err := audit.NewFormatterConfig(&NoopHeaderFormatter{}) if err != nil { return nil, err } @@ -296,8 +318,8 @@ func NewNoopAudit(config *audit.BackendConfig, headerFormatter audit.HeaderForma // have been formatted by the pipeline during audit requests. // The records parameter will be repointed to the one used within the pipeline. func NoopAuditFactory(records **[][]byte) audit.Factory { - return func(_ context.Context, config *audit.BackendConfig, headerFormatter audit.HeaderFormatter) (audit.Backend, error) { - n, err := NewNoopAudit(config, headerFormatter) + return func(_ context.Context, config *audit.BackendConfig, _ audit.HeaderFormatter) (audit.Backend, error) { + n, err := NewNoopAudit(config) if err != nil { return nil, err } diff --git a/http/logical_test.go b/http/logical_test.go index 677893589b5c..e07815896d83 100644 --- a/http/logical_test.go +++ b/http/logical_test.go @@ -35,7 +35,6 @@ import ( "github.com/hashicorp/vault/sdk/physical" "github.com/hashicorp/vault/sdk/physical/inmem" "github.com/hashicorp/vault/vault" - "github.com/stretchr/testify/require" ) func TestLogical(t *testing.T) { @@ -570,9 +569,7 @@ func TestLogical_RespondWithStatusCode(t *testing.T) { func TestLogical_Audit_invalidWrappingToken(t *testing.T) { // Create a noop audit backend - ahc, err := vault.NewAuditedHeadersConfig(&vault.BarrierView{}) - require.NoError(t, err) - noop := corehelpers.TestNoopAudit(t, "noop/", nil, ahc) + noop := corehelpers.TestNoopAudit(t, "noop/", nil) c, _, root := vault.TestCoreUnsealedWithConfig(t, &vault.CoreConfig{ AuditBackends: map[string]audit.Factory{ "noop": func(ctx context.Context, config *audit.BackendConfig, _ audit.HeaderFormatter) (audit.Backend, error) { diff --git a/vault/audit_test.go b/vault/audit_test.go index 32c3b1cf4e87..a0429c5b8481 100644 --- a/vault/audit_test.go +++ b/vault/audit_test.go @@ -37,12 +37,7 @@ func TestAudit_ReadOnlyViewDuringMount(t *testing.T) { t.Fatalf("expected a read-only error") } factory := corehelpers.NoopAuditFactory(nil) - _, barrier, _ := mockBarrier(t) - view := NewBarrierView(barrier, auditedHeadersSubPath) - ahc, err := NewAuditedHeadersConfig(view) - require.NoError(t, err) - require.Len(t, ahc.headerSettings, 0) - return factory(ctx, config, ahc) + return factory(ctx, config, nil) } me := &MountEntry{ @@ -350,10 +345,9 @@ func TestAuditBroker_LogRequest(t *testing.T) { if err != nil { t.Fatal(err) } - ahc, err := NewAuditedHeadersConfig(&BarrierView{}) - require.NoError(t, err) - a1 := corehelpers.TestNoopAudit(t, "foo", nil, ahc) - a2 := corehelpers.TestNoopAudit(t, "bar", nil, ahc) + + a1 := corehelpers.TestNoopAudit(t, "foo", nil) + a2 := corehelpers.TestNoopAudit(t, "bar", nil) err = b.Register("foo", a1, false) require.NoError(t, err) err = b.Register("bar", a2, false) @@ -441,10 +435,8 @@ func TestAuditBroker_LogResponse(t *testing.T) { t.Fatal(err) } - ahc, err := NewAuditedHeadersConfig(&BarrierView{}) - require.NoError(t, err) - a1 := corehelpers.TestNoopAudit(t, "foo", nil, ahc) - a2 := corehelpers.TestNoopAudit(t, "bar", nil, ahc) + a1 := corehelpers.TestNoopAudit(t, "foo", nil) + a2 := corehelpers.TestNoopAudit(t, "bar", nil) err = b.Register("foo", a1, false) require.NoError(t, err) err = b.Register("bar", a2, false) @@ -547,18 +539,9 @@ func TestAuditBroker_AuditHeaders(t *testing.T) { if err != nil { t.Fatal(err) } - _, barrier, _ := mockBarrier(t) - view := NewBarrierView(barrier, "headers/") - headersConf, err := NewAuditedHeadersConfig(view) - require.NoError(t, err) - - err = headersConf.add(context.Background(), "X-Test-Header", false) - require.NoError(t, err) - err = headersConf.add(context.Background(), "X-Vault-Header", false) - require.NoError(t, err) - a1 := corehelpers.TestNoopAudit(t, "foo", nil, headersConf) - a2 := corehelpers.TestNoopAudit(t, "bar", nil, headersConf) + a1 := corehelpers.TestNoopAudit(t, "foo", nil) + a2 := corehelpers.TestNoopAudit(t, "bar", nil) err = b.Register("foo", a1, false) require.NoError(t, err) @@ -579,7 +562,6 @@ func TestAuditBroker_AuditHeaders(t *testing.T) { Headers: map[string][]string{ "X-Test-Header": {"foo"}, "X-Vault-Header": {"bar"}, - "Content-Type": {"baz"}, }, } respErr := fmt.Errorf("permission denied") diff --git a/vault/core_test.go b/vault/core_test.go index 092e57341bae..835e28905a43 100644 --- a/vault/core_test.go +++ b/vault/core_test.go @@ -1545,9 +1545,9 @@ func TestCore_HandleRequest_AuditTrail(t *testing.T) { // Create a noop audit backend var noop *corehelpers.NoopAudit c, _, root := TestCoreUnsealed(t) - c.auditBackends["noop"] = func(ctx context.Context, config *audit.BackendConfig, headerFormatter audit.HeaderFormatter) (audit.Backend, error) { + c.auditBackends["noop"] = func(_ context.Context, config *audit.BackendConfig, _ audit.HeaderFormatter) (audit.Backend, error) { var err error - noop, err = corehelpers.NewNoopAudit(config, headerFormatter) + noop, err = corehelpers.NewNoopAudit(config) return noop, err } @@ -1608,9 +1608,9 @@ func TestCore_HandleRequest_AuditTrail_noHMACKeys(t *testing.T) { // Create a noop audit backend var noop *corehelpers.NoopAudit c, _, root := TestCoreUnsealed(t) - c.auditBackends["noop"] = func(ctx context.Context, config *audit.BackendConfig, headerFormatter audit.HeaderFormatter) (audit.Backend, error) { + c.auditBackends["noop"] = func(_ context.Context, config *audit.BackendConfig, _ audit.HeaderFormatter) (audit.Backend, error) { var err error - noop, err = corehelpers.NewNoopAudit(config, headerFormatter) + noop, err = corehelpers.NewNoopAudit(config) return noop, err } @@ -1729,9 +1729,9 @@ func TestCore_HandleLogin_AuditTrail(t *testing.T) { c.credentialBackends["noop"] = func(context.Context, *logical.BackendConfig) (logical.Backend, error) { return noopBack, nil } - c.auditBackends["noop"] = func(ctx context.Context, config *audit.BackendConfig, headerFormatter audit.HeaderFormatter) (audit.Backend, error) { + c.auditBackends["noop"] = func(_ context.Context, config *audit.BackendConfig, _ audit.HeaderFormatter) (audit.Backend, error) { var err error - noop, err = corehelpers.NewNoopAudit(config, headerFormatter) + noop, err = corehelpers.NewNoopAudit(config) return noop, err } From afa67c57c1a39e97d1e14897d7126b5a04130236 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Wed, 3 Apr 2024 09:57:21 +0100 Subject: [PATCH 6/6] Test update from suggestion --- audit/entry_formatter_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/audit/entry_formatter_test.go b/audit/entry_formatter_test.go index 96afb654d005..b4871a903f1a 100644 --- a/audit/entry_formatter_test.go +++ b/audit/entry_formatter_test.go @@ -1199,9 +1199,7 @@ func TestEntryFormatter_Process_NeverLeaksHeaders(t *testing.T) { err = json.Unmarshal([]byte(testFormatJSONReqBasicStrFmt), &input) require.NoError(t, err) require.NotNil(t, input) - require.Len(t, input.Request.Headers, 1) - require.Len(t, input.Request.Headers["foo"], 1) - require.Equal(t, "bar", input.Request.Headers["foo"][0]) + require.ElementsMatch(t, input.Request.Headers["foo"], []string{"bar"}) e := fakeEvent(t, RequestType, input)