Address Coverity findings in wolfscp, wolfsftp, wolfssh, and wolfsshd

apps/wolfssh/wolfssh.c

@@ -95,9 +95,16 @@ static void ShowUsage(char* appPath)
 
     printf("%s v%s linked with wolfSSL %s\n", appName,
         LIBWOLFSSH_VERSION_STRING, LIBWOLFSSL_VERSION_STRING);
-    printf("usage: %s [-E logfile] [-G] [-l login_name] [-N] [-p port] "
+    printf("usage: %s "
+#ifdef WOLFSSH_AGENT
+            "[-a] "
+#endif
+            "[-E logfile] [-G] [-l login_name] [-N] [-p port] "
             "[-V] destination\n",
             appName);
+#ifdef WOLFSSH_AGENT
+    printf("  -a  attempt to use SSH-AGENT\n");
+#endif
 }
 
 
@@ -112,7 +119,6 @@ static int NonBlockSSH_connect(WOLFSSH* ssh)
     int ret;
     int error;
     SOCKET_T sockfd;
-    int select_ret = 0;
 
     ret = wolfSSH_connect(ssh);
     error = wolfSSH_get_error(ssh);
@@ -121,23 +127,13 @@ static int NonBlockSSH_connect(WOLFSSH* ssh)
     while (ret != WS_SUCCESS &&
             (error == WS_WANT_READ || error == WS_WANT_WRITE))
     {
-        select_ret = tcp_select(sockfd, 1);
-
-        /* Continue in want write cases even if did not select on socket
-         * because there could be pending data to be written. Added continue
-         * on want write for test cases where a forced want read was introduced
-         * and the socket will not be receiving more data. */
-        if (error == WS_WANT_WRITE || error == WS_WANT_READ ||
-            select_ret == WS_SELECT_RECV_READY ||
-            select_ret == WS_SELECT_ERROR_READY)
-        {
-            ret = wolfSSH_connect(ssh);
-            error = wolfSSH_get_error(ssh);
-        }
-        else if (select_ret == WS_SELECT_TIMEOUT)
-            error = WS_WANT_READ;
-        else
-            error = WS_FATAL_ERROR;
+        /* tcp_select only throttles the loop, always retry. On want write
+         * there may be pending data to send, and on want read a test case
+         * may have forced the want read with no more data coming in. */
+        (void)tcp_select(sockfd, 1);
+
+        ret = wolfSSH_connect(ssh);
+        error = wolfSSH_get_error(ssh);
     }
 
     return ret;
@@ -395,6 +391,7 @@ static THREAD_RET readInput(void* in)
     int  bufSz = sizeof(buf);
     thread_args* args = (thread_args*)in;
     int ret = 0;
+    int err = 0;
     word32 sz = 0;
 #ifdef USE_WINDOWS_API
     HANDLE stdinHandle = GetStdHandle(STD_INPUT_HANDLE);
@@ -413,15 +410,28 @@ static THREAD_RET readInput(void* in)
     #endif
         if (ret <= 0) {
             fprintf(stderr, "Error reading stdin\n");
-            return THREAD_RET_SUCCESS;
+            break;
         }
-        /* lock SSH structure access */
-        wc_LockMutex(&args->lock);
-        ret = wolfSSH_stream_send(args->ssh, buf, sz);
-        wc_UnLockMutex(&args->lock);
+        do {
+            /* lock SSH structure access */
+            wc_LockMutex(&args->lock);
+            ret = wolfSSH_stream_send(args->ssh, buf, sz);
+            err = (ret == WS_FATAL_ERROR) ?
+                wolfSSH_get_error(args->ssh) : ret;
+            wc_UnLockMutex(&args->lock);
+            if (err == WS_REKEYING) {
+                /* give readPeer() the lock to finish the rekey, then
+                 * send this buffer again */
+            #ifdef USE_WINDOWS_API
+                Sleep(1);
+            #else
+                usleep(1000);
+            #endif
+            }
+        } while (err == WS_REKEYING);
         if (ret <= 0) {
             fprintf(stderr, "Couldn't send data\n");
-            return THREAD_RET_SUCCESS;
+            break;
         }
     }
 #if !defined(WOLFSSH_NO_ECC) && defined(FP_ECC) && defined(HAVE_THREAD_LS)
@@ -726,6 +736,7 @@ struct config {
     char* command;
     word32 printConfig:1;
     word32 noCommand:1;
+    word32 useAgent:1;
     word16 port;
 };
 
@@ -783,8 +794,18 @@ static int config_parse_command_line(struct config* config,
 {
     int ch;
 
-    while ((ch = mygetopt(argc, argv, "E:Gl:Np:V")) != -1) {
+    while ((ch = mygetopt(argc, argv,
+#ifdef WOLFSSH_AGENT
+                "a"
+#endif
+                "E:Gl:Np:V")) != -1) {
         switch (ch) {
+        #ifdef WOLFSSH_AGENT
+            case 'a':
+                config->useAgent = 1;
+                break;
+        #endif
+
             case 'E':
                 config->logFile = myoptarg;
                 break;
@@ -887,6 +908,9 @@ static int config_print(struct config* config)
         printf("pubKeyFile %s\n",
                 config->pubKeyFile ? config->pubKeyFile : "none");
         printf("noCommand %s\n", config->noCommand ? "true" : "false");
+    #ifdef WOLFSSH_AGENT
+        printf("useAgent %s\n", config->useAgent ? "true" : "false");
+    #endif
         printf("logfile %s\n", config->logFile ? config->logFile : "default");
         printf("command %s\n", config->command ? config->command : "none");
     }
@@ -950,6 +974,10 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args)
             ((func_args*)args)->argc, ((func_args*)args)->argv);
     config_print(&config);
 
+#ifdef WOLFSSH_AGENT
+    useAgent = (byte)config.useAgent;
+#endif
+
     if (config.user == NULL)
         err_sys("client requires a username parameter.");
 

apps/wolfsshd/auth.c

@@ -2088,20 +2088,22 @@ static int RequestAuthentication(WS_UserAuthData* authData,
         authData->type == WOLFSSH_USERAUTH_PUBLICKEY) {
         /* compare user name to UPN in certificate */
         if (authData->sf.publicKey.isCert) {
-            DecodedCert* dCert;
         #ifdef WOLFSSH_SMALL_STACK
+            DecodedCert* dCert;
+
             dCert = (DecodedCert*)WMALLOC(sizeof(DecodedCert), NULL,
                 DYNTYPE_CERT);
-        #else
-            DecodedCert sdCert;
-            dCert = &sdCert;
-        #endif
-
             if (dCert == NULL) {
                 wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error creating cert struct");
                 ret = WOLFSSH_USERAUTH_INVALID_PUBLICKEY;
             }
-            else {
+        #else
+            DecodedCert sdCert;
+            DecodedCert* dCert = &sdCert;
+        #endif
+
+            /* ret is still success unless the allocation above failed */
+            if (ret == WOLFSSH_USERAUTH_SUCCESS) {
                 wc_InitDecodedCert(dCert, authData->sf.publicKey.publicKey,
                         authData->sf.publicKey.publicKeySz, NULL);
                 if (wc_ParseCert(dCert, CERT_TYPE, NO_VERIFY, NULL) != 0) {

apps/wolfsshd/wolfsshd.c

@@ -1388,6 +1388,108 @@ static int SHELL_IsPty(WOLFSSH* ssh)
     return ret;
 }
 
+/* set a descriptor to non blocking, returns 0 on success and -1 on failure */
+static int SHELL_SetNonBlocking(int fd)
+{
+    int flags;
+
+    flags = fcntl(fd, F_GETFL, 0);
+    if (flags < 0) {
+        wolfSSH_Log(WS_LOG_ERROR, "[SSHD] fcntl get failed");
+        return -1;
+    }
+
+    if (fcntl(fd, F_SETFL, flags | O_NONBLOCK) < 0) {
+        wolfSSH_Log(WS_LOG_ERROR, "[SSHD] fcntl set failed");
+        return -1;
+    }
+
+    return 0;
+}
+
+
+#ifndef WOLFSSHD_SHELL_FLUSH_TRIES
+    #define WOLFSSHD_SHELL_FLUSH_TRIES 10
+#endif
+#ifndef WOLFSSHD_SHELL_FLUSH_WAIT_US
+    #define WOLFSSHD_SHELL_FLUSH_WAIT_US 50000
+#endif
+
+/* Send out the last of a shell's output once the child has been reaped. The
+ * SSH socket is non blocking, so retry a bounded number of times on a full
+ * window, a rekey or a would block. 'ext' selects the extended (stderr) data
+ * stream. Returns 0 on success and -1 when the data could not all be sent. */
+static int SHELL_FlushOut(WOLFSSH* ssh, WS_SOCKET_T sshFd, word32 channelId,
+    byte* buf, int sz, int ext)
+{
+    int tries = 0;
+
+    while (sz > 0 && tries < WOLFSSHD_SHELL_FLUSH_TRIES) {
+        int cnt_w;
+
+        if (ext) {
+            cnt_w = wolfSSH_extended_data_send(ssh, buf, sz);
+        }
+        else {
+            cnt_w = wolfSSH_ChannelIdSend(ssh, channelId, buf, sz);
+        }
+
+        if (cnt_w == WS_WINDOW_FULL || cnt_w == WS_REKEYING ||
+                cnt_w == WS_WANT_WRITE) {
+            fd_set fds;
+            struct timeval to;
+
+            FD_ZERO(&fds);
+            FD_SET(sshFd, &fds);
+            to.tv_sec  = 0;
+            to.tv_usec = WOLFSSHD_SHELL_FLUSH_WAIT_US;
+            if (cnt_w == WS_WANT_WRITE) {
+                select((int)sshFd + 1, NULL, &fds, NULL, &to);
+            }
+            else {
+                /* waiting on the peer's window adjust or kex packets */
+                select((int)sshFd + 1, &fds, NULL, NULL, &to);
+            }
+
+            /* process what came in, otherwise the window never opens and
+             * the rekey never finishes, and the send can not progress */
+            if (wolfSSH_worker(ssh, NULL) < 0) {
+                int err = wolfSSH_get_error(ssh);
+
+                if (err != WS_WANT_READ && err != WS_WANT_WRITE &&
+                        err != WS_CHAN_RXD && err != WS_REKEYING) {
+                    wolfSSH_Log(WS_LOG_ERROR,
+                        "[SSHD] Issue draining connection on final flush");
+                    return -1;
+                }
+            }
+            tries++;
+            continue;
+        }
+
+        if (cnt_w < 0) {
+            wolfSSH_Log(WS_LOG_ERROR,
+                "[SSHD] Issue sending final shell output");
+            return -1;
+        }
+
+        sz -= cnt_w;
+        if (sz > 0) {
+            WMEMMOVE(buf, buf + cnt_w, sz);
+            tries++;
+        }
+    }
+
+    if (sz > 0) {
+        wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Unable to send all of the final "
+            "shell output, %d bytes dropped", sz);
+        return -1;
+    }
+
+    return 0;
+}
+
+
 /* handles creating a new shell env. and maintains SSH connection for incoming
  * user input as well as output of the shell.
  * return WS_SUCCESS on success */
@@ -1973,18 +2075,22 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
     if (!ptyReq || forcedCmd) {
         int readSz;
 
-        fcntl(stdoutPipe[0], F_SETFL, fcntl(stdoutPipe[0], F_GETFL)
-            | O_NONBLOCK);
-        readSz = (int)read(stdoutPipe[0], shellBuffer, sizeof shellBuffer);
-        if (readSz > 0) {
-            wolfSSH_ChannelIdSend(ssh, shellChannelId, shellBuffer, readSz);
+        /* when the pipe can not be made non blocking skip the drain, a
+         * blocking read here could hang the connection process */
+        if (SHELL_SetNonBlocking(stdoutPipe[0]) == 0) {
+            readSz = (int)read(stdoutPipe[0], shellBuffer, sizeof shellBuffer);
+            if (readSz > 0) {
+                SHELL_FlushOut(ssh, sshFd, shellChannelId, shellBuffer, readSz,
+                    0);
+            }
         }
 
-        fcntl(stderrPipe[0], F_SETFL, fcntl(stderrPipe[0], F_GETFL)
-            | O_NONBLOCK);
-        readSz = (int)read(stderrPipe[0], shellBuffer, sizeof shellBuffer);
-        if (readSz > 0) {
-            wolfSSH_extended_data_send(ssh, shellBuffer, readSz);
+        if (SHELL_SetNonBlocking(stderrPipe[0]) == 0) {
+            readSz = (int)read(stderrPipe[0], shellBuffer, sizeof shellBuffer);
+            if (readSz > 0) {
+                SHELL_FlushOut(ssh, sshFd, shellChannelId, shellBuffer, readSz,
+                    1);
+            }
         }
 
         close(stdoutPipe[0]);