Skip to content

Shutdown channel lookup and disconnect state #1190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ejohnstown wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Shutdown channel lookup and disconnect state #1190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d4bf5e0
Fix the shutdown channel teardown
ejohnstown Aug 21, 2026
76a7291
Make a disconnect end the session
ejohnstown Aug 21, 2026
12b8f53
Block every send after a disconnect
ejohnstown Aug 21, 2026
ad66ac0
Report a disconnect from stream_peek
ejohnstown Aug 21, 2026
e2e820a
Gate the last three senders on a disconnect
ejohnstown Aug 24, 2026
87327f7
Close the last three post-disconnect sends
ejohnstown Aug 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,10 @@ static int wsHighwater(byte dir, void* ctx)
wolfSSH_GetHighwater(ssh),
(dir == WOLFSSH_HWSIDE_RECEIVE) ? "receive" : "transmit");

ret = wolfSSH_TriggerKeyExchange(ssh);
/* A rekey on a dead session would fail the send that fired the
* mark. */
if (!ssh->disconnected)
ret = wolfSSH_TriggerKeyExchange(ssh);
}

return ret;
Expand Down Expand Up @@ -8166,6 +8169,10 @@ static int DoDisconnect(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)

WOLFSSH_UNUSED(reasonStr);

/* RFC 4253 section 11.1, the peer is gone whether or not the rest of
* the message decodes. */
ssh->disconnected = 1;

ret = GetUint32(&reason, buf, len, &begin);
if (ret == WS_SUCCESS) {
/* Skip the description text. */
Expand Down Expand Up @@ -16731,6 +16738,11 @@ int SendDisconnect(WOLFSSH* ssh, word32 reason)
if (ssh == NULL)
ret = WS_BAD_ARGUMENT;

/* Mark the session over before the send. A partial or failed send
* still ends it. */
if (ret == WS_SUCCESS)
ssh->disconnected = 1;

if (ret == WS_SUCCESS)
ret = PreparePacket(ssh, MSG_ID_SZ + UINT32_SZ + (LENGTH_SZ * 2));

Expand Down
110 changes: 103 additions & 7 deletions src/ssh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1103,6 +1103,21 @@ int wolfSSH_connect(WOLFSSH* ssh)
#endif /* NO_WOLFSSH_CLIENT */


/* A disconnect, sent or received, ends the session, so nothing further may
* go out. RFC 4253 section 11.1. Reads are deliberately not gated on this:
* channel data that arrived before the disconnect is still the caller's.
* Call only after ssh has been checked for NULL. */
static int SendAfterDisconnect(WOLFSSH* ssh)
{
if (ssh->disconnected) {
WLOG(WS_LOG_DEBUG, "Send attempted after a disconnect");
ssh->error = WS_DISCONNECT;
return 1;
}
return 0;
}


int wolfSSH_shutdown(WOLFSSH* ssh)
{
int ret = WS_SUCCESS;
Expand All @@ -1113,9 +1128,17 @@ int wolfSSH_shutdown(WOLFSSH* ssh)
if (ssh == NULL || ssh->channelList == NULL)
ret = WS_BAD_ARGUMENT;

/* look up the channel if it still exists */
/* The session channel is the head of the list. */
if (ret == WS_SUCCESS) {
channel = ChannelFind(ssh, ssh->channelList->peerChannel, WS_CHANNEL_ID_SELF);
channel = ssh->channelList;

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Aug 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

wolfSSH_shutdown() transmits and drains after a disconnect · SSH protocol violations

Taking the head channel directly makes the send block at src/ssh.c:1137 always execute, but wolfSSH_shutdown() is the one send path this PR leaves ungated by ssh->disconnected. After DoDisconnect() it emits CHANNEL_EOF, exit-status and CHANNEL_CLOSE (RFC 4253 §11.1 forbids sending after SSH_MSG_DISCONNECT), then waits in wolfSSH_worker() at src/ssh.c:1163 for a close the peer will not send. Reached in-tree from examples/echoserver/echoserver.c:1640, where error == WS_DISCONNECT passes the guard at line 1639.

Related known finding #8817 (similar but distinct): Both affect wolfSSH_shutdown() channel teardown sends and drain logic, but #8817 faults in ChannelFind using peerChannel with WS_CHANNEL_ID_SELF, while this candidate faults by allowing sends/draining after ssh->disconnected. Their root causes and required patches differ.

Fix: Apply the SendAfterDisconnect() gate in wolfSSH_shutdown() so the channel teardown sends and the drain are skipped once ssh->disconnected is set.

}

/* Session already over. Drop the channel to skip the teardown sends
* and the wait for a close that will not come. RFC 4253 section 11.1. */
if (channel != NULL && ssh->disconnected) {
WLOG(WS_LOG_DEBUG, "Session already disconnected, nothing to send");
ssh->error = WS_DISCONNECT;
channel = NULL;
}

/* if channel close was not already sent then send it */
Expand Down Expand Up @@ -1170,6 +1193,9 @@ int wolfSSH_TriggerKeyExchange(WOLFSSH* ssh)
if (ssh == NULL)
ret = WS_BAD_ARGUMENT;

if (ret == WS_SUCCESS && SendAfterDisconnect(ssh))

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Aug 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Gating wolfSSH_TriggerKeyExchange() makes the default highwater callback fail every send after a disconnect · Incorrect error handling

wsHighwater() (src/internal.c:557) is the default highwaterCb, and wolfSSH_SendPacket() returns HighwaterCheck()'s result (src/internal.c:4318). Once disconnected is set, a firing highwater turns every successful packet send into WS_FATAL_ERROR: SendDisconnect() reports failure after writing the message, and SendChannelEof() leaves eofTxd clear so wolfSSH_shutdown() skips the exit-status and close sends.

Related known finding #8817 (similar but distinct): Both can cause wolfSSH_shutdown() to skip EOF/exit/close traffic, but this finding faults wolfSSH_TriggerKeyExchange() returning WS_FATAL_ERROR after disconnect through the highwater callback; #8817 faults wolfSSH_shutdown() using a peer ID with SELF lookup. Their root causes and required patches are separate.

Fix: Return WS_SUCCESS (skip the rekey) instead of WS_FATAL_ERROR when ssh->disconnected is set, or gate inside wsHighwater() rather than in wolfSSH_TriggerKeyExchange().

ret = WS_FATAL_ERROR;

if (ret == WS_SUCCESS)
ret = ssh->error = SendKexInit(ssh);

Expand All @@ -1183,6 +1209,7 @@ int wolfSSH_TriggerKeyExchange(WOLFSSH* ssh)
int wolfSSH_stream_peek(WOLFSSH* ssh, byte* buf, word32 bufSz)
{
WOLFSSH_BUFFER* inputBuffer;
word32 avail;

WLOG(WS_LOG_DEBUG, "Entering wolfSSH_stream_peek()");

Expand All @@ -1199,11 +1226,22 @@ int wolfSSH_stream_peek(WOLFSSH* ssh, byte* buf, word32 bufSz)
}

inputBuffer = &ssh->channelList->inputBuffer;
bufSz = min(bufSz, inputBuffer->length - inputBuffer->idx);
avail = inputBuffer->length - inputBuffer->idx;

/* Report the disconnect only once the buffered data is drained, the
* same way wolfSSH_stream_read() does. Callers use this to tell a
* drained channel from one with more to come, and a dead session is
* neither. */
if (avail == 0 && ssh->disconnected) {
ssh->error = WS_DISCONNECT;
return WS_FATAL_ERROR;
}

bufSz = min(bufSz, avail);
if (buf != NULL) {
WMEMCPY(buf, inputBuffer->buffer + inputBuffer->idx, bufSz);
}
return bufSz;
return (int)bufSz;
}


Expand All @@ -1228,8 +1266,17 @@ int wolfSSH_stream_read(WOLFSSH* ssh, byte* buf, word32 bufSz)

WLOG(WS_LOG_DEBUG, "Entering wolfSSH_stream_read()");

if (ssh == NULL || buf == NULL || bufSz == 0 || ssh->channelList == NULL)
if (ssh == NULL || buf == NULL || bufSz == 0)
return WS_BAD_ARGUMENT;

if (ssh->channelList == NULL) {
/* No channel left to drain, so the disconnect is all there is. */
if (ssh->disconnected) {
ssh->error = WS_DISCONNECT;
return WS_FATAL_ERROR;
}
return WS_BAD_ARGUMENT;
}

if (ssh->channelList->eofRxd) {
ssh->error = WS_EOF;
Expand All @@ -1244,6 +1291,13 @@ int wolfSSH_stream_read(WOLFSSH* ssh, byte* buf, word32 bufSz)
inputBuffer = &ssh->channelList->inputBuffer;
ssh->error = WS_SUCCESS;

/* Hand back whatever arrived before the disconnect, then report it once
* the buffer runs dry rather than going back to a dead transport. */
if (ssh->disconnected && inputBuffer->length - inputBuffer->idx == 0) {
ssh->error = WS_DISCONNECT;
return WS_FATAL_ERROR;
}

if (ret == WS_SUCCESS) {
WLOG(WS_LOG_DEBUG, " Stream read index of %u", inputBuffer->idx);
WLOG(WS_LOG_DEBUG, " Stream read ava data %u", inputBuffer->length);
Expand Down Expand Up @@ -1307,7 +1361,13 @@ int wolfSSH_stream_send(WOLFSSH* ssh, byte* buf, word32 bufSz)

WLOG(WS_LOG_DEBUG, "Entering wolfSSH_stream_send()");

if (ssh == NULL || buf == NULL || ssh->channelList == NULL)
if (ssh == NULL || buf == NULL)
return WS_BAD_ARGUMENT;

if (SendAfterDisconnect(ssh))
return WS_FATAL_ERROR;

if (ssh->channelList == NULL)
return WS_BAD_ARGUMENT;

if (ssh->isKeying) {
Expand All @@ -1334,6 +1394,9 @@ int wolfSSH_ChannelIdSend(WOLFSSH* ssh, word32 channelId,
if (ssh == NULL || buf == NULL)
ret = WS_BAD_ARGUMENT;

if (ret == WS_SUCCESS && SendAfterDisconnect(ssh))
ret = WS_FATAL_ERROR;

if (ret == WS_SUCCESS) {
channel = ChannelFind(ssh, channelId, WS_CHANNEL_ID_SELF);
if (channel == NULL) {
Expand Down Expand Up @@ -1370,6 +1433,9 @@ int wolfSSH_ChannelIdSendExt(WOLFSSH* ssh, word32 channelId,
if (ssh == NULL || buf == NULL)
ret = WS_BAD_ARGUMENT;

if (ret == WS_SUCCESS && SendAfterDisconnect(ssh))
ret = WS_FATAL_ERROR;

if (ret == WS_SUCCESS) {
channel = ChannelFind(ssh, channelId, WS_CHANNEL_ID_SELF);
if (channel == NULL) {
Expand Down Expand Up @@ -1403,6 +1469,9 @@ int wolfSSH_stream_exit(WOLFSSH* ssh, int status)
if (ssh == NULL || ssh->channelList == NULL)
ret = WS_BAD_ARGUMENT;

if (ret == WS_SUCCESS && SendAfterDisconnect(ssh))
ret = WS_FATAL_ERROR;

if (ret == WS_SUCCESS)
ret = SendChannelExit(ssh, ssh->channelList->peerChannel, status);

Expand All @@ -1426,6 +1495,8 @@ int wolfSSH_global_request(WOLFSSH *ssh, const unsigned char* data, word32 dataS
return WS_BAD_ARGUMENT;
if (reply != 0 && reply != 1)
return WS_BAD_ARGUMENT;
if (SendAfterDisconnect(ssh))
return WS_FATAL_ERROR;
return SendGlobalRequest(ssh, data, dataSz, reply);
}

Expand All @@ -1436,7 +1507,13 @@ int wolfSSH_extended_data_send(WOLFSSH* ssh, byte* buf, word32 bufSz)

WLOG(WS_LOG_DEBUG, "Entering wolfSSH_extended_data_send()");

if (ssh == NULL || buf == NULL || ssh->channelList == NULL)
if (ssh == NULL || buf == NULL)
return WS_BAD_ARGUMENT;

if (SendAfterDisconnect(ssh))
return WS_FATAL_ERROR;

if (ssh->channelList == NULL)
return WS_BAD_ARGUMENT;

if (ssh->isKeying) {
Expand Down Expand Up @@ -1476,6 +1553,13 @@ int wolfSSH_SendIgnore(WOLFSSH* ssh, const byte* buf, word32 bufSz)

WOLFSSH_UNUSED(buf);
WOLFSSH_UNUSED(bufSz);

if (ssh == NULL)
return WS_BAD_ARGUMENT;

if (SendAfterDisconnect(ssh))
return WS_FATAL_ERROR;

WMEMSET(scratch, 0, sizeof(scratch));

return SendIgnore(ssh, scratch, sizeof(scratch));
Expand All @@ -1485,6 +1569,15 @@ int wolfSSH_SendIgnore(WOLFSSH* ssh, const byte* buf, word32 bufSz)
int wolfSSH_SendDisconnect(WOLFSSH *ssh, word32 reason)
{
WLOG(WS_LOG_DEBUG, "Entering wolfSSH_SendDisconnect");

if (ssh == NULL)
return WS_BAD_ARGUMENT;

/* One disconnect ends the session; a second is more traffic on a
* connection that is already over. */
if (SendAfterDisconnect(ssh))
return WS_FATAL_ERROR;

return SendDisconnect(ssh, reason);
}

Expand Down Expand Up @@ -1586,6 +1679,9 @@ int wolfSSH_ChangeTerminalSize(WOLFSSH* ssh, word32 columns, word32 rows,
if (ssh == NULL)
ret = WS_BAD_ARGUMENT;

if (ret == WS_SUCCESS && SendAfterDisconnect(ssh))
ret = WS_FATAL_ERROR;

if (ret == WS_SUCCESS) {
ret = SendChannelTerminalResize(ssh, columns, rows, widthPixels,
heightPixels);
Expand Down
Loading
Loading