Harness fixes: ready-file budget and run scoping

apps/wolfsshd/test/create_sshd_config.sh

@@ -2,8 +2,14 @@
 
 PWD=`pwd`
 
+# $1 is the user to renew certificates for, $2 the port the daemon binds. The
+# port used to be written into all four configs as a literal, so even a caller
+# who set TEST_PORT could not move the daemon. Default it so a direct
+# invocation still produces a usable config.
+PORT=${2:-22222}
+
 cat <<EOF > sshd_config_test
-Port 22222
+Port $PORT
 Protocol 2
 LoginGraceTime 600
 PermitRootLogin yes
@@ -17,7 +23,7 @@ AuthorizedKeysFile $PWD/authorized_keys_test
 EOF
 
 cat <<EOF > sshd_config_test_mldsa
-Port 22222
+Port $PORT
 Protocol 2
 LoginGraceTime 600
 PermitRootLogin yes
@@ -43,7 +49,7 @@ if wolfssh_has FPKI; then
 fi
 
 cat <<EOF > sshd_config_test_x509
-Port 22222
+Port $PORT
 Protocol 2
 LoginGraceTime 600
 PermitRootLogin yes
@@ -60,7 +66,7 @@ $UPN_DOMAIN_GOOD
 EOF
 
 cat <<EOF > sshd_config_test_x509_upn_bad
-Port 22222
+Port $PORT
 Protocol 2
 LoginGraceTime 600
 PermitRootLogin yes

apps/wolfsshd/test/port_lease.sh

@@ -0,0 +1,126 @@
+#!/bin/bash
+
+# Port-block leases for one run of the wolfSSHd test suite.
+#
+# Sourced by run_all_sshd_tests.sh, which takes a block for its run, and by
+# sshd_port_lease_test.sh, which races several processes through these
+# functions to check that a block is never handed to two runs at once. Kept
+# apart from the runner so that test can load the allocator without running
+# the suite.
+
+: "${PORT_BLOCK_FIRST:=28000}"
+: "${PORT_BLOCK_SIZE:=8}"
+: "${PORT_BLOCK_COUNT:=64}"
+
+# True when something is already listening. A shell built without /dev/tcp
+# fails here exactly as a refused connection does, which degrades to taking
+# the pid-derived block unprobed -- still per run, just unverified.
+port_in_use() {
+    (exec 3<>"/dev/tcp/127.0.0.1/$1") 2>/dev/null
+}
+
+# Where blocks are claimed. A port is host wide, so its lease has to be too:
+# under $TMPDIR this was not, because TMPDIR is per user on macOS
+# (/var/folders/...) and sudo's env_reset drops it, so an invoking-user run and
+# a sudo run kept private pools and could lease the same block while each
+# believed it held it alone. A fixed path in /tmp is the one namespace both
+# see. Sticky and world writable like /tmp itself so both can create in it.
+# The sticky bit is not what keeps the two apart -- it still lets the pool's
+# own owner unlink another user's entry, and the first run to arrive creates
+# the pool -- so what keeps a live lease safe is the liveness test below.
+# Overridable only so sshd_port_lease_test.sh can race against a scratch pool.
+: "${PORT_LOCK_ROOT:=/tmp/wolfssh-sshd-ports}"
+
+port_lease_init() {
+    mkdir -p "$PORT_LOCK_ROOT" 2>/dev/null
+    chmod 1777 "$PORT_LOCK_ROOT" 2>/dev/null || true
+    if [ ! -d "$PORT_LOCK_ROOT" ]; then
+        echo "Error: cannot create the port lease directory $PORT_LOCK_ROOT."
+        return 1
+    fi
+    return 0
+}
+
+# A lease on a block is a directory named for the block and for the pid that
+# holds it. A run creates and removes only its own, so no run can delete a
+# lease another still believes it holds. Reclaiming a stale lease in place
+# could not manage that: whether it removed the directory or renamed it aside,
+# the act was authorized by an earlier read of the owner, so a second runner
+# that had read the same dead owner went on to displace the live claim the
+# first had just made, and both used the block. A lease whose owner is gone is
+# simply ignored instead, which costs a run killed before its teardown nothing
+# but a directory entry -- and any run may delete those, since a dead owner's
+# lease is one nothing is relying on.
+claim_port_block() {
+    local base mine d owner
+    base=$1
+    mine="$PORT_LOCK_ROOT/$base.$$"
+    mkdir "$mine" 2>/dev/null || return 1
+    # The block is ours only if no other live lease on it exists. Two runners
+    # that reach this at once each see the other and both stand down, which
+    # costs a block rather than handing one to both; the caller moves on to
+    # the next. Neither can see the other as absent: each creates its lease
+    # before it looks, so the later look always finds the earlier lease.
+    for d in "$PORT_LOCK_ROOT/$base".*; do
+        [ -d "$d" ] || continue
+        [ "$d" = "$mine" ] && continue
+        owner=${d##*.}
+        # "ps -p", not "kill -0": kill reports failure both for a pid that is
+        # gone and for one the caller may not signal, and those are opposite
+        # answers here. A non-root run reading a lease held by a live root run
+        # -- which is CI, where sshd-test.yml runs the suite under sudo and
+        # code-coverage.yml does not -- took the EPERM for "owner gone", swept
+        # the lease and took a block that run was using. ps -p selects by pid
+        # whatever owns it.
+        if ps -p "$owner" >/dev/null 2>&1; then
+            rmdir "$mine" 2>/dev/null
+            return 1
+        fi
+        rmdir "$d" 2>/dev/null
+    done
+    return 0
+}
+
+release_port_block() {
+    [ -n "$1" ] && rmdir "$PORT_LOCK_ROOT/$1.$$" 2>/dev/null
+    return 0
+}
+
+find_port_block() {
+    local start i n base busy
+    # Pid-derived so two runs rarely probe the same candidate first.
+    # PORT_BLOCK_START pins it for the self-test, which has to be able to aim
+    # the scan at a chosen block to exercise the --port skip below.
+    start=${PORT_BLOCK_START:-$(( $$ % PORT_BLOCK_COUNT ))}
+    for (( i = 0; i < PORT_BLOCK_COUNT; i++ )); do
+        base=$(( PORT_BLOCK_FIRST \
+            + ((start + i) % PORT_BLOCK_COUNT) * PORT_BLOCK_SIZE ))
+        # A --port inside the block would be handed to the shared daemon while
+        # the private offsets are still assigned from the same block, so the
+        # caller's own port could be one of them: --port 22303 against base
+        # 22300 put the host key test on the port the shared daemon had. Skip
+        # any block the requested port falls in and the two never overlap.
+        if [ -n "$TEST_PORT" ] \
+                && [ "$TEST_PORT" -ge "$base" ] 2>/dev/null \
+                && [ "$TEST_PORT" -lt $(( base + PORT_BLOCK_SIZE )) ]; then
+            continue
+        fi
+        claim_port_block "$base" || continue
+        busy=0
+        for (( n = 0; n < PORT_BLOCK_SIZE; n++ )); do
+            if port_in_use $(( base + n )); then
+                busy=1
+                break
+            fi
+        done
+        if [ "$busy" -eq 0 ]; then
+            printf '%s' "$base"
+            return 0
+        fi
+        # Claimed but unusable: something outside the suite holds a port in
+        # it. Give the lease back rather than sit on a block we cannot use.
+        release_port_block "$base"
+    done
+    return 1
+}
+

apps/wolfsshd/test/run_all_sshd_tests.sh

@@ -4,6 +4,7 @@ echo "Running all wolfSSHd tests"
 
 # Define an array of test cases
 test_cases=(
+ "sshd_port_lease_test.sh"
  "sshd_exec_test.sh"
  "sshd_term_size_test.sh"
  "sshd_large_sftp_test.sh"
@@ -64,6 +65,95 @@ while [[ "$#" -gt 0 ]]; do
     esac
 done
 
+# Ports for this run, not for this host. Every listener the suite starts used
+# to be on a fixed port -- 22222 for the shared daemon and one constant apiece
+# for the private daemons -- so two runs on one machine collided on the bind
+# and each failed somewhere unrelated to its own change. Take a block of
+# consecutive ports instead, starting at a pid-derived offset so two runs
+# rarely probe the same candidate, and step past a block already in use:
+#
+#   +0  shared wolfSSHd (TEST_PORT)    +3  host key ownership/symlink gate
+#   +1  StrictModes negative test      +4  OpenSSH certificate test
+#   +2  AuthorizedUPNDomains negative  +5  privilege-drop test
+#
+# The range deliberately starts above everything else in the repo that binds
+# a port: CI steps bind 22222, 22225 and 22226, and scripts/fwd-bulk.test
+# takes 22000 + attempt * 1000 + (pid % 1000) over six attempts, so it can
+# land anywhere in 22000-27999 and hard-fails if the port is taken. It picks
+# blindly, so only this side can stay out of the way.
+#
+# Not yet per run: sshd_term_close_test.sh counts "pgrep wolfsshd" before and
+# after its connection, sshd_sftp_idle_cpu_test.sh picks the first wolfsshd
+# that is new since its connection, and sshd_stdin_stall_test.sh sums CPU
+# ticks over every wolfsshd on the machine. All three read the whole process
+# table, so a second run's connection children can perturb them. Two runs at
+# once otherwise pass; these are the remaining single-run-per-host tests.
+. ./port_lease.sh
+port_lease_init || exit 1
+
+PORT_BASE=`find_port_block`
+if [ -z "$PORT_BASE" ]; then
+    echo "Error: no free block of $PORT_BLOCK_SIZE ports found starting at" \
+        "$PORT_BLOCK_FIRST."
+    exit 1
+fi
+
+# The port the local daemon binds and the generated configs carry. A caller
+# who passed --port gets that port: the local branch below used to overwrite
+# it with the constant, so the only option that looked like a way out of a
+# collision was silently discarded.
+LOCAL_PORT="${TEST_PORT:-$PORT_BASE}"
+STRICTMODES_PORT=$((PORT_BASE + 1))
+UPN_PORT=$((PORT_BASE + 2))
+HOSTKEY_PERM_PORT=$((PORT_BASE + 3))
+# Read by the two tests that start a daemon of their own. They are exported
+# rather than derived from the port passed to the test, so they stay inside the
+# probed block even when --port moves the shared daemon off it.
+export WOLFSSHD_TEST_PORT=$((PORT_BASE + 4))
+export WOLFSSHD_PRIVDROP_PORT=$((PORT_BASE + 5))
+
+# Registry of the daemons started during this run, appended to by
+# start_wolfsshd in every test script that sources start_sshd.sh. The exit
+# teardown at the bottom kills what is left in it, which is what makes the
+# teardown specific to this run.
+WOLFSSHD_TEST_PIDFILE=`mktemp 2>/dev/null` \
+    || WOLFSSHD_TEST_PIDFILE=`mktemp -t sshdpids`
+export WOLFSSHD_TEST_PIDFILE
+
+# Teardown safety net: the start/stop pairs below stop each daemon they start,
+# but background test daemons survive across CI steps that share this runner,
+# and a later step (the valgrind "memory after close down" check) binds a port
+# of its own. Make sure no daemon this run started lingers when the script
+# exits, and that the registry does not outlive it either.
+#
+# It is a trap, not a block at the bottom of the file, because the script exits
+# early on a bad --match, a setup failure, a daemon that will not start and
+# every test failure -- none of which would reach the bottom.
+#
+# Scoped to the pids in the registry, not to the wolfsshd name: "pkill -x
+# wolfsshd" here matched every other run's daemon too, so on a shared runner
+# whichever job finished first took down the other's. A port-matched pkill is
+# not an option for the shared daemon -- its port comes from its config file,
+# so it never appears on the command line to match against.
+#
+# USING_LOCAL_HOST is unset on the early exits that precede "source
+# ./start_sshd.sh", so stop_all_wolfsshd is never called before it is defined.
+# Every step ends in "|| true": a failing command in an EXIT trap becomes the
+# script's exit status, which would turn a passing run red.
+run_teardown() {
+    if [ "$USING_LOCAL_HOST" == 1 ]; then
+        # Before the sweep, and idempotent: this is what removes $SSHD_KEYDIR,
+        # the temp dir holding the rewritten config and the root-owned copies
+        # of the trust anchors. The early exits -- a daemon that will not
+        # start, a failed test -- never reach a stop of their own.
+        stop_wolfsshd || true
+        stop_all_wolfsshd || true
+    fi
+    rm -f "$WOLFSSHD_TEST_PIDFILE" || true
+    release_port_block "$PORT_BASE" || true
+}
+trap run_teardown EXIT
+
 TOTAL=0
 SKIPPED=0
 # Set as the last statement of each branch that runs tests, and checked before
@@ -96,7 +186,7 @@ fi
 # setup
 set -e
 ./create_authorized_test_file.sh
-./create_sshd_config.sh $USER
+./create_sshd_config.sh "$USER" "$LOCAL_PORT"
 set +e
 
 if [ ! -z "$TEST_HOST" ] && [ ! -z "$TEST_PORT" ]; then
@@ -105,9 +195,9 @@ if [ ! -z "$TEST_HOST" ] && [ ! -z "$TEST_PORT" ]; then
 else
     USING_LOCAL_HOST=1
     source ./start_sshd.sh
-    echo "Starting up local wolfSSHd for tests on 127.0.0.1:22222"
     TEST_HOST="127.0.0.1"
-    TEST_PORT="22222"
+    TEST_PORT="$LOCAL_PORT"
+    echo "Starting up local wolfSSHd for tests on $TEST_HOST:$TEST_PORT"
     start_wolfsshd "sshd_config_test"
     if [ -z "$PID" ]; then
         echo "Issue starting up wolfSSHd"
@@ -156,7 +246,7 @@ run_strictmodes_negative_test() {
     cp ../../../keys/server-key.pem strictmodes_hostkey.pem
     chmod 644 strictmodes_hostkey.pem
     cat <<EOF > sshd_config_test_strictmodes
-Port 22622
+Port $STRICTMODES_PORT
 StrictModes no
 UsePrivilegeSeparation no
 HostKey strictmodes_hostkey.pem
@@ -202,7 +292,7 @@ run_upn_unenforceable_negative_test() {
     cp ../../../keys/server-key.pem upn_hostkey.pem
     chmod 600 upn_hostkey.pem
     cat <<EOF > sshd_config_test_upn_nofpki
-Port 22623
+Port $UPN_PORT
 UsePrivilegeSeparation no
 HostKey upn_hostkey.pem
 Match User $USER
@@ -308,7 +398,7 @@ run_hostkey_perm_check() {
 
     HK_SSHD=../wolfsshd
     HK_KEY=../../../keys/server-key.pem
-    HK_PORT=22399
+    HK_PORT=$HOSTKEY_PERM_PORT
     if [ ! -x "$HK_SSHD" ] || [ ! -f "$HK_KEY" ]; then
         printf "SKIPPED\n"
         SKIPPED=$((SKIPPED+1))
@@ -447,6 +537,7 @@ if [[ -n "$MATCH" ]]; then
     RUN_COMPLETE=1
 else
     echo "Running all tests..."
+
     for test in "${test_cases[@]}"; do
         if [[ "$test" != "$EXCLUDE" ]]; then
             echo "Running test: $test"
@@ -546,18 +637,6 @@ else
     RUN_COMPLETE=1
 fi
 
-# Teardown safety net: the start/stop pairs above stop each daemon they start,
-# but background test daemons survive across CI steps that share this runner,
-# and a later step (the valgrind "memory after close down" check) binds the same
-# port 22222. Make sure no test daemon lingers when this script exits so that
-# step does not fail with "tcp bind failed". Harmless when nothing is running.
-# Match the process name, not the whole command line: "-f wolfsshd" also matches
-# this script when it is invoked by a path holding "wolfsshd", killing the run
-# before the check below and losing the summary.
-if [ "$USING_LOCAL_HOST" == 1 ]; then
-    sudo pkill -x wolfsshd 2>/dev/null || true
-fi
-
 if [ "$RUN_COMPLETE" != 1 ]; then
     printf "ERROR: test run aborted before all tests ran\n"
     exit 1

apps/wolfsshd/test/sshd_large_sftp_test.sh

@@ -38,23 +38,32 @@ if [ -z "$HOME_DIR" ] || [ "$HOME_DIR" = "/" ]; then
     echo "could not resolve a usable home directory for user '$USER'"
     exit 1
 fi
-REMOTE_FILE="$HOME_DIR/large-random-2.txt"
+# Both names carry this test's pid. The remote one has to: it lands in the
+# daemon user's home, which is one directory for the whole host however many
+# checkouts are running, so two runs uploaded to the same path and each then
+# compared its own local file against the other's upload -- "differ: byte 1"
+# on a pair of files that were both transferred correctly. The local name
+# follows for the same reason one checkout down.
+LOCAL_FILE="`pwd`/large-random.$$.txt"
+REMOTE_FILE="$HOME_DIR/large-random-2.$$.txt"
+
+# 4.4G apiece, so do not leave them behind on the paths that do not reach the
+# removals below: the transfer runs under "set -e" and the comparison can fail.
+trap 'rm -f "$LOCAL_FILE" "$REMOTE_FILE"' EXIT
 
 # create a large file with random data (larger than word32 max value)
-head -c 4400000010 < /dev/random > large-random.txt
+head -c 4400000010 < /dev/random > "$LOCAL_FILE"
 
 set -e
-echo "$TEST_SFTP_CLIENT -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -g -l large-random.txt -r $REMOTE_FILE -h \"$1\" -p \"$2\""
-$TEST_SFTP_CLIENT -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -g -l large-random.txt -r "$REMOTE_FILE" -h "$1" -p "$2"
+echo "$TEST_SFTP_CLIENT -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -g -l $LOCAL_FILE -r $REMOTE_FILE -h \"$1\" -p \"$2\""
+$TEST_SFTP_CLIENT -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -g -l "$LOCAL_FILE" -r "$REMOTE_FILE" -h "$1" -p "$2"
 
-cmp large-random.txt "$REMOTE_FILE"
+cmp "$LOCAL_FILE" "$REMOTE_FILE"
 RESULT=$?
 if [ "$RESULT" != "0" ]; then
     echo "files did not match when compared"
     exit 1
 fi
-rm -f large-random.txt
-rm -f "$REMOTE_FILE"
 
 set +e