privileged: set mounts to rw

Set all OCI mounts as privileged when running in privileged mode. Fixes: #2625 Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
cri-o · Jul 18, 2019 · d3a50d8 · d3a50d8
1 parent 9e0d059
commit d3a50d8
Show file tree

Hide file tree

Showing 3 changed files with 129 additions and 10 deletions.
diff --git a/server/container_create_linux.go b/server/container_create_linux.go
@@ -208,6 +208,7 @@ func makeAccessible(path string, uid, gid int) error {
 	return nil
 }
 
+// nolint:gocyclo
 func (s *Server) createSandboxContainer(ctx context.Context, containerID, containerName string, sb *sandbox.Sandbox, sandboxConfig *pb.PodSandboxConfig, containerConfig *pb.ContainerConfig) (*oci.Container, error) {
 	if sb == nil {
 		return nil, errors.New("createSandboxContainer needs a sandbox")
@@ -510,7 +511,6 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID, contai
 
 		if privileged {
 			specgen.SetupPrivileged(true)
-			setOCIBindMountsPrivileged(&specgen)
 		} else {
 			capabilities := linux.GetSecurityContext().GetCapabilities()
 			// Ensure we don't get a nil pointer error if the config
@@ -570,6 +570,7 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID, contai
 			}
 		}
 	}
+
 	// Join the namespace paths for the pod sandbox container.
 	podInfraState := sb.InfraContainer().State()
 
@@ -606,7 +607,7 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID, contai
 
 		if !isInCRIMounts("/sys", containerConfig.GetMounts()) {
 			specgen.RemoveMount("/sys")
-			specgen.RemoveMount("/sys/cgroup")
+			specgen.RemoveMount("/sys/fs/cgroup")
 			sysMnt := rspec.Mount{
 				Destination: "/sys",
 				Type:        "bind",
@@ -626,6 +627,18 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID, contai
 		if err := specgen.AddOrReplaceLinuxNamespace(string(rspec.NetworkNamespace), netNsPath); err != nil {
 			return nil, err
 		}
+
+		if privileged {
+			specgen.RemoveMount("/sys")
+			specgen.RemoveMount("/sys/fs/cgroup")
+			sysMnt := rspec.Mount{
+				Destination: "/sys",
+				Type:        "bind",
+				Source:      "/sys",
+				Options:     []string{"nosuid", "noexec", "nodev", "rw", "rbind"},
+			}
+			specgen.AddMount(sysMnt)
+		}
 	}
 
 	specgen.AddAnnotation(annotations.IP, sb.IP())
@@ -686,6 +699,10 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID, contai
 		specgen.AddMount(mnt)
 	}
 
+	if privileged {
+		setOCIBindMountsPrivileged(&specgen)
+	}
+
 	// Set hostname and add env for hostname
 	specgen.SetHostname(sb.Hostname())
 	specgen.AddProcessEnv("HOSTNAME", sb.Hostname())
@@ -905,13 +922,8 @@ func setupWorkingDirectory(rootfs, mountLabel, containerCwd string) error {
 func setOCIBindMountsPrivileged(g *generate.Generator) {
 	spec := g.Config
 	// clear readonly for /sys and cgroup
-	for i, m := range spec.Mounts {
-		if spec.Mounts[i].Destination == "/sys" {
-			clearReadOnly(&spec.Mounts[i])
-		}
-		if m.Type == "cgroup" {
-			clearReadOnly(&spec.Mounts[i])
-		}
+	for i := range spec.Mounts {
+		clearReadOnly(&spec.Mounts[i])
 	}
 	spec.Linux.ReadonlyPaths = nil
 	spec.Linux.MaskedPaths = nil
@@ -920,7 +932,9 @@ func setOCIBindMountsPrivileged(g *generate.Generator) {
 func clearReadOnly(m *rspec.Mount) {
 	var opt []string
 	for _, o := range m.Options {
-		if o != "ro" {
+		if o == "rw" {
+			return
+		} else if o != "ro" {
 			opt = append(opt, o)
 		}
 	}

diff --git a/test/ctr.bats b/test/ctr.bats
@@ -1623,3 +1623,34 @@ function teardown() {
 	cleanup_pods
 	stop_crio
 }
+
+
+@test "privileged ctr -- check for rw mounts" {
+	start_crio
+
+	run crictl runp "$TESTDATA"/sandbox_config_privileged.json
+	echo "$output"
+	[ "$status" -eq 0 ]
+	pod_id="$output"
+	run crictl create "$pod_id" "$TESTDATA"/container_redis.json "$TESTDATA"/sandbox_config.json
+	echo "$output"
+	[ "$status" -eq 0 ]
+	ctr_id="$output"
+	run crictl start "$ctr_id"
+	[ "$status" -eq 0 ]
+
+	run crictl exec "$ctr_id" grep ro\, /proc/mounts
+	[ "$status" -eq 0 ]
+	[[ "$output" =~ "tmpfs /sys/fs/cgroup tmpfs" ]]
+
+	run crictl stopp "$pod_id"
+	echo "$output"
+	[ "$status" -eq 0 ]
+	run crictl rmp "$pod_id"
+	echo "$output"
+	[ "$status" -eq 0 ]
+
+	cleanup_ctrs
+	cleanup_pods
+	stop_crio
+}
diff --git a/test/testdata/container_config_privileged.json b/test/testdata/container_config_privileged.json
@@ -0,0 +1,74 @@
+{
+	"metadata": {
+		"name": "container1",
+		"attempt": 1
+	},
+	"image": {
+		"image": "quay.io/crio/redis:alpine"
+	},
+	"command": [
+		"/bin/ls"
+	],
+	"args": [],
+	"working_dir": "/",
+	"envs": [
+		{
+			"key": "PATH",
+			"value": "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
+		},
+		{
+			"key": "TERM",
+			"value": "xterm"
+		},
+		{
+			"key": "TESTDIR",
+			"value": "test/dir1"
+		},
+		{
+			"key": "TESTFILE",
+			"value": "test/file1"
+		}
+	],
+	"labels": {
+		"type": "small",
+		"batch": "no"
+	},
+	"annotations": {
+		"owner": "dragon",
+		"daemon": "crio"
+	},
+	"log_path": "",
+	"stdin": false,
+	"stdin_once": false,
+	"tty": false,
+	"linux": {
+		"privileged": true,
+		"resources": {
+			"cpu_period": 10000,
+			"cpu_quota": 20000,
+			"cpu_shares": 512,
+			"oom_score_adj": 30,
+			"memory_limit_in_bytes": 268435456
+		},
+		"security_context": {
+			"namespace_options": {
+				"pid": 1
+			},
+			"readonly_rootfs": false,
+			"selinux_options": {
+				"user": "system_u",
+				"role": "system_r",
+				"type": "svirt_lxc_net_t",
+				"level": "s0:c4,c5"
+			},
+			"capabilities": {
+				"add_capabilities": [
+					"setuid",
+					"setgid"
+				],
+				"drop_capabilities": [
+				]
+			}
+		}
+	}
+}