diff --git a/src/internal.c b/src/internal.c index 41168633a..fdaf71639 100644 --- a/src/internal.c +++ b/src/internal.c @@ -12225,7 +12225,17 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed) return WS_MSGID_NOT_ALLOWED_E; } - switch (msg) { + /* The session is over, RFC 4253 section 11.1, so skip the whole dispatch: + * the handlers that answer must not, and what the rest would record is of + * no use to a caller that can no longer send. Inbound data from here on is + * dropped rather than buffered. The frame advance at the end steps over + * the packet, so the stream stays in step. A DISCONNECT still dispatches, + * since DoDisconnect() sends nothing and latches the error. */ + if (ssh->disconnected && msg != MSGID_DISCONNECT) { + WLOG(WS_LOG_DEBUG, "Ignoring message ID %u after a disconnect", + (word32)msg); + } + else switch (msg) { case MSGID_DISCONNECT: WLOG(WS_LOG_DEBUG, "Decoding MSGID_DISCONNECT"); diff --git a/src/ssh.c b/src/ssh.c index c15ccb612..32c234b2a 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -550,6 +550,10 @@ int wolfSSH_CTX_UseTpmHostKey(WOLFSSH_CTX* ctx, #endif /* WOLFSSH_TPM */ +/* Defined below, ahead of both drivers; either can be the only one built. */ +static int SendAfterDisconnect(WOLFSSH* ssh); + + #ifndef NO_WOLFSSH_SERVER const char acceptError[] = "accept error: %s, %d"; @@ -563,6 +567,11 @@ int wolfSSH_accept(WOLFSSH* ssh) if (ssh == NULL) return WS_BAD_ARGUMENT; + /* No handshake on a session that is over. The pending-send block below + * would flush a queued disconnect as the next handshake message. */ + if (SendAfterDisconnect(ssh)) + return WS_FATAL_ERROR; + /* clear want read/writes for retry */ if (ssh->error == WS_WANT_READ || ssh->error == WS_WANT_WRITE || ssh->error == WS_AUTH_PENDING) ssh->error = 0; @@ -826,6 +835,11 @@ int wolfSSH_connect(WOLFSSH* ssh) if (ssh == NULL) return WS_BAD_ARGUMENT; + /* See wolfSSH_accept(). No error-state test here, so the peer's + * disconnect reaches the state machine like a local one. */ + if (SendAfterDisconnect(ssh)) + return WS_FATAL_ERROR; + /* check if data pending to be sent */ if (ssh->outputBuffer.length > 0 && ssh->connectState < CONNECT_SERVER_CHANNEL_REQUEST_DONE) { @@ -3590,6 +3604,16 @@ int wolfSSH_worker(WOLFSSH* ssh, word32* channelId) if (ssh == NULL) ret = WS_BAD_ARGUMENT; + /* Nothing left to drive: no reply may go out and inbound messages are + * skipped, so every pass from here on would answer WS_SUCCESS off a + * dispatch that did nothing and a caller turning the crank would never + * see the session end. What arrived before the disconnect is still the + * caller's, through the read calls. RFC 4253 section 11.1. */ + if (ret == WS_SUCCESS && SendAfterDisconnect(ssh)) { + WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_worker(), session disconnected"); + return WS_FATAL_ERROR; + } + #ifdef WOLFSSH_TEST_BLOCK /* In forced non-blocking test mode, keep legacy ordering (send before * receive) to match the harness expectations and avoid synthetic spins. */ @@ -3639,7 +3663,9 @@ int wolfSSH_worker(WOLFSSH* ssh, word32* channelId) } /* WS_EXTDATA is raised once, on arrival; masking it would strand the - * buffered stderr and its window credit. */ + * buffered stderr and its window credit. A disconnect cannot be seen + * here: the gate at the top returns before this, and the DISCONNECT + * that sets the flag mid-pass leaves ret fatal. */ if (ssh->isKeying && ret != WS_EXTDATA) { ssh->error = WS_REKEYING; return WS_REKEYING; diff --git a/tests/regress.c b/tests/regress.c index 76ff4618a..daad3edda 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -265,6 +265,31 @@ static WS_MAYBE_UNUSED word32 BuildExtInfoSigAlgs(byte* buf, word32 bufSz, return AppendString(buf, bufSz, idx, sigAlgs); } +static word32 BuildChannelClosePacket(word32 peerChannelId, byte* out, + word32 outSz) +{ + byte payload[16]; + word32 idx = 0; + + idx = AppendUint32(payload, sizeof(payload), idx, peerChannelId); + + return WrapPacket(MSGID_CHANNEL_CLOSE, payload, idx, out, outSz); +} + + +static word32 BuildChannelDataPacket(word32 peerChannelId, const char* data, + byte* out, word32 outSz) +{ + byte payload[64]; + word32 idx = 0; + + idx = AppendUint32(payload, sizeof(payload), idx, peerChannelId); + idx = AppendString(payload, sizeof(payload), idx, data); + + return WrapPacket(MSGID_CHANNEL_DATA, payload, idx, out, outSz); +} + + #ifdef WOLFSSH_FWD static word32 BuildDirectTcpipExtra(const char* host, word32 hostPort, const char* origin, word32 originPort, byte* out, word32 outSz) @@ -4194,6 +4219,476 @@ static void TestDisconnectOutranksRekey(void) } +/* wolfSSH_worker() is the other drive loop, and the one the SFTP and SCP + * layers turn. Once the session is over it has nothing to drive: the + * dispatch is skipped, so a post-disconnect message would leave + * ssh->error at WS_SUCCESS and the worker would keep reporting a healthy + * session for as long as the peer talks. It also outranks a rekey the peer + * abandoned, which only NEWKEYS could clear. RFC 4253 section 11.1. */ +static void TestWorkerReportsDisconnect(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + MemIo io; + byte in[256]; + byte out[256]; + word32 inSz; + word32 idx; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL); + AssertNotNull(ctx); + + wolfSSH_SetIORecv(ctx, MemRecv); + wolfSSH_SetIOSend(ctx, MemSend); + + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AddSessionChannel(ssh); + ssh->connectState = CONNECT_SERVER_USERAUTH_ACCEPT_DONE; + + /* The peer's KEXINIT, the way DoKexInit records it. Nothing clears it + * after the disconnect below, so it latches for the session. */ + ssh->isKeying |= WOLFSSH_PEER_IS_KEYING; + + /* The peer's disconnect, then a message behind it. An IGNORE draws no + * reply of its own, so what goes out can only come from the worker. */ + idx = BuildDisconnectPacket(WOLFSSH_DISCONNECT_BY_APPLICATION, + in, sizeof(in)); + inSz = idx + BuildPacket(MSGID_IGNORE, in + idx, sizeof(in) - idx); + MemIoInit(&io, in, inSz, out, sizeof(out)); + wolfSSH_SetIOReadCtx(ssh, &io); + wolfSSH_SetIOWriteCtx(ssh, &io); + + AssertIntEQ(wolfSSH_worker(ssh, NULL), WS_FATAL_ERROR); + AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT); + AssertTrue(ssh->disconnected); + AssertTrue(ssh->isKeying != 0); + io.outSz = 0; + + /* The message behind it is still queued, and every further pass reports + * the disconnect rather than the WS_SUCCESS of a skipped dispatch or the + * WS_REKEYING of a rekey that cannot finish. */ + AssertIntEQ(wolfSSH_worker(ssh, NULL), WS_FATAL_ERROR); + AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT); + AssertIntEQ(wolfSSH_worker(ssh, NULL), WS_FATAL_ERROR); + AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT); + + /* Nothing went out on any of them. */ + AssertIntEQ(io.outSz, 0); + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} + + + +#ifndef NO_WOLFSSH_SERVER + +/* wolfSSH_accept() drives the handshake, so a session that is already over + * must stop it the way it stops every other sender. The short-send case is + * the sharp one: the pending-send block at the top would push out a + * disconnect left queued by a short send and then count it as the handshake + * message the state machine was waiting for. RFC 4253 section 11.1. */ +static void TestDisconnectGatesAccept(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + MemIo io; + byte in[128]; + byte out[512]; + word32 inSz; + word32 quietSz; + byte state; + int ret; + + /* A local disconnect leaves ssh->error clear, so the "in error state" + * test in wolfSSH_accept() never sees it. */ + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + AssertNotNull(ctx); + wolfSSH_SetIORecv(ctx, MemRecv); + wolfSSH_SetIOSend(ctx, MemSend); + + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AddSessionChannel(ssh); + /* Not one of the states wolfSSH_accept() holds back, so an unwanted + * advance shows up in the assertions below. */ + ssh->acceptState = ACCEPT_SERVER_USERAUTH_SENT; + + MemIoInit(&io, NULL, 0, out, sizeof(out)); + wolfSSH_SetIOReadCtx(ssh, &io); + wolfSSH_SetIOWriteCtx(ssh, &io); + + AssertIntEQ(wolfSSH_SendDisconnect(ssh, WOLFSSH_DISCONNECT_BY_APPLICATION), + WS_SUCCESS); + AssertTrue(ssh->disconnected); + AssertIntEQ(wolfSSH_get_error(ssh), 0); + quietSz = io.outSz; + state = ssh->acceptState; + + ret = wolfSSH_accept(ssh); + AssertIntEQ(ret, WS_FATAL_ERROR); + AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT); + /* No handshake packet, and the state machine did not move. */ + AssertIntEQ(io.outSz, quietSz); + AssertIntEQ(ssh->acceptState, state); + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + + /* Our disconnect short-sends, so it is sitting in the output buffer + * with a flush owed. wolfSSH_shutdown() and wolfSSH_SendDisconnect() + * own that flush; wolfSSH_accept() must leave it alone. */ + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + AssertNotNull(ctx); + wolfSSH_SetIORecv(ctx, MemRecv); + wolfSSH_SetIOSend(ctx, MemSendWantWrite); + + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AddSessionChannel(ssh); + /* Not one of the states wolfSSH_accept() holds back, so an unwanted + * advance shows up in the assertions below. */ + ssh->acceptState = ACCEPT_SERVER_USERAUTH_SENT; + + MemIoInit(&io, NULL, 0, out, sizeof(out)); + wolfSSH_SetIOReadCtx(ssh, &io); + wolfSSH_SetIOWriteCtx(ssh, &io); + + MemSendWantWriteCount = 1; + AssertIntEQ(wolfSSH_SendDisconnect(ssh, WOLFSSH_DISCONNECT_BY_APPLICATION), + WS_WANT_WRITE); + AssertTrue(ssh->disconnected); + AssertTrue(ssh->disconnectTxd); + AssertTrue(wolfSSH_OutputPending(ssh)); + AssertIntEQ(io.outSz, 0); + state = ssh->acceptState; + + ret = wolfSSH_accept(ssh); + AssertIntEQ(ret, WS_FATAL_ERROR); + AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT); + /* Still queued, and not mistaken for the awaited handshake message. */ + AssertIntEQ(io.outSz, 0); + AssertTrue(wolfSSH_OutputPending(ssh)); + AssertIntEQ(ssh->acceptState, state); + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + + /* The peer's disconnect latches WS_DISCONNECT, which the error-state + * test below the gate used to answer with WS_INVALID_STATE_E. */ + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + AssertNotNull(ctx); + wolfSSH_SetIORecv(ctx, MemRecv); + wolfSSH_SetIOSend(ctx, MemSend); + + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AddSessionChannel(ssh); + ssh->acceptState = ACCEPT_SERVER_USERAUTH_SENT; + + inSz = BuildDisconnectPacket(WOLFSSH_DISCONNECT_BY_APPLICATION, + in, sizeof(in)); + MemIoInit(&io, in, inSz, out, sizeof(out)); + wolfSSH_SetIOReadCtx(ssh, &io); + wolfSSH_SetIOWriteCtx(ssh, &io); + + AssertIntEQ(DoReceive(ssh), WS_FATAL_ERROR); + AssertTrue(ssh->disconnected); + AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT); + quietSz = io.outSz; + state = ssh->acceptState; + + ret = wolfSSH_accept(ssh); + AssertIntEQ(ret, WS_FATAL_ERROR); + AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT); + AssertIntEQ(io.outSz, quietSz); + AssertIntEQ(ssh->acceptState, state); + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} + +#endif /* !NO_WOLFSSH_SERVER */ + + +#ifndef NO_WOLFSSH_CLIENT + +/* The same gate on the client's driver, which has no error-state test of + * its own. Split from the accept test so a single-sided build keeps the + * coverage that applies to it. */ +static void TestDisconnectGatesConnect(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + MemIo io; + byte in[128]; + byte out[512]; + word32 inSz; + byte state; + int ret; + + /* wolfSSH_connect() has no error-state test of its own, so it reaches + * the state machine after a disconnect from either side. */ + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL); + AssertNotNull(ctx); + wolfSSH_SetIORecv(ctx, MemRecv); + wolfSSH_SetIOSend(ctx, MemSendWantWrite); + + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AddSessionChannel(ssh); + ssh->connectState = CONNECT_SERVER_USERAUTH_ACCEPT_DONE; + + MemIoInit(&io, NULL, 0, out, sizeof(out)); + wolfSSH_SetIOReadCtx(ssh, &io); + wolfSSH_SetIOWriteCtx(ssh, &io); + + MemSendWantWriteCount = 1; + AssertIntEQ(wolfSSH_SendDisconnect(ssh, WOLFSSH_DISCONNECT_BY_APPLICATION), + WS_WANT_WRITE); + AssertTrue(ssh->disconnected); + AssertTrue(wolfSSH_OutputPending(ssh)); + AssertIntEQ(io.outSz, 0); + state = ssh->connectState; + + ret = wolfSSH_connect(ssh); + AssertIntEQ(ret, WS_FATAL_ERROR); + AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT); + AssertIntEQ(io.outSz, 0); + AssertTrue(wolfSSH_OutputPending(ssh)); + AssertIntEQ(ssh->connectState, state); + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + + /* The peer's disconnect reaches the same gate. */ + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL); + AssertNotNull(ctx); + wolfSSH_SetIORecv(ctx, MemRecv); + wolfSSH_SetIOSend(ctx, MemSend); + + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AddSessionChannel(ssh); + ssh->connectState = CONNECT_SERVER_USERAUTH_ACCEPT_DONE; + + inSz = BuildDisconnectPacket(WOLFSSH_DISCONNECT_BY_APPLICATION, + in, sizeof(in)); + MemIoInit(&io, in, inSz, out, sizeof(out)); + wolfSSH_SetIOReadCtx(ssh, &io); + wolfSSH_SetIOWriteCtx(ssh, &io); + + AssertIntEQ(DoReceive(ssh), WS_FATAL_ERROR); + AssertTrue(ssh->disconnected); + AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT); + state = ssh->connectState; + + ret = wolfSSH_connect(ssh); + AssertIntEQ(ret, WS_FATAL_ERROR); + AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT); + AssertIntEQ(io.outSz, 0); + AssertIntEQ(ssh->connectState, state); + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} + +#endif /* !NO_WOLFSSH_CLIENT */ + + +/* The public senders sit behind the disconnect gate, but the replies the + * library builds in answer to inbound traffic did not. A channel close + * draws an EOF and a close of ours out of DoChannelClose(), and a channel + * open a confirmation or a failure out of DoChannelOpen(); on a session + * that is already over, none of that may reach the peer. RFC 4253 + * section 11.1. */ +static void TestDisconnectSilencesInboundReplies(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + WOLFSSH_CHANNEL* channel; + MemIo io; + byte in[256]; + byte out[512]; + word32 inSz; + word32 quietSz; + word32 channelId; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL); + AssertNotNull(ctx); + + wolfSSH_SetIORecv(ctx, MemRecv); + wolfSSH_SetIOSend(ctx, MemSend); + + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AddSessionChannel(ssh); + channel = ssh->channelList; + channelId = channel->channel; + /* Past userauth, or the message filter turns the inbound messages away + * on its own and the wire check below proves nothing. */ + ssh->connectState = CONNECT_SERVER_USERAUTH_ACCEPT_DONE; + + inSz = BuildChannelClosePacket(channelId, in, sizeof(in)); + MemIoInit(&io, in, inSz, out, sizeof(out)); + wolfSSH_SetIOReadCtx(ssh, &io); + wolfSSH_SetIOWriteCtx(ssh, &io); + + /* Our own disconnect goes out first, and is the last thing that may. */ + AssertIntEQ(wolfSSH_SendDisconnect(ssh, WOLFSSH_DISCONNECT_BY_APPLICATION), + WS_SUCCESS); + AssertTrue(ssh->disconnected); + quietSz = io.outSz; + AssertTrue(quietSz > 0); + + /* The peer's close arrives anyway: a caller's own wolfSSH_worker() loop + * still reads after a disconnect, and shutdown's pump can find packets + * queued behind the peer's DISCONNECT. A skipped packet + * is not an error; assert that, so the quiet-wire checks below cannot + * pass on a receive that never reached DoPacket(). */ + AssertIntEQ(DoReceive(ssh), WS_SUCCESS); + + /* No EOF and no close went out, and the handler never ran, so the + * channel it would have torn down is still on the list. */ + AssertIntEQ(io.outSz, quietSz); + AssertNotNull(ssh->channelList); + AssertIntEQ(ssh->channelList->channel, channelId); + AssertFalse(channel->eofTxd); + AssertFalse(channel->closeTxd); + + /* A channel open is the other half: it answers with a confirmation or + * a failure, and neither may go out now. */ + inSz = BuildChannelOpenPacket("session", 99, 1024, 1024, NULL, 0, + in, sizeof(in)); + MemIoInit(&io, in, inSz, out, sizeof(out)); + io.outSz = quietSz; + wolfSSH_SetIOReadCtx(ssh, &io); + wolfSSH_SetIOWriteCtx(ssh, &io); + + AssertIntEQ(DoReceive(ssh), WS_SUCCESS); + AssertIntEQ(io.outSz, quietSz); + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} + + +/* Skipping the dispatch is all the gate does: the frame advance steps over + * the whole packet on its own, so a packet behind a skipped one is still + * found where it should be. And a DISCONNECT is not skipped -- DoDisconnect() + * sends nothing, and it is what latches WS_DISCONNECT for the caller, so + * swallowing the peer's would report a live session on a dead one. */ +static void TestDisconnectKeepsStreamInStep(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + WOLFSSH_CHANNEL* channel; + MemIo io; + byte in[256]; + byte out[512]; + word32 inSz; + word32 quietSz; + word32 channelId; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL); + AssertNotNull(ctx); + + wolfSSH_SetIORecv(ctx, MemRecv); + wolfSSH_SetIOSend(ctx, MemSend); + + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AddSessionChannel(ssh); + channel = ssh->channelList; + channelId = channel->channel; + ssh->connectState = CONNECT_SERVER_USERAUTH_ACCEPT_DONE; + + /* A channel close for the gate to skip, with the peer's disconnect behind + * it in the same read. */ + inSz = BuildChannelClosePacket(channelId, in, sizeof(in)); + inSz += BuildDisconnectPacket(WOLFSSH_DISCONNECT_BY_APPLICATION, + in + inSz, (word32)sizeof(in) - inSz); + MemIoInit(&io, in, inSz, out, sizeof(out)); + wolfSSH_SetIOReadCtx(ssh, &io); + wolfSSH_SetIOWriteCtx(ssh, &io); + + /* Ours goes out first, and is the last thing that may. */ + AssertIntEQ(wolfSSH_SendDisconnect(ssh, WOLFSSH_DISCONNECT_BY_APPLICATION), + WS_SUCCESS); + AssertTrue(ssh->disconnected); + quietSz = io.outSz; + AssertTrue(quietSz > 0); + + /* The close is skipped, so no reply and the channel stays. */ + AssertIntEQ(DoReceive(ssh), WS_SUCCESS); + AssertIntEQ(io.outSz, quietSz); + AssertNotNull(ssh->channelList); + AssertIntEQ(ssh->channelList->channel, channelId); + AssertFalse(channel->eofTxd); + AssertFalse(channel->closeTxd); + + /* The disconnect behind it decodes and latches, and still answers + * nothing. */ + AssertIntEQ(DoReceive(ssh), WS_FATAL_ERROR); + AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT); + AssertIntEQ(io.outSz, quietSz); + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} + + +/* Skipping the dispatch drops what arrives, it does not queue it: the gate + * covers inbound data, not only replies. A caller that can no longer send has + * nothing to do with it. Pinned because the read path still hands back data + * that arrived before the disconnect, and the two are easy to confuse. */ +static void TestDisconnectDropsLateChannelData(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + MemIo io; + byte in[256]; + byte out[512]; + byte buf[32]; + word32 inSz; + word32 channelId; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL); + AssertNotNull(ctx); + + wolfSSH_SetIORecv(ctx, MemRecv); + wolfSSH_SetIOSend(ctx, MemSend); + + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AddSessionChannel(ssh); + channelId = ssh->channelList->channel; + ssh->connectState = CONNECT_SERVER_USERAUTH_ACCEPT_DONE; + + inSz = BuildChannelDataPacket(channelId, "late", in, sizeof(in)); + MemIoInit(&io, in, inSz, out, sizeof(out)); + wolfSSH_SetIOReadCtx(ssh, &io); + wolfSSH_SetIOWriteCtx(ssh, &io); + + AssertIntEQ(wolfSSH_SendDisconnect(ssh, WOLFSSH_DISCONNECT_BY_APPLICATION), + WS_SUCCESS); + AssertTrue(ssh->disconnected); + + /* Read it, and it is gone: nothing buffered on the channel. */ + AssertIntEQ(DoReceive(ssh), WS_SUCCESS); + AssertNotNull(ssh->channelList); + AssertIntEQ(ssh->channelList->inputBuffer.length + - ssh->channelList->inputBuffer.idx, 0); + + /* So the read reports the dead session rather than the dropped bytes. */ + AssertIntEQ(wolfSSH_stream_read(ssh, buf, sizeof(buf)), WS_FATAL_ERROR); + AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT); + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} + /* disconnectTxd means "a flush is owed", not "a disconnect was sent". Once * ours has gone out, a teardown call must not push whatever the internal * senders queued behind it. */ @@ -8079,6 +8574,15 @@ int main(int argc, char** argv) TestDisconnectDrainsBufferedData(); TestDisconnectBlocksEverySend(); TestSendDisconnectIsTerminal(); +#ifndef NO_WOLFSSH_SERVER + TestDisconnectGatesAccept(); +#endif +#ifndef NO_WOLFSSH_CLIENT + TestDisconnectGatesConnect(); +#endif + TestDisconnectSilencesInboundReplies(); + TestDisconnectKeepsStreamInStep(); + TestDisconnectDropsLateChannelData(); TestDisconnectQuietWindowAdjust(); TestDisconnectBlocksChannelAndFwdSends(); TestStreamExitReportsDisconnect(); @@ -8093,6 +8597,7 @@ int main(int argc, char** argv) TestShutdownKeepsFlushWantWrite(); TestDisconnectTxdClearsOnFlush(); TestDisconnectOutranksRekey(); + TestWorkerReportsDisconnect(); #if defined(WOLFSSH_TERM) && !defined(NO_FILESYSTEM) TestTerminalResizeBlockedAfterDisconnect(); #endif diff --git a/wolfssh/internal.h b/wolfssh/internal.h index c4a3d8951..f3c7fd1f4 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1105,9 +1105,12 @@ struct WOLFSSH { /* Set when a DISCONNECT is sent or received. Gates the public send * calls, so nothing more goes out. Reads still hand back what arrived * before the disconnect; the head-of-list reads report it once their - * buffer runs dry, unless a CHANNEL_EOF arrived first. wolfSSH_worker(), - * wolfSSH_accept() and wolfSSH_connect() are not gated; the shutdown - * paths pump the worker. */ + * buffer runs dry, unless a CHANNEL_EOF arrived first. + * wolfSSH_worker() reports it as well, so a drive loop stops turning; + * wolfSSH_shutdown() drops the channel first and so never pumps it. + * DoPacket() skips the inbound dispatch too, for every message but a + * DISCONNECT, so nothing arriving afterward is buffered, answered or + * reported through the channel callbacks. */ byte disconnected; /* Set once SendDisconnect() has bundled our own DISCONNECT into the * output buffer, and cleared once wolfSSH_SendPacket() drains it, so it diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index 9d856d771..da09504f9 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -563,13 +563,17 @@ WOLFSSH_API int wolfSSH_connect(WOLFSSH* ssh); 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, - * report WS_DISCONNECT from then on. wolfSSH_accept() and - * wolfSSH_connect() are not gated; do not drive the handshake after a - * disconnect. Reads are not gated either, so channel data that - * arrived before the disconnect can still be drained; wolfSSH_stream_read() - * and wolfSSH_stream_peek() report WS_DISCONNECT once their buffer runs - * dry. A CHANNEL_EOF already received outranks that drain: both report - * WS_EOF with data possibly still buffered. RFC 4253 section 11.1. */ + * report WS_DISCONNECT from then on, as do wolfSSH_accept(), + * wolfSSH_connect() and wolfSSH_worker(). None of the three flushes a + * disconnect of ours left queued by a short send; wolfSSH_shutdown() or + * another wolfSSH_SendDisconnect() owns that. Inbound traffic is dropped: + * the receive path skips every message but a DISCONNECT, so late channel + * data is discarded and the channel callbacks stop firing. Reads are not + * gated, so channel data that arrived before the disconnect can still be + * drained; wolfSSH_stream_read() and wolfSSH_stream_peek() report + * WS_DISCONNECT once their buffer runs dry. A CHANNEL_EOF already received + * outranks that drain: both report WS_EOF with data possibly still + * buffered. RFC 4253 section 11.1. */ WOLFSSH_API int wolfSSH_stream_peek(WOLFSSH* ssh, byte* buf, word32 bufSz); /* Returns the bytes read; the next read clears the status. WS_WANT_WRITE * from wolfSSH_get_error() means the adjust is queued; it goes out on the