tests: guard stop_wolfsshd so it cannot fail its caller

stop_wolfsshd killed $PID unconditionally. With the daemon already gone the
kill failed, and under "set -e" that aborted the caller -- in
sshd_forcedcmd_test.sh before PID was cleared, so the ForceCommand-SFTP
scenario was silently skipped, the EXIT trap killed the dead pid a second
time, and the script exited 1.

- Guard on a non-empty PID, ignore a failed kill and return 0, so the
  function is safe to call from an EXIT trap.
- Clear PID after stopping, so a second call cannot kill a recycled pid.
- Remove the temp key dir even when no daemon was recorded, so a daemon that
  failed to start does not leak it.
- Collapse sshd_forcedcmd_test.sh's cleanup() wrapper to a bare
  trap stop_wolfsshd EXIT now that the function guards itself.
- Check the cd back to the test directory in sshd_x509_upn_fail.sh; the log
  it counts after the client run is the one there.
pull/1187/head
John Safranek 2026-08-19 17:16:03 -07:00 committed by Paul Adelsbach
parent 8496451357
commit e7c8dc2c2c
3 changed files with 30 additions and 24 deletions

View File

@ -16,17 +16,12 @@ TEST_PORT="$2"
TEST_HOST="$1"
source ./start_sshd.sh
# Stop the daemon on every exit path. From the "set -e" below onward an aborted
# client run would otherwise leave a root daemon holding the shared test port,
# and every later test in the suite would talk to this config.
cleanup() {
if [ -n "$PID" ]; then
stop_wolfsshd
PID=""
fi
return 0
}
trap cleanup EXIT
# Stop the daemon on every exit path: the shell-login check below exits
# non-zero, and from the "set -e" onward an aborted client run would leave a
# root daemon holding the shared test port, so every later test in the suite
# would talk to this config. stop_wolfsshd clears PID, so this is a no-op after
# each explicit stop below.
trap stop_wolfsshd EXIT
cat <<EOF > sshd_config_test_forcedcmd
Port $TEST_PORT
@ -66,7 +61,6 @@ echo exit | $TEST_SFTP -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h $TEST_HOST -p
cd "$TESTDIR"
stop_wolfsshd
PID=""
# A configured ForceCommand that is not "internal-sftp" must still permit the
# SFTP subsystem. Only a certificate force-command denies file transfer, so a
@ -94,7 +88,6 @@ echo exit | $TEST_SFTP -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h $TEST_HOST -p
cd "$TESTDIR"
stop_wolfsshd
PID=""
exit 0

View File

@ -40,7 +40,9 @@ echo "$TEST_CLIENT -X -c 'pwd' -u $3 -i $PRIVATE_KEY -J $PUBLIC_KEY -A $CA_CERT
$TEST_CLIENT -X -c 'pwd' -u "$3" -i "$PRIVATE_KEY" -J "$PUBLIC_KEY" -A "$CA_CERT" -h "$1" -p "$2"
RESULT=$?
cd "$TESTDIR"
# Back to the test dir: the log counted below is the one here, so a failed cd
# would silently count matches in the repository root's log.txt instead.
cd "$TESTDIR" || exit 1
# Give the daemon child a moment to flush its rejection to the log.
sleep 1

View File

@ -100,22 +100,33 @@ EOF
printf "SSHD running on PID $PID\n"
}
# closes down the sshd session taking argument $1 as the PID of the session
# closes down the sshd session started by start_wolfsshd, using $PID.
# Idempotent and safe to call from an EXIT trap: with no daemon recorded there
# is nothing to kill, and neither an already-exited daemon nor a missing temp
# dir may become the caller's exit status under "set -e".
stop_wolfsshd() {
printf "Stopping SSHD, killing pid $PID\n"
sudo kill $PID
if [ -n "$PID" ]; then
printf "Stopping SSHD, killing pid $PID\n"
sudo kill $PID || true
# Wait for the process to actually exit so a subsequent start_wolfsshd on
# the same port doesn't race the listening socket's release (EADDRINUSE).
for i in $(seq 1 50); do
sudo kill -0 $PID 2>/dev/null || break
sleep 0.1
done
# Wait for the process to actually exit so a subsequent start_wolfsshd on
# the same port doesn't race the listening socket's release (EADDRINUSE).
for i in $(seq 1 50); do
sudo kill -0 $PID 2>/dev/null || break
sleep 0.1
done
# Cleared so a second call -- an EXIT trap after an explicit stop -- is
# a no-op rather than a kill of whatever pid has since been recycled.
PID=""
fi
# The temp dir is owned by the invoking user, so its root-owned key copies
# can be removed without sudo.
# can be removed without sudo. Done even when no daemon was recorded, so a
# daemon that failed to start does not leak it.
if [ -n "$SSHD_KEYDIR" ]; then
rm -rf "$SSHD_KEYDIR"
SSHD_KEYDIR=""
fi
return 0
}