From c88b2582b89d5c21858eb871680c2c792d4adf44 Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Tue, 15 Sep 2026 11:20:03 +0900 Subject: [PATCH 1/2] ssh: report the owed write when the worker leaves output queued - wolfSSH_shutdown() sets ret to WS_WANT_WRITE whenever the wolfSSH_worker() call it makes ends with output pending. The sendErr local and the arm that returned wolfSSH_get_error() are gone. - tests/unit.c gains EofDisconnectCb(), which sends a disconnect from the channel EOF dispatch, and two tests driving it: test_ShutdownOwedWriteAfterEofDisconnect() asserts wolfSSH_shutdown() returns WS_WANT_WRITE with output pending and WS_SUCCESS with the buffer drained on the retry; test_ShutdownHardEofDisconnectSurfacesOnRetry() refuses the disconnect's send outright and asserts the retry returns WS_SOCKET_ERROR_E. --- src/ssh.c | 10 +--- tests/unit.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+), 8 deletions(-) diff --git a/src/ssh.c b/src/ssh.c index 17054f834..7137f7087 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1291,15 +1291,9 @@ int wolfSSH_shutdown(WOLFSSH* ssh) /* received response */ ret = WS_SUCCESS; } - /* Report a write still owed, or the send's own error if it failed. */ + /* The worker left output queued, so a write is still owed. */ if (ret == WS_SUCCESS && wolfSSH_OutputPending(ssh)) { - int sendErr = wolfSSH_get_error(ssh); - - if (sendErr == WS_WANT_WRITE || sendErr == WS_WANT_READ - || sendErr == WS_SUCCESS) - ret = WS_WANT_WRITE; - else - ret = sendErr; + ret = WS_WANT_WRITE; } } diff --git a/tests/unit.c b/tests/unit.c index 392503d14..9c7d69648 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -9326,6 +9326,150 @@ static int test_ChannelEofCallback(void) if (ret == WS_EOF) { result = -1718; goto done; } if (s_eofCbCalls != 1) { result = -1719; goto done; } +done: + s_recvPkt = NULL; + s_recvPktSz = 0; + s_recvPktOff = 0; + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + return result; +} + +/* Disconnects from inside the EOF dispatch. DoChannelEof() discards what + * this returns, so the refused send's status is not an error to report. */ +static int EofDisconnectCb(WOLFSSH_CHANNEL* channel, void* ctx) +{ + WOLFSSH* ssh = (WOLFSSH*)ctx; + + (void)channel; + + (void)wolfSSH_SendDisconnect(ssh, WOLFSSH_DISCONNECT_BY_APPLICATION); + return WS_SUCCESS; +} + +/* The queued DISCONNECT sets ssh->disconnected, so wolfSSH_worker() skips its + * flush and the pass returns WS_EOF with output still owed. The write outranks + * that event: the teardown reports WS_WANT_WRITE and the retry sends it. */ +static int test_ShutdownOwedWriteAfterEofDisconnect(void) +{ + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + WOLFSSH_CHANNEL* ch = NULL; + int result = 0; + int ret; + byte pkt[16]; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + if (ctx == NULL) + return -1900; + wolfSSH_SetIOSend(ctx, WantWriteIoSend); + wolfSSH_SetIORecv(ctx, PacketIoRecv); + if (wolfSSH_CTX_SetChannelEofCb(ctx, EofDisconnectCb) != WS_SUCCESS) { + result = -1901; + goto done; + } + + ssh = wolfSSH_new(ctx); + if (ssh == NULL) { result = -1902; goto done; } + ssh->acceptState = ACCEPT_SERVER_USERAUTH_SENT; + if (wolfSSH_SetChannelEofCtx(ssh, ssh) != WS_SUCCESS) { + result = -1903; + goto done; + } + + ch = ChannelNew(ssh, ID_CHANTYPE_SESSION, 1024, 1024); + if (ch == NULL) { result = -1904; goto done; } + if (ChannelAppend(ssh, ch) != WS_SUCCESS) { + ChannelDelete(ch, ssh->ctx->heap); + result = -1905; + goto done; + } + ch->openConfirmed = 1; + ch->peerWindowSz = 1024; + ch->peerMaxPacketSz = 1024; + + /* The teardown sends are already done, so the read is all that is left + * and the output buffer is empty going into it. */ + ch->eofTxd = 1; + ch->closeTxd = 1; + + s_recvPkt = pkt; + s_recvPktSz = BuildChannelEofPacket(pkt, ch->channel); + s_recvPktOff = 0; + + ret = wolfSSH_shutdown(ssh); + if (ret != WS_WANT_WRITE) { result = -1906; goto done; } + if (!wolfSSH_OutputPending(ssh)) { result = -1907; goto done; } + + /* And the disconnect really does go out once the socket takes bytes. */ + wolfSSH_SetIOSend(ctx, DiscardIoSend); + ret = wolfSSH_shutdown(ssh); + if (ret != WS_SUCCESS) { result = -1908; goto done; } + if (wolfSSH_OutputPending(ssh)) { result = -1909; goto done; } + +done: + s_recvPkt = NULL; + s_recvPktSz = 0; + s_recvPktOff = 0; + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + return result; +} + +/* The owed write is what the first call reports even when the disconnect's + * send failed outright, and the retry's flush surfaces the transport error. */ +static int test_ShutdownHardEofDisconnectSurfacesOnRetry(void) +{ + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + WOLFSSH_CHANNEL* ch = NULL; + int result = 0; + int ret; + byte pkt[16]; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + if (ctx == NULL) + return -1910; + wolfSSH_SetIOSend(ctx, ConnResetIoSend); + wolfSSH_SetIORecv(ctx, PacketIoRecv); + if (wolfSSH_CTX_SetChannelEofCb(ctx, EofDisconnectCb) != WS_SUCCESS) { + result = -1911; + goto done; + } + + ssh = wolfSSH_new(ctx); + if (ssh == NULL) { result = -1912; goto done; } + ssh->acceptState = ACCEPT_SERVER_USERAUTH_SENT; + if (wolfSSH_SetChannelEofCtx(ssh, ssh) != WS_SUCCESS) { + result = -1913; + goto done; + } + + ch = ChannelNew(ssh, ID_CHANTYPE_SESSION, 1024, 1024); + if (ch == NULL) { result = -1914; goto done; } + if (ChannelAppend(ssh, ch) != WS_SUCCESS) { + ChannelDelete(ch, ssh->ctx->heap); + result = -1915; + goto done; + } + ch->openConfirmed = 1; + ch->peerWindowSz = 1024; + ch->peerMaxPacketSz = 1024; + ch->eofTxd = 1; + ch->closeTxd = 1; + + s_recvPkt = pkt; + s_recvPktSz = BuildChannelEofPacket(pkt, ch->channel); + s_recvPktOff = 0; + + ret = wolfSSH_shutdown(ssh); + if (ret != WS_WANT_WRITE) { result = -1916; goto done; } + if (!wolfSSH_OutputPending(ssh)) { result = -1917; goto done; } + + /* The reset is not lost: it takes the return one call later. */ + ret = wolfSSH_shutdown(ssh); + if (ret != WS_SOCKET_ERROR_E) { result = -1918; goto done; } + done: s_recvPkt = NULL; s_recvPktSz = 0; @@ -22852,6 +22996,16 @@ int wolfSSH_UnitTest(int argc, char** argv) printf("ChannelEofCallback: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; + + unitResult = test_ShutdownOwedWriteAfterEofDisconnect(); + printf("ShutdownOwedWriteAfterEofDisconnect: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; + + unitResult = test_ShutdownHardEofDisconnectSurfacesOnRetry(); + printf("ShutdownHardEofDisconnectSurfacesOnRetry: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; #endif /* NO_WOLFSSH_SERVER */ #ifndef NO_WOLFSSH_SERVER From 6c8fdad165e554eec157a78e457fa9d5241bf145 Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Tue, 15 Sep 2026 11:20:03 +0900 Subject: [PATCH 2/2] ssh.h: correct two statements about wolfSSH_worker(), note the owed write - The wolfSSH_worker() send-failure sentence names WS_CHANNEL_CLOSED and WS_FATAL_ERROR as the statuses that keep the return. - Its channelId sentence narrows the rekey case to a WS_REKEYING that displaced WS_SUCCESS or WS_CHAN_RXD. - The wolfSSH_shutdown() block adds that the read for the peer's close reply can leave output queued, which reports WS_WANT_WRITE, and that a send which failed outright there takes the return on the next call. --- wolfssh/ssh.h | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index e2e4b4c77..65f07758b 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -97,8 +97,8 @@ WOLFSSH_API void wolfSSH_free(WOLFSSH* ssh); * A want is transient either way: call again. A caller that tolerates only * WS_WANT_READ drops live sessions, since a queued write reports * WS_WANT_WRITE. A send that fails outright takes the return instead, since - * the event it arrived with is moot once the transport is gone; only a - * WS_CHANNEL_CLOSED keeps the return there. + * the event it arrived with is moot once the transport is gone. + * WS_CHANNEL_CLOSED and WS_FATAL_ERROR keep the return there. * Any other code is an error, either in the return itself or as * WS_FATAL_ERROR with the cause in wolfSSH_get_error() -- WS_DISCONNECT for * the peer's disconnect, which is how most sessions end. @@ -106,8 +106,8 @@ WOLFSSH_API void wolfSSH_free(WOLFSSH* ssh); * than reading a status: it answers after any return, including a success. * * For WS_CHAN_RXD, WS_EXTDATA, WS_EOF, WS_SUCCESS and a WS_REKEYING that - * displaced one of those, channelId (when not NULL) names the channel the - * event belongs to. It is left alone for every other status, + * displaced WS_SUCCESS or WS_CHAN_RXD, channelId (when not NULL) names the + * channel the event belongs to. It is left alone for every other status, * WS_CHANNEL_CLOSED included; use wolfSSH_GetLastRxId() there. * * Note that after a peer half-close wolfSSH_stream_send() keeps working: the @@ -804,9 +804,11 @@ WOLFSSH_API int wolfSSH_connect(WOLFSSH* ssh); * USERAUTH_FAILURE, and a CHANNEL_CLOSE whose channel was retired the * moment it was bundled, have nothing else left to carry the retry. That * flush can be short too, so a WS_WANT_WRITE from here may be owed to it - * rather than to the teardown sends; either way the caller retries. Once - * the peer has disconnected, only our own queued disconnect still goes - * out, per the comment below. */ + * rather than to the teardown sends; either way the caller retries. + * The read for the peer's close reply can leave output queued behind it, + * which reports WS_WANT_WRITE as well; a send that failed outright there + * takes the return on the next call. Once the peer has disconnected, only + * our own queued disconnect still goes out, per the comment below. */ WOLFSSH_API int wolfSSH_shutdown(WOLFSSH* ssh); /* A disconnect, sent or received, ends the session. Nothing more goes out: * wolfSSH_shutdown() above this comment, and every send call below it,