channel: half-close on the peer's EOF, add a public send-EOF API by ejohnstown · Pull Request #1195 · wolfSSL/wolfssh

ejohnstown · 2026-08-24T22:01:56Z

Re-cut. This PR had grown to 25 commits over four months, most of them
answering review on the others, and carried three unrelated concerns. It is
now ten commits that each stand on their own, and no commit in it exists to
correct another commit in it. The two strands that did not belong left for
#1211 and #1212, and tests/api.c -- which one commit edited and another
reverted -- is gone.

GitHub shows 13 commits and 19 files because the three branches are a linear
stack: the first three commits are #1211 and #1212. The last ten are this PR.

DoChannelEof() answered a peer's half-close with an EOF of its own. That
latched eofTxd, after which the "Cannot send data after EOF" gates in
SendChannelData() and SendChannelExtendedData() refused every later send,
so a peer that half-closed could never be replied to. ssh host 'sort' < file
returns nothing on master today: wolfSSHd closes the child's stdin correctly,
and then cannot send the command's output back. RFC 4254 section 5.3 leaves
that reply to the application, which is what this changes.

The commits, in order:

hoist the shared unit test doubles ahead of both endpoint regions
(relocation only, reads well with --color-moved)
stop echoing the peer's channel EOF -- DoChannelEof() reports WS_EOF,
the worker carries it with the channel it belongs to and does not mask it
during a rekey, and DoReceiveHandshake() absorbs it in the three
accept/connect states that can run with a channel already open (F-1687)
drain buffered data before the EOF -- the eofRxd test moves behind the
buffered-data test in wolfSSH_stream_read() and wolfSSH_stream_peek(),
so a half-close no longer strands what arrived with it
add the channel half-close API -- wolfSSH_stream_send_eof() and
wolfSSH_ChannelSendEof(), gated on NULL, openConfirmed, the disconnect
and the rekey
latch eofTxd on the bundled EOF, not only on a flush that reported
success, so the retry an application makes on WS_WANT_WRITE does not
queue a second EOF behind the first (F-8826)
drop closeTxd on a discarded close -- the mirror of 5, narrowing where
that one widens, which is why they are separate
keep the channel until the peer's close -- wolfSSH_ChannelExit() stops
calling ChannelRemove(), so the pointer survives until the peer answers
(F-8839)
reject teardown on an unconfirmed channel -- peerChannel is 0 until the
open is confirmed, so a teardown aimed at an unconfirmed channel lands on
whichever channel holds peer id 0, normally the live session
carry the EOF status through SFTP and SCP, so a peer half-close does not
abort a live transfer
handle the EOF status in apps and examples

Commit 10 is required by 2 through 4, not tidying. All three worker ladders in
wolfSSHd and the echoservers end in else if (rc != WS_WANT_READ) break, and
wolfSSHd's reaches kill(childPid, SIGKILL), so landing the library change
without its callers ships green and kills the command on every half-close.
Nothing in make check half-closes a channel.

Commits 5 through 8 fix master today and could each have been a same-day PR;
worth knowing if anyone needs a backport.

Merge order. #1212 must land before this one: the wolfSSHd changes here are
written on top of its drain. #1211 is preferred first as well -- the only code
coupling is one adjacency in src/ssh.c, but landing this first would briefly
double the post-disconnect chatter that #1211 removes.

The review threads below are stale anchors after the re-cut, not open
questions -- every one already carried its resolution, and each fix is carried
into the commit that now owns it. The two Fenrir findings on
wolfSSH_stream_send_eof() are commit 8.

Three things deliberately left out, rather than widening this further:

a public accessor for the bytes buffered on a channel. wolfsshd: drain the shell channel before closing the child's stdin #1212 reaches into
inputBuffer because wolfSSH_stream_peek() goes blind once eofRxd is
set, and it lands before this branch, so the API cannot come from here
a release note for wolfSSH_worker()'s changed return contract
a half-close in examples/client, which is a feature rather than a fix.
portfwd is the working demonstration of the new API here

Cross-PR: #1188's unlanded half rewrites echoserver's ssh_worker() with
hunks that bracket this one's insertion point. They auto-merge today, but
whoever lands second should confirm the EOF arm survived -- if it is lost the
SIGKILL comes back and ships green.

Verified: every one of the 13 commits in the stack builds and passes
unit.test and regress.test individually; make check at the tip is 12
passed, 1 skipped, 0 failed; --disable-client and --disable-server build
and pass, matching the merge base; the wolfsshd suite in the wolfssh-testing
VM is 24 passed, 4 skipped, 0 failed against a master baseline of 23 and 4,
the extra pass being a new half-close test that cannot pass on master.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #1195

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 4
4 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 #1195

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 4
4 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 #1195

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 2
2 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 #1195

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 2
2 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 #1195

Scan targets checked: wolfssh-bugs, wolfssh-src

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

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

philljj

Fenrir nits look valid

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #1195

Scan targets checked: wolfssh-bugs, wolfssh-src

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

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

The contract in ssh.h claimed more than the code does. A CHANNEL_EOF already received outranks the drain, so both stream reads report WS_EOF with data still buffered, and wolfSSH_accept()/wolfSSH_connect() never look at the flag at all. Both fixes belong with the channel EOF work in wolfSSL#1195; until then the header says what is really gated. Issue: F-8837

The contract in ssh.h claimed more than the code does. A CHANNEL_EOF already received outranks the drain, so both stream reads report WS_EOF with data still buffered, and wolfSSH_accept()/wolfSSH_connect() never look at the flag at all. Both fixes belong with the channel EOF work in #1195; until then the header says what is really gated. Issue: F-8837

wolfSSH_accept() and wolfSSH_connect() drive the handshake only while the session is live. Both gate on SendAfterDisconnect() ahead of the pending-send block, which would otherwise flush a queued disconnect and count it as the next handshake message; the shutdown paths own that flush. - The prototype sits ahead of both drivers, since either can be the only one built. - TestDisconnectGatesAccept() and TestDisconnectGatesConnect() cover a local disconnect, one from the peer, and a queued short send. One test per endpoint, so a single-sided build keeps the coverage that applies to it. - A received disconnect used to reach the error-state test in wolfSSH_accept() and report WS_INVALID_STATE_E; the gate answers WS_FATAL_ERROR first, and both tests pin that. - The contract comments in ssh.h and internal.h drop the ungated note.

DoPacket() skips the whole message dispatch once ssh->disconnected is set, for every message but a DISCONNECT. The handlers that answer must not -- a close draws an EOF and a close of ours, a request a success or failure, an open a confirmation, an unknown message an UNIMPLEMENTED -- and what the rest would record is of no use to a caller that can no longer send. RFC 4253 section 11.1. - Inbound data from here on is dropped rather than buffered, so the read path hands back only what arrived before the disconnect. ssh.h and internal.h say so, beside the calls and beside the flag. - A DISCONNECT still reaches DoDisconnect(), which sends nothing and is what latches WS_DISCONNECT; SendDisconnect() sets the flag too, so ours can be the one that raised it. - The frame advance steps over the whole packet, so the stream stays in step with no payload bookkeeping of its own. - wolfSSH_worker() stops reporting WS_REKEYING once the session is over. The NEWKEYS that clears isKeying is skipped from here on, so the flag would latch for the rest of the session and the SFTP and SCP drive loops would keep pumping a dead one. - tests/regress.c pins all three: no reply goes out, the stream stays in step with a disconnect queued behind a skipped close, and late channel data is dropped rather than queued.

SHELL_Subsystem() hands the child whatever the peer sent, whichever pass it arrived on, and closes the write end of its stdin only once that buffer is dry. It works off the shell channel's own inputBuffer, so data held back while the window was full is still handed over; the old read ran only on the worker's WS_CHAN_RXD and was skipped outright while windowFull. - The channel id comes from the head of the channel list at entry, the only channel open there, rather than from DEFAULT_NEXT_CHANNEL, which a build can override. - A lookup that finds nothing is not an EOF: only a channel that is present and drained closes the pipe. - Data arriving behind the peer's EOF, which RFC 4254 section 5.3 forbids, is dropped rather than written to a stdin that is already closed. The write would fail with EBADF and end the session mid-stream. - The short-write retry tests for a -1 return before reading errno, which nothing else sets.

The receive mock and the packet builders sit ahead of every endpoint region rather than inside the server half. A staged-packet IORecv and a plaintext packet builder have nothing to do with which endpoint is built, and the tests that follow reach for them from both halves. - Marked WS_MAYBE_UNUSED, since either half can be the only user of any one. - WantWriteIoSend() joins DiscardIoSend() among the send mocks. - A relocation only: no test body changes, and the moved text is unchanged bar the annotation. Read it with --color-moved.

DoChannelEof() latches eofRxd and reports WS_EOF. It used to answer with an EOF of its own, which latched eofTxd, after which the "Cannot send data after EOF" gates in SendChannelData() and SendChannelExtendedData() refused every later send: a peer that half-closed could never be replied to. RFC 4254 section 5.3 leaves that reply to the application. - wolfSSH_worker() carries WS_EOF out with the channel it belongs to, and does not mask it during a rekey; the event is raised once, on arrival. - DoReceiveHandshake() absorbs it in the three accept/connect states that can run with a channel already open. A legal EOF is not a handshake failure. - wolfSSH_shutdown() treats it as the response it was waiting for. - Tests cover the half-close, the channel id, the rekey case, the callback, which had no test anywhere in the tree, and a handshake that survives an EOF. - test_ConnectSurvivesChannelEof() sits in its own client region, since wolfSSH_connect() is not built with NO_WOLFSSH_CLIENT. Issue: F-1687

wolfSSH_stream_read() and wolfSSH_stream_peek() hand back what the peer sent before they report its EOF. The eofRxd test now sits behind the buffered-data test, so a half-close no longer strands whatever arrived with it or just ahead of it. - stream_read() takes the head channel id before DoReceive() and keeps waiting when the EOF belongs to another channel: the head is still open and still has nothing buffered, so reporting it would be indistinguishable. Multi- channel callers see that one through the worker or the callback. - Looping only while the head is unchanged, since DoChannelClose() can free it and the buffer pointer lives in it. - The disconnect contract in ssh.h and internal.h drops the caveat that the EOF outranks the drain. - test_ChannelEofHalfClose() picks up the drain assertions, which the commit ahead of this one cannot satisfy.

wolfSSH_ChannelSendEof() and wolfSSH_stream_send_eof() close an application's sending direction and leave its receiving direction open, the half-close of RFC 4254 section 5.3. Reads keep working until the peer sends its own EOF or closes; data sends on the channel then report WS_EOF, while requests, the exit status and the teardown messages still go out. - Both refuse a channel whose open the peer has not confirmed: peerChannel is 0 until then and the send resolves by peer id, so the EOF would land on whichever channel holds peer id 0. - Both sit behind the disconnect gate and refuse to put a packet between KEXINIT and NEWKEYS. - stream_send_eof() reports WS_REKEYING itself rather than latching it, the way stream_peek() does; ssh.h says so, since stream_send() differs.

SendChannelEof() commits eofTxd once the EOF is in the output buffer, not only once the flush reports success. A short write leaves the bytes queued and they go out on the next flush, so the retry an application makes on WS_WANT_WRITE must not put a second EOF behind the first. - The exception is the send failure that discards the output buffer, taking the EOF with it: latching there would leave the channel refusing every later send for an EOF that never went anywhere. - A reset or a closed peer keeps the bytes, so they still count. The test is what the buffer holds, since wolfSSH_SendPacket() reports all three the same way. - Both arms have a test: the discard through FailIoSend, and the reset that keeps the bytes queued and latches. Issue: F-8826

SendChannelClose() latches closeTxd on the same terms as the EOF beside it: on anything but the send failure that discards the output buffer. It latched unconditionally, so a closeTxd claiming a close that never left made wolfSSH_shutdown()'s gate skip the teardown altogether. - Split from the EOF latch on purpose: that one widens, from success-only to bundled, and this one narrows. Read as one change they read wrong.

wolfSSH_ChannelExit() leaves the channel on the list once it has sent the EOF and the close, so the application's pointer stays valid until the peer answers and wolfSSH_worker() reports WS_CHANNEL_CLOSED. Removing it locally freed the pointer under the caller and left the peer's close matching nothing. - DoChannelClose() sends the EOF ahead of the close, per RFC 4254 section 5.3, and sends both whatever the flush reports: DoPacket() consumes the peer's close either way, so a message skipped over a blocked flush is never sent. - It retires the channel and names it on a short write too. The debt belongs to the output buffer, not the channel, and withholding the close signal would leave the caller timing the teardown out. - The worker flushes that reply, keeps WS_CHANNEL_CLOSED as the return value, and leaves WS_WANT_WRITE latched so the caller knows to drain wolfSSH_OutputPending() before closing the socket. ssh.h says so. Issue: F-8839

DoChannelClose() and wolfSSH_ChannelExit() answer only a channel whose open the peer has confirmed. peerChannel is 0 until then and both senders resolve by peer id, so a teardown aimed at an unconfirmed channel landed on whichever channel held peer id 0 -- normally the live session -- latching its eofTxd and killing its send direction. - The close reply still retires the channel it names; there is simply nothing to say to a peer that has not answered the open. - wolfSSH_ChannelExit() reports WS_CHANNEL_NOT_CONF, the same as the two send-EOF calls.

wolfSSH_SFTP_buffer_send() and ScpStreamSend() keep driving the worker when the peer half-closes. Both return any negative status, so a WS_EOF would have aborted a transfer that is still perfectly able to finish: the peer closed its sending direction, not ours. - wolfSSH_SFTP_buffer_read() reports every negative peek but a rekey instead of spending a receive on it. A drained channel at EOF, a dead session and a channel that is gone all mean no more data can arrive, and the poll only overwrites the latched cause with WS_WANT_READ or blocks on a peer that has hung up. - A rekey is not a drained channel: peek reports it before it looks at the buffer at all, so that one still needs the poll. - DoScpRequest() reads its own EOF case the same way as the rest.

Every in-tree caller of wolfSSH_worker() now recognises a peer half-close. wolfsshd's shell loop and both echoservers need it: all three ladders end in "else if (rc != WS_WANT_READ) break", and wolfsshd's reaches kill(childPid, SIGKILL), so without it a client half-close kills the command it just finished feeding. - wolfsshd closes the child's stdin off the channel's own EOF state instead of off a worker return of zero, which no longer happens on a half-close. - The echoservers answer the half-close off wolfSSH_ChannelGetEof() rather than the WS_EOF status: the flush inside wolfSSH_worker() can supersede that status, and it is raised once. They hand back the backlog first, finish a short send, and only send the EOF once the channel is empty. Answering is not conditional on the shell build, where an echo session is the default. - The SFTP loops peek before leaving, so a half-close with requests still buffered is served rather than dropped, and they report an ordinary session end as success. - The clients -- examples/client, scpclient, sftpclient, apps/wolfssh -- treat it as the graceful case instead of an error. apps/wolfssh counts it as a finished flush as well, since one worker pass can drain the queue and consume the peer's EOF together. - portfwd relays it to the local socket with shutdown(SHUT_WR) so a local reader waiting on end-of-input returns, once the backlog has genuinely been handed over: a read cut short by a rekey leaves the half-close for a later pass. - The Windows half of wolfsshd does not answer with an EOF of its own. That latches eofTxd and the child's remaining output would be refused, which is the defect this series removes from the library. - The mplabx port drains before tearing down, the way its SFTP read path already did; its worker arm was unreachable for a half-close until now.

Copilot AI lite review requested due to automatic review settings August 24, 2026 22:01

Copilot started reviewing on behalf of ejohnstown August 24, 2026 22:36 View session

ejohnstown force-pushed the channel-eof branch from 0d7c135 to 4b89fae Compare August 24, 2026 22:36

ejohnstown requested a review from wolfSSL-Fenrir-bot August 25, 2026 18:35

ejohnstown assigned wolfSSL-Bot Aug 25, 2026

wolfSSL-Fenrir-bot requested changes Aug 25, 2026

View reviewed changes

Comment thread tests/unit.c

Comment thread src/ssh.c

Comment thread src/internal.c Outdated

Comment thread src/internal.c Outdated

ejohnstown commented Aug 25, 2026

View reviewed changes

Comment thread tests/unit.c

ejohnstown commented Aug 25, 2026

View reviewed changes

Comment thread src/ssh.c

ejohnstown commented Aug 25, 2026

View reviewed changes

Comment thread src/internal.c Outdated

ejohnstown commented Aug 25, 2026

View reviewed changes

Comment thread src/internal.c Outdated

ejohnstown requested a review from wolfSSL-Fenrir-bot August 25, 2026 21:50

wolfSSL-Fenrir-bot requested changes Aug 25, 2026

View reviewed changes

ejohnstown force-pushed the channel-eof branch from 3684cc8 to c47f622 Compare August 26, 2026 00:33

ejohnstown requested a review from wolfSSL-Fenrir-bot August 26, 2026 00:34

wolfSSL-Fenrir-bot requested changes Aug 26, 2026

View reviewed changes

Comment thread examples/echoserver/echoserver.c Outdated

Comment thread examples/echoserver/echoserver.c Outdated

ejohnstown force-pushed the channel-eof branch from c47f622 to c665f02 Compare August 26, 2026 01:04

ejohnstown commented Aug 26, 2026

View reviewed changes

Comment thread examples/echoserver/echoserver.c Outdated

ejohnstown commented Aug 26, 2026

View reviewed changes

Comment thread examples/echoserver/echoserver.c Outdated

ejohnstown force-pushed the channel-eof branch from c665f02 to 2b8e647 Compare August 26, 2026 01:17

ejohnstown requested a review from wolfSSL-Fenrir-bot August 26, 2026 04:37

wolfSSL-Fenrir-bot requested changes Aug 26, 2026

View reviewed changes

Comment thread src/wolfsftp.c Outdated

Comment thread src/internal.c

ejohnstown commented Aug 26, 2026

View reviewed changes

Comment thread src/internal.c

ejohnstown commented Aug 26, 2026

View reviewed changes

Comment thread src/wolfsftp.c Outdated

ejohnstown requested a review from wolfSSL-Fenrir-bot August 26, 2026 05:37

wolfSSL-Fenrir-bot requested changes Aug 26, 2026

View reviewed changes

Comment thread src/ssh.c

Comment thread src/internal.c

philljj requested changes Aug 26, 2026

View reviewed changes

philljj assigned ejohnstown and unassigned wolfSSL-Bot Aug 26, 2026

ejohnstown commented Aug 26, 2026

View reviewed changes

Comment thread src/ssh.c

ejohnstown commented Aug 26, 2026

View reviewed changes

Comment thread src/internal.c

ejohnstown requested review from philljj and wolfSSL-Fenrir-bot August 26, 2026 17:50

wolfSSL-Fenrir-bot requested changes Aug 26, 2026

View reviewed changes

Comment thread src/ssh.c

Comment thread src/ssh.c

ejohnstown force-pushed the channel-eof branch from 1dc7abd to 41d1e52 Compare August 27, 2026 03:50

ejohnstown added 13 commits August 27, 2026 15:14

ejohnstown force-pushed the channel-eof branch from b9dcbe8 to 0ab1756 Compare August 27, 2026 23:15

This was referenced Aug 27, 2026

ssh: stop driving a session after a disconnect #1211

Open

wolfsshd: drain the shell channel before closing the child's stdin #1212

Open

ejohnstown changed the title ~~Fix the channel EOF and close lifecycle, add a public send-EOF API~~ channel: half-close on the peer's EOF, add a public send-EOF API Aug 27, 2026

Uh oh!

Conversation

ejohnstown commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #1195

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #1195

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #1195

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #1195

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #1195

Uh oh!

Uh oh!

Uh oh!

philljj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #1195

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ejohnstown commented Aug 24, 2026 •

edited

Loading