Merge pull request #3805 from rpluem-vf/nodev_noexec_nosuid

Allow bind mounts of nodev,nosuid,noexec filesystems

Author: kolyshkin

contrib/completions/bash/runc

@@ -461,6 +461,7 @@ _runc_run() {
 	   --no-subreaper
 	   --no-pivot
 	   --no-new-keyring
+	   --no-mount-fallback
 	"
 
 	local options_with_args="
@@ -567,6 +568,7 @@ _runc_create() {
 	   --help
 	   --no-pivot
 	   --no-new-keyring
+	   --no-mount-fallback
 	"
 
 	local options_with_args="
@@ -627,6 +629,7 @@ _runc_restore() {
 	   --no-pivot
 	   --auto-dedup
 	   --lazy-pages
+	   --no-mount-fallback
 	"
 
 	local options_with_args="

create.go

@@ -51,6 +51,10 @@ command(s) that get executed on start, edit the args parameter of the spec. See
 			Name:  "preserve-fds",
 			Usage: "Pass N additional file descriptors to the container (stdio + $LISTEN_FDS + N in total)",
 		},
+		cli.BoolFlag{
+			Name:  "no-mount-fallback",
+			Usage: "Do not fallback when the specific configuration is not applicable (e.g., do not try to remount a bind mount again after the first attempt failed on source filesystems that have nodev, noexec, nosuid, noatime, relatime, strictatime, nodiratime set)",
+		},
 	},
 	Action: func(context *cli.Context) error {
 		if err := checkArgs(context, 1, exactArgs); err != nil {

libcontainer/configs/config.go

@@ -212,6 +212,10 @@ type Config struct {
 	// RootlessCgroups is set when unlikely to have the full access to cgroups.
 	// When RootlessCgroups is set, cgroups errors are ignored.
 	RootlessCgroups bool `json:"rootless_cgroups,omitempty"`
+
+	// Do not try to remount a bind mount again after the first attempt failed on source
+	// filesystems that have nodev, noexec, nosuid, noatime, relatime, strictatime, nodiratime set
+	NoMountFallback bool `json:"no_mount_fallback,omitempty"`
 }
 
 type (

libcontainer/rootfs_linux.go

@@ -35,6 +35,7 @@ type mountConfig struct {
 	cgroup2Path     string
 	rootlessCgroups bool
 	cgroupns        bool
+	noMountFallback bool
 }
 
 // mountEntry contains mount data specific to a mount point.
@@ -83,6 +84,7 @@ func prepareRootfs(pipe io.ReadWriter, iConfig *initConfig, mountFds mountFds) (
 		cgroup2Path:     iConfig.Cgroup2Path,
 		rootlessCgroups: iConfig.RootlessCgroups,
 		cgroupns:        config.Namespaces.Contains(configs.NEWCGROUP),
+		noMountFallback: config.NoMountFallback,
 	}
 	for i, m := range config.Mounts {
 		entry := mountEntry{Mount: m}
@@ -512,7 +514,7 @@ func mountToRootfs(c *mountConfig, m mountEntry) error {
 		// first check that we have non-default options required before attempting a remount
 		if m.Flags&^(unix.MS_REC|unix.MS_REMOUNT|unix.MS_BIND) != 0 {
 			// only remount if unique mount options are set
-			if err := remount(m, rootfs); err != nil {
+			if err := remount(m, rootfs, c.noMountFallback); err != nil {
 				return err
 			}
 		}
@@ -1101,24 +1103,33 @@ func writeSystemProperty(key, value string) error {
 	return os.WriteFile(path.Join("/proc/sys", keyPath), []byte(value), 0o644)
 }
 
-func remount(m mountEntry, rootfs string) error {
+func remount(m mountEntry, rootfs string, noMountFallback bool) error {
 	return utils.WithProcfd(rootfs, m.Destination, func(dstFD string) error {
 		flags := uintptr(m.Flags | unix.MS_REMOUNT)
 		err := mountViaFDs(m.Source, m.srcFD, m.Destination, dstFD, m.Device, flags, "")
 		if err == nil {
 			return nil
 		}
-		// Check if the source has ro flag...
+		// Check if the source has flags set according to noMountFallback
 		src := m.src()
 		var s unix.Statfs_t
 		if err := unix.Statfs(src, &s); err != nil {
 			return &os.PathError{Op: "statfs", Path: src, Err: err}
 		}
-		if s.Flags&unix.MS_RDONLY != unix.MS_RDONLY {
+		var checkflags int
+		if noMountFallback {
+			// Check for ro only
+			checkflags = unix.MS_RDONLY
+		} else {
+			// Check for ro, nodev, noexec, nosuid, noatime, relatime, strictatime,
+			// nodiratime
+			checkflags = unix.MS_RDONLY | unix.MS_NODEV | unix.MS_NOEXEC | unix.MS_NOSUID | unix.MS_NOATIME | unix.MS_RELATIME | unix.MS_STRICTATIME | unix.MS_NODIRATIME
+		}
+		if int(s.Flags)&checkflags == 0 {
 			return err
 		}
-		// ... and retry the mount with ro flag set.
-		flags |= unix.MS_RDONLY
+		// ... and retry the mount with flags found above.
+		flags |= uintptr(int(s.Flags) & checkflags)
 		return mountViaFDs(m.Source, m.srcFD, m.Destination, dstFD, m.Device, flags, "")
 	})
 }

libcontainer/specconv/spec_linux.go

@@ -312,6 +312,7 @@ type CreateOpts struct {
 	Spec             *specs.Spec
 	RootlessEUID     bool
 	RootlessCgroups  bool
+	NoMountFallback  bool
 }
 
 // getwd is a wrapper similar to os.Getwd, except it always gets
@@ -358,6 +359,7 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) {
 		NoNewKeyring:    opts.NoNewKeyring,
 		RootlessEUID:    opts.RootlessEUID,
 		RootlessCgroups: opts.RootlessCgroups,
+		NoMountFallback: opts.NoMountFallback,
 	}
 
 	for _, m := range spec.Mounts {

restore.go

@@ -98,6 +98,10 @@ using the runc checkpoint command.`,
 			Value: "",
 			Usage: "Specify an LSM mount context to be used during restore.",
 		},
+		cli.BoolFlag{
+			Name:  "no-mount-fallback",
+			Usage: "Do not fallback when the specific configuration is not applicable (e.g., do not try to remount a bind mount again after the first attempt failed on source filesystems that have nodev, noexec, nosuid, noatime, relatime, strictatime, nodiratime set)",
+		},
 	},
 	Action: func(context *cli.Context) error {
 		if err := checkArgs(context, 1, exactArgs); err != nil {

run.go

@@ -64,6 +64,10 @@ command(s) that get executed on start, edit the args parameter of the spec. See
 			Name:  "preserve-fds",
 			Usage: "Pass N additional file descriptors to the container (stdio + $LISTEN_FDS + N in total)",
 		},
+		cli.BoolFlag{
+			Name:  "no-mount-fallback",
+			Usage: "Do not fallback when the specific configuration is not applicable (e.g., do not try to remount a bind mount again after the first attempt failed on source filesystems that have nodev, noexec, nosuid, noatime, relatime, strictatime, nodiratime set)",
+		},
 	},
 	Action: func(context *cli.Context) error {
 		if err := checkArgs(context, 1, exactArgs); err != nil {

tests/integration/mounts_sshfs.bats

@@ -3,7 +3,20 @@
 load helpers
 
 function setup() {
-	# Create a ro fuse-sshfs mount; skip the test if it's not working.
+	setup_busybox
+	update_config '.process.args = ["/bin/echo", "Hello World"]'
+}
+
+function teardown() {
+	# Some distros do not have fusermount installed
+	# as a dependency of fuse-sshfs, and good ol' umount works.
+	fusermount -u "$DIR" || umount "$DIR"
+
+	teardown_bundle
+}
+
+function setup_sshfs() {
+	# Create a fuse-sshfs mount; skip the test if it's not working.
 	local sshfs="sshfs
 		-o UserKnownHostsFile=/dev/null
 		-o StrictHostKeyChecking=no
@@ -12,30 +25,86 @@ function setup() {
 	DIR="$BATS_RUN_TMPDIR/fuse-sshfs"
 	mkdir -p "$DIR"
 
-	if ! $sshfs -o ro rootless@localhost: "$DIR"; then
+	if ! $sshfs -o "$1" rootless@localhost: "$DIR"; then
 		skip "test requires working sshfs mounts"
 	fi
+}
 
-	setup_busybox
-	update_config '.process.args = ["/bin/echo", "Hello World"]'
+@test "runc run [rw bind mount of a ro fuse sshfs mount]" {
+	setup_sshfs "ro"
+	update_config '	  .mounts += [{
+					type: "bind",
+					source: "'"$DIR"'",
+					destination: "/mnt",
+					options: ["rw", "rprivate", "nosuid", "nodev", "rbind"]
+				}]'
+
+	runc run --no-mount-fallback test_busybox
+	[ "$status" -eq 0 ]
 }
 
-function teardown() {
-	# New distros (Fedora 35) do not have fusermount installed
-	# as a dependency of fuse-sshfs, and good ol' umount works.
-	fusermount -u "$DIR" || umount "$DIR"
+@test "runc run [dev,exec,suid,atime bind mount of a nodev,nosuid,noexec,noatime fuse sshfs mount]" {
+	setup_sshfs "nodev,nosuid,noexec,noatime"
+	# The "sync" option is used to trigger a remount with the below options.
+	# It serves no further purpose. Otherwise only a bind mount without
+	# applying the below options will be done.
+	update_config '	  .mounts += [{
+					type: "bind",
+					source: "'"$DIR"'",
+					destination: "/mnt",
+					options: ["dev", "suid", "exec", "atime", "rprivate", "rbind", "sync"]
+				}]'
 
-	teardown_bundle
+	runc run test_busybox
+	[ "$status" -eq 0 ]
 }
 
-@test "runc run [rw bind mount of a ro fuse sshfs mount]" {
+@test "runc run [ro bind mount of a nodev,nosuid,noexec,noatime fuse sshfs mount]" {
+	setup_sshfs "nodev,nosuid,noexec,noatime"
 	update_config '	  .mounts += [{
 					type: "bind",
 					source: "'"$DIR"'",
 					destination: "/mnt",
-					options: ["rw", "rprivate", "nosuid", "nodev", "rbind"]
+					options: ["rbind", "ro"]
 				}]'
 
 	runc run test_busybox
 	[ "$status" -eq 0 ]
 }
+
+@test "runc run [dev,exec,suid,atime bind mount of a nodev,nosuid,noexec,noatime fuse sshfs mount without fallback]" {
+	setup_sshfs "nodev,nosuid,noexec,noatime"
+	# The "sync" option is used to trigger a remount with the below options.
+	# It serves no further purpose. Otherwise only a bind mount without
+	# applying the below options will be done.
+	update_config '	  .mounts += [{
+					type: "bind",
+					source: "'"$DIR"'",
+					destination: "/mnt",
+					options: ["dev", "suid", "exec", "atime", "rprivate", "rbind", "sync"]
+				}]'
+
+	runc run --no-mount-fallback test_busybox
+	# The above will fail as we added --no-mount-fallback which causes us not to
+	# try to remount a bind mount again after the first attempt failed on source
+	# filesystems that have nodev, noexec, nosuid, noatime set.
+	[ "$status" -ne 0 ]
+	[[ "$output" == *"runc run failed: unable to start container process: error during container init: error mounting"*"operation not permitted"* ]]
+}
+
+@test "runc run [ro bind mount of a nodev,nosuid,noexec,noatime fuse sshfs mount without fallback]" {
+	setup_sshfs "nodev,nosuid,noexec,noatime"
+	update_config '	  .mounts += [{
+					type: "bind",
+					source: "'"$DIR"'",
+					destination: "/mnt",
+					options: ["rbind", "ro"]
+				}]'
+
+	runc run --no-mount-fallback test_busybox
+	# The above will fail as we added --no-mount-fallback which causes us not to
+	# try to remount a bind mount again after the first attempt failed on source
+	# filesystems that have nodev, noexec, nosuid, noatime set.
+	[ "$status" -ne 0 ]
+	[[ "$output" == *"runc run failed: unable to start container process: error during container init: error mounting"*"operation not permitted"* ]]
+}

utils_linux.go

@@ -175,6 +175,7 @@ func createContainer(context *cli.Context, id string, spec *specs.Spec) (*libcon
 		Spec:             spec,
 		RootlessEUID:     os.Geteuid() != 0,
 		RootlessCgroups:  rootlessCg,
+		NoMountFallback:  context.Bool("no-mount-fallback"),
 	})
 	if err != nil {
 		return nil, err