diff mbox series

[nft,v2,3/3] tests/shell: run each test in separate namespace and allow rootless

Message ID 20230901150916.183949-4-thaller@redhat.com
State Changes Requested
Delegated to: Pablo Neira
Headers show
Series tests/shell: allow running tests as non-root | expand

Commit Message

Thomas Haller Sept. 1, 2023, 3:05 p.m. UTC
Don't unshare the entire shell script, calling itself again. Instead,
call unshare separately for each test invocation. That means, all tests
use now a different sandbox.

Also, allow to run rootless/unprivileged.

The user still can opt-out from unshare via -U/--no-unshare option
or NFT_TEST_NO_UNSHARE=y. That might be useful if unshare for some
reason doesn't work for the user, or if you want to test on your host
system.

Also try to also run a separate PID namespace, to get more isolation. If
that is not working, then fallback without separate PID namespace.

We no longer require [ `id -u` = 0 ] to proceed, so a rootless user can
run the tests. We detect whether we have real root and set
NFT_TEST_HAVE_REALROOT=y. Some tests won't work rootless. For example,
the socket buffers is limited by /proc/sys/net/core/{wmem_max,rmem_max}
which real-root can override, but rootless tests cannot. Such tests
should check for [ "$NFT_TEST_HAVE_REALROOT" != y ] and skip the test
gracefully. Optimally, tests also work in the rootless environment and
most tests should pass in both, and not check for have-realroot.

Usually, the user doesn't need to tell the script whether they have real
root. The script will autodetect it via [ `id -u` = 0 ]. But that won't
work if you run inside a rootless container already. In that case, you
would want to tell the script that we don't have real-root. Either via
-R/--without-root option or NFT_TEST_HAVE_REALROOT=n.

If they wish, the test can know whether they run inside "unshare"
environment by checking for [ "$NFT_TEST_IS_UNSHARED" = y ].

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/run-tests.sh | 83 +++++++++++++++++++++++++++++-----------
 1 file changed, 61 insertions(+), 22 deletions(-)

Comments

Florian Westphal Sept. 4, 2023, 8:45 a.m. UTC | #1
Thomas Haller <thaller@redhat.com> wrote:
> +	UNSHARE=( unshare -f -p --mount-proc -U --map-root-user -n )
> +	if ! "${UNSHARE[@]}" true ; then
> +		# Try without PID namespace.
> +		UNSHARE=( unshare -f -U --map-root-user -n )
> +		if ! "${UNSHARE[@]}" true ; then
> +			msg_error "Unshare does not work. Rerun with -U/--no-unshare or NFT_TEST_NO_UNSHARE=y"

This will always fail here due to
user.max_user_namespaces=0

in sysctl.cfg.

So please add a fallback to plain unshare -n or only use unpriv userns
if the script isn't called with uid 0.

>  	msg_info "[EXECUTING]	$testfile"
> -	test_output=$(NFT="$NFT" DIFF=$DIFF ${testfile} 2>&1)
> +	test_output=$(NFT="$NFT" DIFF=$DIFF "${UNSHARE[@]}" "$testfile" 2>&1)
>  	rc_got=$?

This is more complicated because we'll also need to collect the ruleset
dump from within the temporary ns.

Once all of that works you can remove kernel_cleanup().
diff mbox series

Patch

diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index 147185cb548a..fda738983ec9 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -17,11 +17,13 @@  usage() {
 	echo " $0 [OPTIONS]"
 	echo
 	echo "OPTIONS:"
-	echo " \"-v\"                 : also VERBOSE=y"
-	echo " \"-g\"                 : also DUMPGEN=y"
-	echo " \"-V\"                 : also VALGRIND=y"
-	echo " \"-K\"                 : also KMEMLEAK=y"
-	echo " \"-L\"|\"-list-tests\" : list the test name and quit"
+	echo " \"-v\"                   : also VERBOSE=y"
+	echo " \"-g\"                   : also DUMPGEN=y"
+	echo " \"-V\"                   : also VALGRIND=y"
+	echo " \"-K\"                   : also KMEMLEAK=y"
+	echo " \"-L\"|\"--list-tests\"  : list the test name and quit"
+	echo " \"-R\"|\"--without-realroot\" : sets NFT_TEST_HAVE_REALROOT=n"
+	echo " \"-U\"|\"--no-unshare\"  : sets NFT_TEST_NO_UNSHARE=y"
 	echo
 	echo "VARIABLES:"
 	echo "  NFT=<PATH>   : Path to nft executable"
@@ -29,31 +31,28 @@  usage() {
 	echo "  DUMPGEN=*|y  : See also \"-g\" option"
 	echo "  VALGRIND=*|y : See also \"-V\" option"
 	echo "  KMEMLEAK=*|y : See also \"-y\" option"
+	echo "  NFT_TEST_HAVE_REALROOT=*|y : To indicate whether the test has real root permissions."
+	echo "                 Usually, you don't need this and it gets autodetected."
+	echo "                 You might want to set it, if you know better than the"
+	echo "                 \`id -u\` check, whether the user is root in the main namespace."
+	echo "                 Note that without real root, certain tests may not work,"
+	echo "                 e.g. due to limited /proc/sys/net/core/{wmem_max,rmem_max}."
+	echo "                 Checks that cannot pass in such environment should check for"
+	echo "                 [ \"$NFT_TEST_HAVE_REALROOT\" != y ] and skip gracefully."
+	echo " NFT_TEST_NO_UNSHARE=*|y : Usually, each test will run in a separate namespace."
+	echo "                 You can opt-out from that by setting NFT_TEST_NO_UNSHARE=y."
 }
 
-# Configuration
 BASEDIR="$(dirname "$0")"
 TESTDIR="$BASEDIR/testcases"
 SRC_NFT="$BASEDIR/../../src/nft"
-DIFF=$(which diff)
-
-if [ "$(id -u)" != "0" ] ; then
-	msg_error "this requires root!"
-fi
-
-if [ "${1}" != "run" ]; then
-	if unshare -f -n true; then
-		unshare -n "${0}" run $@
-		exit $?
-	fi
-	msg_warn "cannot run in own namespace, connectivity might break"
-fi
-shift
 
 VERBOSE="$VERBOSE"
 DUMPGEN="$DUMPGEN"
 VALGRIND="$VALGRIND"
 KMEMLEAK="$KMEMLEAK"
+NFT_TEST_HAVE_REALROOT="$NFT_TEST_HAVE_REALROOT"
+NFT_TEST_NO_UNSHARE="$NFT_TEST_NO_UNSHARE"
 DO_LIST_TESTS=
 
 TESTS=()
@@ -81,6 +80,12 @@  while [ $# -gt 0 ] ; do
 		-L|--list-tests)
 			DO_LIST_TESTS=y
 			;;
+		-R|--without-realroot)
+			NFT_TEST_HAVE_REALROOT=n
+			;;
+		-U|--no-unshare)
+			NFT_TEST_NO_UNSHARE=y
+			;;
 		--)
 			TESTS=("$@")
 			VERBOSE=y
@@ -108,6 +113,33 @@  if [ "$DO_LIST_TESTS" = y ] ; then
 	exit 0
 fi
 
+if [ "$NFT_TEST_HAVE_REALROOT" = "" ] ; then
+	# The caller didn't set NFT_TEST_HAVE_REALROOT and didn't specify
+	# -R/--without-root option. Autodetect it based on `id -u`.
+	export NFT_TEST_HAVE_REALROOT="$(test "$(id -u)" = "0" && echo y || echo n)"
+fi
+
+UNSHARE=()
+NFT_TEST_IS_UNSHARED=n
+if [ "$NFT_TEST_NO_UNSHARE" != y ]; then
+	# We unshare both if we NFT_TEST_HAVE_REALROOT and the rootless/unpriv
+	# case. Without real root, some tests may fail. Tests that don't work
+	# without real root should check for [ "$NFT_TEST_HAVE_REALROOT" != y ]
+	# and skip gracefully.
+	UNSHARE=( unshare -f -p --mount-proc -U --map-root-user -n )
+	if ! "${UNSHARE[@]}" true ; then
+		# Try without PID namespace.
+		UNSHARE=( unshare -f -U --map-root-user -n )
+		if ! "${UNSHARE[@]}" true ; then
+			msg_error "Unshare does not work. Rerun with -U/--no-unshare or NFT_TEST_NO_UNSHARE=y"
+		fi
+	fi
+	NFT_TEST_IS_UNSHARED=y
+fi
+
+# If they wish, test can check whether they run unshared.
+export NFT_TEST_IS_UNSHARED
+
 [ -z "$NFT" ] && NFT=$SRC_NFT
 ${NFT} > /dev/null 2>&1
 ret=$?
@@ -128,7 +160,9 @@  if [ ! -x "$DIFF" ] ; then
 fi
 
 kernel_cleanup() {
-	$NFT flush ruleset
+	if [ "$NFT_TEST_IS_UNSHARED" != y ] ; then
+		$NFT flush ruleset
+	fi
 	$MODPROBE -raq \
 	nft_reject_ipv4 nft_reject_bridge nft_reject_ipv6 nft_reject \
 	nft_redir_ipv4 nft_redir_ipv6 nft_redir \
@@ -253,7 +287,7 @@  for testfile in "${TESTS[@]}" ; do
 	kernel_cleanup
 
 	msg_info "[EXECUTING]	$testfile"
-	test_output=$(NFT="$NFT" DIFF=$DIFF ${testfile} 2>&1)
+	test_output=$(NFT="$NFT" DIFF=$DIFF "${UNSHARE[@]}" "$testfile" 2>&1)
 	rc_got=$?
 	echo -en "\033[1A\033[K" # clean the [EXECUTING] foobar line
 
@@ -312,4 +346,9 @@  check_kmemleak_force
 msg_info "results: [OK] $ok [FAILED] $failed [TOTAL] $((ok+failed))"
 
 kernel_cleanup
+
+if [ "$failed" -gt 0 -a "$NFT_TEST_HAVE_REALROOT" != y ] ; then
+	msg_info "test was not running as real root"
+fi
+
 [ "$failed" -eq 0 ]