From 6bad7e61805202043aebf8a7f6da8cce189843a1 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 31 Jan 2018 13:28:09 -0800 Subject: [PATCH 1/6] Test for systemd cgroupdriver memory setting This is a test case for issue https://github.com/moby/moby/issues/35123, making sure we can set container's memory limit when using `native.cgroupdriver=systemd`. [v2: skip if no systemd present] [v3: add --iptables=false to avoid flaky tests with t.Parallel()] [v4: rebase after PR#36507 merge] Signed-off-by: Kir Kolyshkin Upstream-commit: 4ca5c5361059e29ed31074ca5b96f8b2030b5f99 Component: engine --- .../system/cgroupdriver_systemd_test.go | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 components/engine/integration/system/cgroupdriver_systemd_test.go diff --git a/components/engine/integration/system/cgroupdriver_systemd_test.go b/components/engine/integration/system/cgroupdriver_systemd_test.go new file mode 100644 index 0000000000..c1f864a1c7 --- /dev/null +++ b/components/engine/integration/system/cgroupdriver_systemd_test.go @@ -0,0 +1,64 @@ +package system + +import ( + "context" + "os" + "testing" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/integration-cli/daemon" + + "github.com/gotestyourself/gotestyourself/assert" +) + +// hasSystemd checks whether the host was booted with systemd as its init +// system. Stolen from +// https://github.com/coreos/go-systemd/blob/176f85496f4e/util/util.go#L68 +func hasSystemd() bool { + fi, err := os.Lstat("/run/systemd/system") + if err != nil { + return false + } + return fi.IsDir() +} + +// TestCgroupDriverSystemdMemoryLimit checks that container +// memory limit can be set when using systemd cgroupdriver. +// https://github.com/moby/moby/issues/35123 +func TestCgroupDriverSystemdMemoryLimit(t *testing.T) { + t.Parallel() + + if !hasSystemd() { + t.Skip("systemd not available") + } + + d := daemon.New(t, "docker", "dockerd", daemon.Config{}) + client, err := d.NewClient() + assert.NilError(t, err) + d.StartWithBusybox(t, "--exec-opt", "native.cgroupdriver=systemd", "--iptables=false") + defer d.Stop(t) + + const mem = 64 * 1024 * 1024 // 64 MB + cfg := container.Config{ + Image: "busybox", + Cmd: []string{"top"}, + } + hostcfg := container.HostConfig{ + Resources: container.Resources{ + Memory: mem, + }, + } + + ctx := context.Background() + ctr, err := client.ContainerCreate(ctx, &cfg, &hostcfg, nil, "") + assert.NilError(t, err) + defer client.ContainerRemove(ctx, ctr.ID, types.ContainerRemoveOptions{Force: true}) + + err = client.ContainerStart(ctx, ctr.ID, types.ContainerStartOptions{}) + assert.NilError(t, err) + + s, err := client.ContainerInspect(ctx, ctr.ID) + assert.NilError(t, err) + assert.Equal(t, s.HostConfig.Memory, mem) +} From 3d783d5dbf9b3074bda56496304450a818cc9fc9 Mon Sep 17 00:00:00 2001 From: Justin Cormack Date: Wed, 14 Mar 2018 09:55:54 +0000 Subject: [PATCH 2/6] If container will run as non root user, drop permitted, effective caps early As soon as the initial executable in the container is executed as a non root user, permitted and effective capabilities are dropped. Drop them earlier than this, so that they are dropped before executing the file. The main effect of this is that if `CAP_DAC_OVERRIDE` is set (the default) the user will not be able to execute files they do not have permission to execute, which previously they could. The old behaviour was somewhat surprising and the new one is definitely correct, but it is not in any meaningful way exploitable, and I do not think it is necessary to backport this fix. It is unlikely to have any negative effects as almost all executables have world execute permission anyway. Use the bounding set not the effective set as the canonical set of capabilities, as effective will now vary. Signed-off-by: Justin Cormack Upstream-commit: 15ff09395c001bcb0f284461abbc404a1d8bab4d Component: engine --- components/engine/daemon/oci_linux.go | 8 +++++++- components/engine/profiles/seccomp/seccomp.go | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/components/engine/daemon/oci_linux.go b/components/engine/daemon/oci_linux.go index 256e1c3e55..a83f155fda 100644 --- a/components/engine/daemon/oci_linux.go +++ b/components/engine/daemon/oci_linux.go @@ -255,7 +255,7 @@ func setCapabilities(s *specs.Spec, c *container.Container) error { if c.HostConfig.Privileged { caplist = caps.GetAllCapabilities() } else { - caplist, err = caps.TweakCapabilities(s.Process.Capabilities.Effective, c.HostConfig.CapAdd, c.HostConfig.CapDrop) + caplist, err = caps.TweakCapabilities(s.Process.Capabilities.Bounding, c.HostConfig.CapAdd, c.HostConfig.CapDrop) if err != nil { return err } @@ -264,6 +264,12 @@ func setCapabilities(s *specs.Spec, c *container.Container) error { s.Process.Capabilities.Bounding = caplist s.Process.Capabilities.Permitted = caplist s.Process.Capabilities.Inheritable = caplist + // setUser has already been executed here + // if non root drop capabilities in the way execve does + if s.Process.User.UID != 0 { + s.Process.Capabilities.Effective = []string{} + s.Process.Capabilities.Permitted = []string{} + } return nil } diff --git a/components/engine/profiles/seccomp/seccomp.go b/components/engine/profiles/seccomp/seccomp.go index 36ec76ae05..4438670a58 100644 --- a/components/engine/profiles/seccomp/seccomp.go +++ b/components/engine/profiles/seccomp/seccomp.go @@ -105,7 +105,7 @@ Loop: } if len(call.Excludes.Caps) > 0 { for _, c := range call.Excludes.Caps { - if inSlice(rs.Process.Capabilities.Effective, c) { + if inSlice(rs.Process.Capabilities.Bounding, c) { continue Loop } } @@ -117,7 +117,7 @@ Loop: } if len(call.Includes.Caps) > 0 { for _, c := range call.Includes.Caps { - if !inSlice(rs.Process.Capabilities.Effective, c) { + if !inSlice(rs.Process.Capabilities.Bounding, c) { continue Loop } } From 3d5d533860cd1599de5efc72e405d9a70fb61e90 Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Tue, 20 Mar 2018 17:09:18 -0400 Subject: [PATCH 3/6] container/nat integration tests use unique names for test containers Signed-off-by: Arash Deshmeh Upstream-commit: b4d1547af6b91baa2ffcb8a391c35d9bc5cdc48f Component: engine --- components/engine/integration/container/nat_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/engine/integration/container/nat_test.go b/components/engine/integration/container/nat_test.go index f4a3ea6f3a..5e7402770b 100644 --- a/components/engine/integration/container/nat_test.go +++ b/components/engine/integration/container/nat_test.go @@ -62,14 +62,14 @@ func TestNetworkLoopbackNat(t *testing.T) { defer setupTest(t)() msg := "it works" - startServerContainer(t, msg, 8080) + serverContainerID := startServerContainer(t, msg, 8080) endpoint := getExternalAddress(t) client := request.NewAPIClient(t) ctx := context.Background() - cID := container.Run(t, ctx, client, container.WithCmd("sh", "-c", fmt.Sprintf("stty raw && nc -w 5 %s 8080", endpoint.String())), container.WithTty(true), container.WithNetworkMode("container:server")) + cID := container.Run(t, ctx, client, container.WithCmd("sh", "-c", fmt.Sprintf("stty raw && nc -w 5 %s 8080", endpoint.String())), container.WithTty(true), container.WithNetworkMode("container:"+serverContainerID)) poll.WaitOn(t, container.IsStopped(ctx, client, cID), poll.WithDelay(100*time.Millisecond)) @@ -90,7 +90,7 @@ func startServerContainer(t *testing.T, msg string, port int) string { client := request.NewAPIClient(t) ctx := context.Background() - cID := container.Run(t, ctx, client, container.WithName("server"), container.WithCmd("sh", "-c", fmt.Sprintf("echo %q | nc -lp %d", msg, port)), container.WithExposedPorts(fmt.Sprintf("%d/tcp", port)), func(c *container.TestContainerConfig) { + cID := container.Run(t, ctx, client, container.WithName("server-"+t.Name()), container.WithCmd("sh", "-c", fmt.Sprintf("echo %q | nc -lp %d", msg, port)), container.WithExposedPorts(fmt.Sprintf("%d/tcp", port)), func(c *container.TestContainerConfig) { c.HostConfig.PortBindings = nat.PortMap{ nat.Port(fmt.Sprintf("%d/tcp", port)): []nat.PortBinding{ { From bc6cc53700d713e6c86b8f9c463eef6b31276b02 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 21 Mar 2018 12:57:53 +0100 Subject: [PATCH 4/6] Split daemon service code to _windows file This moves some of the code that was conditionally executed on Windows to a separate, windows-only file. Signed-off-by: Sebastiaan van Stijn Upstream-commit: cd3e84c6b38e74d03ab31db804bc9b49dcab8243 Component: engine --- components/engine/cmd/dockerd/daemon_unix.go | 4 -- components/engine/cmd/dockerd/docker.go | 44 ++----------------- components/engine/cmd/dockerd/docker_unix.go | 8 ++++ .../engine/cmd/dockerd/docker_windows.go | 33 ++++++++++++++ .../engine/cmd/dockerd/service_unsupported.go | 4 -- 5 files changed, 45 insertions(+), 48 deletions(-) create mode 100644 components/engine/cmd/dockerd/docker_unix.go diff --git a/components/engine/cmd/dockerd/daemon_unix.go b/components/engine/cmd/dockerd/daemon_unix.go index a65d8ed012..6ab2ada485 100644 --- a/components/engine/cmd/dockerd/daemon_unix.go +++ b/components/engine/cmd/dockerd/daemon_unix.go @@ -104,10 +104,6 @@ func allocateDaemonPort(addr string) error { return nil } -// notifyShutdown is called after the daemon shuts down but before the process exits. -func notifyShutdown(err error) { -} - func wrapListeners(proto string, ls []net.Listener) []net.Listener { switch proto { case "unix": diff --git a/components/engine/cmd/dockerd/docker.go b/components/engine/cmd/dockerd/docker.go index 2ccca46c18..e90a12e366 100644 --- a/components/engine/cmd/dockerd/docker.go +++ b/components/engine/cmd/dockerd/docker.go @@ -3,7 +3,6 @@ package main import ( "fmt" "os" - "path/filepath" "runtime" "github.com/docker/docker/cli" @@ -25,6 +24,10 @@ func newDaemonCommand() *cobra.Command { SilenceErrors: true, Args: cli.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { + if opts.version { + showVersion() + return nil + } opts.flags = cmd.Flags() return runDaemon(opts) }, @@ -41,45 +44,6 @@ func newDaemonCommand() *cobra.Command { return cmd } -func runDaemon(opts *daemonOptions) error { - if opts.version { - showVersion() - return nil - } - - daemonCli := NewDaemonCli() - - // Windows specific settings as these are not defaulted. - if runtime.GOOS == "windows" { - if opts.daemonConfig.Pidfile == "" { - opts.daemonConfig.Pidfile = filepath.Join(opts.daemonConfig.Root, "docker.pid") - } - if opts.configFile == "" { - opts.configFile = filepath.Join(opts.daemonConfig.Root, `config\daemon.json`) - } - } - - // On Windows, this may be launching as a service or with an option to - // register the service. - stop, runAsService, err := initService(daemonCli) - if err != nil { - logrus.Fatal(err) - } - - if stop { - return nil - } - - // If Windows SCM manages the service - no need for PID files - if runAsService { - opts.daemonConfig.Pidfile = "" - } - - err = daemonCli.start(opts) - notifyShutdown(err) - return err -} - func showVersion() { fmt.Printf("Docker version %s, build %s\n", dockerversion.Version, dockerversion.GitCommit) } diff --git a/components/engine/cmd/dockerd/docker_unix.go b/components/engine/cmd/dockerd/docker_unix.go new file mode 100644 index 0000000000..0dec48663d --- /dev/null +++ b/components/engine/cmd/dockerd/docker_unix.go @@ -0,0 +1,8 @@ +// +build !windows + +package main + +func runDaemon(opts *daemonOptions) error { + daemonCli := NewDaemonCli() + return daemonCli.start(opts) +} diff --git a/components/engine/cmd/dockerd/docker_windows.go b/components/engine/cmd/dockerd/docker_windows.go index 889e35272d..bd8bc5a58e 100644 --- a/components/engine/cmd/dockerd/docker_windows.go +++ b/components/engine/cmd/dockerd/docker_windows.go @@ -1,5 +1,38 @@ package main import ( + "path/filepath" + _ "github.com/docker/docker/autogen/winresources/dockerd" + "github.com/sirupsen/logrus" ) + +func runDaemon(opts *daemonOptions) error { + daemonCli := NewDaemonCli() + + // On Windows, this may be launching as a service or with an option to + // register the service. + stop, runAsService, err := initService(daemonCli) + if err != nil { + logrus.Fatal(err) + } + + if stop { + return nil + } + + // Windows specific settings as these are not defaulted. + if opts.configFile == "" { + opts.configFile = filepath.Join(opts.daemonConfig.Root, `config\daemon.json`) + } + if runAsService { + // If Windows SCM manages the service - no need for PID files + opts.daemonConfig.Pidfile = "" + } else if opts.daemonConfig.Pidfile == "" { + opts.daemonConfig.Pidfile = filepath.Join(opts.daemonConfig.Root, "docker.pid") + } + + err = daemonCli.start(opts) + notifyShutdown(err) + return err +} diff --git a/components/engine/cmd/dockerd/service_unsupported.go b/components/engine/cmd/dockerd/service_unsupported.go index e67ad474b1..bbcb7f3f3b 100644 --- a/components/engine/cmd/dockerd/service_unsupported.go +++ b/components/engine/cmd/dockerd/service_unsupported.go @@ -6,9 +6,5 @@ import ( "github.com/spf13/pflag" ) -func initService(daemonCli *DaemonCli) (bool, bool, error) { - return false, false, nil -} - func installServiceFlags(flags *pflag.FlagSet) { } From f95a3e7bb8d47e6a237aa5ce57850cca3d33407a Mon Sep 17 00:00:00 2001 From: selansen Date: Fri, 9 Mar 2018 17:03:59 -0500 Subject: [PATCH 5/6] Fix for Flaky test TestServiceWithPredefinedNetwork TestServiceWithPredefinedNetwork test case was failing at times. To fix the issue, added new API to check for services after we clean up all services. Tested multiple times and this sould fix flaky issue. Signed-off-by: selansen Upstream-commit: dabffd806c98ab13dbc25e57bee21c5291b9a50c Component: engine --- .../integration/network/service_test.go | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/components/engine/integration/network/service_test.go b/components/engine/integration/network/service_test.go index a9fccf9522..ea7391180a 100644 --- a/components/engine/integration/network/service_test.go +++ b/components/engine/integration/network/service_test.go @@ -6,7 +6,6 @@ import ( "time" "github.com/docker/docker/api/types" - "github.com/docker/docker/api/types/filters" swarmtypes "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/client" "github.com/docker/docker/integration/internal/swarm" @@ -51,10 +50,6 @@ func TestServiceWithPredefinedNetwork(t *testing.T) { err = client.ServiceRemove(context.Background(), serviceID) assert.NilError(t, err) - - poll.WaitOn(t, serviceIsRemoved(client, serviceID), pollSettings) - poll.WaitOn(t, noTasks(client), pollSettings) - } const ingressNet = "ingress" @@ -108,7 +103,7 @@ func TestServiceWithIngressNetwork(t *testing.T) { assert.NilError(t, err) poll.WaitOn(t, serviceIsRemoved(client, serviceID), pollSettings) - poll.WaitOn(t, noTasks(client), pollSettings) + poll.WaitOn(t, noServices(client), pollSettings) // Ensure that "ingress" is not removed or corrupted time.Sleep(10 * time.Second) @@ -125,8 +120,6 @@ func TestServiceWithIngressNetwork(t *testing.T) { func serviceRunningCount(client client.ServiceAPIClient, serviceID string, instances uint64) func(log poll.LogT) poll.Result { return func(log poll.LogT) poll.Result { - filter := filters.NewArgs() - filter.Add("service", serviceID) services, err := client.ServiceList(context.Background(), types.ServiceListOptions{}) if err != nil { return poll.Error(err) @@ -160,3 +153,17 @@ func swarmIngressReady(client client.NetworkAPIClient) func(log poll.LogT) poll. return poll.Success() } } + +func noServices(client client.ServiceAPIClient) func(log poll.LogT) poll.Result { + return func(log poll.LogT) poll.Result { + services, err := client.ServiceList(context.Background(), types.ServiceListOptions{}) + switch { + case err != nil: + return poll.Error(err) + case len(services) == 0: + return poll.Success() + default: + return poll.Continue("Service count at %d waiting for 0", len(services)) + } + } +} From 4ad54eacb8efaa7963fd3d9fb3ce689109fa97cf Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 21 Mar 2018 15:29:44 -0700 Subject: [PATCH 6/6] client: fix hijackedconn reading from buffer Signed-off-by: Tonis Tiigi Upstream-commit: f094a05e260d8748f0fd2018a8a908b4189e454d Component: engine --- components/engine/client/hijack.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/components/engine/client/hijack.go b/components/engine/client/hijack.go index 5a6354561b..dab3a75691 100644 --- a/components/engine/client/hijack.go +++ b/components/engine/client/hijack.go @@ -192,7 +192,7 @@ func (cli *Client) setupHijackConn(req *http.Request, proto string) (net.Conn, e // object that implements CloseWrite iff the underlying connection // implements it. if _, ok := c.(types.CloseWriter); ok { - c = &hijackedConnCloseWriter{c, br} + c = &hijackedConnCloseWriter{&hijackedConn{c, br}} } else { c = &hijackedConn{c, br} } @@ -220,7 +220,9 @@ func (c *hijackedConn) Read(b []byte) (int, error) { // CloseWrite(). It is returned by setupHijackConn in the case that a) there // was already buffered data in the http layer when Hijack() was called, and b) // the underlying net.Conn *does* implement CloseWrite(). -type hijackedConnCloseWriter hijackedConn +type hijackedConnCloseWriter struct { + *hijackedConn +} var _ types.CloseWriter = &hijackedConnCloseWriter{}