From e84e7f9376c2329ae3a022178eb18d0a89c99a8c Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 16 Nov 2021 13:50:09 -0800 Subject: [PATCH] [1.0] Fix failure with rw bind mount of a ro fuse As reported in [1], in a case where read-only fuse (sshfs) mount is used as a volume without specifying ro flag, the kernel fails to remount it (when adding various flags such as nosuid and nodev), returning EPERM. Here's the relevant strace line: > [pid 333966] mount("/tmp/bats-run-PRVfWc/runc.RbNv8g/bundle/mnt", "/proc/self/fd/7", 0xc0001e9164, MS_NOSUID|MS_NODEV|MS_REMOUNT|MS_BIND|MS_REC, NULL) = -1 EPERM (Operation not permitted) I was not able to reproduce it with other read-only mounts as the source (tried tmpfs, read-only bind mount, and an ext2 mount), so somehow this might be specific to fuse. The fix is to check whether the source has RDONLY flag, and retry the remount with this flag added. A test case (which was kind of hard to write) is added, and it fails without the fix. Note that rootless user need to be able to ssh to rootless@localhost in order to sshfs to work -- amend setup scripts to make it work, and skip the test if the setup is not working. [1] https://github.com/containers/podman/issues/12205 Signed-off-by: Kir Kolyshkin (cherry picked from commit 50105de1d87ed0994c32cc9fd57efd0a0f6ef544) Conflicts: - .cirrus.yml: trivial, due to missing commit f0dbefac. - .github/workflows/test.yml: due to missing commits 120f74060 and 3fd1851ce93e2, resolved manually. - Dockerfile: trivial, due to missing commit 24d318b8bb7a. - libcontainer/rootfs_linux.go: due to missing commits 36aefad45dd and 9c444070ec7bb8, resolved manually. Signed-off-by: Kir Kolyshkin --- .cirrus.yml | 8 +++++- .github/workflows/test.yml | 5 ++-- Dockerfile | 1 + Vagrantfile.fedora34 | 5 ++-- libcontainer/rootfs_linux.go | 17 +++++++++++- tests/integration/mounts_sshfs.bats | 40 +++++++++++++++++++++++++++++ 6 files changed, 70 insertions(+), 6 deletions(-) create mode 100644 tests/integration/mounts_sshfs.bats diff --git a/.cirrus.yml b/.cirrus.yml index 74b7deae4..5fe7086f3 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -106,7 +106,7 @@ task: yum config-manager --set-enabled powertools ;; esac - yum install -y -q gcc git iptables jq glibc-static libseccomp-devel make criu + yum install -y -q gcc git iptables jq glibc-static libseccomp-devel make criu fuse-sshfs # install Go curl -fsSL "https://dl.google.com/go/go${GO_VERSION}.linux-amd64.tar.gz" | tar Cxz /usr/local # install bats @@ -118,6 +118,12 @@ task: cd - # Add a user for rootless tests useradd -u2000 -m -d/home/rootless -s/bin/bash rootless + # Allow root and rootless itself to execute `ssh rootless@localhost` in tests/rootless.sh + ssh-keygen -t ecdsa -N "" -f /root/rootless.key + mkdir -m 0700 -p /home/rootless/.ssh + cp /root/rootless.key /home/rootless/.ssh/id_ecdsa + cat /root/rootless.key.pub >> /home/rootless/.ssh/authorized_keys + chown -R rootless.rootless /home/rootless # set PATH echo 'export PATH=/usr/local/go/bin:/usr/local/bin:$PATH' >> /root/.bashrc # Setup ssh localhost for terminal emulation (script -e did not work) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 439513b89..1f4056c9a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -32,7 +32,7 @@ jobs: # criu repo sudo add-apt-repository -y ppa:criu/ppa # apt-add-repository runs apt update so we don't have to - sudo apt -q install libseccomp-dev criu + sudo apt -q install libseccomp-dev criu sshfs - name: install go ${{ matrix.go-version }} uses: actions/setup-go@v2 @@ -56,9 +56,10 @@ jobs: if: matrix.rootless == 'rootless' run: | sudo useradd -u2000 -m -d/home/rootless -s/bin/bash rootless - # Allow root to execute `ssh rootless@localhost` in tests/rootless.sh + # Allow root and rootless itself to execute `ssh rootless@localhost` in tests/rootless.sh ssh-keygen -t ecdsa -N "" -f $HOME/rootless.key sudo mkdir -m 0700 -p /home/rootless/.ssh + sudo cp $HOME/rootless.key /home/rootless/.ssh/id_ecdsa sudo cp $HOME/rootless.key.pub /home/rootless/.ssh/authorized_keys sudo chown -R rootless.rootless /home/rootless diff --git a/Dockerfile b/Dockerfile index 2266903ce..7a56ff45d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -32,6 +32,7 @@ RUN echo 'deb https://download.opensuse.org/repositories/devel:/tools:/criu/Debi libseccomp2 \ pkg-config \ python-minimal \ + sshfs \ sudo \ uidmap \ && apt-get clean \ diff --git a/Vagrantfile.fedora34 b/Vagrantfile.fedora34 index 2c1f049a5..3841e5df8 100644 --- a/Vagrantfile.fedora34 +++ b/Vagrantfile.fedora34 @@ -21,7 +21,7 @@ Vagrant.configure("2") do |config| config exclude kernel,kernel-core config install_weak_deps false update -install iptables gcc make golang-go glibc-static libseccomp-devel bats jq git-core criu +install iptables gcc make golang-go glibc-static libseccomp-devel bats jq git-core criu fuse-sshfs ts run EOF done @@ -30,9 +30,10 @@ EOF # Add a user for rootless tests useradd -u2000 -m -d/home/rootless -s/bin/bash rootless - # Allow root to execute `ssh rootless@localhost` in tests/rootless.sh + # Allow root and rootless itself to execute `ssh rootless@localhost` in tests/rootless.sh ssh-keygen -t ecdsa -N "" -f /root/rootless.key mkdir -m 0700 -p /home/rootless/.ssh + cp /root/rootless.key /home/rootless/.ssh/id_ecdsa cat /root/rootless.key.pub >> /home/rootless/.ssh/authorized_keys chown -R rootless.rootless /home/rootless diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index 430f490de..c9602c217 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -1035,7 +1035,22 @@ func writeSystemProperty(key, value string) error { func remount(m *configs.Mount, rootfs string) error { return utils.WithProcfd(rootfs, m.Destination, func(procfd string) error { - return unix.Mount(m.Source, procfd, m.Device, uintptr(m.Flags|unix.MS_REMOUNT), "") + flags := uintptr(m.Flags | unix.MS_REMOUNT) + err := unix.Mount(m.Source, procfd, m.Device, flags, "") + if err == nil { + return nil + } + // Check if the source has ro flag... + var s unix.Statfs_t + if err := unix.Statfs(m.Source, &s); err != nil { + return &os.PathError{Op: "statfs", Path: m.Source, Err: err} + } + if s.Flags&unix.MS_RDONLY != unix.MS_RDONLY { + return err + } + // ... and retry the mount with ro flag set. + flags |= unix.MS_RDONLY + return unix.Mount(m.Source, procfd, m.Device, flags, "") }) } diff --git a/tests/integration/mounts_sshfs.bats b/tests/integration/mounts_sshfs.bats new file mode 100644 index 000000000..abf82357d --- /dev/null +++ b/tests/integration/mounts_sshfs.bats @@ -0,0 +1,40 @@ +#!/usr/bin/env bats + +load helpers + +function setup() { + # Create a ro fuse-sshfs mount; skip the test if it's not working. + local sshfs="sshfs + -o UserKnownHostsFile=/dev/null + -o StrictHostKeyChecking=no + -o PasswordAuthentication=no" + + DIR="$BATS_RUN_TMPDIR/fuse-sshfs" + mkdir -p "$DIR" + + if ! $sshfs -o ro rootless@localhost: "$DIR"; then + skip "test requires working sshfs mounts" + fi + + setup_hello +} + +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" + + teardown_bundle +} + +@test "runc run [rw bind mount of a ro fuse sshfs mount]" { + update_config ' .mounts += [{ + type: "bind", + source: "'"$DIR"'", + destination: "/mnt", + options: ["rw", "rprivate", "nosuid", "nodev", "rbind"] + }]' + + runc run test_busybox + [ "$status" -eq 0 ] +}