From 66f8a47a6aa3d118c17a38d68d3769b34e29b467 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 12 Mar 2020 13:21:04 -0700 Subject: [PATCH 1/3] Rx Window 1. Remove the CheckPendingReceive() function and the WIOCTL wrapper. 2. If the peer's receive window is considered full, call wolfSSH_worker() which will process a receive. --- .../cs+/common/wolfssh_csplus_usersettings.h | 2 - src/internal.c | 10 ++++ src/ssh.c | 35 ------------ src/wolfscp.c | 8 ++- src/wolfsftp.c | 13 ++--- wolfssh/error.h | 3 +- wolfssh/port.h | 54 ------------------- 7 files changed, 25 insertions(+), 100 deletions(-) diff --git a/ide/Renesas/cs+/common/wolfssh_csplus_usersettings.h b/ide/Renesas/cs+/common/wolfssh_csplus_usersettings.h index a2a23ba..03141d5 100644 --- a/ide/Renesas/cs+/common/wolfssh_csplus_usersettings.h +++ b/ide/Renesas/cs+/common/wolfssh_csplus_usersettings.h @@ -20,9 +20,7 @@ */ #define RENESAS_CSPLUS -#define WIOCTL ws_Ioctl //#define DEBUG_WOLFSSH #define WOLFSSH_NO_TIMESTAMP #define WOLFSSH_USER_IO -#define FIONREAD 1 #define WOLFSSH_THREAD diff --git a/src/internal.c b/src/internal.c index 1f0c628..55b21b5 100644 --- a/src/internal.c +++ b/src/internal.c @@ -287,6 +287,9 @@ const char* GetErrorString(int err) case WC_CHANGE_AUTH_E: return "changing auth type attempt"; + case WS_WINDOW_FULL: + return "peer's channel window full"; + default: return "Unknown error code"; } @@ -8180,6 +8183,13 @@ int SendChannelData(WOLFSSH* ssh, word32 peerChannel, } } + if (ret == WS_SUCCESS) { + if (channel->peerWindowSz == 0) { + WLOG(WS_LOG_DEBUG, "channel window is full"); + ret = WS_WINDOW_FULL; + } + } + if (ret == WS_SUCCESS) { word32 bound = min(channel->peerWindowSz, channel->peerMaxPacketSz); diff --git a/src/ssh.c b/src/ssh.c index 5aedc53..d97a59e 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1645,41 +1645,6 @@ int wolfSSH_ChannelGetEof(WOLFSSH_CHANNEL* channel) } -/* Protocols that have loops over wolfSSH_stream_send without doing any reads - * will run into the issue of not checking for a peer window adjustment packet. - * This function allows for checking for a peer window adjustment packet without - * requiring a read buffer and call to wolfSSH_stream_read. - * - * Data that is read and is not related to packet headers is stored in the - * WOLFSSH input buffer and can be gotten with a call to wolfSSH_stream_read. If - * more data is stored then MAX_PACKET_SZ then no more window adjustment packets - * are searched for. - */ -void wolfSSH_CheckReceivePending(WOLFSSH* ssh) -{ - int bytes = 0; - Buffer* inputBuffer; - - WLOG(WS_LOG_DEBUG, "Entering wolfSSH_CheckReceivePending()"); - - if (ssh == NULL) - return; - - inputBuffer = &ssh->channelList->inputBuffer; - WIOCTL(wolfSSH_get_fd(ssh), WFIONREAD, &bytes); - while (bytes > 0) { /* there is something to read off the wire */ - if (inputBuffer->length - inputBuffer->idx > MAX_PACKET_SZ) { - WLOG(WS_LOG_DEBUG, "Application data to be read"); - break; /* too much application data! */ - } - if (DoReceive(ssh) < 0) { - WLOG(WS_LOG_ERROR, "Error trying to read potential window adjust"); - } - WIOCTL(wolfSSH_get_fd(ssh), WFIONREAD, &bytes); - } -} - - #ifdef WOLFSSH_SHOW_SIZES void wolfSSH_ShowSizes(void) diff --git a/src/wolfscp.c b/src/wolfscp.c index d70ef52..f8d93d9 100644 --- a/src/wolfscp.c +++ b/src/wolfscp.c @@ -574,7 +574,11 @@ int DoScpSource(WOLFSSH* ssh) ret = wolfSSH_stream_send(ssh, ssh->scpFileBuffer, ssh->scpBufferedSz); - wolfSSH_CheckReceivePending(ssh); /*check for adjust window packet*/ + if (ret == WS_WINDOW_FULL) { + ret = wolfSSH_worker(ssh, NULL); + if (ret == WS_SUCCESS) + continue; + } if (ret < 0) { WLOG(WS_LOG_ERROR, scpError, "failed to send file", ret); break; @@ -1575,7 +1579,7 @@ static int wolfSSH_SCP_cmd(WOLFSSH* ssh, const char* localName, else { WLOG(WS_LOG_SCP, "Cannot build scp command"); ssh->error = WS_MEMORY_E; - ret = WS_FATAL_ERROR; + ret = WS_ERROR; } return ret; diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 27956a2..c876a9b 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -1205,7 +1205,8 @@ int wolfSSH_SFTP_read(WOLFSSH* ssh) state->sz - state->idx); if (ret < 0) { if (ssh->error != WS_WANT_READ && - ssh->error != WS_WANT_WRITE) + ssh->error != WS_WANT_WRITE && + ssh->error != WS_WINDOW_FULL) wolfSSH_SFTP_ClearState(ssh, STATE_ID_RECV); return WS_FATAL_ERROR; } @@ -1217,7 +1218,7 @@ int wolfSSH_SFTP_read(WOLFSSH* ssh) state->toSend = 1; if ((int)state->idx < state->sz) { - wolfSSH_CheckReceivePending(ssh); + ret = wolfSSH_worker(ssh, NULL); if (ssh->error == WS_WANT_READ) { /* was something there to read, try again */ state->toSend = 2; @@ -4607,7 +4608,8 @@ int SendPacketType(WOLFSSH* ssh, byte type, byte* buf, word32 bufSz) /* check for adjust window packet */ err = wolfSSH_get_error(ssh); - wolfSSH_CheckReceivePending(ssh); + if (err == WS_WINDOW_FULL) + ret = wolfSSH_worker(ssh, NULL); ssh->error = err; /* don't save potential want read here */ if (ret > 0) state->idx += (word32)ret; @@ -7672,7 +7674,8 @@ int wolfSSH_SFTP_Put(WOLFSSH* ssh, char* from, char* to, byte resume, state->r, state->rSz); if (sz <= 0) { if (ssh->error == WS_WANT_READ || - ssh->error == WS_WANT_WRITE) + ssh->error == WS_WANT_WRITE || + ssh->error == WS_WINDOW_FULL) return WS_FATAL_ERROR; } else { @@ -7686,8 +7689,6 @@ int wolfSSH_SFTP_Put(WOLFSSH* ssh, char* from, char* to, byte resume, statusCb(ssh, state->pOfst, from); } } - /* check for adjust window packet */ - wolfSSH_CheckReceivePending(ssh); } while (sz > 0 && ssh->sftpInt == 0); if (ssh->sftpInt) { diff --git a/wolfssh/error.h b/wolfssh/error.h index 35ada3d..7cb675f 100644 --- a/wolfssh/error.h +++ b/wolfssh/error.h @@ -111,8 +111,9 @@ enum WS_ErrorCodes { WS_SSH_CTX_NULL_E = -1070, /* SSH_CTX was null */ WS_CHANNEL_NOT_CONF = -1071, /* Channel open not confirmed. */ WC_CHANGE_AUTH_E = -1072, /* Changing auth type attempt */ + WS_WINDOW_FULL = -1073, - WS_LAST_E = -1072 /* Update this to indicate last error */ + WS_LAST_E = -1073 /* Update this to indicate last error */ }; diff --git a/wolfssh/port.h b/wolfssh/port.h index 8b4fc66..17f59f7 100644 --- a/wolfssh/port.h +++ b/wolfssh/port.h @@ -1078,60 +1078,6 @@ extern "C" { #define FALL_THROUGH #endif -/* used for checking bytes on wire for window adjust packet read */ -#ifndef WIOCTL -#ifdef WOLFSSL_NUCLEUS - #include "nucleus.h" - #include "networking/nu_networking.h" - #define WFIONREAD FIONREAD - static inline void ws_Ioctl(int fd, int flag, int* ret) - { - SCK_IOCTL_OPTION op; - op.s_optval = (unsigned char*)&fd; - if (NU_Ioctl(flag, &op, sizeof(op)) != NU_SUCCESS) { - *ret = 0; - } - else { - *ret = op.s_ret.sck_bytes_pending; - } - } - #define WIOCTL ws_Ioctl -#elif defined(FREESCALE_MQX) - /* MQX does not have FIONREAD, use SO_RCVNUM with getsockopt() instead */ - #include - #include - #define WFIONREAD SO_RCVNUM - static inline void ws_Ioctl(int fd, int flag, int* ret) - { - int status; - uint32_t bytesSz; - - bytesSz = sizeof(*ret); - status = getsockopt(fd, SOL_SOCKET, SO_RCVNUM, ret, &bytesSz); - if (status != RTCS_OK) { - WLOG(WS_LOG_ERROR, "Error calling getsockopt()"); - *ret = 0; - } - } - #define WIOCTL ws_Ioctl -#elif defined(USE_WINDOWS_API) - #define WFIONREAD FIONREAD - #define WIOCTL ioctlsocket -#elif defined(WOLFSSL_VXWORKS) - #include "ioLib.h" - #include - #define WIOCTL ioctl -#else - #if defined(__CYGWIN__) && !defined(FIONREAD) - /* Cygwin defines FIONREAD in socket.h instead of ioctl.h */ - #include - #endif - #include - #define WFIONREAD FIONREAD - #define WIOCTL ioctl -#endif -#endif /* WIOCTL */ - #if defined(USE_WINDOWS_API) #define WS_SOCKET_T SOCKET From cb277a92fc219bb553018f29951581932b8828a5 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 10 Mar 2020 14:58:35 -0700 Subject: [PATCH 2/3] Error Codes 1. Rename an error code that was prefixed with WC_ instead of WS_. 2. Add a new error code for missing callbacks. 3. Remove a redundant error code. Channel pending appeared twice. 4. Fix the error code vs string test. --- src/internal.c | 8 ++++---- tests/unit.c | 4 ++-- wolfssh/error.h | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/internal.c b/src/internal.c index 55b21b5..4eb23e6 100644 --- a/src/internal.c +++ b/src/internal.c @@ -245,9 +245,6 @@ const char* GetErrorString(int err) case WS_INVALID_EXTDATA: return "invalid extended data type"; - case WS_CHAN_PENDING: - return "channel open pending"; - case WS_SFTP_BAD_REQ_ID: return "sftp bad request id"; @@ -284,12 +281,15 @@ const char* GetErrorString(int err) case WS_CHANNEL_NOT_CONF: return "channel open not confirmed"; - case WC_CHANGE_AUTH_E: + case WS_CHANGE_AUTH_E: return "changing auth type attempt"; case WS_WINDOW_FULL: return "peer's channel window full"; + case WS_MISSING_CALLBACK: + return "missing a callback function"; + default: return "Unknown error code"; } diff --git a/tests/unit.c b/tests/unit.c index 7c65ad5..58cf177 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -420,13 +420,13 @@ static int test_Errors(void) #else int i, j = 0; /* Values that are not or no longer error codes. */ - int missing[] = { 0 }; + int missing[] = { -1059 }; int missingSz = (int)sizeof(missing)/sizeof(missing[0]); /* Check that all errors have a string and it's the same through the two * APIs. Check that the values that are not errors map to the unknown * string. */ - for (i = WS_FATAL_ERROR; i >= WS_LAST_E; i--) { + for (i = WS_ERROR; i >= WS_LAST_E; i--) { errStr = wolfSSH_ErrorToName(i); if (j < missingSz && i == missing[j]) { diff --git a/wolfssh/error.h b/wolfssh/error.h index 7cb675f..a01e1af 100644 --- a/wolfssh/error.h +++ b/wolfssh/error.h @@ -97,7 +97,6 @@ enum WS_ErrorCodes { WS_NEXT_ERROR = -1056, /* Getting next value/state is error */ WS_CHAN_RXD = -1057, /* Status that channel data received. */ WS_INVALID_EXTDATA = -1058, /* invalid Channel Extended Data Type */ - WS_CHAN_PENDING = -1059, /* peer hasn't confirmed channel open */ WS_SFTP_BAD_REQ_ID = -1060, /* SFTP Bad request ID */ WS_SFTP_BAD_REQ_TYPE = -1061, /* SFTP Bad request ID */ WS_SFTP_STATUS_NOT_OK = -1062, /* SFTP Status not OK */ @@ -110,10 +109,11 @@ enum WS_ErrorCodes { WS_SSH_NULL_E = -1069, /* SSH was null */ WS_SSH_CTX_NULL_E = -1070, /* SSH_CTX was null */ WS_CHANNEL_NOT_CONF = -1071, /* Channel open not confirmed. */ - WC_CHANGE_AUTH_E = -1072, /* Changing auth type attempt */ + WS_CHANGE_AUTH_E = -1072, /* Changing auth type attempt */ WS_WINDOW_FULL = -1073, + WS_MISSING_CALLBACK = -1074, /* Callback is missing */ - WS_LAST_E = -1073 /* Update this to indicate last error */ + WS_LAST_E = -1074 /* Update this to indicate last error */ }; From 6457cfac5317369a13c047549f56c55029e31528 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 16 Mar 2020 16:12:04 -0700 Subject: [PATCH 3/3] Delete a prototype for a function that has been removed. --- wolfssh/ssh.h | 1 - 1 file changed, 1 deletion(-) diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index ac437f1..c8e3948 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -218,7 +218,6 @@ typedef enum { WOLFSSH_API WS_SessionType wolfSSH_GetSessionType(const WOLFSSH*); WOLFSSH_API const char* wolfSSH_GetSessionCommand(const WOLFSSH*); WOLFSSH_API int wolfSSH_SetChannelType(WOLFSSH*, byte, byte*, word32); -WOLFSSH_API void wolfSSH_CheckReceivePending(WOLFSSH* ssh); enum WS_HighwaterSide {