From da717745431f3117bd911df3584c99c07ed77649 Mon Sep 17 00:00:00 2001 From: Dan Kortschak Date: Tue, 25 Jun 2024 10:30:26 +0930 Subject: [PATCH] x-pack/filebeat/input/entityanalytics/{okta,azuread/fetcher/graph}: add ability to remove request trace logs This is essentially a replay of #39969, but for the entity analytics providers. The previous configuration system did not allow users to remove trace logs from agents after they are no longer needed. This is potential security risk as it leaves potentially sensitive information on the file system beyond its required lifetime. The mechanism for communicating to the input whether to write logs is not currently powerful enough to indicate that existing logs should be removed without deleting logs from other instances. So add an enabled configuration option to allow the target name to be sent independently of whether the files should be written or removed. The new option is optional, defaulting to the previous behaviour so that it can be opted into via progressive repair in the client integrations. --- CHANGELOG.next.asciidoc | 1 + .../inputs/input-entity-analytics.asciidoc | 32 +++++++++------ .../provider/azuread/fetcher/graph/graph.go | 40 ++++++++++++++++++- .../azuread/fetcher/graph/graph_test.go | 13 +++--- .../entityanalytics/provider/okta/conf.go | 11 ++++- .../entityanalytics/provider/okta/okta.go | 29 ++++++++++++++ .../provider/okta/okta_test.go | 5 ++- 7 files changed, 109 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 64665025f509..d02ff64905d5 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -289,6 +289,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff] - Add ability to remove request trace logs from HTTPJSON input. {pull}40003[40003] - Update CEL mito extensions to v1.13.0 {pull}40035[40035] - Add Jamf entity analytics provider. {pull}39996[39996] +- Add ability to remove request trace logs from entityanalytics input. {pull}40004[40004] *Auditbeat* diff --git a/x-pack/filebeat/docs/inputs/input-entity-analytics.asciidoc b/x-pack/filebeat/docs/inputs/input-entity-analytics.asciidoc index df5ea0630950..9174ee9a4f59 100644 --- a/x-pack/filebeat/docs/inputs/input-entity-analytics.asciidoc +++ b/x-pack/filebeat/docs/inputs/input-entity-analytics.asciidoc @@ -511,17 +511,21 @@ This is a list of optional query parameters. The default is `["accountEnabled", "alternativeSecurityIds"]`. [float] -==== `tracer.filename` +==== `tracer.enabled` It is possible to log HTTP requests and responses to the EntraID API to a local file-system for debugging configurations. -This option is enabled by setting the `tracer.filename` value. Additional options are available to -tune log rotation behavior. - -To differentiate the trace files generated from different input instances, a placeholder `*` can be added to the filename and will be replaced with the input instance id. -For Example, `http-request-trace-*.ndjson`. +This option is enabled by setting `tracer.enabled` to true and setting the `tracer.filename` value. +Additional options are available to tune log rotation behavior. To delete existing logs, set `tracer.enabled` +to false without unsetting the filename option. Enabling this option compromises security and should only be used for debugging. +[float] +==== `tracer.filename` + +To differentiate the trace files generated from different input instances, a placeholder `*` can be added to the +filename and will be replaced with the input instance id. For Example, `http-request-trace-*.ndjson`. + [id="provider-jamf"] ==== Jamf Computer Management (`jamf`) @@ -984,17 +988,21 @@ shorter than the full synchronization interval (`sync_interval`). Expressed as a duration string (e.g., 1m, 3h, 24h). Defaults to `15m` (15 minutes). [float] -==== `tracer.filename` +==== `tracer.enabled` It is possible to log HTTP requests and responses to the Okta API to a local file-system for debugging configurations. -This option is enabled by setting the `tracer.filename` value. Additional options are available to -tune log rotation behavior. - -To differentiate the trace files generated from different input instances, a placeholder `*` can be added to the filename and will be replaced with the input instance id. -For Example, `http-request-trace-*.ndjson`. +This option is enabled by setting `tracer.enabled` to true and setting the `tracer.filename` value. +Additional options are available to tune log rotation behavior. To delete existing logs, set `tracer.enabled` +to false without unsetting the filename option. Enabling this option compromises security and should only be used for debugging. +[float] +==== `tracer.filename` + +To differentiate the trace files generated from different input instances, a placeholder `*` can be added to the +filename and will be replaced with the input instance id. For Example, `http-request-trace-*.ndjson`. + [float] ==== Metrics diff --git a/x-pack/filebeat/input/entityanalytics/provider/azuread/fetcher/graph/graph.go b/x-pack/filebeat/input/entityanalytics/provider/azuread/fetcher/graph/graph.go index a3104ce0d009..4b98bdb05bea 100644 --- a/x-pack/filebeat/input/entityanalytics/provider/azuread/fetcher/graph/graph.go +++ b/x-pack/filebeat/input/entityanalytics/provider/azuread/fetcher/graph/graph.go @@ -13,8 +13,10 @@ import ( "errors" "fmt" "io" + "io/fs" "net/http" "net/url" + "os" "path/filepath" "strings" @@ -112,7 +114,16 @@ type graphConf struct { Transport httpcommon.HTTPTransportSettings `config:",inline"` // Tracer allows configuration of request trace logging. - Tracer *lumberjack.Logger `config:"tracer"` + Tracer *tracerConfig `config:"tracer"` +} + +type tracerConfig struct { + Enabled *bool `config:"enabled"` + lumberjack.Logger `config:",inline"` +} + +func (t *tracerConfig) enabled() bool { + return t != nil && (t.Enabled == nil || *t.Enabled) } type selection struct { @@ -398,12 +409,39 @@ func New(ctx context.Context, id string, cfg *config.C, logger *logp.Logger, aut return &f, nil } +// lumberjackTimestamp is a glob expression matching the time format string used +// by lumberjack when rolling over logs, "2006-01-02T15-04-05.000". +// https://github.com/natefinch/lumberjack/blob/4cb27fcfbb0f35cb48c542c5ea80b7c1d18933d0/lumberjack.go#L39 +const lumberjackTimestamp = "[0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]T[0-9][0-9]-[0-9][0-9]-[0-9][0-9].[0-9][0-9][0-9]" + // requestTrace decorates cli with an httplog.LoggingRoundTripper if cfg.Tracer // is non-nil. func requestTrace(ctx context.Context, cli *http.Client, cfg graphConf, log *logp.Logger) *http.Client { if cfg.Tracer == nil { return cli } + if !cfg.Tracer.enabled() { + // We have a trace log name, but we are not enabled, + // so remove all trace logs we own. + err := os.Remove(cfg.Tracer.Filename) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + log.Errorw("failed to remove request trace log", "path", cfg.Tracer.Filename, "error", err) + } + ext := filepath.Ext(cfg.Tracer.Filename) + base := strings.TrimSuffix(cfg.Tracer.Filename, ext) + paths, err := filepath.Glob(base + "-" + lumberjackTimestamp + ext) + if err != nil { + log.Errorw("failed to collect request trace log path names", "error", err) + } + for _, p := range paths { + err = os.Remove(p) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + log.Errorw("failed to remove request trace log", "path", p, "error", err) + } + } + return cli + } + w := zapcore.AddSync(cfg.Tracer) go func() { // Close the logger when we are done. diff --git a/x-pack/filebeat/input/entityanalytics/provider/azuread/fetcher/graph/graph_test.go b/x-pack/filebeat/input/entityanalytics/provider/azuread/fetcher/graph/graph_test.go index f2fc2effe29e..bafddc20a00a 100644 --- a/x-pack/filebeat/input/entityanalytics/provider/azuread/fetcher/graph/graph_test.go +++ b/x-pack/filebeat/input/entityanalytics/provider/azuread/fetcher/graph/graph_test.go @@ -318,9 +318,10 @@ func TestGraph_Groups(t *testing.T) { APIEndpoint: "http://" + testSrv.addr, } if *trace { - rawConf.Tracer = &lumberjack.Logger{ + // Use legacy behaviour; nil enabled setting. + rawConf.Tracer = &tracerConfig{Logger: lumberjack.Logger{ Filename: "test_trace-*.ndjson", - } + }} } c, err := config.NewConfigFrom(&rawConf) require.NoError(t, err) @@ -382,9 +383,9 @@ func TestGraph_Users(t *testing.T) { APIEndpoint: "http://" + testSrv.addr, } if *trace { - rawConf.Tracer = &lumberjack.Logger{ + rawConf.Tracer = &tracerConfig{Logger: lumberjack.Logger{ Filename: "test_trace-*.ndjson", - } + }} } c, err := config.NewConfigFrom(&rawConf) require.NoError(t, err) @@ -492,9 +493,9 @@ func TestGraph_Devices(t *testing.T) { Select: test.selection, } if *trace { - rawConf.Tracer = &lumberjack.Logger{ + rawConf.Tracer = &tracerConfig{Logger: lumberjack.Logger{ Filename: "test_trace-*.ndjson", - } + }} } c, err := config.NewConfigFrom(&rawConf) require.NoError(t, err) diff --git a/x-pack/filebeat/input/entityanalytics/provider/okta/conf.go b/x-pack/filebeat/input/entityanalytics/provider/okta/conf.go index 873a6195d472..2bab4c9e67d2 100644 --- a/x-pack/filebeat/input/entityanalytics/provider/okta/conf.go +++ b/x-pack/filebeat/input/entityanalytics/provider/okta/conf.go @@ -66,7 +66,16 @@ type conf struct { Request *requestConfig `config:"request"` // Tracer allows configuration of request trace logging. - Tracer *lumberjack.Logger `config:"tracer"` + Tracer *tracerConfig `config:"tracer"` +} + +type tracerConfig struct { + Enabled *bool `config:"enabled"` + lumberjack.Logger `config:",inline"` +} + +func (t *tracerConfig) enabled() bool { + return t != nil && (t.Enabled == nil || *t.Enabled) } type requestConfig struct { diff --git a/x-pack/filebeat/input/entityanalytics/provider/okta/okta.go b/x-pack/filebeat/input/entityanalytics/provider/okta/okta.go index 0980575df3a0..988f44d08407 100644 --- a/x-pack/filebeat/input/entityanalytics/provider/okta/okta.go +++ b/x-pack/filebeat/input/entityanalytics/provider/okta/okta.go @@ -10,8 +10,10 @@ import ( "errors" "fmt" "io" + "io/fs" "net/http" "net/url" + "os" "path/filepath" "strings" "time" @@ -184,12 +186,39 @@ func newClient(ctx context.Context, cfg conf, log *logp.Logger) (*http.Client, e return client.StandardClient(), nil } +// lumberjackTimestamp is a glob expression matching the time format string used +// by lumberjack when rolling over logs, "2006-01-02T15-04-05.000". +// https://github.com/natefinch/lumberjack/blob/4cb27fcfbb0f35cb48c542c5ea80b7c1d18933d0/lumberjack.go#L39 +const lumberjackTimestamp = "[0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]T[0-9][0-9]-[0-9][0-9]-[0-9][0-9].[0-9][0-9][0-9]" + // requestTrace decorates cli with an httplog.LoggingRoundTripper if cfg.Tracer // is non-nil. func requestTrace(ctx context.Context, cli *http.Client, cfg conf, log *logp.Logger) *http.Client { if cfg.Tracer == nil { return cli } + if !cfg.Tracer.enabled() { + // We have a trace log name, but we are not enabled, + // so remove all trace logs we own. + err := os.Remove(cfg.Tracer.Filename) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + log.Errorw("failed to remove request trace log", "path", cfg.Tracer.Filename, "error", err) + } + ext := filepath.Ext(cfg.Tracer.Filename) + base := strings.TrimSuffix(cfg.Tracer.Filename, ext) + paths, err := filepath.Glob(base + "-" + lumberjackTimestamp + ext) + if err != nil { + log.Errorw("failed to collect request trace log path names", "error", err) + } + for _, p := range paths { + err = os.Remove(p) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + log.Errorw("failed to remove request trace log", "path", p, "error", err) + } + } + return cli + } + w := zapcore.AddSync(cfg.Tracer) go func() { // Close the logger when we are done. diff --git a/x-pack/filebeat/input/entityanalytics/provider/okta/okta_test.go b/x-pack/filebeat/input/entityanalytics/provider/okta/okta_test.go index 51701775ca53..6bbd71e9b3ee 100644 --- a/x-pack/filebeat/input/entityanalytics/provider/okta/okta_test.go +++ b/x-pack/filebeat/input/entityanalytics/provider/okta/okta_test.go @@ -162,9 +162,10 @@ func TestOktaDoFetch(t *testing.T) { if name == "" { name = "default" } - a.cfg.Tracer = &lumberjack.Logger{ + // Use legacy behaviour; nil enabled setting. + a.cfg.Tracer = &tracerConfig{Logger: lumberjack.Logger{ Filename: fmt.Sprintf("test_trace_%s.ndjson", name), - } + }} } a.client = requestTrace(context.Background(), a.client, a.cfg, a.logger)