From 793088ed0aebfb99177bd2f1edf7cf872e321ee9 Mon Sep 17 00:00:00 2001 From: Eric Windisch Date: Tue, 2 Jun 2015 12:16:43 -0400 Subject: [PATCH] Make /proc, /sys, /dev readonly for readonly containers If a container is read-only, also set /proc, /sys, & /dev to read-only. This should apply to both privileged and unprivileged containers. Note that when /dev is read-only, device files may still be written to. This change will simply prevent the device paths from being modified, or performing mknod of new devices within the /dev path. Tests are included for all cases. Also adds a test to ensure that /dev/pts is always mounted read/write, even in the case of a read-write rootfs. The kernel restricts writes here naturally and bad things will happen if we mount it ro. Signed-off-by: Eric Windisch Upstream-commit: 5400d8873f730e6099d29af49fe45931665c3b49 Component: engine --- .../engine/daemon/execdriver/native/create.go | 26 +++++++++++++++---- .../integration-cli/docker_cli_run_test.go | 25 +++++++++++++++++- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/components/engine/daemon/execdriver/native/create.go b/components/engine/daemon/execdriver/native/create.go index 1b2d7232d3..a8adda6ded 100644 --- a/components/engine/daemon/execdriver/native/create.go +++ b/components/engine/daemon/execdriver/native/create.go @@ -38,13 +38,16 @@ func (d *driver) createContainer(c *execdriver.Command) (*configs.Config, error) } if c.ProcessConfig.Privileged { - // clear readonly for /sys - for i := range container.Mounts { - if container.Mounts[i].Destination == "/sys" { - container.Mounts[i].Flags &= ^syscall.MS_RDONLY + if !container.Readonlyfs { + // clear readonly for /sys + for i := range container.Mounts { + if container.Mounts[i].Destination == "/sys" { + container.Mounts[i].Flags &= ^syscall.MS_RDONLY + } } + container.ReadonlyPaths = nil } - container.ReadonlyPaths = nil + container.MaskPaths = nil if err := d.setPrivileged(container); err != nil { return nil, err @@ -63,6 +66,19 @@ func (d *driver) createContainer(c *execdriver.Command) (*configs.Config, error) return nil, err } + if container.Readonlyfs { + for i := range container.Mounts { + switch container.Mounts[i].Destination { + case "/proc", "/dev", "/dev/pts": + continue + } + container.Mounts[i].Flags |= syscall.MS_RDONLY + } + + /* These paths must be remounted as r/o */ + container.ReadonlyPaths = append(container.ReadonlyPaths, "/proc", "/dev") + } + if err := d.setupMounts(container, c); err != nil { return nil, err } diff --git a/components/engine/integration-cli/docker_cli_run_test.go b/components/engine/integration-cli/docker_cli_run_test.go index 97b8658e21..a19d04e1f6 100644 --- a/components/engine/integration-cli/docker_cli_run_test.go +++ b/components/engine/integration-cli/docker_cli_run_test.go @@ -2944,11 +2944,25 @@ func (s *DockerSuite) TestRunContainerWithWritableRootfs(c *check.C) { func (s *DockerSuite) TestRunContainerWithReadonlyRootfs(c *check.C) { testRequires(c, NativeExecDriver) - for _, f := range []string{"/file", "/etc/hosts", "/etc/resolv.conf", "/etc/hostname"} { + for _, f := range []string{"/file", "/etc/hosts", "/etc/resolv.conf", "/etc/hostname", "/proc/uptime", "/sys/kernel", "/dev/.dont.touch.me"} { testReadOnlyFile(f, c) } } +func (s *DockerSuite) TestPermissionsPtsReadonlyRootfs(c *check.C) { + testRequires(c, NativeExecDriver) + + // Ensure we have not broken writing /dev/pts + out, status := dockerCmd(c, "run", "--read-only", "--rm", "busybox", "mount") + if status != 0 { + c.Fatal("Could not obtain mounts when checking /dev/pts mntpnt.") + } + expected := "type devpts (rw," + if !strings.Contains(string(out), expected) { + c.Fatalf("expected output to contain %s but contains %s", expected, out) + } +} + func testReadOnlyFile(filename string, c *check.C) { testRequires(c, NativeExecDriver) @@ -2960,6 +2974,15 @@ func testReadOnlyFile(filename string, c *check.C) { if !strings.Contains(string(out), expected) { c.Fatalf("expected output from failure to contain %s but contains %s", expected, out) } + + out, err = exec.Command(dockerBinary, "run", "--read-only", "--privileged", "--rm", "busybox", "touch", filename).CombinedOutput() + if err == nil { + c.Fatal("expected container to error on run with read only error") + } + expected = "Read-only file system" + if !strings.Contains(string(out), expected) { + c.Fatalf("expected output from failure to contain %s but contains %s", expected, out) + } } func (s *DockerSuite) TestRunContainerWithReadonlyEtcHostsAndLinkedContainer(c *check.C) {