From f414e7721a7c8d44865d5e9f5918e6fcf54ebbc6 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 21 Aug 2018 14:09:31 -0700 Subject: [PATCH 1/3] Non-blocking fix 1. If data is still in the buffer, try sending it again. --- src/internal.c | 63 ++++++++++++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/src/internal.c b/src/internal.c index 22edd1d..ed5ffb4 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7051,42 +7051,45 @@ int SendChannelData(WOLFSSH* ssh, word32 peerChannel, ret = WS_REKEYING; } - if (ret == WS_SUCCESS) { - channel = ChannelFind(ssh, peerChannel, WS_CHANNEL_ID_PEER); - if (channel == NULL) { - WLOG(WS_LOG_DEBUG, "Invalid peer channel"); - ret = WS_INVALID_CHANID; - } - } - - if (ret == WS_SUCCESS) { - word32 bound = min(channel->peerWindowSz, channel->peerMaxPacketSz); - - if (dataSz > bound) { - WLOG(WS_LOG_DEBUG, - "Trying to send %u, client will only accept %u, limiting", - dataSz, bound); - dataSz = bound; + if (ssh->outputBuffer.length == 0) { + if (ret == WS_SUCCESS) { + channel = ChannelFind(ssh, peerChannel, WS_CHANNEL_ID_PEER); + if (channel == NULL) { + WLOG(WS_LOG_DEBUG, "Invalid peer channel"); + ret = WS_INVALID_CHANID; + } } - ret = PreparePacket(ssh, MSG_ID_SZ + UINT32_SZ + LENGTH_SZ + dataSz); - } + if (ret == WS_SUCCESS) { + word32 bound = min(channel->peerWindowSz, channel->peerMaxPacketSz); - if (ret == WS_SUCCESS) { - output = ssh->outputBuffer.buffer; - idx = ssh->outputBuffer.length; + if (dataSz > bound) { + WLOG(WS_LOG_DEBUG, + "Trying to send %u, client will only accept %u, limiting", + dataSz, bound); + dataSz = bound; + } - output[idx++] = MSGID_CHANNEL_DATA; - c32toa(channel->peerChannel, output + idx); - idx += UINT32_SZ; - c32toa(dataSz, output + idx); - idx += LENGTH_SZ; - WMEMCPY(output + idx, data, dataSz); - idx += dataSz; + ret = PreparePacket(ssh, + MSG_ID_SZ + UINT32_SZ + LENGTH_SZ + dataSz); + } - ssh->outputBuffer.length = idx; + if (ret == WS_SUCCESS) { + output = ssh->outputBuffer.buffer; + idx = ssh->outputBuffer.length; - ret = BundlePacket(ssh); + output[idx++] = MSGID_CHANNEL_DATA; + c32toa(channel->peerChannel, output + idx); + idx += UINT32_SZ; + c32toa(dataSz, output + idx); + idx += LENGTH_SZ; + WMEMCPY(output + idx, data, dataSz); + idx += dataSz; + + ssh->outputBuffer.length = idx; + + ret = BundlePacket(ssh); + } } if (ret == WS_SUCCESS) From d2a1c2ab1bc5e1141c2c460c0a3a250b59f25c1b Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 16 Nov 2018 13:55:56 -0800 Subject: [PATCH 2/3] Non-blocking fix 1. Added a non-blocking socket option to the client. 2. Added a non-blocking socket option to the server. 3. Added support for select to the test header. 4. Updated the usage strings so they are formatted the same. --- examples/client/client.c | 66 ++++++++++++++++++++++++++++++---- examples/server/server.c | 76 ++++++++++++++++++++++++++++++++++------ wolfssh/test.h | 55 +++++++++++++++++++++++++++++ 3 files changed, 180 insertions(+), 17 deletions(-) diff --git a/examples/client/client.c b/examples/client/client.c index 640a5fc..e99b4e5 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -104,6 +104,7 @@ static void ShowUsage(void) printf(" -P password for username, prompted if omitted\n"); printf(" -x exit after successful connection without doing\n" " read/write\n"); + printf(" -N use non-blocking sockets\n"); } @@ -150,6 +151,41 @@ static int wsUserAuth(byte authType, } +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); + sockfd = (SOCKET_T)wolfSSH_get_fd(ssh); + + while (ret != WS_SUCCESS && + (error == WS_WANT_READ || error == WS_WANT_WRITE)) + { + if (error == WS_WANT_READ) + printf("... client would read block\n"); + else if (error == WS_WANT_WRITE) + printf("... client would write block\n"); + + select_ret = tcp_select(sockfd, 1); + if (select_ret == WS_SELECT_RECV_READY || + select_ret == WS_SELECT_ERROR_READY) + { + ret = wolfSSH_connect(ssh); + } + else if (select_ret == WS_SELECT_TIMEOUT) + error = WS_WANT_READ; + else + error = WS_FATAL_ERROR; + } + + return ret; +} + + THREAD_RETURN WOLFSSH_THREAD client_test(void* args) { WOLFSSH_CTX* ctx = NULL; @@ -165,12 +201,13 @@ THREAD_RETURN WOLFSSH_THREAD client_test(void* args) const char* username = NULL; const char* password = NULL; byte imExit = 0; + byte nonBlock = 0; int argc = ((func_args*)args)->argc; char** argv = ((func_args*)args)->argv; ((func_args*)args)->return_code = 0; - while ((ch = mygetopt(argc, argv, "?h:p:u:P:x")) != -1) { + while ((ch = mygetopt(argc, argv, "?NP:h:p:u:x")) != -1) { switch (ch) { case 'h': host = myoptarg; @@ -197,6 +234,10 @@ THREAD_RETURN WOLFSSH_THREAD client_test(void* args) imExit = 1; break; + case 'N': + nonBlock = 1; + break; + case '?': ShowUsage(); exit(EXIT_SUCCESS); @@ -237,13 +278,21 @@ THREAD_RETURN WOLFSSH_THREAD client_test(void* args) if (ret != 0) err_sys("Couldn't connect to server."); + if (nonBlock) + tcp_set_nonblocking(&sockFd); + ret = wolfSSH_set_fd(ssh, (int)sockFd); if (ret != WS_SUCCESS) err_sys("Couldn't set the session's socket."); - ret = wolfSSH_connect(ssh); - if (ret != WS_SUCCESS) + if (!nonBlock) + ret = wolfSSH_connect(ssh); + else + ret = NonBlockSSH_connect(ssh); + if (ret != WS_SUCCESS) { + printf("err = %s\n", wolfSSH_get_error_name(ssh)); err_sys("Couldn't connect SSH stream."); + } if (!imExit) { ret = wolfSSH_stream_send(ssh, (byte*)testString, @@ -251,9 +300,14 @@ THREAD_RETURN WOLFSSH_THREAD client_test(void* args) if (ret <= 0) err_sys("Couldn't send test string."); - ret = wolfSSH_stream_read(ssh, (byte*)rxBuf, sizeof(rxBuf) - 1); - if (ret <= 0) - err_sys("Stream read failed."); + do { + ret = wolfSSH_stream_read(ssh, (byte*)rxBuf, sizeof(rxBuf) - 1); + if (ret <= 0) { + if (ret != WS_WANT_READ && ret != WS_WANT_WRITE) + err_sys("Stream read failed."); + } + } while (ret == WS_WANT_READ || ret == WS_WANT_WRITE); + rxBuf[ret] = '\0'; printf("Server said: %s\n", rxBuf); } diff --git a/examples/server/server.c b/examples/server/server.c index d07265c..33f6181 100644 --- a/examples/server/server.c +++ b/examples/server/server.c @@ -46,6 +46,7 @@ typedef struct { WOLFSSH* ssh; SOCKET_T fd; word32 id; + char nonBlock; } thread_ctx_t; @@ -97,12 +98,52 @@ static int dump_stats(thread_ctx_t* ctx) } +static int NonBlockSSH_accept(WOLFSSH* ssh) +{ + int ret; + int error; + SOCKET_T sockfd; + int select_ret = 0; + + ret = wolfSSH_accept(ssh); + error = wolfSSH_get_error(ssh); + sockfd = (SOCKET_T)wolfSSH_get_fd(ssh); + + while (ret != WS_SUCCESS && + (error == WS_WANT_READ || error == WS_WANT_WRITE)) + { + if (error == WS_WANT_READ) + printf("... client would read block\n"); + else if (error == WS_WANT_WRITE) + printf("... client would write block\n"); + + select_ret = tcp_select(sockfd, 1); + if (select_ret == WS_SELECT_RECV_READY || + select_ret == WS_SELECT_ERROR_READY) + { + ret = wolfSSH_accept(ssh); + } + else if (select_ret == WS_SELECT_TIMEOUT) + error = WS_WANT_READ; + else + error = WS_FATAL_ERROR; + } + + return ret; +} + + static THREAD_RETURN WOLFSSH_THREAD server_worker(void* vArgs) { int ret; thread_ctx_t* threadCtx = (thread_ctx_t*)vArgs; - if ((ret = wolfSSH_accept(threadCtx->ssh)) == WS_SUCCESS) { + if (!threadCtx->nonBlock) + ret = wolfSSH_accept(threadCtx->ssh); + else + ret = NonBlockSSH_accept(threadCtx->ssh); + + if (ret == WS_SUCCESS) { byte* buf = NULL; byte* tmpBuf; int bufSz, backlogSz = 0, rxSz, txSz, stop = 0, txSum; @@ -117,9 +158,12 @@ static THREAD_RETURN WOLFSSH_THREAD server_worker(void* vArgs) buf = tmpBuf; if (!stop) { - rxSz = wolfSSH_stream_read(threadCtx->ssh, - buf + backlogSz, - EXAMPLE_BUFFER_SZ); + do { + rxSz = wolfSSH_stream_read(threadCtx->ssh, + buf + backlogSz, + EXAMPLE_BUFFER_SZ); + } while (rxSz == WS_WANT_READ || rxSz == WS_WANT_WRITE); + if (rxSz > 0) { backlogSz += rxSz; txSum = 0; @@ -504,9 +548,10 @@ static int wsUserAuth(byte authType, static void ShowUsage(void) { printf("server %s\n", LIBWOLFSSH_VERSION_STRING); - printf("-h Help, print this usage\n"); - printf("-m Allow multiple connections\n"); - printf("-e Use ECC private key\n"); + printf(" -h display this help and exit\n"); + printf(" -m allow multiple connections\n"); + printf(" -e use ECC private key\n"); + printf(" -N use non-blocking sockets\n"); } @@ -517,16 +562,17 @@ THREAD_RETURN WOLFSSH_THREAD server_test(void* args) SOCKET_T listenFd = 0; word32 defaultHighwater = EXAMPLE_HIGHWATER_MARK; word32 threadCount = 0; - int multipleConnections = 0; - int useEcc = 0; - char ch; word16 port = wolfSshPort; + char multipleConnections = 0; + char useEcc = 0; + char ch; + char nonBlock = 0; int argc = ((func_args*)args)->argc; char** argv = ((func_args*)args)->argv; ((func_args*)args)->return_code = 0; - while ((ch = mygetopt(argc, argv, "hme")) != -1) { + while ((ch = mygetopt(argc, argv, "hmeN")) != -1) { switch (ch) { case 'h' : ShowUsage(); @@ -540,6 +586,10 @@ THREAD_RETURN WOLFSSH_THREAD server_test(void* args) useEcc = 1; break; + case 'N' : + nonBlock = 1; + break; + default: ShowUsage(); exit(MY_EX_USAGE); @@ -626,11 +676,15 @@ THREAD_RETURN WOLFSSH_THREAD server_test(void* args) if (clientFd == -1) err_sys("tcp accept failed"); + if (nonBlock) + tcp_set_nonblocking(&clientFd); + wolfSSH_set_fd(ssh, (int)clientFd); threadCtx->ssh = ssh; threadCtx->fd = clientFd; threadCtx->id = threadCount++; + threadCtx->nonBlock = nonBlock; #ifndef SINGLE_THREADED ThreadStart(server_worker, threadCtx, &thread); diff --git a/wolfssh/test.h b/wolfssh/test.h index c5c693c..a77623b 100644 --- a/wolfssh/test.h +++ b/wolfssh/test.h @@ -539,6 +539,61 @@ static INLINE void tcp_listen(SOCKET_T* sockfd, word16* port, int useAnyAddr) #endif /* WOLFSSH_TEST_SERVER */ +static INLINE void tcp_set_nonblocking(SOCKET_T* sockfd) +{ + #ifdef USE_WINDOWS_API + unsigned long blocking = 1; + int ret = ioctlsocket(*sockfd, FIONBIO, &blocking); + if (ret == SOCKET_ERROR) + err_sys("ioctlsocket failed"); + #elif defined(WOLFSSL_MDK_ARM) || defined(WOLFSSL_KEIL_TCP_NET) \ + || defined (WOLFSSL_TIRTOS)|| defined(WOLFSSL_VXWORKS) + /* non blocking not supported, for now */ + #else + int flags = fcntl(*sockfd, F_GETFL, 0); + if (flags < 0) + err_sys("fcntl get failed"); + flags = fcntl(*sockfd, F_SETFL, flags | O_NONBLOCK); + if (flags < 0) + err_sys("fcntl set failed"); + #endif +} + + +enum { + WS_SELECT_FAIL, + WS_SELECT_TIMEOUT, + WS_SELECT_RECV_READY, + WS_SELECT_ERROR_READY +}; + +static INLINE int tcp_select(SOCKET_T socketfd, int to_sec) +{ + fd_set recvfds, errfds; + SOCKET_T nfds = socketfd + 1; + struct timeval timeout = {(to_sec > 0) ? to_sec : 0, 0}; + int result; + + FD_ZERO(&recvfds); + FD_SET(socketfd, &recvfds); + FD_ZERO(&errfds); + FD_SET(socketfd, &errfds); + + result = select(nfds, &recvfds, NULL, &errfds, &timeout); + + if (result == 0) + return WS_SELECT_TIMEOUT; + else if (result > 0) { + if (FD_ISSET(socketfd, &recvfds)) + return WS_SELECT_RECV_READY; + else if(FD_ISSET(socketfd, &errfds)) + return WS_SELECT_ERROR_READY; + } + + return WS_SELECT_FAIL; +} + + /* Wolf Root Directory Helper */ /* KEIL-RL File System does not support relative directory */ #if !defined(WOLFSSL_MDK_ARM) && !defined(WOLFSSL_KEIL_FS) && !defined(WOLFSSL_TIRTOS) \ From dcd924577e8d2c3b13556f238951db768aa690e7 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 21 Nov 2018 11:42:27 -0800 Subject: [PATCH 3/3] Non-blocking fix When sending data, if there is pending data, try to send it first before setting up the next message. --- src/internal.c | 85 ++++++++++++++++++++++++++------------------------ 1 file changed, 45 insertions(+), 40 deletions(-) diff --git a/src/internal.c b/src/internal.c index ed5ffb4..3f29dc9 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7051,58 +7051,63 @@ int SendChannelData(WOLFSSH* ssh, word32 peerChannel, ret = WS_REKEYING; } - if (ssh->outputBuffer.length == 0) { - if (ret == WS_SUCCESS) { - channel = ChannelFind(ssh, peerChannel, WS_CHANNEL_ID_PEER); - if (channel == NULL) { - WLOG(WS_LOG_DEBUG, "Invalid peer channel"); - ret = WS_INVALID_CHANID; - } - } + if (ret == WS_SUCCESS) { + if (ssh->outputBuffer.length != 0) + ret = wolfSSH_SendPacket(ssh); + } - if (ret == WS_SUCCESS) { - word32 bound = min(channel->peerWindowSz, channel->peerMaxPacketSz); - - if (dataSz > bound) { - WLOG(WS_LOG_DEBUG, - "Trying to send %u, client will only accept %u, limiting", - dataSz, bound); - dataSz = bound; - } - - ret = PreparePacket(ssh, - MSG_ID_SZ + UINT32_SZ + LENGTH_SZ + dataSz); - } - - if (ret == WS_SUCCESS) { - output = ssh->outputBuffer.buffer; - idx = ssh->outputBuffer.length; - - output[idx++] = MSGID_CHANNEL_DATA; - c32toa(channel->peerChannel, output + idx); - idx += UINT32_SZ; - c32toa(dataSz, output + idx); - idx += LENGTH_SZ; - WMEMCPY(output + idx, data, dataSz); - idx += dataSz; - - ssh->outputBuffer.length = idx; - - ret = BundlePacket(ssh); + if (ret == WS_SUCCESS) { + channel = ChannelFind(ssh, peerChannel, WS_CHANNEL_ID_PEER); + if (channel == NULL) { + WLOG(WS_LOG_DEBUG, "Invalid peer channel"); + ret = WS_INVALID_CHANID; } } - if (ret == WS_SUCCESS) - ret = wolfSSH_SendPacket(ssh); + if (ret == WS_SUCCESS) { + word32 bound = min(channel->peerWindowSz, channel->peerMaxPacketSz); + + if (dataSz > bound) { + WLOG(WS_LOG_DEBUG, + "Trying to send %u, client will only accept %u, limiting", + dataSz, bound); + dataSz = bound; + } + + ret = PreparePacket(ssh, + MSG_ID_SZ + UINT32_SZ + LENGTH_SZ + dataSz); + } + + if (ret == WS_SUCCESS) { + output = ssh->outputBuffer.buffer; + idx = ssh->outputBuffer.length; + + output[idx++] = MSGID_CHANNEL_DATA; + c32toa(channel->peerChannel, output + idx); + idx += UINT32_SZ; + c32toa(dataSz, output + idx); + idx += LENGTH_SZ; + WMEMCPY(output + idx, data, dataSz); + idx += dataSz; + + ssh->outputBuffer.length = idx; + + ret = BundlePacket(ssh); + } if (ret == WS_SUCCESS) { WLOG(WS_LOG_INFO, " dataSz = %u", dataSz); WLOG(WS_LOG_INFO, " peerWindowSz = %u", channel->peerWindowSz); channel->peerWindowSz -= dataSz; WLOG(WS_LOG_INFO, " update peerWindowSz = %u", channel->peerWindowSz); - ret = dataSz; } + if (ret == WS_SUCCESS) + ret = wolfSSH_SendPacket(ssh); + + if (ret == WS_SUCCESS) + ret = dataSz; + WLOG(WS_LOG_DEBUG, "Leaving SendChannelData(), ret = %d", ret); return ret; }