diff --git a/apps/wolfsshd/test/create_sshd_config.sh b/apps/wolfsshd/test/create_sshd_config.sh index ebd65a0ac..a4af64ba7 100755 --- a/apps/wolfsshd/test/create_sshd_config.sh +++ b/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 < sshd_config_test -Port 22222 +Port $PORT Protocol 2 LoginGraceTime 600 PermitRootLogin yes @@ -17,7 +23,7 @@ AuthorizedKeysFile $PWD/authorized_keys_test EOF cat < 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 < sshd_config_test_x509 -Port 22222 +Port $PORT Protocol 2 LoginGraceTime 600 PermitRootLogin yes @@ -60,7 +66,7 @@ $UPN_DOMAIN_GOOD EOF cat < sshd_config_test_x509_upn_bad -Port 22222 +Port $PORT Protocol 2 LoginGraceTime 600 PermitRootLogin yes diff --git a/apps/wolfsshd/test/port_lease.sh b/apps/wolfsshd/test/port_lease.sh new file mode 100755 index 000000000..317bae182 --- /dev/null +++ b/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 +} + diff --git a/apps/wolfsshd/test/run_all_sshd_tests.sh b/apps/wolfsshd/test/run_all_sshd_tests.sh index 78c76f463..7f7afe2ab 100755 --- a/apps/wolfsshd/test/run_all_sshd_tests.sh +++ b/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 < 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 < 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 diff --git a/apps/wolfsshd/test/sshd_large_sftp_test.sh b/apps/wolfsshd/test/sshd_large_sftp_test.sh index 4f177a375..a87ce4776 100755 --- a/apps/wolfsshd/test/sshd_large_sftp_test.sh +++ b/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 diff --git a/apps/wolfsshd/test/sshd_ossh_cert_test.sh b/apps/wolfsshd/test/sshd_ossh_cert_test.sh index 5fce73c27..da76b0534 100755 --- a/apps/wolfsshd/test/sshd_ossh_cert_test.sh +++ b/apps/wolfsshd/test/sshd_ossh_cert_test.sh @@ -49,7 +49,18 @@ command -v ssh-keygen >/dev/null 2>&1 || \ skip "ssh-keygen not found, skipping OpenSSH cert test" WORK=$(mktemp -d) -trap 'pkill -f "wolfsshd .*sshd_config_ossh" 2>/dev/null; rm -rf "$WORK"' EXIT +# Checked before the trap below is installed: an empty WORK would reduce its +# pattern to "wolfsshd .*", which matches every wolfsshd on the machine and is +# exactly the host-wide teardown this suite no longer does. +if [ -z "$WORK" ] || [ ! -d "$WORK" ]; then + echo "FAIL: could not create a work directory for the OpenSSH cert test" + exit 1 +fi +# Matched on $WORK, this run's own mktemp dir, not on the config basename: +# "sshd_config_ossh" appears in every concurrent run's command line too, +# so the basename pattern tore down another run's daemon along with this +# one's. $WORK is expanded when the trap fires, by which point it is set. +trap 'pkill -f "wolfsshd .*$WORK" 2>/dev/null; rm -rf "$WORK"' EXIT # Under sudo the daemon session runs as the login user: let it traverse $WORK # and own the marker dir (not world-writable, so no other user can fake a PASS). @@ -65,11 +76,19 @@ HOSTKEY="$WORK/hostkey.pem" cp "$ROOT/keys/server-key.pem" "$HOSTKEY" chmod 600 "$HOSTKEY" -# Issue certificates bound to the login user (and the negatives). The -# force-command marker is placed under the per-run work dir, not a fixed, -# world-readable /tmp path. +# Issue certificates bound to the login user (and the negatives), into this +# run's own directory. Written into keys/ they carried only the login user in +# their names, so two runs overwrote each other's: both principals matched, +# and the force-command certificate names the marker directory of whichever +# run issued it last, so a run could authenticate with the other's certificate +# and then fail its own marker assertion. Only the keypairs are shared, and +# those are committed. The force-command marker likewise goes under the +# per-run work dir, not a fixed, world-readable /tmp path. +CERTS="$WORK/certs" +mkdir -p "$CERTS" +chmod 755 "$CERTS" ( cd "$ROOT/keys" && OSSH_FORCED_MARKER="$MARKERDIR/forced_marker" \ - ./renew-ossh-certs.sh "$LOGINUSER" ) + OSSH_CERT_DIR="$CERTS" ./renew-ossh-certs.sh "$LOGINUSER" ) # Trust all three signing CAs (Ed25519, RSA, ECDSA) but not ossh-bad-ca. cat "$ROOT/keys/ossh-ca.pub" "$ROOT/keys/ossh-ca-rsa.pub" \ @@ -121,7 +140,7 @@ connect_ssh() { # user-key cert remote-command # (re)start the daemon, drive the selected client, return its exit code. attempt() { # user-key cert [remote-command] - pkill -f "wolfsshd .*sshd_config_ossh" 2>/dev/null + pkill -f "wolfsshd .*$WORK" 2>/dev/null sleep 1 "$WOLFSSHD" -D -f "$CONFIG" -E "$WORK/sshd.log" & local wp=$! @@ -175,7 +194,7 @@ echo "scp payload" > "$SCPSRC" # (re)start the daemon, leaving its PID in DPID. start_daemon() { - pkill -f "wolfsshd .*sshd_config_ossh" 2>/dev/null + pkill -f "wolfsshd .*$WORK" 2>/dev/null sleep 1 "$WOLFSSHD" -D -f "$CONFIG" -E "$WORK/sshd.log" & DPID=$! @@ -245,38 +264,38 @@ scp_check() { # label user-key cert expect(0=allowed,1=denied) run_suite() { # driver DRIVER=$1 echo "OpenSSH cert test via $DRIVER client (user=$LOGINUSER, port=$PORT):" - check "valid cert" "$ED" "$ROOT/keys/$LOGINUSER-ossh-cert.pub" 0 - check "RSA CA" "$ED" "$ROOT/keys/$LOGINUSER-ossh-rsaca-cert.pub" 0 - check "ECDSA CA" "$ED" "$ROOT/keys/$LOGINUSER-ossh-ecdsaca-cert.pub" 0 - check "RSA user key" "$RSA" "$ROOT/keys/$LOGINUSER-ossh-rsauser-cert.pub" 0 - check "ECDSA user key" "$ECC" "$ROOT/keys/$LOGINUSER-ossh-ecdsauser-cert.pub" 0 - check "untrusted CA" "$ED" "$ROOT/keys/$LOGINUSER-ossh-badca-cert.pub" 1 - check "wrong principal" "$ED" "$ROOT/keys/$LOGINUSER-ossh-wrongprincipal-cert.pub" 1 - check "empty principal" "$ED" "$ROOT/keys/$LOGINUSER-ossh-noprincipal-cert.pub" 1 - check "unknown crit opt" "$ED" "$ROOT/keys/$LOGINUSER-ossh-unkcrit-cert.pub" 1 - check "source-addr match" "$ED" "$ROOT/keys/$LOGINUSER-ossh-srcok-cert.pub" 0 - check "source-addr deny" "$ED" "$ROOT/keys/$LOGINUSER-ossh-srcbad-cert.pub" 1 - check "expired cert" "$ED" "$ROOT/keys/$LOGINUSER-ossh-expired-cert.pub" 1 - force_command_check "$ED" "$ROOT/keys/$LOGINUSER-ossh-forcecmd-cert.pub" + check "valid cert" "$ED" "$CERTS/$LOGINUSER-ossh-cert.pub" 0 + check "RSA CA" "$ED" "$CERTS/$LOGINUSER-ossh-rsaca-cert.pub" 0 + check "ECDSA CA" "$ED" "$CERTS/$LOGINUSER-ossh-ecdsaca-cert.pub" 0 + check "RSA user key" "$RSA" "$CERTS/$LOGINUSER-ossh-rsauser-cert.pub" 0 + check "ECDSA user key" "$ECC" "$CERTS/$LOGINUSER-ossh-ecdsauser-cert.pub" 0 + check "untrusted CA" "$ED" "$CERTS/$LOGINUSER-ossh-badca-cert.pub" 1 + check "wrong principal" "$ED" "$CERTS/$LOGINUSER-ossh-wrongprincipal-cert.pub" 1 + check "empty principal" "$ED" "$CERTS/$LOGINUSER-ossh-noprincipal-cert.pub" 1 + check "unknown crit opt" "$ED" "$CERTS/$LOGINUSER-ossh-unkcrit-cert.pub" 1 + check "source-addr match" "$ED" "$CERTS/$LOGINUSER-ossh-srcok-cert.pub" 0 + check "source-addr deny" "$ED" "$CERTS/$LOGINUSER-ossh-srcbad-cert.pub" 1 + check "expired cert" "$ED" "$CERTS/$LOGINUSER-ossh-expired-cert.pub" 1 + force_command_check "$ED" "$CERTS/$LOGINUSER-ossh-forcecmd-cert.pub" # A force-command must not be bypassed by requesting the SFTP subsystem. # "internal-sftp" still permits SFTP; any other force-command denies it. if sftp_available; then sftp_check "valid cert sftp" "$ED" \ - "$ROOT/keys/$LOGINUSER-ossh-cert.pub" 0 + "$CERTS/$LOGINUSER-ossh-cert.pub" 0 sftp_check "forcecmd sftp deny" "$ED" \ - "$ROOT/keys/$LOGINUSER-ossh-forcecmd-cert.pub" 1 + "$CERTS/$LOGINUSER-ossh-forcecmd-cert.pub" 1 sftp_check "internal-sftp sftp" "$ED" \ - "$ROOT/keys/$LOGINUSER-ossh-internalsftp-cert.pub" 0 + "$CERTS/$LOGINUSER-ossh-internalsftp-cert.pub" 0 # A configured ForceCommand is not a certificate force-command: on its # own it must not deny SFTP, and it must not mask one carried by a # certificate. CONFIG="$WORK/sshd_config_ossh_fc" sftp_check "config forcecmd sftp" "$ED" \ - "$ROOT/keys/$LOGINUSER-ossh-cert.pub" 0 + "$CERTS/$LOGINUSER-ossh-cert.pub" 0 sftp_check "config+cert sftp deny" "$ED" \ - "$ROOT/keys/$LOGINUSER-ossh-forcecmd-cert.pub" 1 + "$CERTS/$LOGINUSER-ossh-forcecmd-cert.pub" 1 CONFIG="$WORK/sshd_config_ossh" else echo " (sftp $DRIVER client unavailable, skipping sftp cases)" @@ -287,11 +306,11 @@ run_suite() { # driver # only; the system "scp" uses the SFTP protocol and is covered above. if [ "$DRIVER" = client ]; then scp_check "valid cert scp" "$ED" \ - "$ROOT/keys/$LOGINUSER-ossh-cert.pub" 0 + "$CERTS/$LOGINUSER-ossh-cert.pub" 0 scp_check "forcecmd scp deny" "$ED" \ - "$ROOT/keys/$LOGINUSER-ossh-forcecmd-cert.pub" 1 + "$CERTS/$LOGINUSER-ossh-forcecmd-cert.pub" 1 scp_check "internal-sftp scp" "$ED" \ - "$ROOT/keys/$LOGINUSER-ossh-internalsftp-cert.pub" 1 + "$CERTS/$LOGINUSER-ossh-internalsftp-cert.pub" 1 fi } diff --git a/apps/wolfsshd/test/sshd_port_lease_test.sh b/apps/wolfsshd/test/sshd_port_lease_test.sh new file mode 100755 index 000000000..6ad574207 --- /dev/null +++ b/apps/wolfsshd/test/sshd_port_lease_test.sh @@ -0,0 +1,256 @@ +#!/bin/bash + +# Self-test for the port-block allocator in port_lease.sh. +# +# The allocator is what keeps two runs of this suite on one machine off each +# other's ports, and its failure mode is a race: nothing goes wrong until two +# runners reach the same code at the same moment, and what goes wrong then is +# a bind collision in some unrelated test much later. Reasoning about it is +# not enough -- an earlier attempt at the stale-lease path looked correct and +# handed one block to eleven runners at once. So each case here forks real +# processes and contends for real leases. +# +# No daemon, no root and no network listener: the pool is a scratch directory +# and the workers only claim and hold. Takes a few seconds. + +TESTDIR=`pwd` +cd "$TESTDIR" || exit 1 + +# Worker modes, re-entered by the racing processes below. A winner holds its +# lease until the parent releases it, as a real run holds one for its whole +# life. Holding matters: a worker that exited the moment it won would leave a +# lease whose owner is dead, which every later worker is then entitled to +# ignore, and the test would pass no matter how broken the allocator was. +# +# Released by the parent rather than after a fixed sleep. A sleep is a guess +# about how far apart forty forked shells reach the claim, and on a loaded +# runner the stragglers arrived after it expired, correctly found a dead owner +# and correctly took the block -- counted as a second winner and reported as a +# failure of the allocator. $4 is the sentinel, $5 the file every worker +# reports to whether it won or lost. +if [ "$1" = "--claim-and-hold" ] || [ "$1" = "--find-and-hold" ]; then + . ./port_lease.sh + won="" + if [ "$1" = "--claim-and-hold" ]; then + claim_port_block "$2" && won="$2" + else + won=`find_port_block` || won="" + fi + [ -n "$won" ] && printf '%s\n' "$won" >> "$3" + printf 'x\n' >> "$5" + while [ -n "$won" ] && [ -e "$4" ]; do + sleep 0.2 + done + exit 0 +fi +if [ "$1" = "--claim-once" ]; then + . ./port_lease.sh + claim_port_block "$2" + exit $? +fi + +PASS=0 +FAIL=0 + +ok() { printf " %-44s PASS\n" "$1"; PASS=$((PASS + 1)); } +bad() { printf " %-44s *** FAIL (%s)\n" "$1" "$2"; FAIL=$((FAIL + 1)); } + +POOL=`mktemp -d 2>/dev/null` || POOL=`mktemp -d -t portlease` +if [ -z "$POOL" ] || [ ! -d "$POOL" ]; then + echo "could not create a scratch pool directory" + exit 1 +fi +trap 'rm -rf "$POOL"' EXIT +chmod 1777 "$POOL" +export PORT_LOCK_ROOT="$POOL" + +# find_port_block probes each candidate port for real, and this test runs with +# the suite's own daemon up. Left on the live range the probes connected to it, +# making it fork a child and log a line per worker. Contend over a range +# nothing in the tree binds instead -- and below 32768, out of the ephemeral +# range Linux allocates source ports from, where a passing connection of the +# host's own could answer a probe and move the result. +export PORT_BLOCK_FIRST=29000 + +WON="$POOL/won.txt" +SENTINEL="$POOL/hold" +DONE="$POOL/done.txt" + +# A pid that is certainly not running: fork one and reap it. Used to plant a +# lease whose owner is gone, the state a run killed before its teardown leaves. +dead_pid() { + ( exit 0 ) & + dp=$! + wait "$dp" 2>/dev/null + printf '%s' "$dp" +} + +race() { # mode arg workers + : > "$WON" + : > "$DONE" + : > "$SENTINEL" + i=0 + while [ "$i" -lt "$3" ]; do + ./sshd_port_lease_test.sh "$1" "$2" "$WON" "$SENTINEL" "$DONE" & + i=$((i + 1)) + done + # Let the winners go only once every worker has had its turn, so a lease + # is never released while another is still starting up. Bounded so a + # worker that died without reporting cannot hang the suite. + i=0 + while [ "`grep -c . "$DONE" 2>/dev/null`" -lt "$3" ] && [ "$i" -lt 300 ]; do + sleep 0.2 + i=$((i + 1)) + done + rm -f "$SENTINEL" + wait +} + +echo "Port lease allocator test:" + +# The race the lease scheme exists to lose safely: many runners arrive at one +# block whose recorded owner is gone. Reclaiming it in place let several of +# them each conclude they had taken it. The bound is an upper one on purpose: +# with forty contenders all standing down for each other, nobody taking the +# block is the documented outcome, not a defect. Progress is asserted by the +# parallel trial below, where a loser moves on to the next block. +T=0 +while [ "$T" -lt 5 ]; do + T=$((T + 1)) + rm -rf "$POOL"/* + mkdir -p "$POOL/22400.`dead_pid`" + race --claim-and-hold 22400 40 + n=`grep -c . "$WON"` + if [ "$n" -le 1 ]; then + ok "contended stale block, trial $T ($n winner)" + else + bad "contended stale block, trial $T" "$n runners hold one block" + fi +done + +# Whole allocations in parallel, the way several suites starting at once do +# it. Every runner must come away with a block of its own. +T=0 +while [ "$T" -lt 3 ]; do + T=$((T + 1)) + rm -rf "$POOL"/* + race --find-and-hold "" 20 + n=`grep -c . "$WON"` + u=`sort -u "$WON" | grep -c .` + # Distinct is the safety property, every worker getting one is the + # progress property, and only the pair means anything: an allocator that + # refused everyone satisfies "all distinct" on an empty set. 64 free + # blocks for 20 workers, so anything short of 20 is a defect. + if [ "$n" -eq 20 ] && [ "$n" -eq "$u" ]; then + ok "parallel allocation, trial $T ($n blocks, all distinct)" + else + bad "parallel allocation, trial $T" "$n of 20 allocated, $u distinct" + fi +done + +# A block whose owner is gone has to come back into circulation, or a run that +# was killed would retire one permanently. +rm -rf "$POOL"/* +STALE=`dead_pid` +mkdir -p "$POOL/22400.$STALE" +( . ./port_lease.sh; claim_port_block 22400 ) && R=0 || R=1 +if [ "$R" -eq 0 ] && [ ! -d "$POOL/22400.$STALE" ]; then + ok "stale lease reclaimed, not retired" +else + bad "stale lease reclaimed, not retired" "claim rc=$R" +fi + +# A live lease is never taken, however many ask for it. +rm -rf "$POOL"/* +: > "$DONE" +: > "$SENTINEL" +./sshd_port_lease_test.sh --claim-and-hold 22400 "$WON" "$SENTINEL" "$DONE" & +HOLDER=$! +i=0 +while [ "`grep -c . "$DONE" 2>/dev/null`" -lt 1 ] && [ "$i" -lt 300 ]; do + sleep 0.2 + i=$((i + 1)) +done +( . ./port_lease.sh; claim_port_block 22400 ) && R=0 || R=1 +if [ "$R" -eq 1 ]; then + ok "live lease not stolen" +else + bad "live lease not stolen" "second claim succeeded" +fi +rm -f "$SENTINEL" +wait "$HOLDER" 2>/dev/null + +# A lease held by a live process of another user must read as live. This is +# the case the pool exists for -- CI runs the suite under sudo in one workflow +# and as the invoking user in another -- and the one a same-user test cannot +# reach: only a caller that may not signal the owner sees the difference +# between "gone" and "not mine". pid 1 always runs and never takes a signal +# from a non-root caller, so it stands in for the other run's pid. +# +# Under sudo, which is how the suite runs, the claim has to drop back to the +# invoking user or the check is vacuous -- root can signal anything. +LEASE_AS="" +if [ "`id -u`" -eq 0 ]; then + [ -n "$SUDO_USER" ] && LEASE_AS="$SUDO_USER" +fi +if [ "`id -u`" -ne 0 ] || [ -n "$LEASE_AS" ]; then + rm -rf "$POOL"/* + mkdir -p "$POOL/22400.1" + if [ -n "$LEASE_AS" ]; then + sudo -u "$LEASE_AS" env PORT_LOCK_ROOT="$POOL" \ + PORT_BLOCK_FIRST="$PORT_BLOCK_FIRST" \ + ./sshd_port_lease_test.sh --claim-once 22400 && R=0 || R=1 + else + ./sshd_port_lease_test.sh --claim-once 22400 && R=0 || R=1 + fi + if [ "$R" -eq 1 ] && [ -d "$POOL/22400.1" ]; then + ok "live owner this run cannot signal is kept" + else + [ -d "$POOL/22400.1" ] && swept=no || swept=yes + bad "live owner this run cannot signal is kept" \ + "claim rc=$R, lease swept=$swept" + fi +else + printf " %-44s SKIPPED (root, no SUDO_USER)\n" \ + "live owner this run cannot signal is kept" +fi + +# A released block is immediately reusable, so a run that gives back a block it +# cannot use does not burn it. +rm -rf "$POOL"/* +( . ./port_lease.sh + claim_port_block 22400 || exit 1 + release_port_block 22400 + claim_port_block 22400 || exit 1 ) && R=0 || R=1 +if [ "$R" -eq 0 ]; then + ok "released block is reusable" +else + bad "released block is reusable" "reclaim after release failed" +fi + +# --port has to land outside the block the private daemons are assigned from: +# the requested port is the shared daemon's, and the offsets come from the +# block, so a block containing it would put two daemons on one port. +# The scan is aimed at the first block so the skip is the branch under test. +# Left to the pid-derived origin the scan almost never reached the block +# holding the port, and the cases passed without executing the skip at all. +# The assertion is the contract -- the block does not contain the port -- not +# a particular base. With the scan pinned to the block holding the port, the +# skip is the only way past it, so this still fails if the skip goes; naming +# 29008 instead would also fail whenever a busy port moved the scan one block +# further, on a test that aborts the whole suite. +for p in 29000 29003 29007; do + rm -rf "$POOL"/* + base=`( . ./port_lease.sh; PORT_BLOCK_START=0 TEST_PORT=$p find_port_block )` + if [ -z "$base" ]; then + bad "--port $p skips its block" "no block allocated" + elif [ "$p" -ge "$base" ] && [ "$p" -lt $((base + 8)) ]; then + bad "--port $p skips its block" "block $base contains it" + else + ok "--port $p skips its block (got $base)" + fi +done + +printf "Port lease allocator test: %d passed, %d failed\n" "$PASS" "$FAIL" +[ "$FAIL" -eq 0 ] || exit 1 +exit 0 diff --git a/apps/wolfsshd/test/sshd_privdrop_fail_test.sh b/apps/wolfsshd/test/sshd_privdrop_fail_test.sh index 67623a075..96f3082e5 100755 --- a/apps/wolfsshd/test/sshd_privdrop_fail_test.sh +++ b/apps/wolfsshd/test/sshd_privdrop_fail_test.sh @@ -7,8 +7,9 @@ # Drives all three dropping subsystems: exec/shell, sftp, scp. if [ -z "$1" ] || [ -z "$2" ]; then - echo "expecting host and port as arguments" - echo "./sshd_privdrop_fail_test.sh 127.0.0.1 22222" + echo "expecting host and the runner's shared port as arguments;" + echo "this test binds WOLFSSHD_PRIVDROP_PORT, or that port + 5" + echo "./sshd_privdrop_fail_test.sh 127.0.0.1 22300" exit 1 fi @@ -19,7 +20,10 @@ USER=`whoami` TEST_HOST="$1" # Own daemon on a dedicated port for isolation from the runner's shared daemon. -TEST_PORT="22822" +# The runner reserves this port as part of the block it probed and exports it, +# so the isolation holds against another run of the suite on the same host. +# Standalone, fall back to an offset from the port passed in. +TEST_PORT="${WOLFSSHD_PRIVDROP_PORT:-$(( $2 + 5 ))}" SSHD_BIN="../wolfsshd" if [ ! -x "$SSHD_BIN" ]; then diff --git a/apps/wolfsshd/test/sshd_term_size_test.sh b/apps/wolfsshd/test/sshd_term_size_test.sh index 2895996dd..806766880 100755 --- a/apps/wolfsshd/test/sshd_term_size_test.sh +++ b/apps/wolfsshd/test/sshd_term_size_test.sh @@ -15,6 +15,12 @@ if [ -z "$1" ] || [ -z "$2" ]; then exit 1 fi +# The tmux session name is shared across everything this user runs, so it is +# derived from the port this run was given rather than being the constant +# "test": two concurrent runs of the suite would otherwise fight over one +# session and the EXIT trap below would kill the other run's. +TMUX_SESSION="wolfsshd_test_$2" + # Check if tmux is available which tmux RESULT=$? @@ -25,20 +31,20 @@ fi # tear down the tmux session on any exit, so a timeout failure does not # leave a stale session that breaks the next run with "duplicate session" -trap 'tmux kill-session -t test 2>/dev/null || true' EXIT +trap 'tmux kill-session -t "$TMUX_SESSION" 2>/dev/null || true' EXIT # Wait until the remote shell produces some output (i.e. a prompt), so the # SSH session is known to be up before keys are sent to it. CI runners can # take several seconds to get through key exchange and login. wait_for_session() { for _ in $(seq 1 10); do - if tmux capture-pane -p -t test | grep -q '[^[:space:]]'; then + if tmux capture-pane -p -t "$TMUX_SESSION" | grep -q '[^[:space:]]'; then return 0 fi sleep 1 done echo "Timed out waiting for SSH session output" - tmux capture-pane -p -t test + tmux capture-pane -p -t "$TMUX_SESSION" return 1 } @@ -50,30 +56,30 @@ get_size_line() { # Re-send the query each pass in case the shell was not yet ready to # read input on an earlier pass; tail -n 1 below tolerates the extra # numeric line a repeat can produce. - tmux send-keys -t test 'echo;echo $COLUMNS $LINES;echo' - tmux send-keys -t test 'ENTER' + tmux send-keys -t "$TMUX_SESSION" 'echo;echo $COLUMNS $LINES;echo' + tmux send-keys -t "$TMUX_SESSION" 'ENTER' sleep 1 - SIZE_LINE=$(tmux capture-pane -p -t test | tr -d '\r' | \ + SIZE_LINE=$(tmux capture-pane -p -t "$TMUX_SESSION" | tr -d '\r' | \ grep -E '^[0-9]+[[:space:]]+[0-9]+[[:space:]]*$' | tail -n 1) if [ -n "$SIZE_LINE" ]; then return 0 fi done echo "Timed out waiting for terminal size output" - tmux capture-pane -p -t test + tmux capture-pane -p -t "$TMUX_SESSION" return 1 } echo "Creating tmux session at $PWD with command :" -echo "tmux new-session -d -s test \"$TEST_CLIENT -q -t -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h \"$1\" -p \"$2\"\"" -tmux new-session -d -s test "$TEST_CLIENT -q -t -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h \"$1\" -p \"$2\"" +echo "tmux new-session -d -s $TMUX_SESSION \"$TEST_CLIENT -q -t -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h \"$1\" -p \"$2\"\"" +tmux new-session -d -s "$TMUX_SESSION" "$TEST_CLIENT -q -t -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h \"$1\" -p \"$2\"" echo "Result of tmux new-session = $?" wait_for_session || exit 1 -COL=`tmux display -p -t test '#{pane_width}'` -ROW=`tmux display -p -t test '#{pane_height}'` -echo "tmux 'test' session has COL = ${COL} and ROW = ${ROW}" +COL=`tmux display -p -t "$TMUX_SESSION" '#{pane_width}'` +ROW=`tmux display -p -t "$TMUX_SESSION" '#{pane_height}'` +echo "tmux $TMUX_SESSION session has COL = ${COL} and ROW = ${ROW}" # get the terminals columns and lines get_size_line || exit 1 @@ -94,20 +100,20 @@ fi # resize tmux after connection is open is not working @TODO #tmux set-window-option -g aggressive-resize #printf '\e[8;50;100t' -#tmux resize-pane -x 50 -y 10 -t test +#tmux resize-pane -x 50 -y 10 -t "$TMUX_SESSION" # close down the SSH session -tmux send-keys -t test 'exit' -tmux send-keys -t test 'ENTER' +tmux send-keys -t "$TMUX_SESSION" 'exit' +tmux send-keys -t "$TMUX_SESSION" 'ENTER' # kill off the session if it's still running, but don't error out if the session # has already closed down -tmux kill-session -t test +tmux kill-session -t "$TMUX_SESSION" set -e echo "Starting another session with a smaller window size" -echo "tmux new-session -d -x 50 -y 10 -s test \"$TEST_CLIENT -q -t -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h \"$1\" -p \"$2\"\"" -tmux new-session -d -x 50 -y 10 -s test "$TEST_CLIENT -q -t -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h \"$1\" -p \"$2\"" +echo "tmux new-session -d -x 50 -y 10 -s $TMUX_SESSION \"$TEST_CLIENT -q -t -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h \"$1\" -p \"$2\"\"" +tmux new-session -d -x 50 -y 10 -s "$TMUX_SESSION" "$TEST_CLIENT -q -t -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h \"$1\" -p \"$2\"" wait_for_session || exit 1 @@ -128,10 +134,10 @@ if [ "10" != "$ROW_FOUND" ]; then fi # close down the SSH session -tmux send-keys -t test 'exit' -tmux send-keys -t test 'ENTER' +tmux send-keys -t "$TMUX_SESSION" 'exit' +tmux send-keys -t "$TMUX_SESSION" 'ENTER' set +e -tmux kill-session -t test +tmux kill-session -t "$TMUX_SESSION" popd exit 0 diff --git a/apps/wolfsshd/test/start_sshd.sh b/apps/wolfsshd/test/start_sshd.sh index 5c62d00fb..d703490d1 100755 --- a/apps/wolfsshd/test/start_sshd.sh +++ b/apps/wolfsshd/test/start_sshd.sh @@ -1,9 +1,17 @@ #!/bin/bash -# Holds the per-daemon temp dir used for root-owned trust-anchor copies, so -# stop_wolfsshd can remove it. Empty when no copies were made. +# Holds the per-daemon temp dir used for the rewritten config, the root-owned +# trust-anchor copies and the daemon's PID file, so stop_wolfsshd can remove it. +# Empty only when the temp dir could not be created. SSHD_KEYDIR="" +# The PID file named by the rewritten config. It is what makes a start +# identifiable: the daemon writes its own pid there after it has finished +# daemonizing, so the pid does not have to be inferred from what appeared in +# the process table, which cannot tell this run's daemon from a concurrent +# run's. +SSHD_PIDFILE="" + # starts up a sshd session, takes in the sshd_config file as an argument start_wolfsshd() { # Snapshot the PIDs of any daemon already running so the new one can be @@ -18,6 +26,7 @@ start_wolfsshd() { CONFIG="$ORIGCFG" # Reset so each invocation is self-contained regardless of call ordering. SSHD_KEYDIR="" + SSHD_PIDFILE="" # wolfSSHd loads each trust anchor (host key, host cert, user CA) through the # secure gate, which refuses a file not owned by the daemon's user or root @@ -27,64 +36,79 @@ start_wolfsshd() { # private dir, make the copies root-owned and mode 0600, and emit a temp # config pointing at them. The version-controlled files are left untouched so # the suite stays re-runnable. - if grep -qE '^[[:space:]]*(HostKey|HostCertificate|TrustedUserCAKeys)[[:space:]]' "$ORIGCFG"; then - SSHD_KEYDIR=$(mktemp -d 2>/dev/null) || SSHD_KEYDIR=$(mktemp -d -t sshdkeys) - if [ -z "$SSHD_KEYDIR" ] || [ ! -d "$SSHD_KEYDIR" ]; then - printf "WARNING: could not create temp dir for trust-anchor copies; using original config\n" >&2 - SSHD_KEYDIR="" - else - CONFIG="$SSHD_KEYDIR/sshd_config" - : > "$CONFIG" || { printf "WARNING: could not write %s; using original config\n" "$CONFIG" >&2; CONFIG="$ORIGCFG"; rm -rf "$SSHD_KEYDIR"; SSHD_KEYDIR=""; } - fi - # Only rewrite when the temp config was set up. On any fallback above - # SSHD_KEYDIR is empty and CONFIG still points at ORIGCFG; running the - # loop then would read from and append to the same file, never reaching - # EOF (runaway append) and would also operate on "/anchorN.pem" at the - # filesystem root. Skipping it leaves the original config untouched. - if [ -n "$SSHD_KEYDIR" ]; then - n=0 - # Rewrite the config line by line. For each trust-anchor directive - # copy the file to a counter-named destination (so distinct - # directories with the same basename do not collide) and emit the - # directive pointing at the copy. Paths are built by string assembly, - # not sed, so a checkout path containing regex or glob metacharacters - # cannot corrupt the rewrite. The directive keyword is the first - # field and the path is the remainder, so a path containing spaces is - # preserved. The "|| [ -n "$line" ]" keeps a final line lacking a - # trailing newline from being dropped. - while IFS= read -r line || [ -n "$line" ]; do - read -r key src </dev/null) || SSHD_KEYDIR=$(mktemp -d -t sshdkeys) + if [ -z "$SSHD_KEYDIR" ] || [ ! -d "$SSHD_KEYDIR" ]; then + printf "WARNING: could not create temp dir for the daemon config; using original config\n" >&2 + SSHD_KEYDIR="" + else + CONFIG="$SSHD_KEYDIR/sshd_config" + : > "$CONFIG" || { printf "WARNING: could not write %s; using original config\n" "$CONFIG" >&2; CONFIG="$ORIGCFG"; rm -rf "$SSHD_KEYDIR"; SSHD_KEYDIR=""; } + fi + # Only rewrite when the temp config was set up. On any fallback above + # SSHD_KEYDIR is empty and CONFIG still points at ORIGCFG; running the + # loop then would read from and append to the same file, never reaching + # EOF (runaway append) and would also operate on "/anchorN.pem" at the + # filesystem root. Skipping it leaves the original config untouched. + if [ -n "$SSHD_KEYDIR" ]; then + # PidFile goes in first, before any line is copied over. It has to land + # in the global section: several of the suite's configs end in a Match + # block, and a directive appended after one is parsed as part of that + # block, where a PidFile is never applied -- the daemon then writes no + # pid file, the start reports no pid, and stop_wolfsshd leaves it + # running on the shared port for the rest of the suite to trip over. + # Absolute, because wolfSSHd chdir()s to "/" while it daemonizes and a + # relative path would land at the filesystem root. None of the suite's + # configs set PidFile themselves, so this cannot conflict with one + # already in the file. + SSHD_PIDFILE="$SSHD_KEYDIR/wolfsshd.pid" + printf 'PidFile %s\n' "$SSHD_PIDFILE" > "$CONFIG" + + n=0 + # Rewrite the config line by line. For each trust-anchor directive + # copy the file to a counter-named destination (so distinct + # directories with the same basename do not collide) and emit the + # directive pointing at the copy. Paths are built by string assembly, + # not sed, so a checkout path containing regex or glob metacharacters + # cannot corrupt the rewrite. The directive keyword is the first + # field and the path is the remainder, so a path containing spaces is + # preserved. The "|| [ -n "$line" ]" keeps a final line lacking a + # trailing newline from being dropped. + while IFS= read -r line || [ -n "$line" ]; do + read -r key src <&2 - printf '%s\n' "$line" >> "$CONFIG" - continue - fi - # Owner-only: satisfies the writable check for every - # trust anchor and the no-group/world-readable check for - # the secret host key. The daemon runs as root and reads - # via the owner bits. - chmod 600 "$dst" - if ! sudo chown 0 "$dst"; then - printf "WARNING: could not chown %s to root; daemon may refuse to load it\n" "$src" >&2 - fi - printf '%s %s\n' "$key" "$dst" >> "$CONFIG" - else + case "$key" in + HostKey|HostCertificate|TrustedUserCAKeys) + if [ -n "$src" ] && [ -e "$src" ]; then + n=`expr $n + 1` + dst="$SSHD_KEYDIR/anchor$n.pem" + if ! cp "$src" "$dst"; then + printf "WARNING: could not copy %s; using original path\n" "$src" >&2 printf '%s\n' "$line" >> "$CONFIG" + continue + fi + # Owner-only: satisfies the writable check for every + # trust anchor and the no-group/world-readable check for + # the secret host key. The daemon runs as root and reads + # via the owner bits. + chmod 600 "$dst" + if ! sudo chown 0 "$dst"; then + printf "WARNING: could not chown %s to root; daemon may refuse to load it\n" "$src" >&2 fi - ;; - *) + printf '%s %s\n' "$key" "$dst" >> "$CONFIG" + else printf '%s\n' "$line" >> "$CONFIG" - ;; - esac - done < "$ORIGCFG" - fi + fi + ;; + *) + printf '%s\n' "$line" >> "$CONFIG" + ;; + esac + done < "$ORIGCFG" fi # SSHD_BIN picks the binary; SSHD_ENV passes env (e.g. LD_PRELOAD) that plain @@ -92,28 +116,100 @@ EOF SSHD_BIN="${SSHD_BIN:-../wolfsshd}" sudo env $SSHD_ENV "$SSHD_BIN" -d -E ./log.txt -f "$CONFIG" - # The PID of the started sshd is the one present now that was not there - # before. wolfSSHd forks twice while daemonizing, so for a moment its two - # short lived parents are listed as well; wait for the new pids to settle - # on the single survivor. Recording a parent instead would leave - # stop_wolfsshd killing a pid that is already gone while the real daemon - # keeps the port. The daemon can also die after sudo returns, so leave PID - # empty in that case and let the caller's empty-PID check report it. + # The daemon writes its own pid to the PID file once it has finished + # daemonizing and before it listens, so read it from there. The pid is not + # inferred from what is new in the process table any more: that could not + # tell this daemon from one a concurrent run started at the same moment, + # and the wait for a single new pid then never settled -- both runs + # reported "Issue starting up wolfSSHd" while both daemons were listening. + # The daemon can also die after sudo returns, so leave PID empty in that + # case and let the caller's empty-PID check report it. PID="" - for i in $(seq 1 50); do - NEW_PIDS=`pgrep -x wolfsshd | sort -n` || true - NEW=`diff <(echo "$CURRENT_PIDS") <(echo "$NEW_PIDS") \ - | sed -n 's/^> *//p'` - NEW_COUNT=`printf '%s\n' $NEW | grep -c .` || NEW_COUNT=0 - if [ "$NEW_COUNT" -eq 1 ]; then - PID="$NEW" - break + if [ -n "$SSHD_PIDFILE" ]; then + for i in $(seq 1 50); do + if [ -s "$SSHD_PIDFILE" ]; then + PID=`cat "$SSHD_PIDFILE"` + break + fi + sleep 0.1 + done + # A pid file left by a daemon that has since died is worse than none: + # stop_wolfsshd would kill whatever has been given that pid since. Ask + # what the process is, not merely whether it exists: "kill -0" answers + # the second question only, and the pid may have been recycled. + if [ -n "$PID" ] && ! wolfsshd_alive "$PID"; then + PID="" fi - sleep 0.1 - done + else + # No temp config, so no PID file: fall back to picking the pid that + # was not in the process table before. wolfSSHd forks twice while + # daemonizing, so for a moment its two short lived parents are listed + # as well; wait for the new pids to settle on the single survivor. + # Recording a parent instead would leave stop_wolfsshd killing a pid + # that is already gone while the real daemon keeps the port. + for i in $(seq 1 50); do + NEW_PIDS=`pgrep -x wolfsshd | sort -n` || true + NEW=`diff <(echo "$CURRENT_PIDS") <(echo "$NEW_PIDS") \ + | sed -n 's/^> *//p'` + NEW_COUNT=`printf '%s\n' $NEW | grep -c .` || NEW_COUNT=0 + if [ "$NEW_COUNT" -eq 1 ]; then + PID="$NEW" + break + fi + sleep 0.1 + done + fi + # wolfSSHd writes its PID file in StartSSHD() immediately before + # tcp_listen(), so the pid appearing does not mean the socket accepts yet. + # A caller that connects the moment this returns -- several do, with no + # sleep -- would be refused, and the daemon's log would show no connection + # at all. Wait for the daemon's own listening line, matched on its pid so + # that a previous daemon's line in this appended log cannot satisfy it. + LISTENING=0 + if [ -n "$PID" ]; then + for i in $(seq 1 100); do + if sudo grep -qF "[PID $PID]: [SSHD] Listening on port" \ + ./log.txt 2>/dev/null; then + LISTENING=1 + break + fi + sleep 0.1 + done + fi + + # Record the daemon in the run's registry, if the runner set one up. Test + # scripts run as children of run_all_sshd_tests.sh, so a variable cannot + # carry their pids back; a file can, which is what lets the runner's exit + # teardown be specific to this run instead of killing every wolfsshd on + # the machine. Registered before the check below so a daemon that came up + # but never listened is still reaped by the runner's teardown. + if [ -n "$PID" ] && [ -n "$WOLFSSHD_TEST_PIDFILE" ]; then + printf '%s\n' "$PID" >> "$WOLFSSHD_TEST_PIDFILE" 2>/dev/null || true + fi + + # Ten seconds and no listening line: report it here rather than return a + # pid the caller will trust. Falling through left the caller's empty-PID + # check satisfied and the test connecting to a daemon that never bound, + # so the run failed as a refused connection somewhere later instead of as + # a daemon that did not come up. Clearing PID puts it through the check + # every caller already has. + if [ -n "$PID" ] && [ "$LISTENING" -eq 0 ]; then + printf "wolfSSHd pid %s never logged a listening port\n" "$PID" >&2 + sudo kill $PID 2>/dev/null || true + PID="" + fi + printf "SSHD running on PID $PID\n" } +# True while $1 is still a wolfsshd. "kill -0" cannot answer this: it reports +# only that some process holds the pid and that we could signal it, and a pid +# the daemon has released may belong to anything by now. That distinction is +# not academic here -- stop_wolfsshd escalates to "sudo kill -9" on the answer. +wolfsshd_alive() { + pgrep -x wolfsshd | grep -qx -- "$1" +} + # 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 @@ -126,10 +222,55 @@ stop_wolfsshd() { # 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 + wolfsshd_alive "$PID" || break sleep 0.1 done + # Five seconds and still a wolfsshd: escalate rather than let the loop + # expire quietly. Everything below hands this pid back, so a daemon + # left running here is one nothing goes on to clean up, still holding + # the port the next start_wolfsshd wants. Gated on the process still + # being a wolfsshd, not on the pid being in use: this sends SIGKILL as + # root, and the pid may have been recycled by anything. + if wolfsshd_alive "$PID"; then + printf "SSHD pid $PID ignored SIGTERM, sending SIGKILL\n" + sudo kill -9 $PID 2>/dev/null || true + for i in $(seq 1 50); do + wolfsshd_alive "$PID" || break + sleep 0.1 + done + fi + + # Drop it from the run registry now that it is stopped. Left there, it + # would still be a candidate for the end-of-run sweep, which can only + # ask whether some wolfsshd holds that pid today -- and a concurrent + # run forks one per connection, so a recycled pid would be that run's + # daemon. A run accumulates about nine of these, all dead but one. + # + # Only once it really has stopped. Removing the entry unconditionally + # discarded the last handle on a daemon that had survived both signals, + # and the sweep is what would otherwise have retried it at the end of + # the run. + if ! wolfsshd_alive "$PID" \ + && [ -n "$WOLFSSHD_TEST_PIDFILE" ] \ + && [ -f "$WOLFSSHD_TEST_PIDFILE" ]; then + # Status kept through "|| gstat=$?", not read afterwards: callers + # source this under "set -e", where grep's 1 for a registry that + # held only this pid would end the test. + gstat=0 + grep -vx -- "$PID" "$WOLFSSHD_TEST_PIDFILE" \ + > "$WOLFSSHD_TEST_PIDFILE.new" 2>/dev/null || gstat=$? + # 0 is lines kept, 1 is none kept -- this was the only entry. + # Anything else is grep failing, and the empty file it left would + # install as the registry and lose every other daemon's pid. + if [ "$gstat" -le 1 ]; then + mv -f "$WOLFSSHD_TEST_PIDFILE.new" "$WOLFSSHD_TEST_PIDFILE" \ + 2>/dev/null || rm -f "$WOLFSSHD_TEST_PIDFILE.new" + else + rm -f "$WOLFSSHD_TEST_PIDFILE.new" + fi + fi + # 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="" @@ -141,6 +282,34 @@ stop_wolfsshd() { if [ -n "$SSHD_KEYDIR" ]; then rm -rf "$SSHD_KEYDIR" SSHD_KEYDIR="" + # Lived in the dir just removed, so it must not look readable to a + # second start that fails before writing a new one. + SSHD_PIDFILE="" fi return 0 } + +# End-of-run safety net: kill any daemon this run started that is still alive. +# Only the pids in the registry are considered, so a concurrent run's daemon is +# left alone. Safe to call when no registry was set up or nothing was started. +stop_all_wolfsshd() { + local alive p + if [ -z "$WOLFSSHD_TEST_PIDFILE" ] || [ ! -f "$WOLFSSHD_TEST_PIDFILE" ]; then + return 0 + fi + + # Match the recorded pids against the live wolfsshd pids rather than + # killing them blind: a pid recorded early in the run may since have + # exited and been recycled by an unrelated process. + alive=`pgrep -x wolfsshd` || true + while read -r p; do + [ -n "$p" ] || continue + if printf '%s\n' $alive | grep -qx -- "$p"; then + printf "Stopping leftover SSHD, killing pid %s\n" "$p" + sudo kill "$p" 2>/dev/null || true + fi + done < "$WOLFSSHD_TEST_PIDFILE" + + : > "$WOLFSSHD_TEST_PIDFILE" 2>/dev/null || true + return 0 +} diff --git a/keys/renew-ossh-certs.sh b/keys/renew-ossh-certs.sh index ad8806785..443b11f50 100755 --- a/keys/renew-ossh-certs.sh +++ b/keys/renew-ossh-certs.sh @@ -10,6 +10,14 @@ set -e USER_NAME=${1:-fred} +# Where the certificates are written. The keypairs below are committed and +# shared, but a certificate carries the run that made it -- the force-command +# one names that run's marker directory -- so two runs writing them here would +# each authenticate with the other's certificate. The test points this at a +# per-run directory; the default keeps standalone use writing next to the keys. +OSSH_CERT_DIR="${OSSH_CERT_DIR:-.}" +mkdir -p "$OSSH_CERT_DIR" + # Where the force-command certificate writes. The test overrides this with a # per-run temp dir; the fallback is per-process so it is not a shared path. OSSH_FORCED_MARKER="${OSSH_FORCED_MARKER:-${TMPDIR:-/tmp}/wolfsshd_ossh_forced_marker.$$}" @@ -43,7 +51,7 @@ chmod 600 ossh-ca ossh-ca-rsa ossh-ca-ecdsa ossh-bad-ca \ # gen_cert_u [opts...] gen_cert_u() { - out_base=$1; user_pub=$2; ca=$3; key_id=$4; principal=$5 + out_base=$OSSH_CERT_DIR/$1; user_pub=$2; ca=$3; key_id=$4; principal=$5 shift 5 cp "$user_pub" "$out_base.pub" ssh-keygen -q -s "$ca" -I "$key_id" -n "$principal" -V always:forever \ @@ -65,10 +73,10 @@ gen_cert "$USER_NAME-ossh-badca" ossh-bad-ca "ossh-badca" \ gen_cert "$USER_NAME-ossh-wrongprincipal" ossh-ca "ossh-wrongprincipal" \ "other-$USER_NAME" # No principals (signed without -n): must not log in, like OpenSSH sshd. -cp ossh-user.pub "$USER_NAME-ossh-noprincipal.pub" +cp ossh-user.pub "$OSSH_CERT_DIR/$USER_NAME-ossh-noprincipal.pub" ssh-keygen -q -s ossh-ca -I "ossh-noprincipal" -V always:forever \ - "$USER_NAME-ossh-noprincipal.pub" -rm -f "$USER_NAME-ossh-noprincipal.pub" + "$OSSH_CERT_DIR/$USER_NAME-ossh-noprincipal.pub" +rm -f "$OSSH_CERT_DIR/$USER_NAME-ossh-noprincipal.pub" # RSA and ECDSA (P-384) CAs cover those CA-signature paths. The user key stays # Ed25519, so the ECDSA case also covers taking the digest from the CA curve. gen_cert "$USER_NAME-ossh-rsaca" ossh-ca-rsa "ossh-rsaca" \ diff --git a/scripts/get-put.test b/scripts/get-put.test index 10a305617..1c29fc79b 100755 --- a/scripts/get-put.test +++ b/scripts/get-put.test @@ -45,12 +45,14 @@ READY_COUNTER=0 wait_for_server() { - while [ ! -s "$READY_FILE" ] && [ "$READY_COUNTER" -lt 20 ]; do + while [ ! -s "$READY_FILE" ] && [ "$READY_COUNTER" -lt 100 ]; do sleep 0.1 READY_COUNTER=$((READY_COUNTER+ 1)) done - if test -e "$READY_FILE" + # -s, not -e: the echoserver creates the file empty and writes the + # port afterward, so -e can catch it mid-write and yield no port + if [ -s "$READY_FILE" ] then # get created port 0 ephemeral port PORT=$(cat "$READY_FILE") diff --git a/scripts/scp.test b/scripts/scp.test index 20f9737eb..b13751e3d 100755 --- a/scripts/scp.test +++ b/scripts/scp.test @@ -25,13 +25,15 @@ fi create_port() { # each server gets its own wait budget, the count must not carry over counter=0 - while [ ! -s "$ready_file" ] && [ "$counter" -lt 20 ]; do + while [ ! -s "$ready_file" ] && [ "$counter" -lt 100 ]; do echo -e "waiting for ready file..." sleep 0.1 counter=$((counter+ 1)) done - if test -e $ready_file; then + # -s, not -e: the echoserver creates the file empty and writes the + # port afterward, so -e can catch it mid-write and yield no port + if [ -s "$ready_file" ]; then echo -e "found ready file, starting client..." # get created port 0 ephemeral port diff --git a/scripts/sftp.test b/scripts/sftp.test index 3d5304cfe..ac1a0000d 100755 --- a/scripts/sftp.test +++ b/scripts/sftp.test @@ -35,7 +35,7 @@ create_port() { # counted per server start, so a slow first start cannot eat the wait # the later ones need counter=0 - while [ ! -s "$ready_file" ] && [ "$counter" -lt 20 ]; do + while [ ! -s "$ready_file" ] && [ "$counter" -lt 100 ]; do echo "waiting for ready file..." sleep 0.1 counter=$((counter+ 1))