Skip to content

Commit

Permalink
redact secret_paths from elastic-agent inspect output (elastic#5621)
Browse files Browse the repository at this point in the history
redact secret_paths from elastic-agent inspect output.
elastic-agent inspect will now redact the value for any key in the secret_paths array.
secret_paths is expected to be part of the policy.
  • Loading branch information
michel-laterman authored Oct 7, 2024
1 parent 862b044 commit d7546dd
Show file tree
Hide file tree
Showing 7 changed files with 365 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Kind can be one of:
# - breaking-change: a change to previously-documented behavior
# - deprecation: functionality that is being removed in a later release
# - bug-fix: fixes a problem in a previous version
# - enhancement: extends functionality but does not break or fix existing behavior
# - feature: new functionality
# - known-issue: problems that we are aware of in a given version
# - security: impacts on the security of a product or a user’s deployment.
# - upgrade: important information for someone upgrading from a prior version
# - other: does not fit into any of the other categories
kind: enhancement

# Change summary; a 80ish characters long description of the change.
summary: inspect command will redact secret_paths in policy

# Long description; in case the summary is not enough to describe the change
# this field accommodate a description without length limits.
# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
description: |
The elastic-agent inspect command will now redact any secret values when displaying output.
The keys that are redacted are expected to be defined in the "secret_paths" attribute.
# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
component:

# PR URL; optional; the PR number that added the changeset.
# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
# Please provide it if you are adding a fragment for a different PR.
#pr: https://github.com/owner/repo/1234

# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
# If not present is automatically filled by the tooling with the issue linked to the PR number.
#issue: https://github.com/owner/repo/1234
3 changes: 2 additions & 1 deletion internal/pkg/agent/cmd/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/elastic/elastic-agent/internal/pkg/cli"
"github.com/elastic/elastic-agent/internal/pkg/config"
"github.com/elastic/elastic-agent/internal/pkg/config/operations"
"github.com/elastic/elastic-agent/internal/pkg/diagnostics"
"github.com/elastic/elastic-agent/pkg/component"
"github.com/elastic/elastic-agent/pkg/core/logger"
"github.com/elastic/elastic-agent/pkg/utils"
Expand Down Expand Up @@ -235,7 +236,7 @@ func inspectConfig(ctx context.Context, cfgPath string, opts inspectConfigOpts,
}

func printMapStringConfig(mapStr map[string]interface{}, streams *cli.IOStreams) error {
data, err := yaml.Marshal(mapStr)
data, err := yaml.Marshal(diagnostics.RedactSecretPaths(mapStr, streams.Err))
if err != nil {
return errors.New(err, "could not marshal to YAML")
}
Expand Down
38 changes: 38 additions & 0 deletions internal/pkg/diagnostics/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ import (
"time"

"github.com/elastic/elastic-agent/pkg/control/v2/client"
"github.com/elastic/go-ucfg"

"gopkg.in/yaml.v3"

"github.com/elastic/elastic-agent/internal/pkg/agent/application/paths"
"github.com/elastic/elastic-agent/internal/pkg/config"
"github.com/elastic/elastic-agent/internal/pkg/release"
"github.com/elastic/elastic-agent/pkg/component"
"github.com/elastic/elastic-agent/version"
Expand Down Expand Up @@ -570,3 +572,39 @@ func saveLogs(name string, logPath string, zw *zip.Writer) error {

return nil
}

// RedactSecretPaths will check the passed mapStr input for a secret_paths attribute.
// If found it will replace the value for every key in the paths list with <REDACTED> and return the resulting map.
// Any issues or errors will be written to the errOut writer.
func RedactSecretPaths(mapStr map[string]any, errOut io.Writer) map[string]any {
v, ok := mapStr["secret_paths"]
if !ok {
fmt.Fprintln(errOut, "No output redaction: secret_paths attribute not found.")
return mapStr
}
arr, ok := v.([]interface{})
if !ok {
fmt.Fprintln(errOut, "No output redaction: secret_paths attribute is not a list.")
return mapStr
}
cfg := ucfg.MustNewFrom(mapStr)
for _, v := range arr {
key, ok := v.(string)
if !ok {
fmt.Fprintf(errOut, "No output redaction for %q: expected type string, is type %T.\n", v, v)
continue
}

if ok, _ := cfg.Has(key, -1, ucfg.PathSep(".")); ok {
err := cfg.SetString(key, -1, REDACTED, ucfg.PathSep("."))
if err != nil {
fmt.Fprintf(errOut, "No output redaction for %q: %v.\n", key, err)
}
}
}
result, err := config.MustNewConfigFrom(cfg).ToMapStr()
if err != nil {
return mapStr
}
return result
}
194 changes: 194 additions & 0 deletions internal/pkg/diagnostics/diagnostics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,3 +340,197 @@ func isPprof(input []byte) (bool, error) {
}
return true, nil
}

func TestRedactSecretPaths(t *testing.T) {
tests := []struct {
name string
input map[string]interface{}
expect map[string]interface{}
}{{
name: "no secret_paths",
input: map[string]interface{}{
"outputs": map[string]interface{}{
"default": map[string]interface{}{
"type": "elasticsearch",
"api_key": "apikeyvalue",
},
},
"inputs": []interface{}{
map[string]interface{}{
"type": "example",
"secret": "secretvalue",
},
},
},
expect: map[string]interface{}{
"outputs": map[string]interface{}{
"default": map[string]interface{}{
"type": "elasticsearch",
"api_key": "apikeyvalue",
},
},
"inputs": []interface{}{
map[string]interface{}{
"type": "example",
"secret": "secretvalue",
},
},
},
}, {
name: "secret paths is not an array",
input: map[string]interface{}{
"secret_paths": "inputs.0.secret,outputs.default.api_key",
"outputs": map[string]interface{}{
"default": map[string]interface{}{
"type": "elasticsearch",
"api_key": "apikeyvalue",
},
},
"inputs": []interface{}{
map[string]interface{}{
"type": "example",
"secret": "secretvalue",
},
},
},
expect: map[string]interface{}{
"secret_paths": "inputs.0.secret,outputs.default.api_key",
"outputs": map[string]interface{}{
"default": map[string]interface{}{
"type": "elasticsearch",
"api_key": "apikeyvalue",
},
},
"inputs": []interface{}{
map[string]interface{}{
"type": "example",
"secret": "secretvalue",
},
},
},
}, {
name: "secret_paths are redacted",
input: map[string]interface{}{
"secret_paths": []interface{}{
"inputs.0.secret",
"outputs.default.api_key",
},
"outputs": map[string]interface{}{
"default": map[string]interface{}{
"type": "elasticsearch",
"api_key": "apikeyvalue",
},
},
"inputs": []interface{}{
map[string]interface{}{
"type": "example",
"secret": "secretvalue",
},
},
},
expect: map[string]interface{}{
"secret_paths": []interface{}{
"inputs.0.secret",
"outputs.default.api_key",
},
"outputs": map[string]interface{}{
"default": map[string]interface{}{
"type": "elasticsearch",
"api_key": REDACTED,
},
},
"inputs": []interface{}{
map[string]interface{}{
"type": "example",
"secret": REDACTED,
},
},
},
}, {
name: "secret_paths contains extra keys",
input: map[string]interface{}{
"secret_paths": []interface{}{
"inputs.0.secret",
"outputs.default.api_key",
"inputs.1.secret",
},
"outputs": map[string]interface{}{
"default": map[string]interface{}{
"type": "elasticsearch",
"api_key": "apikeyvalue",
},
},
"inputs": []interface{}{
map[string]interface{}{
"type": "example",
"secret": "secretvalue",
},
},
},
expect: map[string]interface{}{
"secret_paths": []interface{}{
"inputs.0.secret",
"outputs.default.api_key",
"inputs.1.secret",
},
"outputs": map[string]interface{}{
"default": map[string]interface{}{
"type": "elasticsearch",
"api_key": REDACTED,
},
},
"inputs": []interface{}{
map[string]interface{}{
"type": "example",
"secret": REDACTED,
},
},
},
}, {
name: "secret_paths contains non string key",
input: map[string]interface{}{
"secret_paths": []interface{}{
"inputs.0.secret",
"outputs.default.api_key",
2,
},
"outputs": map[string]interface{}{
"default": map[string]interface{}{
"type": "elasticsearch",
"api_key": "apikeyvalue",
},
},
"inputs": []interface{}{
map[string]interface{}{
"type": "example",
"secret": "secretvalue",
},
},
},
expect: map[string]interface{}{
"secret_paths": []interface{}{
"inputs.0.secret",
"outputs.default.api_key",
uint64(2), // go-ucfg serializing/deserializing flattens types
},
"outputs": map[string]interface{}{
"default": map[string]interface{}{
"type": "elasticsearch",
"api_key": REDACTED,
},
},
"inputs": []interface{}{
map[string]interface{}{
"type": "example",
"secret": REDACTED,
},
},
},
}}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
result := RedactSecretPaths(tc.input, io.Discard)
assert.Equal(t, tc.expect, result)
})
}
}
2 changes: 2 additions & 0 deletions testing/fleetservertest/checkin.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,11 @@ const (
"hosts": {{ toJson .FleetHosts }}
},
"id": "{{.PolicyID}}",
"secret_paths": ["inputs.0.secret_key"],
"inputs": [
{
"id": "fake-input",
"secret_key": "secretValue",
"revision": 1,
"name": "fake-input",
"type": "fake-input",
Expand Down
4 changes: 4 additions & 0 deletions testing/fleetservertest/fleetserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ func ExampleNewServer_checkin_fleetConnectionParams() {
// "name": "fake-input",
// "package_policy_id": "",
// "revision": 1,
// "secret_key": "secretValue",
// "streams": [],
// "type": "fake-input",
// "use_output": "default"
Expand All @@ -287,6 +288,9 @@ func ExampleNewServer_checkin_fleetConnectionParams() {
// }
// },
// "revision": 2,
// "secret_paths": [
// "inputs.0.secret_key"
// ],
// "secret_references": [],
// "signed": {
// "data": "eyJpZCI6IjI0ZTRkMDMwLWZmYTctMTFlZC1iMDQwLTlkZWJhYTVmZWNiOCIsImFnZW50Ijp7InByb3RlY3Rpb24iOnsiZW5hYmxlZCI6ZmFsc2UsInVuaW5zdGFsbF90b2tlbl9oYXNoIjoibE9SU2FESVFxNG5nbFVNSndXaktyd2V4ajRJRFJKQStGdG9RZWVxS0gvST0iLCJzaWduaW5nX2tleSI6Ik1Ga3dFd1lIS29aSXpqMENBUVlJS29aSXpqMERBUWNEUWdBRVE5QlBvSFVDeUx5RWxWcGZ3dktlRmRVdDZVOXdCYitRbFpOZjRjeTVlQXdLOVhoNEQ4ZkNsZ2NpUGVSczNqNjJpNklFZUd5dk9zOVUzK2ZFbHlVaWdnPT0ifX19",
Expand Down
Loading

0 comments on commit d7546dd

Please sign in to comment.