Merge pull request moby#25540 from estesp/ro-plus-userns

thaJeztah · web-flow · commit 8ac2000f5065 · 2016-09-14T13:53:58.000+02:00
Remove --read-only restriction when user ns enabled
diff --git 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
@@ -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
@@ -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
@@ -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) {

Original file line number	Diff line number	Diff line change
`@@ -496,9 +496,6 @@ func verifyPlatformContainerSettings(daemon Daemon, hostConfig containertypes.`
`496`	`496`	`if hostConfig.PidMode.IsHost() && !hostConfig.UsernsMode.IsHost() {`
`497`	`497`	`return warnings, fmt.Errorf("Cannot share the host PID namespace when user namespaces are enabled")`
`498`	`498`	`}`
`499`		`- if hostConfig.ReadonlyRootfs {`
`500`		`- return warnings, fmt.Errorf("Cannot use the --read-only option when user namespaces are enabled")`
`501`		`- }`
`502`	`499`	`}`
`503`	`500`	`if hostConfig.CgroupParent != "" && UsingSystemd(daemon.configStore) {`
`504`	`501`	`// CgroupParent for systemd cgroup should be named as "xxx.slice"`