diff --git a/cmd/cli/cli.go b/cmd/cli/cli.go index 41e3999..842e89f 100644 --- a/cmd/cli/cli.go +++ b/cmd/cli/cli.go @@ -952,6 +952,22 @@ func provisionSecrets() []string { return []string{cdUID, cdOrg, uid, clientID} } +// redactedArgs formats command-line arguments for a log line with the +// provision secrets removed. "ctrld start --cd-org " carries the code +// in its arguments, and a code enrolls endpoints into the organization until +// it expires or runs out. +func redactedArgs(args []string) string { + return redactSecrets(fmt.Sprint(args), provisionSecrets()...) +} + +// interceptUpgradeCheckLine is the debug line "ctrld start" logs before its +// intercept upgrade check. It is built here, not inline, so a test can hold the +// line itself, not only redactedArgs, to never carrying a provision secret. +func interceptUpgradeCheckLine(args []string, interceptOnly, svcConfigExists bool, mode string) string { + return fmt.Sprintf("intercept upgrade check: args=%s interceptOnly=%v svcConfigExists=%v interceptMode=%q", + redactedArgs(args), interceptOnly, svcConfigExists, mode) +} + // uninstallInvalidCdUIDFn is a var so tests can observe the self-uninstall // without driving the OS service manager. var uninstallInvalidCdUIDFn = uninstallInvalidCdUID diff --git a/cmd/cli/commands_service_start.go b/cmd/cli/commands_service_start.go index bb09c5e..584d7fc 100644 --- a/cmd/cli/commands_service_start.go +++ b/cmd/cli/commands_service_start.go @@ -130,7 +130,7 @@ func (sc *ServiceCommand) Start(cmd *cobra.Command, args []string) error { osArgsEarly = filterEmptyStrings(osArgsEarly) interceptOnly := onlyInterceptFlags(osArgsEarly) svcExists := serviceConfigFileExists() - logger.Debug().Msgf("intercept upgrade check: args=%v interceptOnly=%v svcConfigExists=%v interceptMode=%q", osArgsEarly, interceptOnly, svcExists, interceptMode) + logger.Debug().Msg(interceptUpgradeCheckLine(osArgsEarly, interceptOnly, svcExists, interceptMode)) if interceptOnly && svcExists { // An explicit "off" argument must override a previously persisted config // value while the service clears that value on startup. diff --git a/cmd/cli/redacted_args_test.go b/cmd/cli/redacted_args_test.go new file mode 100644 index 0000000..18edeeb --- /dev/null +++ b/cmd/cli/redacted_args_test.go @@ -0,0 +1,56 @@ +package cli + +import ( + "strings" + "testing" +) + +// The arguments of "ctrld start" go into a debug line; neither the +// provision code nor the resolver uid may reach it, in either flag form. +func TestRedactedArgsRemovesProvisionSecrets(t *testing.T) { + origOrg, origUID := cdOrg, cdUID + t.Cleanup(func() { cdOrg, cdUID = origOrg, origUID }) + cdOrg = "org-v1-provisioncode0123" + cdUID = "resolveruid9/client-name" + + got := redactedArgs([]string{"--cd-org", cdOrg, "--cd=" + cdUID, "-vv", "--log", "C:\\ctrld\\ctrld.log"}) + + for _, secret := range []string{cdOrg, cdUID, "resolveruid9", "client-name"} { + if strings.Contains(got, secret) { + t.Fatalf("redactedArgs kept %q: %s", secret, got) + } + } + for _, kept := range []string{"--cd-org [redacted]", "--cd=[redacted]", "-vv", "--log"} { + if !strings.Contains(got, kept) { + t.Fatalf("redactedArgs lost %q: %s", kept, got) + } + } +} + +// The debug line itself, not only the helper it uses, must never carry the +// provision code or the resolver uid (issue #632). +func TestInterceptUpgradeCheckLineRedactsProvisionSecrets(t *testing.T) { + origOrg, origUID := cdOrg, cdUID + t.Cleanup(func() { cdOrg, cdUID = origOrg, origUID }) + cdOrg = "org-v1-provisioncode0123" + cdUID = "resolveruid9/client-name" + + for _, args := range [][]string{ + {"--cd-org", cdOrg, "-vv"}, + {"--cd-org=" + cdOrg, "--intercept-mode", "dns"}, + {"--cd", cdUID, "--log", "/var/log/ctrld.log"}, + {"--cd=" + cdUID}, + } { + line := interceptUpgradeCheckLine(args, false, true, "dns") + for _, secret := range []string{cdOrg, cdUID, "resolveruid9", "client-name"} { + if strings.Contains(line, secret) { + t.Fatalf("line for %q carries %q: %s", args, secret, line) + } + } + for _, kept := range []string{"intercept upgrade check: args=[", "interceptOnly=false", "svcConfigExists=true", `interceptMode="dns"`} { + if !strings.Contains(line, kept) { + t.Fatalf("line lost %q: %s", kept, line) + } + } + } +}