From da780e4d275444e9be5fc75d2005f51d71669a8e Mon Sep 17 00:00:00 2001 From: Ruediger Pluem Date: Tue, 9 May 2023 10:51:39 +0200 Subject: [PATCH] Fix bind mounts of filesystems with certain options set Currently bind mounts of filesystems with nodev, nosuid, noexec, noatime, relatime, strictatime, nodiratime options set fail in rootless mode if the same options are not set for the bind mount. For ro filesystems this was resolved by #2570 by remounting again with ro set. Follow the same approach for nodev, nosuid, noexec, noatime, relatime, strictatime, nodiratime but allow to revert back to the old behaviour via the new `--no-mount-fallback` command line option. Add a testcase to verify that bind mounts of filesystems with nodev, nosuid, noexec, noatime options set work in rootless mode. Add a testcase that mounts a nodev, nosuid, noexec, noatime filesystem with a ro flag. Add two further testcases that ensure that the above testcases would fail if the `--no-mount-fallback` command line option is set. * contrib/completions/bash/runc: Add `--no-mount-fallback` command line option for bash completion. * create.go: Add `--no-mount-fallback` command line option. * restore.go: Add `--no-mount-fallback` command line option. * run.go: Add `--no-mount-fallback` command line option. * libcontainer/configs/config.go: Add `NoMountFallback` field to the `Config` struct to store the command line option value. * libcontainer/specconv/spec_linux.go: Add `NoMountFallback` field to the `CreateOpts` struct to store the command line option value and store it in the libcontainer config. * utils_linux.go: Store the command line option value in the `CreateOpts` struct. * libcontainer/rootfs_linux.go: In case that `--no-mount-fallback` is not set try to remount the bind filesystem again with the options nodev, nosuid, noexec, noatime, relatime, strictatime or nodiratime if they are set on the source filesystem. * tests/integration/mounts_sshfs.bats: Add testcases and rework sshfs setup to allow specifying different mount options depending on the test case. Signed-off-by: Ruediger Pluem --- contrib/completions/bash/runc | 3 + create.go | 4 ++ libcontainer/configs/config.go | 4 ++ libcontainer/rootfs_linux.go | 23 ++++++-- libcontainer/specconv/spec_linux.go | 2 + restore.go | 4 ++ run.go | 4 ++ tests/integration/mounts_sshfs.bats | 91 +++++++++++++++++++++++++---- utils_linux.go | 1 + 9 files changed, 119 insertions(+), 17 deletions(-) diff --git a/contrib/completions/bash/runc b/contrib/completions/bash/runc index 1f2da1c045f..69a3b953750 100644 --- a/contrib/completions/bash/runc +++ b/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=" diff --git a/create.go b/create.go index 97854b846cb..3788a532fce 100644 --- a/create.go +++ b/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 { diff --git a/libcontainer/configs/config.go b/libcontainer/configs/config.go index 576db59523e..bb5dbba6588 100644 --- a/libcontainer/configs/config.go +++ b/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 ( diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index 4894400b625..32e61778e1a 100644 --- a/libcontainer/rootfs_linux.go +++ b/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, "") }) } diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go index 7fb67d8eee5..d3938da516c 100644 --- a/libcontainer/specconv/spec_linux.go +++ b/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 { diff --git a/restore.go b/restore.go index d65afcfc788..de5b48d54c2 100644 --- a/restore.go +++ b/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 { diff --git a/run.go b/run.go index 82781669d10..8b4f4d1fb23 100644 --- a/run.go +++ b/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 { diff --git a/tests/integration/mounts_sshfs.bats b/tests/integration/mounts_sshfs.bats index 41f1cf4ebc9..540403e4051 100644 --- a/tests/integration/mounts_sshfs.bats +++ b/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"* ]] +} diff --git a/utils_linux.go b/utils_linux.go index 4c00b2092db..0f787cb3387 100644 --- a/utils_linux.go +++ b/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