From 73946a36c6aa09303c48a618016a109982ef1e0f Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 10 Apr 2023 15:57:25 -0700 Subject: [PATCH 1/6] Decoupling SFTP from SSH wolfSSH has a reach too deep into the wolfSSH internals. SFTP should be more like an application running on top of wolfSSH. We prefer to keep it all in one application, so it will work well in an embedded environment, but there needs more separation. 1. The sftp_worker loop in the echoserver should prioritize reading the socket and processing the SSH layer messages. Next it should ensure the transmit buffer for the SFTP channel is being written. Last it should check the receive buffer for the SFTP for new messages from the peer. wolfSSH_worker() will ensure data is read from the socket, and distributed to the appropriate channel receieve buffers, and will make sure the SSH bookkeeping is performed. 2. SendChannelData() should also bound the send data amount by the local maxPacketSz. 3. wolfSSH_SFTP_buffer_send() should send only one chunk of data, not looping until everything is gone. Need to send, but cannot flood the output. 4. Remove the call to wolfSSH_worker() from the buffer send. 5. The wolfSSH_SFTP_read() state machine should check the return status from the call to buffer send, and if it isn't an error, don't clear the state, allow for a want-write write again later. 6. Do not limit the file chunk size to send to WOLFSSH_MAX_SFTP_RW. This limit is provided during SFTP negotiation. If the peer requests that much data, send it. 6. Modify the select wrapper to have a very small microseconds value in addition to the requested seconds. --- examples/echoserver/echoserver.c | 116 +++++++++++++------------------ src/internal.c | 1 + src/wolfsftp.c | 30 ++++---- wolfssh/test.h | 2 +- 4 files changed, 68 insertions(+), 81 deletions(-) diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c index 459139b7..0d6ad62c 100644 --- a/examples/echoserver/echoserver.c +++ b/examples/echoserver/echoserver.c @@ -107,8 +107,6 @@ #ifndef NO_WOLFSSH_SERVER -#define TEST_SFTP_TIMEOUT 1 - static const char echoserverBanner[] = "wolfSSH Example Echo Server\n"; static int quit = 0; @@ -1133,84 +1131,70 @@ static int ssh_worker(thread_ctx_t* threadCtx) #ifdef WOLFSSH_SFTP + +#define TEST_SFTP_TIMEOUT_NONE 0 +#define TEST_SFTP_TIMEOUT 1 + /* handle SFTP operations * returns 0 on success */ static int sftp_worker(thread_ctx_t* threadCtx) { - byte tmp[1]; - int ret = WS_SUCCESS; - int error = WS_SUCCESS; - WS_SOCKET_T sockfd; - int select_ret = 0; + WOLFSSH* ssh = threadCtx->ssh; + WS_SOCKET_T s; + int ret = WS_SUCCESS; + int error; + int selected; + unsigned char peek_buf[1]; + int timeout = TEST_SFTP_TIMEOUT; + + s = (WS_SOCKET_T)wolfSSH_get_fd(ssh); - error = wolfSSH_get_error(threadCtx->ssh); - sockfd = (WS_SOCKET_T)wolfSSH_get_fd(threadCtx->ssh); do { - if (threadCtx->nonBlock) { - if (error == WS_WANT_READ) { - WOLFSSH_CHANNEL* c; - printf("... sftp server would read block\n"); - - /* if all channels are closed then close connection */ - c = wolfSSH_ChannelNext(threadCtx->ssh, NULL); - if (c && wolfSSH_ChannelGetEof(c)) { - ret = 0; - break; - } - } - else if (error == WS_WANT_WRITE) { - word32 c; - printf("... sftp server would write block\n"); - - /* handle backlog of send packets */ - wolfSSH_worker(threadCtx->ssh, &c); - ret = error = wolfSSH_get_error(threadCtx->ssh); - continue; - } + selected = tcp_select(s, timeout); + if (selected == WS_SELECT_ERROR_READY) { + break; } - /* if there is a current send in progress then continue to process it */ - if (wolfSSH_SFTP_PendingSend(threadCtx->ssh)) { - ret = wolfSSH_SFTP_read(threadCtx->ssh); - error = wolfSSH_get_error(threadCtx->ssh); - } - else { - if (wolfSSH_stream_peek(threadCtx->ssh, tmp, 1) > 0) { - select_ret = WS_SELECT_RECV_READY; + if (selected == WS_SELECT_RECV_READY) { + ret = wolfSSH_worker(ssh, NULL); + error = wolfSSH_get_error(ssh); + if (error == WS_EOF) { + break; } - else { - select_ret = tcp_select(sockfd, TEST_SFTP_TIMEOUT); - } - - if (select_ret == WS_SELECT_RECV_READY || - select_ret == WS_SELECT_ERROR_READY || - error == WS_WANT_WRITE) - { - ret = wolfSSH_SFTP_read(threadCtx->ssh); - error = wolfSSH_get_error(threadCtx->ssh); - } - else if (select_ret == WS_SELECT_TIMEOUT) - error = WS_WANT_READ; - else - error = WS_FATAL_ERROR; - } - - if (error == WS_WANT_READ || error == WS_WANT_WRITE || - error == WS_CHAN_RXD || error == WS_REKEYING || - error == WS_WINDOW_FULL) - ret = error; - - if (ret == WS_FATAL_ERROR && error == 0) { - WOLFSSH_CHANNEL* channel = - wolfSSH_ChannelNext(threadCtx->ssh, NULL); - if (channel && wolfSSH_ChannelGetEof(channel)) { - ret = 0; + if (ret != WS_SUCCESS && ret != WS_CHAN_RXD) { + /* If not successful and no channel data, leave. */ break; } } - } while (ret != WS_FATAL_ERROR && ret != WS_SOCKET_ERROR_E); + if (wolfSSH_SFTP_PendingSend(ssh)) { + /* Yes, process the SFTP data. */ + ret = wolfSSH_SFTP_read(ssh); + timeout = TEST_SFTP_TIMEOUT_NONE; + continue; + } + + ret = wolfSSH_stream_peek(ssh, peek_buf, sizeof(peek_buf)); + if (ret > 0) { + /* Yes, process the SFTP data. */ + ret = wolfSSH_SFTP_read(ssh); + timeout = TEST_SFTP_TIMEOUT_NONE; + continue; + } + + /* Old check for EOF here */ + { + WOLFSSH_CHANNEL* channel = + wolfSSH_ChannelNext(ssh, NULL); + if (channel && wolfSSH_ChannelGetEof(channel)) { + ret = WS_EOF; + break; + } + } + + timeout = TEST_SFTP_TIMEOUT; + } while (1); return ret; } diff --git a/src/internal.c b/src/internal.c index 15822c97..1a469702 100644 --- a/src/internal.c +++ b/src/internal.c @@ -11896,6 +11896,7 @@ int SendChannelData(WOLFSSH* ssh, word32 channelId, if (ret == WS_SUCCESS) { word32 bound = min(channel->peerWindowSz, channel->peerMaxPacketSz); + bound = min(bound, channel->maxPacketSz); if (dataSz > bound) { WLOG(WS_LOG_DEBUG, diff --git a/src/wolfsftp.c b/src/wolfsftp.c index d915a370..ddf4a38d 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -512,7 +512,7 @@ static int wolfSSH_SFTP_buffer_send(WOLFSSH* ssh, WS_SFTP_BUFFER* buffer) return WS_BUFFER_E; } - while (buffer->idx < buffer->sz && (ret > 0 || ret == WS_SUCCESS)) { + if (buffer->idx < buffer->sz) { ret = wolfSSH_stream_send(ssh, buffer->data + buffer->idx, buffer->sz - buffer->idx); if (ret > 0) { @@ -520,11 +520,6 @@ static int wolfSSH_SFTP_buffer_send(WOLFSSH* ssh, WS_SFTP_BUFFER* buffer) } WLOG(WS_LOG_SFTP, "SFTP buffer sent %d / %d bytes", buffer->idx, buffer->sz); - - /* interupt sending for a rekey or full window */ - if (ret == WS_WINDOW_FULL || ret == WS_REKEYING) { - ret = wolfSSH_worker(ssh, NULL); - } } return ret; @@ -1488,12 +1483,23 @@ int wolfSSH_SFTP_read(WOLFSSH* ssh) case STATE_RECV_SEND: if (state->toSend) { ret = wolfSSH_SFTP_buffer_send(ssh, &state->buffer); - if (ret == WS_SUCCESS || ret > 0) { - ret = WS_SUCCESS; - state->toSend = 0; - wolfSSH_SFTP_ClearState(ssh, STATE_ID_RECV); + if (ret < 0) { + if (ssh->error != WS_WANT_READ && + ssh->error != WS_WANT_WRITE && + ssh->error != WS_REKEYING && + ssh->error != WS_WINDOW_FULL) + wolfSSH_SFTP_ClearState(ssh, STATE_ID_RECV); + return WS_FATAL_ERROR; } + if (wolfSSH_SFTP_buffer_idx(&state->buffer) + < wolfSSH_SFTP_buffer_size(&state->buffer)) { + ssh->error = WS_WANT_WRITE; + return WS_FATAL_ERROR; + } + ret = WS_SUCCESS; + state->toSend = 0; } + wolfSSH_SFTP_ClearState(ssh, STATE_ID_RECV); return ret; default: @@ -3407,10 +3413,6 @@ int wolfSSH_SFTP_RecvRead(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) return WS_BUFFER_E; } - if (sz > WOLFSSH_MAX_SFTP_RW) { - sz = WOLFSSH_MAX_SFTP_RW; - } - /* read from handle and send data back to client */ out = (byte*)WMALLOC(sz + WOLFSSH_SFTP_HEADER + UINT32_SZ, ssh->ctx->heap, DYNTYPE_BUFFER); diff --git a/wolfssh/test.h b/wolfssh/test.h index 34d650e6..52062e9f 100644 --- a/wolfssh/test.h +++ b/wolfssh/test.h @@ -681,7 +681,7 @@ static INLINE int tcp_select(SOCKET_T socketfd, int to_sec) { WFD_SET_TYPE recvfds, errfds; int nfds = (int)socketfd + 1; - struct timeval timeout = {(to_sec > 0) ? to_sec : 0, 0}; + struct timeval timeout = {(to_sec > 0) ? to_sec : 0, 100}; int result; WFD_ZERO(&recvfds); From c97e8ea31913f2c5a4cc0b3812476a188101308a Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 11 Apr 2023 19:50:55 -0700 Subject: [PATCH 2/6] Rekey With the previous change for speed and timing, rekeying started having trouble in SFTP. Each file block travels as two messages, and the rekeying starts, but the messages are sent and lost. The client would terminate the connection. 1. Tweak the timeout in the echoserver's SFTP loop. 2. Better checking for rekeying. 3. Returning that rekeying is happening. --- examples/echoserver/echoserver.c | 15 +++++++++++++-- src/ssh.c | 14 +++++++++++++- src/wolfsftp.c | 7 +++++-- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c index 0d6ad62c..e33167b0 100644 --- a/examples/echoserver/echoserver.c +++ b/examples/echoserver/echoserver.c @@ -1159,6 +1159,11 @@ static int sftp_worker(thread_ctx_t* threadCtx) if (selected == WS_SELECT_RECV_READY) { ret = wolfSSH_worker(ssh, NULL); error = wolfSSH_get_error(ssh); + if (ret == WS_REKEYING) { + /* In a rekey, keeping turning the crank. */ + timeout = TEST_SFTP_TIMEOUT; + continue; + } if (error == WS_EOF) { break; } @@ -1171,7 +1176,8 @@ static int sftp_worker(thread_ctx_t* threadCtx) if (wolfSSH_SFTP_PendingSend(ssh)) { /* Yes, process the SFTP data. */ ret = wolfSSH_SFTP_read(ssh); - timeout = TEST_SFTP_TIMEOUT_NONE; + timeout = (ret == WS_REKEYING) ? + TEST_SFTP_TIMEOUT : TEST_SFTP_TIMEOUT_NONE; continue; } @@ -1179,7 +1185,12 @@ static int sftp_worker(thread_ctx_t* threadCtx) if (ret > 0) { /* Yes, process the SFTP data. */ ret = wolfSSH_SFTP_read(ssh); - timeout = TEST_SFTP_TIMEOUT_NONE; + timeout = (ret == WS_REKEYING) ? + TEST_SFTP_TIMEOUT : TEST_SFTP_TIMEOUT_NONE; + continue; + } + else if (ret == WS_REKEYING) { + timeout = TEST_SFTP_TIMEOUT; continue; } diff --git a/src/ssh.c b/src/ssh.c index e6674a1c..72f2dd07 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -998,6 +998,10 @@ int wolfSSH_stream_peek(WOLFSSH* ssh, byte* buf, word32 bufSz) if (ssh == NULL || ssh->channelList == NULL) return WS_BAD_ARGUMENT; + if (ssh->isKeying) { + return WS_REKEYING; + } + inputBuffer = &ssh->channelList->inputBuffer; bufSz = min(bufSz, inputBuffer->length - inputBuffer->idx); if (buf != NULL) { @@ -1089,6 +1093,10 @@ int wolfSSH_stream_send(WOLFSSH* ssh, byte* buf, word32 bufSz) if (ssh == NULL || buf == NULL || ssh->channelList == NULL) return WS_BAD_ARGUMENT; + if (ssh->isKeying) { + return WS_REKEYING; + } + bytesTxd = SendChannelData(ssh, ssh->channelList->channel, buf, bufSz); WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_stream_send(), txd = %d", bytesTxd); @@ -1771,8 +1779,12 @@ int wolfSSH_worker(WOLFSSH* ssh, word32* channelId) if (ret == WS_CHAN_RXD) WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_worker(), " "data received on channel %u", ssh->lastRxId); - else + else { + if (ret == WS_SUCCESS && ssh->isKeying) { + ret = WS_REKEYING; + } WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_worker(), ret = %d", ret); + } return ret; } diff --git a/src/wolfsftp.c b/src/wolfsftp.c index ddf4a38d..a28fba34 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -1484,11 +1484,14 @@ int wolfSSH_SFTP_read(WOLFSSH* ssh) if (state->toSend) { ret = wolfSSH_SFTP_buffer_send(ssh, &state->buffer); if (ret < 0) { + if (ret == WS_REKEYING || ssh->error == WS_REKEYING) { + return WS_REKEYING; + } if (ssh->error != WS_WANT_READ && ssh->error != WS_WANT_WRITE && - ssh->error != WS_REKEYING && - ssh->error != WS_WINDOW_FULL) + ssh->error != WS_WINDOW_FULL) { wolfSSH_SFTP_ClearState(ssh, STATE_ID_RECV); + } return WS_FATAL_ERROR; } if (wolfSSH_SFTP_buffer_idx(&state->buffer) From 7e7c5553df58e1b120e6e163b2ab71da4fbfcb74 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 13 Apr 2023 11:52:22 -0700 Subject: [PATCH 3/6] Update wolfSSHd 1. Update wolfSSHd with changes made for the echoserver. 2. Better indication of when the SFTP channel closes. --- apps/wolfsshd/wolfsshd.c | 78 ++++++++++++++++++++++++++++++---------- src/ssh.c | 10 ++++++ 2 files changed, 70 insertions(+), 18 deletions(-) diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index dc3363d5..6e5cd797 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -438,6 +438,7 @@ static int SetupChroot(WOLFSSHD_CONFIG* usrConf) #ifdef WOLFSSH_SFTP #define TEST_SFTP_TIMEOUT 1 +#define TEST_SFTP_TIMEOUT_NONE 0 /* handle SFTP operations * returns WS_SUCCESS on success @@ -445,11 +446,12 @@ static int SetupChroot(WOLFSSHD_CONFIG* usrConf) static int SFTP_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, WPASSWD* pPasswd, WOLFSSHD_CONFIG* usrConf) { - byte tmp[1]; int ret = WS_SUCCESS; int error = WS_SUCCESS; WS_SOCKET_T sockfd; int select_ret = 0; + int timeout = TEST_SFTP_TIMEOUT_NONE; + byte peek_buf[1]; /* temporarily elevate permissions to get users information */ if (wolfSSHD_AuthRaisePermissions(conn->auth) != WS_SUCCESS) { @@ -501,29 +503,67 @@ static int SFTP_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, sockfd = (WS_SOCKET_T)wolfSSH_get_fd(ssh); do { - if (wolfSSH_stream_peek(ssh, tmp, 1) > 0) { - select_ret = WS_SELECT_RECV_READY; - } - else { - select_ret = tcp_select(sockfd, TEST_SFTP_TIMEOUT); + select_ret = tcp_select(sockfd, timeout); + if (select_ret == WS_SELECT_ERROR_READY) { + break; } - if (select_ret == WS_SELECT_RECV_READY || - select_ret == WS_SELECT_ERROR_READY || - error == WS_WANT_WRITE) - { + if (select_ret == WS_SELECT_RECV_READY) { + ret = wolfSSH_worker(ssh, NULL); + error = wolfSSH_get_error(ssh); + if (ret == WS_REKEYING) { + /* In a rekey, keeping turning the crank. */ + timeout = TEST_SFTP_TIMEOUT; + continue; + } + if (error == WS_EOF) { + break; + } + if (ret != WS_SUCCESS && ret != WS_CHAN_RXD) { + /* If not successful and no channel data, leave. */ + break; + } + } + + if (wolfSSH_SFTP_PendingSend(ssh)) { + /* Yes, process the SFTP data. */ ret = wolfSSH_SFTP_read(ssh); error = wolfSSH_get_error(ssh); + timeout = (ret == WS_REKEYING) ? + TEST_SFTP_TIMEOUT : TEST_SFTP_TIMEOUT_NONE; + if (error == WS_WANT_READ || error == WS_WANT_WRITE || + error == WS_CHAN_RXD || error == WS_REKEYING || + error == WS_WINDOW_FULL) + ret = error; + if (error == WS_EOF) + break; + continue; } - else if (select_ret == WS_SELECT_TIMEOUT) - error = WS_WANT_READ; - else - error = WS_FATAL_ERROR; - if (error == WS_WANT_READ || error == WS_WANT_WRITE || - error == WS_CHAN_RXD || error == WS_REKEYING || - error == WS_WINDOW_FULL) - ret = error; + ret = wolfSSH_stream_peek(ssh, peek_buf, sizeof(peek_buf)); + if (ret > 0) { + /* Yes, process the SFTP data. */ + ret = wolfSSH_SFTP_read(ssh); + error = wolfSSH_get_error(ssh); + timeout = (ret == WS_REKEYING) ? + TEST_SFTP_TIMEOUT : TEST_SFTP_TIMEOUT_NONE; + if (error == WS_WANT_READ || error == WS_WANT_WRITE || + error == WS_CHAN_RXD || error == WS_REKEYING || + error == WS_WINDOW_FULL) + ret = error; + if (error == WS_EOF) + break; + continue; + } + else if (ret == WS_REKEYING) { + timeout = TEST_SFTP_TIMEOUT; + continue; + } + else if (ret < 0) { + error = wolfSSH_get_error(ssh); + if (error == WS_EOF) + break; + } if (ret == WS_FATAL_ERROR && error == 0) { WOLFSSH_CHANNEL* channel = @@ -533,6 +573,8 @@ static int SFTP_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, break; } } + + timeout = TEST_SFTP_TIMEOUT; } while (ret != WS_FATAL_ERROR); (void)conn; diff --git a/src/ssh.c b/src/ssh.c index 72f2dd07..17feac85 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -999,8 +999,13 @@ int wolfSSH_stream_peek(WOLFSSH* ssh, byte* buf, word32 bufSz) return WS_BAD_ARGUMENT; if (ssh->isKeying) { + ssh->error = WS_REKEYING; return WS_REKEYING; } + if (ssh->channelList->eofRxd) { + ssh->error = WS_EOF; + return WS_ERROR; + } inputBuffer = &ssh->channelList->inputBuffer; bufSz = min(bufSz, inputBuffer->length - inputBuffer->idx); @@ -1034,6 +1039,11 @@ int wolfSSH_stream_read(WOLFSSH* ssh, byte* buf, word32 bufSz) if (ssh == NULL || buf == NULL || bufSz == 0 || ssh->channelList == NULL) return WS_BAD_ARGUMENT; + if (ssh->channelList->eofRxd) { + ssh->error = WS_EOF; + return WS_ERROR; + } + inputBuffer = &ssh->channelList->inputBuffer; ssh->error = WS_SUCCESS; From 35554745bc07238662b5dd76e910ec0cbb50c105 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 13 Apr 2023 16:17:59 -0700 Subject: [PATCH 4/6] Update sftpclient 1. Update the example SFTP client with changes made for the echoserver. 2. Better handling of rekeying status. --- examples/sftpclient/sftpclient.c | 8 +++++++- src/internal.c | 3 ++- src/ssh.c | 8 +++++++- src/wolfsftp.c | 6 +++++- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/examples/sftpclient/sftpclient.c b/examples/sftpclient/sftpclient.c index 31292476..142f8d11 100644 --- a/examples/sftpclient/sftpclient.c +++ b/examples/sftpclient/sftpclient.c @@ -501,8 +501,14 @@ static int doCmds(func_args* args) if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) { ret = wolfSSH_get_error(ssh); } + 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); + } + } } while (ret == WS_WANT_READ || ret == WS_WANT_WRITE || - ret == WS_CHAN_RXD || ret == WS_REKEYING); + ret == WS_CHAN_RXD); #ifndef WOLFSSH_NO_TIMESTAMP WMEMSET(currentFile, 0, WOLFSSH_MAX_FILENAME); diff --git a/src/internal.c b/src/internal.c index 1a469702..8df184f1 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7411,7 +7411,8 @@ int DoReceive(WOLFSSH* ssh) ret = DoPacket(ssh); ssh->error = ret; if (ret < 0 && !(ret == WS_CHAN_RXD || ret == WS_EXTDATA || - ret == WS_CHANNEL_CLOSED || ret == WS_WANT_WRITE)) { + ret == WS_CHANNEL_CLOSED || ret == WS_WANT_WRITE || + ret == WS_REKEYING)) { return WS_FATAL_ERROR; } WLOG(WS_LOG_DEBUG, "PR3: peerMacSz = %u", peerMacSz); diff --git a/src/ssh.c b/src/ssh.c index 17feac85..55e13c17 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1104,6 +1104,7 @@ int wolfSSH_stream_send(WOLFSSH* ssh, byte* buf, word32 bufSz) return WS_BAD_ARGUMENT; if (ssh->isKeying) { + ssh->error = WS_REKEYING; return WS_REKEYING; } @@ -1786,11 +1787,16 @@ int wolfSSH_worker(WOLFSSH* ssh, word32* channelId) *channelId = ssh->lastRxId; } + if (ssh->isKeying) { + ssh->error = WS_REKEYING; + return WS_REKEYING; + } + if (ret == WS_CHAN_RXD) WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_worker(), " "data received on channel %u", ssh->lastRxId); else { - if (ret == WS_SUCCESS && ssh->isKeying) { + if (ret != WS_SUCCESS && ssh->isKeying) { ret = WS_REKEYING; } WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_worker(), ret = %d", ret); diff --git a/src/wolfsftp.c b/src/wolfsftp.c index a28fba34..47a2ac43 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -404,7 +404,8 @@ static INLINE int NoticeError(WOLFSSH* ssh) { return (ssh->error == WS_WANT_READ || ssh->error == WS_WANT_WRITE || - ssh->error == WS_CHAN_RXD); + ssh->error == WS_CHAN_RXD || + ssh->error == WS_REKEYING); } @@ -6927,6 +6928,9 @@ int wolfSSH_SFTP_SendReadPacket(WOLFSSH* ssh, byte* handle, word32 handleSz, /* send header and type specific data */ ret = wolfSSH_SFTP_buffer_send(ssh, &state->buffer); if (ret < 0) { + if (ret == WS_REKEYING) { + return ret; + } if (ssh->error != WS_WANT_READ && ssh->error != WS_WANT_WRITE) { state->state = STATE_SEND_READ_CLEANUP; From 748f085f12c7a8d453e8fb406bd92d58d77fb6b7 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 13 Apr 2023 16:43:05 -0700 Subject: [PATCH 5/6] Rekey 1. Fix an issue found in testing. Some checks for rekeying might happen when ssh is NULL. --- src/ssh.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/ssh.c b/src/ssh.c index 55e13c17..104dbf4d 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1783,22 +1783,22 @@ int wolfSSH_worker(WOLFSSH* ssh, word32* channelId) ret = DoReceive(ssh); } - if (channelId != NULL && ssh != NULL) { - *channelId = ssh->lastRxId; + if (ret == WS_SUCCESS) { + if (channelId != NULL) { + *channelId = ssh->lastRxId; + } + + if (ssh->isKeying) { + ssh->error = WS_REKEYING; + return WS_REKEYING; + } } - if (ssh->isKeying) { - ssh->error = WS_REKEYING; - return WS_REKEYING; - } - - if (ret == WS_CHAN_RXD) + if (ret == WS_CHAN_RXD) { WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_worker(), " "data received on channel %u", ssh->lastRxId); + } else { - if (ret != WS_SUCCESS && ssh->isKeying) { - ret = WS_REKEYING; - } WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_worker(), ret = %d", ret); } return ret; From d00db44b2f9c6c1fb8397e150412f5062f286532 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 19 Apr 2023 13:39:35 -0700 Subject: [PATCH 6/6] SFTP Update 1. In the API test for SFTP, change the amount of the file requested to the limit WOLFSSH_MAX_SFTP_RW. 2. The check for want read on the exit of the server worker should clear ret if it is want read. --- examples/echoserver/echoserver.c | 6 +++++- tests/api.c | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c index e33167b0..a7a16228 100644 --- a/examples/echoserver/echoserver.c +++ b/examples/echoserver/echoserver.c @@ -1330,7 +1330,11 @@ static THREAD_RETURN WOLFSSH_THREAD server_worker(void* vArgs) break; } - if (error != WS_WANT_READ && error != WS_WANT_WRITE) { + if (error == WS_WANT_READ || error == WS_WANT_WRITE) { + /* Wanting read or wanting write. Clear ret. */ + ret = 0; + } + else { break; } } diff --git a/tests/api.c b/tests/api.c index f0dea551..eaccd418 100644 --- a/tests/api.c +++ b/tests/api.c @@ -816,14 +816,14 @@ static void test_wolfSSH_SFTP_SendReadPacket(void) } /* partial read */ - outSz = tmp->atrb.sz[0] / 2; + outSz = WOLFSSH_MAX_SFTP_RW / 2; rxSz = wolfSSH_SFTP_SendReadPacket(ssh, handle, handleSz, ofst, out, outSz); AssertIntGT(rxSz, 0); AssertIntLE(rxSz, outSz); /* read all */ - outSz = tmp->atrb.sz[0]; + outSz = WOLFSSH_MAX_SFTP_RW; rxSz = wolfSSH_SFTP_SendReadPacket(ssh, handle, handleSz, ofst, out, outSz); AssertIntGT(rxSz, 0);