From 6062ae5742e49ec1a79073c327f3d1343c218a12 Mon Sep 17 00:00:00 2001 From: Phil Estes Date: Tue, 9 Aug 2016 12:14:38 -0400 Subject: [PATCH] Remove --read-only restriction when user ns enabled The restriction is no longer necessary given changes at the runc layer related to mount options of the rootfs. Also cleaned up the docs on restrictions left for userns enabled mode. Re-enabled tests related to --read-only when testing a userns-enabled daemon in integration-cli. Docker-DCO-1.1-Signed-off-by: Phil Estes (github: estesp) --- daemon/daemon_unix.go | 3 --- docs/reference/commandline/dockerd.md | 8 +++---- integration-cli/docker_cli_run_test.go | 32 ++++++++++++++------------ integration-cli/requirements.go | 13 +++++++++++ 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index d36d3cac6c..4ed0594c6a 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -496,9 +496,6 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *containertypes. if hostConfig.PidMode.IsHost() && !hostConfig.UsernsMode.IsHost() { return warnings, fmt.Errorf("Cannot share the host PID namespace when user namespaces are enabled") } - if hostConfig.ReadonlyRootfs { - return warnings, fmt.Errorf("Cannot use the --read-only option when user namespaces are enabled") - } } if hostConfig.CgroupParent != "" && UsingSystemd(daemon.configStore) { // CgroupParent for systemd cgroup should be named as "xxx.slice" diff --git a/docs/reference/commandline/dockerd.md b/docs/reference/commandline/dockerd.md index 3552bf0fd9..658a18a48a 100644 --- a/docs/reference/commandline/dockerd.md +++ b/docs/reference/commandline/dockerd.md @@ -955,16 +955,16 @@ This option will completely disable user namespace mapping for the container's u The following standard Docker features are currently incompatible when running a Docker daemon with user namespaces enabled: - - sharing PID or NET namespaces with the host (`--pid=host` or `--network=host`) - - A `--read-only` container filesystem (this is a Linux kernel restriction against remounting with modified flags of a currently mounted filesystem when inside a user namespace) - - external (volume or graph) drivers which are unaware/incapable of using daemon user mappings + - sharing PID or NET namespaces with the host (`--pid=host` or `--net=host`) - Using `--privileged` mode flag on `docker run` (unless also specifying `--userns=host`) In general, user namespaces are an advanced feature and will require coordination with other capabilities. For example, if volumes are mounted from the host, file ownership will have to be pre-arranged if the user or administrator wishes the containers to have expected access to the volume -contents. +contents. Note that when using external volume or graph driver plugins, those +external software programs must be made aware of user and group mapping ranges +if they are to work seamlessly with user namespace support. Finally, while the `root` user inside a user namespaced container process has many of the expected admin privileges that go along with being the superuser, the diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index f5b1eb9d81..5410b2e4f7 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -2859,16 +2859,20 @@ func (s *DockerSuite) TestRunContainerWithWritableRootfs(c *check.C) { func (s *DockerSuite) TestRunContainerWithReadonlyRootfs(c *check.C) { // Not applicable on Windows which does not support --read-only - testRequires(c, DaemonIsLinux) + testRequires(c, DaemonIsLinux, UserNamespaceROMount) - testReadOnlyFile(c, "/file", "/etc/hosts", "/etc/resolv.conf", "/etc/hostname", "/sys/kernel", "/dev/.dont.touch.me") + testPriv := true + // don't test privileged mode subtest if user namespaces enabled + if root := os.Getenv("DOCKER_REMAP_ROOT"); root != "" { + testPriv = false + } + testReadOnlyFile(c, testPriv, "/file", "/etc/hosts", "/etc/resolv.conf", "/etc/hostname", "/sys/kernel", "/dev/.dont.touch.me") } func (s *DockerSuite) TestPermissionsPtsReadonlyRootfs(c *check.C) { // Not applicable on Windows due to use of Unix specific functionality, plus // the use of --read-only which is not supported. - // --read-only + userns has remount issues - testRequires(c, DaemonIsLinux, NotUserNamespace) + testRequires(c, DaemonIsLinux, UserNamespaceROMount) // Ensure we have not broken writing /dev/pts out, status := dockerCmd(c, "run", "--read-only", "--rm", "busybox", "mount") @@ -2881,9 +2885,7 @@ func (s *DockerSuite) TestPermissionsPtsReadonlyRootfs(c *check.C) { } } -func testReadOnlyFile(c *check.C, filenames ...string) { - // Not applicable on Windows which does not support --read-only - testRequires(c, DaemonIsLinux, NotUserNamespace) +func testReadOnlyFile(c *check.C, testPriv bool, filenames ...string) { touch := "touch " + strings.Join(filenames, " ") out, _, err := dockerCmdWithError("run", "--read-only", "--rm", "busybox", "sh", "-c", touch) c.Assert(err, checker.NotNil) @@ -2893,6 +2895,10 @@ func testReadOnlyFile(c *check.C, filenames ...string) { c.Assert(out, checker.Contains, expected) } + if !testPriv { + return + } + out, _, err = dockerCmdWithError("run", "--read-only", "--privileged", "--rm", "busybox", "sh", "-c", touch) c.Assert(err, checker.NotNil) @@ -2904,8 +2910,7 @@ func testReadOnlyFile(c *check.C, filenames ...string) { func (s *DockerSuite) TestRunContainerWithReadonlyEtcHostsAndLinkedContainer(c *check.C) { // Not applicable on Windows which does not support --link - // --read-only + userns has remount issues - testRequires(c, DaemonIsLinux, NotUserNamespace) + testRequires(c, DaemonIsLinux, UserNamespaceROMount) dockerCmd(c, "run", "-d", "--name", "test-etc-hosts-ro-linked", "busybox", "top") @@ -2917,8 +2922,7 @@ func (s *DockerSuite) TestRunContainerWithReadonlyEtcHostsAndLinkedContainer(c * func (s *DockerSuite) TestRunContainerWithReadonlyRootfsWithDNSFlag(c *check.C) { // Not applicable on Windows which does not support either --read-only or --dns. - // --read-only + userns has remount issues - testRequires(c, DaemonIsLinux, NotUserNamespace) + testRequires(c, DaemonIsLinux, UserNamespaceROMount) out, _ := dockerCmd(c, "run", "--read-only", "--dns", "1.1.1.1", "busybox", "/bin/cat", "/etc/resolv.conf") if !strings.Contains(string(out), "1.1.1.1") { @@ -2928,8 +2932,7 @@ func (s *DockerSuite) TestRunContainerWithReadonlyRootfsWithDNSFlag(c *check.C) func (s *DockerSuite) TestRunContainerWithReadonlyRootfsWithAddHostFlag(c *check.C) { // Not applicable on Windows which does not support --read-only - // --read-only + userns has remount issues - testRequires(c, DaemonIsLinux, NotUserNamespace) + testRequires(c, DaemonIsLinux, UserNamespaceROMount) out, _ := dockerCmd(c, "run", "--read-only", "--add-host", "testreadonly:127.0.0.1", "busybox", "/bin/cat", "/etc/hosts") if !strings.Contains(string(out), "testreadonly") { @@ -3284,8 +3287,7 @@ func (s *DockerSuite) TestRunNetworkFilesBindMountRO(c *check.C) { func (s *DockerSuite) TestRunNetworkFilesBindMountROFilesystem(c *check.C) { // Not applicable on Windows as uses Unix specific functionality - // --read-only + userns has remount issues - testRequires(c, SameHostDaemon, DaemonIsLinux, NotUserNamespace) + testRequires(c, SameHostDaemon, DaemonIsLinux, UserNamespaceROMount) filename := createTmpFile(c, "test123") defer os.Remove(filename) diff --git a/integration-cli/requirements.go b/integration-cli/requirements.go index 51bc603154..bba6f815f8 100644 --- a/integration-cli/requirements.go +++ b/integration-cli/requirements.go @@ -153,6 +153,19 @@ var ( }, "Test requires support for IPv6", } + UserNamespaceROMount = testRequirement{ + func() bool { + // quick case--userns not enabled in this test run + if os.Getenv("DOCKER_REMAP_ROOT") == "" { + return true + } + if _, _, err := dockerCmdWithError("run", "--rm", "--read-only", "busybox", "date"); err != nil { + return false + } + return true + }, + "Test cannot be run if user namespaces enabled but readonly mounts fail on this kernel.", + } UserNamespaceInKernel = testRequirement{ func() bool { if _, err := os.Stat("/proc/self/uid_map"); os.IsNotExist(err) {