From 846aa98a9c1a5ec5c3b23d155d9e3f4a688be20d Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Fri, 18 Sep 2015 15:09:08 +0100 Subject: [PATCH 1/4] renaming test so it runs --- test/{666_the_beast.sh => 666_abuse_of_start_test.sh} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/{666_the_beast.sh => 666_abuse_of_start_test.sh} (100%) diff --git a/test/666_the_beast.sh b/test/666_abuse_of_start_test.sh similarity index 100% rename from test/666_the_beast.sh rename to test/666_abuse_of_start_test.sh From 74c3b0d0795bdab258792bcedb7d7b6fa19de3dd Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Fri, 18 Sep 2015 16:17:35 +0100 Subject: [PATCH 2/4] [proxy] Override sneaky /start HostConfig params --- proxy/common.go | 5 ++ proxy/create_container_interceptor.go | 73 +++------------------------ proxy/proxy.go | 54 ++++++++++++++++++++ proxy/start_container_interceptor.go | 31 ++++++++++-- test/666_abuse_of_start_test.sh | 10 +++- 5 files changed, 103 insertions(+), 70 deletions(-) diff --git a/proxy/common.go b/proxy/common.go index 4c0f3fc3..0ea7ebe6 100644 --- a/proxy/common.go +++ b/proxy/common.go @@ -50,6 +50,10 @@ func unmarshalRequestBody(r *http.Request, target interface{}) error { } r.Body = ioutil.NopCloser(bytes.NewReader(body)) + if len(body) == 0 { + return nil + } + d := json.NewDecoder(bytes.NewReader(body)) d.UseNumber() // don't want large numbers in scientific format return d.Decode(&target) @@ -62,6 +66,7 @@ func marshalRequestBody(r *http.Request, body interface{}) error { } r.Body = ioutil.NopCloser(bytes.NewReader(newBody)) r.ContentLength = int64(len(newBody)) + r.Header.Set("Content-Type", "application/json") return nil } diff --git a/proxy/create_container_interceptor.go b/proxy/create_container_interceptor.go index 5ea64704..70e0198f 100644 --- a/proxy/create_container_interceptor.go +++ b/proxy/create_container_interceptor.go @@ -2,16 +2,11 @@ package proxy import ( "errors" - "fmt" - "io/ioutil" "net/http" "strings" "github.com/fsouza/go-dockerclient" - . "github.com/weaveworks/weave/common" - "github.com/weaveworks/weave/nameserver" - "github.com/weaveworks/weave/router" ) const MaxDockerHostname = 64 @@ -55,7 +50,7 @@ func (i *createContainerInterceptor) InterceptRequest(r *http.Request) error { if container.Config == nil { container.Config = &docker.Config{} } - i.addWeaveWaitVolume(container.HostConfig) + i.proxy.addWeaveWaitVolume(container.HostConfig) if err := i.setWeaveWaitEntrypoint(container.Config); err != nil { return err } @@ -66,8 +61,11 @@ func (i *createContainerInterceptor) InterceptRequest(r *http.Request) error { } } hostname = i.proxy.hostnameMatchRegexp.ReplaceAllString(hostname, i.proxy.HostnameReplacement) - if err := i.setWeaveDNS(&container, hostname); err != nil { - return err + if dnsDomain, withDNS := i.proxy.getDNSDomain(); withDNS { + i.setHostname(&container, hostname, dnsDomain) + if err := i.proxy.setWeaveDNS(container.HostConfig, container.Hostname, dnsDomain); err != nil { + return err + } } return marshalRequestBody(r, container) @@ -76,18 +74,6 @@ func (i *createContainerInterceptor) InterceptRequest(r *http.Request) error { return nil } -func (i *createContainerInterceptor) addWeaveWaitVolume(hostConfig *docker.HostConfig) { - var binds []string - for _, bind := range hostConfig.Binds { - s := strings.Split(bind, ":") - if len(s) >= 2 && s[1] == "/w" { - continue - } - binds = append(binds, bind) - } - hostConfig.Binds = append(binds, fmt.Sprintf("%s:/w:ro", i.proxy.weaveWaitVolume)) -} - func (i *createContainerInterceptor) setWeaveWaitEntrypoint(container *docker.Config) error { if len(container.Entrypoint) == 0 { image, err := i.proxy.client.InspectImage(container.Image) @@ -117,18 +103,7 @@ func (i *createContainerInterceptor) setWeaveWaitEntrypoint(container *docker.Co return nil } -func (i *createContainerInterceptor) setWeaveDNS(container *createContainerRequestBody, name string) error { - if i.proxy.WithoutDNS { - return nil - } - - dnsDomain, dnsRunning := i.getDNSDomain() - if !(dnsRunning || i.proxy.WithDNS) { - return nil - } - - container.HostConfig.DNS = append(container.HostConfig.DNS, i.proxy.dockerBridgeIP) - +func (i *createContainerInterceptor) setHostname(container *createContainerRequestBody, name, dnsDomain string) { if container.Hostname == "" && name != "" { // Strip trailing period because it's unusual to see it used on the end of a host name trimmedDNSDomain := strings.TrimSuffix(dnsDomain, ".") @@ -140,39 +115,7 @@ func (i *createContainerInterceptor) setWeaveDNS(container *createContainerReque } } - if len(container.HostConfig.DNSSearch) == 0 { - if container.Hostname == "" { - container.HostConfig.DNSSearch = []string{dnsDomain} - } else { - container.HostConfig.DNSSearch = []string{"."} - } - } - - return nil -} - -func (i *createContainerInterceptor) getDNSDomain() (domain string, running bool) { - domain = nameserver.DefaultDomain - weaveContainer, err := i.proxy.client.InspectContainer("weave") - if err != nil || - weaveContainer.NetworkSettings == nil || - weaveContainer.NetworkSettings.IPAddress == "" { - return - } - - url := fmt.Sprintf("http://%s:%d/domain", weaveContainer.NetworkSettings.IPAddress, router.HTTPPort) - resp, err := http.Get(url) - if err != nil || resp.StatusCode != http.StatusOK { - return - } - defer resp.Body.Close() - - b, err := ioutil.ReadAll(resp.Body) - if err != nil { - return - } - - return string(b), true + return } func (i *createContainerInterceptor) InterceptResponse(r *http.Response) error { diff --git a/proxy/proxy.go b/proxy/proxy.go index eeead60a..789a6232 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -4,6 +4,7 @@ import ( "crypto/tls" "errors" "fmt" + "io/ioutil" "net" "net/http" "os" @@ -15,6 +16,8 @@ import ( docker "github.com/fsouza/go-dockerclient" . "github.com/weaveworks/weave/common" weavedocker "github.com/weaveworks/weave/common/docker" + "github.com/weaveworks/weave/nameserver" + "github.com/weaveworks/weave/router" ) const ( @@ -365,3 +368,54 @@ func (proxy *Proxy) weaveCIDRsFromConfig(config *docker.Config, hostConfig *dock } return nil, nil } + +func (proxy *Proxy) addWeaveWaitVolume(hostConfig *docker.HostConfig) { + var binds []string + for _, bind := range hostConfig.Binds { + s := strings.Split(bind, ":") + if len(s) >= 2 && s[1] == "/w" { + continue + } + binds = append(binds, bind) + } + hostConfig.Binds = append(binds, fmt.Sprintf("%s:/w:ro", proxy.weaveWaitVolume)) +} + +func (proxy *Proxy) setWeaveDNS(hostConfig *docker.HostConfig, hostname, dnsDomain string) error { + hostConfig.DNS = append(hostConfig.DNS, proxy.dockerBridgeIP) + + if len(hostConfig.DNSSearch) == 0 { + if hostname == "" { + hostConfig.DNSSearch = []string{dnsDomain} + } else { + hostConfig.DNSSearch = []string{"."} + } + } + + return nil +} + +func (proxy *Proxy) getDNSDomain() (domain string, running bool) { + domain = nameserver.DefaultDomain + running = proxy.WithDNS + weaveContainer, err := proxy.client.InspectContainer("weave") + if err != nil || + weaveContainer.NetworkSettings == nil || + weaveContainer.NetworkSettings.IPAddress == "" { + return + } + + url := fmt.Sprintf("http://%s:%d/domain", weaveContainer.NetworkSettings.IPAddress, router.HTTPPort) + resp, err := http.Get(url) + if err != nil || resp.StatusCode != http.StatusOK { + return + } + defer resp.Body.Close() + + b, err := ioutil.ReadAll(resp.Body) + if err != nil { + return + } + + return string(b), true +} diff --git a/proxy/start_container_interceptor.go b/proxy/start_container_interceptor.go index 6d549cf2..3dbaf0f0 100644 --- a/proxy/start_container_interceptor.go +++ b/proxy/start_container_interceptor.go @@ -2,16 +2,41 @@ package proxy import ( "net/http" + + "github.com/fsouza/go-dockerclient" ) type startContainerInterceptor struct{ proxy *Proxy } +type startContainerRequestBody struct { + HostConfig *docker.HostConfig `json:"HostConfig,omitempty" yaml:"HostConfig,omitempty"` +} + func (i *startContainerInterceptor) InterceptRequest(r *http.Request) error { container, err := inspectContainerInPath(i.proxy.client, r.URL.Path) - if err == nil && containerShouldAttach(container) { - i.proxy.createWait(r, container.ID) + if err != nil || !containerShouldAttach(container) { + return err } - return err + + if r.Header.Get("Content-Type") != "application/json" { + i.proxy.createWait(r, container.ID) + return nil + } + + hostConfig := &docker.HostConfig{} + if err := unmarshalRequestBody(r, &hostConfig); err != nil { + return err + } + + i.proxy.addWeaveWaitVolume(hostConfig) + if dnsDomain, withDNS := i.proxy.getDNSDomain(); withDNS { + if err := i.proxy.setWeaveDNS(hostConfig, container.Config.Hostname, dnsDomain); err != nil { + return err + } + } + + i.proxy.createWait(r, container.ID) + return marshalRequestBody(r, hostConfig) } func (i *startContainerInterceptor) InterceptResponse(r *http.Response) error { diff --git a/test/666_abuse_of_start_test.sh b/test/666_abuse_of_start_test.sh index a0073532..e892a687 100755 --- a/test/666_abuse_of_start_test.sh +++ b/test/666_abuse_of_start_test.sh @@ -9,7 +9,13 @@ weave_on $HOST1 launch proxy_start_container $HOST1 --name=c1 proxy docker_on $HOST1 create --name=c2 $SMALL_IMAGE grep "^1$" /sys/class/net/ethwe/carrier # Now start c2 with a sneaky HostConfig -curl -X POST -H Content-Type:application/json -d '{"HostConfig": {"NetworkMode": "container:c1"}}' http://$HOST1:12375/containers/c2/start -assert "docker_on $HOST1 inspect -f '{{.State.Running}} {{.State.ExitCode}}' c2" "false 0" +curl -X POST -H Content-Type:application/json -d '{"NetworkMode": "container:c1"}' http://$HOST1:12375/v1.20/containers/c2/start +docker_bridge_ip=$(weave_on $HOST1 docker-bridge-ip) +assert "docker_on $HOST1 inspect -f '{{.State.Running}} {{.State.ExitCode}} {{.HostConfig.Dns}}' c2" "false 0 [$docker_bridge_ip]" + +# Now start c3 with a mostly null HostConfig +proxy docker_on $HOST1 create --name=c3 -v /tmp:/hosttmp $SMALL_IMAGE grep "^1$" /sys/class/net/ethwe/carrier +curl -X POST -H Content-Type:application/json -d '{"Binds":[],"ContainerIDFile":"","LxcConf":null,"Memory":0,"MemorySwap":0,"CpuShares":0,"CpuPeriod":0,"CpusetCpus":"","CpusetMems":"","CpuQuota":0,"BlkioWeight":0,"OomKillDisable":false,"Privileged":false,"PortBindings":null,"Links":[],"PublishAllPorts":false,"Dns":null,"DnsSearch":null,"ExtraHosts":null,"VolumesFrom":null,"Devices":null,"NetworkMode":"","IpcMode":"","PidMode":"","UTSMode":"","CapAdd":null,"CapDrop":null,"RestartPolicy":{"Name":"","MaximumRetryCount":0},"SecurityOpt":null,"ReadonlyRootfs":false,"Ulimits":null,"LogConfig":{"type":"","config":null},"CgroupParent":""}' http://$HOST1:12375/v1.20/containers/c3/start +assert "docker_on $HOST1 inspect -f '{{.State.Running}} {{.State.ExitCode}} {{.HostConfig.Dns}} {{index .HostConfig.Binds 0}}' c3" "false 0 [$docker_bridge_ip] /tmp:/hosttmp" end_suite From 3e4d8a3ffe21a84cc26e7d2b8f2a49ecbdc537f3 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Tue, 22 Sep 2015 10:06:17 +0100 Subject: [PATCH 3/4] review feedback --- proxy/common.go | 5 ----- proxy/proxy.go | 4 ++++ proxy/start_container_interceptor.go | 9 ++++++--- test/666_abuse_of_start_test.sh | 18 +++++++++--------- test/config.sh | 10 ++++++++++ 5 files changed, 29 insertions(+), 17 deletions(-) diff --git a/proxy/common.go b/proxy/common.go index 0ea7ebe6..4c0f3fc3 100644 --- a/proxy/common.go +++ b/proxy/common.go @@ -50,10 +50,6 @@ func unmarshalRequestBody(r *http.Request, target interface{}) error { } r.Body = ioutil.NopCloser(bytes.NewReader(body)) - if len(body) == 0 { - return nil - } - d := json.NewDecoder(bytes.NewReader(body)) d.UseNumber() // don't want large numbers in scientific format return d.Decode(&target) @@ -66,7 +62,6 @@ func marshalRequestBody(r *http.Request, body interface{}) error { } r.Body = ioutil.NopCloser(bytes.NewReader(newBody)) r.ContentLength = int64(len(newBody)) - r.Header.Set("Content-Type", "application/json") return nil } diff --git a/proxy/proxy.go b/proxy/proxy.go index 789a6232..89c4a59d 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -396,6 +396,10 @@ func (proxy *Proxy) setWeaveDNS(hostConfig *docker.HostConfig, hostname, dnsDoma } func (proxy *Proxy) getDNSDomain() (domain string, running bool) { + if proxy.WithoutDNS { + return "", false + } + domain = nameserver.DefaultDomain running = proxy.WithDNS weaveContainer, err := proxy.client.InspectContainer("weave") diff --git a/proxy/start_container_interceptor.go b/proxy/start_container_interceptor.go index 3dbaf0f0..56a482e2 100644 --- a/proxy/start_container_interceptor.go +++ b/proxy/start_container_interceptor.go @@ -14,11 +14,11 @@ type startContainerRequestBody struct { func (i *startContainerInterceptor) InterceptRequest(r *http.Request) error { container, err := inspectContainerInPath(i.proxy.client, r.URL.Path) - if err != nil || !containerShouldAttach(container) { + if err != nil { return err } - if r.Header.Get("Content-Type") != "application/json" { + if !containerShouldAttach(container) || r.Header.Get("Content-Type") != "application/json" { i.proxy.createWait(r, container.ID) return nil } @@ -35,8 +35,11 @@ func (i *startContainerInterceptor) InterceptRequest(r *http.Request) error { } } + if err := marshalRequestBody(r, hostConfig); err != nil { + return err + } i.proxy.createWait(r, container.ID) - return marshalRequestBody(r, hostConfig) + return nil } func (i *startContainerInterceptor) InterceptResponse(r *http.Response) error { diff --git a/test/666_abuse_of_start_test.sh b/test/666_abuse_of_start_test.sh index e892a687..4689208a 100755 --- a/test/666_abuse_of_start_test.sh +++ b/test/666_abuse_of_start_test.sh @@ -5,17 +5,17 @@ start_suite "Abuse of 'start' operation" weave_on $HOST1 launch - -proxy_start_container $HOST1 --name=c1 -proxy docker_on $HOST1 create --name=c2 $SMALL_IMAGE grep "^1$" /sys/class/net/ethwe/carrier -# Now start c2 with a sneaky HostConfig -curl -X POST -H Content-Type:application/json -d '{"NetworkMode": "container:c1"}' http://$HOST1:12375/v1.20/containers/c2/start docker_bridge_ip=$(weave_on $HOST1 docker-bridge-ip) +proxy_start_container $HOST1 --name=c1 + +# Start c2 with a sneaky HostConfig +proxy docker_on $HOST1 create --name=c2 $SMALL_IMAGE $CHECK_ETHWE_UP +proxy docker_api_on $HOST1 POST /containers/c2/start '{"NetworkMode": "container:c1"}' assert "docker_on $HOST1 inspect -f '{{.State.Running}} {{.State.ExitCode}} {{.HostConfig.Dns}}' c2" "false 0 [$docker_bridge_ip]" -# Now start c3 with a mostly null HostConfig -proxy docker_on $HOST1 create --name=c3 -v /tmp:/hosttmp $SMALL_IMAGE grep "^1$" /sys/class/net/ethwe/carrier -curl -X POST -H Content-Type:application/json -d '{"Binds":[],"ContainerIDFile":"","LxcConf":null,"Memory":0,"MemorySwap":0,"CpuShares":0,"CpuPeriod":0,"CpusetCpus":"","CpusetMems":"","CpuQuota":0,"BlkioWeight":0,"OomKillDisable":false,"Privileged":false,"PortBindings":null,"Links":[],"PublishAllPorts":false,"Dns":null,"DnsSearch":null,"ExtraHosts":null,"VolumesFrom":null,"Devices":null,"NetworkMode":"","IpcMode":"","PidMode":"","UTSMode":"","CapAdd":null,"CapDrop":null,"RestartPolicy":{"Name":"","MaximumRetryCount":0},"SecurityOpt":null,"ReadonlyRootfs":false,"Ulimits":null,"LogConfig":{"type":"","config":null},"CgroupParent":""}' http://$HOST1:12375/v1.20/containers/c3/start -assert "docker_on $HOST1 inspect -f '{{.State.Running}} {{.State.ExitCode}} {{.HostConfig.Dns}} {{index .HostConfig.Binds 0}}' c3" "false 0 [$docker_bridge_ip] /tmp:/hosttmp" +# Start c3 with HostConfig having empty binds and null dns/networking settings +proxy docker_on $HOST1 create --name=c3 -v /tmp:/hosttmp $SMALL_IMAGE $CHECK_ETHWE_UP +proxy docker_api_on $HOST1 POST /containers/c3/start '{"Binds":[],"Dns":null,"DnsSearch":null,"ExtraHosts":null,"VolumesFrom":null,"Devices":null,"NetworkMode":""}' +assert "docker_on $HOST1 inspect -f '{{.State.Running}} {{.State.ExitCode}} {{.HostConfig.Dns}}' c3" "false 0 [$docker_bridge_ip]" end_suite diff --git a/test/config.sh b/test/config.sh index 1d9e6b49..ac9b2453 100644 --- a/test/config.sh +++ b/test/config.sh @@ -99,6 +99,16 @@ docker_on() { docker -H tcp://$host:$DOCKER_PORT "$@" } +docker_api_on() { + host=$1 + method=$2 + url=$3 + data=$4 + shift 4 + [ -z "$DEBUG" ] || greyly echo "Docker (API) on $host:$DOCKER_PORT: $method $url" >&2 + echo -n "$data" | curl -s -X "$method" -H Content-Type:application/json "http://$host:$DOCKER_PORT/v1.15$url" -d @- +} + proxy() { DOCKER_PORT=12375 "$@" } From 547c9e7f5a2010bdc4e6384d01c82a57902b8680 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Tue, 22 Sep 2015 13:34:25 +0100 Subject: [PATCH 4/4] Removing confusing variable from getDNSDomain, just return "" when disabled --- proxy/create_container_interceptor.go | 2 +- proxy/proxy.go | 12 +++++++----- proxy/start_container_interceptor.go | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/proxy/create_container_interceptor.go b/proxy/create_container_interceptor.go index 70e0198f..d40efaff 100644 --- a/proxy/create_container_interceptor.go +++ b/proxy/create_container_interceptor.go @@ -61,7 +61,7 @@ func (i *createContainerInterceptor) InterceptRequest(r *http.Request) error { } } hostname = i.proxy.hostnameMatchRegexp.ReplaceAllString(hostname, i.proxy.HostnameReplacement) - if dnsDomain, withDNS := i.proxy.getDNSDomain(); withDNS { + if dnsDomain := i.proxy.getDNSDomain(); dnsDomain != "" { i.setHostname(&container, hostname, dnsDomain) if err := i.proxy.setWeaveDNS(container.HostConfig, container.Hostname, dnsDomain); err != nil { return err diff --git a/proxy/proxy.go b/proxy/proxy.go index 89c4a59d..e6ca9599 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -395,13 +395,15 @@ func (proxy *Proxy) setWeaveDNS(hostConfig *docker.HostConfig, hostname, dnsDoma return nil } -func (proxy *Proxy) getDNSDomain() (domain string, running bool) { +func (proxy *Proxy) getDNSDomain() (domain string) { if proxy.WithoutDNS { - return "", false + return "" + } + + if proxy.WithDNS { + domain = nameserver.DefaultDomain } - domain = nameserver.DefaultDomain - running = proxy.WithDNS weaveContainer, err := proxy.client.InspectContainer("weave") if err != nil || weaveContainer.NetworkSettings == nil || @@ -421,5 +423,5 @@ func (proxy *Proxy) getDNSDomain() (domain string, running bool) { return } - return string(b), true + return string(b) } diff --git a/proxy/start_container_interceptor.go b/proxy/start_container_interceptor.go index 56a482e2..019ac8cd 100644 --- a/proxy/start_container_interceptor.go +++ b/proxy/start_container_interceptor.go @@ -29,7 +29,7 @@ func (i *startContainerInterceptor) InterceptRequest(r *http.Request) error { } i.proxy.addWeaveWaitVolume(hostConfig) - if dnsDomain, withDNS := i.proxy.getDNSDomain(); withDNS { + if dnsDomain := i.proxy.getDNSDomain(); dnsDomain != "" { if err := i.proxy.setWeaveDNS(hostConfig, container.Config.Hostname, dnsDomain); err != nil { return err }