From 3c5320ef09e1a3ae6ec2f9a6ee1785cc2d44f055 Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Fri, 15 Feb 2019 18:15:12 +0100 Subject: [PATCH] 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 a1d993cb0..3899f69e3 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 136d6960f..af94890ad 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 129aba6e2..1ed67e6f2 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 0cdf9e8a2..781790857 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 0313e88da..459cc593f 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 51bbb40eb..5e5f9e635 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 7bbdb2810..a6c24c95d 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 1400232f1..d3a6c3d98 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 +// 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 hideEnvironmentVariables { + censorTopology(&rep.Container, keyStartsWith(DockerEnvPrefix), makeEmpty) + } + return rep } -// 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 +// 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 887b10848..a6094c6ee 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 45a777c60..a6a446463 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) } }