From 932fa4a477ddaf68c21072cc77c19081fd9811dd Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Mon, 15 Jun 2026 16:42:40 +0900 Subject: [PATCH] Run threaded api-test SFTP/SCP tests on Windows --- .github/workflows/windows-check.yml | 13 ++++--- examples/echoserver/echoserver.c | 24 +++++++++++- examples/sftpclient/sftpclient.c | 14 +++++-- src/wolfscp.c | 15 ++++++-- tests/api.c | 60 +++++++++++++---------------- wolfssh/port.h | 8 ++++ wolfssh/test.h | 33 +++++++++++++++- 7 files changed, 120 insertions(+), 47 deletions(-) diff --git a/.github/workflows/windows-check.yml b/.github/workflows/windows-check.yml index c9fe6f32c..aed2420da 100644 --- a/.github/workflows/windows-check.yml +++ b/.github/workflows/windows-check.yml @@ -139,12 +139,13 @@ jobs: run: nuget restore ${{env.WOLFSSL_SOLUTION_FILE_PATH}} # These env paths already include the wolfssh/ and wolfssl/ checkout - # prefixes, so they must run from the workspace root (as the build job does). - - name: updated user_settings.h for sshd and x509 - run: cp ${{env.USER_SETTINGS_H_NEW}} ${{env.USER_SETTINGS_H}} - - - name: replace wolfSSL user_settings.h with wolfSSH user_settings.h - run: get-content ${{env.USER_SETTINGS_H_NEW}} | %{$_ -replace "if 0","if 1"} + # prefixes, so this runs from the default working directory, the workspace + # root. + - name: Enable wolfSSH options (sshd, sftp, x509) in user_settings.h + shell: bash + run: | + sed -i 's/#if 0/#if 1/g' ${{env.USER_SETTINGS_H_NEW}} + cp ${{env.USER_SETTINGS_H_NEW}} ${{env.USER_SETTINGS_H}} # WholeProgramOptimization=false disables /GL so the v142-independent objects # link without a cross-version code-generation requirement (C1047). diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c index f86fbc75f..a04e5c4b9 100644 --- a/examples/echoserver/echoserver.c +++ b/examples/echoserver/echoserver.c @@ -2260,6 +2260,11 @@ static int LoadPasswdList(StrList* strList, PwMapList* mapList) int count = 0; while (strList) { + if (WSTRLEN(strList->str) >= sizeof names - 1) { + fprintf(stderr, "Ignoring over-long entry\n"); + strList = strList->next; + continue; + } WSTRNCPY(names, strList->str, sizeof names - 1); passwd = WSTRCHR(names, ':'); if (passwd != NULL) { @@ -2289,6 +2294,11 @@ static int LoadKeyboardList(StrList* strList, PwMapList* mapList, int count = 0; while (strList) { + if (WSTRLEN(strList->str) >= sizeof names - 1) { + fprintf(stderr, "Ignoring over-long entry\n"); + strList = strList->next; + continue; + } WSTRNCPY(names, strList->str, sizeof names - 1); passwd = WSTRCHR(names, ':'); if (passwd != NULL) { @@ -2325,6 +2335,11 @@ static int LoadPubKeyList(StrList* strList, int format, PwMapList* mapList) buf = NULL; bufSz = 0; + if (WSTRLEN(strList->str) >= sizeof names - 1) { + fprintf(stderr, "Ignoring over-long entry\n"); + strList = strList->next; + continue; + } WSTRNCPY(names, strList->str, sizeof names - 1); fileName = WSTRCHR(names, ':'); if (fileName != NULL) { @@ -2841,6 +2856,13 @@ static INLINE void SignalTcpReady(tcp_ready* ready, word16 port) ready->port = port; pthread_cond_signal(&ready->cond); pthread_mutex_unlock(&ready->mutex); +#elif defined(USE_WINDOWS_API) && defined(NO_MAIN_DRIVER) && \ + !defined(SINGLE_THREADED) + EnterCriticalSection(&ready->cs); + ready->ready = 1; + ready->port = port; + LeaveCriticalSection(&ready->cs); + SetEvent(ready->readyEvent); #else WOLFSSH_UNUSED(ready); WOLFSSH_UNUSED(port); @@ -2986,7 +3008,7 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) } else { port = (word16)atoi(myoptarg); - #if !defined(NO_MAIN_DRIVER) || defined(USE_WINDOWS_API) + #if !defined(NO_MAIN_DRIVER) if (port == 0) { ES_ERROR("port number cannot be 0"); } diff --git a/examples/sftpclient/sftpclient.c b/examples/sftpclient/sftpclient.c index b7e7c332d..2525ff033 100644 --- a/examples/sftpclient/sftpclient.c +++ b/examples/sftpclient/sftpclient.c @@ -156,6 +156,7 @@ static void myStatusCb(WOLFSSH* sshIn, word32* bytes, char* name) static word32 lastOutputTime = 0; static word32 lastPrintedBytes[2] = {0, 0}; word32 elapsedTime; + word32 nameSz; #endif char buf[80]; word64 longBytes = ((word64)bytes[1] << 32) | bytes[0]; @@ -181,8 +182,11 @@ static void myStatusCb(WOLFSSH* sshIn, word32* bytes, char* name) lastOutputTime = 0; /* Reset timer for new file transfer */ lastPrintedBytes[0] = 0; lastPrintedBytes[1] = 0; - WMEMSET(currentFile, 0, WOLFSSH_MAX_FILENAME); - WSTRNCPY(currentFile, name, WOLFSSH_MAX_FILENAME); + WMEMSET(currentFile, 0, sizeof(currentFile)); + nameSz = (word32)WSTRLEN(name); + if (nameSz >= sizeof(currentFile)) + return; + WMEMCPY(currentFile, name, nameSz + 1); } elapsedTime = currentTime - startTime; WSNPRINTF(buf, sizeof(buf), "Processed %8llu\t bytes in %d seconds\r", @@ -1445,6 +1449,7 @@ static int doAutopilot(int cmd, char* local, char* remote) int ret = WS_SUCCESS; char fullpath[128] = "."; WS_SFTPNAME* name = NULL; + word32 remoteSz; byte remoteAbsPath = 0; /* check if is absolute path before making it one */ @@ -1462,8 +1467,11 @@ static int doAutopilot(int cmd, char* local, char* remote) if (remoteAbsPath) { /* use remote absolute path if provided */ + remoteSz = (word32)WSTRLEN(remote); + if (remoteSz >= sizeof(fullpath)) + return WS_BUFFER_E; WMEMSET(fullpath, 0, sizeof(fullpath)); - WSTRNCPY(fullpath, remote, sizeof(fullpath) - 1); + WMEMCPY(fullpath, remote, remoteSz + 1); } else { err = doBuildRemotePath(remote, fullpath, sizeof(fullpath), &name); diff --git a/src/wolfscp.c b/src/wolfscp.c index c6ed7a54c..8e6cdda27 100644 --- a/src/wolfscp.c +++ b/src/wolfscp.c @@ -1908,7 +1908,11 @@ static char* MakeScpCmd(const char* name, char dir, void* heap) char* cmd; int sz; +#ifdef USE_WINDOWS_API + sz = WSCPRINTF("scp -%c %s", dir, name) + 1; +#else sz = WSNPRINTF(NULL, 0, "scp -%c %s", dir, name) + 1; +#endif if (sz <= 0) { return NULL; } @@ -2663,10 +2667,15 @@ static ScpDir* ScpNewDir(void *fs, const char* path, void* heap) int ScpPushDir(void *fs, ScpSendCtx* ctx, const char* path, void* heap) { ScpDir* entry; + word32 pathSz; if (ctx == NULL || path == NULL) return WS_BAD_ARGUMENT; + pathSz = (word32)WSTRLEN(path); + if (pathSz >= sizeof(ctx->dirName)) + return WS_BUFFER_E; + entry = ScpNewDir(fs, path, heap); if (entry == NULL) { return WS_FATAL_ERROR; @@ -2680,9 +2689,9 @@ int ScpPushDir(void *fs, ScpSendCtx* ctx, const char* path, void* heap) ctx->currentDir = entry; } - /* append directory name to ctx->dirName */ - WSTRNCPY(ctx->dirName, path, DEFAULT_SCP_FILE_NAME_SZ-1); - ctx->dirName[DEFAULT_SCP_FILE_NAME_SZ-1] = '\0'; + /* append directory name to ctx->dirName, terminator included; the guard + * above bounds pathSz so the copy always fits */ + WMEMCPY(ctx->dirName, path, pathSz + 1); return WS_SUCCESS; } diff --git a/tests/api.c b/tests/api.c index 54959cbd3..c4673e606 100644 --- a/tests/api.c +++ b/tests/api.c @@ -1502,6 +1502,20 @@ static void test_wolfSSH_agent_signrequest_success(void) #if defined(WOLFSSH_SFTP) && !defined(NO_WOLFSSH_CLIENT) && \ !defined(SINGLE_THREADED) +/* A peer that is already gone at shutdown is not a test failure. The send path + * reports the reset as the return value; the recv path returns the generic + * WS_ERROR and stashes the specific code in wolfSSH_get_error. */ +static int AbsorbBenignReset(WOLFSSH* ssh, int ret) +{ + if (ret == WS_SOCKET_ERROR_E || + (ret == WS_ERROR && + wolfSSH_get_error(ssh) == WS_SOCKET_ERROR_E)) { + ret = WS_SUCCESS; + } + + return ret; +} + byte userPassword[256]; static int sftpUserAuth(byte authType, WS_UserAuthData* authData, void* ctx) @@ -1635,10 +1649,8 @@ static void test_wolfSSH_SFTP_SendReadPacket(void) argsCount = 0; args[argsCount++] = "."; args[argsCount++] = "-1"; -#ifndef USE_WINDOWS_API args[argsCount++] = "-p"; args[argsCount++] = "0"; -#endif ser.argv = (char**)args; ser.argc = argsCount; ser.signal = &ready; @@ -1819,11 +1831,7 @@ static void test_wolfSSH_SFTP_SendReadPacket(void) 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. */ - argsCount = WS_SUCCESS; - } + argsCount = AbsorbBenignReset(ssh, wolfSSH_shutdown(ssh)); #if DEFAULT_HIGHWATER_MARK < 8000 if (argsCount == WS_REKEYING) { @@ -1845,6 +1853,7 @@ static void test_wolfSSH_SFTP_SendReadPacket(void) k_sleep(Z_TIMEOUT_TICKS(100)); #endif ThreadJoin(serThread); + FreeTcpReady(&ready); } @@ -1905,10 +1914,8 @@ static void test_wolfSSH_SFTP_PartialSend(void) argsCount = 0; args[argsCount++] = "."; args[argsCount++] = "-1"; -#ifndef USE_WINDOWS_API args[argsCount++] = "-p"; args[argsCount++] = "0"; -#endif ser.argv = (char**)args; ser.argc = argsCount; ser.signal = &ready; @@ -2094,10 +2101,7 @@ static void test_wolfSSH_SFTP_PartialSend(void) wolfSSH_worker(ssh, NULL); } - argsCount = wolfSSH_shutdown(ssh); - if (argsCount == WS_SOCKET_ERROR_E) { - argsCount = WS_SUCCESS; - } + argsCount = AbsorbBenignReset(ssh, wolfSSH_shutdown(ssh)); #if DEFAULT_HIGHWATER_MARK < 8000 if (argsCount == WS_REKEYING) { argsCount = WS_SUCCESS; @@ -2114,6 +2118,7 @@ static void test_wolfSSH_SFTP_PartialSend(void) k_sleep(Z_TIMEOUT_TICKS(100)); #endif ThreadJoin(serThread); + FreeTcpReady(&ready); #endif /* WOLFSSH_TEST_INTERNAL */ } @@ -2145,10 +2150,8 @@ static void sftp_rekey_test(int nonBlock) argsCount = 0; args[argsCount++] = "."; args[argsCount++] = "-1"; -#ifndef USE_WINDOWS_API args[argsCount++] = "-p"; args[argsCount++] = "0"; -#endif ser.argv = (char**)args; ser.argc = argsCount; ser.signal = &ready; @@ -2232,9 +2235,7 @@ static void sftp_rekey_test(int nonBlock) * before the WS_REKEYING fixup below could otherwise mask it. The loop cap * is one above the assert threshold to leave last-iteration headroom. */ AssertIntLE(tries, SFTP_REKEY_MAX_TRIES); - if (argsCount == WS_SOCKET_ERROR_E) { - argsCount = WS_SUCCESS; - } + argsCount = AbsorbBenignReset(ssh, argsCount); #if DEFAULT_HIGHWATER_MARK < 8000 if (argsCount == WS_REKEYING) { /* in cases where highwater mark is really small a re-key could happen */ @@ -2252,6 +2253,7 @@ static void sftp_rekey_test(int nonBlock) k_sleep(Z_TIMEOUT_TICKS(100)); #endif ThreadJoin(serThread); + FreeTcpReady(&ready); } static void test_wolfSSH_SFTP_ReKey(void) @@ -2430,10 +2432,8 @@ static void test_wolfSSH_SFTP_Confinement(void) argsCount = 0; args[argsCount++] = "."; args[argsCount++] = "-1"; -#ifndef USE_WINDOWS_API args[argsCount++] = "-p"; args[argsCount++] = "0"; -#endif ser.argv = (char**)args; ser.argc = argsCount; ser.signal = &ready; @@ -2606,10 +2606,7 @@ static void test_wolfSSH_SFTP_Confinement(void) while (wolfSSH_get_error(ssh) == WS_REKEYING) wolfSSH_worker(ssh, NULL); - ret = wolfSSH_shutdown(ssh); - if (ret == WS_SOCKET_ERROR_E) { - ret = WS_SUCCESS; - } + ret = AbsorbBenignReset(ssh, wolfSSH_shutdown(ssh)); #if DEFAULT_HIGHWATER_MARK < 8000 if (ret == WS_REKEYING) { ret = WS_SUCCESS; @@ -2624,6 +2621,7 @@ static void test_wolfSSH_SFTP_Confinement(void) k_sleep(Z_TIMEOUT_TICKS(100)); #endif ThreadJoin(serThread); + FreeTcpReady(&ready); /* remove staged targets; escMkdir/escDest only exist if confinement * leaked, and inJailDir only if the positive-case RMDIR did not run, so @@ -2945,10 +2943,8 @@ static void scp_rekey_test(int nonBlock, int toServer) argsCount = 0; args[argsCount++] = "."; args[argsCount++] = "-1"; -#ifndef USE_WINDOWS_API args[argsCount++] = "-p"; args[argsCount++] = "0"; -#endif ser.argv = (char**)args; ser.argc = argsCount; ser.signal = &ready; @@ -3043,6 +3039,7 @@ static void scp_rekey_test(int nonBlock, int toServer) wolfSSH_free(ssh); wolfSSH_CTX_free(ctx); ThreadJoin(serThread); + FreeTcpReady(&ready); /* verify the transferred file matches the source once the server is done */ AssertIntEQ(scpFilesMatch(verifyName, fileData, SCP_REKEY_FILE_SZ), 0); @@ -3759,10 +3756,8 @@ static void test_wolfSSH_KeyboardInteractive(void) args[argsCount++] = "-1"; args[argsCount++] = "-i"; args[argsCount++] = "test:test"; -#ifndef USE_WINDOWS_API args[argsCount++] = "-p"; args[argsCount++] = "0"; -#endif ser.argv = (char**)args; ser.argc = argsCount; ser.signal = &ready; @@ -3775,11 +3770,7 @@ static void test_wolfSSH_KeyboardInteractive(void) AssertNotNull(ssh); - argsCount = wolfSSH_shutdown(ssh); - if (argsCount == WS_SOCKET_ERROR_E) { - /* If the socket is closed on shutdown, peer is gone, this is OK. */ - argsCount = WS_SUCCESS; - } + argsCount = AbsorbBenignReset(ssh, wolfSSH_shutdown(ssh)); #if DEFAULT_HIGHWATER_MARK < 8000 if (argsCount == WS_REKEYING) { @@ -3801,6 +3792,7 @@ static void test_wolfSSH_KeyboardInteractive(void) k_sleep(Z_TIMEOUT_TICKS(100)); #endif ThreadJoin(serThread); + FreeTcpReady(&ready); } #else /* WOLFSSH_SFTP && !NO_WOLFSSH_CLIENT && !SINGLE_THREADED */ @@ -3819,6 +3811,8 @@ int wolfSSH_ApiTest(int argc, char** argv) #ifdef WOLFSSH_TEST_BLOCK return 77; #else + WSTARTTCP(); + AssertIntEQ(wolfSSH_Init(), WS_SUCCESS); #if defined(FIPS_VERSION_GE) && FIPS_VERSION_GE(5,2) diff --git a/wolfssh/port.h b/wolfssh/port.h index 983818378..97f6d73b2 100644 --- a/wolfssh/port.h +++ b/wolfssh/port.h @@ -635,6 +635,14 @@ extern "C" { #endif #endif /* WSTRING_USER */ +/* Measures a formatted string's length; _snprintf_s rejects the NULL/0 probe + * form that WSNPRINTF maps to on Windows. Defined outside WSTRING_USER so + * custom string ports need not supply it, and left overridable. */ +#if defined(USE_WINDOWS_API) && !defined(WSCPRINTF) + #include + #define WSCPRINTF(f,...) _scprintf((f),##__VA_ARGS__) +#endif + /* get local time for debug print out */ #if defined(USE_WINDOWS_API) || defined(__MINGW32__) #define WTIME time diff --git a/wolfssh/test.h b/wolfssh/test.h index 0f22fd64c..dfea98c4e 100644 --- a/wolfssh/test.h +++ b/wolfssh/test.h @@ -676,7 +676,7 @@ static INLINE void tcp_listen(WS_SOCKET_T* sockfd, word16* port, int useAnyAddr) err_sys("tcp listen failed"); #endif - #if !defined(USE_WINDOWS_API) && !defined(WOLFSSL_TIRTOS) && !defined(WOLFSSL_NUCLEUS) + #if !defined(WOLFSSL_TIRTOS) && !defined(WOLFSSL_NUCLEUS) if (*port == 0) { socklen_t len = sizeof(addr); if (getsockname(*sockfd, (struct sockaddr*)&addr, &len) == 0) { @@ -826,6 +826,9 @@ typedef struct tcp_ready { #if defined(_POSIX_THREADS) && !defined(__MINGW32__) pthread_mutex_t mutex; pthread_cond_t cond; +#elif defined(USE_WINDOWS_API) + CRITICAL_SECTION cs; + HANDLE readyEvent; /* manual-reset event, set when ready */ #endif } tcp_ready; @@ -849,6 +852,14 @@ typedef struct func_args { #ifdef WOLFSSH_TEST_LOCKING +#if defined(USE_WINDOWS_API) && defined(NO_MAIN_DRIVER) && \ + !defined(SINGLE_THREADED) +/* Upper bound on the wait for the server to become ready. The signal fires + * right after the listen() so the real wait is sub-second; this generous + * bound only keeps a dead server from hanging the run. */ +#define TCP_READY_TIMEOUT_MS 30000 +#endif + static INLINE void InitTcpReady(tcp_ready* ready) { ready->ready = 0; @@ -858,6 +869,14 @@ static INLINE void InitTcpReady(tcp_ready* ready) !defined(__MINGW32__) && !defined(SINGLE_THREADED) pthread_mutex_init(&ready->mutex, NULL); pthread_cond_init(&ready->cond, NULL); +#elif defined(USE_WINDOWS_API) && defined(NO_MAIN_DRIVER) && \ + !defined(SINGLE_THREADED) + InitializeCriticalSection(&ready->cs); + ready->readyEvent = CreateEvent(NULL, TRUE, FALSE, NULL); + if (ready->readyEvent == NULL) { + DeleteCriticalSection(&ready->cs); + err_sys("Create tcp ready event failed"); + } #endif } @@ -868,6 +887,13 @@ static INLINE void FreeTcpReady(tcp_ready* ready) !defined(__MINGW32__) && !defined(SINGLE_THREADED) pthread_mutex_destroy(&ready->mutex); pthread_cond_destroy(&ready->cond); +#elif defined(USE_WINDOWS_API) && defined(NO_MAIN_DRIVER) && \ + !defined(SINGLE_THREADED) + if (ready->readyEvent != NULL) { + CloseHandle(ready->readyEvent); + ready->readyEvent = NULL; + } + DeleteCriticalSection(&ready->cs); #else WOLFSSH_UNUSED(ready); #endif @@ -891,6 +917,11 @@ static INLINE void WaitTcpReady(tcp_ready* ready) * seems to help. This is not ideal. (XXX) */ k_sleep(Z_TIMEOUT_TICKS(300)); #endif /* WOLFSSH_ZEPHYR */ +#elif defined(USE_WINDOWS_API) && defined(NO_MAIN_DRIVER) && \ + !defined(SINGLE_THREADED) + /* manual-reset event, so a late waiter still sees the ready state; on + * timeout fall through with port still 0 so the caller fails fast. */ + WaitForSingleObject(ready->readyEvent, TCP_READY_TIMEOUT_MS); #else WOLFSSH_UNUSED(ready); #endif