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.

Author: ejohnstown

apps/wolfsshd/test/sshd_forcedcmd_test.sh

@@ -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
 
 

apps/wolfsshd/test/sshd_x509_upn_fail.sh

@@ -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

apps/wolfsshd/test/start_sshd.sh

@@ -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
 }