From cc5eb9209b9d5dee5d167a17d75bac31406dea2d Mon Sep 17 00:00:00 2001 From: Andrew Cholakian Date: Tue, 2 Nov 2021 18:50:53 -0500 Subject: [PATCH] [Heartbeat] Stop logging sensitive params (#28774) * [Heartbeat] Stop logging sensitive params This change causes heartbeat to no longer log parameter values, instead interpolating an opaque string. Fixes #28771 We now create logs like: ``` 2021-11-02T17:25:27.804-0500 INFO synthexec/synthexec.go:148 Running command: /tmp/elastic-synthetics-unzip-714880603/node_modules/.bin/elastic-synthetics /tmp/elastic-synthetics-unzip-714880603/node_modules/.bin/elastic-synthetics /tmp/elastic-synthetics-unzip-714880603 --screenshots on --rich-events --params "{1 hidden params}" in directory: '/tmp/elastic-synthetics-unzip-714880603' ``` * Changelog (cherry picked from commit 40e949d8f100dc318d5298b227c0198b1499d2d8) --- CHANGELOG.next.asciidoc | 1 + .../monitors/browser/synthexec/synthexec.go | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 450e2c0a4b2..222b6f3b410 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -151,6 +151,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Fix broken seccomp filtering and improve security via `setcap` and `setuid` when running as root on linux in containers. {pull}27878[27878] - Log browser `zip_url` download failures as `warn` instead of as `info`. {pull}28440[28440] - Properly locate base stream in fleet configs. {pull}28455[28455] +- Stop logging params values. {pull}28774[28774] *Journalbeat* diff --git a/x-pack/heartbeat/monitors/browser/synthexec/synthexec.go b/x-pack/heartbeat/monitors/browser/synthexec/synthexec.go index 31ab4d2fc24..e5bcd1332f5 100644 --- a/x-pack/heartbeat/monitors/browser/synthexec/synthexec.go +++ b/x-pack/heartbeat/monitors/browser/synthexec/synthexec.go @@ -122,11 +122,6 @@ func runCmd( cmd.Env = append(os.Environ(), "NODE_ENV=production") cmd.Args = append(cmd.Args, "--rich-events") - if len(params) > 0 { - paramsBytes, _ := json.Marshal(params) - cmd.Args = append(cmd.Args, "--params", string(paramsBytes)) - } - if len(filterJourneys.Tags) > 0 { cmd.Args = append(cmd.Args, "--tags", strings.Join(filterJourneys.Tags, " ")) } @@ -135,6 +130,14 @@ func runCmd( cmd.Args = append(cmd.Args, "--match", filterJourneys.Match) } + // Variant of the command with no params, which could contain sensitive stuff + loggableCmd := exec.Command(cmd.Path, cmd.Args...) + if len(params) > 0 { + paramsBytes, _ := json.Marshal(params) + cmd.Args = append(cmd.Args, "--params", string(paramsBytes)) + loggableCmd.Args = append(loggableCmd.Args, "--params", fmt.Sprintf("\"{%d hidden params}\"", len(params))) + } + // We need to pass both files in here otherwise we get a broken pipe, even // though node only touches the writer cmd.ExtraFiles = []*os.File{jsonWriter, jsonReader} @@ -142,7 +145,7 @@ func runCmd( // see the docs for ExtraFiles in https://golang.org/pkg/os/exec/#Cmd cmd.Args = append(cmd.Args, "--outfd", "3") - logp.Info("Running command: %s in directory: '%s'", cmd.String(), cmd.Dir) + logp.Info("Running command: %s in directory: '%s'", loggableCmd.String(), cmd.Dir) if stdinStr != nil { logp.Debug(debugSelector, "Using stdin str %s", *stdinStr) @@ -189,14 +192,14 @@ func runCmd( err := cmd.Wait() jsonWriter.Close() jsonReader.Close() - logp.Info("Command has completed(%d): %s", cmd.ProcessState.ExitCode(), cmd.String()) + logp.Info("Command has completed(%d): %s", cmd.ProcessState.ExitCode(), loggableCmd.String()) if err != nil { str := fmt.Sprintf("command exited with status %d: %s", cmd.ProcessState.ExitCode(), err) mpx.writeSynthEvent(&SynthEvent{ Type: "cmd/status", Error: &SynthError{Name: "cmdexit", Message: str}, }) - logp.Warn("Error executing command '%s' (%d): %s", cmd.String(), cmd.ProcessState.ExitCode(), err) + logp.Warn("Error executing command '%s' (%d): %s", loggableCmd.String(), cmd.ProcessState.ExitCode(), err) } wg.Wait() mpx.Close()