diff --git a/app/stacksnipe/stacksnipe.go b/app/stacksnipe/stacksnipe.go index b3a87cb594..7efdb2e464 100644 --- a/app/stacksnipe/stacksnipe.go +++ b/app/stacksnipe/stacksnipe.go @@ -8,6 +8,7 @@ import ( "io/fs" "os" "path/filepath" + "regexp" "strconv" "strings" "time" @@ -38,6 +39,31 @@ var maybeVCs = map[string]struct{}{ "node": {}, } +// sensitiveFlagFragments are the flag name fragments whose values must never leave the process. +// Validator client command lines routinely carry keystore passwords, keymanager bearer tokens and +// the paths of files holding them, none of which belong in a metric label or a log line. +var sensitiveFlagFragments = []string{ + "auth", + "jwt", + "key", + "mnemonic", + "passphrase", + "password", + "secret", + "token", +} + +// redactedValue replaces the value of a sensitive flag in an exported command line. +const redactedValue = "" + +// basicAuthRe matches the userinfo of a URL so a credential embedded in an otherwise innocuous +// flag value (e.g. https://user:pass@host) is redacted while the scheme, user and host stay visible. +var basicAuthRe = regexp.MustCompile(`([a-zA-Z][a-zA-Z0-9+.-]*://[^:@/?#\s]+):[^@/?#\s]+@`) + +// queryParamRe matches a single URL query parameter so sensitive ones (?token=..., &jwt=...) +// can be redacted by name while the rest of the value is preserved. +var queryParamRe = regexp.MustCompile(`([?&])([^=&#\s]+)=([^&#\s]*)`) + // StackComponent is a named process of the Ethereum validator stack running on the machine, // whose CLI parameters (also called cmdline) is read from a /proc-like filesystem. type StackComponent struct { @@ -79,6 +105,10 @@ func (i *Instance) Run(ctx context.Context) { return } + log.Info(ctx, "Stack component sniping enabled: command lines of detected validator clients are exported "+ + "to the monitoring endpoint and debug logs, with the values of secret-shaped flags redacted", + z.Str("proc_directory", i.procPath)) + ticker := time.NewTicker(i.interval) defer ticker.Stop() @@ -135,6 +165,157 @@ func snipe(ctx context.Context, procPath string) ([]StackComponent, error) { return ret, nil } +// isSensitiveFlag reports whether name looks like a flag whose value carries secret material. +func isSensitiveFlag(name string) bool { + name = strings.ToLower(strings.TrimLeft(name, "-")) + + for _, fragment := range sensitiveFlagFragments { + if strings.Contains(name, fragment) { + return true + } + } + + return false +} + +// redactValue scrubs secret material embedded inside a value that a flag name check misses: +// basic-auth credentials in a URL and the values of sensitive query parameters. The rest of the +// value (scheme, host, path, innocuous parameters) is preserved so telemetry stays useful. +func redactValue(value string) string { + value = basicAuthRe.ReplaceAllString(value, "${1}:"+redactedValue+"@") + + value = queryParamRe.ReplaceAllStringFunc(value, func(param string) string { + groups := queryParamRe.FindStringSubmatch(param) + if groups[3] != "" && isSensitiveFlag(groups[2]) { + return groups[1] + groups[2] + "=" + redactedValue + } + + return param + }) + + return value +} + +// splitCmdlineBlob tokenises a whole command line that arrived as a single blob, honouring single +// and double quotes so a quoted value containing spaces stays one argument (and is redacted whole) +// rather than being split into leaking fragments. +func splitCmdlineBlob(blob string) []string { + var ( + tokens []string + cur strings.Builder + quote rune + inTok bool + ) + + flush := func() { + if inTok { + tokens = append(tokens, cur.String()) + cur.Reset() + + inTok = false + } + } + + for _, r := range blob { + switch { + case quote != 0: + if r == quote { + quote = 0 + } else { + cur.WriteRune(r) + } + + inTok = true + case r == '\'' || r == '"': + quote = r + inTok = true + case r == ' ' || r == '\t': + flush() + default: + cur.WriteRune(r) + + inTok = true + } + } + + flush() + + return tokens +} + +// redactCmdline redacts the value of every sensitive flag while keeping flag names and innocuous +// values, so the exported command line stays diagnostically useful. It covers the "--flag value" +// and "--flag=value" forms, secrets embedded in a URL value of an innocuous flag (see redactValue), +// and values beginning with "-". Args are normally the NUL separated /proc elements; a whole command +// line handed over as one blob is tokenised (see splitCmdlineBlob) and its sensitive values redacted +// greedily, so redaction fails safe rather than leaking. +func redactCmdline(args []string) []string { + greedy := false + + if len(args) == 1 && strings.ContainsAny(args[0], " \t") { + args = splitCmdlineBlob(args[0]) + greedy = true + } + + redacted := make([]string, 0, len(args)) + + var ( + redactNext bool // the following token(s) are the value of a sensitive flag + valueEmitted bool // the single marker for that value has already been appended + ) + + for _, arg := range args { + if redactNext { + // A long flag means the sensitive flag took no value; stop redacting and reprocess + // this token as a flag. Anything else is (part of) the value. + if !strings.HasPrefix(arg, "--") { + if !valueEmitted { + redacted = append(redacted, redactedValue) + valueEmitted = true + } + + // A NUL separated cmdline gives one value per flag; only the ambiguous blob + // fallback keeps consuming tokens into the same value. + if !greedy { + redactNext = false + } + + continue + } + + redactNext = false + } + + if !strings.HasPrefix(arg, "-") { + redacted = append(redacted, redactValue(arg)) + continue + } + + name, value, hasValue := strings.Cut(arg, "=") + if !isSensitiveFlag(name) { + if hasValue { + redacted = append(redacted, name+"="+redactValue(value)) + } else { + redacted = append(redacted, arg) + } + + continue + } + + if hasValue { + redacted = append(redacted, name+"="+redactedValue) + continue + } + + redactNext = true + valueEmitted = false + + redacted = append(redacted, arg) + } + + return redacted +} + // walkFunc walks a /proc-like filesystem as invoked by filepath.WalkDir, and sends entries to wb. func walkFunc(ctx context.Context, wb chan<- StackComponent) fs.WalkDirFunc { cmdlineDedup := make(map[string]struct{}) @@ -216,7 +397,7 @@ func walkFunc(ctx context.Context, wb chan<- StackComponent) fs.WalkDirFunc { return nil } - cmdLineStr := strings.Join(cmdLine, " ") + cmdLineStr := strings.Join(redactCmdline(cmdLine), " ") log.Debug(ctx, "Detected stack component", z.Str("name", vcName), z.U64("host_pid", hostPID), z.Str("cmdline", cmdLineStr)) diff --git a/app/stacksnipe/stacksnipe_internal_test.go b/app/stacksnipe/stacksnipe_internal_test.go new file mode 100644 index 0000000000..2a6a456a87 --- /dev/null +++ b/app/stacksnipe/stacksnipe_internal_test.go @@ -0,0 +1,134 @@ +// Copyright © 2022-2026 Obol Labs Inc. Licensed under the terms of a Business Source License 1.1 + +package stacksnipe + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestRedactCmdline(t *testing.T) { + tests := []struct { + name string + args []string + want []string + }{ + { + name: "sensitive flag space form", + args: []string{"--keystore-password", "SuperSecretPw123!"}, + want: []string{"--keystore-password", redactedValue}, + }, + { + name: "sensitive flag equals form", + args: []string{"--keymanager-auth-token=eyJhbGciOiJSUzI1NiJ9.secret"}, + want: []string{"--keymanager-auth-token=" + redactedValue}, + }, + { + name: "non-sensitive flags untouched", + args: []string{"--datadir", "/var/lib/lighthouse", "--debug-level", "info"}, + want: []string{"--datadir", "/var/lib/lighthouse", "--debug-level", "info"}, + }, + { + name: "value starting with a dash is still redacted", + args: []string{"--keystore-password", "-hunter2", "--debug-level", "info"}, + want: []string{"--keystore-password", redactedValue, "--debug-level", "info"}, + }, + { + name: "sensitive boolean flag followed by a long flag", + args: []string{"--keymanager", "--datadir", "/var/lib/teku"}, + want: []string{"--keymanager", "--datadir", "/var/lib/teku"}, + }, + { + name: "basic-auth credentials in a URL value of an innocuous flag", + args: []string{"--beacon-nodes", "https://user:s3cret@bn.internal:5052"}, + want: []string{"--beacon-nodes", "https://user:" + redactedValue + "@bn.internal:5052"}, + }, + { + name: "basic-auth credentials in the equals form", + args: []string{"--beacon-nodes=https://user:s3cret@bn.internal"}, + want: []string{"--beacon-nodes=https://user:" + redactedValue + "@bn.internal"}, + }, + { + name: "sensitive query parameter in a URL value", + args: []string{"--metrics-url", "https://push.internal/ingest?token=abc123&interval=5s"}, + want: []string{"--metrics-url", "https://push.internal/ingest?token=" + redactedValue + "&interval=5s"}, + }, + { + name: "innocuous query parameters untouched", + args: []string{"--beacon-nodes", "https://bn.internal?timeout=5s&retries=3"}, + want: []string{"--beacon-nodes", "https://bn.internal?timeout=5s&retries=3"}, + }, + { + name: "plain host:port URL without credentials untouched", + args: []string{"--validators-external-signer-url", "https://signer.internal:9000"}, + want: []string{"--validators-external-signer-url", "https://signer.internal:9000"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, redactCmdline(tt.args)) + }) + } +} + +// TestRedactCmdlineBlob covers the fail-safe fallback for a whole command line that arrives as a +// single blob rather than the NUL separated form a real /proc exposes. +func TestRedactCmdlineBlob(t *testing.T) { + tests := []struct { + name string + blob string + wantContain []string + wantNotContain []string + }{ + { + name: "unquoted multi word value redacts every token, not just the first", + blob: "lighthouse vc --keystore-password My Secret Pw --debug-level info", + wantContain: []string{"--keystore-password " + redactedValue, "--debug-level info"}, + wantNotContain: []string{"My", "Secret", "Pw"}, + }, + { + name: "quoted value with spaces stays one argument and is redacted whole", + blob: `lighthouse vc --keystore-password "alpha beta gamma" --debug-level info`, + wantContain: []string{"--keystore-password " + redactedValue, "--debug-level info"}, + wantNotContain: []string{"alpha", "beta", "gamma"}, + }, + { + name: "single quotes are honoured too", + blob: `teku vc --validators-keystore-password 'p a s s'`, + wantContain: []string{"--validators-keystore-password " + redactedValue}, + wantNotContain: []string{"p a s s", "a s s"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := strings.Join(redactCmdline([]string{tt.blob}), " ") + + for _, want := range tt.wantContain { + require.Contains(t, got, want) + } + + for _, notWant := range tt.wantNotContain { + require.NotContains(t, got, notWant) + } + }) + } +} + +func TestIsSensitiveFlag(t *testing.T) { + sensitive := []string{ + "--keystore-password", "--jwt-secret", "--api-token", "--wallet-passphrase", + "--mnemonic-file", "--auth-token", "--graffiti-key", + } + for _, name := range sensitive { + require.True(t, isSensitiveFlag(name), name) + } + + innocuous := []string{"--datadir", "--debug-level", "--suggested-fee-recipient", "--network"} + for _, name := range innocuous { + require.False(t, isSensitiveFlag(name), name) + } +} diff --git a/app/stacksnipe/stacksnipe_test.go b/app/stacksnipe/stacksnipe_test.go index cc8aecb2d7..c77b17eb24 100644 --- a/app/stacksnipe/stacksnipe_test.go +++ b/app/stacksnipe/stacksnipe_test.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "strconv" + "strings" "testing" "time" @@ -103,6 +104,106 @@ func Test_StackSnipe(t *testing.T) { require.NotContains(t, result.cmdlines, extraCmdlines[idx]) } }) + + t.Run("redacts secret flag values", func(t *testing.T) { + baseDir := t.TempDir() + + // A real /proc cmdline is NUL separated, one argument per element. + populateProc(t, baseDir, procEntry{ + pid: 42, + procName: "lighthouse", + cmdline: strings.Join([]string{ + "/usr/bin/lighthouse", "vc", + "--datadir", "/var/lib/lighthouse", + "--validators-keystore-password-file", "/run/secrets/vc-password.txt", + "--keystore-password", "SuperSecretPw123!", + "--suggested-fee-recipient", "0xdeadbeef", + "--debug-level", "info", + }, "\x00"), + }) + + populateProc(t, baseDir, procEntry{ + pid: 43, + procName: "teku", + cmdline: strings.Join([]string{ + "/usr/bin/teku", "validator-client", + "--data-path", "/var/lib/teku", + "--keymanager-auth-token=eyJhbGciOiJSUzI1NiJ9.secret-jwt", + "--validators-external-signer-url", "https://signer.internal:9000", + }, "\x00"), + }) + + var ( + ctx, cancel = context.WithCancel(context.Background()) + resultChan = make(chan snipeResult) + ) + + defer cancel() + + snipe := stacksnipe.NewWithInterval(baseDir, func(names []string, cmdlines []string) { + resultChan <- snipeResult{names: names, cmdlines: cmdlines} + + cancel() + }, 50*time.Millisecond) + + go snipe.Run(ctx) + + result := <-resultChan + require.Len(t, result.cmdlines, 2) + + exported := strings.Join(result.cmdlines, "\n") + + // No secret material survives export, in either the "--flag value" or the "--flag=value" form. + require.NotContains(t, exported, "SuperSecretPw123!") + require.NotContains(t, exported, "/run/secrets/vc-password.txt") + require.NotContains(t, exported, "eyJhbGciOiJSUzI1NiJ9.secret-jwt") + + // The flag names themselves are kept, so the telemetry stays useful. + require.Contains(t, exported, "--validators-keystore-password-file ") + require.Contains(t, exported, "--keystore-password ") + require.Contains(t, exported, "--keymanager-auth-token=") + + // Non-sensitive flags and their values are untouched. + require.Contains(t, exported, "--datadir /var/lib/lighthouse") + require.Contains(t, exported, "--suggested-fee-recipient 0xdeadbeef") + require.Contains(t, exported, "--debug-level info") + require.Contains(t, exported, "--data-path /var/lib/teku") + require.Contains(t, exported, "--validators-external-signer-url https://signer.internal:9000") + }) + + // Redaction must not silently no-op if the whole command line ever arrives as a single + // argument rather than the NUL separated form a real /proc exposes. + t.Run("redacts secret flag values in an unsplit command line", func(t *testing.T) { + baseDir := t.TempDir() + + populateProc(t, baseDir, procEntry{ + pid: 42, + procName: "lighthouse", + cmdline: "/usr/bin/lighthouse vc --keystore-password SuperSecretPw123! --debug-level info", + }) + + var ( + ctx, cancel = context.WithCancel(context.Background()) + resultChan = make(chan snipeResult) + ) + + defer cancel() + + snipe := stacksnipe.NewWithInterval(baseDir, func(names []string, cmdlines []string) { + resultChan <- snipeResult{names: names, cmdlines: cmdlines} + + cancel() + }, 50*time.Millisecond) + + go snipe.Run(ctx) + + result := <-resultChan + require.Len(t, result.cmdlines, 1) + + require.NotContains(t, result.cmdlines[0], "SuperSecretPw123!") + require.Contains(t, result.cmdlines[0], "--keystore-password ") + require.Contains(t, result.cmdlines[0], "--debug-level info") + }) } func populateProc(t *testing.T, base string, entry procEntry) { diff --git a/cmd/run.go b/cmd/run.go index b98d0a24d5..6dea05dbb2 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -97,7 +97,7 @@ func bindRunFlags(cmd *cobra.Command, config *app.Config) { cmd.Flags().Uint64Var(&config.TestnetConfig.ChainID, "testnet-chain-id", 0, "Chain ID of the custom test network.") cmd.Flags().Int64Var(&config.TestnetConfig.GenesisTimestamp, "testnet-genesis-timestamp", 0, "Genesis timestamp of the custom test network.") cmd.Flags().StringVar(&config.TestnetConfig.CapellaHardFork, "testnet-capella-hard-fork", "", "Capella hard fork version of the custom test network.") - cmd.Flags().StringVar(&config.ProcDirectory, "proc-directory", "", "Directory to look into in order to detect other stack components running on the host.") + cmd.Flags().StringVar(&config.ProcDirectory, "proc-directory", "", "Directory to look into in order to detect other stack components running on the host. Enabling this exports the command lines of detected validator clients to the monitoring endpoint and debug logs, with the values of secret-shaped flags redacted.") cmd.Flags().StringVar(&config.ConsensusProtocol, "consensus-protocol", "", "Preferred consensus protocol name for the node. Selected automatically when not specified.") cmd.Flags().StringVar(&config.Nickname, "nickname", "", "Human friendly peer nickname. Maximum 32 characters.") cmd.Flags().StringSliceVar(&config.BeaconNodeHeaders, "beacon-node-headers", nil, "Comma separated list of headers formatted as header=value") diff --git a/docs/configuration.md b/docs/configuration.md index 09cb850501..296bb7d916 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -199,7 +199,7 @@ Flags: --p2p-udp-address strings Comma-separated list of listening UDP addresses (ip and port) for libP2P traffic. Empty default doesn't bind to local port therefore only supports outgoing connections. --private-key-file string The path to the charon enr private key file. (default ".charon/charon-enr-private-key") --private-key-file-lock Enables private key locking to prevent multiple instances using the same key. - --proc-directory string Directory to look into in order to detect other stack components running on the host. + --proc-directory string Directory to look into in order to detect other stack components running on the host. Enabling this exports the command lines of detected validator clients to the monitoring endpoint and debug logs, with the values of secret-shaped flags redacted. --publish-address string The URL of the remote API for background fee recipient fetching. (default "https://api.obol.tech/v1") --publish-timeout duration Timeout for accessing the remote API. (default 5m0s) --simnet-beacon-mock Enables an internal mock beacon node for running a simnet.