From 3584b1b29442fac3b27b16800e691ce9205da792 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 10 Aug 2026 17:13:03 -0700 Subject: [PATCH 01/10] Wire up the wolfssh client's -E log file option - -E was parsed into config.logFile and printed by -G, never read. - Install a logging callback that writes to the named file, following what wolfsshd does for its own -E. - Turn logging on with wolfSSH_Debugging_ON(). Installing the callback is not enough on its own, the file came out empty in any build that wasn't --enable-debug, including the --enable-all builds where the library has all of its logging compiled in. wolfsshd turns logging on the same way. - Match DefaultLoggingCb()'s format, timestamp and level tag, so a log written to the file and one written to stderr are comparable. That function's GetLogStr() is private to the library, so the level names are repeated in the app. - Parse the command line and open the file in main(), before wolfSSH_Init(), so the start up messages land in the file. - Close the file after wolfSSH_Cleanup(). The callback cannot be uninstalled, so it ran with a closed stream and segfaulted on exit. It falls back to stderr. - Name the stream logFileStream, apart from struct config's logFile, which is the path it was opened from. - Drop the always true condition around the session threads. --- apps/wolfssh/README.md | 5 +- apps/wolfssh/wolfssh.c | 131 ++++++++++++++++++++++++++++++++--------- 2 files changed, 108 insertions(+), 28 deletions(-) diff --git a/apps/wolfssh/README.md b/apps/wolfssh/README.md index 44a8db078..cc368ac4c 100644 --- a/apps/wolfssh/README.md +++ b/apps/wolfssh/README.md @@ -15,7 +15,10 @@ have support for SSH-AGENT and forwarding. Command Line Options -------------------- - -E logfile : Specify a different log file. + -E logfile : Append the log to this file instead of stderr, and turn + logging on. The log is empty unless the library has + logging compiled in, with `--enable-debug` or + `--enable-sshd`. -G : Print out the configuration as used. -l login_name : Overrides the login name specified in the destination. -p port : Overrides the destination port number. diff --git a/apps/wolfssh/wolfssh.c b/apps/wolfssh/wolfssh.c index 9702122e5..4507c3d94 100644 --- a/apps/wolfssh/wolfssh.c +++ b/apps/wolfssh/wolfssh.c @@ -31,6 +31,7 @@ #endif #include +#include #include #include #include @@ -73,6 +74,10 @@ #include #endif +#ifndef WOLFSSH_NO_TIMESTAMP + #include +#endif + #ifdef WOLFSSH_CERTS #include #endif @@ -81,6 +86,55 @@ int myoptind = 0; char* myoptarg = NULL; +/* The file named by -E, when given. Named apart from struct config's + * logFile, which is the path this was opened from. */ +static WFILE* logFileStream = NULL; + + +/* Same names DefaultLoggingCb() logs with. That function's GetLogStr() is + * private to the library, so the list is repeated here. */ +static const char* ClientLogLevelStr(enum wolfSSH_LogLevel level) +{ + switch (level) { + case WS_LOG_INFO: return "INFO"; + case WS_LOG_WARN: return "WARNING"; + case WS_LOG_ERROR: return "ERROR"; + case WS_LOG_DEBUG: return "DEBUG"; + case WS_LOG_USER: return "USER"; + case WS_LOG_SFTP: return "SFTP"; + case WS_LOG_SCP: return "SCP"; + case WS_LOG_AGENT: return "AGENT"; + case WS_LOG_CERTMAN: return "CERTMAN"; + default: return "UNKNOWN"; + } +} + + +/* Write the log to the file named by -E instead of stderr. The format + * matches DefaultLoggingCb() so the two are comparable. The callback cannot + * be uninstalled, so fall back to stderr when the file isn't open. */ +static void ClientLoggingCb(enum wolfSSH_LogLevel level, const char *const str) +{ + WFILE* out = (logFileStream != NULL) ? logFileStream : stderr; + char timeStr[24]; + + timeStr[0] = '\0'; +#ifndef WOLFSSH_NO_TIMESTAMP + { + time_t current; + struct tm local; + + current = WTIME(NULL); + if (WLOCALTIME(¤t, &local)) { + strftime(timeStr, sizeof(timeStr), "%F %T ", &local); + } + } +#endif + fprintf(out, "%s[%s] %s\r\n", timeStr, ClientLogLevelStr(level), str); + /* flush so the log is complete when the client is interrupted */ + fflush(out); +} + static void ShowUsage(char* appPath) { @@ -748,6 +802,11 @@ struct config { }; +/* Parsed by main() before wolfSSH_Init() so the -E log file catches the + * library's start up messages. */ +static struct config clientConfig; + + static int config_init_default(struct config* config) { char* env; @@ -972,37 +1031,31 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args) byte useAgent = 0; WS_AgentCbActionCtx agentCbCtx; #endif - struct config config; MODES_STORE(); ((func_args*)args)->return_code = 0; - config_init_default(&config); - config_parse_command_line(&config, - ((func_args*)args)->argc, ((func_args*)args)->argv); - config_print(&config); - /* Only ask for an interactive terminal session when no remote command * was given. Requesting both discards the command. */ - keepOpen = (byte)(config.command == NULL); + keepOpen = (byte)(clientConfig.command == NULL); #ifdef WOLFSSH_AGENT - useAgent = (byte)config.useAgent; + useAgent = (byte)clientConfig.useAgent; #endif - if (config.user == NULL) + if (clientConfig.user == NULL) err_sys("client requires a username parameter."); - if (config.hostname == NULL) + if (clientConfig.hostname == NULL) err_sys("client requires a hostname parameter."); #ifdef SINGLE_THREADED err_sys("Threading needed for terminal and command sessions\n"); #endif - if (config.keyFile) { - ret = ClientSetPrivateKey(config.keyFile); + if (clientConfig.keyFile) { + ret = ClientSetPrivateKey(clientConfig.keyFile); if (ret == 0) { #ifdef WOLFSSH_CERTS /* passed in certificate to use */ @@ -1011,8 +1064,8 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args) } else #endif - if (config.pubKeyFile) { - (void)ClientUsePubKey(config.pubKeyFile); + if (clientConfig.pubKeyFile) { + (void)ClientUsePubKey(clientConfig.pubKeyFile); } } } @@ -1054,13 +1107,13 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args) } #endif - wolfSSH_SetPublicKeyCheckCtx(ssh, (void*)config.hostname); + wolfSSH_SetPublicKeyCheckCtx(ssh, (void*)clientConfig.hostname); - ret = wolfSSH_SetUsername(ssh, config.user); + ret = wolfSSH_SetUsername(ssh, clientConfig.user); if (ret != WS_SUCCESS) err_sys("Couldn't set the username."); - build_addr(&clientAddr, config.hostname, config.port); + build_addr(&clientAddr, clientConfig.hostname, clientConfig.port); tcp_socket(&sockFd, ((struct sockaddr_in *)&clientAddr)->sin_family); ret = connect(sockFd, (const struct sockaddr *)&clientAddr, clientAddrSz); @@ -1073,10 +1126,10 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args) if (ret != WS_SUCCESS) err_sys("Couldn't set the session's socket."); - if (config.command != NULL) { + if (clientConfig.command != NULL) { ret = wolfSSH_SetChannelType(ssh, WOLFSSH_SESSION_EXEC, - (byte*)config.command, - (word32)WSTRLEN((char*)config.command)); + (byte*)clientConfig.command, + (word32)WSTRLEN((char*)clientConfig.command)); if (ret != WS_SUCCESS) err_sys("Couldn't set the channel type."); } @@ -1107,7 +1160,7 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args) /* Every session, shell or command, runs its I/O on threads. */ { - #if defined(_POSIX_THREADS) +#if defined(_POSIX_THREADS) thread_args arg; pthread_t thread[3]; @@ -1120,7 +1173,7 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args) err_sys("Couldn't initialize window semaphore."); } - if (config.command) { + if (clientConfig.command) { int err; /* exec command does not contain initial terminal size, @@ -1151,7 +1204,7 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args) wolfSSH_SEMAPHORE_Release(&windowSem); #endif /* WOLFSSH_TERM */ ioErr = arg.readError; - #elif defined(_MSC_VER) +#elif defined(_MSC_VER) thread_args arg; HANDLE thread[2]; @@ -1160,7 +1213,7 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args) arg.readError = 0; wc_InitMutex(&arg.lock); - if (config.command) { + if (clientConfig.command) { int err; /* exec command does not contain initial terminal size, @@ -1178,9 +1231,9 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args) CloseHandle(thread[0]); CloseHandle(thread[1]); ioErr = arg.readError; - #else +#else err_sys("No threading to use"); - #endif +#endif if (keepOpen) ClientSetEcho(1); } @@ -1230,7 +1283,6 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args) wc_ecc_fp_free(); /* free per thread cache */ #endif - config_cleanup(&config); MODES_RESET(); return 0; @@ -1248,6 +1300,23 @@ int main(int argc, char** argv) WSTARTTCP(); + config_init_default(&clientConfig); + config_parse_command_line(&clientConfig, argc, argv); + config_print(&clientConfig); + + /* Install the log callback before wolfSSH_Init() so the file named by + * -E gets the library's start up messages too. */ + if (clientConfig.logFile != NULL) { + if (WFOPEN(NULL, &logFileStream, clientConfig.logFile, "ab") != 0 + || logFileStream == WBADFILE) { + err_sys("Couldn't open the log file."); + } + wolfSSH_SetLoggingCb(ClientLoggingCb); + /* Asking for a log file is asking for logging. A no-op when the + * library has none compiled in, same as wolfsshd's -d. */ + wolfSSH_Debugging_ON(); + } + #ifdef DEBUG_WOLFSSH wolfSSH_Debugging_ON(); #endif @@ -1258,5 +1327,13 @@ int main(int argc, char** argv) wolfSSH_Cleanup(); + /* Close the log last, wolfSSH_Cleanup() still logs and the callback + * cannot be uninstalled. */ + if (logFileStream != NULL) { + WFCLOSE(NULL, logFileStream); + logFileStream = NULL; + } + config_cleanup(&clientConfig); + return args.return_code; } From 7eec4a43484a17bdf1c2cf85f2a0792b0188959a Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 11 Aug 2026 09:01:06 -0700 Subject: [PATCH 02/10] Build and test the wolfssh client app in CI - --enable-sshclient defaults to no, so the app was built only by the configs that use --enable-all, and never under the multi-compiler warning flags. Add it to the multi-compiler matrix. - Add scripts/sshclient.test, run by make check. It covers the client's sessions and the -E log file against the echoserver. - The script is not gated on BUILD_SSHCLIENT. It exits 77 when the client app or the echoserver isn't there, so every build runs it and the ones without the app report it as a skip. - Check the client and the echoserver by asking each for its usage message, not by looking for the file. Both are libtool wrapper scripts in the build tree, and a wrapper outlives a reconfigure that drops the program it wraps, then runs only far enough to say so. - The echoserver runs in echo mode and the client's stdin comes from a fifo written a piece at a time, so the session carries data and ends on its own. Each client run has a watchdog. - Rename sshd-test.yml's job to cover both apps. That workflow builds the client app along with wolfsshd. - Check that the command reaches the server, now that the client sends it rather than discarding it. - Make the SINGLE_THREADED guard a preprocessor #error. The runtime err_sys() only caught the misconfiguration in an autotools build that got as far as running; the #error catches it at compile time for the IDE and plain Makefile builds too. - Treat WS_WANT_READ and WS_WANT_WRITE out of wolfSSH_worker() as a clean shutdown. The socket is non-blocking, so the peer having nothing ready is not a session failure. --- .github/workflows/multi-compiler.yml | 2 +- .github/workflows/sshd-test.yml | 2 +- apps/wolfssh/wolfssh.c | 15 +- scripts/include.am | 4 + scripts/sshclient.test | 280 +++++++++++++++++++++++++++ 5 files changed, 295 insertions(+), 8 deletions(-) create mode 100755 scripts/sshclient.test diff --git a/.github/workflows/multi-compiler.yml b/.github/workflows/multi-compiler.yml index 2305c0c89..1ae8d4b85 100644 --- a/.github/workflows/multi-compiler.yml +++ b/.github/workflows/multi-compiler.yml @@ -88,7 +88,7 @@ jobs: CXX: ${{ matrix.cxx }} run: | ./autogen.sh - ./configure CFLAGS="-Wall -Wextra -Wpedantic" + ./configure --enable-sshclient CFLAGS="-Wall -Wextra -Wpedantic" make -j$(nproc) - name: Make dist diff --git a/.github/workflows/sshd-test.yml b/.github/workflows/sshd-test.yml index b98a5c009..c3e06d8e5 100644 --- a/.github/workflows/sshd-test.yml +++ b/.github/workflows/sshd-test.yml @@ -72,7 +72,7 @@ jobs: os: [ ubuntu-latest ] wolfssl: ${{ fromJson(needs.create_matrix.outputs['versions']) }} mldsa: [ 'yes', 'no' ] - name: Build and test wolfsshd + name: Build and test the wolfsshd and wolfssh apps runs-on: ${{ matrix.os }} timeout-minutes: 10 steps: diff --git a/apps/wolfssh/wolfssh.c b/apps/wolfssh/wolfssh.c index 4507c3d94..3ecddfd1e 100644 --- a/apps/wolfssh/wolfssh.c +++ b/apps/wolfssh/wolfssh.c @@ -82,6 +82,10 @@ #include #endif +#ifdef SINGLE_THREADED + #error "Threading needed for terminal and command sessions." +#endif + int myoptind = 0; char* myoptarg = NULL; @@ -1050,10 +1054,6 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args) if (clientConfig.hostname == NULL) err_sys("client requires a hostname parameter."); -#ifdef SINGLE_THREADED - err_sys("Threading needed for terminal and command sessions\n"); -#endif - if (clientConfig.keyFile) { ret = ClientSetPrivateKey(clientConfig.keyFile); if (ret == 0) { @@ -1249,8 +1249,11 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args) else { ret = wolfSSH_worker(ssh, NULL); } - if (ret == WS_CHANNEL_CLOSED) { - /* Shutting down, channel closing isn't a fail. */ + if (ret == WS_CHANNEL_CLOSED || ret == WS_WANT_READ + || ret == WS_WANT_WRITE) { + /* Shutting down. The channel closing isn't a fail, and neither + * is the peer having nothing ready on this non-blocking socket; + * either way there is nothing left to wait for. */ ret = WS_SUCCESS; } else if (ret != WS_SUCCESS) { diff --git a/scripts/include.am b/scripts/include.am index 2f7693625..4fbfe39ad 100644 --- a/scripts/include.am +++ b/scripts/include.am @@ -11,5 +11,9 @@ if BUILD_SCP dist_noinst_SCRIPTS+= scripts/scp.test endif +# Not gated on BUILD_SSHCLIENT. The script skips itself when the client +# app wasn't built. +dist_noinst_SCRIPTS+= scripts/sshclient.test + dist_noinst_SCRIPTS+= scripts/external.test scripts/fwd.test EXTRA_DIST += scripts/fwd.test.expect diff --git a/scripts/sshclient.test b/scripts/sshclient.test new file mode 100755 index 000000000..58c02bd27 --- /dev/null +++ b/scripts/sshclient.test @@ -0,0 +1,280 @@ +#!/bin/sh + +# wolfssh client app test +# +# Runs the wolfssh client against the echoserver, covering the remote +# command session, the terminal session, and the -E log file option. + +no_pid=-1 +server_pid=$no_pid +client_pid=$no_pid +input_pid=$no_pid +killer_pid=$no_pid +work_dir=`pwd`/wolfssh_client_test$$ +ready_file=$work_dir/ready +input_file=$work_dir/input +client_out=$work_dir/client.out +port=0 +counter=0 +# Seconds to give the client before killing it. Nothing here takes more +# than a moment, the limit is only so a stuck session fails this test +# instead of hanging make check. +client_limit=60 + +[ ! -x ./apps/wolfssh/wolfssh ] \ + && echo "wolfssh client app doesn't exist, skipping" && exit 77 +./apps/wolfssh/wolfssh -h 2>&1 | grep -q "usage: " \ + || { echo "wolfssh client app doesn't run, skipping"; exit 77; } +[ ! -x ./examples/echoserver/echoserver ] \ + && echo "echoserver doesn't exist, skipping" && exit 77 +./examples/echoserver/echoserver '-?' 2>&1 | grep -q "^echoserver " \ + || { echo "echoserver doesn't run, skipping"; exit 77; } + +do_cleanup() { + echo "in cleanup" + + if [ $killer_pid != $no_pid ] + then + kill $killer_pid 2>/dev/null + killer_pid=$no_pid + fi + if [ $input_pid != $no_pid ] + then + kill $input_pid 2>/dev/null + input_pid=$no_pid + fi + if [ $client_pid != $no_pid ] + then + echo "killing client" + kill -9 $client_pid 2>/dev/null + client_pid=$no_pid + fi + if [ $server_pid != $no_pid ] + then + echo "killing server" + kill -9 $server_pid 2>/dev/null + server_pid=$no_pid + fi + rm -rf $work_dir +} + +do_trap() { + echo "got trap" + do_cleanup + exit 1 +} + +trap do_trap INT TERM + +# The echoserver is one shot, start a new one for each connection. It picks +# an ephemeral port and writes it to the ready file. +# +# -f keeps the server in echo mode. Without it a build with shell support +# tries to fork a login shell for the user, which fails since jill isn't a +# real account, and the session ends before anything crosses the channel. +start_server() { + # The -1 server exits after its connection, but a client run that failed + # before connecting leaves one listening. Reap it, server_pid is about + # to be overwritten. + if [ $server_pid != $no_pid ] + then + kill -9 $server_pid 2>/dev/null + wait $server_pid 2>/dev/null + server_pid=$no_pid + fi + + rm -f $ready_file + ./examples/echoserver/echoserver -1 -f -R $ready_file \ + > $work_dir/server.log 2>&1 & + server_pid=$! + + # A debug build starting up under a parallel make check needs more than + # the couple of seconds the other scripts allow. + counter=0 + while [ ! -s "$ready_file" ] && [ "$counter" -lt 100 ]; do + echo "waiting for ready file..." + sleep 0.1 + counter=$((counter + 1)) + done + + if [ ! -s "$ready_file" ]; then + echo -e "\n\nNO ready file ending test..." + do_cleanup + exit 1 + fi + + port=`cat $ready_file` + echo "server listening on port $port" +} + +fail() { + echo -e "\n\n$1" + do_cleanup + exit 1 +} + +# Wait for the client to write something to its output. The prompts are +# flushed as they are printed, so this tells us the client is about to read +# the answer. +wait_for_output() { + count=0 + while [ "$count" -lt 300 ]; do + grep -q "$1" $client_out 2>/dev/null && return 0 + sleep 0.1 + count=$((count + 1)) + done + return 1 +} + +# Run the client with its stdin coming from a fifo. +# +# The client answers its prompts with stdio, which buffers everything that +# is ready to be read, so anything written along with a prompt's answer is +# swallowed with it and never reaches the session. Writing each piece only +# once the client has asked for it keeps them in separate reads. The +# echoserver only ends the session when it receives a 0x03, so every +# session has to send one or both ends wait for the other forever. +# +# $1 - "confirm" when the client will ask about the unknown server key +# rest - client arguments +run_client() { + confirm=$1 + shift + + rm -f $input_file $client_out + touch $client_out + mkfifo $input_file || fail "couldn't create the input fifo" + + ( + # GetConfirmation() reads a single character. A newline here would + # be left behind for the password prompt to read as an empty + # password. + [ "$confirm" = "confirm" ] && printf 'Y' + + wait_for_output "Password:" || exit 1 + printf 'upthehill\n' + + # Let the client consume the password before sending the session + # data. A single read that catches both loses the data. + sleep 2 + + printf 'hello\003' + ) > $input_file 2>/dev/null & + input_pid=$! + + HOME=$work_dir ./apps/wolfssh/wolfssh "$@" \ + < $input_file > $client_out 2>&1 & + client_pid=$! + + # Poll rather than sleep through the whole limit. Killing a subshell + # that is waiting on a sleep leaves the sleep running. + ( + watched=0 + while kill -0 $client_pid 2>/dev/null; do + if [ $watched -ge $client_limit ]; then + kill -9 $client_pid 2>/dev/null + break + fi + sleep 1 + watched=$((watched + 1)) + done + ) 2>/dev/null & + killer_pid=$! + + wait $client_pid + client_status=$? + client_pid=$no_pid + + kill $killer_pid 2>/dev/null + killer_pid=$no_pid + kill $input_pid 2>/dev/null + input_pid=$no_pid + + cat $client_out + return $client_status +} + +mkdir -p $work_dir/.ssh + +# The known hosts check rejects a missing or empty file without asking, so +# seed the file with an entry for another host. The first connection then +# gets the "server is unknown" prompt and answers it. +echo "example.invalid ssh-rsa AAAA" > $work_dir/.ssh/known_hosts + +echo "Test learning the server's key" +start_server +run_client confirm -E $work_dir/learn.log -p $port jill@127.0.0.1 "echo one" +RESULT=$? + +if [ $RESULT -ne 0 ]; then + [ $RESULT -gt 128 ] && fail "the client had to be killed, session stuck" + fail "failed to connect" +fi + +grep -q "^127.0.0.1 " $work_dir/.ssh/known_hosts \ + || fail "server key not added to the known hosts" +grep -q "hello" $client_out \ + || fail "the echoserver's reply didn't make it back" + +# With the server's key known, the client only prompts for the password. +# The log file is empty unless the library has logging compiled in. +echo "Test a session given a command, with a log file" +start_server +run_client "" -E $work_dir/command.log -p $port jill@127.0.0.1 "echo two" +[ $? -ne 0 ] && fail "failed to open the session" + +grep -q "hello" $client_out \ + || fail "the echoserver's reply didn't make it back" + +if [ -s $work_dir/command.log ]; then + echo "checking the log file" + + # The log is redirected before wolfSSH_Init(), so the library's start up + # message is the first thing in the file. + head -n 1 $work_dir/command.log | grep -q "Entering wolfSSH_Init()" \ + || fail "log file is missing the wolfSSH_Init() message" + + # The log is closed after wolfSSH_Cleanup(), which logs as well. + grep -q "Leaving wolfSSH_Cleanup()" $work_dir/command.log \ + || fail "log file is missing the wolfSSH_Cleanup() message" + + # Given a command the client opens an exec channel to carry it, with no + # terminal request to discard it. + grep -q "type = exec" $work_dir/command.log \ + || fail "the client didn't open an exec channel for the command" + + # The command string itself is only logged by a debug build. + if grep -q " command = " $work_dir/command.log; then + grep -q "command = echo two" $work_dir/command.log \ + || fail "the client didn't send the command it was given" + fi +else + echo "empty log file, library built without logging" +fi + +echo "Test terminal session" +start_server +run_client "" -E $work_dir/terminal.log -p $port jill@127.0.0.1 +[ $? -ne 0 ] && fail "failed to open the terminal session" + +grep -q "hello" $client_out \ + || fail "the echoserver's reply didn't make it back" + +if [ -s $work_dir/terminal.log ]; then + grep -q "Leaving wolfSSH_Cleanup()" $work_dir/terminal.log \ + || fail "log file is missing the wolfSSH_Cleanup() message" + + # No command was given, the client asks for a terminal instead. + grep -q "type = exec" $work_dir/terminal.log \ + && fail "the client opened an exec channel it wasn't asked for" + grep -q " command = " $work_dir/terminal.log \ + && fail "the client sent a command it wasn't given" +fi + +echo "Test the usage message" +./apps/wolfssh/wolfssh -Z 2>&1 | grep -q "usage:" \ + || fail "no usage message for a bad option" + +do_cleanup +echo "wolfssh client tests passed" +exit 0 From c100950475e909ddc51f73c7574ba1d015a7fdda Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 12 Aug 2026 16:20:22 -0700 Subject: [PATCH 03/10] Keep a failed shutdown send out of the client's exit status wolfSSH_shutdown() returns WS_WANT_WRITE when the channel EOF, exit and close messages are still queued on the non-blocking socket. Masking that to WS_SUCCESS reported a clean exit for a session whose close messages never reached the peer. Mask a want write from the drain worker only, where the close messages are already sent. The want read masking stays on both, wolfSSH_shutdown() runs a worker of its own and passes that want read back. --- apps/wolfssh/wolfssh.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/apps/wolfssh/wolfssh.c b/apps/wolfssh/wolfssh.c index 3ecddfd1e..f6fb9ee94 100644 --- a/apps/wolfssh/wolfssh.c +++ b/apps/wolfssh/wolfssh.c @@ -1248,12 +1248,18 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args) } else { ret = wolfSSH_worker(ssh, NULL); + if (ret == WS_WANT_WRITE) { + /* The close messages are already out, whatever the drain + * still wants to send is a reply to the peer. */ + ret = WS_SUCCESS; + } } - if (ret == WS_CHANNEL_CLOSED || ret == WS_WANT_READ - || ret == WS_WANT_WRITE) { + if (ret == WS_CHANNEL_CLOSED || ret == WS_WANT_READ) { /* Shutting down. The channel closing isn't a fail, and neither * is the peer having nothing ready on this non-blocking socket; - * either way there is nothing left to wait for. */ + * either way there is nothing left to wait for. A want write + * from wolfSSH_shutdown() is different, the close messages are + * still queued, so that stays a failure. */ ret = WS_SUCCESS; } else if (ret != WS_SUCCESS) { From 46b9af162ce4302035201ab077923d9a8e092e41 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 12 Aug 2026 16:19:55 -0700 Subject: [PATCH 04/10] Don't build the client app against a single threaded wolfSSL The client runs every session's I/O on threads, so it needs a threaded wolfSSL. configure probes for SINGLE_THREADED when the client app is enabled. Asking for the app with --enable-sshclient is an error, getting it from --enable-all drops the app instead, so --enable-all still configures against a single threaded wolfSSL. The compile time check stays for the builds that never run configure. That leaves the SINGLE_THREADED terms in the app's own guards unreachable, so drop them. --- apps/wolfssh/README.md | 5 +++++ apps/wolfssh/wolfssh.c | 10 ++++++---- configure.ac | 22 ++++++++++++++++++++++ 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/apps/wolfssh/README.md b/apps/wolfssh/README.md index cc368ac4c..80bcb2760 100644 --- a/apps/wolfssh/README.md +++ b/apps/wolfssh/README.md @@ -12,6 +12,11 @@ Phase 2 is going to bring reading the config files `/etc/ssh/ssh_config` and `$HOME/.ssh/config`. It will handle OpenSSH style modern keys. It will also have support for SSH-AGENT and forwarding. +Every session, terminal or command, runs its I/O on threads, so the client +needs a threaded wolfSSL. Configuring `--enable-sshclient` against a +single-threaded wolfSSL is an error, and `--enable-all` leaves the client out +rather than failing. + Command Line Options -------------------- diff --git a/apps/wolfssh/wolfssh.c b/apps/wolfssh/wolfssh.c index f6fb9ee94..b18232a41 100644 --- a/apps/wolfssh/wolfssh.c +++ b/apps/wolfssh/wolfssh.c @@ -82,8 +82,10 @@ #include #endif +/* Every session, terminal or command, runs its I/O on threads. configure + * catches this first; the check is here for the builds that don't use it. */ #ifdef SINGLE_THREADED - #error "Threading needed for terminal and command sessions." + #error "The wolfSSH client app requires a threaded wolfSSL." #endif @@ -260,7 +262,7 @@ static void modes_reset(void) #define MODES_RESET() do {} while(0) #endif /* HAVE_TERMIOS_H && WOLFSSH_TERM */ -#if !defined(SINGLE_THREADED) && !defined(WOLFSSL_NUCLEUS) +#ifndef WOLFSSL_NUCLEUS #if defined(WOLFSSH_AGENT) static inline void ato32(const byte* c, word32* u32) @@ -657,7 +659,7 @@ static THREAD_RET readPeer(void* in) return THREAD_RET_SUCCESS; } -#endif /* !SINGLE_THREADED && !WOLFSSL_NUCLEUS */ +#endif /* !WOLFSSL_NUCLEUS */ #if defined(WOLFSSL_PTHREADS) && defined(WOLFSSL_TEST_GLOBAL_REQ) @@ -1152,7 +1154,7 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args) MODES_CLEAR(); } -#if !defined(SINGLE_THREADED) && !defined(WOLFSSL_NUCLEUS) +#ifndef WOLFSSL_NUCLEUS #if 0 if (keepOpen) /* set up for pseudo-terminal */ ClientSetEcho(2); diff --git a/configure.ac b/configure.ac index 279d491c5..80b07a47a 100644 --- a/configure.ac +++ b/configure.ac @@ -259,6 +259,28 @@ AS_IF([test "x$ENABLED_ALL" = "xyes"], AS_IF([test "x$ENABLED_SSHD" = "xyes"], [ENABLED_SHELL=yes]) +# The client app runs every session's I/O on threads, so it needs a threaded +# wolfSSL. Probe for the macro rather than trust the flags, it arrives through +# wolfSSL's options.h. Asking for the client outright is an error; getting it +# from --enable-all only drops it, so --enable-all still works here. +AS_IF([test "x$ENABLED_SSHCLIENT" = "xyes"],[ + AC_MSG_CHECKING([whether wolfSSL is single threaded]) + AC_COMPILE_IFELSE( + [AC_LANG_PROGRAM([[#ifdef WOLFSSL_USER_SETTINGS + #include + #else + #include + #endif]], + [[#ifndef SINGLE_THREADED + #error "threaded" + #endif]])], + [AC_MSG_RESULT([yes]) + AS_IF([test "x$enable_sshclient" = "xyes"], + [AC_MSG_ERROR([--enable-sshclient requires a threaded wolfSSL.])], + [AC_MSG_NOTICE([single threaded wolfSSL, not building the ssh client app]) + ENABLED_SSHCLIENT=no])], + [AC_MSG_RESULT([no])])]) + # Set the defined flags for the code. AS_IF([test "x$ENABLED_INLINE" = "xno"], [AM_CPPFLAGS="$AM_CPPFLAGS -DNO_INLINE"]) From f78ccf237540866b1adfdb46bec5f9560b973b72 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 13 Aug 2026 14:58:21 -0700 Subject: [PATCH 05/10] Flush a queued client write instead of waiting on the peer A send the socket wasn't ready for stays queued but still reports the data as taken, so the client waited on a reply to a message it never sent. Flush after a queued send, a terminal size change, and at shutdown. The shutdown drain reports its want read as WS_FATAL_ERROR, so read the status with wolfSSH_get_error(); an ordinary shutdown was exiting 1. Time out readPeer()'s select() so a flush can't strand the reader. --- apps/wolfssh/wolfssh.c | 105 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 93 insertions(+), 12 deletions(-) diff --git a/apps/wolfssh/wolfssh.c b/apps/wolfssh/wolfssh.c index b18232a41..d8dab620f 100644 --- a/apps/wolfssh/wolfssh.c +++ b/apps/wolfssh/wolfssh.c @@ -291,6 +291,53 @@ typedef struct thread_args { #endif +/* Sleep long enough for the socket to drain, the socket is non-blocking and + * a busy retry loop would only starve the peer. */ +static void PauseForSocket(void) +{ +#ifdef USE_WINDOWS_API + Sleep(1); +#else + usleep(1000); +#endif +} + + +/* A packet the socket wasn't ready for stays queued in the session, and the + * send that queued it still reports the data as taken. The peer can't answer + * a message it never received, so push the queue out here rather than go + * back to waiting on the peer. The lock is NULL when no other thread is + * using the session. */ +static int FlushQueuedSend(WOLFSSH* ssh, wolfSSL_Mutex* lock) +{ + int ret; + + do { + PauseForSocket(); + + if (lock != NULL) { + wc_LockMutex(lock); + } + ret = wolfSSH_worker(ssh, NULL); + if (ret == WS_FATAL_ERROR) { + /* the session holds the detail behind a fatal error */ + ret = wolfSSH_get_error(ssh); + } + if (lock != NULL) { + wc_UnLockMutex(lock); + } + } while (ret == WS_WANT_WRITE); + + /* The queue is out. Whatever the worker made of the peer's end of the + * conversation is for the reader to sort out. */ + if (ret == WS_WANT_READ || ret == WS_CHAN_RXD || ret == WS_EXTDATA) { + ret = WS_SUCCESS; + } + + return ret; +} + + #ifdef WOLFSSH_TERM static int sendCurrentWindowSize(thread_args* args) { @@ -322,6 +369,10 @@ static int sendCurrentWindowSize(thread_args* args) ret = wolfSSH_ChangeTerminalSize(args->ssh, col, row, xpix, ypix); wc_UnLockMutex(&args->lock); + if (ret == WS_WANT_WRITE) { + ret = FlushQueuedSend(args->ssh, &args->lock); + } + return ret; } @@ -452,6 +503,7 @@ static THREAD_RET readInput(void* in) thread_args* args = (thread_args*)in; int ret = 0; int err = 0; + int queued = 0; word32 sz = 0; #ifdef USE_WINDOWS_API HANDLE stdinHandle = GetStdHandle(STD_INPUT_HANDLE); @@ -478,21 +530,24 @@ static THREAD_RET readInput(void* in) ret = wolfSSH_stream_send(args->ssh, buf, sz); err = (ret == WS_FATAL_ERROR) ? wolfSSH_get_error(args->ssh) : ret; + /* A send the socket wasn't ready for still counts the data as + * taken, it is left queued in the session instead. */ + queued = (wolfSSH_get_error(args->ssh) == WS_WANT_WRITE); wc_UnLockMutex(&args->lock); if (err == WS_REKEYING) { /* give readPeer() the lock to finish the rekey, then * send this buffer again */ - #ifdef USE_WINDOWS_API - Sleep(1); - #else - usleep(1000); - #endif + PauseForSocket(); } } while (err == WS_REKEYING); if (ret <= 0) { fprintf(stderr, "Couldn't send data\n"); break; } + if (queued && FlushQueuedSend(args->ssh, &args->lock) != WS_SUCCESS) { + fprintf(stderr, "Couldn't send data\n"); + break; + } } #if !defined(WOLFSSH_NO_ECC) && defined(FP_ECC) && defined(HAVE_THREAD_LS) wc_ecc_fp_free(); /* free per thread cache */ @@ -515,6 +570,7 @@ static THREAD_RET readPeer(void* in) #endif fd_set readSet; fd_set errSet; + struct timeval timeout; #ifdef USE_WINDOWS_API if (args->rawMode == 0) { @@ -552,7 +608,16 @@ static THREAD_RET readPeer(void* in) FD_SET(fd, &readSet); FD_SET(fd, &errSet); - bytes = select(fd + 1, &readSet, NULL, &errSet, NULL); + timeout.tv_sec = 1; + timeout.tv_usec = 0; + bytes = select(fd + 1, &readSet, NULL, &errSet, &timeout); + if (bytes == 0) { + /* Nothing new on the socket, but a flush in the send thread may + * have already taken the peer's reply off it, so run the read + * path anyway. It only costs an empty read. */ + bytes = 1; + FD_SET(fd, &readSet); + } wc_LockMutex(&args->lock); while (bytes > 0 && (FD_ISSET(fd, &readSet) || FD_ISSET(fd, &errSet))) { /* there is something to read off the wire */ @@ -1242,26 +1307,42 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args) #endif ret = wolfSSH_shutdown(ssh); + /* WS_FATAL_ERROR only says to go look, the session has the detail. The + * drain inside the shutdown reports a want read that way. */ + if (ret == WS_FATAL_ERROR) { + ret = wolfSSH_get_error(ssh); + } + /* do not continue on with shutdown process if peer already disconnected */ if (ret != WS_SOCKET_ERROR_E && wolfSSH_get_error(ssh) != WS_SOCKET_ERROR_E) { - if (ret != WS_SUCCESS) { - WLOG(WS_LOG_DEBUG, "Sending the shutdown messages failed."); +#ifndef WOLFSSL_NUCLEUS + if (ret == WS_WANT_WRITE) { + /* The close messages are queued and the threads are done, no + * one else is going to send them. */ + ret = FlushQueuedSend(ssh, NULL); } - else { +#endif + + if (ret == WS_SUCCESS) { ret = wolfSSH_worker(ssh, NULL); + if (ret == WS_FATAL_ERROR) { + ret = wolfSSH_get_error(ssh); + } if (ret == WS_WANT_WRITE) { /* The close messages are already out, whatever the drain * still wants to send is a reply to the peer. */ ret = WS_SUCCESS; } } + else if (ret != WS_CHANNEL_CLOSED && ret != WS_WANT_READ) { + WLOG(WS_LOG_DEBUG, "Sending the shutdown messages failed."); + } + if (ret == WS_CHANNEL_CLOSED || ret == WS_WANT_READ) { /* Shutting down. The channel closing isn't a fail, and neither * is the peer having nothing ready on this non-blocking socket; - * either way there is nothing left to wait for. A want write - * from wolfSSH_shutdown() is different, the close messages are - * still queued, so that stays a failure. */ + * either way there is nothing left to wait for. */ ret = WS_SUCCESS; } else if (ret != WS_SUCCESS) { From f69252f50db870f6aa1aacaed0a4436050d37e4d Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 13 Aug 2026 14:58:21 -0700 Subject: [PATCH 06/10] Skip the client app test in a forced non-blocking build The echoserver needs -N under WOLFSSH_TEST_BLOCK, and even with it leaves a failed write queued while it waits on the peer, so a session stalls. scp.test and get-put.test skip the build too. --- scripts/sshclient.test | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/scripts/sshclient.test b/scripts/sshclient.test index 58c02bd27..d3ad9035e 100755 --- a/scripts/sshclient.test +++ b/scripts/sshclient.test @@ -30,6 +30,17 @@ client_limit=60 ./examples/echoserver/echoserver '-?' 2>&1 | grep -q "^echoserver " \ || { echo "echoserver doesn't run, skipping"; exit 77; } +# A WOLFSSH_TEST_BLOCK build fails writes at random. The echoserver leaves a +# failed write queued and then waits on the peer for a reply to the message +# it never sent, so a session stalls no matter what the client does. The +# other echoserver scripts skip this build for the same reason. +if [ -x ./examples/client/client ] \ + && ./examples/client/client -h 2>&1 | grep -q "WOLFSSH_TEST_BLOCK" +then + echo "macro WOLFSSH_TEST_BLOCK was used, skipping" + exit 77 +fi + do_cleanup() { echo "in cleanup" From a8e01edeca5fe606a29b8e1e05b526f00ca4ba6a Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 14 Aug 2026 09:27:47 -0700 Subject: [PATCH 07/10] Quote the client test's paths and use printf for escapes The paths are built from pwd, so an unquoted use split on a build directory with a space in it, and the cleanup's rm -rf then deleted whatever the first word named. - Quote work_dir and every path derived from it - Pass the directory to rm after -- - Replace the two echo -e calls, dash prints a literal -e --- scripts/sshclient.test | 76 +++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/scripts/sshclient.test b/scripts/sshclient.test index d3ad9035e..af145f968 100755 --- a/scripts/sshclient.test +++ b/scripts/sshclient.test @@ -10,10 +10,10 @@ server_pid=$no_pid client_pid=$no_pid input_pid=$no_pid killer_pid=$no_pid -work_dir=`pwd`/wolfssh_client_test$$ -ready_file=$work_dir/ready -input_file=$work_dir/input -client_out=$work_dir/client.out +work_dir="`pwd`/wolfssh_client_test$$" +ready_file="$work_dir/ready" +input_file="$work_dir/input" +client_out="$work_dir/client.out" port=0 counter=0 # Seconds to give the client before killing it. Nothing here takes more @@ -66,7 +66,7 @@ do_cleanup() { kill -9 $server_pid 2>/dev/null server_pid=$no_pid fi - rm -rf $work_dir + rm -rf -- "$work_dir" } do_trap() { @@ -94,9 +94,9 @@ start_server() { server_pid=$no_pid fi - rm -f $ready_file - ./examples/echoserver/echoserver -1 -f -R $ready_file \ - > $work_dir/server.log 2>&1 & + rm -f "$ready_file" + ./examples/echoserver/echoserver -1 -f -R "$ready_file" \ + > "$work_dir/server.log" 2>&1 & server_pid=$! # A debug build starting up under a parallel make check needs more than @@ -109,17 +109,17 @@ start_server() { done if [ ! -s "$ready_file" ]; then - echo -e "\n\nNO ready file ending test..." + printf '\n\nNO ready file ending test...\n' do_cleanup exit 1 fi - port=`cat $ready_file` + port=`cat "$ready_file"` echo "server listening on port $port" } fail() { - echo -e "\n\n$1" + printf '\n\n%s\n' "$1" do_cleanup exit 1 } @@ -130,7 +130,7 @@ fail() { wait_for_output() { count=0 while [ "$count" -lt 300 ]; do - grep -q "$1" $client_out 2>/dev/null && return 0 + grep -q "$1" "$client_out" 2>/dev/null && return 0 sleep 0.1 count=$((count + 1)) done @@ -152,9 +152,9 @@ run_client() { confirm=$1 shift - rm -f $input_file $client_out - touch $client_out - mkfifo $input_file || fail "couldn't create the input fifo" + rm -f "$input_file" "$client_out" + touch "$client_out" + mkfifo "$input_file" || fail "couldn't create the input fifo" ( # GetConfirmation() reads a single character. A newline here would @@ -170,11 +170,11 @@ run_client() { sleep 2 printf 'hello\003' - ) > $input_file 2>/dev/null & + ) > "$input_file" 2>/dev/null & input_pid=$! - HOME=$work_dir ./apps/wolfssh/wolfssh "$@" \ - < $input_file > $client_out 2>&1 & + HOME="$work_dir" ./apps/wolfssh/wolfssh "$@" \ + < "$input_file" > "$client_out" 2>&1 & client_pid=$! # Poll rather than sleep through the whole limit. Killing a subshell @@ -201,20 +201,20 @@ run_client() { kill $input_pid 2>/dev/null input_pid=$no_pid - cat $client_out + cat "$client_out" return $client_status } -mkdir -p $work_dir/.ssh +mkdir -p "$work_dir/.ssh" # The known hosts check rejects a missing or empty file without asking, so # seed the file with an entry for another host. The first connection then # gets the "server is unknown" prompt and answers it. -echo "example.invalid ssh-rsa AAAA" > $work_dir/.ssh/known_hosts +echo "example.invalid ssh-rsa AAAA" > "$work_dir/.ssh/known_hosts" echo "Test learning the server's key" start_server -run_client confirm -E $work_dir/learn.log -p $port jill@127.0.0.1 "echo one" +run_client confirm -E "$work_dir/learn.log" -p $port jill@127.0.0.1 "echo one" RESULT=$? if [ $RESULT -ne 0 ]; then @@ -222,41 +222,41 @@ if [ $RESULT -ne 0 ]; then fail "failed to connect" fi -grep -q "^127.0.0.1 " $work_dir/.ssh/known_hosts \ +grep -q "^127.0.0.1 " "$work_dir/.ssh/known_hosts" \ || fail "server key not added to the known hosts" -grep -q "hello" $client_out \ +grep -q "hello" "$client_out" \ || fail "the echoserver's reply didn't make it back" # With the server's key known, the client only prompts for the password. # The log file is empty unless the library has logging compiled in. echo "Test a session given a command, with a log file" start_server -run_client "" -E $work_dir/command.log -p $port jill@127.0.0.1 "echo two" +run_client "" -E "$work_dir/command.log" -p $port jill@127.0.0.1 "echo two" [ $? -ne 0 ] && fail "failed to open the session" -grep -q "hello" $client_out \ +grep -q "hello" "$client_out" \ || fail "the echoserver's reply didn't make it back" -if [ -s $work_dir/command.log ]; then +if [ -s "$work_dir/command.log" ]; then echo "checking the log file" # The log is redirected before wolfSSH_Init(), so the library's start up # message is the first thing in the file. - head -n 1 $work_dir/command.log | grep -q "Entering wolfSSH_Init()" \ + head -n 1 "$work_dir/command.log" | grep -q "Entering wolfSSH_Init()" \ || fail "log file is missing the wolfSSH_Init() message" # The log is closed after wolfSSH_Cleanup(), which logs as well. - grep -q "Leaving wolfSSH_Cleanup()" $work_dir/command.log \ + grep -q "Leaving wolfSSH_Cleanup()" "$work_dir/command.log" \ || fail "log file is missing the wolfSSH_Cleanup() message" # Given a command the client opens an exec channel to carry it, with no # terminal request to discard it. - grep -q "type = exec" $work_dir/command.log \ + grep -q "type = exec" "$work_dir/command.log" \ || fail "the client didn't open an exec channel for the command" # The command string itself is only logged by a debug build. - if grep -q " command = " $work_dir/command.log; then - grep -q "command = echo two" $work_dir/command.log \ + if grep -q " command = " "$work_dir/command.log"; then + grep -q "command = echo two" "$work_dir/command.log" \ || fail "the client didn't send the command it was given" fi else @@ -265,20 +265,20 @@ fi echo "Test terminal session" start_server -run_client "" -E $work_dir/terminal.log -p $port jill@127.0.0.1 +run_client "" -E "$work_dir/terminal.log" -p $port jill@127.0.0.1 [ $? -ne 0 ] && fail "failed to open the terminal session" -grep -q "hello" $client_out \ +grep -q "hello" "$client_out" \ || fail "the echoserver's reply didn't make it back" -if [ -s $work_dir/terminal.log ]; then - grep -q "Leaving wolfSSH_Cleanup()" $work_dir/terminal.log \ +if [ -s "$work_dir/terminal.log" ]; then + grep -q "Leaving wolfSSH_Cleanup()" "$work_dir/terminal.log" \ || fail "log file is missing the wolfSSH_Cleanup() message" # No command was given, the client asks for a terminal instead. - grep -q "type = exec" $work_dir/terminal.log \ + grep -q "type = exec" "$work_dir/terminal.log" \ && fail "the client opened an exec channel it wasn't asked for" - grep -q " command = " $work_dir/terminal.log \ + grep -q " command = " "$work_dir/terminal.log" \ && fail "the client sent a command it wasn't given" fi From f04b04e228c222806eb79cdbfd05261bcf8c64d4 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 14 Aug 2026 09:27:47 -0700 Subject: [PATCH 08/10] Don't close the client's log file while a thread can still write it On the MSVC path the input thread is never waited on, it blocks in a console read with nothing to cancel it, so it can still be logging when main closes the file named by -E. - Flush the log there and let process exit close the stream - The POSIX path joins its threads first, it still closes the file --- apps/wolfssh/wolfssh.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apps/wolfssh/wolfssh.c b/apps/wolfssh/wolfssh.c index d8dab620f..78088a91f 100644 --- a/apps/wolfssh/wolfssh.c +++ b/apps/wolfssh/wolfssh.c @@ -1422,8 +1422,16 @@ int main(int argc, char** argv) /* Close the log last, wolfSSH_Cleanup() still logs and the callback * cannot be uninstalled. */ if (logFileStream != NULL) { +#ifdef _MSC_VER + /* The terminal session's input thread is left running, it blocks in + * a console read with nothing to cancel it. Flush the log and let + * process exit close it, rather than close the stream out from under + * a write that thread is making. */ + WFFLUSH(logFileStream); +#else WFCLOSE(NULL, logFileStream); logFileStream = NULL; +#endif } config_cleanup(&clientConfig); From 339b4c8853a48ecb3cf8058a800fc8fbbca6de7d Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 14 Aug 2026 09:34:59 -0700 Subject: [PATCH 09/10] Bound the client's queued send flush and let a rekey through FlushQueuedSend() retried wolfSSH_worker() for as long as it reported WS_WANT_WRITE. A peer that stops reading never lets the socket drain, so the sending thread spun there, and at the shutdown drain that thread was main, leaving the client unable to exit. - Give the retry a ten second deadline - Return the still pending WS_WANT_WRITE to the caller - Take that for done at the shutdown drain, the socket closes next - Mask WS_REKEYING, the worker only reports it once the send is out, and readInput() was taking it for a send failure --- apps/wolfssh/wolfssh.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/apps/wolfssh/wolfssh.c b/apps/wolfssh/wolfssh.c index 78088a91f..150aaefaf 100644 --- a/apps/wolfssh/wolfssh.c +++ b/apps/wolfssh/wolfssh.c @@ -74,9 +74,8 @@ #include #endif -#ifndef WOLFSSH_NO_TIMESTAMP - #include -#endif +#include +#include #ifdef WOLFSSH_CERTS #include @@ -303,14 +302,21 @@ static void PauseForSocket(void) } +/* Seconds to keep pushing a queued packet at a peer that isn't reading. A + * busy peer gets time to come back, a stalled one doesn't hang the client. */ +#define FLUSH_QUEUE_TIMEOUT 10 + + /* A packet the socket wasn't ready for stays queued in the session, and the * send that queued it still reports the data as taken. The peer can't answer * a message it never received, so push the queue out here rather than go * back to waiting on the peer. The lock is NULL when no other thread is - * using the session. */ + * using the session. Returns WS_WANT_WRITE with the packet still queued when + * the peer stops reading for the whole timeout. */ static int FlushQueuedSend(WOLFSSH* ssh, wolfSSL_Mutex* lock) { int ret; + time_t deadline = WTIME(NULL) + FLUSH_QUEUE_TIMEOUT; do { PauseForSocket(); @@ -326,11 +332,13 @@ static int FlushQueuedSend(WOLFSSH* ssh, wolfSSL_Mutex* lock) if (lock != NULL) { wc_UnLockMutex(lock); } - } while (ret == WS_WANT_WRITE); + } while (ret == WS_WANT_WRITE && WTIME(NULL) < deadline); /* The queue is out. Whatever the worker made of the peer's end of the - * conversation is for the reader to sort out. */ - if (ret == WS_WANT_READ || ret == WS_CHAN_RXD || ret == WS_EXTDATA) { + * conversation is for the reader to sort out. A rekey started on the way + * through is the reader's as well, the send itself went out. */ + if (ret == WS_WANT_READ || ret == WS_CHAN_RXD || ret == WS_EXTDATA + || ret == WS_REKEYING) { ret = WS_SUCCESS; } @@ -1321,6 +1329,11 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args) /* The close messages are queued and the threads are done, no * one else is going to send them. */ ret = FlushQueuedSend(ssh, NULL); + if (ret == WS_WANT_WRITE) { + /* The peer stopped reading. The socket closes next, there is + * nothing left to push the messages out with. */ + ret = WS_SUCCESS; + } } #endif From d5bea3f16b048c9f8447c43a0bec9a262f95c899 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 14 Aug 2026 09:35:06 -0700 Subject: [PATCH 10/10] Handle a select() error in the client's peer reader bytes held select()'s return in a word32, so a -1 became 0xFFFFFFFF and ran the read path on descriptor sets select() had left alone. The SIGWINCH handler interrupts this select, so a terminal resize reaches it. - Keep the result in an int - Retry on EINTR, report anything else - Same fix readPeer() in examples/client/client.c already carries --- apps/wolfssh/wolfssh.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/apps/wolfssh/wolfssh.c b/apps/wolfssh/wolfssh.c index 150aaefaf..f1b965b73 100644 --- a/apps/wolfssh/wolfssh.c +++ b/apps/wolfssh/wolfssh.c @@ -572,7 +572,7 @@ static THREAD_RET readPeer(void* in) int ret = 0; int stop = 0; int fd = wolfSSH_get_fd(args->ssh); - word32 bytes; + int bytes; #ifdef USE_WINDOWS_API HANDLE stdoutHandle = GetStdHandle(STD_OUTPUT_HANDLE); #endif @@ -619,6 +619,20 @@ static THREAD_RET readPeer(void* in) timeout.tv_sec = 1; timeout.tv_usec = 0; bytes = select(fd + 1, &readSet, NULL, &errSet, &timeout); + if (bytes < 0) { + #ifdef USE_WINDOWS_API + if (WSAGetLastError() == WSAEINTR) + continue; + fprintf(stderr, "select on peer socket failed, error %d\n", + WSAGetLastError()); + #else + /* the SIGWINCH handler interrupts this select */ + if (errno == EINTR) + continue; + perror("select on peer socket failed "); + #endif + break; + } if (bytes == 0) { /* Nothing new on the socket, but a flush in the send thread may * have already taken the peer's reply off it, so run the read