From 27491f82e439f7a461056172e24fbe45608b58a5 Mon Sep 17 00:00:00 2001 From: hc-github-team-secure-vault-core <82990506+hc-github-team-secure-vault-core@users.noreply.github.com> Date: Mon, 11 Sep 2023 05:12:00 -0400 Subject: [PATCH] backport of commit ae774b93d3e2ed04f3fcd0b5f03dd20f0d77d69d (#22955) Co-authored-by: Peter Wilson --- audit/options.go | 11 +-- audit/options_test.go | 6 +- audit/writer_json_test.go | 2 +- builtin/audit/file/backend.go | 94 ++++++++++---------- builtin/audit/socket/backend.go | 56 ++++++------ builtin/audit/syslog/backend.go | 57 ++++++------ internal/observability/event/options.go | 15 ++-- internal/observability/event/options_test.go | 8 +- 8 files changed, 120 insertions(+), 129 deletions(-) diff --git a/audit/options.go b/audit/options.go index 812ac51d54..71ae8cf843 100644 --- a/audit/options.go +++ b/audit/options.go @@ -13,8 +13,9 @@ import ( // getDefaultOptions returns options with their default values. func getDefaultOptions() options { return options{ - withNow: time.Now(), - withFormat: JSONFormat, + withNow: time.Now(), + withFormat: JSONFormat, + withHMACAccessor: true, } } @@ -108,11 +109,7 @@ func WithFormat(f string) Option { // WithPrefix provides an Option to represent a prefix for a file sink. func WithPrefix(prefix string) Option { return func(o *options) error { - prefix = strings.TrimSpace(prefix) - - if prefix != "" { - o.withPrefix = prefix - } + o.withPrefix = prefix return nil } diff --git a/audit/options_test.go b/audit/options_test.go index 5d089f229b..edb8e6142f 100644 --- a/audit/options_test.go +++ b/audit/options_test.go @@ -211,9 +211,9 @@ func TestOptions_WithPrefix(t *testing.T) { ExpectedValue: "", }, "whitespace": { - Value: " ", - IsErrorExpected: false, - ExpectedErrorMessage: "", + Value: " ", + IsErrorExpected: false, + ExpectedValue: " ", }, "valid": { Value: "test", diff --git a/audit/writer_json_test.go b/audit/writer_json_test.go index 47320cc8a1..822f26851b 100644 --- a/audit/writer_json_test.go +++ b/audit/writer_json_test.go @@ -98,7 +98,7 @@ func TestFormatJSON_formatRequest(t *testing.T) { for name, tc := range cases { var buf bytes.Buffer - cfg, err := NewFormatterConfig() + cfg, err := NewFormatterConfig(WithHMACAccessor(false)) require.NoError(t, err) f, err := NewEntryFormatter(cfg, ss) require.NoError(t, err) diff --git a/builtin/audit/file/backend.go b/builtin/audit/file/backend.go index 402c064472..f715c20e32 100644 --- a/builtin/audit/file/backend.go +++ b/builtin/audit/file/backend.go @@ -22,6 +22,11 @@ import ( "github.com/hashicorp/vault/sdk/logical" ) +const ( + stdout = "stdout" + discard = "discard" +) + func Factory(ctx context.Context, conf *audit.BackendConfig, useEventLogger bool, headersConfig audit.HeaderFormatter) (audit.Backend, error) { if conf.SaltConfig == nil { return nil, fmt.Errorf("nil salt config") @@ -39,53 +44,45 @@ func Factory(ctx context.Context, conf *audit.BackendConfig, useEventLogger bool } // normalize path if configured for stdout - if strings.EqualFold(path, "stdout") { - path = "stdout" + if strings.EqualFold(path, stdout) { + path = stdout } - if strings.EqualFold(path, "discard") { - path = "discard" + if strings.EqualFold(path, discard) { + path = discard } - format, ok := conf.Config["format"] - if !ok { - format = audit.JSONFormat.String() - } - switch format { - case audit.JSONFormat.String(), audit.JSONxFormat.String(): - default: - return nil, fmt.Errorf("unknown format type %q", format) + var cfgOpts []audit.Option + + if format, ok := conf.Config["format"]; ok { + cfgOpts = append(cfgOpts, audit.WithFormat(format)) } // Check if hashing of accessor is disabled - hmacAccessor := true if hmacAccessorRaw, ok := conf.Config["hmac_accessor"]; ok { - value, err := strconv.ParseBool(hmacAccessorRaw) + v, err := strconv.ParseBool(hmacAccessorRaw) if err != nil { return nil, err } - hmacAccessor = value + cfgOpts = append(cfgOpts, audit.WithHMACAccessor(v)) } // Check if raw logging is enabled - logRaw := false if raw, ok := conf.Config["log_raw"]; ok { - b, err := strconv.ParseBool(raw) + v, err := strconv.ParseBool(raw) if err != nil { return nil, err } - logRaw = b + cfgOpts = append(cfgOpts, audit.WithRaw(v)) } - elideListResponses := false if elideListResponsesRaw, ok := conf.Config["elide_list_responses"]; ok { - value, err := strconv.ParseBool(elideListResponsesRaw) + v, err := strconv.ParseBool(elideListResponsesRaw) if err != nil { return nil, err } - elideListResponses = value + cfgOpts = append(cfgOpts, audit.WithElision(v)) } - // Check if mode is provided mode := os.FileMode(0o600) if modeRaw, ok := conf.Config["mode"]; ok { m, err := strconv.ParseUint(modeRaw, 8, 32) @@ -95,7 +92,7 @@ func Factory(ctx context.Context, conf *audit.BackendConfig, useEventLogger bool switch m { case 0: // if mode is 0000, then do not modify file mode - if path != "stdout" && path != "discard" { + if path != stdout && path != discard { fileInfo, err := os.Stat(path) if err != nil { return nil, err @@ -107,12 +104,7 @@ func Factory(ctx context.Context, conf *audit.BackendConfig, useEventLogger bool } } - cfg, err := audit.NewFormatterConfig( - audit.WithElision(elideListResponses), - audit.WithFormat(format), - audit.WithHMACAccessor(hmacAccessor), - audit.WithRaw(logRaw), - ) + cfg, err := audit.NewFormatterConfig(cfgOpts...) if err != nil { return nil, err } @@ -136,11 +128,13 @@ func Factory(ctx context.Context, conf *audit.BackendConfig, useEventLogger bool return nil, fmt.Errorf("error creating formatter: %w", err) } var w audit.Writer - switch format { - case "json": + switch b.formatConfig.RequiredFormat { + case audit.JSONFormat: w = &audit.JSONWriter{Prefix: conf.Config["prefix"]} - case "jsonx": + case audit.JSONxFormat: w = &audit.JSONxWriter{Prefix: conf.Config["prefix"]} + default: + return nil, fmt.Errorf("unknown format type %q", b.formatConfig.RequiredFormat) } fw, err := audit.NewEntryFormatterWriter(b.formatConfig, f, w) @@ -164,16 +158,24 @@ func Factory(ctx context.Context, conf *audit.BackendConfig, useEventLogger bool var sinkNode eventlogger.Node switch path { - case "stdout": - sinkNode = &audit.SinkWrapper{Name: path, Sink: event.NewStdoutSinkNode(format)} - case "discard": + case stdout: + sinkNode = &audit.SinkWrapper{Name: path, Sink: event.NewStdoutSinkNode(b.formatConfig.RequiredFormat.String())} + case discard: sinkNode = &audit.SinkWrapper{Name: path, Sink: event.NewNoopSink()} default: var err error + var opts []event.Option + // Check if mode is provided + if modeRaw, ok := conf.Config["mode"]; ok { + opts = append(opts, event.WithFileMode(modeRaw)) + } + // The NewFileSink function attempts to open the file and will // return an error if it can't. - n, err := event.NewFileSink(b.path, format, event.WithFileMode(strconv.FormatUint(uint64(mode), 8))) + n, err := event.NewFileSink( + b.path, + b.formatConfig.RequiredFormat.String(), opts...) if err != nil { return nil, fmt.Errorf("file sink creation failed for path %q: %w", path, err) } @@ -189,8 +191,8 @@ func Factory(ctx context.Context, conf *audit.BackendConfig, useEventLogger bool b.nodeMap[sinkNodeID] = sinkNode } else { switch path { - case "stdout": - case "discard": + case stdout: + case discard: default: // Ensure that the file can be successfully opened for writing; // otherwise it will be too late to catch later without problems @@ -257,9 +259,9 @@ func (b *Backend) Salt(ctx context.Context) (*salt.Salt, error) { func (b *Backend) LogRequest(ctx context.Context, in *logical.LogInput) error { var writer io.Writer switch b.path { - case "stdout": + case stdout: writer = os.Stdout - case "discard": + case discard: return nil } @@ -288,7 +290,7 @@ func (b *Backend) log(_ context.Context, buf *bytes.Buffer, writer io.Writer) er if _, err := reader.WriteTo(writer); err == nil { b.fileLock.Unlock() return nil - } else if b.path == "stdout" { + } else if b.path == stdout { b.fileLock.Unlock() return err } @@ -313,9 +315,9 @@ func (b *Backend) log(_ context.Context, buf *bytes.Buffer, writer io.Writer) er func (b *Backend) LogResponse(ctx context.Context, in *logical.LogInput) error { var writer io.Writer switch b.path { - case "stdout": + case stdout: writer = os.Stdout - case "discard": + case discard: return nil } @@ -337,9 +339,9 @@ func (b *Backend) LogTestMessage(ctx context.Context, in *logical.LogInput, conf // Old behavior var writer io.Writer switch b.path { - case "stdout": + case stdout: writer = os.Stdout - case "discard": + case discard: return nil } @@ -390,7 +392,7 @@ func (b *Backend) open() error { func (b *Backend) Reload(_ context.Context) error { switch b.path { - case "stdout", "discard": + case stdout, discard: return nil } diff --git a/builtin/audit/socket/backend.go b/builtin/audit/socket/backend.go index cd87689b46..85370a3506 100644 --- a/builtin/audit/socket/backend.go +++ b/builtin/audit/socket/backend.go @@ -12,9 +12,10 @@ import ( "sync" "time" + "github.com/hashicorp/go-secure-stdlib/parseutil" + "github.com/hashicorp/eventlogger" "github.com/hashicorp/go-multierror" - "github.com/hashicorp/go-secure-stdlib/parseutil" "github.com/hashicorp/vault/audit" "github.com/hashicorp/vault/internal/observability/event" "github.com/hashicorp/vault/sdk/helper/salt" @@ -38,7 +39,6 @@ func Factory(ctx context.Context, conf *audit.BackendConfig, useEventLogger bool if !ok { socketType = "tcp" } - writeDeadline, ok := conf.Config["write_timeout"] if !ok { writeDeadline = "2s" @@ -48,51 +48,39 @@ func Factory(ctx context.Context, conf *audit.BackendConfig, useEventLogger bool return nil, err } - format, ok := conf.Config["format"] - if !ok { - format = audit.JSONFormat.String() - } - switch format { - case audit.JSONFormat.String(), audit.JSONxFormat.String(): - default: - return nil, fmt.Errorf("unknown format type %q", format) + var cfgOpts []audit.Option + + if format, ok := conf.Config["format"]; ok { + cfgOpts = append(cfgOpts, audit.WithFormat(format)) } // Check if hashing of accessor is disabled - hmacAccessor := true if hmacAccessorRaw, ok := conf.Config["hmac_accessor"]; ok { - value, err := strconv.ParseBool(hmacAccessorRaw) + v, err := strconv.ParseBool(hmacAccessorRaw) if err != nil { return nil, err } - hmacAccessor = value + cfgOpts = append(cfgOpts, audit.WithHMACAccessor(v)) } // Check if raw logging is enabled - logRaw := false if raw, ok := conf.Config["log_raw"]; ok { - b, err := strconv.ParseBool(raw) + v, err := strconv.ParseBool(raw) if err != nil { return nil, err } - logRaw = b + cfgOpts = append(cfgOpts, audit.WithRaw(v)) } - elideListResponses := false if elideListResponsesRaw, ok := conf.Config["elide_list_responses"]; ok { - value, err := strconv.ParseBool(elideListResponsesRaw) + v, err := strconv.ParseBool(elideListResponsesRaw) if err != nil { return nil, err } - elideListResponses = value + cfgOpts = append(cfgOpts, audit.WithElision(v)) } - cfg, err := audit.NewFormatterConfig( - audit.WithElision(elideListResponses), - audit.WithFormat(format), - audit.WithHMACAccessor(hmacAccessor), - audit.WithRaw(logRaw), - ) + cfg, err := audit.NewFormatterConfig(cfgOpts...) if err != nil { return nil, err } @@ -113,10 +101,10 @@ func Factory(ctx context.Context, conf *audit.BackendConfig, useEventLogger bool return nil, fmt.Errorf("error creating formatter: %w", err) } var w audit.Writer - switch format { - case audit.JSONFormat.String(): + switch b.formatConfig.RequiredFormat { + case audit.JSONFormat: w = &audit.JSONWriter{Prefix: conf.Config["prefix"]} - case audit.JSONxFormat.String(): + case audit.JSONxFormat: w = &audit.JSONxWriter{Prefix: conf.Config["prefix"]} } @@ -128,6 +116,16 @@ func Factory(ctx context.Context, conf *audit.BackendConfig, useEventLogger bool b.formatter = fw if useEventLogger { + var opts []event.Option + + if socketType, ok := conf.Config["socket_type"]; ok { + opts = append(opts, event.WithSocketType(socketType)) + } + + if writeDeadline, ok := conf.Config["write_timeout"]; ok { + opts = append(opts, event.WithMaxDuration(writeDeadline)) + } + b.nodeIDList = make([]eventlogger.NodeID, 2) b.nodeMap = make(map[eventlogger.NodeID]eventlogger.Node) @@ -138,7 +136,7 @@ func Factory(ctx context.Context, conf *audit.BackendConfig, useEventLogger bool b.nodeIDList[0] = formatterNodeID b.nodeMap[formatterNodeID] = f - n, err := event.NewSocketSink(format, address, event.WithSocketType(socketType), event.WithMaxDuration(writeDuration.String())) + n, err := event.NewSocketSink(b.formatConfig.RequiredFormat.String(), address, opts...) if err != nil { return nil, fmt.Errorf("error creating socket sink node: %w", err) } diff --git a/builtin/audit/syslog/backend.go b/builtin/audit/syslog/backend.go index 07fe94b3fe..f1b7f81790 100644 --- a/builtin/audit/syslog/backend.go +++ b/builtin/audit/syslog/backend.go @@ -39,57 +39,45 @@ func Factory(ctx context.Context, conf *audit.BackendConfig, useEventLogger bool tag = "vault" } - format, ok := conf.Config["format"] - if !ok { - format = audit.JSONFormat.String() - } - switch format { - case audit.JSONFormat.String(), audit.JSONxFormat.String(): - default: - return nil, fmt.Errorf("unknown format type %q", format) + var cfgOpts []audit.Option + + if format, ok := conf.Config["format"]; ok { + cfgOpts = append(cfgOpts, audit.WithFormat(format)) } // Check if hashing of accessor is disabled - hmacAccessor := true if hmacAccessorRaw, ok := conf.Config["hmac_accessor"]; ok { - value, err := strconv.ParseBool(hmacAccessorRaw) + v, err := strconv.ParseBool(hmacAccessorRaw) if err != nil { return nil, err } - hmacAccessor = value + cfgOpts = append(cfgOpts, audit.WithHMACAccessor(v)) } // Check if raw logging is enabled - logRaw := false if raw, ok := conf.Config["log_raw"]; ok { - b, err := strconv.ParseBool(raw) + v, err := strconv.ParseBool(raw) if err != nil { return nil, err } - logRaw = b + cfgOpts = append(cfgOpts, audit.WithRaw(v)) } - elideListResponses := false if elideListResponsesRaw, ok := conf.Config["elide_list_responses"]; ok { - value, err := strconv.ParseBool(elideListResponsesRaw) + v, err := strconv.ParseBool(elideListResponsesRaw) if err != nil { return nil, err } - elideListResponses = value + cfgOpts = append(cfgOpts, audit.WithElision(v)) } - // Get the logger - logger, err := gsyslog.NewLogger(gsyslog.LOG_INFO, facility, tag) + cfg, err := audit.NewFormatterConfig(cfgOpts...) if err != nil { return nil, err } - cfg, err := audit.NewFormatterConfig( - audit.WithElision(elideListResponses), - audit.WithFormat(format), - audit.WithHMACAccessor(hmacAccessor), - audit.WithRaw(logRaw), - ) + // Get the logger + logger, err := gsyslog.NewLogger(gsyslog.LOG_INFO, facility, tag) if err != nil { return nil, err } @@ -108,10 +96,10 @@ func Factory(ctx context.Context, conf *audit.BackendConfig, useEventLogger bool } var w audit.Writer - switch format { - case audit.JSONFormat.String(): + switch b.formatConfig.RequiredFormat { + case audit.JSONFormat: w = &audit.JSONWriter{Prefix: conf.Config["prefix"]} - case audit.JSONxFormat.String(): + case audit.JSONxFormat: w = &audit.JSONxWriter{Prefix: conf.Config["prefix"]} } @@ -123,6 +111,17 @@ func Factory(ctx context.Context, conf *audit.BackendConfig, useEventLogger bool b.formatter = fw if useEventLogger { + var opts []event.Option + + // Get facility or default to AUTH + if facility, ok := conf.Config["facility"]; ok { + opts = append(opts, event.WithFacility(facility)) + } + + if tag, ok := conf.Config["tag"]; ok { + opts = append(opts, event.WithTag(tag)) + } + b.nodeIDList = make([]eventlogger.NodeID, 2) b.nodeMap = make(map[eventlogger.NodeID]eventlogger.Node) @@ -133,7 +132,7 @@ func Factory(ctx context.Context, conf *audit.BackendConfig, useEventLogger bool b.nodeIDList[0] = formatterNodeID b.nodeMap[formatterNodeID] = f - n, err := event.NewSyslogSink(format, event.WithFacility(facility), event.WithTag(tag)) + n, err := event.NewSyslogSink(b.formatConfig.RequiredFormat.String(), opts...) if err != nil { return nil, fmt.Errorf("error creating syslog sink node: %w", err) } diff --git a/internal/observability/event/options.go b/internal/observability/event/options.go index 2cd7cf5c4b..80ad1bf999 100644 --- a/internal/observability/event/options.go +++ b/internal/observability/event/options.go @@ -32,12 +32,15 @@ type options struct { // getDefaultOptions returns Options with their default values. func getDefaultOptions() options { + fileMode := os.FileMode(0o600) + return options{ withNow: time.Now(), withFacility: "AUTH", withTag: "vault", withSocketType: "tcp", withMaxDuration: 2 * time.Second, + withFileMode: &fileMode, } } @@ -110,11 +113,7 @@ func WithNow(now time.Time) Option { // WithFacility provides an Option to represent a 'facility' for a syslog sink. func WithFacility(facility string) Option { return func(o *options) error { - facility = strings.TrimSpace(facility) - - if facility != "" { - o.withFacility = facility - } + o.withFacility = facility return nil } @@ -123,11 +122,7 @@ func WithFacility(facility string) Option { // WithTag provides an Option to represent a 'tag' for a syslog sink. func WithTag(tag string) Option { return func(o *options) error { - tag = strings.TrimSpace(tag) - - if tag != "" { - o.withTag = tag - } + o.withTag = tag return nil } diff --git a/internal/observability/event/options_test.go b/internal/observability/event/options_test.go index 0f36014740..676c798330 100644 --- a/internal/observability/event/options_test.go +++ b/internal/observability/event/options_test.go @@ -205,7 +205,7 @@ func TestOptions_WithFacility(t *testing.T) { }, "whitespace": { Value: " ", - ExpectedValue: "", + ExpectedValue: " ", }, "value": { Value: "juan", @@ -213,7 +213,7 @@ func TestOptions_WithFacility(t *testing.T) { }, "spacey-value": { Value: " juan ", - ExpectedValue: "juan", + ExpectedValue: " juan ", }, } @@ -243,7 +243,7 @@ func TestOptions_WithTag(t *testing.T) { }, "whitespace": { Value: " ", - ExpectedValue: "", + ExpectedValue: " ", }, "value": { Value: "juan", @@ -251,7 +251,7 @@ func TestOptions_WithTag(t *testing.T) { }, "spacey-value": { Value: " juan ", - ExpectedValue: "juan", + ExpectedValue: " juan ", }, }