bwrap, pivot_root: do not require write access to the rootfs

Keep a reference to the previous working directory and use it for the
umount.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #256
Approved by: cgwalters

3 files changed, 47 insertions, 27 deletions

diff --git a/bubblewrap.c b/bubblewrap.c
index aae2217..e932b5f 100644
--- a/bubblewrap.c
+++ b/bubblewrap.c
@@ -2295,6 +2295,9 @@ main (int    argc,
   if (mkdir ("newroot", 0755))
     die_with_error ("Creating newroot failed");
 
+  if (mount ("newroot", "newroot", NULL, MS_MGC_VAL | MS_BIND | MS_REC, NULL) < 0)
+    die_with_error ("setting up newroot bind");
+
   if (mkdir ("oldroot", 0755))
     die_with_error ("Creating oldroot failed");
 
@@ -2369,32 +2372,29 @@ main (int    argc,
    * We're aiming to make /newroot the real root, and get rid of /oldroot. To do
    * that we need a temporary place to store it before we can unmount it.
    */
-  { cleanup_free char *pivot_tmp = xstrdup ("bwrap-pivot-old-XXXXXX");
-    if (mount ("/newroot", "/newroot", NULL, MS_MGC_VAL | MS_BIND | MS_REC, NULL) < 0)
-      die_with_error ("setting up newroot bind");
+  { cleanup_fd int oldrootfd = open ("/", O_DIRECTORY | O_RDONLY);
+    if (oldrootfd < 0)
+      die_with_error ("can't open /");
     if (chdir ("/newroot") != 0)
       die_with_error ("chdir /newroot");
-    if (mkdtemp (pivot_tmp) == NULL)
-      {
-        /* If the user did a bind mount of /, try /tmp */
-        if (errno == EROFS || errno == EPERM || errno == EACCES)
-          {
-            free (pivot_tmp);
-            pivot_tmp = xstrdup ("tmp/bwrap-pivot-old-XXXXXX");
-            if (mkdtemp (pivot_tmp) == NULL)
-              die_with_error ("mkdtemp");
-          }
-        else
-          die_with_error ("mkdtemp");
-      }
-    if (pivot_root (".", pivot_tmp) != 0)
+    /* While the documentation claims that put_old must be underneath
+     * new_root, it is perfectly fine to use the same directory as the
+     * kernel checks only if old_root is accessible from new_root.
+     *
+     * Both runc and LXC are using this "alternative" method for
+     * setting up the root of the container:
+     *
+     * https://github.com/opencontainers/runc/blob/master/libcontainer/rootfs_linux.go#L671
+     * https://github.com/lxc/lxc/blob/master/src/lxc/conf.c#L1121
+     */
+    if (pivot_root (".", ".") != 0)
       die_with_error ("pivot_root(/newroot)");
-    if (chroot (".") != 0)
-      die_with_error ("chroot .");
+    if (fchdir (oldrootfd) < 0)
+      die_with_error ("fchdir to oldroot");
+    if (umount2 (".", MNT_DETACH) < 0)
+      die_with_error ("umount old root");
     if (chdir ("/") != 0)
       die_with_error ("chdir /");
-    if (umount2 (pivot_tmp, MNT_DETACH) < 0)
-      die_with_error ("unmount old root");
   }
 
   if (opt_unshare_user &&
diff --git a/ci/papr.sh b/ci/papr.sh
index 5cdc43f..5e26642 100755
--- a/ci/papr.sh
+++ b/ci/papr.sh
@@ -29,7 +29,7 @@ buildinstall_to_host() {
         fi
     done
     rsync -rlv ${tmpd}/usr/ /host/usr/
-    if ${BWRAP_SUID}; then
+    if test -n "${BWRAP_SUID:-}"; then
         chmod u+s /host/usr/bin/bwrap
     fi
     rm ${tmpd} -rf
diff --git a/tests/test-run.sh b/tests/test-run.sh
index 07f0e2b..c010a34 100755
--- a/tests/test-run.sh
+++ b/tests/test-run.sh
@@ -24,6 +24,9 @@ trap cleanup EXIT
 cd ${tempdir}
 
 : "${BWRAP:=bwrap}"
+if test -u "$(type -p ${BWRAP})"; then
+    bwrap_is_suid=true
+fi
 
 FUSE_DIR=
 for mp in $(cat /proc/self/mounts | grep " fuse[. ]" | grep user_id=$(id -u) | awk '{print $2}'); do
@@ -46,6 +49,17 @@ if ${is_uidzero} || test -x `dirname $UNREADABLE`; then
     UNREADABLE=
 fi
 
+# https://github.com/projectatomic/bubblewrap/issues/217
+BWRAP_RO_HOST_ARGS="--ro-bind /usr /usr
+          --ro-bind /etc /etc
+          --dir /var/tmp
+          --symlink usr/lib /lib
+          --symlink usr/lib64 /lib64
+          --symlink usr/bin /bin
+          --symlink usr/sbin /sbin
+          --proc /proc
+          --dev /dev"
+
 # Default arg, bind whole host fs to /, tmpfs on /tmp
 RUN="${BWRAP} --bind / / --tmpfs /tmp"
 
@@ -53,7 +67,7 @@ if ! $RUN true; then
     skip Seems like bwrap is not working at all. Maybe setuid is not working
 fi
 
-echo "1..37"
+echo "1..38"
 
 # Test help
 ${BWRAP} --help > help.txt
@@ -78,7 +92,7 @@ for ALT in "" "--unshare-user-try"  "--unshare-pid" "--unshare-user-try --unshar
     echo -n "expect EPERM: " >&2
 
     # Test caps when bwrap is not setuid
-    if ! test -u ${BWRAP}; then
+    if test -n "${bwrap_is_suid:-}"; then
         CAP="--cap-add ALL"
     else
         CAP=""
@@ -113,13 +127,19 @@ $RUN --unshare-pid --as-pid-1 --bind / / bash -c 'echo $$' > as_pid_1.txt
 assert_file_has_content as_pid_1.txt "1"
 echo "ok - can run as pid 1"
 
-if ! test -u ${BWRAP}; then
+# These tests require --unshare-user
+if test -n "${bwrap_is_suid:-}"; then
+    echo "ok - # SKIP no --cap-add support"
     echo "ok - # SKIP no --cap-add support"
 else
-    $BWRAP --unshare-all --uid 0 --gid 0 --cap-add ALL --bind / / --proc /proc \
-           $BWRAP --unshare-all --bind / / --proc /proc echo hello > recursive_proc.txt
+    BWRAP_RECURSE="$BWRAP --unshare-all --uid 0 --gid 0 --cap-add ALL --bind / / --bind /proc /proc"
+    $BWRAP_RECURSE -- $BWRAP --unshare-all --bind / / --bind /proc /proc echo hello > recursive_proc.txt
     assert_file_has_content recursive_proc.txt "hello"
     echo "ok - can mount /proc recursively"
+
+    $BWRAP_RECURSE -- $BWRAP --unshare-all  ${BWRAP_RO_HOST_ARGS} findmnt > recursive-newroot.txt
+    assert_file_has_content recursive-newroot.txt "/usr"
+    echo "ok - can pivot to new rootfs recursively"
 fi
 
 # Test error prefixing