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

Fix interrupted and failed sends in SendPacket 

A signal was turned into a fatal error, and a refused send left its
packet counted in the output buffer. Both fixes are in
wolfSSH_SendPacket(), so they cover every sender.

- WS_CBIO_ERR_ISR fell through to WS_SOCKET_ERROR_E. Nothing went out
  and the session is unharmed, so retry, as ReceiveData() already does.
- Callers that discard a packet on error, like the KEX and userauth
  sends, were throwing away framed output the peer never refused.
- On WS_CBIO_ERR_GENERAL the buffer was shrunk with the packet still
  counted in plainSz, so SendChannelData() flushed nothing and called it
  a success. Clear it with the packet it described.
- Tests pin the retry from a forwarding sender and a plain global
  request, and drive a channel send through a would-block and a refused
  flush.
pull/1214/head
John Safranek 2026-08-06 11:53:35 -07:00 committed by philljj
parent e396a0a4b6
commit 8d3f20bfde
2 changed files with 111 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
src/internal.c
View File

@ -4896,6 +4896,14 @@ int wolfSSH_SendPacket(WOLFSSH* ssh)
ssh->error = WS_WANT_WRITE;
return WS_WANT_WRITE;
case WS_CBIO_ERR_ISR:
/* A signal interrupted the send. Nothing went out and the
* session is unharmed, so retry, as ReceiveData() does for
* the same condition. Reporting it instead loses framed
* output the peer never refused, since callers discard
* their packet on an error. */
continue;
case WS_CBIO_ERR_CONN_RST: /* connection reset */
ssh->connReset = 1;
break;
@ -4905,6 +4913,11 @@ int wolfSSH_SendPacket(WOLFSSH* ssh)
break;
case WS_CBIO_ERR_GENERAL:
/* plainSz counts plaintext the caller was told was
* accepted, so it goes with the packet being discarded.
* Left standing, it has SendChannelData() flush an empty
* buffer and call that a success. */
ssh->outputBuffer.plainSz = 0;
ShrinkBuffer(&ssh->outputBuffer, 1);
}
return WS_SOCKET_ERROR_E;
@ -17603,12 +17616,12 @@ int SendIgnore(WOLFSSH* ssh, const unsigned char* data, word32 dataSz)
* Comparing the flush count across the send tells those apart.
*
* Short of a flush, WS_WANT_WRITE is the one outcome that keeps the packet
* framed for the next one, and reading the buffer instead would call a packet
* delivered that a later purge or a discarding error path throws away.
* Anything else counts as not sent, which at worst leaves the peer holding a
* request this side did not register; guessing the other way would desync the
* reply queue for the life of the session. Call before anything else runs,
* since a later send flushes this packet and would read as this one's. */
* framed for the next one; an interrupted send is retried inside
* wolfSSH_SendPacket() rather than reported. Anything else counts as not sent,
* which at worst leaves the peer holding a request this side did not register;
* guessing the other way would desync the reply queue for the life of the
* session. Call before anything else runs, since a later send flushes this
* packet and would read as this one's. */
static INLINE int SendPacketDelivered(WOLFSSH* ssh, word32 flushes, int ret)
{
return ssh->txFlushCount != flushes || ret == WS_WANT_WRITE;

92
tests/regress.c
View File

@ -331,6 +331,7 @@ typedef struct {
word32 outSz;
word32 outCap;
byte blockNext; /* make the next send report a would-block */
byte isrNext; /* make the next send report an interrupted call */
} MemIo;
static int MemRecv(WOLFSSH* ssh, void* buf, word32 sz, void* ctx)
@ -355,6 +356,10 @@ static int MemSend(WOLFSSH* ssh, void* buf, word32 sz, void* ctx)
io->blockNext = 0;
return WS_CBIO_ERR_WANT_WRITE;
}
if (io->isrNext) {
io->isrNext = 0;
return WS_CBIO_ERR_ISR;
}
if (io->outSz + sz > io->outCap) {
return WS_CBIO_ERR_GENERAL;
}
@ -372,6 +377,7 @@ static void MemIoInit(MemIo* io, byte* in, word32 inSz, byte* out, word32 outCap
io->outSz = 0;
io->outCap = outCap;
io->blockNext = 0;
io->isrNext = 0;
}
/* The in-memory session harness. The struct and its teardown are shared; the
@ -2500,6 +2506,45 @@ static void TestServerServiceRequestRejectedDuringKeying(void)
}
/* A send the transport refuses discards the packet it had framed, and plainSz
* counted that packet's plaintext. Left standing over an emptied buffer, it
* has the next SendChannelData() flush nothing and call that a success. */
static void TestFailedSendClearsPendingPlaintext(void)
{
ChannelOpenHarness harness;
WOLFSSH_CHANNEL* channel;
byte payload[16];
InitChannelOpenHarness(&harness, NULL, 0);
WMEMSET(payload, 'a', sizeof(payload));
channel = ChannelNew(harness.ssh, ID_CHANTYPE_SESSION, 1024, 1024);
AssertNotNull(channel);
AssertIntEQ(ChannelUpdatePeer(channel, 0, 1024, 1024), WS_SUCCESS);
AssertIntEQ(ChannelAppend(harness.ssh, channel), WS_SUCCESS);
/* The transport blocks, so the packet stays framed and the caller is told
* its data was taken. */
harness.io.blockNext = 1;
AssertIntEQ(wolfSSH_stream_send(harness.ssh, payload, sizeof(payload)),
(int)sizeof(payload));
AssertIntEQ(harness.ssh->outputBuffer.plainSz, (int)sizeof(payload));
AssertIntEQ(wolfSSH_OutputPending(harness.ssh), 1);
/* The next call flushes that packet first, and this send fails outright,
* so what it was flushing is thrown away. */
harness.io.outSz = harness.io.outCap;
AssertIntEQ(wolfSSH_stream_send(harness.ssh, payload, sizeof(payload)),
WS_SOCKET_ERROR_E);
/* Nothing is framed any more, so nothing may still be counted as
* pending. */
AssertIntEQ(wolfSSH_OutputPending(harness.ssh), 0);
AssertIntEQ(harness.ssh->outputBuffer.plainSz, 0);
FreeChannelOpenHarness(&harness);
}
static void TestChannelOpenCallbackRejectSendsOpenFail(void)
{
ChannelOpenHarness harness;
@ -4676,6 +4721,50 @@ static void TestForwardedTcpipWantWriteStillRegisters(void)
FreeChannelOpenHarness(&harness);
}
/* A signal interrupts the send, which retries rather than reporting it, so the
* request goes out and the forward registers like any other. */
static void TestForwardedTcpipInterruptedSendStillRegisters(void)
{
ChannelOpenHarness harness;
InitFwdRemoteHarness(&harness);
harness.io.isrNext = 1;
AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 0),
WS_SUCCESS);
/* Retried and out, with nothing left framed and the session unharmed. */
AssertTrue(harness.io.outSz > 0);
AssertIntEQ(wolfSSH_OutputPending(harness.ssh), 0);
AssertIntEQ(harness.ssh->connReset, 0);
AssertIntEQ(harness.ssh->isClosed, 0);
harness.io.outSz = 0;
AssertForwardedOpenRefused(&harness, "10.0.0.1", 9999);
AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1);
FreeChannelOpenHarness(&harness);
}
/* The same retry on a sender unrelated to forwarding: it is in
* wolfSSH_SendPacket(), so it governs every send in the library. */
static void TestInterruptedSendRetriesForAnySender(void)
{
ChannelOpenHarness harness;
const byte req[] = "keepalive@openssh.com";
InitFwdRemoteHarness(&harness);
harness.io.isrNext = 1;
AssertIntEQ(wolfSSH_global_request(harness.ssh, req,
(word32)sizeof(req) - 1, 0), WS_SUCCESS);
AssertTrue(harness.io.outSz > 0);
AssertIntEQ(wolfSSH_OutputPending(harness.ssh), 0);
FreeChannelOpenHarness(&harness);
}
/* A request the application sends without want-reply is answered by nothing,
* so it must not take a place in the reply queue and eat the answer owed to an
* outstanding tcpip-forward. */
@ -10158,6 +10247,7 @@ int main(int argc, char** argv)
TestServerOnlyUserauthMsgsBlocked(serverSsh);
TestServerServiceRequestStateGated(serverSsh);
TestServerServiceRequestRejectedDuringKeying();
TestFailedSendClearsPendingPlaintext();
TestChannelOpenCallbackRejectSendsOpenFail();
TestSecondSessionChannelRejected();
TestUsernameChangeDisconnects();
@ -10232,6 +10322,8 @@ int main(int argc, char** argv)
TestForwardedTcpipReentrantCancelDuringSend();
TestForwardedTcpipAppRequestKeepsItsOwnReply();
TestForwardedTcpipWantWriteStillRegisters();
TestForwardedTcpipInterruptedSendStillRegisters();
TestInterruptedSendRetriesForAnySender();
TestGlobalRequestNoReplyQueuesNothing();
TestForwardedTcpipRepliesPairInSendOrder();
TestForwardedTcpipUnusablePortReplySendsOpenFail();