Skip to content

Commit

Permalink
x-pack/filebeat/input/http_endpoint: add ability to remove request tr…
Browse files Browse the repository at this point in the history
…ace logs

This is essentially a replay of elastic#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.
  • Loading branch information
efd6 committed Jun 25, 2024
1 parent 463dc4e commit 92c9359
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff]
- Allow elision of set and append failure logging. {issue}34544[34544] {pull}39929[39929]
- Add ability to remove request trace logs from CEL input. {pull}39969[39969]
- Add ability to remove request trace logs from HTTPJSON input. {pull}40003[40003]
- Add ability to remove request trace logs from http_endpoint input. {pull}40005[40005]

*Auditbeat*

Expand Down
16 changes: 10 additions & 6 deletions x-pack/filebeat/docs/inputs/input-http-endpoint.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down
11 changes: 10 additions & 1 deletion x-pack/filebeat/input/http_endpoint/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions x-pack/filebeat/input/http_endpoint/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
}
28 changes: 27 additions & 1 deletion x-pack/filebeat/input/http_endpoint/input.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ import (
"encoding/json"
"errors"
"fmt"
"io/fs"
"net"
"net/http"
"net/url"
"os"
"path/filepath"
"reflect"
"strings"
Expand Down Expand Up @@ -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.
Expand All @@ -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))
Expand Down

0 comments on commit 92c9359

Please sign in to comment.