Merge pull request #491 from ejohnstown/stall

SFTP Stall Fix

README.md

@@ -53,10 +53,10 @@ For building under Windows with Visual Studio, see the file
 
 NOTE: On resource constrained devices the `DEFAULT_WINDOW_SZ` may need
 to be set to a lower size. It can also be increased in desktop use cases
-to help with large file transfers. By default channels are set to handle
+to help with large file transfers. By default channels are set to receive
-16,384 bytes of data being sent and received. An example of setting a
+up to 128kB of data before sending a channel window adjust message. An
-window size for new channels would be as follows
+example of setting a window size for new channels would be as follows
-`./configure CPPFLAGS=-DDEFAULT_WINDOW_SZ=16384`
+`./configure CPPFLAGS="-DDEFAULT_WINDOW_SZ=16384"`
 
 For 32bit Linux platforms you can add support for files > 2GB by compling
 with `CFLAGS=-D_FILE_OFFSET_BITS=64`.

examples/sftpclient/sftpclient.c

@@ -1017,7 +1017,7 @@ static int doCmds(func_args* args)
                     ret = wolfSSH_get_error(ssh);
                 }
             } while (ret == WS_WANT_READ || ret == WS_WANT_WRITE ||
-                    ret == WS_CHAN_RXD);
+                    ret == WS_CHAN_RXD || ret == WS_REKEYING);
 
 #ifndef WOLFSSH_NO_TIMESTAMP
             WMEMSET(currentFile, 0, WOLFSSH_MAX_FILENAME);
@@ -1113,7 +1113,8 @@ static int doCmds(func_args* args)
                 ret = wolfSSH_SFTP_Put(ssh, pt, to, resume, &myStatusCb);
                 err = wolfSSH_get_error(ssh);
             } while ((err == WS_WANT_READ || err == WS_WANT_WRITE ||
-                        err == WS_CHAN_RXD) && ret == WS_FATAL_ERROR);
+                        err == WS_CHAN_RXD || err == WS_REKEYING) &&
+                    ret == WS_FATAL_ERROR);
 
 #ifndef WOLFSSH_NO_TIMESTAMP
             WMEMSET(currentFile, 0, WOLFSSH_MAX_FILENAME);

src/ssh.c

@@ -1007,48 +1007,7 @@ int wolfSSH_stream_peek(WOLFSSH* ssh, byte* buf, word32 bufSz)
 }
 
 
-/* moves the window for more room
+static int _UpdateChannelWindow(WOLFSSH_CHANNEL* channel);
- * returns WS_SUCCESS on success */
-static int wolfSSH_stream_adjust_window(WOLFSSH* ssh)
-{
-    word32  usedSz;
-    word32  bytesToAdd;
-    int     ret;
-    WOLFSSH_BUFFER* inputBuffer;
-
-    inputBuffer = &ssh->channelList->inputBuffer;
-    usedSz      = inputBuffer->length - inputBuffer->idx;
-    bytesToAdd  = inputBuffer->idx;
-
-    WLOG(WS_LOG_DEBUG, "Making more room: %u", usedSz);
-    WLOG(WS_LOG_DEBUG, "  Current index into buffer = %u", inputBuffer->idx);
-    WLOG(WS_LOG_DEBUG, "  Current max index for available data = %u",
-        inputBuffer->length);
-    WLOG(WS_LOG_DEBUG, "  Current total buffer size = %u",
-        inputBuffer->bufferSz);
-    if (usedSz) {
-        WLOG(WS_LOG_DEBUG, "  ...moving %d used bytes down", usedSz);
-        WMEMMOVE(inputBuffer->buffer, inputBuffer->buffer + bytesToAdd, usedSz);
-        inputBuffer->length = usedSz;
-        inputBuffer->idx = 0;
-    }
-
-    ret = SendChannelWindowAdjust(ssh, ssh->channelList->channel,
-            bytesToAdd);
-    if (ret != WS_SUCCESS && ret != WS_WANT_WRITE) {
-        WLOG(WS_LOG_ERROR, "Error adjusting window");
-    }
-    else {
-        WLOG(WS_LOG_INFO, "  bytesToAdd = %u", bytesToAdd);
-        WLOG(WS_LOG_INFO, "  windowSz = %u", ssh->channelList->windowSz);
-        ssh->channelList->windowSz += bytesToAdd;
-        WLOG(WS_LOG_INFO, "  update windowSz = %u", ssh->channelList->windowSz);
-        inputBuffer->length = usedSz;
-        inputBuffer->idx = 0;
-    }
-
-    return ret;
-}
 
 
 /* Wrapper function for ease of use to get data after it has been decrypted from
@@ -1072,18 +1031,14 @@ int wolfSSH_stream_read(WOLFSSH* ssh, byte* buf, word32 bufSz)
         return WS_BAD_ARGUMENT;
 
     inputBuffer = &ssh->channelList->inputBuffer;
-
+    ssh->error = WS_SUCCESS;
-    /* check if the window needs updated before attempting to read */
-    if ((ssh->channelList->windowSz == 0) || (!ssh->isKeying &&
-            (inputBuffer->length > inputBuffer->bufferSz / 2)))
-        ret = wolfSSH_stream_adjust_window(ssh);
-
 
     if (ret == WS_SUCCESS) {
         WLOG(WS_LOG_DEBUG, "    Stream read index of %u", inputBuffer->idx);
         WLOG(WS_LOG_DEBUG, "    Stream read ava data %u", inputBuffer->length);
         while (inputBuffer->length - inputBuffer->idx == 0) {
-            WLOG(WS_LOG_DEBUG, "Starting to recieve data at current index of %u",
+            WLOG(WS_LOG_DEBUG,
+                    "Starting to recieve data at current index of %u",
                     inputBuffer->idx);
             ret = DoReceive(ssh);
             if (ssh->channelList == NULL || ssh->channelList->eofRxd)
@@ -1105,12 +1060,18 @@ int wolfSSH_stream_read(WOLFSSH* ssh, byte* buf, word32 bufSz)
 
     /* update internal input buffer based on data read */
     if (ret == WS_SUCCESS) {
-        ret = min(bufSz, inputBuffer->length - inputBuffer->idx);
+        int n;
-        if (ret <= 0)
+
+        n = min(bufSz, inputBuffer->length - inputBuffer->idx);
+        if (n <= 0)
             ret = WS_BUFFER_E;
         else {
-            WMEMCPY(buf, inputBuffer->buffer + inputBuffer->idx, ret);
+            WMEMCPY(buf, inputBuffer->buffer + inputBuffer->idx, n);
-            inputBuffer->idx += ret;
+            ret = _UpdateChannelWindow(ssh->channelList);
+            if (ret == WS_SUCCESS) {
+                inputBuffer->idx += n;
+                ret = n;
+            }
         }
     }
 
@@ -2034,6 +1995,8 @@ int wolfSSH_ChannelGetFwdFd(const WOLFSSH_CHANNEL* channel)
 #endif /* WOLFSSH_FWD */
 
 
+/* moves the window for more room
+ * returns WS_SUCCESS on success */
 static int _UpdateChannelWindow(WOLFSSH_CHANNEL* channel)
 {
     WOLFSSH_BUFFER* inputBuffer;
@@ -2044,8 +2007,8 @@ static int _UpdateChannelWindow(WOLFSSH_CHANNEL* channel)
 
     inputBuffer = &channel->inputBuffer;
 
-    if (!channel->ssh->isKeying &&
+    if ((inputBuffer->length > inputBuffer->bufferSz / 2) ||
-        (inputBuffer->length > inputBuffer->bufferSz / 2)) {
+         (channel->windowSz == 0)) {
 
         word32 usedSz = inputBuffer->length - inputBuffer->idx;
         word32 bytesToAdd = inputBuffer->idx;
@@ -2055,8 +2018,6 @@ static int _UpdateChannelWindow(WOLFSSH_CHANNEL* channel)
             WLOG(WS_LOG_DEBUG, "  ...moving data down");
             WMEMMOVE(inputBuffer->buffer,
                      inputBuffer->buffer + bytesToAdd, usedSz);
-            inputBuffer->length = usedSz;
-            inputBuffer->idx = 0;
         }
 
         sendResult = SendChannelWindowAdjust(channel->ssh, channel->channel,

wolfssh/internal.h

@@ -341,10 +341,11 @@ enum {
     #define DEFAULT_HIGHWATER_MARK ((1024 * 1024 * 1024) - (32 * 1024))
 #endif
 #ifndef DEFAULT_WINDOW_SZ
-    #define DEFAULT_WINDOW_SZ 16384
+    #define DEFAULT_WINDOW_SZ (128 * 1024)
 #endif
 #ifndef DEFAULT_MAX_PACKET_SZ
-    #define DEFAULT_MAX_PACKET_SZ (16 * 1024)
+    /* This is from RFC 4253 section 6.1. */
+    #define DEFAULT_MAX_PACKET_SZ 32768
 #endif
 #ifndef DEFAULT_NEXT_CHANNEL
     #define DEFAULT_NEXT_CHANNEL 0