From bbc9f9e1571fcf6feb21f9eb50d144ff2264441b Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Fri, 29 Sep 2023 15:04:43 -0700 Subject: [PATCH] add more test debug prints and better rekeying handling --- examples/echoserver/echoserver.c | 61 ++++++++++++++++++++++---------- examples/sftpclient/sftpclient.c | 54 ++++++++++++++++++++-------- src/wolfsftp.c | 21 +++++++---- tests/sftp.c | 15 ++++++-- 4 files changed, 110 insertions(+), 41 deletions(-) diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c index ab1338e0..0ac164c2 100644 --- a/examples/echoserver/echoserver.c +++ b/examples/echoserver/echoserver.c @@ -1161,24 +1161,47 @@ static int sftp_worker(thread_ctx_t* threadCtx) s = (WS_SOCKET_T)wolfSSH_get_fd(ssh); do { + 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_WANT_WRITE && wolfSSH_SFTP_PendingSend(ssh)) { + continue; /* no need to spend time attempting to pull data + * if there is still pending sends */ + } + if (error == WS_EOF) { + break; + } + } + selected = tcp_select(s, timeout); if (selected == WS_SELECT_ERROR_READY) { break; } - if (selected == WS_SELECT_RECV_READY) { + if (ret == WS_WANT_READ || ret == WS_WANT_WRITE || + selected == WS_SELECT_RECV_READY) { ret = wolfSSH_worker(ssh, NULL); error = wolfSSH_get_error(ssh); if (ret == WS_REKEYING) { - /* In a rekey, keep turning the crank. */ + /* In a rekey, keeping turning the crank. */ timeout = TEST_SFTP_TIMEOUT; continue; } - if (error == WS_WANT_READ) { - /* If would block, keep turning the crank. */ + + if (error == WS_WANT_READ || error == WS_WANT_WRITE || + error == WS_WINDOW_FULL) { timeout = TEST_SFTP_TIMEOUT; + ret = error; continue; } + if (error == WS_EOF) { break; } @@ -1188,39 +1211,41 @@ static int sftp_worker(thread_ctx_t* threadCtx) } } - if (wolfSSH_SFTP_PendingSend(ssh)) { - /* Yes, process the SFTP data. */ - ret = wolfSSH_SFTP_read(ssh); - timeout = (ret == WS_REKEYING) ? - TEST_SFTP_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); + 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; + } - /* Old check for EOF here */ - { + if (ret == WS_FATAL_ERROR && error == 0) { WOLFSSH_CHANNEL* channel = - wolfSSH_ChannelNext(ssh, NULL); + wolfSSH_ChannelNext(ssh, NULL); if (channel && wolfSSH_ChannelGetEof(channel)) { - ret = WS_EOF; + ret = 0; break; } } - timeout = TEST_SFTP_TIMEOUT; - } while (1); + } while (ret != WS_FATAL_ERROR); return ret; } diff --git a/examples/sftpclient/sftpclient.c b/examples/sftpclient/sftpclient.c index 52b6b89f..2a5c795c 100644 --- a/examples/sftpclient/sftpclient.c +++ b/examples/sftpclient/sftpclient.c @@ -503,18 +503,19 @@ static int doCmds(func_args* args) } do { - ret = wolfSSH_SFTP_Get(ssh, pt, to, resume, &myStatusCb); - 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); } } + + ret = wolfSSH_SFTP_Get(ssh, pt, to, resume, &myStatusCb); + 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_CHAN_RXD || ret == WS_REKEYING); #ifndef WOLFSSH_NO_TIMESTAMP WMEMSET(currentFile, 0, WOLFSSH_MAX_FILENAME); @@ -607,11 +608,19 @@ 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_Put(ssh, pt, to, resume, &myStatusCb); - err = wolfSSH_get_error(ssh); - } while ((err == WS_WANT_READ || err == WS_WANT_WRITE || - err == WS_CHAN_RXD || err == WS_REKEYING) && - ret == WS_FATAL_ERROR); + 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); #ifndef WOLFSSH_NO_TIMESTAMP WMEMSET(currentFile, 0, WOLFSSH_MAX_FILENAME); @@ -921,10 +930,19 @@ 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_Rename(ssh, pt, to); - err = wolfSSH_get_error(ssh); - } while ((err == WS_WANT_READ || err == WS_WANT_WRITE) - && ret != WS_SUCCESS); + 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); if (ret != WS_SUCCESS) { if (SFTP_FPUTS(args, "Error with rename\n") < 0) { err_msg("fputs error"); @@ -942,10 +960,18 @@ static int doCmds(func_args* args) WS_SFTPNAME* current; 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); + } + } + current = wolfSSH_SFTP_LS(ssh, workingDir); err = wolfSSH_get_error(ssh); - } while ((err == WS_WANT_READ || err == WS_WANT_WRITE) - && current == NULL && err != WS_SUCCESS); + } while ((err == WS_WANT_READ || err == WS_WANT_WRITE || + err == WS_REKEYING) && + (current == NULL && err != WS_SUCCESS)); if (WSTRNSTR(msg, "-s", MAX_CMD_SZ) != NULL) { char tmpStr[WOLFSSH_MAX_FILENAME]; diff --git a/src/wolfsftp.c b/src/wolfsftp.c index bbf07dcc..67c02d20 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -6265,7 +6265,8 @@ WS_SFTPNAME* wolfSSH_SFTP_LS(WOLFSSH* ssh, char* dir) case STATE_LS_REALPATH: state->name = wolfSSH_SFTP_RealPath(ssh, dir); if (state->name == NULL) { - if (ssh->error != WS_WANT_READ && ssh->error != WS_WANT_WRITE) { + if (ssh->error != WS_WANT_READ && ssh->error != WS_WANT_WRITE && + ssh->error != WS_REKEYING) { wolfSSH_SFTP_ClearState(ssh, STATE_ID_LS); } return NULL; @@ -6277,7 +6278,8 @@ WS_SFTPNAME* wolfSSH_SFTP_LS(WOLFSSH* ssh, char* dir) if (wolfSSH_SFTP_OpenDir(ssh, (byte*)state->name->fName, state->name->fSz) != WS_SUCCESS) { WLOG(WS_LOG_SFTP, "Unable to open directory"); - if (ssh->error != WS_WANT_READ && ssh->error != WS_WANT_WRITE) { + if (ssh->error != WS_WANT_READ && ssh->error != WS_WANT_WRITE && + ssh->error != WS_REKEYING) { wolfSSH_SFTPNAME_list_free(state->name); state->name = NULL; wolfSSH_SFTP_ClearState(ssh, STATE_ID_LS); } @@ -6293,7 +6295,8 @@ WS_SFTPNAME* wolfSSH_SFTP_LS(WOLFSSH* ssh, char* dir) if (wolfSSH_SFTP_GetHandle(ssh, state->handle, (word32*)&state->sz) != WS_SUCCESS) { WLOG(WS_LOG_SFTP, "Unable to get handle"); - if (ssh->error != WS_WANT_READ && ssh->error != WS_WANT_WRITE) { + if (ssh->error != WS_WANT_READ && ssh->error != WS_WANT_WRITE && + ssh->error != WS_REKEYING) { wolfSSH_SFTP_ClearState(ssh, STATE_ID_LS); } return NULL; @@ -6306,7 +6309,8 @@ WS_SFTPNAME* wolfSSH_SFTP_LS(WOLFSSH* ssh, char* dir) * times so we have to assign to state->name later. */ names = wolfSSH_SFTP_ReadDir(ssh, state->handle, state->sz); if (names == NULL) { - if (ssh->error == WS_WANT_READ || ssh->error == WS_WANT_WRITE) { + if (ssh->error == WS_WANT_READ || ssh->error == WS_WANT_WRITE || + ssh->error == WS_REKEYING) { return NULL; } WLOG(WS_LOG_SFTP, "Error reading directory"); @@ -6328,7 +6332,8 @@ WS_SFTPNAME* wolfSSH_SFTP_LS(WOLFSSH* ssh, char* dir) names = wolfSSH_SFTP_ReadDir(ssh, state->handle, state->sz); if (names == NULL) { if (ssh->error == WS_WANT_READ - || ssh->error == WS_WANT_WRITE) { + || ssh->error == WS_WANT_WRITE + || ssh->error == WS_REKEYING) { /* State does not change so we will get back to this case * clause in non-blocking mode. */ return NULL; @@ -6346,7 +6351,8 @@ WS_SFTPNAME* wolfSSH_SFTP_LS(WOLFSSH* ssh, char* dir) if (wolfSSH_SFTP_Close(ssh, state->handle, state->sz) != WS_SUCCESS) { WLOG(WS_LOG_SFTP, "Error closing handle"); - if (ssh->error != WS_WANT_READ && ssh->error != WS_WANT_WRITE) { + if (ssh->error != WS_WANT_READ && ssh->error != WS_WANT_WRITE && + ssh->error != WS_REKEYING) { wolfSSH_SFTPNAME_list_free(state->name); state->name = NULL; wolfSSH_SFTP_ClearState(ssh, STATE_ID_LS); @@ -8692,7 +8698,8 @@ int wolfSSH_SFTP_Put(WOLFSSH* ssh, char* from, char* to, byte resume, state->handleSz); if (ret != WS_SUCCESS) { if (ssh->error == WS_WANT_READ || - ssh->error == WS_WANT_WRITE) { + ssh->error == WS_WANT_WRITE || + ssh->error == WS_REKEYING) { return WS_FATAL_ERROR; } WLOG(WS_LOG_SFTP, "Error closing handle"); diff --git a/tests/sftp.c b/tests/sftp.c index 317c4dfd..2094879d 100644 --- a/tests/sftp.c +++ b/tests/sftp.c @@ -88,15 +88,23 @@ static int Expected(int command) char expt1[] = ".\n..\nwolfSSH sftp> "; char expt2[] = "..\n.\nwolfSSH sftp> "; if (WMEMCMP(expt1, inBuf, sizeof(expt1)) != 0 && - WMEMCMP(expt2, inBuf, sizeof(expt2)) != 0) + WMEMCMP(expt2, inBuf, sizeof(expt2)) != 0) { + fprintf(stderr, "Unexpected string of %s\n", inBuf); return -1; + } else return 0; } case 6: - return (WSTRNSTR(inBuf, "configure", sizeof(inBuf)) == NULL); + if (WSTRNSTR(inBuf, "configure", sizeof(inBuf)) == NULL) { + fprintf(stderr, "configure not found in %s\n", inBuf); + return 1; + } + else { + return 0; + } case 10: return (WSTRNSTR(inBuf, "test-get", sizeof(inBuf)) == NULL); @@ -135,6 +143,7 @@ static int commandCb(const char* in, char* out, int outSz) } if (Expected(commandIdx) != 0) { + fprintf(stderr, "Failed on command index %d\n", commandIdx); exit(1); /* abort out */ } commandIdx++; @@ -163,6 +172,8 @@ int wolfSSH_SftpTest(int flag) WMEMSET(&cli, 0, sizeof(func_args)); commandIdx = 0; + wolfSSH_Debugging_ON(); + argsCount = 0; args[argsCount++] = "."; args[argsCount++] = "-1";