Run threaded api-test SFTP/SCP tests on Windows

Author: yosuke-wolfssl

.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).

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");
                             }

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);

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;
 }

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)

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 <stdio.h>
+    #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