From 276ffa338effbcdc1c05865757b1da111256304a Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Thu, 11 Aug 2016 15:07:08 +0000 Subject: [PATCH 1/2] Elide service-token when logging commandline arguments --- prog/app.go | 3 +-- prog/main.go | 23 ++++++++++++++++++++++- prog/probe.go | 2 +- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/prog/app.go b/prog/app.go index ae800cc06..10e2285c1 100644 --- a/prog/app.go +++ b/prog/app.go @@ -6,7 +6,6 @@ import ( "net/http" _ "net/http/pprof" "net/url" - "os" "regexp" "runtime" "strconv" @@ -213,7 +212,7 @@ func appMain(flags appFlags) { app.UniqueID = strconv.FormatInt(rand.Int63(), 16) app.Version = version log.Infof("app starting, version %s, ID %s", app.Version, app.UniqueID) - log.Infof("command line: %v", os.Args) + logCensoredArgs() userIDer := multitenant.NoopUserIDer if flags.userIDHeader != "" { diff --git a/prog/main.go b/prog/main.go index 0810a5b24..b0f839859 100644 --- a/prog/main.go +++ b/prog/main.go @@ -19,6 +19,8 @@ import ( var version = "dev" // set at build time +const tokenFlag = "service-token" + type prefixFormatter struct { prefix []byte next log.Formatter @@ -119,6 +121,25 @@ type appFlags struct { consulInf string } +func logCensoredArgs() { + var prettyPrintedArgs string + // we show the args followed by the flags which is likely to change the + // original ordering. However the flag parser doesn't keep positioning + // information to allow reconstructing it more accurately. + for _, arg := range flag.Args() { + prettyPrintedArgs += " " + arg + } + flag.Visit(func(f *flag.Flag) { + value := f.Value.String() + // omit sensitive information + if f.Name == tokenFlag { + value = "" + } + prettyPrintedArgs += fmt.Sprintf(" --%s=%s", f.Name, value) + }) + log.Infof("command line args:%s", prettyPrintedArgs) +} + func main() { var ( flags = flags{} @@ -145,7 +166,7 @@ func main() { flag.Bool("no-probe", false, "Don't run the probe.") // Probe flags - flag.StringVar(&flags.probe.token, "service-token", "", "Token to use to authenticate with cloud.weave.works") + flag.StringVar(&flags.probe.token, tokenFlag, "", "Token to use to authenticate with cloud.weave.works") flag.StringVar(&flags.probe.token, "probe.token", "", "Token to use to authenticate with cloud.weave.works") flag.StringVar(&flags.probe.httpListen, "probe.http.listen", "", "listen address for HTTP profiling and instrumentation server") flag.DurationVar(&flags.probe.publishInterval, "probe.publish.interval", 3*time.Second, "publish (output) interval") diff --git a/prog/probe.go b/prog/probe.go index f461664cb..01c030136 100644 --- a/prog/probe.go +++ b/prog/probe.go @@ -74,7 +74,7 @@ func probeMain(flags probeFlags) { sig := metrics.DefaultInmemSignal(inm) defer sig.Stop() metrics.NewGlobal(metrics.DefaultConfig("scope-probe"), inm) - + logCensoredArgs() defer log.Info("probe exiting") if flags.spyProcs && os.Getegid() != 0 { From 0c3235ae3d82c000138fb00be2486224a27c69ea Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Thu, 11 Aug 2016 15:41:07 +0000 Subject: [PATCH 2/2] Review feedback --- prog/main.go | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/prog/main.go b/prog/main.go index b0f839859..9cc5c57d3 100644 --- a/prog/main.go +++ b/prog/main.go @@ -17,9 +17,14 @@ import ( "github.com/weaveworks/weave/common" ) -var version = "dev" // set at build time - -const tokenFlag = "service-token" +var ( + // set at build time + version = "dev" + // tokens to be elided when logging + serviceTokenFlag = "service-token" + probeTokenFlag = "probe.token" + sensitiveFlags = []string{serviceTokenFlag, probeTokenFlag} +) type prefixFormatter struct { prefix []byte @@ -123,20 +128,23 @@ type appFlags struct { func logCensoredArgs() { var prettyPrintedArgs string - // we show the args followed by the flags which is likely to change the - // original ordering. However the flag parser doesn't keep positioning - // information to allow reconstructing it more accurately. - for _, arg := range flag.Args() { - prettyPrintedArgs += " " + arg - } + // We show the flags followed by the args. This may change the original + // ordering. However the flag parser doesn't keep positioning + // information, not allowing for a more accurate reconstruction. flag.Visit(func(f *flag.Flag) { value := f.Value.String() // omit sensitive information - if f.Name == tokenFlag { - value = "" + for _, sensitiveFlag := range sensitiveFlags { + if f.Name == sensitiveFlag { + value = "" + break + } } prettyPrintedArgs += fmt.Sprintf(" --%s=%s", f.Name, value) }) + for _, arg := range flag.Args() { + prettyPrintedArgs += " " + arg + } log.Infof("command line args:%s", prettyPrintedArgs) } @@ -166,8 +174,8 @@ func main() { flag.Bool("no-probe", false, "Don't run the probe.") // Probe flags - flag.StringVar(&flags.probe.token, tokenFlag, "", "Token to use to authenticate with cloud.weave.works") - flag.StringVar(&flags.probe.token, "probe.token", "", "Token to use to authenticate with cloud.weave.works") + flag.StringVar(&flags.probe.token, serviceTokenFlag, "", "Token to use to authenticate with cloud.weave.works") + flag.StringVar(&flags.probe.token, probeTokenFlag, "", "Token to use to authenticate with cloud.weave.works") flag.StringVar(&flags.probe.httpListen, "probe.http.listen", "", "listen address for HTTP profiling and instrumentation server") flag.DurationVar(&flags.probe.publishInterval, "probe.publish.interval", 3*time.Second, "publish (output) interval") flag.DurationVar(&flags.probe.spyInterval, "probe.spy.interval", time.Second, "spy (scan) interval")