diff --git a/.github/workflows/code-coverage.yml b/.github/workflows/code-coverage.yml new file mode 100644 index 000000000..8bd25f259 --- /dev/null +++ b/.github/workflows/code-coverage.yml @@ -0,0 +1,180 @@ +name: Code Coverage + +on: + push: + branches: [ 'master', 'main', 'release/**' ] + pull_request: + branches: [ '*' ] + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + build_wolfssl: + name: Build wolfSSL + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - name: Checkout wolfSSL + uses: actions/checkout@v6 + with: + repository: wolfssl/wolfssl + path: wolfssl + + # Match the sshd-test workflow so the cert and ML-DSA paths are built + # and measured rather than compiled out. + - name: Build wolfSSL + working-directory: ./wolfssl + run: | + ./autogen.sh + ./configure --enable-all --enable-mldsa + make -j$(nproc) + sudo make install + sudo ldconfig + + - name: tar build-dir + run: tar -zcf wolfssl-install.tgz /usr/local/lib/libwolfssl* /usr/local/include/wolfssl + + - name: Upload built lib + uses: actions/upload-artifact@v7 + with: + name: wolfssl-coverage + path: wolfssl-install.tgz + retention-days: 5 + + # Use clang to report line, branch, function and MC/DC coverage in one run. + coverage: + name: Coverage + runs-on: ubuntu-latest + timeout-minutes: 45 + needs: build_wolfssl + steps: + - name: Checkout wolfSSH + uses: actions/checkout@v6 + + # clang 18 is the min: -fcoverage-mcdc does not exist before it. + - name: Install clang and LLVM coverage tools + run: | + sudo apt-get update + sudo apt-get install -y clang-18 llvm-18 libclang-rt-18-dev + + - name: Download wolfSSL + uses: actions/download-artifact@v8 + with: + name: wolfssl-coverage + + - name: Install wolfSSL + run: | + sudo tar -xzf wolfssl-install.tgz -C / + sudo ldconfig + + # -O0 keeps line and branch attribution honest; atomic counters are + # required because several tests drive client and server on separate + # threads, and the default non-atomic updates lose increments. + - name: Build wolfSSH + run: | + ./autogen.sh + ./configure --enable-all --enable-ossh-certs CC=clang-18 \ + CPPFLAGS="-DMAX_PATH_SZ=120" \ + CFLAGS="-fprofile-instr-generate -fcoverage-mapping -fcoverage-mcdc -fprofile-update=atomic -O0 -g" \ + LDFLAGS="-fprofile-instr-generate" + make -j$(nproc) + + # %p in the pattern keeps forked servers from overwriting the raw + # profile of the client that spawned them. + - name: Run tests + run: | + mkdir -p prof + LLVM_PROFILE_FILE="$PWD/prof/%p-%m.profraw" \ + timeout -k 30 1200 make check + + # 'make check' does not execute wolfsshd, so run it separately + - name: Run wolfSSHd tests + working-directory: ./apps/wolfsshd/test + run: | + prof="$GITHUB_WORKSPACE/prof/%p-%m.profraw" + sudo LLVM_PROFILE_FILE="$prof" SSHD_ENV="LLVM_PROFILE_FILE=$prof" \ + ./run_all_sshd_tests.sh + sudo chown -R "$(id -u):$(id -g)" "$GITHUB_WORKSPACE/prof" + + - name: Report coverage + run: | + llvm-profdata-18 merge -sparse prof/*.profraw -o wolfssh.profdata + # llvm-cov takes one binary positionally and the rest via -object. + # Programs linking the shared library are libtool wrapper scripts, so + # take the real binary from .libs when one is there. The apps are + # optional, so skip whatever this configuration did not build. + first="" + args=() + for t in tests/*.test apps/wolfssh/wolfssh apps/wolfsshd/wolfsshd \ + apps/wolfsshd/test/test_configuration; do + [ -e "$t" ] || continue + real="$(dirname "$t")/.libs/$(basename "$t")" + [ -x "$real" ] || real="$t" + if [ -z "$first" ]; then + first="$real" + else + args+=(-object "$real") + fi + done + if [ -z "$first" ]; then + echo "no instrumented binaries found" + exit 1 + fi + ignore='(tests|examples)/.*|apps/wolfsshd/test/.*' + ignore="$ignore"'|.*/include/wolfssl/.*|.*/wolfssh/.*\.h' + llvm-cov-18 report "$first" "${args[@]}" \ + -instr-profile=wolfssh.profdata \ + --show-mcdc-summary \ + --ignore-filename-regex="$ignore" | tee coverage-report.txt + llvm-cov-18 show "$first" "${args[@]}" \ + -instr-profile=wolfssh.profdata \ + --show-mcdc --format=html --output-dir=coverage-html \ + --ignore-filename-regex="$ignore" + # lcov text for any external dashboard that consumes it. + llvm-cov-18 export "$first" "${args[@]}" \ + -instr-profile=wolfssh.profdata \ + --format=lcov \ + --ignore-filename-regex="$ignore" > coverage.lcov + { + echo '### Coverage' + echo '```' + cat coverage-report.txt + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + + - name: Upload coverage report + uses: actions/upload-artifact@v7 + with: + name: coverage-report + path: | + coverage-report.txt + coverage.lcov + coverage-html/ + retention-days: 30 + + - name: Show test logs on failure + if: failure() + run: | + echo "=== test-suite.log ===" + cat test-suite.log || true + for f in tests/*.log scripts/*.log; do + [ -f "$f" ] || continue + echo "" + echo "=== $f ===" + cat "$f" + done + + - name: Upload failure logs + if: failure() + uses: actions/upload-artifact@v7 + with: + name: wolfssh-coverage-logs + path: | + test-suite.log + tests/*.log + scripts/*.log + config.log + retention-days: 5 diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c index e7743c2cf..0ea3b2f36 100644 --- a/apps/wolfsshd/auth.c +++ b/apps/wolfsshd/auth.c @@ -1294,7 +1294,7 @@ static int SearchKeysFile(const char* keysFilePath, const byte* key, if (wolfSSHD_OpenSecureFile(keysFilePath, uid, 0 /* rejectReadable */, NULL, &f) != WS_SUCCESS) { wolfSSH_Log(WS_LOG_ERROR, - "[SSHD] Keys file %s failed StrictModes check", keysFilePath); + "[SSHD] Keys file failed StrictModes check: %s", keysFilePath); ret = WSSHD_AUTH_FAILURE; } } diff --git a/apps/wolfsshd/test/run_all_sshd_tests.sh b/apps/wolfsshd/test/run_all_sshd_tests.sh index 137ff992d..b2df71ca6 100755 --- a/apps/wolfsshd/test/run_all_sshd_tests.sh +++ b/apps/wolfsshd/test/run_all_sshd_tests.sh @@ -195,7 +195,7 @@ run_strictmodes_authkeys_negative_test() { # the right reason and not an unrelated client error. Count existing # rejection lines first so a re-run is not confused by stale matches. local before - before=$(grep -c "failed StrictModes check" log.txt 2>/dev/null || echo 0) + before=$(grep -c "failed StrictModes check" log.txt 2>/dev/null) || before=0 chmod 0666 authorized_keys_test ( cd ../../.. && $tmo ./examples/client/client -c 'exit' -u "$USER" \ -i ./keys/hansel-key-ecc.der -j ./keys/hansel-key-ecc.pub \ @@ -203,7 +203,7 @@ run_strictmodes_authkeys_negative_test() { local result=$? chmod 0644 authorized_keys_test local after - after=$(grep -c "failed StrictModes check" log.txt 2>/dev/null || echo 0) + after=$(grep -c "failed StrictModes check" log.txt 2>/dev/null) || after=0 if [ "$result" != 0 ] && [ "$after" -gt "$before" ]; then printf "PASSED\n" else diff --git a/apps/wolfsshd/test/sshd_forcedcmd_test.sh b/apps/wolfsshd/test/sshd_forcedcmd_test.sh index 9aa3adacc..0bbd5a72d 100755 --- a/apps/wolfsshd/test/sshd_forcedcmd_test.sh +++ b/apps/wolfsshd/test/sshd_forcedcmd_test.sh @@ -8,7 +8,9 @@ if [ -z "$1" ] || [ -z "$2" ]; then exit 1 fi -PWD=`pwd` +# Not PWD: that name is maintained by the shell and every cd overwrites it, +# so the cd back to this directory would land wherever the last cd went. +TEST_DIR=`pwd` USER=`whoami` TEST_PORT="$2" TEST_HOST="$1" @@ -22,8 +24,8 @@ PasswordAuthentication yes PermitEmptyPasswords no UsePrivilegeSeparation no UseDNS no -HostKey $PWD/../../../keys/server-key.pem -AuthorizedKeysFile $PWD/authorized_keys_test +HostKey $TEST_DIR/../../../keys/server-key.pem +AuthorizedKeysFile $TEST_DIR/authorized_keys_test Match User $USER ForceCommand internal-sftp @@ -49,7 +51,7 @@ fi set -e echo exit | $TEST_SFTP -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h $TEST_HOST -p $TEST_PORT -cd $PWD +cd $TEST_DIR stop_wolfsshd # A configured ForceCommand that is not "internal-sftp" must still permit the @@ -64,8 +66,8 @@ PasswordAuthentication yes PermitEmptyPasswords no UsePrivilegeSeparation no UseDNS no -HostKey $PWD/../../../keys/server-key.pem -AuthorizedKeysFile $PWD/authorized_keys_test +HostKey $TEST_DIR/../../../keys/server-key.pem +AuthorizedKeysFile $TEST_DIR/authorized_keys_test Match User $USER ForceCommand /bin/echo @@ -76,7 +78,7 @@ cd ../../.. echo exit | $TEST_SFTP -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h $TEST_HOST -p $TEST_PORT -cd $PWD +cd $TEST_DIR stop_wolfsshd exit 0 diff --git a/apps/wolfsshd/test/sshd_privdrop_fail_test.sh b/apps/wolfsshd/test/sshd_privdrop_fail_test.sh index 73a22c0d4..69df11f1c 100755 --- a/apps/wolfsshd/test/sshd_privdrop_fail_test.sh +++ b/apps/wolfsshd/test/sshd_privdrop_fail_test.sh @@ -49,8 +49,11 @@ touch log.txt TEST_CLIENT="../../../examples/client/client" SFTP_CLIENT="../../../examples/sftpclient/wolfsftp" SCP_CLIENT="../../../examples/scpclient/wolfscp" -PRIVATE_KEY="../../../keys/hansel-key-ecc.der" -PUBLIC_KEY="../../../keys/hansel-key-ecc.pub" +# Absolute: the client and sftp examples call ChangeToWolfSshRoot(), which +# changes directory to the wolfSSH root before opening these, so a path +# relative to this directory would not be found. +PRIVATE_KEY="$PWD/../../../keys/hansel-key-ecc.der" +PUBLIC_KEY="$PWD/../../../keys/hansel-key-ecc.pub" # Small payload for the sftp/scp transfers. The connection dies at the failed # drop long before any data moves, so the contents do not matter. @@ -164,7 +167,7 @@ check_subsystem "exec" \ # SFTP_Subsystem. -g is a one-shot put, so the client cannot sit at a prompt. check_subsystem "sftp" \ "$SFTP_CLIENT" -u "$USER" -i "$PRIVATE_KEY" -j "$PUBLIC_KEY" \ - -g -l "$PAYLOAD" -r "/tmp/privdrop_remote_$$.txt" \ + -g -l "$PWD/$PAYLOAD" -r "/tmp/privdrop_remote_$$.txt" \ -h "$TEST_HOST" -p "$TEST_PORT" # SCP_Subsystem. diff --git a/apps/wolfsshd/test/start_sshd.sh b/apps/wolfsshd/test/start_sshd.sh index 1c553c07a..f894fb3ae 100755 --- a/apps/wolfsshd/test/start_sshd.sh +++ b/apps/wolfsshd/test/start_sshd.sh @@ -6,7 +6,9 @@ SSHD_KEYDIR="" # starts up a sshd session, takes in the sshd_config file as an argument start_wolfsshd() { - CURRENT_PIDS=`ps -e | grep wolfsshd | grep -oE "[0-9]+"` + # Having no daemon running yet is normal, but grep then exits non-zero and + # would end a caller that runs under "set -e", so absorb it here. + CURRENT_PIDS=`ps -e | grep wolfsshd | grep -oE "[0-9]+"` || CURRENT_PIDS="" ORIGCFG="$1" CONFIG="$ORIGCFG" @@ -87,7 +89,7 @@ EOF sudo env $SSHD_ENV "$SSHD_BIN" -d -E ./log.txt -f "$CONFIG" # set the PID of started sshd - NEW_PID=`ps -e | grep wolfsshd | grep -oE "[0-9]+"` + NEW_PID=`ps -e | grep wolfsshd | grep -oE "[0-9]+"` || NEW_PID="" PID=`diff <(echo "$CURRENT_PIDS") <(echo "$NEW_PID") | grep '>' | grep -oE "[0-9]+" | head -n1` printf "SSHD running on PID $PID\n" } diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index 5b023955c..174b3c87b 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -202,7 +202,9 @@ static void ShowUsage(void) printf(" -D Run in foreground (do not detach)\n"); printf(" -h host private key file to use\n"); printf(" -E append to log file\n"); -#ifdef WOLFSSL_FPKI +/* WOLFSSH_NO_FPKI, not WOLFSSL_FPKI: the check is left out whenever wolfSSH + * turns FPKI off, which a build can do on its own even when wolfSSL has it. */ +#ifndef WOLFSSH_NO_FPKI /* build-capability note, separated from the option list; also greppable by * test scripts, for the cert UPN domain check (AuthorizedUPNDomains) */ printf("\n");