internal: take channel data only after framing it

[yosuke-wolfssl]

Problem

SendChannelData() and SendChannelExtendedData() flush queued output before framing new data. If that flush would block and the queued packet is not channel data (a window adjust, IGNORE, KEXINIT or close), they framed nothing but still returned dataSz. The caller then reuses its buffer, so the bytes are lost with no error.

The flush can also cross the highwater mark and send our KEXINIT. isKeying was checked only on entry, so CHANNEL_DATA was framed behind it, which RFC 4253 7.1 forbids.

Fix (src/internal.c)

• One pre-flush, gated on wolfSSH_OutputPending(), replaces the plainSz > 0 and length != 0 blocks. If the flush blocks, the call returns WS_WANT_WRITE.
• isKeying is checked again after the flush, returning WS_REKEYING. ssh->error is not written.
• dataSz is returned only after this call's own packet is framed.
• plainSz removed: its only reader was the old guard.
• ssh.h: one note on what wolfSSH_stream_send() and wolfSSH_extended_data_send() return.

Tests (tests/regress.c)

Run on the in-memory client/server pair, and check what the server actually receives:

• TestChannelSendPreFlushWantWrite(): an IGNORE queued ahead (data and stderr), then channel data queued ahead.
• TestChannelSendPreFlushStartsRekey(): the flush starts a rekey (data and stderr).
• TestChannelSendPreFlushRekeyBlocks(): the flush starts a rekey, but the KEXINIT itself cannot be sent.

Verification

• unit, regress, api, testsuite and the script tests pass. gcc-13 -Werror builds in 6 configs are clean, ASan + UBSan are clean, and flowstress passed 18/18.
• Against master's library, the IGNORE and blocked-KEXINIT cases get 16 back with nothing framed. Without the new isKeying check, the rekey test gets 16.

[Copilot]

🟡 Changes recommended

Post-flush highwater errors can still report already-delivered channel data as untaken, risking duplicate sends.

1 open finding

• Report sent data after post-flush highwater callback errors · New

What changed in this PR

Fixes channel sends so buffers are consumed only after packet framing and prevents channel traffic during rekeying.

Changes:

• Pre-flushes queued output before framing channel data.
• Rechecks rekey state after flushing.
• Adds regression coverage and updates API documentation.

File	Description
src/​internal.c	Revises channel-data framing and flushing.
wolfssh/​internal.h	Removes obsolete plaintext tracking.
wolfssh/​ssh.h	Documents send return semantics.
tests/​regress.c	Adds queued-output and rekey regression tests.

🧠 Review effort: Balanced

---

💡 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 #1318

Scan targets checked: wolfssh-src, wolfssh-bugs
Coverage: 2 of 4 in-scope changed file(s) opened by the reviewer; not opened: wolfssh/internal.h, wolfssh/ssh.h

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

Review tier: Lite

[ejohnstown]

This returns a bare WS_REKEYING and leaves ssh->error alone. The entry check in wolfSSH_stream_send() handles a rekey differently: it sets ssh->error = WS_REKEYING and returns WS_FATAL_ERROR. As a result, a rekey started by this flush can't be retried from wolfSSH_SFTP_connect():

1. Output is queued when SFTP_ClientSendInit() calls wolfSSH_stream_send().
2. The flush here completes, HighwaterCheck() fires, and wsHighwater() calls wolfSSH_TriggerKeyExchange(), which sets isKeying.
3. This branch returns WS_REKEYING. wolfSSH_SFTP_connect() maps any failed INIT send to WS_FATAL_ERROR without setting ssh->error, and it cleared WS_WANT_READ/WS_WANT_WRITE on entry, so wolfSSH_get_error() usually returns 0.
4. The retry loop in examples/sftpclient/sftpclient.c checks wolfSSH_get_error() for WS_REKEYING and the WANT codes, so it gives up even though the operation could be retried.

This is hard to hit with the default highwater mark, but easier with a low one (e.g. -DDEFAULT_HIGHWATER_MARK=6000). Checking for a rekey after the flush is correct: before this PR, the code framed channel data during the key exchange.

Suggested fix: set ssh->error = WS_REKEYING here, in the matching branch of SendChannelExtendedData(), and in both functions' entry isKeying checks. Return values don't change, so callers that compare the wolfSSH_ChannelSend() return to WS_REKEYING behave the same, and wolfSSH_extended_data_send() already sets it. Optionally, wolfSSH_stream_send() could convert a WS_REKEYING from SendChannelData() into WS_FATAL_ERROR, which brings back the one-form contract the old ssh.h comment described. A regression test would help: queue output, set a low highwater mark, call wolfSSH_SFTP_connect(), and check that wolfSSH_get_error() == WS_REKEYING.