From 97fdcdc525edfe7caf0f203c093ea7232dccd9d1 Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Fri, 15 Feb 2019 17:31:08 +0100 Subject: [PATCH 1/7] Option to censor raw reports by command line args and env vars. --- app/api_report.go | 8 ++++-- app/api_topology.go | 20 +++++++++---- probe/docker/container.go | 2 +- probe/process/reporter.go | 3 +- prog/main.go | 2 +- render/detailed/node.go | 10 +++---- render/detailed/node_test.go | 2 +- render/detailed/summary.go | 6 ++-- report/censor.go | 55 ++++++++++++++++++++++++++++++++++++ report/map_keys.go | 5 ++++ report/metadata_template.go | 11 ++++++++ 11 files changed, 103 insertions(+), 21 deletions(-) create mode 100644 report/censor.go diff --git a/app/api_report.go b/app/api_report.go index 992db2f423..a1d993cb04 100644 --- a/app/api_report.go +++ b/app/api_report.go @@ -13,12 +13,16 @@ import ( // Raw report handler func makeRawReportHandler(rep Reporter) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) { - report, err := rep.Report(ctx, time.Now()) + censorConfig := report.CensorConfig{ + HideCommandLineArguments: r.URL.Query().Get("hideCommandLineArguments") == "true", + HideEnvironmentVariables: r.URL.Query().Get("hideEnvironmentVariables") == "true", + } + rawReport, err := rep.Report(ctx, time.Now()) if err != nil { respondWith(w, http.StatusInternalServerError, err) return } - respondWith(w, http.StatusOK, report) + respondWith(w, http.StatusOK, report.CensorReport(rawReport, censorConfig)) } } diff --git a/app/api_topology.go b/app/api_topology.go index 9f2357af23..129aba6e22 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -5,6 +5,7 @@ import ( "time" "context" + "github.com/gorilla/mux" log "github.com/sirupsen/logrus" @@ -41,17 +42,21 @@ type rendererHandler func(context.Context, render.Renderer, render.Transformer, // Full topology. func handleTopology(ctx context.Context, renderer render.Renderer, transformer render.Transformer, rc detailed.RenderContext, w http.ResponseWriter, r *http.Request) { + var ( + hideCommandLineArguments = true + ) respondWith(w, http.StatusOK, APITopology{ - Nodes: detailed.Summaries(ctx, rc, render.Render(ctx, rc.Report, renderer, transformer).Nodes), + Nodes: detailed.Summaries(ctx, rc, hideCommandLineArguments, render.Render(ctx, rc.Report, renderer, transformer).Nodes), }) } // Individual nodes. func handleNode(ctx context.Context, renderer render.Renderer, transformer render.Transformer, rc detailed.RenderContext, w http.ResponseWriter, r *http.Request) { var ( - vars = mux.Vars(r) - topologyID = vars["topology"] - nodeID = vars["id"] + vars = mux.Vars(r) + topologyID = vars["topology"] + nodeID = vars["id"] + hideCommandLineArguments = true ) // We must not lose the node during filtering. We achieve that by // (1) rendering the report with the base renderer, without @@ -71,7 +76,7 @@ func handleNode(ctx context.Context, renderer render.Renderer, transformer rende nodes.Nodes[nodeID] = node nodes.Filtered-- } - respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, rc, nodes.Nodes, node)}) + respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, rc, hideCommandLineArguments, nodes.Nodes, node)}) } // Websocket for the full topology. @@ -81,6 +86,9 @@ func handleWebsocket( w http.ResponseWriter, r *http.Request, ) { + var ( + hideCommandLineArguments = true + ) if err := r.ParseForm(); err != nil { respondWith(w, http.StatusInternalServerError, err) return @@ -145,7 +153,7 @@ func handleWebsocket( log.Errorf("Error generating report: %v", err) return } - newTopo := detailed.Summaries(ctx, RenderContextForReporter(rep, re), render.Render(ctx, re, renderer, filter).Nodes) + newTopo := detailed.Summaries(ctx, RenderContextForReporter(rep, re), hideCommandLineArguments, render.Render(ctx, re, renderer, filter).Nodes) diff := detailed.TopoDiff(previousTopo, newTopo) previousTopo = newTopo diff --git a/probe/docker/container.go b/probe/docker/container.go index 832086b82a..6ce59a8cfb 100644 --- a/probe/docker/container.go +++ b/probe/docker/container.go @@ -53,7 +53,7 @@ const ( CPUSystemCPUUsage = "docker_cpu_system_cpu_usage" LabelPrefix = "docker_label_" - EnvPrefix = "docker_env_" + EnvPrefix = report.DockerEnvPrefix ) // These 'constants' are used for node states. diff --git a/probe/process/reporter.go b/probe/process/reporter.go index fc035c46ed..35253bbcbf 100644 --- a/probe/process/reporter.go +++ b/probe/process/reporter.go @@ -2,7 +2,6 @@ package process import ( "strconv" - "strings" "github.com/weaveworks/common/mtime" "github.com/weaveworks/scope/report" @@ -93,7 +92,7 @@ func (r *Reporter) processTopology() (report.Topology, error) { if p.Cmdline != "" { if r.noCommandLineArguments { - node = node.WithLatest(Cmdline, now, strings.Split(p.Cmdline, " ")[0]) + node = node.WithLatest(Cmdline, now, report.StripCommandArgs(p.Cmdline)) } else { node = node.WithLatest(Cmdline, now, p.Cmdline) } diff --git a/prog/main.go b/prog/main.go index 7817908579..0cdf9e8a2e 100644 --- a/prog/main.go +++ b/prog/main.go @@ -300,7 +300,7 @@ func setupFlags(flags *flags) { flag.StringVar(&flags.probe.pluginsRoot, "probe.plugins.root", "/var/run/scope/plugins", "Root directory to search for plugins") flag.BoolVar(&flags.probe.noControls, "probe.no-controls", false, "Disable controls (e.g. start/stop containers, terminals, logs ...)") flag.BoolVar(&flags.probe.noCommandLineArguments, "probe.omit.cmd-args", false, "Disable collection of command-line arguments") - flag.BoolVar(&flags.probe.noEnvironmentVariables, "probe.omit.env-vars", true, "Disable collection of environment variables") + flag.BoolVar(&flags.probe.noEnvironmentVariables, "probe.omit.env-vars", false, "Disable collection of environment variables") flag.BoolVar(&flags.probe.insecure, "probe.insecure", false, "(SSL) explicitly allow \"insecure\" SSL connections and transfers") flag.StringVar(&flags.probe.resolver, "probe.resolver", "", "IP address & port of resolver to use. Default is to use system resolver.") diff --git a/render/detailed/node.go b/render/detailed/node.go index 459cc593f0..0313e88da1 100644 --- a/render/detailed/node.go +++ b/render/detailed/node.go @@ -86,12 +86,12 @@ type RenderContext struct { // MakeNode transforms a renderable node to a detailed node. It uses // aggregate metadata, plus the set of origin node IDs, to produce tables. -func MakeNode(topologyID string, rc RenderContext, ns report.Nodes, n report.Node) Node { - summary, _ := MakeNodeSummary(rc, n) +func MakeNode(topologyID string, rc RenderContext, hideCommandLineArguments bool, ns report.Nodes, n report.Node) Node { + summary, _ := MakeNodeSummary(rc, hideCommandLineArguments, n) return Node{ NodeSummary: summary, Controls: controls(rc.Report, n), - Children: children(rc, n), + Children: children(rc, hideCommandLineArguments, n), Connections: []ConnectionsSummary{ incomingConnectionsSummary(topologyID, rc.Report, n, ns), outgoingConnectionsSummary(topologyID, rc.Report, n, ns), @@ -222,13 +222,13 @@ var nodeSummaryGroupSpecs = []struct { }, } -func children(rc RenderContext, n report.Node) []NodeSummaryGroup { +func children(rc RenderContext, hideCommandLineArguments bool, n report.Node) []NodeSummaryGroup { summaries := map[string][]NodeSummary{} n.Children.ForEach(func(child report.Node) { if child.ID == n.ID { return } - summary, ok := MakeNodeSummary(rc, child) + summary, ok := MakeNodeSummary(rc, hideCommandLineArguments, child) if !ok { return } diff --git a/render/detailed/node_test.go b/render/detailed/node_test.go index 5e5f9e635f..51bbb40eb9 100644 --- a/render/detailed/node_test.go +++ b/render/detailed/node_test.go @@ -19,7 +19,7 @@ import ( ) func child(t *testing.T, r render.Renderer, id string) detailed.NodeSummary { - s, ok := detailed.MakeNodeSummary(detailed.RenderContext{Report: fixture.Report}, r.Render(context.Background(), fixture.Report).Nodes[id]) + s, ok := detailed.MakeNodeSummary(detailed.RenderContext{Report: fixture.Report}, false, r.Render(context.Background(), fixture.Report).Nodes[id]) if !ok { t.Fatalf("Expected node %s to be summarizable, but wasn't", id) } diff --git a/render/detailed/summary.go b/render/detailed/summary.go index a6c24c95dd..7bbdb28105 100644 --- a/render/detailed/summary.go +++ b/render/detailed/summary.go @@ -151,7 +151,7 @@ func MakeBasicNodeSummary(r report.Report, n report.Node) (BasicNodeSummary, boo } // MakeNodeSummary summarizes a node, if possible. -func MakeNodeSummary(rc RenderContext, n report.Node) (NodeSummary, bool) { +func MakeNodeSummary(rc RenderContext, hideCommandLineArguments bool, n report.Node) (NodeSummary, bool) { base, ok := MakeBasicNodeSummary(rc.Report, n) if !ok { return NodeSummary{}, false @@ -449,13 +449,13 @@ func (s nodeSummariesByID) Less(i, j int) bool { return s[i].ID < s[j].ID } type NodeSummaries map[string]NodeSummary // Summaries converts RenderableNodes into a set of NodeSummaries -func Summaries(ctx context.Context, rc RenderContext, rns report.Nodes) NodeSummaries { +func Summaries(ctx context.Context, rc RenderContext, hideCommandLineArguments bool, rns report.Nodes) NodeSummaries { span, ctx := opentracing.StartSpanFromContext(ctx, "detailed.Summaries") defer span.Finish() result := NodeSummaries{} for id, node := range rns { - if summary, ok := MakeNodeSummary(rc, node); ok { + if summary, ok := MakeNodeSummary(rc, hideCommandLineArguments, node); ok { for i, m := range summary.Metrics { summary.Metrics[i] = m.Summary() } diff --git a/report/censor.go b/report/censor.go new file mode 100644 index 0000000000..1400232f11 --- /dev/null +++ b/report/censor.go @@ -0,0 +1,55 @@ +package report + +import "strings" + +// import log "github.com/sirupsen/logrus" + +type keyMatcher func(string) bool + +func keyEquals(fixedKey string) keyMatcher { + return func(key string) bool { + return key == fixedKey + } +} + +func keyStartsWith(prefix string) keyMatcher { + return func(key string) bool { + return strings.HasPrefix(key, prefix) + } +} + +type censorValueFunc func(string) string + +func assignEmpty(key string) string { + return "" +} + +func censorTopology(t *Topology, match keyMatcher, censor censorValueFunc) { + for nodeID := range t.Nodes { + for entryID := range t.Nodes[nodeID].Latest { + entry := &t.Nodes[nodeID].Latest[entryID] + if match(entry.key) { + // log.Infof("Blabla ... %s ... %s ... %s", entry.key, entry.Value, censor(entry.Value)) + entry.Value = censor(entry.Value) + } + } + } +} + +// CensorConfig describe which parts of the report needs to be censored. +type CensorConfig struct { + HideCommandLineArguments bool + HideEnvironmentVariables bool +} + +// CensorReport removes any sensitive data from the report. +func CensorReport(r Report, cfg CensorConfig) Report { + if cfg.HideCommandLineArguments { + censorTopology(&r.Process, keyEquals(Cmdline), StripCommandArgs) + censorTopology(&r.Container, keyEquals(DockerContainerCommand), StripCommandArgs) + } + if cfg.HideEnvironmentVariables { + censorTopology(&r.Container, keyStartsWith(DockerEnvPrefix), assignEmpty) + } + return r +} diff --git a/report/map_keys.go b/report/map_keys.go index dc72f6cc58..887b10848b 100644 --- a/report/map_keys.go +++ b/report/map_keys.go @@ -43,6 +43,7 @@ const ( DockerContainerUptime = "docker_container_uptime" DockerContainerRestartCount = "docker_container_restart_count" DockerContainerNetworkMode = "docker_container_network_mode" + DockerEnvPrefix = "docker_env_" // probe/kubernetes KubernetesName = "kubernetes_name" KubernetesNamespace = "kubernetes_namespace" @@ -214,3 +215,7 @@ func lookupCommonKey(b []byte) string { } return string(b) } + +func isCommandKey(key string) bool { + return key == Cmdline || key == DockerContainerCommand +} diff --git a/report/metadata_template.go b/report/metadata_template.go index a6a4464633..45a777c60b 100644 --- a/report/metadata_template.go +++ b/report/metadata_template.go @@ -4,6 +4,8 @@ import ( "sort" "strconv" "strings" + + log "github.com/sirupsen/logrus" ) const ( @@ -28,6 +30,11 @@ type MetadataTemplate struct { From string `json:"from,omitempty"` // Defines how to get the value from a report node } +// StripCommandArgs removes all the arguments from the command +func StripCommandArgs(command string) string { + return strings.Split(command, " ")[0] +} + // MetadataRow returns the row for a node func (t MetadataTemplate) MetadataRow(n Node) (MetadataRow, bool) { from := fromDefault @@ -96,6 +103,10 @@ func (e MetadataTemplates) MetadataRows(n Node) []MetadataRow { rows := make([]MetadataRow, 0, len(e)) for _, template := range e { if row, ok := template.MetadataRow(n); ok { + if isCommandKey(row.ID) { + row.Value = StripCommandArgs(row.Value) + log.Infof("Blublu %s -- %v", n.ID, row) + } rows = append(rows, row) } } From 3c5320ef09e1a3ae6ec2f9a6ee1785cc2d44f055 Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Fri, 15 Feb 2019 18:15:12 +0100 Subject: [PATCH 2/7] Polished the code and applied censoring to other API endpoints. --- app/api_report.go | 6 +---- app/api_topologies.go | 5 ++-- app/api_topology.go | 26 ++++++++------------ prog/main.go | 2 +- render/detailed/node.go | 10 ++++---- render/detailed/node_test.go | 2 +- render/detailed/summary.go | 6 ++--- report/censor.go | 46 +++++++++++++++++++----------------- report/map_keys.go | 4 ---- report/metadata_template.go | 11 --------- 10 files changed, 48 insertions(+), 70 deletions(-) diff --git a/app/api_report.go b/app/api_report.go index a1d993cb04..3899f69e33 100644 --- a/app/api_report.go +++ b/app/api_report.go @@ -13,16 +13,12 @@ import ( // Raw report handler func makeRawReportHandler(rep Reporter) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) { - censorConfig := report.CensorConfig{ - HideCommandLineArguments: r.URL.Query().Get("hideCommandLineArguments") == "true", - HideEnvironmentVariables: r.URL.Query().Get("hideEnvironmentVariables") == "true", - } rawReport, err := rep.Report(ctx, time.Now()) if err != nil { respondWith(w, http.StatusInternalServerError, err) return } - respondWith(w, http.StatusOK, report.CensorReport(rawReport, censorConfig)) + respondWith(w, http.StatusOK, report.CensorReportForRequest(rawReport, r)) } } diff --git a/app/api_topologies.go b/app/api_topologies.go index 136d6960f6..af94890ad2 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -577,11 +577,12 @@ func (r *Registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFu return } req.ParseForm() - renderer, filter, err := r.RendererForTopology(topologyID, req.Form, rpt) + rc := RenderContextForReporter(rep, rpt, req) + renderer, filter, err := r.RendererForTopology(topologyID, req.Form, rc.Report) if err != nil { respondWith(w, http.StatusInternalServerError, err) return } - f(ctx, renderer, filter, RenderContextForReporter(rep, rpt), w, req) + f(ctx, renderer, filter, rc, w, req) } } diff --git a/app/api_topology.go b/app/api_topology.go index 129aba6e22..1ed67e6f2a 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -30,8 +30,8 @@ type APINode struct { } // RenderContextForReporter creates the rendering context for the given reporter. -func RenderContextForReporter(rep Reporter, r report.Report) detailed.RenderContext { - rc := detailed.RenderContext{Report: r} +func RenderContextForReporter(rep Reporter, r report.Report, req *http.Request) detailed.RenderContext { + rc := detailed.RenderContext{Report: report.CensorReportForRequest(r, req)} if wrep, ok := rep.(WebReporter); ok { rc.MetricsGraphURL = wrep.MetricsGraphURL } @@ -42,21 +42,17 @@ type rendererHandler func(context.Context, render.Renderer, render.Transformer, // Full topology. func handleTopology(ctx context.Context, renderer render.Renderer, transformer render.Transformer, rc detailed.RenderContext, w http.ResponseWriter, r *http.Request) { - var ( - hideCommandLineArguments = true - ) respondWith(w, http.StatusOK, APITopology{ - Nodes: detailed.Summaries(ctx, rc, hideCommandLineArguments, render.Render(ctx, rc.Report, renderer, transformer).Nodes), + Nodes: detailed.Summaries(ctx, rc, render.Render(ctx, rc.Report, renderer, transformer).Nodes), }) } // Individual nodes. func handleNode(ctx context.Context, renderer render.Renderer, transformer render.Transformer, rc detailed.RenderContext, w http.ResponseWriter, r *http.Request) { var ( - vars = mux.Vars(r) - topologyID = vars["topology"] - nodeID = vars["id"] - hideCommandLineArguments = true + vars = mux.Vars(r) + topologyID = vars["topology"] + nodeID = vars["id"] ) // We must not lose the node during filtering. We achieve that by // (1) rendering the report with the base renderer, without @@ -76,7 +72,7 @@ func handleNode(ctx context.Context, renderer render.Renderer, transformer rende nodes.Nodes[nodeID] = node nodes.Filtered-- } - respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, rc, hideCommandLineArguments, nodes.Nodes, node)}) + respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, rc, nodes.Nodes, node)}) } // Websocket for the full topology. @@ -86,9 +82,6 @@ func handleWebsocket( w http.ResponseWriter, r *http.Request, ) { - var ( - hideCommandLineArguments = true - ) if err := r.ParseForm(); err != nil { respondWith(w, http.StatusInternalServerError, err) return @@ -148,12 +141,13 @@ func handleWebsocket( log.Errorf("Error generating report: %v", err) return } - renderer, filter, err := topologyRegistry.RendererForTopology(topologyID, r.Form, re) + rc := RenderContextForReporter(rep, re, r) + renderer, filter, err := topologyRegistry.RendererForTopology(topologyID, r.Form, rc.Report) if err != nil { log.Errorf("Error generating report: %v", err) return } - newTopo := detailed.Summaries(ctx, RenderContextForReporter(rep, re), hideCommandLineArguments, render.Render(ctx, re, renderer, filter).Nodes) + newTopo := detailed.Summaries(ctx, rc, render.Render(ctx, rc.Report, renderer, filter).Nodes) diff := detailed.TopoDiff(previousTopo, newTopo) previousTopo = newTopo diff --git a/prog/main.go b/prog/main.go index 0cdf9e8a2e..7817908579 100644 --- a/prog/main.go +++ b/prog/main.go @@ -300,7 +300,7 @@ func setupFlags(flags *flags) { flag.StringVar(&flags.probe.pluginsRoot, "probe.plugins.root", "/var/run/scope/plugins", "Root directory to search for plugins") flag.BoolVar(&flags.probe.noControls, "probe.no-controls", false, "Disable controls (e.g. start/stop containers, terminals, logs ...)") flag.BoolVar(&flags.probe.noCommandLineArguments, "probe.omit.cmd-args", false, "Disable collection of command-line arguments") - flag.BoolVar(&flags.probe.noEnvironmentVariables, "probe.omit.env-vars", false, "Disable collection of environment variables") + flag.BoolVar(&flags.probe.noEnvironmentVariables, "probe.omit.env-vars", true, "Disable collection of environment variables") flag.BoolVar(&flags.probe.insecure, "probe.insecure", false, "(SSL) explicitly allow \"insecure\" SSL connections and transfers") flag.StringVar(&flags.probe.resolver, "probe.resolver", "", "IP address & port of resolver to use. Default is to use system resolver.") diff --git a/render/detailed/node.go b/render/detailed/node.go index 0313e88da1..459cc593f0 100644 --- a/render/detailed/node.go +++ b/render/detailed/node.go @@ -86,12 +86,12 @@ type RenderContext struct { // MakeNode transforms a renderable node to a detailed node. It uses // aggregate metadata, plus the set of origin node IDs, to produce tables. -func MakeNode(topologyID string, rc RenderContext, hideCommandLineArguments bool, ns report.Nodes, n report.Node) Node { - summary, _ := MakeNodeSummary(rc, hideCommandLineArguments, n) +func MakeNode(topologyID string, rc RenderContext, ns report.Nodes, n report.Node) Node { + summary, _ := MakeNodeSummary(rc, n) return Node{ NodeSummary: summary, Controls: controls(rc.Report, n), - Children: children(rc, hideCommandLineArguments, n), + Children: children(rc, n), Connections: []ConnectionsSummary{ incomingConnectionsSummary(topologyID, rc.Report, n, ns), outgoingConnectionsSummary(topologyID, rc.Report, n, ns), @@ -222,13 +222,13 @@ var nodeSummaryGroupSpecs = []struct { }, } -func children(rc RenderContext, hideCommandLineArguments bool, n report.Node) []NodeSummaryGroup { +func children(rc RenderContext, n report.Node) []NodeSummaryGroup { summaries := map[string][]NodeSummary{} n.Children.ForEach(func(child report.Node) { if child.ID == n.ID { return } - summary, ok := MakeNodeSummary(rc, hideCommandLineArguments, child) + summary, ok := MakeNodeSummary(rc, child) if !ok { return } diff --git a/render/detailed/node_test.go b/render/detailed/node_test.go index 51bbb40eb9..5e5f9e635f 100644 --- a/render/detailed/node_test.go +++ b/render/detailed/node_test.go @@ -19,7 +19,7 @@ import ( ) func child(t *testing.T, r render.Renderer, id string) detailed.NodeSummary { - s, ok := detailed.MakeNodeSummary(detailed.RenderContext{Report: fixture.Report}, false, r.Render(context.Background(), fixture.Report).Nodes[id]) + s, ok := detailed.MakeNodeSummary(detailed.RenderContext{Report: fixture.Report}, r.Render(context.Background(), fixture.Report).Nodes[id]) if !ok { t.Fatalf("Expected node %s to be summarizable, but wasn't", id) } diff --git a/render/detailed/summary.go b/render/detailed/summary.go index 7bbdb28105..a6c24c95dd 100644 --- a/render/detailed/summary.go +++ b/render/detailed/summary.go @@ -151,7 +151,7 @@ func MakeBasicNodeSummary(r report.Report, n report.Node) (BasicNodeSummary, boo } // MakeNodeSummary summarizes a node, if possible. -func MakeNodeSummary(rc RenderContext, hideCommandLineArguments bool, n report.Node) (NodeSummary, bool) { +func MakeNodeSummary(rc RenderContext, n report.Node) (NodeSummary, bool) { base, ok := MakeBasicNodeSummary(rc.Report, n) if !ok { return NodeSummary{}, false @@ -449,13 +449,13 @@ func (s nodeSummariesByID) Less(i, j int) bool { return s[i].ID < s[j].ID } type NodeSummaries map[string]NodeSummary // Summaries converts RenderableNodes into a set of NodeSummaries -func Summaries(ctx context.Context, rc RenderContext, hideCommandLineArguments bool, rns report.Nodes) NodeSummaries { +func Summaries(ctx context.Context, rc RenderContext, rns report.Nodes) NodeSummaries { span, ctx := opentracing.StartSpanFromContext(ctx, "detailed.Summaries") defer span.Finish() result := NodeSummaries{} for id, node := range rns { - if summary, ok := MakeNodeSummary(rc, hideCommandLineArguments, node); ok { + if summary, ok := MakeNodeSummary(rc, node); ok { for i, m := range summary.Metrics { summary.Metrics[i] = m.Summary() } diff --git a/report/censor.go b/report/censor.go index 1400232f11..d3a6c3d980 100644 --- a/report/censor.go +++ b/report/censor.go @@ -1,8 +1,9 @@ package report -import "strings" - -// import log "github.com/sirupsen/logrus" +import ( + "net/http" + "strings" +) type keyMatcher func(string) bool @@ -20,36 +21,37 @@ func keyStartsWith(prefix string) keyMatcher { type censorValueFunc func(string) string -func assignEmpty(key string) string { - return "" -} - +// TODO: Implement this in a more systematic way. func censorTopology(t *Topology, match keyMatcher, censor censorValueFunc) { for nodeID := range t.Nodes { for entryID := range t.Nodes[nodeID].Latest { entry := &t.Nodes[nodeID].Latest[entryID] if match(entry.key) { - // log.Infof("Blabla ... %s ... %s ... %s", entry.key, entry.Value, censor(entry.Value)) entry.Value = censor(entry.Value) } } } } -// CensorConfig describe which parts of the report needs to be censored. -type CensorConfig struct { - HideCommandLineArguments bool - HideEnvironmentVariables bool -} - -// CensorReport removes any sensitive data from the report. -func CensorReport(r Report, cfg CensorConfig) Report { - if cfg.HideCommandLineArguments { - censorTopology(&r.Process, keyEquals(Cmdline), StripCommandArgs) - censorTopology(&r.Container, keyEquals(DockerContainerCommand), StripCommandArgs) +// CensorReportForRequest removes any sensitive data +// from the report based on the request query params. +func CensorReportForRequest(rep Report, req *http.Request) Report { + var ( + hideCommandLineArguments = req.URL.Query().Get("hideCommandLineArguments") == "true" + hideEnvironmentVariables = req.URL.Query().Get("hideEnvironmentVariables") == "true" + makeEmpty = func(string) string { return "" } + ) + if hideCommandLineArguments { + censorTopology(&rep.Process, keyEquals(Cmdline), StripCommandArgs) + censorTopology(&rep.Container, keyEquals(DockerContainerCommand), StripCommandArgs) } - if cfg.HideEnvironmentVariables { - censorTopology(&r.Container, keyStartsWith(DockerEnvPrefix), assignEmpty) + if hideEnvironmentVariables { + censorTopology(&rep.Container, keyStartsWith(DockerEnvPrefix), makeEmpty) } - return r + return rep +} + +// StripCommandArgs removes all the arguments from the command +func StripCommandArgs(command string) string { + return strings.Split(command, " ")[0] } diff --git a/report/map_keys.go b/report/map_keys.go index 887b10848b..a6094c6ee4 100644 --- a/report/map_keys.go +++ b/report/map_keys.go @@ -215,7 +215,3 @@ func lookupCommonKey(b []byte) string { } return string(b) } - -func isCommandKey(key string) bool { - return key == Cmdline || key == DockerContainerCommand -} diff --git a/report/metadata_template.go b/report/metadata_template.go index 45a777c60b..a6a4464633 100644 --- a/report/metadata_template.go +++ b/report/metadata_template.go @@ -4,8 +4,6 @@ import ( "sort" "strconv" "strings" - - log "github.com/sirupsen/logrus" ) const ( @@ -30,11 +28,6 @@ type MetadataTemplate struct { From string `json:"from,omitempty"` // Defines how to get the value from a report node } -// StripCommandArgs removes all the arguments from the command -func StripCommandArgs(command string) string { - return strings.Split(command, " ")[0] -} - // MetadataRow returns the row for a node func (t MetadataTemplate) MetadataRow(n Node) (MetadataRow, bool) { from := fromDefault @@ -103,10 +96,6 @@ func (e MetadataTemplates) MetadataRows(n Node) []MetadataRow { rows := make([]MetadataRow, 0, len(e)) for _, template := range e { if row, ok := template.MetadataRow(n); ok { - if isCommandKey(row.ID) { - row.Value = StripCommandArgs(row.Value) - log.Infof("Blublu %s -- %v", n.ID, row) - } rows = append(rows, row) } } From 0f1b7e5972abea5ca5ebefe477360d0e1908cd44 Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Thu, 21 Feb 2019 16:18:24 +0100 Subject: [PATCH 3/7] Prepare to filter node summaries post-render. --- app/api_report.go | 3 ++- app/api_topologies.go | 2 +- app/api_topology.go | 23 +++++++++++++++++------ render/detailed/censor.go | 15 +++++++++++++++ report/censor.go | 37 +++++++++++++++++++++++++------------ 5 files changed, 60 insertions(+), 20 deletions(-) create mode 100644 render/detailed/censor.go diff --git a/app/api_report.go b/app/api_report.go index 3899f69e33..3ccdf94dbe 100644 --- a/app/api_report.go +++ b/app/api_report.go @@ -13,12 +13,13 @@ import ( // Raw report handler func makeRawReportHandler(rep Reporter) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) { + censorCfg := report.GetCensorConfigFromQueryParams(r) rawReport, err := rep.Report(ctx, time.Now()) if err != nil { respondWith(w, http.StatusInternalServerError, err) return } - respondWith(w, http.StatusOK, report.CensorReportForRequest(rawReport, r)) + respondWith(w, http.StatusOK, report.CensorRawReport(rawReport, censorCfg)) } } diff --git a/app/api_topologies.go b/app/api_topologies.go index af94890ad2..294bd2c723 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -577,7 +577,7 @@ func (r *Registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFu return } req.ParseForm() - rc := RenderContextForReporter(rep, rpt, req) + rc := RenderContextForReporter(rep, rpt) renderer, filter, err := r.RendererForTopology(topologyID, req.Form, rc.Report) if err != nil { respondWith(w, http.StatusInternalServerError, err) diff --git a/app/api_topology.go b/app/api_topology.go index 1ed67e6f2a..c67266d4c0 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -30,8 +30,8 @@ type APINode struct { } // RenderContextForReporter creates the rendering context for the given reporter. -func RenderContextForReporter(rep Reporter, r report.Report, req *http.Request) detailed.RenderContext { - rc := detailed.RenderContext{Report: report.CensorReportForRequest(r, req)} +func RenderContextForReporter(rep Reporter, r report.Report) detailed.RenderContext { + rc := detailed.RenderContext{Report: r} if wrep, ok := rep.(WebReporter); ok { rc.MetricsGraphURL = wrep.MetricsGraphURL } @@ -42,8 +42,13 @@ type rendererHandler func(context.Context, render.Renderer, render.Transformer, // Full topology. func handleTopology(ctx context.Context, renderer render.Renderer, transformer render.Transformer, rc detailed.RenderContext, w http.ResponseWriter, r *http.Request) { + // log.Infof("blublu %v", rc.Report.Container.Nodes) + // log.Infof("blublu %v", rc.Report.ContainerImage.Nodes) + // log.Infof("blibli %v", render.Render(ctx, rc.Report, renderer, transformer).Nodes) + censorCfg := report.GetCensorConfigFromQueryParams(r) + nodeSummaries := detailed.Summaries(ctx, rc, render.Render(ctx, rc.Report, renderer, transformer).Nodes) respondWith(w, http.StatusOK, APITopology{ - Nodes: detailed.Summaries(ctx, rc, render.Render(ctx, rc.Report, renderer, transformer).Nodes), + Nodes: detailed.CensorNodeSummaries(nodeSummaries, censorCfg), }) } @@ -72,7 +77,9 @@ func handleNode(ctx context.Context, renderer render.Renderer, transformer rende nodes.Nodes[nodeID] = node nodes.Filtered-- } - respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, rc, nodes.Nodes, node)}) + censorCfg := report.GetCensorConfigFromQueryParams(r) + rawNode := detailed.MakeNode(topologyID, rc, nodes.Nodes, node) + respondWith(w, http.StatusOK, APINode{Node: detailed.CensorNode(rawNode, censorCfg)}) } // Websocket for the full topology. @@ -141,16 +148,20 @@ func handleWebsocket( log.Errorf("Error generating report: %v", err) return } - rc := RenderContextForReporter(rep, re, r) + rc := RenderContextForReporter(rep, re) renderer, filter, err := topologyRegistry.RendererForTopology(topologyID, r.Form, rc.Report) if err != nil { log.Errorf("Error generating report: %v", err) return } + // log.Infof("blublu %v", rc.Report.Container.Nodes) + censorCfg := report.GetCensorConfigFromQueryParams(r) newTopo := detailed.Summaries(ctx, rc, render.Render(ctx, rc.Report, renderer, filter).Nodes) + newTopo = detailed.CensorNodeSummaries(newTopo, censorCfg) + // log.Infof("blublu %v", newTopo) diff := detailed.TopoDiff(previousTopo, newTopo) previousTopo = newTopo - + // log.Infof("dfdfdf %v", diff) if err := conn.WriteJSON(diff); err != nil { if !xfer.IsExpectedWSCloseError(err) { log.Errorf("cannot serialize topology diff: %s", err) diff --git a/render/detailed/censor.go b/render/detailed/censor.go new file mode 100644 index 0000000000..a36bca95eb --- /dev/null +++ b/render/detailed/censor.go @@ -0,0 +1,15 @@ +package detailed + +import ( + "github.com/weaveworks/scope/report" +) + +// CensorNode ... +func CensorNode(n Node, cfg report.CensorConfig) Node { + return n +} + +// CensorNodeSummaries ... +func CensorNodeSummaries(n NodeSummaries, cfg report.CensorConfig) NodeSummaries { + return n +} diff --git a/report/censor.go b/report/censor.go index d3a6c3d980..913ddf1c92 100644 --- a/report/censor.go +++ b/report/censor.go @@ -33,22 +33,35 @@ func censorTopology(t *Topology, match keyMatcher, censor censorValueFunc) { } } -// CensorReportForRequest removes any sensitive data -// from the report based on the request query params. -func CensorReportForRequest(rep Report, req *http.Request) Report { +// CensorConfig describes how probe reports should +// be censored when rendered through the API. +type CensorConfig struct { + hideCommandLineArguments bool + hideEnvironmentVariables bool +} + +// GetCensorConfigFromQueryParams extracts censor config from request query params. +func GetCensorConfigFromQueryParams(req *http.Request) CensorConfig { + return CensorConfig{ + hideCommandLineArguments: true || req.URL.Query().Get("hideCommandLineArguments") == "true", + hideEnvironmentVariables: true || req.URL.Query().Get("hideEnvironmentVariables") == "true", + } +} + +// CensorRawReport removes any sensitive data from +// the raw report based on the request query params. +func CensorRawReport(r Report, cfg CensorConfig) Report { var ( - hideCommandLineArguments = req.URL.Query().Get("hideCommandLineArguments") == "true" - hideEnvironmentVariables = req.URL.Query().Get("hideEnvironmentVariables") == "true" - makeEmpty = func(string) string { return "" } + makeEmpty = func(string) string { return "" } ) - if hideCommandLineArguments { - censorTopology(&rep.Process, keyEquals(Cmdline), StripCommandArgs) - censorTopology(&rep.Container, keyEquals(DockerContainerCommand), StripCommandArgs) + if cfg.hideCommandLineArguments { + censorTopology(&r.Process, keyEquals(Cmdline), StripCommandArgs) + censorTopology(&r.Container, keyEquals(DockerContainerCommand), StripCommandArgs) } - if hideEnvironmentVariables { - censorTopology(&rep.Container, keyStartsWith(DockerEnvPrefix), makeEmpty) + if cfg.hideEnvironmentVariables { + censorTopology(&r.Container, keyStartsWith(DockerEnvPrefix), makeEmpty) } - return rep + return r } // StripCommandArgs removes all the arguments from the command From 2c56ec2bf154cd7241e92b6e8fd851b0f9c908f0 Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Thu, 21 Feb 2019 16:46:17 +0100 Subject: [PATCH 4/7] Made censoring work properly. --- render/detailed/censor.go | 33 +++++++++++++++++++++++++++++++-- report/censor.go | 12 ++++++------ 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/render/detailed/censor.go b/render/detailed/censor.go index a36bca95eb..6a90c7a786 100644 --- a/render/detailed/censor.go +++ b/render/detailed/censor.go @@ -4,12 +4,41 @@ import ( "github.com/weaveworks/scope/report" ) +func isCommand(key string) bool { + return key == report.Cmdline || key == report.DockerContainerCommand +} + +func censorNodeSummary(s *NodeSummary, cfg report.CensorConfig) { + if cfg.HideEnvironmentVariables { + tables := []report.Table{} + for _, t := range s.Tables { + if t.ID != report.DockerEnvPrefix { + tables = append(tables, t) + } + } + s.Tables = tables + } + if cfg.HideCommandLineArguments { + for r := range s.Metadata { + if isCommand(s.Metadata[r].ID) { + s.Metadata[r].Value = report.StripCommandArgs(s.Metadata[r].Value) + } + } + } +} + // CensorNode ... func CensorNode(n Node, cfg report.CensorConfig) Node { + censorNodeSummary(&n.NodeSummary, cfg) return n } // CensorNodeSummaries ... -func CensorNodeSummaries(n NodeSummaries, cfg report.CensorConfig) NodeSummaries { - return n +func CensorNodeSummaries(ns NodeSummaries, cfg report.CensorConfig) NodeSummaries { + for key := range ns { + n := ns[key] + censorNodeSummary(&n, cfg) + ns[key] = n + } + return ns } diff --git a/report/censor.go b/report/censor.go index 913ddf1c92..aced7710aa 100644 --- a/report/censor.go +++ b/report/censor.go @@ -36,15 +36,15 @@ func censorTopology(t *Topology, match keyMatcher, censor censorValueFunc) { // CensorConfig describes how probe reports should // be censored when rendered through the API. type CensorConfig struct { - hideCommandLineArguments bool - hideEnvironmentVariables bool + HideCommandLineArguments bool + HideEnvironmentVariables bool } // GetCensorConfigFromQueryParams extracts censor config from request query params. func GetCensorConfigFromQueryParams(req *http.Request) CensorConfig { return CensorConfig{ - hideCommandLineArguments: true || req.URL.Query().Get("hideCommandLineArguments") == "true", - hideEnvironmentVariables: true || req.URL.Query().Get("hideEnvironmentVariables") == "true", + HideCommandLineArguments: true || req.URL.Query().Get("hideCommandLineArguments") == "true", + HideEnvironmentVariables: true || req.URL.Query().Get("hideEnvironmentVariables") == "true", } } @@ -54,11 +54,11 @@ func CensorRawReport(r Report, cfg CensorConfig) Report { var ( makeEmpty = func(string) string { return "" } ) - if cfg.hideCommandLineArguments { + if cfg.HideCommandLineArguments { censorTopology(&r.Process, keyEquals(Cmdline), StripCommandArgs) censorTopology(&r.Container, keyEquals(DockerContainerCommand), StripCommandArgs) } - if cfg.hideEnvironmentVariables { + if cfg.HideEnvironmentVariables { censorTopology(&r.Container, keyStartsWith(DockerEnvPrefix), makeEmpty) } return r From c5022bd2bb68d7824453300580d82e6e2f40270a Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Thu, 21 Feb 2019 17:02:02 +0100 Subject: [PATCH 5/7] Code cleanup. --- app/api_report.go | 2 +- app/api_topologies.go | 5 +-- app/api_topology.go | 27 ++++++------- render/detailed/censor.go | 39 +++++++++---------- report/censor.go | 81 ++++++++++++++++++--------------------- 5 files changed, 73 insertions(+), 81 deletions(-) diff --git a/app/api_report.go b/app/api_report.go index 3ccdf94dbe..c7cc0c5f66 100644 --- a/app/api_report.go +++ b/app/api_report.go @@ -13,12 +13,12 @@ import ( // Raw report handler func makeRawReportHandler(rep Reporter) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) { - censorCfg := report.GetCensorConfigFromQueryParams(r) rawReport, err := rep.Report(ctx, time.Now()) if err != nil { respondWith(w, http.StatusInternalServerError, err) return } + censorCfg := report.GetCensorConfigFromRequest(r) respondWith(w, http.StatusOK, report.CensorRawReport(rawReport, censorCfg)) } } diff --git a/app/api_topologies.go b/app/api_topologies.go index 294bd2c723..136d6960f6 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -577,12 +577,11 @@ func (r *Registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFu return } req.ParseForm() - rc := RenderContextForReporter(rep, rpt) - renderer, filter, err := r.RendererForTopology(topologyID, req.Form, rc.Report) + renderer, filter, err := r.RendererForTopology(topologyID, req.Form, rpt) if err != nil { respondWith(w, http.StatusInternalServerError, err) return } - f(ctx, renderer, filter, rc, w, req) + f(ctx, renderer, filter, RenderContextForReporter(rep, rpt), w, req) } } diff --git a/app/api_topology.go b/app/api_topology.go index c67266d4c0..9a48ada0f4 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -42,10 +42,7 @@ type rendererHandler func(context.Context, render.Renderer, render.Transformer, // Full topology. func handleTopology(ctx context.Context, renderer render.Renderer, transformer render.Transformer, rc detailed.RenderContext, w http.ResponseWriter, r *http.Request) { - // log.Infof("blublu %v", rc.Report.Container.Nodes) - // log.Infof("blublu %v", rc.Report.ContainerImage.Nodes) - // log.Infof("blibli %v", render.Render(ctx, rc.Report, renderer, transformer).Nodes) - censorCfg := report.GetCensorConfigFromQueryParams(r) + censorCfg := report.GetCensorConfigFromRequest(r) nodeSummaries := detailed.Summaries(ctx, rc, render.Render(ctx, rc.Report, renderer, transformer).Nodes) respondWith(w, http.StatusOK, APITopology{ Nodes: detailed.CensorNodeSummaries(nodeSummaries, censorCfg), @@ -55,6 +52,7 @@ func handleTopology(ctx context.Context, renderer render.Renderer, transformer r // Individual nodes. func handleNode(ctx context.Context, renderer render.Renderer, transformer render.Transformer, rc detailed.RenderContext, w http.ResponseWriter, r *http.Request) { var ( + censorCfg = report.GetCensorConfigFromRequest(r) vars = mux.Vars(r) topologyID = vars["topology"] nodeID = vars["id"] @@ -77,7 +75,6 @@ func handleNode(ctx context.Context, renderer render.Renderer, transformer rende nodes.Nodes[nodeID] = node nodes.Filtered-- } - censorCfg := report.GetCensorConfigFromQueryParams(r) rawNode := detailed.MakeNode(topologyID, rc, nodes.Nodes, node) respondWith(w, http.StatusOK, APINode{Node: detailed.CensorNode(rawNode, censorCfg)}) } @@ -128,6 +125,7 @@ func handleWebsocket( wait = make(chan struct{}, 1) topologyID = mux.Vars(r)["topology"] startReportingAt = deserializeTimestamp(r.Form.Get("timestamp")) + censorCfg = report.GetCensorConfigFromRequest(r) channelOpenedAt = time.Now() ) @@ -148,20 +146,23 @@ func handleWebsocket( log.Errorf("Error generating report: %v", err) return } - rc := RenderContextForReporter(rep, re) - renderer, filter, err := topologyRegistry.RendererForTopology(topologyID, r.Form, rc.Report) + renderer, filter, err := topologyRegistry.RendererForTopology(topologyID, r.Form, re) if err != nil { log.Errorf("Error generating report: %v", err) return } - // log.Infof("blublu %v", rc.Report.Container.Nodes) - censorCfg := report.GetCensorConfigFromQueryParams(r) - newTopo := detailed.Summaries(ctx, rc, render.Render(ctx, rc.Report, renderer, filter).Nodes) - newTopo = detailed.CensorNodeSummaries(newTopo, censorCfg) - // log.Infof("blublu %v", newTopo) + + newTopo := detailed.CensorNodeSummaries( + detailed.Summaries( + ctx, + RenderContextForReporter(rep, re), + render.Render(ctx, re, renderer, filter).Nodes, + ), + censorCfg, + ) diff := detailed.TopoDiff(previousTopo, newTopo) previousTopo = newTopo - // log.Infof("dfdfdf %v", diff) + if err := conn.WriteJSON(diff); err != nil { if !xfer.IsExpectedWSCloseError(err) { log.Errorf("cannot serialize topology diff: %s", err) diff --git a/render/detailed/censor.go b/render/detailed/censor.go index 6a90c7a786..56dd1d09fa 100644 --- a/render/detailed/censor.go +++ b/render/detailed/censor.go @@ -4,41 +4,40 @@ import ( "github.com/weaveworks/scope/report" ) -func isCommand(key string) bool { - return key == report.Cmdline || key == report.DockerContainerCommand -} - func censorNodeSummary(s *NodeSummary, cfg report.CensorConfig) { - if cfg.HideEnvironmentVariables { - tables := []report.Table{} - for _, t := range s.Tables { - if t.ID != report.DockerEnvPrefix { - tables = append(tables, t) + if cfg.HideCommandLineArguments { + // Iterate through all the metadata rows and strip the + // arguments from all the values containing a command. + for index := range s.Metadata { + row := &s.Metadata[index] + if report.IsCommandEntry(row.ID) { + row.Value = report.StripCommandArgs(row.Value) } } - s.Tables = tables } - if cfg.HideCommandLineArguments { - for r := range s.Metadata { - if isCommand(s.Metadata[r].ID) { - s.Metadata[r].Value = report.StripCommandArgs(s.Metadata[r].Value) + if cfg.HideEnvironmentVariables { + // Go through all the tables and if environment variables + // table is found, drop it from the list and stop the loop. + for index, table := range s.Tables { + if report.IsEnvironmentVarsEntry(table.ID) { + s.Tables = append(s.Tables[:index], s.Tables[index+1:]...) + break } } } } -// CensorNode ... +// CensorNode removes any sensitive data from a node. func CensorNode(n Node, cfg report.CensorConfig) Node { censorNodeSummary(&n.NodeSummary, cfg) return n } -// CensorNodeSummaries ... +// CensorNodeSummaries removes any sensitive data from a list of node summaries. func CensorNodeSummaries(ns NodeSummaries, cfg report.CensorConfig) NodeSummaries { - for key := range ns { - n := ns[key] - censorNodeSummary(&n, cfg) - ns[key] = n + for key, summary := range ns { + censorNodeSummary(&summary, cfg) + ns[key] = summary } return ns } diff --git a/report/censor.go b/report/censor.go index aced7710aa..83b220c905 100644 --- a/report/censor.go +++ b/report/censor.go @@ -5,34 +5,6 @@ import ( "strings" ) -type keyMatcher func(string) bool - -func keyEquals(fixedKey string) keyMatcher { - return func(key string) bool { - return key == fixedKey - } -} - -func keyStartsWith(prefix string) keyMatcher { - return func(key string) bool { - return strings.HasPrefix(key, prefix) - } -} - -type censorValueFunc func(string) string - -// TODO: Implement this in a more systematic way. -func censorTopology(t *Topology, match keyMatcher, censor censorValueFunc) { - for nodeID := range t.Nodes { - for entryID := range t.Nodes[nodeID].Latest { - entry := &t.Nodes[nodeID].Latest[entryID] - if match(entry.key) { - entry.Value = censor(entry.Value) - } - } - } -} - // CensorConfig describes how probe reports should // be censored when rendered through the API. type CensorConfig struct { @@ -40,31 +12,52 @@ type CensorConfig struct { HideEnvironmentVariables bool } -// GetCensorConfigFromQueryParams extracts censor config from request query params. -func GetCensorConfigFromQueryParams(req *http.Request) CensorConfig { +// GetCensorConfigFromRequest extracts censor config from request query params. +func GetCensorConfigFromRequest(req *http.Request) CensorConfig { return CensorConfig{ HideCommandLineArguments: true || req.URL.Query().Get("hideCommandLineArguments") == "true", HideEnvironmentVariables: true || req.URL.Query().Get("hideEnvironmentVariables") == "true", } } -// CensorRawReport removes any sensitive data from -// the raw report based on the request query params. -func CensorRawReport(r Report, cfg CensorConfig) Report { - var ( - makeEmpty = func(string) string { return "" } - ) - if cfg.HideCommandLineArguments { - censorTopology(&r.Process, keyEquals(Cmdline), StripCommandArgs) - censorTopology(&r.Container, keyEquals(DockerContainerCommand), StripCommandArgs) - } - if cfg.HideEnvironmentVariables { - censorTopology(&r.Container, keyStartsWith(DockerEnvPrefix), makeEmpty) - } - return r +// IsCommandEntry returns true iff the entry comes from a command line +// that might need to be conditionally censored. +func IsCommandEntry(key string) bool { + return key == Cmdline || key == DockerContainerCommand +} + +// IsEnvironmentVarsEntry returns true if the entry might expose some +// environment variables data might need to be conditionally censored. +func IsEnvironmentVarsEntry(key string) bool { + return strings.HasPrefix(key, DockerEnvPrefix) } // StripCommandArgs removes all the arguments from the command func StripCommandArgs(command string) string { return strings.Split(command, " ")[0] } + +// CensorRawReport removes any sensitive data from +// the raw report based on the request query params. +func CensorRawReport(r Report, cfg CensorConfig) Report { + r.WalkTopologies(func(t *Topology) { + for nodeID, node := range t.Nodes { + latest := StringLatestMap{} + for _, entry := range node.Latest { + // If environment variables are to be hidden, omit passing them to the final report. + if cfg.HideEnvironmentVariables && IsEnvironmentVarsEntry(entry.key) { + continue + } + // If command line arguments are to be hidden, strip them away. + if cfg.HideCommandLineArguments && IsCommandEntry(entry.key) { + entry.Value = StripCommandArgs(entry.Value) + } + // Pass the latest entry to the final report. + latest = append(latest, entry) + } + node.Latest = latest + t.Nodes[nodeID] = node + } + }) + return r +} From 353ab75ddbfc521f1d34144db2774a3e4b71eb52 Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Sat, 23 Feb 2019 19:30:42 +0100 Subject: [PATCH 6/7] Added some tests for censoring. --- render/detailed/censor.go | 45 ++++--- render/detailed/censor_test.go | 228 +++++++++++++++++++++++++++++++++ report/censor.go | 45 ++++--- report/censor_test.go | 102 +++++++++++++++ report/controls.go | 3 + report/dns.go | 3 + report/table.go | 2 +- report/topology.go | 3 + 8 files changed, 390 insertions(+), 41 deletions(-) create mode 100644 render/detailed/censor_test.go create mode 100644 report/censor_test.go diff --git a/render/detailed/censor.go b/render/detailed/censor.go index 56dd1d09fa..5e434856d3 100644 --- a/render/detailed/censor.go +++ b/render/detailed/censor.go @@ -4,40 +4,45 @@ import ( "github.com/weaveworks/scope/report" ) -func censorNodeSummary(s *NodeSummary, cfg report.CensorConfig) { - if cfg.HideCommandLineArguments { +func censorNodeSummary(s NodeSummary, cfg report.CensorConfig) NodeSummary { + if cfg.HideCommandLineArguments && s.Metadata != nil { // Iterate through all the metadata rows and strip the - // arguments from all the values containing a command. - for index := range s.Metadata { - row := &s.Metadata[index] + // arguments from all the values containing a command + // (while making sure everything is done in a non-mutable way). + metadata := []report.MetadataRow{} + for _, row := range s.Metadata { if report.IsCommandEntry(row.ID) { row.Value = report.StripCommandArgs(row.Value) } + metadata = append(metadata, row) } + s.Metadata = metadata } - if cfg.HideEnvironmentVariables { - // Go through all the tables and if environment variables - // table is found, drop it from the list and stop the loop. - for index, table := range s.Tables { - if report.IsEnvironmentVarsEntry(table.ID) { - s.Tables = append(s.Tables[:index], s.Tables[index+1:]...) - break + if cfg.HideEnvironmentVariables && s.Tables != nil { + // Copy across all the tables except the environment + // variable ones (ensuring the operation is non-mutable). + tables := []report.Table{} + for _, table := range s.Tables { + if !report.IsEnvironmentVarsEntry(table.ID) { + tables = append(tables, table) } } + s.Tables = tables } + return s } // CensorNode removes any sensitive data from a node. -func CensorNode(n Node, cfg report.CensorConfig) Node { - censorNodeSummary(&n.NodeSummary, cfg) - return n +func CensorNode(node Node, cfg report.CensorConfig) Node { + node.NodeSummary = censorNodeSummary(node.NodeSummary, cfg) + return node } // CensorNodeSummaries removes any sensitive data from a list of node summaries. -func CensorNodeSummaries(ns NodeSummaries, cfg report.CensorConfig) NodeSummaries { - for key, summary := range ns { - censorNodeSummary(&summary, cfg) - ns[key] = summary +func CensorNodeSummaries(summaries NodeSummaries, cfg report.CensorConfig) NodeSummaries { + censored := NodeSummaries{} + for key := range summaries { + censored[key] = censorNodeSummary(summaries[key], cfg) } - return ns + return censored } diff --git a/render/detailed/censor_test.go b/render/detailed/censor_test.go new file mode 100644 index 0000000000..88cf0e0721 --- /dev/null +++ b/render/detailed/censor_test.go @@ -0,0 +1,228 @@ +package detailed_test + +import ( + "reflect" + "testing" + + "github.com/weaveworks/common/test" + "github.com/weaveworks/scope/render/detailed" + "github.com/weaveworks/scope/report" +) + +func TestCensorNode(t *testing.T) { + node := detailed.Node{ + NodeSummary: detailed.NodeSummary{ + Metadata: []report.MetadataRow{ + {ID: "cmdline", Label: "Command", Value: "prog -a --b=c"}, + }, + Tables: []report.Table{ + {ID: "blibli", Rows: []report.Row{{ID: "bli"}}}, + {ID: "docker_env_", Rows: []report.Row{{ID: "env_var"}}}, + }, + }, + } + + for _, c := range []struct { + label string + have, want detailed.Node + }{ + { + label: "no censoring", + have: detailed.CensorNode(node, report.CensorConfig{ + HideCommandLineArguments: false, + HideEnvironmentVariables: false, + }), + want: detailed.Node{ + NodeSummary: detailed.NodeSummary{ + Metadata: []report.MetadataRow{ + {ID: "cmdline", Label: "Command", Value: "prog -a --b=c"}, + }, + Tables: []report.Table{ + {ID: "blibli", Rows: []report.Row{{ID: "bli"}}}, + {ID: "docker_env_", Rows: []report.Row{{ID: "env_var"}}}, + }, + }, + }, + }, + { + label: "censor only command line args", + have: detailed.CensorNode(node, report.CensorConfig{ + HideCommandLineArguments: true, + HideEnvironmentVariables: false, + }), + want: detailed.Node{ + NodeSummary: detailed.NodeSummary{ + Metadata: []report.MetadataRow{ + {ID: "cmdline", Label: "Command", Value: "prog"}, + }, + Tables: []report.Table{ + {ID: "blibli", Rows: []report.Row{{ID: "bli"}}}, + {ID: "docker_env_", Rows: []report.Row{{ID: "env_var"}}}, + }, + }, + }, + }, + { + label: "censor only env variables", + have: detailed.CensorNode(node, report.CensorConfig{ + HideCommandLineArguments: false, + HideEnvironmentVariables: true, + }), + want: detailed.Node{ + NodeSummary: detailed.NodeSummary{ + Metadata: []report.MetadataRow{ + {ID: "cmdline", Label: "Command", Value: "prog -a --b=c"}, + }, + Tables: []report.Table{ + {ID: "blibli", Rows: []report.Row{{ID: "bli"}}}, + }, + }, + }, + }, + { + label: "censor both command line args and env vars", + have: detailed.CensorNode(node, report.CensorConfig{ + HideCommandLineArguments: true, + HideEnvironmentVariables: true, + }), + want: detailed.Node{ + NodeSummary: detailed.NodeSummary{ + Metadata: []report.MetadataRow{ + {ID: "cmdline", Label: "Command", Value: "prog"}, + }, + Tables: []report.Table{ + {ID: "blibli", Rows: []report.Row{{ID: "bli"}}}, + }, + }, + }, + }, + } { + if !reflect.DeepEqual(c.want, c.have) { + t.Errorf("%s - %s", c.label, test.Diff(c.want, c.have)) + } + } +} + +func TestCensorNodeSummaries(t *testing.T) { + summaries := detailed.NodeSummaries{ + "a": detailed.NodeSummary{ + Metadata: []report.MetadataRow{ + {ID: "blublu", Label: "blabla", Value: "blu blu"}, + {ID: "docker_container_command", Label: "Command", Value: "scope --token=blibli"}, + }, + }, + "b": detailed.NodeSummary{ + Metadata: []report.MetadataRow{ + {ID: "cmdline", Label: "Command", Value: "prog -a --b=c"}, + }, + Tables: []report.Table{ + {ID: "blibli", Rows: []report.Row{{ID: "bli"}}}, + {ID: "docker_env_", Rows: []report.Row{{ID: "env_var"}}}, + }, + }, + } + + for _, c := range []struct { + label string + have, want detailed.NodeSummaries + }{ + { + label: "no censoring", + have: detailed.CensorNodeSummaries(summaries, report.CensorConfig{ + HideCommandLineArguments: false, + HideEnvironmentVariables: false, + }), + want: detailed.NodeSummaries{ + "a": detailed.NodeSummary{ + Metadata: []report.MetadataRow{ + {ID: "blublu", Label: "blabla", Value: "blu blu"}, + {ID: "docker_container_command", Label: "Command", Value: "scope --token=blibli"}, + }, + }, + "b": detailed.NodeSummary{ + Metadata: []report.MetadataRow{ + {ID: "cmdline", Label: "Command", Value: "prog -a --b=c"}, + }, + Tables: []report.Table{ + {ID: "blibli", Rows: []report.Row{{ID: "bli"}}}, + {ID: "docker_env_", Rows: []report.Row{{ID: "env_var"}}}, + }, + }, + }, + }, + { + label: "censor only command line args", + have: detailed.CensorNodeSummaries(summaries, report.CensorConfig{ + HideCommandLineArguments: true, + HideEnvironmentVariables: false, + }), + want: detailed.NodeSummaries{ + "a": detailed.NodeSummary{ + Metadata: []report.MetadataRow{ + {ID: "blublu", Label: "blabla", Value: "blu blu"}, + {ID: "docker_container_command", Label: "Command", Value: "scope"}, + }, + }, + "b": detailed.NodeSummary{ + Metadata: []report.MetadataRow{ + {ID: "cmdline", Label: "Command", Value: "prog"}, + }, + Tables: []report.Table{ + {ID: "blibli", Rows: []report.Row{{ID: "bli"}}}, + {ID: "docker_env_", Rows: []report.Row{{ID: "env_var"}}}, + }, + }, + }, + }, + { + label: "censor only env variables", + have: detailed.CensorNodeSummaries(summaries, report.CensorConfig{ + HideCommandLineArguments: false, + HideEnvironmentVariables: true, + }), + want: detailed.NodeSummaries{ + "a": detailed.NodeSummary{ + Metadata: []report.MetadataRow{ + {ID: "blublu", Label: "blabla", Value: "blu blu"}, + {ID: "docker_container_command", Label: "Command", Value: "scope --token=blibli"}, + }, + }, + "b": detailed.NodeSummary{ + Metadata: []report.MetadataRow{ + {ID: "cmdline", Label: "Command", Value: "prog -a --b=c"}, + }, + Tables: []report.Table{ + {ID: "blibli", Rows: []report.Row{{ID: "bli"}}}, + }, + }, + }, + }, + { + label: "censor both command line args and env vars", + have: detailed.CensorNodeSummaries(summaries, report.CensorConfig{ + HideCommandLineArguments: true, + HideEnvironmentVariables: true, + }), + want: detailed.NodeSummaries{ + "a": detailed.NodeSummary{ + Metadata: []report.MetadataRow{ + {ID: "blublu", Label: "blabla", Value: "blu blu"}, + {ID: "docker_container_command", Label: "Command", Value: "scope"}, + }, + }, + "b": detailed.NodeSummary{ + Metadata: []report.MetadataRow{ + {ID: "cmdline", Label: "Command", Value: "prog"}, + }, + Tables: []report.Table{ + {ID: "blibli", Rows: []report.Row{{ID: "bli"}}}, + }, + }, + }, + }, + } { + if !reflect.DeepEqual(c.want, c.have) { + t.Errorf("%s - %s", c.label, test.Diff(c.want, c.have)) + } + } +} diff --git a/report/censor.go b/report/censor.go index 83b220c905..819ceb1867 100644 --- a/report/censor.go +++ b/report/censor.go @@ -15,8 +15,8 @@ type CensorConfig struct { // GetCensorConfigFromRequest extracts censor config from request query params. func GetCensorConfigFromRequest(req *http.Request) CensorConfig { return CensorConfig{ - HideCommandLineArguments: true || req.URL.Query().Get("hideCommandLineArguments") == "true", - HideEnvironmentVariables: true || req.URL.Query().Get("hideEnvironmentVariables") == "true", + HideCommandLineArguments: req.URL.Query().Get("hideCommandLineArguments") == "true", + HideEnvironmentVariables: req.URL.Query().Get("hideEnvironmentVariables") == "true", } } @@ -37,27 +37,32 @@ func StripCommandArgs(command string) string { return strings.Split(command, " ")[0] } -// CensorRawReport removes any sensitive data from -// the raw report based on the request query params. -func CensorRawReport(r Report, cfg CensorConfig) Report { - r.WalkTopologies(func(t *Topology) { +// CensorRawReport removes any sensitive data from the raw report based on the request query params. +func CensorRawReport(rawReport Report, cfg CensorConfig) Report { + // Create a copy of the report first to make sure the operation is immutable. + censoredReport := rawReport.Copy() + censoredReport.ID = rawReport.ID + + censoredReport.WalkTopologies(func(t *Topology) { for nodeID, node := range t.Nodes { - latest := StringLatestMap{} - for _, entry := range node.Latest { - // If environment variables are to be hidden, omit passing them to the final report. - if cfg.HideEnvironmentVariables && IsEnvironmentVarsEntry(entry.key) { - continue - } - // If command line arguments are to be hidden, strip them away. - if cfg.HideCommandLineArguments && IsCommandEntry(entry.key) { - entry.Value = StripCommandArgs(entry.Value) + if node.Latest != nil { + latest := make(StringLatestMap, 0, cap(node.Latest)) + for _, entry := range node.Latest { + // If environment variables are to be hidden, omit passing them to the final report. + if cfg.HideEnvironmentVariables && IsEnvironmentVarsEntry(entry.key) { + continue + } + // If command line arguments are to be hidden, strip them away. + if cfg.HideCommandLineArguments && IsCommandEntry(entry.key) { + entry.Value = StripCommandArgs(entry.Value) + } + // Pass the latest entry to the final report. + latest = append(latest, entry) } - // Pass the latest entry to the final report. - latest = append(latest, entry) + node.Latest = latest + t.Nodes[nodeID] = node } - node.Latest = latest - t.Nodes[nodeID] = node } }) - return r + return censoredReport } diff --git a/report/censor_test.go b/report/censor_test.go new file mode 100644 index 0000000000..807766d313 --- /dev/null +++ b/report/censor_test.go @@ -0,0 +1,102 @@ +package report_test + +import ( + "testing" + + "github.com/weaveworks/common/test" + "github.com/weaveworks/scope/report" + "github.com/weaveworks/scope/test/reflect" +) + +func TestCensorRawReport(t *testing.T) { + r := report.Report{ + Container: report.Topology{ + Nodes: report.Nodes{ + "a": report.MakeNodeWith("a", map[string]string{ + "docker_container_command": "prog -a --b=c", + "blublu": "blu blu", + "docker_env_": "env_var", + }), + }, + }, + Process: report.Topology{ + Nodes: report.Nodes{ + "b": report.MakeNodeWith("b", map[string]string{ + "cmdline": "scope --token=blibli", + "blibli": "bli bli", + }), + "c": report.MakeNodeWith("c", map[string]string{ + "docker_env_": "var", + }), + }, + }, + } + + for _, c := range []struct { + label string + have, want report.Report + }{ + { + label: "no censoring", + have: report.CensorRawReport(r, report.CensorConfig{ + HideCommandLineArguments: false, + HideEnvironmentVariables: false, + }), + want: report.Report{ + Container: report.Topology{ + Nodes: report.Nodes{ + "a": report.MakeNodeWith("a", map[string]string{ + "docker_container_command": "prog -a --b=c", + "blublu": "blu blu", + "docker_env_": "env_var", + }), + }, + }, + Process: report.Topology{ + Nodes: report.Nodes{ + "b": report.MakeNodeWith("b", map[string]string{ + "cmdline": "scope --token=blibli", + "blibli": "bli bli", + }), + "c": report.MakeNodeWith("c", map[string]string{ + "docker_env_": "var", + }), + }, + }, + }, + }, + // { + // label: "censor only command line args", + // have: report.CensorRawReport(r, report.CensorConfig{ + // HideCommandLineArguments: true, + // HideEnvironmentVariables: false, + // }), + // want: report.Report{ + // Container: report.Topology{ + // Nodes: report.Nodes{ + // "a": report.MakeNodeWith("a", map[string]string{ + // "docker_container_command": "prog", + // "blublu": "blu blu", + // "docker_env_": "env_var", + // }), + // }, + // }, + // Process: report.Topology{ + // Nodes: report.Nodes{ + // "b": report.MakeNodeWith("b", map[string]string{ + // "cmdline": "scope", + // "blibli": "bli bli", + // }), + // "c": report.MakeNodeWith("c", map[string]string{ + // "docker_env_": "var", + // }), + // }, + // }, + // }, + // }, + } { + if !reflect.DeepEqual(c.want, c.have) { + t.Errorf("%s - %s", c.label, test.Diff(c.want, c.have)) + } + } +} diff --git a/report/controls.go b/report/controls.go index 6f151bfdc5..f534de5dda 100644 --- a/report/controls.go +++ b/report/controls.go @@ -35,6 +35,9 @@ func (cs Controls) Merge(other Controls) Controls { // Copy produces a copy of cs. func (cs Controls) Copy() Controls { + if cs == nil { + return nil + } result := Controls{} for k, v := range cs { result[k] = v diff --git a/report/dns.go b/report/dns.go index d50667c643..5a398ceffb 100644 --- a/report/dns.go +++ b/report/dns.go @@ -11,6 +11,9 @@ type DNSRecords map[string]DNSRecord // Copy makes a copy of the DNSRecords func (r DNSRecords) Copy() DNSRecords { + if r == nil { + return nil + } cp := make(DNSRecords, len(r)) for k, v := range r { cp[k] = v diff --git a/report/table.go b/report/table.go index c3a1c05631..a05fef984a 100644 --- a/report/table.go +++ b/report/table.go @@ -136,7 +136,7 @@ func (node Node) ExtractTable(template TableTemplate) (rows []Row, truncationCou truncationCount = 0 if str, ok := node.Latest.Lookup(truncationCountPrefix + template.Prefix); ok { if n, err := fmt.Sscanf(str, "%d", &truncationCount); n != 1 || err != nil { - log.Warn("Unexpected truncation count format %q", str) + log.Warnf("Unexpected truncation count format %q", str) } } diff --git a/report/topology.go b/report/topology.go index d10094bac2..69b1379711 100644 --- a/report/topology.go +++ b/report/topology.go @@ -214,6 +214,9 @@ type Nodes map[string]Node // Copy returns a value copy of the Nodes. func (n Nodes) Copy() Nodes { + if n == nil { + return nil + } cp := make(Nodes, len(n)) for k, v := range n { cp[k] = v From b9e692c3b3d2967f116be2bda18b16e0e20224cb Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Mon, 25 Feb 2019 11:35:33 +0100 Subject: [PATCH 7/7] Lock the time in tests to make them pass. --- report/censor_test.go | 115 +++++++++++++++++++++++++++++++----------- 1 file changed, 86 insertions(+), 29 deletions(-) diff --git a/report/censor_test.go b/report/censor_test.go index 807766d313..ce25de6695 100644 --- a/report/censor_test.go +++ b/report/censor_test.go @@ -2,13 +2,18 @@ package report_test import ( "testing" + "time" + "github.com/weaveworks/common/mtime" "github.com/weaveworks/common/test" "github.com/weaveworks/scope/report" "github.com/weaveworks/scope/test/reflect" ) func TestCensorRawReport(t *testing.T) { + mtime.NowForce(time.Now()) + defer mtime.NowReset() + r := report.Report{ Container: report.Topology{ Nodes: report.Nodes{ @@ -65,35 +70,87 @@ func TestCensorRawReport(t *testing.T) { }, }, }, - // { - // label: "censor only command line args", - // have: report.CensorRawReport(r, report.CensorConfig{ - // HideCommandLineArguments: true, - // HideEnvironmentVariables: false, - // }), - // want: report.Report{ - // Container: report.Topology{ - // Nodes: report.Nodes{ - // "a": report.MakeNodeWith("a", map[string]string{ - // "docker_container_command": "prog", - // "blublu": "blu blu", - // "docker_env_": "env_var", - // }), - // }, - // }, - // Process: report.Topology{ - // Nodes: report.Nodes{ - // "b": report.MakeNodeWith("b", map[string]string{ - // "cmdline": "scope", - // "blibli": "bli bli", - // }), - // "c": report.MakeNodeWith("c", map[string]string{ - // "docker_env_": "var", - // }), - // }, - // }, - // }, - // }, + { + label: "censor only command line args", + have: report.CensorRawReport(r, report.CensorConfig{ + HideCommandLineArguments: true, + HideEnvironmentVariables: false, + }), + want: report.Report{ + Container: report.Topology{ + Nodes: report.Nodes{ + "a": report.MakeNodeWith("a", map[string]string{ + "docker_container_command": "prog", + "blublu": "blu blu", + "docker_env_": "env_var", + }), + }, + }, + Process: report.Topology{ + Nodes: report.Nodes{ + "b": report.MakeNodeWith("b", map[string]string{ + "cmdline": "scope", + "blibli": "bli bli", + }), + "c": report.MakeNodeWith("c", map[string]string{ + "docker_env_": "var", + }), + }, + }, + }, + }, + { + label: "censor only env variables", + have: report.CensorRawReport(r, report.CensorConfig{ + HideCommandLineArguments: false, + HideEnvironmentVariables: true, + }), + want: report.Report{ + Container: report.Topology{ + Nodes: report.Nodes{ + "a": report.MakeNodeWith("a", map[string]string{ + "docker_container_command": "prog -a --b=c", + "blublu": "blu blu", + }), + }, + }, + Process: report.Topology{ + Nodes: report.Nodes{ + "b": report.MakeNodeWith("b", map[string]string{ + "cmdline": "scope --token=blibli", + "blibli": "bli bli", + }), + "c": report.MakeNodeWith("c", map[string]string{}), + }, + }, + }, + }, + { + label: "censor both command line args and env vars", + have: report.CensorRawReport(r, report.CensorConfig{ + HideCommandLineArguments: true, + HideEnvironmentVariables: true, + }), + want: report.Report{ + Container: report.Topology{ + Nodes: report.Nodes{ + "a": report.MakeNodeWith("a", map[string]string{ + "docker_container_command": "prog", + "blublu": "blu blu", + }), + }, + }, + Process: report.Topology{ + Nodes: report.Nodes{ + "b": report.MakeNodeWith("b", map[string]string{ + "cmdline": "scope", + "blibli": "bli bli", + }), + "c": report.MakeNodeWith("c", map[string]string{}), + }, + }, + }, + }, } { if !reflect.DeepEqual(c.want, c.have) { t.Errorf("%s - %s", c.label, test.Diff(c.want, c.have))