ssh: add a rekey-state accessor by yosuke-wolfssl · Pull Request #1260 · wolfSSL/wolfssh

yosuke-wolfssl · 2026-09-17T06:42:38Z

Problem

Consumers have no way to ask whether a key exchange is running. 24 of the 118
consumer reads of wolfSSH_get_error() / ssh->error ask exactly that; 21 are
one repeated pump in examples/sftpclient/sftpclient.c, the only consumer code
that reads the WOLFSSH struct directly. wolfSSH_worker() withholds
WS_REKEYING when its flush failed, so that pump can exit mid-rekey.

SendKexInit() also set WOLFSSH_SELF_IS_KEYING before building the packet, so
a send that never reached the transport left the session believing a rekey was
in flight. DoKexInit() gates the answering KEXINIT on that flag, so the peer
then waits forever for a KEXINIT we had already discarded.

Fix (`src/internal.c`, `src/ssh.c`)

wolfSSH_RekeyPending(const WOLFSSH*) returns nonzero during a key
exchange, the first one included, and 0 otherwise or for a NULL session. As a
pure ssh->isKeying read it cannot disagree with wolfSSH_worker()'s
WS_REKEYING; only NEWKEYS from both sides clears it, so the prototype notes
a loop on it must also end on wolfSSH_worker().
SendKexInit() flushes with SendPacketFlush(), sets the flag when
SendPacketDelivered() reports the packet sent or queued, and purges on the
same decision. It then runs HighwaterCheck() itself, so a callback fired by
the KEXINIT's own flush sees the flag set; the return is unchanged. This is the
pattern SendGlobalRequestFwd() already uses.

Tests

File	Coverage
`tests/regress.c`	each keying bit alone, both together, and NULL; runs in every build configuration
`tests/unit.c`	a KEX init the transport rejects, one it refuses with a would-block, and one it takes whole behind a highwater callback that fails or reads the flag; the first two also check whether the packet is left owed

Verification

Clean under six gcc-13 -Werror configs, --disable-server and
--disable-client.
unit 170 against a baseline of 169, plus regress, testsuite, kex,
api and auth; the scp, sshclient, get-put and sftp scripts pass.
Five mutants each fail only the assertion written for them: dropping or
forcing PurgePacket(), setting the flag after HighwaterCheck(), gating on
the send's return, and running HighwaterCheck() after a failed send.

Rebase note

25513731 (@ejohnstown) is folded into this branch: its placement of the shared
send mock and its s_sendRefusals comment are in verbatim. The rebase removed
the commit, not the change.

Not in this PR

No consumer conversion. The sftpclient pumps and the Renesas loop should
change against a settled contract, not alongside it.
tests/testsuite.c drops its wolfSSH_OutputPending() probe, the only
export check among the public functions; covering all of them is not worth it.

Copilot

🟢 Approval recommended

The API addition and keying-flag gating change are consistent with existing worker/error semantics, and the PR includes targeted regression/unit coverage for the new behavior and the previously problematic edge case.

Pull request overview

This PR adds a small public API to let consumers query whether a key exchange (rekey) is currently in flight, and fixes a keying-state edge case where SendKexInit() could leave the session stuck “keying” even when the KEXINIT send failed outright.

Changes:

Add wolfSSH_RekeyPending(const WOLFSSH*) accessor (NULL-safe, pure ssh->isKeying read) and document its intended use alongside wolfSSH_worker() / wolfSSH_get_error().
Adjust SendKexInit() to set WOLFSSH_SELF_IS_KEYING only once the KEXINIT packet is successfully sent or queued, avoiding a stale keying flag on outright send failure.
Add regression/unit tests for the accessor and for the “KEX init send fails / short-writes” keying-flag behavior; remove an export-visibility check from tests/testsuite.c.

File summaries

File	Description
wolfssh/ssh.h	Documents the contract and adds the public `wolfSSH_RekeyPending()` declaration.
wolfssh/internal.h	Updates the semantics comment for `WOLFSSH_SELF_IS_KEYING` to match the new gating.
src/ssh.c	Implements `wolfSSH_RekeyPending()` as a NULL-safe `isKeying` predicate.
src/internal.c	Moves `WOLFSSH_SELF_IS_KEYING` set to only after KEXINIT is sent/queued.
tests/unit.c	Adds assertions covering predicate behavior and the KEXINIT send-failure/short-write keying gate.
tests/testsuite.c	Removes the `wolfSSH_OutputPending()` export check from the testsuite harness.
tests/regress.c	Adds regression coverage for `wolfSSH_RekeyPending()` over NULL and keying-bit combinations.

Review details

Files reviewed: 7/7 changed files
Comments generated: 0
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #1260

Scan targets checked: wolfssh-src, wolfssh-bugs

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #1260

Scan targets checked: wolfssh-src, wolfssh-bugs

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

wolfSSL-Fenrir-bot