From 94eb6513c1d66773b76009029062f67ef4ba240b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 16 Aug 2017 14:29:53 +0200 Subject: [PATCH 1/6] Add unit test for swarm labels on containers Signed-off-by: Sebastiaan van Stijn Upstream-commit: 6f8d17dad3cf29c2ff2d019204c68227ebadfcc8 Component: engine --- .../executor/container/container_test.go | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/components/engine/daemon/cluster/executor/container/container_test.go b/components/engine/daemon/cluster/executor/container/container_test.go index 1bf6f6cf02..5f967c2f77 100644 --- a/components/engine/daemon/cluster/executor/container/container_test.go +++ b/components/engine/daemon/cluster/executor/container/container_test.go @@ -35,3 +35,48 @@ func TestIsolationConversion(t *testing.T) { }) } } + +func TestContainerLabels(t *testing.T) { + c := &containerConfig{ + task: &swarmapi.Task{ + ID: "real-task.id", + Spec: swarmapi.TaskSpec{ + Runtime: &swarmapi.TaskSpec_Container{ + Container: &swarmapi.ContainerSpec{ + Labels: map[string]string{ + "com.docker.swarm.task": "user-specified-task", + "com.docker.swarm.task.id": "user-specified-task.id", + "com.docker.swarm.task.name": "user-specified-task.name", + "com.docker.swarm.node.id": "user-specified-node.id", + "com.docker.swarm.service.id": "user-specified-service.id", + "com.docker.swarm.service.name": "user-specified-service.name", + "this-is-a-user-label": "this is a user label's value", + }, + }, + }, + }, + ServiceID: "real-service.id", + Slot: 123, + NodeID: "real-node.id", + Annotations: swarmapi.Annotations{ + Name: "real-service.name.123.real-task.id", + }, + ServiceAnnotations: swarmapi.Annotations{ + Name: "real-service.name", + }, + }, + } + + expected := map[string]string{ + "com.docker.swarm.task": "", + "com.docker.swarm.task.id": "real-task.id", + "com.docker.swarm.task.name": "real-service.name.123.real-task.id", + "com.docker.swarm.node.id": "real-node.id", + "com.docker.swarm.service.id": "real-service.id", + "com.docker.swarm.service.name": "real-service.name", + "this-is-a-user-label": "this is a user label's value", + } + + labels := c.labels() + assert.DeepEqual(t, expected, labels) +} From 7469deec1a5dadb6a63ab0dbadfc490753b8de7d Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 21 Aug 2018 14:06:06 +0200 Subject: [PATCH 2/6] Add warning if REST API is accessible through an insecure connection The remote API allows full privilege escalation and is equivalent to having root access on the host. Because of this, the API should never be accessible through an insecure connection (TCP without TLS, or TCP without TLS verification). Although a warning is already logged on startup if the daemon uses an insecure configuration, this warning is not very visible (unless someone decides to read the logs). This patch attempts to make insecure configuration more visible by sending back warnings through the API (which will be printed when using `docker info`). Signed-off-by: Sebastiaan van Stijn Upstream-commit: 547b993e07330f3e74cba935975fce05e8661381 Component: engine --- components/engine/daemon/info.go | 27 +++++++++++++++++++ .../engine/integration/system/info_test.go | 24 +++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/components/engine/daemon/info.go b/components/engine/daemon/info.go index cc9ad8ac61..9dcfb95f03 100644 --- a/components/engine/daemon/info.go +++ b/components/engine/daemon/info.go @@ -68,6 +68,7 @@ func (daemon *Daemon) SystemInfo() (*types.Info, error) { Isolation: daemon.defaultIsolation, } + daemon.fillAPIInfo(v) // Retrieve platform specific info daemon.fillPlatformInfo(v, sysInfo) daemon.fillDriverInfo(v) @@ -171,6 +172,32 @@ func (daemon *Daemon) fillSecurityOptions(v *types.Info, sysInfo *sysinfo.SysInf v.SecurityOptions = securityOptions } +func (daemon *Daemon) fillAPIInfo(v *types.Info) { + const warn string = ` + Access to the remote API is equivalent to root access on the host. Refer + to the 'Docker daemon attack surface' section in the documentation for + more information: https://docs.docker.com/engine/security/security/#docker-daemon-attack-surface` + + cfg := daemon.configStore + for _, host := range cfg.Hosts { + // cnf.Hosts is normalized during startup, so should always have a scheme/proto + h := strings.SplitN(host, "://", 2) + proto := h[0] + addr := h[1] + if proto != "tcp" { + continue + } + if !cfg.TLS { + v.Warnings = append(v.Warnings, fmt.Sprintf("WARNING: API is accessible on http://%s without encryption.%s", addr, warn)) + continue + } + if !cfg.TLSVerify { + v.Warnings = append(v.Warnings, fmt.Sprintf("WARNING: API is accessible on https://%s without TLS client verification.%s", addr, warn)) + continue + } + } +} + func hostName() string { hostname := "" if hn, err := os.Hostname(); err != nil { diff --git a/components/engine/integration/system/info_test.go b/components/engine/integration/system/info_test.go index 2a05dfbb74..b8bdcf0049 100644 --- a/components/engine/integration/system/info_test.go +++ b/components/engine/integration/system/info_test.go @@ -5,6 +5,7 @@ import ( "fmt" "testing" + "github.com/docker/docker/internal/test/daemon" "github.com/docker/docker/internal/test/request" "gotest.tools/assert" is "gotest.tools/assert/cmp" @@ -40,3 +41,26 @@ func TestInfoAPI(t *testing.T) { assert.Check(t, is.Contains(out, linePrefix)) } } + +func TestInfoAPIWarnings(t *testing.T) { + d := daemon.New(t) + + client, err := d.NewClient() + assert.NilError(t, err) + + d.StartWithBusybox(t, "--iptables=false", "-H=0.0.0.0:23756", "-H=unix://"+d.Sock()) + defer d.Stop(t) + + info, err := client.Info(context.Background()) + assert.NilError(t, err) + + stringsToCheck := []string{ + "Access to the remote API is equivalent to root access", + "http://0.0.0.0:23756", + } + + out := fmt.Sprintf("%+v", info) + for _, linePrefix := range stringsToCheck { + assert.Check(t, is.Contains(out, linePrefix)) + } +} From cea4607c21c765cad83a5f9f261ef0742261b264 Mon Sep 17 00:00:00 2001 From: Andrew Hsu Date: Tue, 21 Aug 2018 22:19:45 +0000 Subject: [PATCH 3/6] remove experimental guard for buildkit Signed-off-by: Andrew Hsu Upstream-commit: 239047c2d36706f2826b0a9bc115e0a08b1c3d27 Component: engine --- components/engine/api/server/router/build/build_routes.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/components/engine/api/server/router/build/build_routes.go b/components/engine/api/server/router/build/build_routes.go index 62bcf3f13d..acacfac2ed 100644 --- a/components/engine/api/server/router/build/build_routes.go +++ b/components/engine/api/server/router/build/build_routes.go @@ -231,8 +231,7 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * } // check if the builder feature has been enabled from daemon as well. - if buildOptions.Version == types.BuilderBuildKit && - (br.builderVersion != types.BuilderBuildKit || !br.daemon.HasExperimental()) { + if buildOptions.Version == types.BuilderBuildKit && br.builderVersion != types.BuilderBuildKit { return errdefs.InvalidParameter(errors.New("buildkit is not enabled on daemon")) } From 2594f77b01f738be9ebb1c78c11e57ff46ee74fc Mon Sep 17 00:00:00 2001 From: Andrew Hsu Date: Tue, 21 Aug 2018 22:43:34 +0000 Subject: [PATCH 4/6] move /session api endpoint out of experimental Signed-off-by: Andrew Hsu Upstream-commit: 01c9e7082eba71cbe60ce2e47acb9aad2c83c7ef Component: engine --- components/engine/api/server/router/session/session.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/engine/api/server/router/session/session.go b/components/engine/api/server/router/session/session.go index de6d63008a..79ddc134e7 100644 --- a/components/engine/api/server/router/session/session.go +++ b/components/engine/api/server/router/session/session.go @@ -24,6 +24,6 @@ func (r *sessionRouter) Routes() []router.Route { func (r *sessionRouter) initRoutes() { r.routes = []router.Route{ - router.Experimental(router.NewPostRoute("/session", r.startSession)), + router.NewPostRoute("/session", r.startSession), } } From ed97b30d09757fa5beed3f3ffab2feb367344e16 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Tue, 21 Aug 2018 23:20:19 +0000 Subject: [PATCH 5/6] Fix logic when enabling buildkit Signed-off-by: Tibor Vass Upstream-commit: c973cde7606dc7a2557094fc90d8e6bb595fa354 Component: engine --- components/engine/api/server/router/build/build_routes.go | 2 +- components/engine/daemon/config/config.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/components/engine/api/server/router/build/build_routes.go b/components/engine/api/server/router/build/build_routes.go index acacfac2ed..c2a15c0ad3 100644 --- a/components/engine/api/server/router/build/build_routes.go +++ b/components/engine/api/server/router/build/build_routes.go @@ -231,7 +231,7 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * } // check if the builder feature has been enabled from daemon as well. - if buildOptions.Version == types.BuilderBuildKit && br.builderVersion != types.BuilderBuildKit { + if buildOptions.Version == types.BuilderBuildKit && br.builderVersion != "" && br.builderVersion != types.BuilderBuildKit { return errdefs.InvalidParameter(errors.New("buildkit is not enabled on daemon")) } diff --git a/components/engine/daemon/config/config.go b/components/engine/daemon/config/config.go index 6081b5c8ed..451078d8c5 100644 --- a/components/engine/daemon/config/config.go +++ b/components/engine/daemon/config/config.go @@ -54,6 +54,7 @@ var flatOptions = map[string]bool{ "log-opts": true, "runtimes": true, "default-ulimits": true, + "features": true, } // skipValidateOptions contains configuration keys From ebad0ccbe4573dc8d4c2d9015bc4d1bed927b343 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Wed, 22 Aug 2018 03:00:02 +0000 Subject: [PATCH 6/6] builder: temporarily disable bridge networking when using buildkit Signed-off-by: Tibor Vass Upstream-commit: 16084ea8c82fe3b3a0aae2573def0d1857365408 Component: engine --- components/engine/builder/builder-next/executor_unix.go | 9 +++------ components/engine/vendor.conf | 2 +- .../moby/buildkit/executor/runcexecutor/executor.go | 6 ------ .../engine/vendor/github.com/moby/buildkit/vendor.conf | 2 +- 4 files changed, 5 insertions(+), 14 deletions(-) diff --git a/components/engine/builder/builder-next/executor_unix.go b/components/engine/builder/builder-next/executor_unix.go index 8ca1f85de0..44f2dfcd96 100644 --- a/components/engine/builder/builder-next/executor_unix.go +++ b/components/engine/builder/builder-next/executor_unix.go @@ -18,16 +18,13 @@ import ( const networkName = "bridge" -func init() { - // FIXME: https://github.com/moby/moby/issues/37676 - runcexecutor.DisableSubReaper() -} - func newExecutor(root string, net libnetwork.NetworkController) (executor.Executor, error) { + // FIXME: fix bridge networking + _ = bridgeProvider{} return runcexecutor.New(runcexecutor.Opt{ Root: filepath.Join(root, "executor"), CommandCandidates: []string{"docker-runc", "runc"}, - }, &bridgeProvider{NetworkController: net}) + }, nil) } type bridgeProvider struct { diff --git a/components/engine/vendor.conf b/components/engine/vendor.conf index b13e022123..4269c9f438 100644 --- a/components/engine/vendor.conf +++ b/components/engine/vendor.conf @@ -26,7 +26,7 @@ github.com/imdario/mergo v0.3.6 golang.org/x/sync 1d60e4601c6fd243af51cc01ddf169918a5407ca # buildkit -github.com/moby/buildkit 46f9075ab68a07df2c40ae6e240ce4f9392b3a66 git://github.com/tiborvass/buildkit.git +github.com/moby/buildkit 49906c62925ed429ec9174a0b6869982967f1a39 github.com/tonistiigi/fsutil b19464cd1b6a00773b4f2eb7acf9c30426f9df42 github.com/grpc-ecosystem/grpc-opentracing 8e809c8a86450a29b90dcc9efbf062d0fe6d9746 github.com/opentracing/opentracing-go 1361b9cd60be79c4c3a7fa9841b3c132e40066a7 diff --git a/components/engine/vendor/github.com/moby/buildkit/executor/runcexecutor/executor.go b/components/engine/vendor/github.com/moby/buildkit/executor/runcexecutor/executor.go index 3d19d4e375..2874314198 100644 --- a/components/engine/vendor/github.com/moby/buildkit/executor/runcexecutor/executor.go +++ b/components/engine/vendor/github.com/moby/buildkit/executor/runcexecutor/executor.go @@ -404,12 +404,6 @@ func (s *forwardIO) writeCloserToFile(wc io.WriteCloser) (*os.File, error) { var subReaperOnce sync.Once var subReaperError error -// DisableSubReaper prevents setting subreaper on the current process. -// Do not rely on this function it may change or be removed. -func DisableSubReaper() { - subReaperOnce.Do(func() {}) -} - func setSubReaper() error { subReaperOnce.Do(func() { subReaperError = runcsystem.SetSubreaper(1) diff --git a/components/engine/vendor/github.com/moby/buildkit/vendor.conf b/components/engine/vendor/github.com/moby/buildkit/vendor.conf index a08531dbff..7195f48073 100644 --- a/components/engine/vendor/github.com/moby/buildkit/vendor.conf +++ b/components/engine/vendor/github.com/moby/buildkit/vendor.conf @@ -14,7 +14,7 @@ google.golang.org/grpc v1.12.0 github.com/opencontainers/go-digest c9281466c8b2f606084ac71339773efd177436e7 golang.org/x/net 0ed95abb35c445290478a5348a7b38bb154135fd github.com/gogo/protobuf v1.0.0 -github.com/gogo/googleapis 08a7655d27152912db7aaf4f983275eaf8d128ef +github.com/gogo/googleapis b23578765ee54ff6bceff57f397d833bf4ca6869 github.com/golang/protobuf v1.1.0 github.com/containerd/continuity d3c23511c1bf5851696cba83143d9cbcd666869b github.com/opencontainers/image-spec v1.0.1