Skip to content

Commit

Permalink
Add options to hide args and env vars (#2306)
Browse files Browse the repository at this point in the history
* Add options to hide args and env vars

To allow for use of weave-scope in an unauthenticated environment,
add options to the probe to hide comand line arguments and
environment variables, which might contain secret data.

Fixes #2222

* Change docker.NewRegistry arguments to be a struct

* Remove redundant declarations of default values

* Move registry options outside to improve readability
  • Loading branch information
mikebryant authored and Alfonso Acosta committed Mar 7, 2017
1 parent a391ae8 commit 764afb6
Show file tree
Hide file tree
Showing 9 changed files with 176 additions and 55 deletions.
38 changes: 26 additions & 12 deletions probe/docker/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,20 +98,24 @@ type Container interface {

type container struct {
sync.RWMutex
container *docker.Container
stopStats chan<- bool
latestStats docker.Stats
pendingStats [60]docker.Stats
numPending int
hostID string
baseNode report.Node
container *docker.Container
stopStats chan<- bool
latestStats docker.Stats
pendingStats [60]docker.Stats
numPending int
hostID string
baseNode report.Node
noCommandLineArguments bool
noEnvironmentVariables bool
}

// NewContainer creates a new Container
func NewContainer(c *docker.Container, hostID string) Container {
func NewContainer(c *docker.Container, hostID string, noCommandLineArguments bool, noEnvironmentVariables bool) Container {
result := &container{
container: c,
hostID: hostID,
container: c,
hostID: hostID,
noCommandLineArguments: noCommandLineArguments,
noEnvironmentVariables: noEnvironmentVariables,
}
result.baseNode = result.getBaseNode()
return result
Expand Down Expand Up @@ -369,18 +373,28 @@ func (c *container) env() map[string]string {
return result
}

func (c *container) getSanitizedCommand() string {
result := c.container.Path
if !c.noCommandLineArguments {
result = result + " " + strings.Join(c.container.Args, " ")
}
return result
}

func (c *container) getBaseNode() report.Node {
result := report.MakeNodeWith(report.MakeContainerNodeID(c.ID()), map[string]string{
ContainerID: c.ID(),
ContainerCreated: c.container.Created.Format(time.RFC3339Nano),
ContainerCommand: c.container.Path + " " + strings.Join(c.container.Args, " "),
ContainerCommand: c.getSanitizedCommand(),
ImageID: c.Image(),
ContainerHostname: c.Hostname(),
}).WithParents(report.EmptySets.
Add(report.ContainerImage, report.MakeStringSet(report.MakeContainerImageNodeID(c.Image()))),
)
result = result.AddPrefixPropertyList(LabelPrefix, c.container.Config.Labels)
result = result.AddPrefixPropertyList(EnvPrefix, c.env())
if !c.noEnvironmentVariables {
result = result.AddPrefixPropertyList(EnvPrefix, c.env())
}
return result
}

Expand Down
42 changes: 40 additions & 2 deletions probe/docker/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package docker_test

import (
"net"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -40,7 +41,7 @@ func TestContainer(t *testing.T) {
defer mtime.NowReset()

const hostID = "scope"
c := docker.NewContainer(container1, hostID)
c := docker.NewContainer(container1, hostID, false, false)
s := newMockStatsGatherer()
err := c.StartGatheringStats(s)
if err != nil {
Expand Down Expand Up @@ -69,7 +70,7 @@ func TestContainer(t *testing.T) {
docker.RemoveContainer: {Dead: true},
}
want := report.MakeNodeWith("ping;<container>", map[string]string{
"docker_container_command": " ",
"docker_container_command": "ping foo.bar.local",
"docker_container_created": "0001-01-01T00:00:00Z",
"docker_container_id": "ping",
"docker_container_name": "pong",
Expand All @@ -79,6 +80,7 @@ func TestContainer(t *testing.T) {
"docker_container_state": "running",
"docker_container_state_human": c.Container().State.String(),
"docker_container_uptime": uptime.String(),
"docker_env_FOO": "secret-bar",
}).WithLatestControls(
controls,
).WithMetrics(report.Metrics{
Expand Down Expand Up @@ -125,3 +127,39 @@ func TestContainer(t *testing.T) {
t.Errorf("%v != %v", have, []string{"1.2.3.4", "5.6.7.8"})
}
}

func TestContainerHidingArgs(t *testing.T) {
const hostID = "scope"
c := docker.NewContainer(container1, hostID, true, false)
node := c.GetNode()
node.Latest.ForEach(func(k string, _ time.Time, v string) {
if strings.Contains(v, "foo.bar.local") {
t.Errorf("Found command line argument in node")
}
})
}

func TestContainerHidingEnv(t *testing.T) {
const hostID = "scope"
c := docker.NewContainer(container1, hostID, false, true)
node := c.GetNode()
node.Latest.ForEach(func(k string, _ time.Time, v string) {
if strings.Contains(v, "secret-bar") {
t.Errorf("Found environment variable in node")
}
})
}

func TestContainerHidingBoth(t *testing.T) {
const hostID = "scope"
c := docker.NewContainer(container1, hostID, true, true)
node := c.GetNode()
node.Latest.ForEach(func(k string, _ time.Time, v string) {
if strings.Contains(v, "foo.bar.local") {
t.Errorf("Found command line argument in node")
}
if strings.Contains(v, "secret-bar") {
t.Errorf("Found environment variable in node")
}
})
}
10 changes: 8 additions & 2 deletions probe/docker/controls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ func TestControls(t *testing.T) {
mdc := newMockClient()
setupStubs(mdc, func() {
hr := controls.NewDefaultHandlerRegistry()
registry, _ := docker.NewRegistry(10*time.Second, nil, false, "", hr, "")
registry, _ := docker.NewRegistry(docker.RegistryOptions{
Interval: 10 * time.Second,
HandlerRegistry: hr,
})
defer registry.Stop()

for _, tc := range []struct{ command, result string }{
Expand Down Expand Up @@ -59,7 +62,10 @@ func TestPipes(t *testing.T) {
mdc := newMockClient()
setupStubs(mdc, func() {
hr := controls.NewDefaultHandlerRegistry()
registry, _ := docker.NewRegistry(10*time.Second, nil, false, "", hr, "")
registry, _ := docker.NewRegistry(docker.RegistryOptions{
Interval: 10 * time.Second,
HandlerRegistry: hr,
})
defer registry.Stop()

test.Poll(t, 100*time.Millisecond, true, func() interface{} {
Expand Down
45 changes: 30 additions & 15 deletions probe/docker/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,15 @@ type ContainerUpdateWatcher func(report.Node)

type registry struct {
sync.RWMutex
quit chan chan struct{}
interval time.Duration
collectStats bool
client Client
pipes controls.PipeClient
hostID string
handlerRegistry *controls.HandlerRegistry
quit chan chan struct{}
interval time.Duration
collectStats bool
client Client
pipes controls.PipeClient
hostID string
handlerRegistry *controls.HandlerRegistry
noCommandLineArguments bool
noEnvironmentVariables bool

watchers []ContainerUpdateWatcher
containers *radix.Tree
Expand Down Expand Up @@ -93,9 +95,20 @@ func newDockerClient(endpoint string) (Client, error) {
return docker_client.NewClient(endpoint)
}

type RegistryOptions struct {
Interval time.Duration
Pipes controls.PipeClient
CollectStats bool
HostID string
HandlerRegistry *controls.HandlerRegistry
DockerEndpoint string
NoCommandLineArguments bool
NoEnvironmentVariables bool
}

// NewRegistry returns a usable Registry. Don't forget to Stop it.
func NewRegistry(interval time.Duration, pipes controls.PipeClient, collectStats bool, hostID string, handlerRegistry *controls.HandlerRegistry, dockerEndpoint string) (Registry, error) {
client, err := NewDockerClientStub(dockerEndpoint)
func NewRegistry(options RegistryOptions) (Registry, error) {
client, err := NewDockerClientStub(options.DockerEndpoint)
if err != nil {
return nil, err
}
Expand All @@ -107,12 +120,14 @@ func NewRegistry(interval time.Duration, pipes controls.PipeClient, collectStats
pipeIDToexecID: map[string]string{},

client: client,
pipes: pipes,
interval: interval,
collectStats: collectStats,
hostID: hostID,
handlerRegistry: handlerRegistry,
pipes: options.Pipes,
interval: options.Interval,
collectStats: options.CollectStats,
hostID: options.HostID,
handlerRegistry: options.HandlerRegistry,
quit: make(chan chan struct{}),
noCommandLineArguments: options.NoCommandLineArguments,
noEnvironmentVariables: options.NoEnvironmentVariables,
}

r.registerControls()
Expand Down Expand Up @@ -339,7 +354,7 @@ func (r *registry) updateContainerState(containerID string, intendedState *strin
o, ok := r.containers.Get(containerID)
var c Container
if !ok {
c = NewContainerStub(dockerContainer, r.hostID)
c = NewContainerStub(dockerContainer, r.hostID, r.noCommandLineArguments, r.noEnvironmentVariables)
r.containers.Insert(containerID, c)
} else {
c = o.(Container)
Expand Down
15 changes: 13 additions & 2 deletions probe/docker/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ import (

func testRegistry() docker.Registry {
hr := controls.NewDefaultHandlerRegistry()
registry, _ := docker.NewRegistry(10*time.Second, nil, true, "", hr, "")
registry, _ := docker.NewRegistry(docker.RegistryOptions{
Interval: 10 * time.Second,
CollectStats: true,
HandlerRegistry: hr,
})
return registry
}

Expand Down Expand Up @@ -203,6 +207,10 @@ var (
ID: "ping",
Name: "pong",
Image: "baz",
Path: "ping",
Args: []string{
"foo.bar.local",
},
State: client.State{
Pid: 2,
Running: true,
Expand All @@ -226,6 +234,9 @@ var (
},
},
Config: &client.Config{
Env: []string{
"FOO=secret-bar",
},
Labels: map[string]string{
"foo1": "bar1",
"foo2": "bar2",
Expand Down Expand Up @@ -294,7 +305,7 @@ func setupStubs(mdc *mockDockerClient, f func()) {
return mdc, nil
}

docker.NewContainerStub = func(c *client.Container, _ string) docker.Container {
docker.NewContainerStub = func(c *client.Container, _ string, _ bool, _ bool) docker.Container {
return &mockContainer{c}
}

Expand Down
26 changes: 18 additions & 8 deletions probe/process/reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package process

import (
"strconv"
"strings"

"github.com/weaveworks/common/mtime"
"github.com/weaveworks/scope/report"
Expand Down Expand Up @@ -37,20 +38,22 @@ var (

// Reporter generates Reports containing the Process topology.
type Reporter struct {
scope string
walker Walker
jiffies Jiffies
scope string
walker Walker
jiffies Jiffies
noCommandLineArguments bool
}

// Jiffies is the type for the function used to fetch the elapsed jiffies.
type Jiffies func() (uint64, float64, error)

// NewReporter makes a new Reporter.
func NewReporter(walker Walker, scope string, jiffies Jiffies) *Reporter {
func NewReporter(walker Walker, scope string, jiffies Jiffies, noCommandLineArguments bool) *Reporter {
return &Reporter{
scope: scope,
walker: walker,
jiffies: jiffies,
scope: scope,
walker: walker,
jiffies: jiffies,
noCommandLineArguments: noCommandLineArguments,
}
}

Expand Down Expand Up @@ -85,14 +88,21 @@ func (r *Reporter) processTopology() (report.Topology, error) {
for _, tuple := range []struct{ key, value string }{
{PID, pidstr},
{Name, p.Name},
{Cmdline, p.Cmdline},
{Threads, strconv.Itoa(p.Threads)},
} {
if tuple.value != "" {
node = node.WithLatests(map[string]string{tuple.key: tuple.value})
}
}

if p.Cmdline != "" {
if r.noCommandLineArguments {
node = node.WithLatests(map[string]string{Cmdline: strings.Split(p.Cmdline, " ")[0]})
} else {
node = node.WithLatests(map[string]string{Cmdline: p.Cmdline})
}
}

if p.PPID > 0 {
node = node.WithLatests(map[string]string{PPID: strconv.Itoa(p.PPID)})
}
Expand Down
15 changes: 14 additions & 1 deletion probe/process/reporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func TestReporter(t *testing.T) {
mtime.NowForce(now)
defer mtime.NowReset()

rpt, err := process.NewReporter(walker, "", getDeltaTotalJiffies).Report()
rpt, err := process.NewReporter(walker, "", getDeltaTotalJiffies, false).Report()
if err != nil {
t.Error(err)
}
Expand Down Expand Up @@ -97,4 +97,17 @@ func TestReporter(t *testing.T) {
if cmdline, ok := node.Latest.Lookup(process.Cmdline); !ok || cmdline != fmt.Sprint(processes[4].Cmdline) {
t.Errorf("Expected %q got %q", processes[4].Cmdline, cmdline)
}

// It doesn't report Cmdline args when asked not to
rpt, err = process.NewReporter(walker, "", getDeltaTotalJiffies, true).Report()
if err != nil {
t.Error(err)
}
node, ok = rpt.Process.Nodes[report.MakeProcessNodeID("", "4")]
if !ok {
t.Errorf("Expected report to include the pid 4 ping")
}
if cmdline, ok := node.Latest.Lookup(process.Cmdline); !ok || cmdline != fmt.Sprint("ping") {
t.Errorf("Expected %q got %q", "ping", cmdline)
}
}
Loading

0 comments on commit 764afb6

Please sign in to comment.