Skip to content

Enforce LoginGraceTime in wolfsshd on Windows and make the grace flag per connection#1028

Draft
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_5577
Draft

Enforce LoginGraceTime in wolfsshd on Windows and make the grace flag per connection#1028
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_5577

Conversation

@yosuke-wolfssl

Copy link
Copy Markdown
Contributor

Enforce LoginGraceTime in wolfsshd on Windows

Problem

wolfSSHD_ConfigNew() defaults LoginGraceTime to 120 seconds, but on Windows
HandleConnection() only logged a warning and never armed a timer. The
pre-authentication accept loop ran as long as the timeout flag was clear and
wolfSSH_accept() returned WS_WANT_READ/WS_WANT_WRITE, so a Windows build
left 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

  • Windows enforcement via CreateThreadpoolTimer. The grace timer is armed
    before wolfSSH_accept(); its callback marks the connection as timed out, and
    it is cancelled both after the accept loop and on authentication success —
    mirroring the existing Unix alarm()/SIGALRM path. UserAuthResult()
    reaches its connection through wolfSSH_SetUserAuthResultCtx().
  • Per-connection timeout flag. Moved the timeout flag out of the file-scope
    global into WOLFSSHD_CONNECTION. The Unix SIGALRM handler reaches the
    active connection via a per-process activeConn pointer (POSIX signal
    handlers take no user-data argument).
  • Logging: wolfSSHDLoggingCb now 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_ERROR to
    WS_LOG_INFO.

Tests

  • Reworked sshd_login_grace_test.sh to drive the pre-auth stall
    deterministically 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.
  • Added sshd_login_grace_test.ps1 and a windows-check workflow step so the
    Windows enforcement path is exercised in CI.

@yosuke-wolfssl yosuke-wolfssl self-assigned this Jun 15, 2026
Copilot AI review requested due to automatic review settings June 15, 2026 06:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 LoginGraceTime on Windows using a per-connection threadpool timer and move the timeout flag into WOLFSSHD_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.

Comment thread apps/wolfsshd/wolfsshd.c Outdated
Comment thread apps/wolfsshd/wolfsshd.c
Comment thread apps/wolfsshd/wolfsshd.c Outdated
Comment thread apps/wolfsshd/wolfsshd.c
Comment thread apps/wolfsshd/wolfsshd.c Outdated
Comment thread apps/wolfsshd/test/sshd_login_grace_test.ps1
@yosuke-wolfssl yosuke-wolfssl marked this pull request as draft June 15, 2026 06:32

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #1028

Scan targets checked: none
Failed targets: wolfssh-bugs, wolfssh-src

⚠️ Review incomplete — one or more scan targets failed before findings could be produced. See the Fenrir PR review detail page for logs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants