ssh: flush the worker's queued output and report status directly

- wolfSSH_worker() calls wolfSSH_SendPacket() whenever
  ssh->outputBuffer holds bytes, in place of doing so only for
  WS_SUCCESS, WS_WANT_READ or WS_CHAN_RXD. It replaces DoReceive()'s
  WS_FATAL_ERROR with ssh->error when that holds a specific code,
  writes *channelId before the send, and takes the send status only
  for a failure other than WS_WANT_WRITE. Drops the second
  DoReceive(), its WS_WINDOW_FULL case, and the WOLFSSH_TEST_BLOCK
  fork.
- GetInputData() returns its condition in place of assigning
  ssh->error and returning WS_FATAL_ERROR; DoReceive() records it
  at both call sites.
- SendChannelData() and SendChannelExtendedData() capture the send
  status into sendRet and record it inside the existing
  ret == WS_SUCCESS block, which now also gates plainSz. Both set
  plainSz as well when the flush of already-queued output returns
  WS_WANT_WRITE.
- _ChannelRead(), _ChannelReadExt() and
  SendPendingChannelWindowAdjust() drop their savedError capture
  and restore, keeping the assignment on failure.
- wolfSSH_shutdown() reads ret in its teardown-send guards.
- FlushQueuedSend() loops while wolfSSH_get_error() reports
  WS_WANT_WRITE within its deadline, leaves the loop on a status
  outside the receive's own, and returns WS_WANT_WRITE when the
  queue is still out.
- echoserver, wolfsshd, sftpclient, client and scpclient gate on the
  worker's and shutdown's return values, and sftpclient and the
  wolfssh app drop 12 now-dead WS_FATAL_ERROR translations.
- server_worker() and HandleConnection() treat WS_CHANNEL_CLOSED
  from wolfSSH_shutdown() as a clean end alongside WS_SOCKET_ERROR_E;
  wolfsshd's drain loops on WS_SUCCESS, the wants, WS_CHAN_RXD,
  WS_EXTDATA and WS_REKEYING; echoserver's SFTP trigger and its
  end-of-connection report read wolfSSH_get_error().
- tests cover the worker's flush and the codes it now returns, and
  assert ssh->error carries the owed flush while one is queued.

Author: yosuke-wolfssl

apps/wolfssh/wolfssh.c

@@ -325,21 +325,26 @@ static int FlushQueuedSend(WOLFSSH* ssh, wolfSSL_Mutex* lock)
             wc_LockMutex(lock);
         }
         ret = wolfSSH_worker(ssh, NULL);
-        if (ret == WS_FATAL_ERROR) {
-            /* the session holds the detail behind a fatal error */
-            ret = wolfSSH_get_error(ssh);
-        }
         if (lock != NULL) {
             wc_UnLockMutex(lock);
         }
-    } while (ret == WS_WANT_WRITE && WTIME(NULL) < deadline);
-
-    /* The queue is out. Whatever the worker made of the peer's end of the
-     * conversation is for the reader to sort out. A rekey started on the way
-     * through is the reader's as well, the send itself went out. */
-    if (ret == WS_WANT_READ || ret == WS_CHAN_RXD || ret == WS_EXTDATA
-            || ret == WS_REKEYING) {
-        ret = WS_SUCCESS;
+
+        /* Anything outside the receive's own statuses is a failure. */
+        if (ret != WS_SUCCESS && ret != WS_WANT_READ && ret != WS_CHAN_RXD
+                && ret != WS_EXTDATA && ret != WS_REKEYING) {
+            break;
+        }
+    } while (wolfSSH_get_error(ssh) == WS_WANT_WRITE
+            && WTIME(NULL) < deadline);
+
+    /* Report only whether the queue went out. The deadline can run out
+     * with the packet still queued. */
+    if (ret == WS_SUCCESS || ret == WS_WANT_READ || ret == WS_CHAN_RXD
+            || ret == WS_EXTDATA || ret == WS_REKEYING) {
+        if (wolfSSH_get_error(ssh) == WS_WANT_WRITE)
+            ret = WS_WANT_WRITE;
+        else
+            ret = WS_SUCCESS;
     }
 
     return ret;
@@ -1353,9 +1358,6 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args)
 
         if (ret == WS_SUCCESS) {
             ret = wolfSSH_worker(ssh, NULL);
-            if (ret == WS_FATAL_ERROR) {
-                ret = wolfSSH_get_error(ssh);
-            }
             if (ret == WS_WANT_WRITE) {
                 /* The close messages are already out, whatever the drain
                  * still wants to send is a reply to the peer. */

apps/wolfsshd/wolfsshd.c

@@ -2618,20 +2618,17 @@ static void* HandleConnection(void* arg)
         wolfSSH_Log(WS_LOG_INFO, "[SSHD] Attempting to close down connection");
         ret = wolfSSH_shutdown(ssh);
 
-        /* peer hung up, stop shutdown */
-        if (ret == WS_SOCKET_ERROR_E) {
+        /* peer hung up or the channel is already gone, stop shutdown */
+        if (ret == WS_SOCKET_ERROR_E || ret == WS_CHANNEL_CLOSED) {
             ret = 0;
         }
 
-        error = wolfSSH_get_error(ssh);
-        if (error != WS_SOCKET_ERROR_E &&
-                (error == WS_WANT_READ || error == WS_WANT_WRITE)) {
+        if (ret == WS_WANT_READ || ret == WS_WANT_WRITE) {
             int maxAttempt = 10; /* make 10 attempts max before giving up */
             int attempt;
 
             for (attempt = 0; attempt < maxAttempt; attempt++) {
                 ret = wolfSSH_worker(ssh, NULL);
-                error = wolfSSH_get_error(ssh);
 
                 /* peer successfully closed down gracefully */
                 if (ret == WS_CHANNEL_CLOSED) {
@@ -2645,9 +2642,11 @@ static void* HandleConnection(void* arg)
                     break;
                 }
 
-                if (ret == WS_FATAL_ERROR &&
-                   (error != WS_WANT_READ &&
-                    error != WS_WANT_WRITE)) {
+                /* Keep draining while the socket blocks or the peer is still
+                 * talking. Anything else is a failure worth giving up on. */
+                if (ret != WS_SUCCESS && ret != WS_WANT_READ &&
+                        ret != WS_WANT_WRITE && ret != WS_CHAN_RXD &&
+                        ret != WS_EXTDATA && ret != WS_REKEYING) {
                     break;
                 }
             #ifdef _WIN32

examples/client/client.c

@@ -1209,7 +1209,8 @@ THREAD_RETURN WOLFSSH_THREAD client_test(void* args)
         }
         ret = wolfSSH_worker(ssh, NULL);
         if (ret != WS_SUCCESS && ret != WS_SOCKET_ERROR_E &&
-            ret != WS_CHANNEL_CLOSED) {
+            ret != WS_CHANNEL_CLOSED &&
+            ret != WS_WANT_READ && ret != WS_WANT_WRITE) {
             ClientFreeBuffers(pubKeyName, privKeyName, NULL);
             wolfSSH_free(ssh);
             wolfSSH_CTX_free(ctx);
@@ -1226,7 +1227,8 @@ THREAD_RETURN WOLFSSH_THREAD client_test(void* args)
     wolfSSH_free(ssh);
     wolfSSH_CTX_free(ctx);
     if (ret != WS_SUCCESS && ret != WS_SOCKET_ERROR_E &&
-            ret != WS_CHANNEL_CLOSED) {
+            ret != WS_CHANNEL_CLOSED &&
+            ret != WS_WANT_READ && ret != WS_WANT_WRITE) {
         err_sys("Closing client stream failed");
     }
 

examples/echoserver/echoserver.c

@@ -1383,7 +1383,7 @@ static int sftp_worker(thread_ctx_t* threadCtx)
     }
 
     do {
-        if (ret == WS_WANT_WRITE || ret == WS_CHAN_RXD ||
+        if (ret == WS_CHAN_RXD || error == WS_WANT_WRITE ||
                 wolfSSH_SFTP_PendingSend(ssh)) {
             /* Yes, process the SFTP data. */
             ret = wolfSSH_SFTP_read(ssh);
@@ -1639,14 +1639,12 @@ static THREAD_RETURN WOLFSSH_THREAD server_worker(void* vArgs)
     if (error != WS_SOCKET_ERROR_E && error != WS_FATAL_ERROR) {
         ret = wolfSSH_shutdown(threadCtx->ssh);
 
-        /* peer hung up, stop shutdown */
-        if (ret == WS_SOCKET_ERROR_E) {
+        /* peer hung up or the channel is already gone, stop shutdown */
+        if (ret == WS_SOCKET_ERROR_E || ret == WS_CHANNEL_CLOSED) {
             ret = 0;
         }
 
-        error = wolfSSH_get_error(threadCtx->ssh);
-        if (error != WS_SOCKET_ERROR_E &&
-                (error == WS_WANT_READ || error == WS_WANT_WRITE)) {
+        if (ret == WS_WANT_READ || ret == WS_WANT_WRITE) {
             int maxAttempt = 10; /* make 10 attempts max before giving up */
             int attempt;
 
@@ -1681,6 +1679,10 @@ static THREAD_RETURN WOLFSSH_THREAD server_worker(void* vArgs)
         }
     }
 
+    /* The report below names how the connection ended, and the shutdown
+     * drain refreshes error only on the paths that enter it. */
+    error = wolfSSH_get_error(threadCtx->ssh);
+
     if (threadCtx->fd != -1) {
         WCLOSESOCKET(threadCtx->fd);
         threadCtx->fd = -1;

examples/scpclient/scpclient.c

@@ -326,7 +326,8 @@ THREAD_RETURN WOLFSSH_THREAD scp_client(void* args)
         }
         else {
             ret = wolfSSH_worker(ssh, NULL);
-            if (ret != WS_SUCCESS && ret != WS_CHANNEL_CLOSED) {
+            if (ret != WS_SUCCESS && ret != WS_CHANNEL_CLOSED &&
+                    ret != WS_WANT_READ && ret != WS_WANT_WRITE) {
                 WLOG(WS_LOG_DEBUG,
                     "Failed to listen for close messages from the peer.");
             }

examples/sftpclient/sftpclient.c

@@ -661,9 +661,6 @@ static int doCmds(func_args* args)
             do {
                 while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
                     ret = wolfSSH_worker(ssh, NULL);
-                    if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
-                        ret = wolfSSH_get_error(ssh);
-                    }
                 }
 
                 ret = wolfSSH_SFTP_Get(ssh, pt, to, resume, &myStatusCb);
@@ -772,9 +769,6 @@ static int doCmds(func_args* args)
             do {
                 while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
                     ret = wolfSSH_worker(ssh, NULL);
-                    if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
-                        ret = wolfSSH_get_error(ssh);
-                    }
                 }
 
                 ret = wolfSSH_SFTP_Put(ssh, pt, to, resume, &myStatusCb);
@@ -866,9 +860,6 @@ static int doCmds(func_args* args)
             do {
                 while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
                     ret = wolfSSH_worker(ssh, NULL);
-                    if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
-                        ret = wolfSSH_get_error(ssh);
-                    }
                 }
 
                 ret = wolfSSH_SFTP_STAT(ssh, pt, &atrb);
@@ -920,9 +911,6 @@ static int doCmds(func_args* args)
             do {
                 while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
                     ret = wolfSSH_worker(ssh, NULL);
-                    if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
-                        ret = wolfSSH_get_error(ssh);
-                    }
                 }
 
                 ret = wolfSSH_SFTP_CHMOD(ssh, path, mode);
@@ -987,9 +975,6 @@ static int doCmds(func_args* args)
                 do {
                     while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
                         ret = wolfSSH_worker(ssh, NULL);
-                        if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
-                            ret = wolfSSH_get_error(ssh);
-                        }
                     }
                     ret = wolfSSH_SFTP_Open(ssh, path,
                             WOLFSSH_FXF_WRITE | WOLFSSH_FXF_CREAT |
@@ -1003,9 +988,6 @@ static int doCmds(func_args* args)
                 do {
                     while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
                         ret = wolfSSH_worker(ssh, NULL);
-                        if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
-                            ret = wolfSSH_get_error(ssh);
-                        }
                     }
                     ret = wolfSSH_SFTP_Close(ssh, handle, handleSz);
                     err = wolfSSH_get_error(ssh);
@@ -1060,9 +1042,6 @@ static int doCmds(func_args* args)
             do {
                 while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
                     ret = wolfSSH_worker(ssh, NULL);
-                    if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
-                        ret = wolfSSH_get_error(ssh);
-                    }
                 }
 
                 ret = wolfSSH_SFTP_RMDIR(ssh, pt);
@@ -1117,9 +1096,6 @@ static int doCmds(func_args* args)
             do {
                 while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
                     ret = wolfSSH_worker(ssh, NULL);
-                    if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
-                        ret = wolfSSH_get_error(ssh);
-                    }
                 }
 
                 ret = wolfSSH_SFTP_Remove(ssh, pt);
@@ -1210,9 +1186,6 @@ static int doCmds(func_args* args)
             do {
                 while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
                     ret = wolfSSH_worker(ssh, NULL);
-                    if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
-                        ret = wolfSSH_get_error(ssh);
-                    }
                 }
 
                 ret = wolfSSH_SFTP_Rename(ssh, pt, to);
@@ -1348,9 +1321,6 @@ static int doCmds(func_args* args)
             do {
                 while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
                     ret = wolfSSH_worker(ssh, NULL);
-                    if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
-                        ret = wolfSSH_get_error(ssh);
-                    }
                 }
 
                 current = wolfSSH_SFTP_LS(ssh, workingDir);
@@ -1821,9 +1791,7 @@ THREAD_RETURN WOLFSSH_THREAD sftpclient_test(void* args)
             ret = 0;
         }
 
-        err = wolfSSH_get_error(ssh);
-        if (err != WS_SOCKET_ERROR_E &&
-                (err == WS_WANT_READ || err == WS_WANT_WRITE)) {
+        if (ret == WS_WANT_READ || ret == WS_WANT_WRITE) {
             int maxAttempt = 10; /* make 10 attempts max before giving up */
             int attempt;