diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 699d1e886..cccf00b06 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -711,6 +711,15 @@ int wolfSSH_TestSftpRecvSizeCheck(int sz) { return SFTP_CheckRecvSz(sz); } + + +/* Test hook: exposes the static SendPacketType() so the unit tests can + * check that no request is sent when SFTP negotiation did not succeed. */ +int wolfSSH_TestSftpSendPacketType(WOLFSSH* ssh, byte type, byte* buf, + word32 bufSz) +{ + return SendPacketType(ssh, type, buf, bufSz); +} #endif @@ -6905,8 +6914,14 @@ int SendPacketType(WOLFSSH* ssh, byte type, byte* buf, word32 bufSz) /* reset state for sending data */ wolfSSH_SFTP_buffer_rewind(&state->buffer); + state->state = SFTP_SEND_PACKET; + } + else { + if (!NoticeError(ssh)) { + wolfSSH_SFTP_ClearState(ssh, STATE_ID_SEND); + } + return ret; } - state->state = SFTP_SEND_PACKET; FALL_THROUGH; case SFTP_SEND_PACKET: diff --git a/tests/api.c b/tests/api.c index 5be9c300d..41b10a0fd 100644 --- a/tests/api.c +++ b/tests/api.c @@ -6267,6 +6267,201 @@ static void test_wolfSSH_SFTP_SaveOfst(void) } +/* SendPacketType must not send a request when negotiation did not succeed */ +static void test_wolfSSH_SFTP_SendPacketType_negotiate(void) +{ +#ifdef WOLFSSH_TEST_INTERNAL + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + byte payload[4] = { 0x00, 0x01, 0x02, 0x03 }; + int ret; + + AssertNotNull(ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL)); + AssertNotNull(ssh = wolfSSH_new(ctx)); + + ret = wolfSSH_TestSftpSendPacketType(ssh, WOLFSSH_FTP_REALPATH, + payload, (word32)sizeof(payload)); + + /* The SFTP connection was not established, so negotiation cannot succeed. + * SendPacketType() must report that failure instead of returning + * WS_SUCCESS */ + AssertIntNE(ret, WS_SUCCESS); + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +#endif /* WOLFSSH_TEST_INTERNAL */ +} + +/* Same as sftp_client_connect(), but stops before wolfSSH_SFTP_connect() and + * leaves the socket non-blocking. The first SFTP request then has to drive + * the SSH handshake and the SFTP negotiation itself. */ +static void sftp_client_connect_nonblock(WOLFSSH_CTX** ctx, WOLFSSH** ssh, + int port) +{ + SOCKET_T sockFd = WOLFSSH_SOCKET_INVALID; + SOCKADDR_IN_T clientAddr; + socklen_t clientAddrSz = sizeof(clientAddr); + int ret; + char* host = (char*)wolfSshIp; + const char* username = "jill"; + const char* password = "upthehill"; + + if (ctx == NULL || ssh == NULL) { + return; + } + + *ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL); + if (*ctx == NULL) { + return; + } + + wolfSSH_CTX_SetPublicKeyCheck(*ctx, AcceptAnyServerHostKey); + wolfSSH_SetUserAuth(*ctx, sftpUserAuth); + *ssh = wolfSSH_new(*ctx); + if (*ssh == NULL) { + wolfSSH_CTX_free(*ctx); + *ctx = NULL; + return; + } + + build_addr(&clientAddr, host, port); + tcp_socket(&sockFd, ((struct sockaddr_in *)&clientAddr)->sin_family); + if (sockFd < 0) { + wolfSSH_free(*ssh); + wolfSSH_CTX_free(*ctx); + *ctx = NULL; + *ssh = NULL; + return; + } + + ret = connect(sockFd, (const struct sockaddr *)&clientAddr, clientAddrSz); + if (ret != 0) { + WCLOSESOCKET(sockFd); + wolfSSH_free(*ssh); + wolfSSH_CTX_free(*ctx); + *ctx = NULL; + *ssh = NULL; + return; + } + + wolfSSH_SetUserAuthCtx(*ssh, (void*)password); + ret = wolfSSH_SetUsername(*ssh, username); + if (ret == WS_SUCCESS) + ret = wolfSSH_set_fd(*ssh, (int)sockFd); + + if (ret != WS_SUCCESS) { + WCLOSESOCKET(sockFd); + wolfSSH_free(*ssh); + wolfSSH_CTX_free(*ctx); + *ctx = NULL; + *ssh = NULL; + return; + } + + /* no wolfSSH_connect() or wolfSSH_SFTP_connect() here; the caller's first + * SFTP request negotiates over the non-blocking socket */ + tcp_set_nonblocking(&sockFd); +} + +/* Upper bound on non-blocking retry iterations. Each wait is at most 1s, so a + * regression fails the AssertIntLE below instead of hanging CI. */ +#define SFTP_MAX_NEGOTIATION_TRIES 100 + +/* The retryable half of the SendPacketType() negotiation fix. Without + * wolfSSH_SFTP_connect(), the first request on a non-blocking socket must + * negotiate first. Negotiation returns WS_WANT_READ/WS_WANT_WRITE, the send + * state is kept, and a later call finishes negotiating and actually sends the + * request. Before the fix the empty request was "sent" and the client + * waited for a reply that never came, until the retry bound tripped. */ +static void test_wolfSSH_SFTP_SendPacketType_negotiation_non_block(void) +{ + func_args ser; + tcp_ready ready; + int argsCount; + WS_SOCKET_T clientFd; + const char* args[10]; + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + WS_SFTPNAME* name = NULL; + THREAD_TYPE serThread; + int sawPartial = 0; + int err; + int tries = 0; + + /* one-shot echoserver on a free port */ + WMEMSET(&ser, 0, sizeof(func_args)); + argsCount = 0; + args[argsCount++] = "."; + args[argsCount++] = "-1"; + args[argsCount++] = "-p"; + args[argsCount++] = "0"; + ser.argv = (char**)args; + ser.argc = argsCount; + ser.signal = &ready; + InitTcpReady(ser.signal); + ThreadStart(echoserver_test, (void*)&ser, &serThread); + WaitTcpReady(&ready); + + sftp_client_connect_nonblock(&ctx, &ssh, ready.port); + AssertNotNull(ctx); + AssertNotNull(ssh); + clientFd = wolfSSH_get_fd(ssh); + + /* RealPath(".") goes through SendPacketType(). Retry it until + * the negotiation completes and the server answers with a name. */ + do { + name = wolfSSH_SFTP_RealPath(ssh, (char*)"."); + err = wolfSSH_get_error(ssh); + if (name == NULL && (err == WS_WANT_READ || err == WS_WANT_WRITE)) { + sawPartial = 1; + tcp_select(clientFd, 1); + } + tries++; + } while (name == NULL && (err == WS_WANT_READ || err == WS_WANT_WRITE) && + tries <= SFTP_MAX_NEGOTIATION_TRIES); + + AssertIntLE(tries, SFTP_MAX_NEGOTIATION_TRIES); + AssertIntEQ(sawPartial, 1); + AssertNotNull(name); + AssertNotNull(name->fName); + wolfSSH_SFTPNAME_list_free(name); + + /* shut down over the non-blocking socket, retrying the same way */ + tries = 0; + do { + argsCount = wolfSSH_shutdown(ssh); + err = wolfSSH_get_error(ssh); + if (argsCount != WS_SUCCESS && (err == WS_WANT_READ || + err == WS_WANT_WRITE || err == WS_REKEYING)) { + tcp_select(clientFd, 1); + } + tries++; + } while (argsCount != WS_SUCCESS && (err == WS_WANT_READ || + err == WS_WANT_WRITE || err == WS_REKEYING) && + tries <= SFTP_MAX_NEGOTIATION_TRIES); + + AssertIntLE(tries, SFTP_MAX_NEGOTIATION_TRIES); + argsCount = AbsorbBenignReset(ssh, argsCount); + +#if DEFAULT_HIGHWATER_MARK < 8000 + if (argsCount == WS_REKEYING) { + argsCount = WS_SUCCESS; + } +#endif + AssertIntEQ(argsCount, WS_SUCCESS); + + WCLOSESOCKET(clientFd); + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +#ifdef WOLFSSH_ZEPHYR + k_sleep(Z_TIMEOUT_TICKS(100)); +#endif + ThreadJoin(serThread); + FreeTcpReady(&ready); +} + + + #if !defined(NO_FILESYSTEM) && !defined(WOLFSSH_USER_FILESYSTEM) && \ !defined(WOLFSSH_ZEPHYR) @@ -6690,6 +6885,8 @@ static void test_wolfSSH_SFTP_StartPathNotConfined(void) { ; } static void test_wolfSSH_SFTP_SetConfinePath(void) { ; } static void test_wolfSSH_SFTP_SetDefaultPath(void) { ; } static void test_wolfSSH_SFTP_SaveOfst(void) { ; } +static void test_wolfSSH_SFTP_SendPacketType_negotiate(void) { ; } +static void test_wolfSSH_SFTP_SendPacketType_negotiation_non_block(void) { ; } static void test_wolfSSH_SFTP_PutResume(void) { ; } static void test_wolfSSH_SFTP_GetResume(void) { ; } #endif /* WOLFSSH_SFTP && !NO_WOLFSSH_CLIENT && !SINGLE_THREADED */ @@ -8823,6 +9020,8 @@ int wolfSSH_ApiTest(int argc, char** argv) test_wolfSSH_SFTP_SetConfinePath(); test_wolfSSH_SFTP_SetDefaultPath(); test_wolfSSH_SFTP_SaveOfst(); + test_wolfSSH_SFTP_SendPacketType_negotiate(); + test_wolfSSH_SFTP_SendPacketType_negotiation_non_block(); test_wolfSSH_SFTP_PutResume(); test_wolfSSH_SFTP_GetResume(); diff --git a/wolfssh/wolfsftp.h b/wolfssh/wolfsftp.h index 9b3c9aefd..ec3a377ab 100644 --- a/wolfssh/wolfsftp.h +++ b/wolfssh/wolfsftp.h @@ -360,6 +360,8 @@ WOLFSSH_LOCAL void wolfSSH_SFTP_ShowSizes(void); WS_SFTP_FILEATRB* atr, word32* idx); WOLFSSH_API int wolfSSH_TestSftpSendCap(WOLFSSH* ssh, word32 cap); WOLFSSH_API int wolfSSH_TestSftpStallPending(WOLFSSH* ssh, word32 count); + WOLFSSH_API int wolfSSH_TestSftpSendPacketType(WOLFSSH* ssh, byte type, + byte* buf, word32 bufSz); #if !defined(NO_WOLFSSH_SERVER) && !defined(NO_FILESYSTEM) WOLFSSH_API int wolfSSH_SFTP_TestRecvStateInit(WOLFSSH* ssh); WOLFSSH_API const byte* wolfSSH_SFTP_TestRecvReply(WOLFSSH* ssh,