WolfSSL
/
wolfssh
mirror of https://github.com/wolfSSL/wolfssh.git
1
0
Fork 0
Code Issues Releases Wiki Activity

ssh, internal: always flush the worker's output 

- wolfSSH_worker() calls wolfSSH_SendPacket() whenever
  ssh->outputBuffer holds bytes and the session is not
  disconnected. ssh->error keeps the receive's code when the
  receive failed, and the close's when a WS_CHANNEL_CLOSED pass
  hard-failed its flush; WS_REKEYING is withheld on a failed
  flush. Drops the second DoReceive(), the WOLFSSH_TEST_BLOCK
  fork and the separate WS_CHANNEL_CLOSED flush.
- BundlePacket() resets ssh->outputBuffer.length to
  ssh->packetStartIdx when the framing fails. wolfSSH_shutdown()
  reports WS_WANT_WRITE when its close-read leaves output queued,
  and the send's own error in place of it when that send failed.
  SendPacketFlush() records its code in ssh->error on every
  transport failure path, and wolfSSH_TriggerKeyExchange() writes
  it only when SendKexInit() fails.
- portfwd, client and scpclient accept WS_WANT_WRITE from
  wolfSSH_shutdown(); in scpclient the close-message drain runs
  on it.
- wolfssh/ssh.h drops WS_WINDOW_FULL from wolfSSH_worker() and says
  to read the return and wolfSSH_get_error() as independent channels
  on every pass.
- Twenty unit tests and the extended TestWorkerReportsDisconnect
  cover what ret and ssh->error hold after a receive, send, buffer,
  callback or framing failure.
pull/1249/head
Yosuke Shimizu 2026-09-02 17:40:50 +09:00 committed by John Safranek
parent ab979865aa
commit 586b697b18
10 changed files with 1451 additions and 131 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
examples/client/client.c
View File

@ -1203,7 +1203,7 @@ THREAD_RETURN WOLFSSH_THREAD client_test(void* args)
* is still owed, so the drain below is exactly what is wanted. */
if (ret != WS_SOCKET_ERROR_E && wolfSSH_get_error(ssh) != WS_SOCKET_ERROR_E
&& wolfSSH_get_error(ssh) != WS_CHANNEL_CLOSED) {
if (ret != WS_SUCCESS) {
if (ret != WS_SUCCESS && ret != WS_WANT_WRITE) {
ClientFreeBuffers(pubKeyName, privKeyName, NULL);
wolfSSH_free(ssh);
wolfSSH_CTX_free(ctx);

9
examples/echoserver/echoserver.c
View File

@ -1040,12 +1040,9 @@ static int ssh_worker(thread_ctx_t* threadCtx)
rc = wolfSSH_get_error(ssh);
/* The peer is done sending: hand back the backlog and answer
* its EOF, or a client that half-closed waits on a server
* that never finishes -- the library no longer answers for
* us. Off the channel's own state, not the WS_EOF status: the
* flush inside wolfSSH_worker() can supersede that, and it is
* raised once. Echo mode only; a shell child on a pty is
* still producing, so its EOF waits for the child to exit. */
* its EOF, since the library no longer answers for us. Off
* the channel's own state, not the once-only WS_EOF status.
* Echo mode only; a shell child on a pty still produces. */
if (!eofAnswered && echoOnly) {
WOLFSSH_CHANNEL* eofChannel;

11
examples/portfwd/portfwd.c
View File

@ -832,10 +832,9 @@ THREAD_RETURN WOLFSSH_THREAD portfwd_worker(void* args)
/* Relay the half-close so a local reader waiting on end-of-input
* returns; nothing else relays it. Driven off the latched channel
* state, not the WS_EOF status: the flush inside wolfSSH_worker()
* can supersede that, and it is raised only once. Only the channel
* appFd is wired to, since half-closing the wrong socket truncates
* a live transfer. */
* state, not the once-only WS_EOF status. Only the channel appFd
* is wired to: half-closing the wrong socket truncates a live
* transfer. */
if (appFdSet && fwdChannel != NULL && !appFdHalfClosed
&& wolfSSH_ChannelGetEof(fwdChannel)) {
int drained;
@ -935,7 +934,9 @@ THREAD_RETURN WOLFSSH_THREAD portfwd_worker(void* args)
}
ret = wolfSSH_shutdown(ssh);
if (ret != WS_SUCCESS)
/* The socket closes next, so a queued write and a retired channel are
* both done as far as this teardown is concerned. */
if (ret != WS_SUCCESS && ret != WS_WANT_WRITE && ret != WS_CHANNEL_CLOSED)
err_sys("Closing port forward stream failed.");
WCLOSESOCKET(sshFd);

4
examples/scpclient/scpclient.c
View File

@ -323,7 +323,7 @@ THREAD_RETURN WOLFSSH_THREAD scp_client(void* args)
if (ret != WS_CHANNEL_CLOSED && ret != WS_SOCKET_ERROR_E &&
wolfSSH_get_error(ssh) != WS_SOCKET_ERROR_E &&
wolfSSH_get_error(ssh) != WS_CHANNEL_CLOSED) {
if (ret != WS_SUCCESS) {
if (ret != WS_SUCCESS && ret != WS_WANT_WRITE) {
WLOG(WS_LOG_DEBUG, "Sending the shutdown messages failed.");
}
else {
@ -351,7 +351,7 @@ THREAD_RETURN WOLFSSH_THREAD scp_client(void* args)
#endif
if ((ret != WS_SUCCESS) && (ret != WS_CHANNEL_CLOSED)
&& (ret != WS_EOF))
&& (ret != WS_EOF) && (ret != WS_WANT_WRITE))
((func_args*)args)->return_code = 1;
return 0;
}

9
ide/Espressif/ESP-IDF/examples/wolfssh_echoserver/main/echoserver.c
View File

@ -1000,12 +1000,9 @@ static int ssh_worker(thread_ctx_t* threadCtx)
rc = wolfSSH_get_error(ssh);
/* The peer is done sending: hand back the backlog and answer
* its EOF, or a client that half-closed waits on a server
* that never finishes -- the library no longer answers for
* us. Off the channel's own state, not the WS_EOF status: the
* flush inside wolfSSH_worker() can supersede that, and it is
* raised once. Echo mode only; a shell child on a pty is
* still producing, so its EOF waits for the child to exit. */
* its EOF, since the library no longer answers for us. Off
* the channel's own state, not the once-only WS_EOF status.
* Echo mode only; a shell child on a pty still produces. */
if (!eofAnswered && echoOnly) {
WOLFSSH_CHANNEL* eofChannel;

12
src/internal.c
View File

@ -5479,6 +5479,7 @@ static int SendPacketFlush(WOLFSSH* ssh)
if (ssh->ctx->ioSendCb == NULL) {
WLOG(WS_LOG_DEBUG, "Your IO Send callback is null, please set");
ssh->error = WS_SOCKET_ERROR_E;
return WS_SOCKET_ERROR_E;
}
@ -5489,6 +5490,7 @@ static int SendPacketFlush(WOLFSSH* ssh)
if (ssh->outputBuffer.length > ssh->outputBuffer.bufferSz ||
ssh->outputBuffer.length < ssh->outputBuffer.idx) {
WLOG(WS_LOG_ERROR, "Bad buffer state");
ssh->error = WS_BUFFER_E;
return WS_BUFFER_E;
}
@ -5527,11 +5529,13 @@ static int SendPacketFlush(WOLFSSH* ssh)
ssh->outputBuffer.plainSz = 0;
ShrinkBuffer(&ssh->outputBuffer, 1);
}
ssh->error = WS_SOCKET_ERROR_E;
return WS_SOCKET_ERROR_E;
}
if ((word32)sent > ssh->outputBuffer.length) {
WLOG(WS_LOG_DEBUG, "wolfSSH_SendPacket() out of bounds read");
ssh->error = WS_SEND_OOB_READ_E;
return WS_SEND_OOB_READ_E;
}
@ -5556,7 +5560,9 @@ static int SendPacketFlush(WOLFSSH* ssh)
}
/* returns WS_SUCCESS on success */
/* returns WS_SUCCESS on success. Transport failures record their code in
* ssh->error, so a later write to that field on the same pass has to be
* conditional on this having succeeded, or it hides the dead transport. */
int wolfSSH_SendPacket(WOLFSSH* ssh)
{
int ret;
@ -14908,6 +14914,10 @@ static int BundlePacket(WOLFSSH* ssh)
}
else {
WLOG(WS_LOG_DEBUG, "BP: failed to encrypt buffer");
if (ssh != NULL) {
/* Drop the aborted packet */
ssh->outputBuffer.length = ssh->packetStartIdx;
}
}
return ret;

97
src/ssh.c
View File

@ -1292,6 +1292,16 @@ int wolfSSH_shutdown(WOLFSSH* ssh)
/* received response */
ret = WS_SUCCESS;
}
/* Report a write still owed, or the send's own error if it failed. */
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;
}
}
if (ssh != NULL && ssh->channelList == NULL) {
@ -1328,8 +1338,11 @@ int wolfSSH_TriggerKeyExchange(WOLFSSH* ssh)
if (ret == WS_SUCCESS && SendAfterDisconnect(ssh))
ret = WS_FATAL_ERROR;
if (ret == WS_SUCCESS)
ret = ssh->error = SendKexInit(ssh);
if (ret == WS_SUCCESS) {
ret = SendKexInit(ssh);
if (ret != WS_SUCCESS)
ssh->error = ret;
}
WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_TriggerKeyExchange(), ret = %d", ret);
return ret;
@ -4594,6 +4607,7 @@ word32 wolfSSH_GetSessionCommandSz(const WOLFSSH* ssh)
int wolfSSH_worker(WOLFSSH* ssh, word32* channelId)
{
int ret = WS_SUCCESS;
int sendRet = WS_SUCCESS;
WLOG(WS_LOG_DEBUG, "Entering wolfSSH_worker()");
@ -4610,58 +4624,29 @@ int wolfSSH_worker(WOLFSSH* ssh, word32* channelId)
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. */
if (ret == WS_SUCCESS) {
if (ssh->outputBuffer.length != 0)
ret = wolfSSH_SendPacket(ssh);
}
if (ret == WS_SUCCESS)
ret = DoReceive(ssh);
#else
/* Always service inbound data first so window updates can unblock sends. */
if (ret == WS_SUCCESS) {
ret = DoReceive(ssh);
}
/* If receive only wanted read or delivered channel data, still try to
* flush any pending outbound packets. */
if (ret == WS_SUCCESS || ret == WS_WANT_READ || ret == WS_CHAN_RXD
|| ret == WS_EOF) {
int sendRet = WS_SUCCESS;
/* Flush queued output whatever DoReceive() made of the socket, since an
* idle receive reports WS_FATAL_ERROR. !ssh->disconnected gates it. */
if (ssh != NULL && !ssh->disconnected && ssh->outputBuffer.length != 0) {
int rxErr = ssh->error;
if (ssh->outputBuffer.length != 0)
sendRet = wolfSSH_SendPacket(ssh);
/* If send is back-pressured, immediately try another receive to pick
* up potential window-adjusts and then return the send status. The
* send status wins; a peer EOF stays latched on the channel. */
if (sendRet == WS_WANT_WRITE || sendRet == WS_WINDOW_FULL) {
int recv2 = DoReceive(ssh);
if (recv2 == WS_SUCCESS || recv2 == WS_WANT_READ || recv2 == WS_CHAN_RXD
|| recv2 == WS_EOF)
sendRet = wolfSSH_SendPacket(ssh);
if (sendRet != WS_SUCCESS) {
if (ret == WS_SUCCESS) {
ret = sendRet;
else
ret = recv2;
}
else if ((ret == WS_CHANNEL_CLOSED && sendRet != WS_WANT_WRITE)
|| (ret == WS_FATAL_ERROR && rxErr != WS_WANT_READ)) {
/* A failed receive outranks the flush, and so does a close
* whose flush hard-failed: callers route teardown on it.
* Every other status keeps the code the send set. */
ssh->error = rxErr;
}
}
else {
/* Preserve meaningful receive status when send succeeded. */
if (sendRet != WS_SUCCESS)
ret = sendRet;
/* else leave ret as prior receive result (SUCCESS/WANT_READ/CHAN_RXD). */
}
}
#endif /* WOLFSSH_TEST_BLOCK */
/* DoChannelClose() bundles the reply inside DoReceive(), and callers
* treat the close as terminal, so flush it here. The close stays the
* return value; a short flush leaves WS_WANT_WRITE latched. */
if (ret == WS_CHANNEL_CLOSED && ssh->outputBuffer.length != 0) {
int closeErr = ssh->error;
if (wolfSSH_SendPacket(ssh) == WS_SUCCESS)
ssh->error = closeErr;
}
/* WS_EXTDATA and WS_EOF report the channel too, so a multi-channel caller
@ -4672,12 +4657,10 @@ int wolfSSH_worker(WOLFSSH* ssh, word32* channelId)
*channelId = ssh->lastRxId;
}
/* WS_EXTDATA and WS_EOF are raised once, on arrival; masking either
* strands the event, and the stderr window credit with it. 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 && ret != WS_EOF) {
/* Report the rekey, unless it would hide a once-only WS_EXTDATA
* or WS_EOF, or the error from a flush that failed. */
if (ssh->isKeying && ret != WS_EXTDATA && ret != WS_EOF
&& sendRet == WS_SUCCESS) {
ssh->error = WS_REKEYING;
return WS_REKEYING;
}
@ -5116,8 +5099,9 @@ static int _ChannelRead(WOLFSSH_CHANNEL* channel, byte* buf, word32 bufSz)
}
}
else {
/* SendPacket() records only WS_WANT_WRITE, so a hard failure has to
* be recorded here; rewriting WS_WANT_WRITE is deliberate. */
/* The adjust can fail before it reaches the transport, so the code
* is recorded here; the log skips a WS_WANT_WRITE, which only asks
* for a retry. */
ssh->error = updateResult;
if (updateResult != WS_WANT_WRITE) {
WLOG(WS_LOG_ERROR,
@ -5177,8 +5161,9 @@ static int _ChannelReadExt(WOLFSSH_CHANNEL* channel, byte* buf, word32 bufSz)
ssh->error = savedError;
}
else {
/* SendPacket() sets ssh->error only for WS_WANT_WRITE, so hard
* failures must be recorded here or they stay hidden. */
/* The adjust can fail before it reaches the transport, so the
* code is recorded here; the log skips a WS_WANT_WRITE, which
* only asks for a retry. */
ssh->error = adjustResult;
if (adjustResult != WS_WANT_WRITE) {
WLOG(WS_LOG_ERROR,

31
tests/regress.c
View File

@ -9851,11 +9851,19 @@ static void TestWorkerReportsDisconnect(void)
wolfSSH_SetIOReadCtx(ssh, &io);
wolfSSH_SetIOWriteCtx(ssh, &io);
/* Queued output, so the flush on this pass has something to push. */
ssh->outputBuffer.length = 1;
ssh->outputBuffer.idx = 0;
ssh->outputBuffer.buffer[0] = 0;
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 queued byte stays put: the session ended on this very pass. */
AssertIntEQ(io.outSz, 0);
AssertTrue(wolfSSH_OutputPending(ssh));
/* 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
@ -10533,11 +10541,9 @@ static void TestPasswordEofNoCrash(void)
ClientFreeBuffers();
}
/* When the send path is back-pressured (WANT_WRITE), wolfSSH_worker()
* still needs to service Receive() so window-adjusts can arrive and
* unblock the flow control. Verify the receive callback is invoked even
* when the first send attempt would block. */
#ifndef WOLFSSH_TEST_BLOCK
/* An idle receive still gets its flush. The receive runs first and reports
* want-read, and the queued byte is pushed anyway, so ssh->error carries the
* write the socket would not take. */
static int recvCallCount;
static int WantWriteSend(WOLFSSH* ssh, void* buf, word32 sz, void* ctx)
@ -10553,7 +10559,6 @@ static int WantReadRecv(WOLFSSH* ssh, void* buf, word32 sz, void* ctx)
return WS_CBIO_ERR_WANT_READ;
}
#ifndef WOLFSSH_TEST_BLOCK
static void TestWorkerReadsWhenSendWouldBlock(void)
{
WOLFSSH_CTX* ctx;
@ -10576,20 +10581,16 @@ static void TestWorkerReadsWhenSendWouldBlock(void)
recvCallCount = 0;
/* call worker; expect it to attempt send, notice back-pressure, and have
* invoked recv once. Depending on how DoReceive handles WANT_READ, the
* return may be WANT_WRITE or a fatal error; the important part is that
* recv was exercised. */
ret = wolfSSH_worker(ssh, NULL);
AssertTrue(ret == WS_WANT_WRITE || ret == WS_FATAL_ERROR);
AssertIntEQ(ret, WS_FATAL_ERROR);
AssertIntEQ(wolfSSH_get_error(ssh), WS_WANT_WRITE);
AssertIntEQ(recvCallCount, 1);
AssertTrue(wolfSSH_OutputPending(ssh));
wolfSSH_free(ssh);
wolfSSH_CTX_free(ctx);
}
#endif /* !WOLFSSH_TEST_BLOCK */
#endif
#ifdef WOLFSSH_SFTP
@ -14951,9 +14952,7 @@ int main(int argc, char** argv)
TestClientBuffersIdempotent();
#endif
TestPasswordEofNoCrash();
#ifndef WOLFSSH_TEST_BLOCK
TestWorkerReadsWhenSendWouldBlock();
#endif
#ifdef KEXDH_REPLY_REGRESS_KEX_ALGO
#ifndef WOLFSSH_NO_RSA_SHA2_256

1362
tests/unit.c
View File

File diff suppressed because it is too large Load Diff

45
wolfssh/ssh.h
View File

@ -82,22 +82,29 @@ WOLFSSH_API void wolfSSH_free(WOLFSSH* ssh);
* wolfSSH_ChannelIdReadExt()
* WS_EOF the peer half-closed a channel; it sends no more data,
* but the channel is still open for sending. Raised once,
* on arrival, and a back-pressure status from the flush
* that follows can supersede it, so an application that
* must not miss one tests wolfSSH_ChannelGetEof() or takes
* the channel EOF callback. Reply, if the protocol wants
* one, with wolfSSH_ChannelSendEof(); the library does
* not.
* on arrival, so an application that must not miss one
* tests wolfSSH_ChannelGetEof() or takes the channel EOF
* callback. Reply, if the protocol wants one, with
* wolfSSH_ChannelSendEof(); the library does not.
* WS_CHANNEL_CLOSED the peer closed a channel, which has been retired
* WS_WANT_READ / WS_WANT_WRITE / WS_REKEYING / WS_WINDOW_FULL
* WS_WANT_READ / WS_WANT_WRITE / WS_REKEYING
* transient; call again
* Anything else is an error: WS_BAD_ARGUMENT, or WS_FATAL_ERROR with the
* Take the event from the return, not from wolfSSH_get_error(). The return
* names what arrived; wolfSSH_get_error() names what the transport did. On
* any pass the two are independent: the return can carry an event while
* wolfSSH_get_error() carries an owed or failed write. Read both, and treat
* neither as a substitute for the other.
* 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.
* Any other code is an error: WS_BAD_ARGUMENT, or WS_FATAL_ERROR with the
* cause in wolfSSH_get_error() -- WS_DISCONNECT for the peer's disconnect,
* which is how most sessions end.
*
* For WS_CHAN_RXD, WS_EXTDATA, WS_EOF and WS_SUCCESS, 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.
* 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,
* WS_CHANNEL_CLOSED included; use wolfSSH_GetLastRxId() there.
*
* Note that after a peer half-close wolfSSH_stream_send() keeps working: the
* library latches only the EOF it sends, not the one it receives. */
@ -399,10 +406,9 @@ WOLFSSH_API int wolfSSH_ChannelSendExt(WOLFSSH_CHANNEL* channel,
* A WS_WANT_WRITE means the teardown is incomplete: the close is only built
* once the EOF is away, so call again until it reports something else. The
* retry costs nothing, a bundled EOF is not sent twice. WS_SUCCESS means both
* messages are bundled, not that they reached the peer: keep driving
* wolfSSH_worker() until it stops reporting WS_WANT_WRITE before dropping the
* socket. A channel whose open the peer has not confirmed has no peer id to
* address and reports WS_CHANNEL_NOT_CONF.
* messages are bundled, not that they reached the peer. A channel whose open
* the peer has not confirmed has no peer id to address and reports
* WS_CHANNEL_NOT_CONF.
*
* A peer that never answers leaves the channel on the list for the life of
* the session; there is no reclaim short of wolfSSH_free(). */
@ -417,9 +423,8 @@ WOLFSSH_API int wolfSSH_ChannelExit(WOLFSSH_CHANNEL* channel);
*
* The library never answers a received EOF with one of its own. It reports it
* as WS_EOF and through the channel EOF callback, and the application decides
* whether to reply, with this call or wolfSSH_stream_send_eof(). A
* back-pressure status can supersede the WS_EOF from wolfSSH_worker();
* wolfSSH_ChannelGetEof() is the durable check.
* whether to reply, with this call or wolfSSH_stream_send_eof().
* wolfSSH_ChannelGetEof() is the durable check for a received EOF.
* wolfSSH_ChannelExit() and wolfSSH_shutdown() send an EOF themselves while
* tearing the channel down.
*
@ -812,7 +817,7 @@ WOLFSSH_API int wolfSSH_shutdown(WOLFSSH* ssh);
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
* next send or a wolfSSH_worker() whose receive succeeded. Others failed. */
* next send or the next wolfSSH_worker(). Others failed. */
WOLFSSH_API int wolfSSH_stream_read(WOLFSSH* ssh, byte* buf, word32 bufSz);
WOLFSSH_API int wolfSSH_stream_send(WOLFSSH* ssh, byte* buf, word32 bufSz);
/* Half-closes the first channel in the list. See wolfSSH_ChannelSendEof().
@ -851,6 +856,8 @@ WOLFSSH_API int wolfSSH_extended_data_send(WOLFSSH* ssh, byte* buf, word32 bufSz
* (the peer's CHANNEL_CLOSE) is discarded with it. */
WOLFSSH_API int wolfSSH_extended_data_read(WOLFSSH* ssh, byte* out,
word32 outSz);
/* Starts a key exchange. A clean start leaves ssh->error alone; only a
* failure records its code there. */
WOLFSSH_API int wolfSSH_TriggerKeyExchange(WOLFSSH* ssh);
WOLFSSH_API int wolfSSH_SendIgnore(WOLFSSH* ssh, const byte* buf, word32 bufSz);
/* One disconnect ends the session, so a second call reports WS_DISCONNECT.