rootless: make /sys/fs/cgroup/* read-only

The default rootless spec bind-mounts /sys with "rbind,ro" (because mounting sysfs
requires netns to be unshared), however, it does not make subfilesystems mounted
under /sys read-only.

So, files under /sys/fs/cgroup were unexpectedly writable when they are chmod/chowned
via privileged helpers such as pam_cgfs.

This patch fixes the issue by mounting /sys/fs/cgroup/* as read-only explicitly.

Signed-off-by: Akihiro Suda <[email protected]>

libcontainer/rootfs_linux.go

@@ -297,6 +297,12 @@ func mountToRootfs(m *configs.Mount, rootfs, mountLabel string) error {
 		if err := mountToRootfs(tmpfs, rootfs, mountLabel); err != nil {
 			return err
 		}
+		// Add a dummy directory for cgroup2.
+		// Without this hack, `df` fails with `df: /sys/fs/cgroup/unified: No such file or directory`
+		// when /sys is bind-mounted from the host to the container (typically when netns is not unshared),
+		if err := os.MkdirAll(filepath.Join(rootfs, m.Destination, "unified"), 0755); err != nil {
+			return err
+		}
 		for _, b := range binds {
 			if err := mountToRootfs(b, rootfs, mountLabel); err != nil {
 				return err
@@ -360,6 +366,12 @@ func getCgroupMounts(m *configs.Mount) ([]*configs.Mount, error) {
 	var binds []*configs.Mount
 
 	for _, mm := range mounts {
+		// when /sys is bind-mounted from the host to the container (typically when netns is not unshared),
+		// mm.Mountpoint can be like "/containers/foo/rootfs/sys/fs/cgroup/systemd/user.slice/user-1001.slice/session-1.scope/foo".
+		//
+		if !strings.HasPrefix(mm.Mountpoint, "/sys/fs/cgroup") {
+			continue
+		}
 		dir, err := mm.GetOwnCgroup(cgroupPaths)
 		if err != nil {
 			return nil, err

libcontainer/specconv/example.go

@@ -162,6 +162,10 @@ func ToRootless(spec *specs.Spec) {
 	var namespaces []specs.LinuxNamespace
 
 	// Remove networkns from the spec.
+	//
+	// TODO: removal of networkns should be optional,
+	// because networkns can be made useful with usermode network stack
+	// such as slirp4netns or VPNKit.
 	for _, ns := range spec.Linux.Namespaces {
 		switch ns.Type {
 		case specs.NetworkNamespace, specs.UserNamespace:
@@ -189,33 +193,66 @@ func ToRootless(spec *specs.Spec) {
 	}}
 
 	// Fix up mounts.
-	var mounts []specs.Mount
-	for _, mount := range spec.Mounts {
-		// Ignore all mounts that are under /sys.
-		if strings.HasPrefix(mount.Destination, "/sys") {
-			continue
-		}
-
+	fixupRootlessBindSys(spec)
+	for i := range spec.Mounts {
+		mount := &spec.Mounts[i]
 		// Remove all gid= and uid= mappings.
 		var options []string
 		for _, option := range mount.Options {
 			if !strings.HasPrefix(option, "gid=") && !strings.HasPrefix(option, "uid=") {
 				options = append(options, option)
 			}
 		}
-
 		mount.Options = options
-		mounts = append(mounts, mount)
 	}
-	// Add the sysfs mount as an rbind.
-	mounts = append(mounts, specs.Mount{
-		Source:      "/sys",
-		Destination: "/sys",
-		Type:        "none",
-		Options:     []string{"rbind", "nosuid", "noexec", "nodev", "ro"},
-	})
-	spec.Mounts = mounts
 
 	// Remove cgroup settings.
 	spec.Linux.Resources = nil
 }
+
+// fixupRootlessBindSys bind-mounts /sys.
+// This is required when netns is not unshared.
+func fixupRootlessBindSys(spec *specs.Spec) {
+	// Fix up mounts.
+	var mounts []specs.Mount
+	for _, mount := range spec.Mounts {
+		// Ignore all mounts that are under /sys.
+		if strings.HasPrefix(mount.Destination, "/sys") {
+			continue
+		}
+		mounts = append(mounts, mount)
+	}
+	mounts = append(mounts, []specs.Mount{
+		// Add the sysfs mount as an rbind.
+		// Note:
+		// * "ro" does not make submounts read-only recursively.
+		// * rbind work for sysfs but bind does not.
+		{
+			Source:      "/sys",
+			Destination: "/sys",
+			Type:        "none",
+			Options:     []string{"rbind", "nosuid", "noexec", "nodev", "ro"},
+		},
+		// Add cgroup mount so as to make them read-only
+		{
+			Destination: "/sys/fs/cgroup",
+			Type:        "cgroup",
+			Source:      "cgroup",
+			Options:     []string{"nosuid", "noexec", "nodev", "relatime", "ro"},
+		},
+	}...)
+	spec.Mounts = mounts
+	var maskedPaths []string
+	for _, masked := range spec.Linux.MaskedPaths {
+		// Ignore masked paths that are under /sys. (should be safe for rootless mode)
+		//
+		// Without this hack, `df` fails with "df: /sys/firmware/efi/efivars"
+		// because the efivars entry exists in mtab (because /sys is rbind-mounted)
+		// but /sys/firmware is masked.
+		if strings.HasPrefix(masked, "/sys") {
+			continue
+		}
+		maskedPaths = append(maskedPaths, masked)
+	}
+	spec.Linux.MaskedPaths = maskedPaths
+}

tests/integration/cgroups.bats

@@ -125,3 +125,19 @@ EOF
     [ "$status" -eq 0 ]
     [[ ${lines[0]} == *"cgroups_exec"* ]]
 }
+
+@test "runc exec write cgroup (limits + cgrouppath + permission on the cgroup dir) fails" {
+   [[ "$ROOTLESS" -ne 0 ]] && requires rootless_cgroup
+
+    set_cgroups_path "$BUSYBOX_BUNDLE"
+    set_resources_limit "$BUSYBOX_BUNDLE"
+
+    runc run -d --console-socket $CONSOLE_SOCKET test_cgroups_permissions
+    [ "$status" -eq 0 ]
+
+# In previous version of rootless runc, the whole /sys was just recursively bind-mounted
+# but not recursively read-only
+    runc exec test_cgroups_permissions mkdir -p /sys/fs/cgroup/cpuset/runc-cgroups-integration-test/dummy
+    [ "$status" -eq 1 ]
+    [[ ${lines[0]} == *"Read-only file system"* ]]
+}

tests/integration/mounts.bats

@@ -19,3 +19,12 @@ function teardown() {
 	[ "$status" -eq 0 ]
 	[[ "${lines[0]}" =~ '/tmp/bind/config.json' ]]
 }
+
+# make sure there is no "df: /foo/bar: No such file or directory" issue in the default configuration
+@test "runc run [df]" {
+	  CONFIG=$(jq '.process.args = ["df"]' config.json)
+	  echo "${CONFIG}" >config.json
+
+	  runc run test_df
+	  [ "$status" -eq 0 ]
+}