Enforce LoginGraceTime in wolfsshd on Windows and make the grace flag per connection#1028
Enforce LoginGraceTime in wolfsshd on Windows and make the grace flag per connection#1028yosuke-wolfssl wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a fail-open behavior in wolfsshd where LoginGraceTime was not enforced on Windows, and corrects the timeout bookkeeping so it is safe under the Windows thread-per-connection model. It also updates logging behavior and adds/reenables tests to prevent regressions across Unix and Windows CI.
Changes:
- Enforce
LoginGraceTimeon Windows using a per-connection threadpool timer and move the timeout flag intoWOLFSSHD_CONNECTION. - Adjust authentication result handling to cancel grace timers/alarms on successful authentication.
- Rework and expand tests (Bash + PowerShell) and extend Windows CI to exercise the Windows enforcement path.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/wolfsshd/wolfsshd.c | Adds per-connection timeout state and Windows threadpool timer enforcement; logging flush behavior updated. |
| apps/wolfsshd/test/sshd_login_grace_test.sh | Makes grace-time test deterministic by stalling with a raw TCP connection; reorders logic accordingly. |
| apps/wolfsshd/test/sshd_login_grace_test.ps1 | Adds Windows regression test for grace-time enforcement. |
| apps/wolfsshd/test/run_all_sshd_tests.sh | Re-enables the grace-time test in the SSHD test suite and updates skipped count. |
| .github/workflows/windows-check.yml | Adds a workflow step to locate wolfsshd.exe, stage wolfssl.dll, and run the new Windows grace-time test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
07c2dff to
f195d73
Compare
de58d1d to
14e248f
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1028
Scan targets checked: none
Failed targets: wolfssh-bugs, wolfssh-src
Enforce LoginGraceTime in wolfsshd on Windows
Problem
wolfSSHD_ConfigNew()defaultsLoginGraceTimeto 120 seconds, but on WindowsHandleConnection()only logged a warning and never armed a timer. Thepre-authentication accept loop ran as long as the timeout flag was clear and
wolfSSH_accept()returnedWS_WANT_READ/WS_WANT_WRITE, so a Windows buildleft unauthenticated connections open indefinitely despite the configured (or
default) grace time — a fail-open daemon default.
A second, latent issue: the timeout flag was a single file-scope variable. This
was safe under the Unix fork-per-connection model, but on Windows daemon
connections are threads sharing memory, so one connection timing out would have
tripped every in-flight handshake.
Changes
CreateThreadpoolTimer. The grace timer is armedbefore
wolfSSH_accept(); its callback marks the connection as timed out, andit is cancelled both after the accept loop and on authentication success —
mirroring the existing Unix
alarm()/SIGALRMpath.UserAuthResult()reaches its connection through
wolfSSH_SetUserAuthResultCtx().global into
WOLFSSHD_CONNECTION. The UnixSIGALRMhandler reaches theactive connection via a per-process
activeConnpointer (POSIX signalhandlers take no user-data argument).
wolfSSHDLoggingCbnow flushes after each line so log consumers(e.g. tests reading the file while the daemon runs) see entries promptly; the
per-connection grace/timeout summary line was demoted from
WS_LOG_ERRORtoWS_LOG_INFO.Tests
sshd_login_grace_test.shto drive the pre-auth stalldeterministically with a raw TCP connection instead of an interactive client
password prompt (which never stalled without a TTY under CI), and re-enabled
it in
run_all_sshd_tests.sh.sshd_login_grace_test.ps1and awindows-checkworkflow step so theWindows enforcement path is exercised in CI.