redact sensitive information on diagnostics collect command

. . PR changes pr changes Update CHANGELOG.next.asciidoc Co-authored-by: Michel Laterman <[email protected]>
elastic · Jun 30, 2022 · 06f707c · 06f707c
1 parent 662a07a
commit 06f707c
Show file tree

Hide file tree

Showing 3 changed files with 155 additions and 10 deletions.
diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
@@ -186,3 +186,4 @@
 - Support scheduled actions and cancellation of pending actions. {issue}393[393] {pull}419[419]
 - Add `@metadata.input_id` and `@metadata.stream_id` when applying the inject stream processor {pull}527[527]
 - Add liveness endpoint, allow fleet-gateway component to report degraded state, add update time and messages to status output. {issue}390[390] {pull}569[569]
+- Redact sensitive information on diagnostics collect command. {issue}[241] {pull}[566]
diff --git a/internal/pkg/agent/cmd/diagnostics.go b/internal/pkg/agent/cmd/diagnostics.go
@@ -14,6 +14,7 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
+	"reflect"
 	"runtime"
 	"strings"
 	"text/tabwriter"
@@ -34,10 +35,17 @@ import (
 	"github.com/elastic/elastic-agent/internal/pkg/config/operations"
 )
 
+const (
+	HUMAN    = "human"
+	JSON     = "json"
+	YAML     = "yaml"
+	REDACTED = "<REDACTED>"
+)
+
 var diagOutputs = map[string]outputter{
-	"human": humanDiagnosticsOutput,
-	"json":  jsonOutput,
-	"yaml":  yamlOutput,
+	HUMAN: humanDiagnosticsOutput,
+	JSON:  jsonOutput,
+	YAML:  yamlOutput,
 }
 
 // DiagnosticsInfo a struct to track all information related to diagnostics for the agent.
@@ -83,6 +91,7 @@ func newDiagnosticsCommand(s []string, streams *cli.IOStreams) *cobra.Command {
 }
 
 func newDiagnosticsCollectCommandWithArgs(_ []string, streams *cli.IOStreams) *cobra.Command {
+
 	cmd := &cobra.Command{
 		Use:   "collect",
 		Short: "Collect diagnostics information from the elastic-agent and write it to a zip archive.",
@@ -115,7 +124,7 @@ func newDiagnosticsCollectCommandWithArgs(_ []string, streams *cli.IOStreams) *c
 	}
 
 	cmd.Flags().StringP("file", "f", "", "name of the output diagnostics zip archive")
-	cmd.Flags().String("output", "yaml", "Output the collected information in either json, or yaml (default: yaml)") // replace output flag with different options
+	cmd.Flags().String("output", YAML, "Output the collected information in either json, or yaml (default: yaml)") // replace output flag with different options
 	cmd.Flags().Bool("pprof", false, "Collect all pprof data from all running applications.")
 	cmd.Flags().Duration("pprof-duration", time.Second*30, "The duration to collect trace and profiling data from the debug/pprof endpoints. (default: 30s)")
 	cmd.Flags().Duration("timeout", time.Second*30, "The timeout for the diagnostics collect command, will be either 30s or 30s+pprof-duration by default. Should be longer then pprof-duration when pprof is enabled as the command needs time to process/archive the response.")
@@ -690,16 +699,77 @@ func saveLogs(name string, logPath string, zw *zip.Writer) error {
 
 // writeFile writes json or yaml data from the interface to the writer.
 func writeFile(w io.Writer, outputFormat string, v interface{}) error {
-	if outputFormat == "json" {
+	redacted, err := redact(v)
+	if err != nil {
+		return err
+	}
+
+	if outputFormat == JSON {
 		je := json.NewEncoder(w)
 		je.SetIndent("", "  ")
-		return je.Encode(v)
+		return je.Encode(redacted)
 	}
+
 	ye := yaml.NewEncoder(w)
-	err := ye.Encode(v)
+	err = ye.Encode(redacted)
 	return closeHandlers(err, ye)
 }
 
+func redact(v interface{}) (map[string]interface{}, error) {
+	redacted := map[string]interface{}{}
+	bs, err := yaml.Marshal(v)
+	if err != nil {
+		return nil, fmt.Errorf("could not marshal data to redact: %w", err)
+	}
+
+	err = yaml.Unmarshal(bs, &redacted)
+	if err != nil {
+		return nil, fmt.Errorf("could not unmarshal data to redact: %w", err)
+	}
+
+	return redactMap(redacted), nil
+}
+
+func toMapStr(v interface{}) map[string]interface{} {
+	mm := map[string]interface{}{}
+	m, ok := v.(map[interface{}]interface{})
+	if !ok {
+		return mm
+	}
+
+	for k, v := range m {
+		mm[k.(string)] = v
+	}
+	return mm
+}
+
+func redactMap(m map[string]interface{}) map[string]interface{} {
+	for k, v := range m {
+		if v != nil && reflect.TypeOf(v).Kind() == reflect.Map {
+			v = redactMap(toMapStr(v))
+		}
+		if redactKey(k) {
+			v = REDACTED
+		}
+		m[k] = v
+	}
+	return m
+}
+
+func redactKey(k string) bool {
+	// "routekey" shouldn't be redacted.
+	// Add any other exceptions here.
+	if k == "routekey" {
+		return false
+	}
+
+	return strings.Contains(k, "certificate") ||
+		strings.Contains(k, "passphrase") ||
+		strings.Contains(k, "password") ||
+		strings.Contains(k, "token") ||
+		strings.Contains(k, "key")
+}
+
 // closeHandlers will close all passed closers attaching any errors to the passed err and returning the result
 func closeHandlers(err error, closers ...io.Closer) error {
 	var mErr *multierror.Error

diff --git a/internal/pkg/agent/cmd/diagnostics_test.go b/internal/pkg/agent/cmd/diagnostics_test.go
@@ -15,6 +15,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/elastic/elastic-agent-libs/transport/tlscommon"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -30,7 +31,7 @@ var testDiagnostics = DiagnosticsInfo{
 		BuildTime: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC),
 		Snapshot:  false,
 	},
-	ProcMeta: []client.ProcMeta{client.ProcMeta{
+	ProcMeta: []client.ProcMeta{{
 		Process:            "filebeat",
 		Name:               "filebeat",
 		Hostname:           "test-host",
@@ -45,7 +46,7 @@ var testDiagnostics = DiagnosticsInfo{
 		BinaryArchitecture: "test-architecture",
 		RouteKey:           "test",
 		ElasticLicensed:    true,
-	}, client.ProcMeta{
+	}, {
 		Process:            "filebeat",
 		Name:               "filebeat_monitoring",
 		Hostname:           "test-host",
@@ -60,7 +61,7 @@ var testDiagnostics = DiagnosticsInfo{
 		BinaryArchitecture: "test-architecture",
 		RouteKey:           "test",
 		ElasticLicensed:    true,
-	}, client.ProcMeta{
+	}, {
 		Name:     "metricbeat",
 		RouteKey: "test",
 		Error:    "failed to get metricbeat data",
@@ -137,3 +138,76 @@ func Test_collectEndpointSecurityLogs_noEndpointSecurity(t *testing.T) {
 	err := collectEndpointSecurityLogs(zw, specs)
 	assert.NoError(t, err, "collectEndpointSecurityLogs should not return an error")
 }
+
+func Test_redact(t *testing.T) {
+	tests := []struct {
+		name         string
+		arg          interface{}
+		wantRedacted []string
+		wantErr      assert.ErrorAssertionFunc
+	}{
+		{
+			name: "tlscommon.Config",
+			arg: tlscommon.Config{
+				Enabled:          nil,
+				VerificationMode: 0,
+				Versions:         nil,
+				CipherSuites:     nil,
+				CAs:              []string{"ca1", "ca2"},
+				Certificate: tlscommon.CertificateConfig{
+					Certificate: "Certificate",
+					Key:         "Key",
+					Passphrase:  "Passphrase",
+				},
+				CurveTypes:           nil,
+				Renegotiation:        0,
+				CASha256:             nil,
+				CATrustedFingerprint: "",
+			},
+			wantRedacted: []string{
+				"certificate", "key", "key_passphrase", "certificate_authorities"},
+		},
+		{
+			name: "some map",
+			arg: map[string]interface{}{
+				"s":          "sss",
+				"some_key":   "hey, a key!",
+				"a_password": "changeme",
+				"my_token":   "a_token",
+				"nested": map[string]string{
+					"4242":            "4242",
+					"4242key":         "4242key",
+					"4242password":    "4242password",
+					"4242certificate": "4242certificate",
+				},
+			},
+			wantRedacted: []string{
+				"some_key", "a_password", "my_token", "4242key", "4242password", "4242certificate"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := redact(tt.arg)
+			require.NoError(t, err)
+
+			for k, v := range got {
+				if contains(tt.wantRedacted, k) {
+					assert.Equal(t, v, REDACTED)
+				} else {
+					assert.NotEqual(t, v, REDACTED)
+				}
+			}
+		})
+	}
+}
+
+func contains(list []string, val string) bool {
+	for _, k := range list {
+		if val == k {
+			return true
+		}
+	}
+
+	return false
+}