From 596ca142e01753bd8a20b01c61d528415759c45a Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 27 Nov 2017 16:10:44 -0800 Subject: [PATCH] daemon: use 'private' ipc mode by default This changes the default ipc mode of daemon/engine to be private, meaning the containers will not have their /dev/shm bind-mounted from the host by default. The benefits of doing this are: 1. No leaked mounts. Eliminate a possibility to leak mounts into other namespaces (and therefore unfortunate errors like "Unable to remove filesystem for : remove /var/lib/docker/containers//shm: device or resource busy"). 2. Working checkpoint/restore. Make `docker checkpoint` not lose the contents of `/dev/shm`, but save it to the dump, and be restored back upon `docker start --checkpoint` (currently it is lost -- while CRIU handles tmpfs mounts, the "shareable" mount is seen as external to container, and thus rightfully ignored). 3. Better security. Currently any container is opened to share its /dev/shm with any other container. Obviously, this change will break the following usage scenario: $ docker run -d --name donor busybox top $ docker run --rm -it --ipc container:donor busybox sh Error response from daemon: linux spec namespaces: can't join IPC of container : non-shareable IPC (hint: use IpcMode:shareable for the donor container) The soution, as hinted by the (amended) error message, is to explicitly enable donor sharing by using --ipc shareable: $ docker run -d --name donor --ipc shareable busybox top Compatibility notes: 1. This only applies to containers created _after_ this change. Existing containers are not affected and will work fine as their ipc mode is stored in HostConfig. 2. Old backward compatible behavior ("shareable" containers by default) can be enabled by either using `--default-ipc-mode shareable` daemon command line option, or by adding a `"default-ipc-mode": "shareable"` line in `/etc/docker/daemon.json` configuration file. 3. If an older client (API < 1.40) is used, a "shareable" container is created. A test to check that is added. Signed-off-by: Kir Kolyshkin --- .../router/container/container_routes.go | 5 ++++ daemon/config/config_unix.go | 2 +- daemon/container_operations_unix.go | 2 +- docs/api/version-history.md | 3 ++ integration/container/ipcmode_linux_test.go | 28 +++++++++++++++++++ 5 files changed, 38 insertions(+), 2 deletions(-) diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go index 58cd43af45..87292ffb62 100644 --- a/api/server/router/container/container_routes.go +++ b/api/server/router/container/container_routes.go @@ -477,6 +477,11 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo // Ignore Capabilities because it was added in API 1.40. hostConfig.Capabilities = nil + + // Older clients (API < 1.40) expects the default to be shareable, make them happy + if hostConfig.IpcMode.IsEmpty() { + hostConfig.IpcMode = container.IpcMode("shareable") + } } ccr, err := s.backend.ContainerCreate(types.ContainerCreateConfig{ diff --git a/daemon/config/config_unix.go b/daemon/config/config_unix.go index 0ba7e754fb..10743d5040 100644 --- a/daemon/config/config_unix.go +++ b/daemon/config/config_unix.go @@ -12,7 +12,7 @@ import ( const ( // DefaultIpcMode is default for container's IpcMode, if not set otherwise - DefaultIpcMode = "shareable" // TODO: change to private + DefaultIpcMode = "private" ) // Config defines the configuration of a docker daemon. diff --git a/daemon/container_operations_unix.go b/daemon/container_operations_unix.go index c7984f7a07..0d74158bb2 100644 --- a/daemon/container_operations_unix.go +++ b/daemon/container_operations_unix.go @@ -72,7 +72,7 @@ func (daemon *Daemon) getIpcContainer(id string) (*container.Container, error) { // Check the container ipc is shareable if st, err := os.Stat(container.ShmPath); err != nil || !st.IsDir() { if err == nil || os.IsNotExist(err) { - return nil, errors.New(errMsg + ": non-shareable IPC") + return nil, errors.New(errMsg + ": non-shareable IPC (hint: use IpcMode:shareable for the donor container)") } // stat() failed? return nil, errors.Wrap(err, errMsg+": unexpected error from stat "+container.ShmPath) diff --git a/docs/api/version-history.md b/docs/api/version-history.md index ccb36d16a8..578f395025 100644 --- a/docs/api/version-history.md +++ b/docs/api/version-history.md @@ -61,6 +61,9 @@ keywords: "API, Docker, rcli, REST, documentation" * `POST /containers/create` now takes a `Capabilities` field to set the list of kernel capabilities to be available for the container (this overrides the default set). +* `POST /containers/create` on Linux now creates a container with `HostConfig.IpcMode=private` + by default, if IpcMode is not explicitly specified. The per-daemon default can be changed + back to `shareable` by using `DefaultIpcMode` daemon configuration parameter. * `POST /containers/{id}/update` now accepts a `PidsLimit` field to tune a container's PID limit. Set `0` or `-1` for unlimited. Leave `null` to not change the current value. diff --git a/integration/container/ipcmode_linux_test.go b/integration/container/ipcmode_linux_test.go index 49634ca732..b4a9a15a18 100644 --- a/integration/container/ipcmode_linux_test.go +++ b/integration/container/ipcmode_linux_test.go @@ -11,8 +11,11 @@ import ( "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/versions" + "github.com/docker/docker/client" "github.com/docker/docker/integration/internal/container" "github.com/docker/docker/internal/test/daemon" + "github.com/docker/docker/internal/test/request" "gotest.tools/assert" is "gotest.tools/assert/cmp" "gotest.tools/fs" @@ -292,3 +295,28 @@ func TestDaemonIpcModeShareableFromConfig(t *testing.T) { testDaemonIpcFromConfig(t, "shareable", true) } + +// TestIpcModeOlderClient checks that older client gets shareable IPC mode +// by default, even when the daemon default is private. +func TestIpcModeOlderClient(t *testing.T) { + skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.40"), "requires a daemon with DefaultIpcMode: private") + t.Parallel() + + ctx := context.Background() + + // pre-check: default ipc mode in daemon is private + c := testEnv.APIClient() + cID := container.Create(t, ctx, c, container.WithAutoRemove) + + inspect, err := c.ContainerInspect(ctx, cID) + assert.NilError(t, err) + assert.Check(t, is.Equal(string(inspect.HostConfig.IpcMode), "private")) + + // main check: using older client creates "shareable" container + c = request.NewAPIClient(t, client.WithVersion("1.39")) + cID = container.Create(t, ctx, c, container.WithAutoRemove) + + inspect, err = c.ContainerInspect(ctx, cID) + assert.NilError(t, err) + assert.Check(t, is.Equal(string(inspect.HostConfig.IpcMode), "shareable")) +}