Browse code

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 <ID>: remove /var/lib/docker/containers/<ID>/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 <ID>: 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 <kolyshkin@gmail.com>

Kir Kolyshkin authored on 2017/11/28 09:10:44
Showing 5 changed files
... ...
@@ -477,6 +477,11 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo
477 477
 
478 478
 		// Ignore Capabilities because it was added in API 1.40.
479 479
 		hostConfig.Capabilities = nil
480
+
481
+		// Older clients (API < 1.40) expects the default to be shareable, make them happy
482
+		if hostConfig.IpcMode.IsEmpty() {
483
+			hostConfig.IpcMode = container.IpcMode("shareable")
484
+		}
480 485
 	}
481 486
 
482 487
 	ccr, err := s.backend.ContainerCreate(types.ContainerCreateConfig{
... ...
@@ -12,7 +12,7 @@ import (
12 12
 
13 13
 const (
14 14
 	// DefaultIpcMode is default for container's IpcMode, if not set otherwise
15
-	DefaultIpcMode = "shareable" // TODO: change to private
15
+	DefaultIpcMode = "private"
16 16
 )
17 17
 
18 18
 // Config defines the configuration of a docker daemon.
... ...
@@ -72,7 +72,7 @@ func (daemon *Daemon) getIpcContainer(id string) (*container.Container, error) {
72 72
 	// Check the container ipc is shareable
73 73
 	if st, err := os.Stat(container.ShmPath); err != nil || !st.IsDir() {
74 74
 		if err == nil || os.IsNotExist(err) {
75
-			return nil, errors.New(errMsg + ": non-shareable IPC")
75
+			return nil, errors.New(errMsg + ": non-shareable IPC (hint: use IpcMode:shareable for the donor container)")
76 76
 		}
77 77
 		// stat() failed?
78 78
 		return nil, errors.Wrap(err, errMsg+": unexpected error from stat "+container.ShmPath)
... ...
@@ -61,6 +61,9 @@ keywords: "API, Docker, rcli, REST, documentation"
61 61
 * `POST /containers/create` now takes a `Capabilities` field to set the list of
62 62
   kernel capabilities to be available for the container (this overrides the default
63 63
   set).
64
+* `POST /containers/create` on Linux now creates a container with `HostConfig.IpcMode=private`
65
+  by default, if IpcMode is not explicitly specified. The per-daemon default can be changed
66
+  back to `shareable` by using `DefaultIpcMode` daemon configuration parameter.
64 67
 * `POST /containers/{id}/update` now accepts a `PidsLimit` field to tune a container's
65 68
   PID limit. Set `0` or `-1` for unlimited. Leave `null` to not change the current value.
66 69
 
... ...
@@ -11,8 +11,11 @@ import (
11 11
 
12 12
 	"github.com/docker/docker/api/types"
13 13
 	containertypes "github.com/docker/docker/api/types/container"
14
+	"github.com/docker/docker/api/types/versions"
15
+	"github.com/docker/docker/client"
14 16
 	"github.com/docker/docker/integration/internal/container"
15 17
 	"github.com/docker/docker/internal/test/daemon"
18
+	"github.com/docker/docker/internal/test/request"
16 19
 	"gotest.tools/assert"
17 20
 	is "gotest.tools/assert/cmp"
18 21
 	"gotest.tools/fs"
... ...
@@ -292,3 +295,28 @@ func TestDaemonIpcModeShareableFromConfig(t *testing.T) {
292 292
 
293 293
 	testDaemonIpcFromConfig(t, "shareable", true)
294 294
 }
295
+
296
+// TestIpcModeOlderClient checks that older client gets shareable IPC mode
297
+// by default, even when the daemon default is private.
298
+func TestIpcModeOlderClient(t *testing.T) {
299
+	skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.40"), "requires a daemon with DefaultIpcMode: private")
300
+	t.Parallel()
301
+
302
+	ctx := context.Background()
303
+
304
+	// pre-check: default ipc mode in daemon is private
305
+	c := testEnv.APIClient()
306
+	cID := container.Create(t, ctx, c, container.WithAutoRemove)
307
+
308
+	inspect, err := c.ContainerInspect(ctx, cID)
309
+	assert.NilError(t, err)
310
+	assert.Check(t, is.Equal(string(inspect.HostConfig.IpcMode), "private"))
311
+
312
+	// main check: using older client creates "shareable" container
313
+	c = request.NewAPIClient(t, client.WithVersion("1.39"))
314
+	cID = container.Create(t, ctx, c, container.WithAutoRemove)
315
+
316
+	inspect, err = c.ContainerInspect(ctx, cID)
317
+	assert.NilError(t, err)
318
+	assert.Check(t, is.Equal(string(inspect.HostConfig.IpcMode), "shareable"))
319
+}