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 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,