From 8f502d4b97c9f52bfa2a97afe5c6ed05c327d58e Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Fri, 26 Jun 2026 18:16:14 +0900 Subject: [PATCH] wolfsshd: fail closed when a per-connection privilege drop fails --- .gitignore | 1 + apps/wolfsshd/auth.c | 26 +++ apps/wolfsshd/include.am | 13 ++ apps/wolfsshd/test/run_all_sshd_tests.sh | 3 +- apps/wolfsshd/test/sshd_privdrop_fail_test.sh | 161 ++++++++++++++++++ apps/wolfsshd/test/start_sshd.sh | 7 +- apps/wolfsshd/wolfsshd.c | 97 +++++------ 7 files changed, 246 insertions(+), 62 deletions(-) create mode 100755 apps/wolfsshd/test/sshd_privdrop_fail_test.sh diff --git a/.gitignore b/.gitignore index ff72dc839..ad5e90b02 100644 --- a/.gitignore +++ b/.gitignore @@ -68,6 +68,7 @@ examples/scpclient/wolfscp apps/wolfssh/wolfssh apps/wolfsshd/wolfsshd apps/wolfsshd/test/test_configuration +apps/wolfsshd/test/wolfsshd_faultinj apps/wolfsshd/test/log.txt apps/wolfsshd/test/sshd_config_* apps/wolfsshd/test/authorized_keys_test diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c index 793ee2dcc..ee2091cbe 100644 --- a/apps/wolfsshd/auth.c +++ b/apps/wolfsshd/auth.c @@ -93,8 +93,34 @@ #endif #if defined(WOLFSSHD_UNIT_TEST) && !defined(_WIN32) +#ifdef WOLFSSHD_FAULT_INJECT +/* Simulate a kernel privilege-drop failure when WOLFSSHD_FAULT_PRIVDROP is set + * in the environment, so the test suite can reach the fail-closed paths taken + * by the subsystem handlers when the drop to the authenticated user fails. */ +static int wsshd_setregid_fault(WGID_T rgid, WGID_T egid) +{ + if (getenv("WOLFSSHD_FAULT_PRIVDROP") != NULL) { + errno = EPERM; + return -1; + } + return setregid(rgid, egid); +} + +static int wsshd_setreuid_fault(WUID_T ruid, WUID_T euid) +{ + if (getenv("WOLFSSHD_FAULT_PRIVDROP") != NULL) { + errno = EPERM; + return -1; + } + return setreuid(ruid, euid); +} + +int (*wsshd_setregid_cb)(WGID_T, WGID_T) = wsshd_setregid_fault; +int (*wsshd_setreuid_cb)(WUID_T, WUID_T) = wsshd_setreuid_fault; +#else int (*wsshd_setregid_cb)(WGID_T, WGID_T) = setregid; int (*wsshd_setreuid_cb)(WUID_T, WUID_T) = setreuid; +#endif /* WOLFSSHD_FAULT_INJECT */ #endif struct WOLFSSHD_AUTH { diff --git a/apps/wolfsshd/include.am b/apps/wolfsshd/include.am index 60ddf8d0f..922e48e1d 100644 --- a/apps/wolfsshd/include.am +++ b/apps/wolfsshd/include.am @@ -21,4 +21,17 @@ apps_wolfsshd_test_test_configuration_DEPENDENCIES = src/libwolfssh.la # the getgrouplist buffer-growth/realloc loop under the sanitizers. apps_wolfsshd_test_test_configuration_CPPFLAGS = $(AM_CPPFLAGS) -DWOLFSSH_SSHD -DWOLFSSHD_UNIT_TEST -DWOLFSSHD_GROUP_LIST_INIT=1 -I$(srcdir)/apps/wolfsshd/ +# Same daemon as wolfsshd, but able to simulate a failing privilege drop when +# WOLFSSHD_FAULT_PRIVDROP is set. Never installed; used by +# sshd_privdrop_fail_test.sh to exercise the fail-closed subsystem paths. +noinst_PROGRAMS += apps/wolfsshd/test/wolfsshd_faultinj +apps_wolfsshd_test_wolfsshd_faultinj_SOURCES = apps/wolfsshd/wolfsshd.c \ + apps/wolfsshd/configuration.c \ + apps/wolfsshd/configuration.h \ + apps/wolfsshd/auth.c \ + apps/wolfsshd/auth.h +apps_wolfsshd_test_wolfsshd_faultinj_LDADD = src/libwolfssh.la +apps_wolfsshd_test_wolfsshd_faultinj_DEPENDENCIES = src/libwolfssh.la +apps_wolfsshd_test_wolfsshd_faultinj_CPPFLAGS = $(AM_CPPFLAGS) -DWOLFSSH_SSHD -DWOLFSSHD_UNIT_TEST -DWOLFSSHD_FAULT_INJECT -I$(srcdir)/apps/wolfsshd/ + endif BUILD_SSHD diff --git a/apps/wolfsshd/test/run_all_sshd_tests.sh b/apps/wolfsshd/test/run_all_sshd_tests.sh index 50fd68fea..298bf18a8 100755 --- a/apps/wolfsshd/test/run_all_sshd_tests.sh +++ b/apps/wolfsshd/test/run_all_sshd_tests.sh @@ -397,9 +397,10 @@ else run_test "sshd_empty_password_test.sh" run_strictmodes_negative_test run_test "sshd_login_grace_test.sh" + run_test "sshd_privdrop_fail_test.sh" else printf "Skipping tests that need to setup local SSHD\n" - SKIPPED=$((SKIPPED+5)) + SKIPPED=$((SKIPPED+6)) fi # these tests run with X509 sshd-config loaded diff --git a/apps/wolfsshd/test/sshd_privdrop_fail_test.sh b/apps/wolfsshd/test/sshd_privdrop_fail_test.sh new file mode 100755 index 000000000..eabd19a0d --- /dev/null +++ b/apps/wolfsshd/test/sshd_privdrop_fail_test.sh @@ -0,0 +1,161 @@ +#!/bin/bash + +# Regression test for the fail-closed privilege-drop paths in wolfsshd. +# +# When the per-connection drop to the authenticated user's uid/gid fails, the +# subsystem handlers must terminate the connection process. Previously they +# attempted a fallback that is a no-op under "UsePrivilegeSeparation no" and +# then returned, leaving HandleConnection to run wolfSSH_shutdown and up to ten +# wolfSSH_worker iterations while still at the daemon's elevated privilege +# level. +# +# The drop is forced to fail with the wolfsshd_faultinj build, which honors +# WOLFSSHD_FAULT_PRIVDROP. Every subsystem that drops privileges is driven: +# exec/shell (SHELL_Subsystem), sftp (SFTP_Subsystem) and scp (SCP_Subsystem). + +if [ -z "$1" ] || [ -z "$2" ]; then + echo "expecting host and port as arguments" + echo "./sshd_privdrop_fail_test.sh 127.0.0.1 22222" + exit 1 +fi + +PWD=`pwd` +USER=`whoami` +TEST_HOST="$1" +TEST_PORT="$2" + +FAULT_SSHD="./wolfsshd_faultinj" +if [ ! -x "$FAULT_SSHD" ]; then + echo "SKIP: $FAULT_SSHD not built" + exit 77 +fi + +if [ -f ./log.txt ]; then + sudo rm -rf log.txt +fi +touch log.txt + +TEST_CLIENT="../../../examples/client/client" +SFTP_CLIENT="../../../examples/sftpclient/wolfsftp" +SCP_CLIENT="../../../examples/scpclient/wolfscp" +PRIVATE_KEY="../../../keys/hansel-key-ecc.der" +PUBLIC_KEY="../../../keys/hansel-key-ecc.pub" + +# Small payload for the sftp/scp transfers. The connection dies at the failed +# drop long before any data moves, so the contents do not matter. +PAYLOAD="privdrop_payload.txt" +echo "privdrop" > "$PAYLOAD" + +source ./start_sshd.sh + +cat < sshd_config_test_privdrop +Port $TEST_PORT +Protocol 2 +LoginGraceTime 600 +PermitRootLogin yes +PasswordAuthentication yes +PermitEmptyPasswords no +UsePrivilegeSeparation no +UseDNS no +HostKey $PWD/../../../keys/server-key.pem +AuthorizedKeysFile $PWD/authorized_keys_test +EOF + +# Run the fault-injecting daemon so the drop to the authenticated user fails. +# "UsePrivilegeSeparation no" is the worst case: the old fallback did nothing +# there, so the handler would have carried on as root. +SSHD_BIN="$FAULT_SSHD" +SSHD_ENV="WOLFSSHD_FAULT_PRIVDROP=1" +export SSHD_BIN SSHD_ENV + +start_wolfsshd "sshd_config_test_privdrop" + +# Stop the daemon and drop the generated files on every exit path, including +# the failure exits below. log.txt is left in place for debugging, matching the +# other tests in this directory. +cleanup() { + stop_wolfsshd + # stop_wolfsshd kills the pid it captured at startup, which can be a + # wrapper rather than the daemon that outlived it. The fault binary is + # unique to this test, so clearing it by name cannot hit another daemon. + sudo pkill -f wolfsshd_faultinj > /dev/null 2>&1 + rm -f sshd_config_test_privdrop "$PAYLOAD" + return 0 +} +trap cleanup EXIT + +DEADLINE=30 + +# Drives one client and checks the daemon's behavior after the drop fails. The +# client is expected to lose its connection, so its exit status is not the +# assertion and it is killed if it outlives the deadline. +check_subsystem() { + LABEL="$1" + shift + + BEFORE=`grep -c "Error setting user ID" log.txt` + + "$@" > /dev/null 2>&1 & + CLIENT_PID=$! + + # Wait, bounded, for the drop error to be logged and the connection process + # to exit, leaving only the listening daemon. A process that has exited + # cannot log anything more, so the absence check below is not a race. + WAITED=0 + while [ "$WAITED" -lt "$DEADLINE" ]; do + AFTER=`grep -c "Error setting user ID" log.txt` + LIVE=`pgrep -f wolfsshd_faultinj | wc -l | tr -d ' '` + if [ "$AFTER" -gt "$BEFORE" ] && [ "$LIVE" -le 1 ]; then + break + fi + sleep 1 + WAITED=`expr $WAITED + 1` + done + + kill $CLIENT_PID > /dev/null 2>&1 + wait $CLIENT_PID > /dev/null 2>&1 + + # The drop must actually have failed, otherwise the rest proves nothing. + AFTER=`grep -c "Error setting user ID" log.txt` + if [ "$AFTER" -le "$BEFORE" ]; then + echo "FAIL: $LABEL never reached the privilege drop" + exit 1 + fi + + # A handler still alive here never terminated, which is the failure this + # test exists to catch. + if [ "$WAITED" -ge "$DEADLINE" ]; then + echo "FAIL: $LABEL connection process still running after ${DEADLINE}s" + exit 1 + fi + + # The fail-closed assertion. exit(1) means the connection process is gone, + # so HandleConnection can never log this. If it appears, the handler kept + # running after the failed drop at an un-dropped privilege level. + if grep -q "Attempting to close down connection" log.txt; then + echo "FAIL: $LABEL handler continued after a failed privilege drop" + exit 1 + fi + + printf " %s: connection process terminated on privilege-drop failure\n" \ + "$LABEL" +} + +# SHELL_Subsystem, via an exec session. +check_subsystem "exec" \ + "$TEST_CLIENT" -c 'echo privdrop' -u "$USER" -i "$PRIVATE_KEY" \ + -j "$PUBLIC_KEY" -h "$TEST_HOST" -p "$TEST_PORT" + +# SFTP_Subsystem. -g is a one-shot put, so the client cannot sit at a prompt. +check_subsystem "sftp" \ + "$SFTP_CLIENT" -u "$USER" -i "$PRIVATE_KEY" -j "$PUBLIC_KEY" \ + -g -l "$PAYLOAD" -r "/tmp/privdrop_remote_$$.txt" \ + -h "$TEST_HOST" -p "$TEST_PORT" + +# SCP_Subsystem. +check_subsystem "scp" \ + "$SCP_CLIENT" -u "$USER" -i "$PRIVATE_KEY" -j "$PUBLIC_KEY" \ + -S"$PWD/$PAYLOAD:." -H "$TEST_HOST" -p "$TEST_PORT" + +echo "PASS: all subsystems terminate on privilege-drop failure" +exit 0 diff --git a/apps/wolfsshd/test/start_sshd.sh b/apps/wolfsshd/test/start_sshd.sh index 2dfdc4fb1..960e4c8ef 100755 --- a/apps/wolfsshd/test/start_sshd.sh +++ b/apps/wolfsshd/test/start_sshd.sh @@ -81,8 +81,11 @@ EOF fi fi - # find a port - sudo ../wolfsshd -d -E ./log.txt -f "$CONFIG" + # Default to the normal daemon. A test can set SSHD_BIN to run a different + # build (e.g. ./wolfsshd_faultinj) and SSHD_ENV to pass it environment, + # which plain sudo would otherwise strip. + SSHD_BIN="${SSHD_BIN:-../wolfsshd}" + sudo env $SSHD_ENV "$SSHD_BIN" -d -E ./log.txt -f "$CONFIG" # set the PID of started sshd NEW_PID=`ps -e | grep wolfsshd | grep -oE "[0-9]+"` diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index 0776839d9..2e79bb404 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -601,12 +601,10 @@ static int SCP_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, if (wolfSSHD_AuthReducePermissionsUser(conn->auth, pPasswd->pw_uid, pPasswd->pw_gid) != WS_SUCCESS) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error setting user ID"); - if (wolfSSHD_AuthReducePermissions(conn->auth) != WS_SUCCESS) { - /* stop everything if not able to reduce permissions level */ - exit(1); - } - - return WS_FATAL_ERROR; + /* could not drop to the authenticated user; terminate the + * per-connection process rather than continue at a higher + * privilege level */ + exit(1); } #else /* impersonate the logged on user for file permissions */ @@ -720,12 +718,10 @@ static int SFTP_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, if (wolfSSHD_AuthReducePermissionsUser(conn->auth, pPasswd->pw_uid, pPasswd->pw_gid) != WS_SUCCESS) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error setting user ID"); - if (wolfSSHD_AuthReducePermissions(conn->auth) != WS_SUCCESS) { - /* stop everything if not able to reduce permissions level */ - exit(1); - } - - return WS_FATAL_ERROR; + /* could not drop to the authenticated user; terminate the + * per-connection process rather than continue at a higher + * privilege level */ + exit(1); } #else char r[MAX_PATH]; @@ -1430,29 +1426,23 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, if (dup2(stdinPipe[0], STDIN_FILENO) == -1) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error redirecting stdin pipe"); - if (wolfSSHD_AuthReducePermissions(conn->auth) != WS_SUCCESS) { - exit(1); - } - - return WS_FATAL_ERROR; + /* exit rather than return into the connection handler + * while still at a raised privilege level */ + exit(1); } if (dup2(stdoutPipe[1], STDOUT_FILENO) == -1) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error redirecting stdout pipe"); - if (wolfSSHD_AuthReducePermissions(conn->auth) != WS_SUCCESS) { - exit(1); - } - - return WS_FATAL_ERROR; + /* exit rather than return into the connection handler + * while still at a raised privilege level */ + exit(1); } if (dup2(stderrPipe[1], STDERR_FILENO) == -1) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error redirecting stderr pipe"); - if (wolfSSHD_AuthReducePermissions(conn->auth) != WS_SUCCESS) { - exit(1); - } - - return WS_FATAL_ERROR; + /* exit rather than return into the connection handler + * while still at a raised privilege level */ + exit(1); } } @@ -1460,47 +1450,36 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, if (wolfSSHD_AuthSetGroups(conn->auth, wolfSSH_GetUsername(ssh), pPasswd->pw_gid) != WS_SUCCESS) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error setting groups"); - if (wolfSSHD_AuthReducePermissions(conn->auth) != WS_SUCCESS) { - /* stop everything if not able to reduce permissions level */ - exit(1); - } - - return WS_FATAL_ERROR; + /* exit rather than return into the connection handler while + * still at a raised privilege level */ + exit(1); } rc = SetupChroot(usrConf); if (rc < 0) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error setting chroot"); - if (wolfSSHD_AuthReducePermissions(conn->auth) != WS_SUCCESS) { - /* stop everything if not able to reduce permissions level */ - exit(1); - } - - return WS_FATAL_ERROR; + /* exit rather than return into the connection handler while + * still at a raised privilege level */ + exit(1); } else if (rc == 1) { rc = chdir("/"); if (rc != 0) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error going to / after chroot"); - if (wolfSSHD_AuthReducePermissions(conn->auth) != WS_SUCCESS) { - /* stop everything if not able to reduce permissions level */ - exit(1); - } - - return WS_FATAL_ERROR; + /* exit rather than return into the connection handler + * while still at a raised privilege level */ + exit(1); } } if (wolfSSHD_AuthReducePermissionsUser(conn->auth, pPasswd->pw_uid, pPasswd->pw_gid) != WS_SUCCESS) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error setting user ID"); - if (wolfSSHD_AuthReducePermissions(conn->auth) != WS_SUCCESS) { - /* stop everything if not able to reduce permissions level */ - exit(1); - } - - return WS_FATAL_ERROR; + /* could not drop to the authenticated user; terminate this + * child rather than return into the connection handler at a + * higher privilege level */ + exit(1); } setenv("HOME", pPasswd->pw_dir, 1); @@ -1565,12 +1544,11 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, if (wolfSSHD_AuthReducePermissionsUser(conn->auth, pPasswd->pw_uid, pPasswd->pw_gid) != WS_SUCCESS) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error setting user ID"); - if (wolfSSHD_AuthReducePermissions(conn->auth) != WS_SUCCESS) { - /* stop everything if not able to reduce permissions level */ - exit(1); - } - - return WS_FATAL_ERROR; + /* could not drop to the authenticated user; kill the already + * forked shell child and terminate the per-connection process + * rather than continue at a higher privilege level */ + kill(childPid, SIGKILL); + exit(1); } sshFd = wolfSSH_get_fd(ssh); @@ -2176,7 +2154,8 @@ static void* HandleConnection(void* arg) #ifdef WOLFSSH_SHELL if (ret == WS_SUCCESS) { wolfSSH_Log(WS_LOG_INFO, "[SSHD] Entering new shell"); - SHELL_Subsystem(conn, ssh, pPasswd, usrConf, NULL); + ret = SHELL_Subsystem(conn, ssh, pPasswd, usrConf, + NULL); } #else wolfSSH_Log(WS_LOG_ERROR, @@ -2222,7 +2201,7 @@ static void* HandleConnection(void* arg) wolfSSH_Log(WS_LOG_INFO, "[SSHD] Entering exec session [%s]", wolfSSH_GetSessionCommand(ssh)); - SHELL_Subsystem(conn, ssh, pPasswd, usrConf, + ret = SHELL_Subsystem(conn, ssh, pPasswd, usrConf, wolfSSH_GetSessionCommand(ssh)); } #endif /* WOLFSSH_SHELL */