From 5caa07073c8841beeea865398cd6323e90d967dc Mon Sep 17 00:00:00 2001 From: Dan Kortschak Date: Fri, 28 Jun 2024 20:45:59 +0930 Subject: [PATCH] x-pack/filebeat/input/http_endpoint: add ability to remove request trace logs (#40005) This is essentially a replay of #39969, but for the http_endpoint input. 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 + .../docs/inputs/input-http-endpoint.asciidoc | 16 +++++++---- x-pack/filebeat/input/http_endpoint/config.go | 11 +++++++- .../input/http_endpoint/handler_test.go | 8 +++--- x-pack/filebeat/input/http_endpoint/input.go | 28 ++++++++++++++++++- 5 files changed, 52 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 64665025f50..c0926539967 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 http_endpoint input. {pull}40005[40005] *Auditbeat* diff --git a/x-pack/filebeat/docs/inputs/input-http-endpoint.asciidoc b/x-pack/filebeat/docs/inputs/input-http-endpoint.asciidoc index 6acceb3bd50..251163fe432 100644 --- a/x-pack/filebeat/docs/inputs/input-http-endpoint.asciidoc +++ b/x-pack/filebeat/docs/inputs/input-http-endpoint.asciidoc @@ -349,17 +349,21 @@ The HTTP method handled by the endpoint. If specified, `method` must be `POST`, The default method is `POST`. If `PUT` or `PATCH` are specified, requests using those method types are accepted, but are treated as `POST` requests and are expected to have a request body containing the request data. [float] -==== `tracer.filename` +==== `tracer.enabled` It is possible to log HTTP requests 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] ==== `tracer.maxsize` diff --git a/x-pack/filebeat/input/http_endpoint/config.go b/x-pack/filebeat/input/http_endpoint/config.go index 1618dc90758..5e1c93d2bc3 100644 --- a/x-pack/filebeat/input/http_endpoint/config.go +++ b/x-pack/filebeat/input/http_endpoint/config.go @@ -48,7 +48,16 @@ type config struct { CRCSecret string `config:"crc.secret"` IncludeHeaders []string `config:"include_headers"` PreserveOriginalEvent bool `config:"preserve_original_event"` - 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) } func defaultConfig() config { diff --git a/x-pack/filebeat/input/http_endpoint/handler_test.go b/x-pack/filebeat/input/http_endpoint/handler_test.go index b41d20e72c2..f108420fd63 100644 --- a/x-pack/filebeat/input/http_endpoint/handler_test.go +++ b/x-pack/filebeat/input/http_endpoint/handler_test.go @@ -473,7 +473,7 @@ func Test_apiResponse(t *testing.T) { pub := new(publisher) metrics := newInputMetrics("") defer metrics.Close() - apiHandler := newHandler(ctx, tracerConfig(tc.name, tc.conf, *withTraces), nil, pub.Publish, logp.NewLogger("http_endpoint.test"), metrics) + apiHandler := newHandler(ctx, newTracerConfig(tc.name, tc.conf, *withTraces), nil, pub.Publish, logp.NewLogger("http_endpoint.test"), metrics) // Execute handler. respRec := httptest.NewRecorder() @@ -491,12 +491,12 @@ func Test_apiResponse(t *testing.T) { } } -func tracerConfig(name string, cfg config, withTrace bool) config { +func newTracerConfig(name string, cfg config, withTrace bool) config { if !withTrace { return cfg } - cfg.Tracer = &lumberjack.Logger{ + cfg.Tracer = &tracerConfig{Logger: lumberjack.Logger{ Filename: filepath.Join(traceLogsDir, name+".ndjson"), - } + }} return cfg } diff --git a/x-pack/filebeat/input/http_endpoint/input.go b/x-pack/filebeat/input/http_endpoint/input.go index 7f0440deb60..263188a481e 100644 --- a/x-pack/filebeat/input/http_endpoint/input.go +++ b/x-pack/filebeat/input/http_endpoint/input.go @@ -13,9 +13,11 @@ import ( "encoding/json" "errors" "fmt" + "io/fs" "net" "net/http" "net/url" + "os" "path/filepath" "reflect" "strings" @@ -353,7 +355,7 @@ func newHandler(ctx context.Context, c config, prg *program, pub func(beat.Event preserveOriginalEvent: c.PreserveOriginalEvent, crc: newCRC(c.CRCProvider, c.CRCSecret), } - if c.Tracer != nil { + if c.Tracer.enabled() { w := zapcore.AddSync(c.Tracer) go func() { // Close the logger when we are done. @@ -372,10 +374,34 @@ func newHandler(ctx context.Context, c config, prg *program, pub func(beat.Event } else { h.scheme = "http" } + } else if c.Tracer != nil { + // We have a trace log name, but we are not enabled, + // so remove all trace logs we own. + err := os.Remove(c.Tracer.Filename) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + log.Errorw("failed to remove request trace log", "path", c.Tracer.Filename, "error", err) + } + ext := filepath.Ext(c.Tracer.Filename) + base := strings.TrimSuffix(c.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 h } +// 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]" + func htmlEscape(s string) string { var buf bytes.Buffer json.HTMLEscape(&buf, []byte(s))