ssh: report the owed write when the worker leaves output queued

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;
         }
     }
 

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

wolfssh/ssh.h

@@ -97,17 +97,17 @@ 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.
  * To ask whether a write is still owed, call wolfSSH_OutputPending() rather
  * 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,