From 34a9939cbcf3230752bae45bdcd919ba997afa34 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 28 Aug 2026 11:59:00 -0700 Subject: [PATCH] tests: report the sshd suite's pass count The summary prints "N run, N passed, N skipped" from a PASSED counter incremented at each success site. TOTAL counts every test reached, skips included, so printing it under the label "passed" overstated the pass count by exactly the number of skips. - pair each orphan SKIPPED bump with a TOTAL bump, and count the local-only tests skipped against an external host by name, so the printed numbers are commensurable and the skip count cannot go stale - count the host key gate's WOLFSSH_NO_HOSTKEY_PERMS exit as a pass: it prints PASSED and returns with TOTAL already charged, so leaving it uncounted would fail the new assertion on every build with that macro - assert that passed plus skipped equals ran and, without --match, that ran equals the known test count, so a miscounting skip site or a name no call site reaches fails the run - verified in the container: a full run reports 32 run, 30 passed, 2 skipped against 30 PASSED lines, and --exclude sshd_forcedcmd_test.sh reports 32 run, 29 passed, 3 skipped against 29 --- apps/wolfsshd/test/README.md | 10 ++- apps/wolfsshd/test/run_all_sshd_tests.sh | 109 +++++++++++++++++------ 2 files changed, 90 insertions(+), 29 deletions(-) diff --git a/apps/wolfsshd/test/README.md b/apps/wolfsshd/test/README.md index 85d67b5f..48814c9c 100644 --- a/apps/wolfsshd/test/README.md +++ b/apps/wolfsshd/test/README.md @@ -27,10 +27,13 @@ SSHD running on PID 7979 sshd_exec_test.sh ... PASSED Shutting down test wolfSSHd Stopping SSHD, killing pid 7979 -All tests ran, 1 passed, 0 skipped +All tests ran, 1 run, 1 passed, 0 skipped ``` +"run" counts every test reached, skips included. Passed plus skipped must equal +run; the runner fails if they do not. + To run a specific test do: startup wolfSSHd @@ -89,6 +92,11 @@ the shared daemon stopped first -- and keep the `run_test` call where that setup happens. Both arrays drive `--match` and `--exclude`, so a test that is missing from them cannot be selected or skipped from the command line. +A test that can only run against a daemon the runner starts itself is skipped +when `--host` and `--port` point at an external one. Pass its name to +`skip_local_only` on that path, or add it to `local_sshd_tests` if it belongs in +that block, so the external run still counts it as skipped. + ## Selecting Tests `--match ` runs a single test and `--exclude ` drops one from a full diff --git a/apps/wolfsshd/test/run_all_sshd_tests.sh b/apps/wolfsshd/test/run_all_sshd_tests.sh index 3f364134..47ea0227 100755 --- a/apps/wolfsshd/test/run_all_sshd_tests.sh +++ b/apps/wolfsshd/test/run_all_sshd_tests.sh @@ -208,6 +208,7 @@ run_teardown() { trap run_teardown EXIT TOTAL=0 +PASSED=0 SKIPPED=0 # Set as the last statement of each branch that runs tests, and checked before # the summary. A shell expansion error (a bad arithmetic expansion, say) unwinds @@ -247,6 +248,7 @@ should_skip() { fi if [ "$1" == "$EXCLUDE" ]; then echo "Test '$1' is excluded. Skipping." + TOTAL=$((TOTAL+1)) SKIPPED=$((SKIPPED+1)) return 0 fi @@ -261,6 +263,19 @@ matches() { [ -z "$MATCH" ] || [ "$1" == "$MATCH" ] } +# Counts each named test as skipped, for tests that need a local daemon on a run +# against an external host. Takes the names rather than a number so the count +# cannot drift from the tests it stands in for. +skip_local_only() { + local test + for test in "$@"; do + if matches "$test"; then + TOTAL=$((TOTAL+1)) + SKIPPED=$((SKIPPED+1)) + fi + done +} + # Collect the directories above $1 that wolfSSHd's StrictModes check will # reject: a group or world writable directory anywhere above authorized_keys # makes the daemon refuse to read it, unless the directory is sticky. This @@ -351,6 +366,7 @@ run_test() { SKIPPED=$((SKIPPED+1)) elif [ "$RESULT" == 0 ]; then printf "PASSED\n" + PASSED=$((PASSED+1)) else printf "FAILED!\n" cat stdout.txt @@ -402,6 +418,7 @@ EOF TOTAL=$((TOTAL+1)) if grep -q "group or world readable" strictmodes_log.txt; then printf "PASSED\n" + PASSED=$((PASSED+1)) else printf "FAILED!\n" cat strictmodes_log.txt @@ -460,6 +477,7 @@ EOF grep -q "but this build cannot enforce it" upn_nofpki_log.txt && ! grep -q "Refusing to load" upn_nofpki_log.txt; then printf "PASSED\n" + PASSED=$((PASSED+1)) else printf "FAILED!\n" cat upn_nofpki_log.txt @@ -519,6 +537,7 @@ run_strictmodes_authkeys_negative_test() { after=${after:-0} if [ "$result" != 0 ] && [ "$after" -gt "$before" ]; then printf "PASSED\n" + PASSED=$((PASSED+1)) else printf "FAILED! (expected StrictModes rejection: client exit=%s, new log matches=%s)\n" \ "$result" "$((after - before))" @@ -655,6 +674,7 @@ EOF rm -rf "$HK_WORK" printf "PASSED (mode and owner cases skipped, built with " printf "WOLFSSH_NO_HOSTKEY_PERMS)\n" + PASSED=$((PASSED+1)) return fi @@ -674,6 +694,7 @@ EOF rm -rf "$HK_WORK" printf "PASSED\n" + PASSED=$((PASSED+1)) } # Run the tests. There is one path whether or not --match was given: every test @@ -712,15 +733,15 @@ run_hostkey_perm_check # rejected. Reads the local ./log.txt, so only when we own the daemon. if [ "$USING_LOCAL_HOST" == 1 ]; then run_test "sshd_pubkey_reject_test.sh" -elif matches "sshd_pubkey_reject_test.sh"; then - SKIPPED=$((SKIPPED+1)) +else + skip_local_only "sshd_pubkey_reject_test.sh" fi # exercise the authorized_keys StrictModes path against the running sshd if [ "$USING_LOCAL_HOST" == 1 ]; then run_strictmodes_authkeys_negative_test -elif matches "strictmodes_authkeys_negative"; then - SKIPPED=$((SKIPPED+1)) +else + skip_local_only "strictmodes_authkeys_negative" fi if [ "$USING_LOCAL_HOST" == 1 ]; then @@ -729,27 +750,32 @@ if [ "$USING_LOCAL_HOST" == 1 ]; then fi # these tests require setting up an sshd +local_sshd_tests=( + "sshd_forcedcmd_test.sh" + "sshd_match_overlap_test.sh" + "sshd_window_full_test.sh" + "sshd_stderr_eof_test.sh" + "sshd_empty_password_test.sh" + "sshd_permitroot_test.sh" + "sshd_permitroot_prohibit_password.sh" + "sshd_permitroot_forced_cmd.sh" + "strictmodes_hostkey_negative" + "upn_unenforceable_negative" + "sshd_login_grace_test.sh" + "sshd_privdrop_fail_test.sh" + "sshd_chroot_fail_test.sh" +) if [ "$USING_LOCAL_HOST" == 1 ]; then - run_test "sshd_forcedcmd_test.sh" - run_test "sshd_match_overlap_test.sh" - run_test "sshd_window_full_test.sh" - run_test "sshd_stderr_eof_test.sh" - run_test "sshd_empty_password_test.sh" - run_test "sshd_permitroot_test.sh" - run_test "sshd_permitroot_prohibit_password.sh" - run_test "sshd_permitroot_forced_cmd.sh" - run_strictmodes_negative_test - run_upn_unenforceable_negative_test - run_test "sshd_login_grace_test.sh" - run_test "sshd_privdrop_fail_test.sh" - run_test "sshd_chroot_fail_test.sh" + for test in "${local_sshd_tests[@]}"; do + case "$test" in + strictmodes_hostkey_negative) run_strictmodes_negative_test ;; + upn_unenforceable_negative) run_upn_unenforceable_negative_test ;; + *) run_test "$test" ;; + esac + done else printf "Skipping tests that need to setup local SSHD\n" - # the thirteen calls above; with --match at most one of them was going to run, - # and should_skip() has already counted it if it was excluded - if [ -z "$MATCH" ]; then - SKIPPED=$((SKIPPED+13)) - fi + skip_local_only "${local_sshd_tests[@]}" fi # these tests run with X509 sshd-config loaded. The matches() guard keeps a @@ -768,7 +794,9 @@ fi # negative test: a certificate UPN realm outside AuthorizedUPNDomains must # be rejected. Needs the dedicated bad-domain config, so only runs when we # control the local daemon. -if [ "$USING_LOCAL_HOST" == 1 ] && matches "sshd_x509_upn_fail.sh"; then +if [ "$USING_LOCAL_HOST" != 1 ]; then + skip_local_only "sshd_x509_upn_fail.sh" +elif matches "sshd_x509_upn_fail.sh"; then start_wolfsshd "sshd_config_test_x509_upn_bad" run_test "sshd_x509_upn_fail.sh" printf "Shutting down test wolfSSHd\n" @@ -780,7 +808,9 @@ fi # sshd_config_test_mldsa has no other host key, so an ML-DSA-less build # cannot start the daemon; check out here, not in the test script. The check # is the composite, not the umbrella: the ECDSA half can be missing on its own. -if [ "$USING_LOCAL_HOST" == 1 ] && matches "sshd_mldsa_composite_test.sh"; then +if [ "$USING_LOCAL_HOST" != 1 ]; then + skip_local_only "sshd_mldsa_composite_test.sh" +elif matches "sshd_mldsa_composite_test.sh"; then if wolfssh_has MLDSA87_ES384; then start_wolfsshd "sshd_config_test_mldsa" run_test "sshd_mldsa_composite_test.sh" @@ -799,6 +829,8 @@ fi # client when present. if [ "$USING_LOCAL_HOST" == 1 ]; then run_test "sshd_ossh_cert_test.sh" +else + skip_local_only "sshd_ossh_cert_test.sh" fi RUN_COMPLETE=1 } @@ -809,14 +841,35 @@ if [ "$RUN_COMPLETE" != 1 ]; then fi # A --match that reached no call site ran nothing and skipped nothing, which -# would otherwise report a clean run. The local-only tests are unreachable -# against an external host, so a name that is valid on a local run selects -# nothing here; say so rather than exit 0 on an empty run. +# would otherwise add up and report a clean "0 run". Every name is counted +# somewhere -- the local-only ones as skipped against an external host -- so +# this catches a call site that stopped counting; say so rather than exit 0 on +# an empty run. if [ -n "$MATCH" ] && [ "$TOTAL" -eq 0 ]; then printf "ERROR: --match %s selected no test that could run here\n" "$MATCH" exit 1 fi -printf "All tests ran, $TOTAL passed, $SKIPPED skipped\n" +# TOTAL counts every test that was reached, skips included, so it is a "ran" +# count and not a pass count. Print the three separately, and assert they add +# up: a skip site that bumps one counter and not the other then fails the run +# instead of quietly printing a wrong number. +printf "All tests ran, %d run, %d passed, %d skipped\n" \ + "$TOTAL" "$PASSED" "$SKIPPED" + +if [ "$((PASSED + SKIPPED))" -ne "$TOTAL" ]; then + printf "ERROR: counter mismatch (ran %d, passed %d, skipped %d)\n" \ + "$TOTAL" "$PASSED" "$SKIPPED" + exit 1 +fi + +# Without --match every known test is reached once, run or skipped, so a +# name no call site reaches -- one dropped from local_sshd_tests, say -- +# shows up here as a short count. +KNOWN=$(( ${#test_cases[@]} + ${#extra_test_cases[@]} )) +if [ -z "$MATCH" ] && [ "$TOTAL" -ne "$KNOWN" ]; then + printf "ERROR: %d tests known but %d reached\n" "$KNOWN" "$TOTAL" + exit 1 +fi exit 0