From e72372eafcff8fe9fb2d72cbeb6cac5445c81c1c Mon Sep 17 00:00:00 2001
From: "Tianyu (Ezri) Zhu" <me@ezrizhu.com>
Date: Fri, 8 Mar 2024 15:01:03 -0500
Subject: [PATCH] keep toplevel dir modes and symlinks (#138)

In this commit, we are ensuring the dirs in the `/` root path of the temproot is as close as the 'real' one as possible.

`$SANDBOX_DIR/temproot` is now being created with the same permission as the host, and every other directories on the top level are created with the same mode as the real one.
Symlinks are now also created in the unshare, and removed after unshare finishes.
Tests are created to check the mode, ownership, and symlink of the files in the `/` directory.

Known issues
In the test, we're ignoring files with the name swap.
And also /proc, our current `mount -t proc proc /proc` invocation are creating the /proc dir with nobody and nogroup ownership.  We're tracking this in #151
This PR currently assumes there are no regular files in the root dir besides the swap.img. We're tracking this in issue #150

* feat: keep toplevel dir perms in temproot - fixes #80

* feat: recreate symlinks in temproot - fixes #139

* feat: set correct permission for root dir, and remove symlink after unshare

* feat: set temproot to be writible before removing symlinks

* test: add new test to verify consistency of root dir (see known issues)

* test(reuse_problematic_sandbox): set test to use a non-symblink dir

* test(toplevel-perms): ignore acl bit, user&group ownership
---
 test/reuse_problematic_sandbox.sh |  2 +-
 test/toplevel-perms.sh            | 50 +++++++++++++++++++++++++++++++
 try                               | 25 ++++++++++++++--
 3 files changed, 74 insertions(+), 3 deletions(-)
 create mode 100755 test/toplevel-perms.sh

diff --git a/test/reuse_problematic_sandbox.sh b/test/reuse_problematic_sandbox.sh
index 2cac0204..93ee3f45 100755
--- a/test/reuse_problematic_sandbox.sh
+++ b/test/reuse_problematic_sandbox.sh
@@ -30,5 +30,5 @@ try_example_dir=$(mktemp -d)
 #               at the moment, this modification will be caught as illegal by `try`,
 #               but it doesn't seem to both overlayfs at all.
 # TODO: Extend this with more problematic overlayfs modifications.
-touch "$try_example_dir/temproot/bin/foo"
+touch "$try_example_dir/temproot/etc/foo"
 ! "$TRY" -D "$try_example_dir" "rm file_1.txt; echo test2 >>file_2.txt; touch file.txt.gz" 2>/dev/null
diff --git a/test/toplevel-perms.sh b/test/toplevel-perms.sh
new file mode 100755
index 00000000..9f654a8a
--- /dev/null
+++ b/test/toplevel-perms.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+TRY_TOP="${TRY_TOP:-$(git rev-parse --show-toplevel --show-superproject-working-tree)}"
+TRY="$TRY_TOP/try"
+
+cleanup() {
+    cd /
+
+    if [ -d "$try_workspace" ]
+    then
+        rm -rf "$try_workspace" >/dev/null 2>&1
+    fi
+
+    if [ -f "$expected" ]
+    then
+        rm "$expected"
+    fi
+
+    if [ -f "$target" ]
+    then
+        rm "$target"
+    fi
+
+    if [ -f "$cmd" ]
+    then
+        rm "$cmd"
+    fi
+}
+
+trap 'cleanup' EXIT
+
+try_workspace="$(mktemp -d)"
+cd "$try_workspace" || return 9
+touch test
+
+cmd="$(mktemp)"
+echo "find / -maxdepth 1 -print0 | xargs -0 ls -ld | awk '{print substr(\$1, 1, 10), \$9, \$10, \$11}' | grep -v 'proc' | grep -v 'swap'" > "$cmd"
+# Use this after gidmapper to show user and group ownership
+#echo "find / -maxdepth 1 -print0 | xargs -0 ls -ld | awk '{print substr(\$1, 1, 10), \$3, \$4, \$9, \$10, \$11}' | grep -v 'proc' | grep -v 'swap'" > "$cmd"
+
+# Set up expected output
+expected="$(mktemp)"
+sh "$cmd" >"$expected"
+
+# Set up target output
+target="$(mktemp)"
+
+"$TRY" "sh $cmd" > "$target" || return 1
+#diff -q "$expected" "$target"
+diff "$expected" "$target"
diff --git a/try b/try
index aa95f860..dc948f15 100755
--- a/try
+++ b/try
@@ -109,12 +109,15 @@ try() {
     while IFS="" read -r mountpoint
     do
         ## Only make the directory if the original is a directory too
-        if [ -d "$mountpoint" ]
+        if [ -d "$mountpoint" ] && ! [ -L "$mountpoint" ]
         then
-            mkdir -p "${SANDBOX_DIR}/upperdir/${mountpoint}" "${SANDBOX_DIR}/workdir/${mountpoint}" "${SANDBOX_DIR}/temproot/${mountpoint}"
+            # shellcheck disable=SC2174 # warning acknowledged, "When used with -p, -m only applies to the deepest directory."
+            mkdir -m "$(stat -c %a "$mountpoint")" -p "${SANDBOX_DIR}/upperdir/${mountpoint}" "${SANDBOX_DIR}/workdir/${mountpoint}" "${SANDBOX_DIR}/temproot/${mountpoint}"
         fi
     done <"$DIRS_AND_MOUNTS"
 
+    chmod "$(stat -c %a /)" "$SANDBOX_DIR/temproot"
+
     mount_and_execute="$(mktemp)"
     chroot_executable="$(mktemp)"
     try_mount_log="$(mktemp)"
@@ -187,6 +190,13 @@ do
         continue
     fi
 
+    ## Symlinks
+    if [ -L "$mountpoint" ]
+    then
+        ln -s $(readlink "$mountpoint") "$SANDBOX_DIR/temproot/$mountpoint"
+        continue
+    fi
+
     ## Don't do anything for the root and skip if it is /dev or /proc, we will mount it later
     case "$pure_mountpoint" in
         (/|/dev|/proc) continue;;
@@ -273,6 +283,17 @@ EOF
     unshare --mount --map-root-user --user --pid --fork $EXTRA_NS "$mount_and_execute"
     TRY_EXIT_STATUS=$?
 
+    # remove symlink
+    # first set temproot to be writible, rhel derivatives defaults / to r-xr-xr-x
+    chmod 755 "${SANDBOX_DIR}/temproot"
+    while IFS="" read -r mountpoint
+    do
+        if [  -L "$mountpoint" ]
+        then
+            rm "${SANDBOX_DIR}/temproot/${mountpoint}"
+        fi
+    done <"$DIRS_AND_MOUNTS"
+
     ################################################################################
     # commit?