From 696339eabf38b507f9a02326b6306b5d32e9da5b Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 24 Jul 2026 14:10:24 -0700 Subject: [PATCH] Report whether a global request reached the peer A send's return code cannot tell a caller its request is on its way. The highwater callback runs after the last byte goes out, so a rekey's errors surface as the send's, and WS_WANT_WRITE leaves the packet framed for the next flush. - Count the flushes wolfSSH_SendPacket() completes. - Compare that count across a send to tell those outcomes apart. - SendGlobalRequest() and SendGlobalRequestFwd() carry the answer in an optional out-param. - Both callers pass NULL, so nothing acts on it yet. Issue: ZD-22195 --- src/internal.c | 50 ++++++++++++++++++++++++++++++++++++++++++---- src/ssh.c | 8 +++++--- wolfssh/internal.h | 10 ++++++++-- 3 files changed, 59 insertions(+), 9 deletions(-) diff --git a/src/internal.c b/src/internal.c index 5b385b1c..8df903a9 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4350,6 +4350,11 @@ int wolfSSH_SendPacket(WOLFSSH* ssh) * call a licence to push whatever gets queued next. */ ssh->disconnectTxd = 0; + /* Everything framed is on the wire. What runs below can fail, and the + * return code alone cannot tell a caller its packet was delivered, so + * record the flush first. */ + ssh->txFlushCount++; + WLOG(WS_LOG_DEBUG, "SB: Shrinking output buffer"); ShrinkBuffer(&ssh->outputBuffer, 0); return HighwaterCheck(ssh, WOLFSSH_HWSIDE_TRANSMIT); @@ -16995,13 +17000,34 @@ int SendIgnore(WOLFSSH* ssh, const unsigned char* data, word32 dataSz) return ret; } +/* Will the packet just framed reach the peer? A completed flush says so; the + * return does not, since the highwater callback runs after the last byte goes + * out and the rekey it starts fails with the same codes a lost send does. + * 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. */ +static INLINE int SendPacketDelivered(WOLFSSH* ssh, word32 flushes, int ret) +{ + return ssh->txFlushCount != flushes || ret == WS_WANT_WRITE; +} + + int SendGlobalRequest(WOLFSSH* ssh, - const unsigned char* data, word32 dataSz, int reply) + const unsigned char* data, word32 dataSz, int reply, int* sent) { byte* output; word32 idx = 0; int ret = WS_SUCCESS; + if (sent != NULL) + *sent = 0; + if (ssh == NULL || (data == NULL && dataSz > 0)) ret = WS_BAD_ARGUMENT; @@ -17029,9 +17055,15 @@ int SendGlobalRequest(WOLFSSH* ssh, ret = BundlePacket(ssh); } - if (ret == WS_SUCCESS) + if (ret == WS_SUCCESS) { + word32 flushes = ssh->txFlushCount; + ret = wolfSSH_SendPacket(ssh); + if (sent != NULL) + *sent = SendPacketDelivered(ssh, flushes, ret); + } + WLOG(WS_LOG_DEBUG, "Leaving SendGlobalRequest(), ret = %d", ret); return ret; @@ -17043,7 +17075,8 @@ int SendGlobalRequest(WOLFSSH* ssh, * address and port follow the want-reply boolean, an ordering the generic * SendGlobalRequest() framing cannot express. RFC 4254 7.1. */ int SendGlobalRequestFwd(WOLFSSH* ssh, - const char* bindAddr, word32 bindPort, int isCancel, int wantReply) + const char* bindAddr, word32 bindPort, int isCancel, int wantReply, + int* sent) { byte* output; word32 idx = 0; @@ -17054,6 +17087,9 @@ int SendGlobalRequestFwd(WOLFSSH* ssh, WLOG(WS_LOG_DEBUG, "Entering SendGlobalRequestFwd()"); + if (sent != NULL) + *sent = 0; + if (ssh == NULL || bindAddr == NULL) ret = WS_BAD_ARGUMENT; @@ -17088,9 +17124,15 @@ int SendGlobalRequestFwd(WOLFSSH* ssh, ret = BundlePacket(ssh); } - if (ret == WS_SUCCESS) + if (ret == WS_SUCCESS) { + word32 flushes = ssh->txFlushCount; + ret = wolfSSH_SendPacket(ssh); + if (sent != NULL) + *sent = SendPacketDelivered(ssh, flushes, ret); + } + WLOG(WS_LOG_DEBUG, "Leaving SendGlobalRequestFwd(), ret = %d", ret); return ret; diff --git a/src/ssh.c b/src/ssh.c index 9388c8d8..273e68f3 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1648,7 +1648,7 @@ int wolfSSH_global_request(WOLFSSH *ssh, const unsigned char* data, word32 dataS return WS_BAD_ARGUMENT; if (SendAfterDisconnect(ssh)) return WS_FATAL_ERROR; - return SendGlobalRequest(ssh, data, dataSz, reply); + return SendGlobalRequest(ssh, data, dataSz, reply, NULL); } @@ -3953,7 +3953,8 @@ int wolfSSH_FwdRemoteSetup(WOLFSSH* ssh, const char* bindAddr, ret = WS_REKEYING; if (ret == WS_SUCCESS) - ret = SendGlobalRequestFwd(ssh, bindAddr, bindPort, 0, wantReply); + ret = SendGlobalRequestFwd(ssh, bindAddr, bindPort, 0, wantReply, + NULL); WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_FwdRemoteSetup(), ret = %d", ret); return ret; @@ -3991,7 +3992,8 @@ int wolfSSH_FwdRemoteCancel(WOLFSSH* ssh, const char* bindAddr, ret = WS_REKEYING; if (ret == WS_SUCCESS) - ret = SendGlobalRequestFwd(ssh, bindAddr, bindPort, 1, wantReply); + ret = SendGlobalRequestFwd(ssh, bindAddr, bindPort, 1, wantReply, + NULL); WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_FwdRemoteCancel(), ret = %d", ret); return ret; diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 2627602e..6254cb69 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1024,6 +1024,7 @@ struct WOLFSSH { word32 rxCount; word32 txMsgCount; /* Packets sent under current keys */ word32 rxMsgCount; /* Packets received under current keys */ + word32 txFlushCount; /* Output buffer drained, whatever came after */ word32 highwaterMark; word32 msgHighwaterMark; /* Per-key packet limit (RFC 4344 Sec 3.1) */ byte highwaterFlag; /* Set when highwater CB called */ @@ -1559,11 +1560,16 @@ WOLFSSH_LOCAL int SendIgnore(WOLFSSH* ssh, const unsigned char* data, word32 dataSz); WOLFSSH_LOCAL int SendGlobalRequestFwdSuccess(WOLFSSH * ssh, int success, word32 port); +/* The optional sent out-param reports whether the request is on its way to the + * peer -- flushed, or still framed for the next flush -- which the return does + * not answer: the highwater callback runs after the last byte goes out, so its + * failure surfaces as this call's. */ WOLFSSH_LOCAL int SendGlobalRequest(WOLFSSH * ssh, - const unsigned char * data, word32 dataSz, int reply); + const unsigned char * data, word32 dataSz, int reply, int* sent); #ifdef WOLFSSH_FWD WOLFSSH_LOCAL int SendGlobalRequestFwd(WOLFSSH* ssh, - const char* bindAddr, word32 bindPort, int isCancel, int wantReply); + const char* bindAddr, word32 bindPort, int isCancel, int wantReply, + int* sent); #endif WOLFSSH_LOCAL int SendDebug(WOLFSSH* ssh, byte alwaysDisplay, const char* msg); WOLFSSH_LOCAL int SendServiceRequest(WOLFSSH* ssh, byte serviceId);