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
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
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
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.
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.
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.
- --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.
- -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.
wolfSSH_SetChannelType() discarded an exec or subsystem name it could
not use and still returned WS_SUCCESS. SendChannelRequest() then omits
the name field entirely, which the peer rejects as malformed, dropping
the connection. Both an oversized name and an empty one reach it; the
empty case is reachable from the command line as "wolfssh -c ''".
- Return WS_BAD_ARGUMENT for a name at or above WOLFSSH_MAX_CHN_NAMESZ,
matching how the function already reports a bad type or side.
- Return WS_BAD_ARGUMENT when no name is given and none was stored by
an earlier call, and when a size arrives with no name behind it.
- Keep returning WS_SUCCESS when an earlier call stored a name, which
is what the SFTP and SCP retry loops depend on.
- Return before setting connectChannelId so a rejected call leaves
no state behind, as the server-side exec rejection does.
- Keep the stored name intact when a later call is refused.
- api.c asserts each refusal, and the largest name still admitted.
QNX system images fix the host key's owner and modes, and the daemon cannot
change either, so the secure gate refuses to load a key the integrator has no
way to correct. Add a hand-defined WOLFSSH_NO_HOSTKEY_PERMS, further
conditional on QNX, that hands only that policy to the platform.
- Fold the macro and the QNX test into the internal
WOLFSSHD_HOSTKEY_RELAX_PERMS in wolfsshd.c.
- Add a relaxPerms argument to wolfSSHD_OpenSecureFile() that skips the owner,
mode and ancestor-directory checks.
- Keep the structural checks: lstat, O_NOFOLLOW, S_ISREG and the dev/ino
recheck, so a symlink, a non-regular file or a swap during the open is still
refused.
- Set it only on the host key load, leaving the host cert, UserCAKeysFile,
authorized_keys and shadow gates unchanged.
- Log at startup when the guard is built in.
- Add six test_OpenSecureFile scenarios for the relaxed path, unreachable at
runtime off QNX and so otherwise uncovered.
Issue: ZD-22308
wolfSSHD_GetUserConf returned the first matching Match block whole, so a
keyword named only in a later matching block was dropped and the outcome
depended on the order the blocks were written.
- track in a new setMask which keywords a node set itself, so a value
inherited from the globals can be told from one the block named, with a
compile time check that no option tag shifts out of the mask
- resolve into a fresh config seeded from the globals, letting every
matching block contribute the keywords no earlier block claimed
- the resolved config now belongs to the caller, so wolfsshd and the auth
paths free it and the tests compare values rather than node identity
- put sshd_match_overlap_test.sh back in the suite
Issue: ZD-22324
OpenSSH resolves sshd_config one keyword at a time, scanning every Match
block that applies. wolfSSHD_GetUserConf returns the first matching block
whole, so a setting made only in a later matching block is dropped.
- add test_GetUserConfMatchOverlapCompose, covering a user matched by both
a Match User and a Match Group block, in either order
- add sshd_match_overlap_test.sh, the same case against a live daemon,
where the group block's ForceCommand is the setting that goes missing
- both fail until per keyword composition lands, so the unit test runs
last and the script stays commented out of run_all_sshd_tests.sh
- IsShadowExpired() in auth.c returns 1 when a shadow entry's
sp_expire date has arrived, its sp_lstchg is 0, or the day is at or
past sp_lstchg + sp_max. Negative fields leave the matching check
off; a negative day count, standing for an unavailable clock,
denies the entries that carry aging. WSSHD_SECS_PER_DAY converts
WTIME() into the unit those fields use. The helper is compiled
under HAVE_SHADOW and !WOLFSSH_USE_PAM, as its caller is.
- CheckPasswordUnix() runs the shadow entry it looked up through the
helper and, after an otherwise successful hash compare, logs the
denial and returns WSSHD_AUTH_FAILURE.
- auth.h declares IsShadowExpired() for the unit test build.
- test_configuration.c adds test_IsShadowExpired() over a table of
aging fields and day counts, and test_CheckPasswordUnix_expired()
for the denial of a correct password.
- The CheckPasswordUnix() tests share one driver,
wsshd_test_CheckPasswordUnixCase(), with the crypt() setup in
wsshd_test_LoadShadowHash() and the three fail-closed shadow
lookups gathered into test_CheckPasswordUnix_failClosed().
Issue: F-10577
stop_wolfsshd killed $PID unconditionally. With the daemon already gone the
kill failed, and under "set -e" that aborted the caller -- in
sshd_forcedcmd_test.sh before PID was cleared, so the ForceCommand-SFTP
scenario was silently skipped, the EXIT trap killed the dead pid a second
time, and the script exited 1.
- Guard on a non-empty PID, ignore a failed kill and return 0, so the
function is safe to call from an EXIT trap.
- Clear PID after stopping, so a second call cannot kill a recycled pid.
- Remove the temp key dir even when no daemon was recorded, so a daemon that
failed to start does not leak it.
- Collapse sshd_forcedcmd_test.sh's cleanup() wrapper to a bare
trap stop_wolfsshd EXIT now that the function guards itself.
- Check the cd back to the test directory in sshd_x509_upn_fail.sh; the log
it counts after the client run is the one there.
sshd_privdrop_fail_test.sh runs from apps/wolfsshd/test and handed the
example clients relative key paths, but the clients call
ChangeToWolfSshRoot() before parsing arguments. Every client died at
"Error setting private key" and the test blamed the privilege drop.
- Anchor the key, payload and client paths at the script's own directory.
- Rename the saved directory to TESTDIR so a later cd cannot clobber it.
- Report "no fork at all" separately in the timeout diagnostic.
- Print the client's own output at every failure exit, so a client that
never connects cannot be read as a daemon fault.
- Keep the client logs like log.txt, gitignored and removed on success.
Eight scripts saved their starting directory in PWD, which the shell
rewrites on every cd, so the "cd $PWD" restore landed in the repository
root. sshd_forcedcmd_test.sh's second scenario and the log count in
sshd_x509_upn_fail.sh silently never did what they cover.
- Save the starting directory in TESTDIR, as sshd_pubkey_reject_test.sh does.
- Quote "$TESTDIR" at every cd, now that the saved value is really used.
- Stop the daemon from a trap in sshd_forcedcmd_test.sh, so its now reachable
second scenario cannot leave one on the shared port when set -e aborts.
- Take start_wolfsshd's before and after daemon PID snapshots from pgrep -x
instead of scraping every digit run out of "ps -e", which mixed the TIME
field's clock digits in with the PID and, with a leftover daemon running,
stopped the wrong process.
- Let both snapshot pipelines fail, so a set -e caller survives no daemon
being up and a daemon that dies after sudo returns is reported by the
caller's own empty-PID check.
grep -c prints 0 and exits 1 when nothing matches, so the "|| echo 0"
fallback fired too and the count became "0\n0". The arithmetic error
unwound bash out of the test block, skipping the last eleven tests while
the summary still printed a pass and exited 0.
- Use the bare grep -c result and default only the empty case.
- Add a RUN_COMPLETE sentinel at the end of each branch that runs tests.
- Check the sentinel before the summary so an abort exits non-zero.
- Kill lingering daemons by process name, so the teardown does not kill
the run itself before that check when invoked by a path holding
"wolfsshd".
The scripts now read the build options from the probe instead of grepping
usage text, config.log and daemon logs. Drops the usage lines only tests read.
- keepOpen was hardcoded to 1, so the TERMINAL channel type was always
set right after EXEC, freeing the command and reverting to a shell.
- Derive keepOpen from whether a command was given.
- Gate MODES_CLEAR() the same way. A remote command runs with no pty,
so its LF terminated output needs OPOST left on.
- The SINGLE_THREADED guard is now unconditional, matching what it did
when keepOpen was always 1.
- Remove -N. It was parsed into config.noCommand and read nowhere, and
making it work needs a way to request no session at all.
- Add the optional command to the usage line.
- Reject a command of WOLFSSH_MAX_CHN_NAMESZ or longer. SetChannelType
discards it and still returns WS_SUCCESS, so the client would send an
exec request with no command string at all.
- Drop the I/O thread guard. With keepOpen derived from config.command,
the condition was true for every input.
Issue: F-8816
Without WOLFSSH_SMALL_STACK dCert is the address of a stack variable, so
the NULL check could never fire. Keep the check under the small stack
build where the WMALLOC can actually fail.
Issue: CID-573006
The drain after waitpid ignored the send return, so on a non-blocking
socket the tail of a command's output was dropped on a full window, a
rekey or a would block. Retry a bounded number of times and log when
the data still can not be sent.
Issue: CID-572907
The leftover-data drain after waitpid ignored both fcntl calls. Check
the get and the set, and skip the drain read for a pipe that could not
be made non-blocking so the read cannot hang the connection process.
Issue: CID-572931
The loop condition already guarantees a want-read or want-write error,
so the select_ret arms of the retry test and the else chain could never
run. Retry unconditionally; tcp_select still throttles the loop. Same
change applied to the copy in apps/wolfssh/wolfssh.c.
Issue: CID-572884
Both loop exits in readInput() returned early, leaving the per-thread
ECC cache cleanup after the loop unreachable. Break out of the loop
instead.
wolfSSH_stream_send() reports a rekey in progress by returning
WS_FATAL_ERROR with the ssh error set to WS_REKEYING, so testing the
return value against WS_REKEYING never matches. Read the code with
wolfSSH_get_error() and resend the same buffer once the rekey
finishes, in both the app and the example client. Looping back to
read() instead would drop the input already taken from stdin.
Issue: CID-572833
The useAgent flag was never set, so both agent setup blocks in
wolfSSH_Client were dead code. Add the -a option to the parser and
carry it through the config struct, matching examples/client/client.c.
Issue: CID-572857
SHELL_Subsystem() raises privileges to look up user information, but the
three pipe() failures and the forkpty() failure return WS_FATAL_ERROR
without dropping them again. HandleConnection() then runs its teardown --
wolfSSH_shutdown(), up to ten wolfSSH_worker() iterations, and the socket
drain -- still elevated when UsePrivilegeSeparation is yes or sandbox.
Drop permissions before each of the four returns, reusing the wording the
daemon already logs for a failed drop.
The forkpty() path also returned with all six pipe descriptors still
open. Close them there, as the pipe() failure paths already do, and reset
the slots to -1 to keep the child-branch invariant that an entry is
either a live descriptor or -1.
Issue: F-6980
- The zero check ran WSTRCMP() on the caller's buffer, which is a
length-bounded slice of the config line and not NUL terminated, so it
read past inSz and rejected valid "0" values whose slice had trailing
text.
- Compare num, the NUL-terminated copy that atol() was given.
Issue: F-7213
- crypt() returns a pointer into a static buffer that keeps the hashed
password after CheckPasswordHashUnix() returns; wipe it once the
comparison is done.
Issue: F-7220
- Scenarios 1 and 5 fopen() once and fstat() that handle instead of
stat()ing the path a second time.
- A failed open is now a logged error rather than an indirect rd == 0.
- The FIFO lstat() and failPath existence test stay as they are: neither
has a descriptor to stat, and mkdtemp()'s 0700 directory makes them
unraceable.
Issues: CID-651701
John Safranek
Yosuke Shimizu
JacobBarthelmeh
Paul Adelsbach
Emma Stensland
Ruby Martin
Mark Atwood