From 5fa6c0fce30a421879b355a007ef843eb48332d3 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Thu, 27 Mar 2025 08:54:23 -0600 Subject: [PATCH 1/8] sanity checks on message types during rekey --- src/internal.c | 55 +++++++++++++++++++++++++++++++++++++++++++--- wolfssh/internal.h | 5 ++++- 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/src/internal.c b/src/internal.c index 63f0e1af7..739a9bad2 100644 --- a/src/internal.c +++ b/src/internal.c @@ -595,6 +595,40 @@ static void HandshakeInfoFree(HandshakeInfo* hs, void* heap) } +/* RFC 4253 section 7.1, Once having sent SSH_MSG_KEXINIT the only messages +* that can be sent are 1-19 (except SSH_MSG_SERVICE_REQUEST and +* SSH_MSG_SERVICE_ACCEPT), 20-29 (except SSH_MSG_KEXINIT again), and 30-49 +*/ +INLINE static int IsMessageAllowedKeying(WOLFSSH *ssh, byte msg) +{ + if (ssh->isKeying == 0) { + return 1; + } + + /* case of servie request or accept in 1-19 */ + if (msg == MSGID_SERVICE_REQUEST || msg == MSGID_SERVICE_ACCEPT) { + WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by during rekeying", msg); + ssh->error = WS_REKEYING; + return 0; + } + + /* case of resending SSH_MSG_KEXINIT */ + if (msg == MSGID_KEXINIT) { + WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by during rekeying", msg); + ssh->error = WS_REKEYING; + return 0; + } + + /* case where message id greater than 49 */ + if (msg >= MSGID_USERAUTH_REQUEST) { + WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by during rekeying", msg); + ssh->error = WS_REKEYING; + return 0; + } + return 1; +} + + #ifndef NO_WOLFSSH_SERVER INLINE static int IsMessageAllowedServer(WOLFSSH *ssh, byte msg) { @@ -673,8 +707,12 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg) #endif /* NO_WOLFSSH_CLIENT */ -INLINE static int IsMessageAllowed(WOLFSSH *ssh, byte msg) +INLINE static int IsMessageAllowed(WOLFSSH *ssh, byte msg, byte state) { + if (state == WS_MSG_SEND && !IsMessageAllowedKeying(ssh, msg)) { + return 0; + } + #ifndef NO_WOLFSSH_SERVER if (ssh->ctx->side == WOLFSSH_ENDPOINT_SERVER) { return IsMessageAllowedServer(ssh, msg); @@ -5905,7 +5943,6 @@ static int DoNewKeys(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) HandshakeInfoFree(ssh->handshake, ssh->ctx->heap); ssh->handshake = NULL; WLOG(WS_LOG_DEBUG, "Keying completed"); - if (ssh->ctx->keyingCompletionCb) ssh->ctx->keyingCompletionCb(ssh->keyingCompletionCtx); } @@ -9309,7 +9346,7 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed) return WS_OVERFLOW_E; } - if (!IsMessageAllowed(ssh, msg)) { + if (!IsMessageAllowed(ssh, msg, WS_MSG_RECV)) { return WS_MSGID_NOT_ALLOWED_E; } @@ -15649,6 +15686,12 @@ int SendChannelEof(WOLFSSH* ssh, word32 peerChannelId) if (ssh == NULL) ret = WS_BAD_ARGUMENT; + if (ret == WS_SUCCESS) { + if (!IsMessageAllowed(ssh, MSGID_CHANNEL_EOF, WS_MSG_SEND)) { + ret = WS_MSGID_NOT_ALLOWED_E; + } + } + if (ret == WS_SUCCESS) { channel = ChannelFind(ssh, peerChannelId, WS_CHANNEL_ID_PEER); if (channel == NULL) @@ -16077,6 +16120,12 @@ int SendChannelWindowAdjust(WOLFSSH* ssh, word32 channelId, if (ssh == NULL) ret = WS_BAD_ARGUMENT; + if (ret == WS_SUCCESS) { + if (!IsMessageAllowed(ssh, MSGID_CHANNEL_WINDOW_ADJUST, WS_MSG_SEND)) { + ret = WS_MSGID_NOT_ALLOWED_E; + } + } + channel = ChannelFind(ssh, channelId, WS_CHANNEL_ID_SELF); if (channel == NULL) { WLOG(WS_LOG_DEBUG, "Invalid channel"); diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 261ae6d42..29a6f8ef8 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1249,6 +1249,10 @@ enum WS_MessageIds { #define CHANNEL_EXTENDED_DATA_STDERR WOLFSSH_EXT_DATA_STDERR +/* Used when checking IsMessageAllowed() to determine if createing and sending + * the message or receiving the message is allowed */ +#define WS_MSG_SEND 1 +#define WS_MSG_RECV 2 /* dynamic memory types */ enum WS_DynamicTypes { @@ -1442,4 +1446,3 @@ enum TerminalModes { #endif #endif /* _WOLFSSH_INTERNAL_H_ */ - From af45bc3719ddeac112d9d70b2e6a969f1aa3f3e7 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Tue, 13 May 2025 16:01:01 -0600 Subject: [PATCH 2/8] update example client for rekey and sanity check on window update after read attempt --- examples/client/client.c | 11 +++++++++++ src/ssh.c | 12 +++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/examples/client/client.c b/examples/client/client.c index e27305f72..f415bd801 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -342,6 +342,9 @@ static THREAD_RET readInput(void* in) ret = wolfSSH_stream_send(args->ssh, buf, sz); wc_UnLockMutex(&args->lock); if (ret <= 0) { + if (ret == WS_REKEYING) { + continue; + } fprintf(stderr, "Couldn't send data\n"); return THREAD_RET_SUCCESS; } @@ -472,8 +475,16 @@ static THREAD_RET readPeer(void* in) continue; } #endif /* WOLFSSH_AGENT */ + else if (ret == WS_REKEYING) { + wolfSSH_worker(args->ssh, NULL); + ret = 0; + } } else if (ret != WS_EOF) { + if (ret == 0) { + bytes = 0; + continue; + } err_sys("Stream read failed."); } } diff --git a/src/ssh.c b/src/ssh.c index 05c1a7b3e..2ed75d1bc 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1135,6 +1135,11 @@ int wolfSSH_stream_read(WOLFSSH* ssh, byte* buf, word32 bufSz) return WS_ERROR; } + if (ssh->isKeying) { + ssh->error = WS_REKEYING; + return WS_FATAL_ERROR; + } + inputBuffer = &ssh->channelList->inputBuffer; ssh->error = WS_SUCCESS; @@ -1164,7 +1169,7 @@ int wolfSSH_stream_read(WOLFSSH* ssh, byte* buf, word32 bufSz) } /* update internal input buffer based on data read */ - if (ret == WS_SUCCESS) { + if (ret == WS_SUCCESS && !ssh->isKeying) { int n; n = min(bufSz, inputBuffer->length - inputBuffer->idx); @@ -2901,6 +2906,11 @@ int wolfSSH_ChannelRead(WOLFSSH_CHANNEL* channel, byte* buf, word32 bufSz) if (channel == NULL || buf == NULL || bufSz == 0) return WS_BAD_ARGUMENT; + if (channel->ssh->isKeying) { + channel->ssh->error = WS_REKEYING; + return WS_REKEYING; + } + bufSz = _ChannelRead(channel, buf, bufSz); WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_ChannelRead(), bytesRxd = %d", From d74c942c84d44fb46d3a10cb56233b704733e466 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Thu, 15 May 2025 11:38:45 -0600 Subject: [PATCH 3/8] refactor SFTP to use NoticeError --- examples/sftpclient/sftpclient.c | 8 +++++++- src/ssh.c | 2 +- src/wolfsftp.c | 25 +++++++++++-------------- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/examples/sftpclient/sftpclient.c b/examples/sftpclient/sftpclient.c index 779baff1f..a990bc14e 100644 --- a/examples/sftpclient/sftpclient.c +++ b/examples/sftpclient/sftpclient.c @@ -1119,7 +1119,7 @@ static int doCmds(func_args* args) /* alternate main loop for the autopilot get/receive */ static int doAutopilot(int cmd, char* local, char* remote) { - int err; + int err = 0; int ret = WS_SUCCESS; char fullpath[128] = "."; WS_SFTPNAME* name = NULL; @@ -1156,6 +1156,12 @@ static int doAutopilot(int cmd, char* local, char* remote) } do { + if (err == WS_REKEYING) { /* handle rekeying state */ + do { + ret = wolfSSH_worker(ssh, NULL); + } while (ret == WS_REKEYING); + } + if (cmd == AUTOPILOT_PUT) { ret = wolfSSH_SFTP_Put(ssh, local, fullpath, 0, NULL); } diff --git a/src/ssh.c b/src/ssh.c index 2ed75d1bc..56c248589 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1201,7 +1201,7 @@ int wolfSSH_stream_send(WOLFSSH* ssh, byte* buf, word32 bufSz) if (ssh->isKeying) { ssh->error = WS_REKEYING; - return WS_REKEYING; + return WS_FATAL_ERROR; } bytesTxd = SendChannelData(ssh, ssh->channelList->channel, buf, bufSz); diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 761830a34..a95428fd0 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -1418,7 +1418,11 @@ int wolfSSH_SFTP_read(WOLFSSH* ssh) ret = wolfSSH_SFTP_buffer_read(ssh, &state->buffer, state->buffer.sz); if (ret < 0) { - if (!NoticeError(ssh)) { + if (NoticeError(ssh)) { + /* keep state for returning to */ + ret = WS_FATAL_ERROR; + } + else { wolfSSH_SFTP_ClearState(ssh, STATE_ID_RECV); } return ret; @@ -7452,8 +7456,7 @@ int wolfSSH_SFTP_SendWritePacket(WOLFSSH* ssh, byte* handle, word32 handleSz, /* send header and type specific data */ ret = wolfSSH_SFTP_buffer_send(ssh, &state->buffer); if (ret < 0) { - if (ssh->error == WS_WANT_READ || - ssh->error == WS_WANT_WRITE) { + if (NoticeError(ssh)) { return WS_FATAL_ERROR; } state->state = STATE_SEND_WRITE_CLEANUP; @@ -7465,12 +7468,8 @@ int wolfSSH_SFTP_SendWritePacket(WOLFSSH* ssh, byte* handle, word32 handleSz, case STATE_SEND_WRITE_SEND_BODY: WLOG(WS_LOG_SFTP, "SFTP SEND_WRITE STATE: SEND_BODY"); state->sentSz = wolfSSH_stream_send(ssh, in, inSz); - if (state->sentSz == WS_WINDOW_FULL || - state->sentSz == WS_REKEYING || - state->sentSz == WS_WANT_READ || - state->sentSz == WS_WANT_WRITE) { - ret = wolfSSH_worker(ssh, NULL); - continue; /* skip past rest and send more */ + if (NoticeError(ssh)) { + return WS_FATAL_ERROR; } if (state->sentSz <= 0) { ssh->error = state->sentSz; @@ -7496,8 +7495,7 @@ int wolfSSH_SFTP_SendWritePacket(WOLFSSH* ssh, byte* handle, word32 handleSz, state->maxSz = SFTP_GetHeader(ssh, &state->reqId, &type, &state->buffer); if (state->maxSz <= 0) { - if (ssh->error == WS_WANT_READ || - ssh->error == WS_WANT_WRITE) { + if (NoticeError(ssh)) { return WS_FATAL_ERROR; } ssh->error = WS_SFTP_BAD_HEADER; @@ -9167,10 +9165,9 @@ int wolfSSH_SFTP_Put(WOLFSSH* ssh, char* from, char* to, byte resume, state->handle, state->handleSz, state->pOfst, state->r, state->rSz); if (sz <= 0) { - if (ssh->error == WS_WANT_READ || - ssh->error == WS_WANT_WRITE || - ssh->error == WS_WINDOW_FULL) + if (NoticeError(ssh)) { return WS_FATAL_ERROR; + } } else { AddAssign64(state->pOfst, sz); From ff95f3c3029d766b114a91d98b013e0a1636a6c1 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Thu, 15 May 2025 13:32:33 -0600 Subject: [PATCH 4/8] increase timeout time on test, fix spelling, add comment on new arg --- .github/workflows/sshd-test.yml | 2 +- src/internal.c | 2 ++ wolfssh/internal.h | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/sshd-test.yml b/.github/workflows/sshd-test.yml index 3fbe3daf8..eb075a6f1 100644 --- a/.github/workflows/sshd-test.yml +++ b/.github/workflows/sshd-test.yml @@ -66,7 +66,7 @@ jobs: wolfssl: ${{ fromJson(needs.create_matrix.outputs['versions']) }} name: Build and test wolfsshd runs-on: ${{ matrix.os }} - timeout-minutes: 10 + timeout-minutes: 15 steps: - name: Checking cache for wolfssl uses: actions/cache@v4 diff --git a/src/internal.c b/src/internal.c index 739a9bad2..ff912ef74 100644 --- a/src/internal.c +++ b/src/internal.c @@ -707,6 +707,8 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg) #endif /* NO_WOLFSSH_CLIENT */ +/* 'state' argument is for if trying to send a message or receive one. + * Returns 1 if allowed 0 if not allowed. */ INLINE static int IsMessageAllowed(WOLFSSH *ssh, byte msg, byte state) { if (state == WS_MSG_SEND && !IsMessageAllowedKeying(ssh, msg)) { diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 29a6f8ef8..ad5e00b0e 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1249,7 +1249,7 @@ enum WS_MessageIds { #define CHANNEL_EXTENDED_DATA_STDERR WOLFSSH_EXT_DATA_STDERR -/* Used when checking IsMessageAllowed() to determine if createing and sending +/* Used when checking IsMessageAllowed() to determine if creating and sending * the message or receiving the message is allowed */ #define WS_MSG_SEND 1 #define WS_MSG_RECV 2 From 2a11471bb717a2ee6f06b3e1beab8a3e2b0ef261 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Tue, 16 Sep 2025 13:30:25 -0600 Subject: [PATCH 5/8] refactor introducing more use of NoticeError --- examples/echoserver/echoserver.c | 5 +- examples/sftpclient/sftpclient.c | 81 ++++++++++++++++++++++++++++---- src/wolfsftp.c | 35 +++++++------- tests/api.c | 5 ++ 4 files changed, 100 insertions(+), 26 deletions(-) diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c index 1fbd58a0c..8d14a7c95 100644 --- a/examples/echoserver/echoserver.c +++ b/examples/echoserver/echoserver.c @@ -1416,8 +1416,11 @@ static int sftp_worker(thread_ctx_t* threadCtx) } else if (ret < 0) { error = wolfSSH_get_error(ssh); - if (error == WS_EOF) + if (error == WS_EOF) { + /* shutdown is happening, clear peek error */ + ret = 0; break; + } } if (ret == WS_FATAL_ERROR && error == 0) { diff --git a/examples/sftpclient/sftpclient.c b/examples/sftpclient/sftpclient.c index a990bc14e..b735c0d51 100644 --- a/examples/sftpclient/sftpclient.c +++ b/examples/sftpclient/sftpclient.c @@ -566,11 +566,8 @@ static int doCmds(func_args* args) } do { - while (ret == WS_REKEYING || ssh->error == WS_REKEYING) { + while (wolfSSH_get_error(ssh) == WS_REKEYING) { ret = wolfSSH_worker(ssh, NULL); - if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) { - ret = wolfSSH_get_error(ssh); - } } ret = wolfSSH_SFTP_Get(ssh, pt, to, resume, &myStatusCb); @@ -747,6 +744,13 @@ static int doCmds(func_args* args) /* check directory is valid */ do { + 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); + } + } + ret = wolfSSH_SFTP_STAT(ssh, pt, &atrb); err = wolfSSH_get_error(ssh); } while ((err == WS_WANT_READ || err == WS_WANT_WRITE) @@ -828,6 +832,13 @@ static int doCmds(func_args* args) /* update permissions */ do { + 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); + } + } + ret = wolfSSH_SFTP_CHMOD(ssh, pt, mode); err = wolfSSH_get_error(ssh); } while ((err == WS_WANT_READ || err == WS_WANT_WRITE) @@ -878,6 +889,13 @@ static int doCmds(func_args* args) } do { + 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); + } + } + ret = wolfSSH_SFTP_RMDIR(ssh, pt); err = wolfSSH_get_error(ssh); } while ((err == WS_WANT_READ || err == WS_WANT_WRITE) @@ -924,6 +942,13 @@ static int doCmds(func_args* args) } do { + 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); + } + } + ret = wolfSSH_SFTP_Remove(ssh, pt); err = wolfSSH_get_error(ssh); } while ((err == WS_WANT_READ || err == WS_WANT_WRITE) @@ -1458,14 +1483,52 @@ THREAD_RETURN WOLFSSH_THREAD sftpclient_test(void* args) WFREE(workingDir, NULL, DYNAMIC_TYPE_TMP_BUFFER); if (ret == WS_SUCCESS) { - if (wolfSSH_shutdown(ssh) != WS_SUCCESS) { - int rc; - rc = wolfSSH_get_error(ssh); + int err; + ret = wolfSSH_shutdown(ssh); + + /* peer hung up, stop trying to shutdown */ + if (ret == WS_SOCKET_ERROR_E) { + ret = 0; + } + + err = wolfSSH_get_error(ssh); + if (err != WS_SOCKET_ERROR_E && + (err == WS_WANT_READ || err == WS_WANT_WRITE)) { + int maxAttempt = 10; /* make 10 attempts max before giving up */ + int attempt; + + for (attempt = 0; attempt < maxAttempt; attempt++) { + ret = wolfSSH_worker(ssh, NULL); + err = wolfSSH_get_error(ssh); + + /* peer succesfully closed down gracefully */ + if (ret == WS_CHANNEL_CLOSED) { + ret = 0; + break; + } - if (rc != WS_SOCKET_ERROR_E && rc != WS_EOF) - printf("error with wolfSSH_shutdown()\n"); + /* peer hung up, stop shutdown */ + if (ret == WS_SOCKET_ERROR_E) { + ret = 0; + break; + } + + if (err == WS_WANT_READ || err == WS_WANT_WRITE) { + /* Wanting read or wanting write. Clear ret. */ + ret = 0; + } + else { + break; + } + } + + if (attempt == maxAttempt) { + printf("SFTP client gave up on gracefull shutdown," + "closing the socket\n"); + } } } + WCLOSESOCKET(sockFd); wolfSSH_free(ssh); wolfSSH_CTX_free(ctx); diff --git a/src/wolfsftp.c b/src/wolfsftp.c index a95428fd0..53c5dfd1a 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -865,6 +865,7 @@ static int SFTP_GetHeader(WOLFSSH* ssh, word32* reqId, byte* type, */ static int SFTP_SetHeader(WOLFSSH* ssh, word32 reqId, byte type, word32 len, byte* buf) { + c32toa(len + LENGTH_SZ + MSG_ID_SZ, buf); buf[LENGTH_SZ] = type; c32toa(reqId, buf + LENGTH_SZ + MSG_ID_SZ); @@ -1170,8 +1171,9 @@ int wolfSSH_SFTP_accept(WOLFSSH* ssh) case SFTP_EXT: ret = SFTP_ServerRecvInit(ssh); if (ret != WS_SUCCESS) { - if (ssh->error != WS_WANT_READ && ssh->error != WS_WANT_WRITE) + if (!NoticeError(ssh)) { wolfSSH_SFTP_ClearState(ssh, STATE_ID_ALL); + } return ret; } ssh->sftpState = SFTP_RECV; @@ -1573,8 +1575,9 @@ int wolfSSH_SFTP_read(WOLFSSH* ssh) /* break out if encountering an error with nothing stored to send */ if (ret < 0 && !state->toSend) { - if (ssh->error != WS_WANT_READ && ssh->error != WS_WANT_WRITE) + if (!NoticeError(ssh)) { wolfSSH_SFTP_ClearState(ssh, STATE_ID_RECV); + } return ret; } state->buffer.idx = 0; @@ -7674,8 +7677,8 @@ 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 (NoticeError(ssh)) { + return WS_FATAL_ERROR; } if (ssh->error != WS_WANT_READ && ssh->error != WS_WANT_WRITE) { @@ -7693,14 +7696,12 @@ int wolfSSH_SFTP_SendReadPacket(WOLFSSH* ssh, byte* handle, word32 handleSz, /* Get response */ if ((ret = SFTP_GetHeader(ssh, &state->reqId, &state->type, &state->buffer)) <= 0) { - if (ssh->error != WS_WANT_READ && - ssh->error != WS_WANT_WRITE) { + if (!NoticeError(ssh)) { state->state = STATE_SEND_READ_CLEANUP; continue; } return WS_FATAL_ERROR; } - ret = wolfSSH_SFTP_buffer_create(ssh, &state->buffer, ret); if (ret != WS_SUCCESS) { state->state = STATE_SEND_READ_CLEANUP; @@ -7718,8 +7719,9 @@ int wolfSSH_SFTP_SendReadPacket(WOLFSSH* ssh, byte* handle, word32 handleSz, state->state = STATE_SEND_READ_CLEANUP; continue; } - else + else { ssh->reqId++; + } if (state->type == WOLFSSH_FTP_DATA) state->state = STATE_SEND_READ_FTP_DATA; @@ -7737,8 +7739,7 @@ int wolfSSH_SFTP_SendReadPacket(WOLFSSH* ssh, byte* handle, word32 handleSz, /* get size of string and place it into out buffer */ ret = wolfSSH_stream_read(ssh, szFlat, UINT32_SZ); if (ret < 0) { - if (ssh->error != WS_WANT_READ && - ssh->error != WS_WANT_WRITE) { + if (!NoticeError(ssh)) { state->state = STATE_SEND_READ_CLEANUP; continue; } @@ -7917,8 +7918,9 @@ int wolfSSH_SFTP_MKDIR(WOLFSSH* ssh, char* dir, WS_SFTP_FILEATRB* atr) /* send header and type specific data */ ret = wolfSSH_SFTP_buffer_send(ssh, &state->buffer); if (ret < 0) { - if (ssh->error != WS_WANT_READ && ssh->error != WS_WANT_WRITE) + if (!NoticeError(ssh)) { wolfSSH_SFTP_ClearState(ssh, STATE_ID_MKDIR); + } return ret; } @@ -7931,8 +7933,9 @@ int wolfSSH_SFTP_MKDIR(WOLFSSH* ssh, char* dir, WS_SFTP_FILEATRB* atr) /* Get response */ if ((ret = SFTP_GetHeader(ssh, &state->reqId, &type, &state->buffer)) <= 0) { - if (ssh->error != WS_WANT_READ && ssh->error != WS_WANT_WRITE) + if (!NoticeError(ssh)) { wolfSSH_SFTP_ClearState(ssh, STATE_ID_MKDIR); + } return WS_FATAL_ERROR; } @@ -7963,8 +7966,9 @@ int wolfSSH_SFTP_MKDIR(WOLFSSH* ssh, char* dir, WS_SFTP_FILEATRB* atr) ret = wolfSSH_SFTP_buffer_read(ssh, &state->buffer, wolfSSH_SFTP_buffer_size(&state->buffer)); if (ret < 0) { - if (ssh->error != WS_WANT_READ && ssh->error != WS_WANT_WRITE) - wolfSSH_SFTP_ClearState(ssh, STATE_ID_MKDIR); + if (!NoticeError(ssh)) { + wolfSSH_SFTP_ClearState(ssh, STATE_ID_MKDIR); + } return WS_FATAL_ERROR; } @@ -8031,8 +8035,7 @@ WS_SFTPNAME* wolfSSH_SFTP_ReadDir(WOLFSSH* ssh, byte* handle, case STATE_READDIR_NAME: name = wolfSSH_SFTP_DoName(ssh); if (name == NULL) { - if (ssh->error != WS_WANT_READ - && ssh->error != WS_WANT_WRITE) { + if (!NoticeError(ssh)) { wolfSSH_SFTP_ClearState(ssh, STATE_ID_READDIR); } return NULL; diff --git a/tests/api.c b/tests/api.c index 2bef34998..701425d20 100644 --- a/tests/api.c +++ b/tests/api.c @@ -1075,6 +1075,11 @@ static void test_wolfSSH_SFTP_SendReadPacket(void) } } + /* take care of re-keying state before shutdown call */ + while (wolfSSH_get_error(ssh) == WS_REKEYING) { + wolfSSH_worker(ssh, NULL); + } + argsCount = wolfSSH_shutdown(ssh); if (argsCount == WS_SOCKET_ERROR_E) { /* If the socket is closed on shutdown, peer is gone, this is OK. */ From 813ec263cc56e7c9093135d854b3fc887633d368 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Tue, 16 Sep 2025 13:52:12 -0600 Subject: [PATCH 6/8] fix for scan-build report of unused return value --- examples/sftpclient/sftpclient.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/examples/sftpclient/sftpclient.c b/examples/sftpclient/sftpclient.c index b735c0d51..1194b0de5 100644 --- a/examples/sftpclient/sftpclient.c +++ b/examples/sftpclient/sftpclient.c @@ -566,8 +566,11 @@ static int doCmds(func_args* args) } do { - while (wolfSSH_get_error(ssh) == WS_REKEYING) { + 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); + } } ret = wolfSSH_SFTP_Get(ssh, pt, to, resume, &myStatusCb); From cc17941a6125daefbadb8c236fdeaeb1a21ec786 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Tue, 16 Sep 2025 14:16:16 -0600 Subject: [PATCH 7/8] adjust test case to account for re-keying return --- tests/api.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/api.c b/tests/api.c index 701425d20..9da02849b 100644 --- a/tests/api.c +++ b/tests/api.c @@ -1057,14 +1057,16 @@ static void test_wolfSSH_SFTP_SendReadPacket(void) outSz = WOLFSSH_MAX_SFTP_RW / 2; rxSz = wolfSSH_SFTP_SendReadPacket(ssh, handle, handleSz, ofst, out, outSz); - AssertIntGT(rxSz, 0); - AssertIntLE(rxSz, outSz); + if (wolfSSH_get_error(ssh) != WS_REKEYING) { + AssertIntGT(rxSz, 0); + AssertIntLE(rxSz, outSz); + } /* read all */ outSz = WOLFSSH_MAX_SFTP_RW; rxSz = wolfSSH_SFTP_SendReadPacket(ssh, handle, handleSz, ofst, out, outSz); - if (rxSz != WS_REKEYING) { + if (wolfSSH_get_error(ssh) != WS_REKEYING) { AssertIntGT(rxSz, 0); AssertIntLE(rxSz, outSz); } From 4862400a374253216e596ff5c3b018b857015cbb Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Mon, 6 Oct 2025 00:42:05 -0600 Subject: [PATCH 8/8] fix spelling issues and SFTP send state --- .github/workflows/sshd-test.yml | 2 +- examples/client/client.c | 4 ++-- examples/sftpclient/sftpclient.c | 7 ++++--- src/internal.c | 2 +- src/wolfsftp.c | 15 +++++++++------ 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/.github/workflows/sshd-test.yml b/.github/workflows/sshd-test.yml index eb075a6f1..3fbe3daf8 100644 --- a/.github/workflows/sshd-test.yml +++ b/.github/workflows/sshd-test.yml @@ -66,7 +66,7 @@ jobs: wolfssl: ${{ fromJson(needs.create_matrix.outputs['versions']) }} name: Build and test wolfsshd runs-on: ${{ matrix.os }} - timeout-minutes: 15 + timeout-minutes: 10 steps: - name: Checking cache for wolfssl uses: actions/cache@v4 diff --git a/examples/client/client.c b/examples/client/client.c index f415bd801..49d00f44f 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -481,10 +481,10 @@ static THREAD_RET readPeer(void* in) } } else if (ret != WS_EOF) { - if (ret == 0) { + if (ret == 0) { bytes = 0; continue; - } + } err_sys("Stream read failed."); } } diff --git a/examples/sftpclient/sftpclient.c b/examples/sftpclient/sftpclient.c index 1194b0de5..e074b3d13 100644 --- a/examples/sftpclient/sftpclient.c +++ b/examples/sftpclient/sftpclient.c @@ -1184,7 +1184,7 @@ static int doAutopilot(int cmd, char* local, char* remote) } do { - if (err == WS_REKEYING) { /* handle rekeying state */ + if (err == WS_REKEYING || err == WS_WINDOW_FULL) { /* handle rekeying state */ do { ret = wolfSSH_worker(ssh, NULL); } while (ret == WS_REKEYING); @@ -1198,7 +1198,8 @@ static int doAutopilot(int cmd, char* local, char* remote) } err = wolfSSH_get_error(ssh); } while ((err == WS_WANT_READ || err == WS_WANT_WRITE || - err == WS_CHAN_RXD || err == WS_REKEYING) && + err == WS_CHAN_RXD || err == WS_REKEYING || + err == WS_WINDOW_FULL) && ret == WS_FATAL_ERROR); if (ret != WS_SUCCESS) { @@ -1504,7 +1505,7 @@ THREAD_RETURN WOLFSSH_THREAD sftpclient_test(void* args) ret = wolfSSH_worker(ssh, NULL); err = wolfSSH_get_error(ssh); - /* peer succesfully closed down gracefully */ + /* peer successfully closed down gracefully */ if (ret == WS_CHANNEL_CLOSED) { ret = 0; break; diff --git a/src/internal.c b/src/internal.c index ff912ef74..b9e3a3432 100644 --- a/src/internal.c +++ b/src/internal.c @@ -605,7 +605,7 @@ INLINE static int IsMessageAllowedKeying(WOLFSSH *ssh, byte msg) return 1; } - /* case of servie request or accept in 1-19 */ + /* case of service request or accept in 1-19 */ if (msg == MSGID_SERVICE_REQUEST || msg == MSGID_SERVICE_ACCEPT) { WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by during rekeying", msg); ssh->error = WS_REKEYING; diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 53c5dfd1a..998806aef 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -416,6 +416,7 @@ 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_WINDOW_FULL || ssh->error == WS_REKEYING); } @@ -865,7 +866,6 @@ static int SFTP_GetHeader(WOLFSSH* ssh, word32* reqId, byte* type, */ static int SFTP_SetHeader(WOLFSSH* ssh, word32 reqId, byte type, word32 len, byte* buf) { - c32toa(len + LENGTH_SZ + MSG_ID_SZ, buf); buf[LENGTH_SZ] = type; c32toa(reqId, buf + LENGTH_SZ + MSG_ID_SZ); @@ -7471,12 +7471,15 @@ int wolfSSH_SFTP_SendWritePacket(WOLFSSH* ssh, byte* handle, word32 handleSz, case STATE_SEND_WRITE_SEND_BODY: WLOG(WS_LOG_SFTP, "SFTP SEND_WRITE STATE: SEND_BODY"); state->sentSz = wolfSSH_stream_send(ssh, in, inSz); - if (NoticeError(ssh)) { - return WS_FATAL_ERROR; - } if (state->sentSz <= 0) { - ssh->error = state->sentSz; ret = WS_FATAL_ERROR; + if (NoticeError(ssh)) { + ret = wolfSSH_worker(ssh,NULL); + continue; + } + + /* if it was not a notice error then clean up the state and + * exit out */ state->state = STATE_SEND_WRITE_CLEANUP; continue; } @@ -9170,7 +9173,7 @@ int wolfSSH_SFTP_Put(WOLFSSH* ssh, char* from, char* to, byte resume, if (sz <= 0) { if (NoticeError(ssh)) { return WS_FATAL_ERROR; - } + } } else { AddAssign64(state->pOfst, sz);