From 593216020e52e9f00e075be6588e02dca10cb050 Mon Sep 17 00:00:00 2001 From: Hideki Miyazaki Date: Thu, 13 Aug 2026 13:02:00 -0400 Subject: [PATCH] fix Windows SFTP --- apps/wolfsshd/wolfsshd.c | 4 +- src/wolfsftp.c | 59 ++++++---- tests/regress.c | 238 +++++++++++++++++++++++++++++++++++---- wolfssh/wolfsftp.h | 7 +- 4 files changed, 262 insertions(+), 46 deletions(-) diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index 3a40029f..712af79d 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -3067,7 +3067,9 @@ static int StartSSHD(int argc, char** argv) if (ret == WS_SUCCESS) { for (i = 0; i < argc; i++) { - if (WSTRCMP((char*)(cmdArgs[i]), "-D") == 0) { + /* cmdArgs entries are wide strings (CommandLineToArgvW); compare + * as such instead of reinterpreting as narrow char data. */ + if (wcscmp(cmdArgs[i], L"-D") == 0) { isDaemon = 0; } } diff --git a/src/wolfsftp.c b/src/wolfsftp.c index d05146d4..b2d6dfc8 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -2339,6 +2339,32 @@ static void SFTP_HandleIdNext(WOLFSSH* ssh, word32 id[2]) #endif /* !NO_WOLFSSH_SERVER */ +#ifdef USE_WINDOWS_API +/* dwCreationDisposition takes one enumerated value, not a bitmask, so + * resolve CREAT/EXCL/TRUNC to a single disposition here. */ +static DWORD SFTP_WinCreationDisp(word32 reason) +{ + DWORD disp; + + if (reason & WOLFSSH_FXF_CREAT) { + if (reason & WOLFSSH_FXF_EXCL) + disp = CREATE_NEW; + else if (reason & WOLFSSH_FXF_TRUNC) + disp = CREATE_ALWAYS; + else + disp = OPEN_ALWAYS; + } + else { + if (reason & WOLFSSH_FXF_TRUNC) + disp = TRUNCATE_EXISTING; + else + disp = OPEN_EXISTING; + } + + return disp; +} +#endif /* USE_WINDOWS_API */ + /* Handles packet to open a file * * returns WS_SUCCESS on success @@ -2662,25 +2688,14 @@ cleanup: } #endif - if (reason & WOLFSSH_FXF_READ) { + if (reason & WOLFSSH_FXF_READ) desiredAccess |= GENERIC_READ; - creationDisp |= OPEN_EXISTING; - } - if (reason & WOLFSSH_FXF_WRITE) { + if (reason & WOLFSSH_FXF_WRITE) desiredAccess |= GENERIC_WRITE; - if (reason & WOLFSSH_FXF_CREAT) { - if (reason & WOLFSSH_FXF_TRUNC) - creationDisp = CREATE_ALWAYS; - else - creationDisp = OPEN_ALWAYS; - } - #if 0 - if (reason & WOLFSSH_FXF_EXCL) - creationDisp |= CREATE_NEW; - if (reason & WOLFSSH_FXF_APPEND) - desiredAccess |= FILE_APPEND_DATA; - #endif - } + if (reason & WOLFSSH_FXF_APPEND) + desiredAccess |= FILE_APPEND_DATA; + + creationDisp = SFTP_WinCreationDisp(reason); #if 0 /* if file permissions not set then use default */ @@ -6401,8 +6416,7 @@ int wolfSSH_SFTP_RecvFSetSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) #endif /* _WIN32_WCE */ -#if defined(WOLFSSH_TEST_INTERNAL) && !defined(USE_WINDOWS_API) && \ - !defined(NO_FILESYSTEM) +#if defined(WOLFSSH_TEST_INTERNAL) && !defined(NO_FILESYSTEM) /* Test-only plumbing for the forged-handle regression test in tests/regress.c. * * The SFTP request handlers buffer their status/handle reply into ssh->recvState @@ -6485,10 +6499,12 @@ int wolfSSH_SFTP_TestDirHandleCount(WOLFSSH* ssh) } #endif /* NO_WOLFSSH_DIR */ +#ifndef USE_WINDOWS_API /* Close the underlying descriptor of the head tracked file handle out of band, * leaving the node in the list with a now-stale fd. The next RecvClose on that * handle will see its close() fail, exercising the path that must still drop - * the handle from the tracking list. Returns WS_SUCCESS if a node was found. */ + * the handle from the tracking list. Returns WS_SUCCESS if a node was found. + * Not provided for Windows, where fd is a HANDLE, not a WCLOSE-able fd. */ int wolfSSH_SFTP_TestInvalidateHeadFd(WOLFSSH* ssh) { if (ssh == NULL || ssh->fileList == NULL) { @@ -6501,7 +6517,8 @@ int wolfSSH_SFTP_TestInvalidateHeadFd(WOLFSSH* ssh) #endif return WS_SUCCESS; } -#endif /* WOLFSSH_TEST_INTERNAL && !USE_WINDOWS_API && !NO_FILESYSTEM */ +#endif /* !USE_WINDOWS_API */ +#endif /* WOLFSSH_TEST_INTERNAL && !NO_FILESYSTEM */ #endif /* !NO_WOLFSSH_SERVER */ diff --git a/tests/regress.c b/tests/regress.c index 3f09b397..bda8a37e 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -7624,8 +7624,7 @@ static void TestSftpBufferSendPendingOutput(void) wolfSSH_CTX_free(ctx); } -#if !defined(NO_WOLFSSH_SERVER) && !defined(USE_WINDOWS_API) && \ - !defined(NO_FILESYSTEM) +#if !defined(NO_WOLFSSH_SERVER) && !defined(NO_FILESYSTEM) /* Write a big-endian uint32 (the SFTP wire encoding). */ static void SftpPutU32(word32 val, byte* out) { @@ -7642,6 +7641,28 @@ static word32 SftpGetU32(const byte* in) ((word32)in[2] << 8) | (word32)in[3]; } +/* A refused request must still answer the peer with an FXP_STATUS carrying the + * expected code. Asserting only that the call returned non-success would not + * catch a refusal that dropped the reply and left the session hung. + * The request id is checked too: TestRecvReply returns whatever is currently + * buffered, so a handler that dropped its reply would otherwise pass here by + * re-presenting the previous request's status. */ +static void AssertSftpStatusReply(WOLFSSH* ssh, int reqId, word32 code) +{ + const byte* reply; + word32 replySz; + + reply = wolfSSH_SFTP_TestRecvReply(ssh, &replySz); + AssertNotNull(reply); + AssertTrue(replySz >= WOLFSSH_SFTP_HEADER + UINT32_SZ); + AssertIntEQ(reply[LENGTH_SZ], WOLFSSH_FTP_STATUS); + AssertIntEQ((int)SftpGetU32(reply + LENGTH_SZ + MSG_ID_SZ), reqId); + AssertIntEQ((int)SftpGetU32(reply + WOLFSSH_SFTP_HEADER), (int)code); +} +#endif /* !NO_WOLFSSH_SERVER && !NO_FILESYSTEM */ + +#if !defined(NO_WOLFSSH_SERVER) && !defined(USE_WINDOWS_API) && \ + !defined(NO_FILESYSTEM) /* Return 1 if needle occurs in haystack, 0 otherwise. */ static int SftpBufContains(const byte* hay, word32 haySz, const byte* needle, word32 needleSz) @@ -8004,25 +8025,6 @@ static void TestSftpHandleNamespaceIsolation(void) } #endif /* NO_WOLFSSH_DIR */ -/* A refused request must still answer the peer with an FXP_STATUS carrying the - * expected code. Asserting only that the call returned non-success would not - * catch a refusal that dropped the reply and left the session hung. - * The request id is checked too: TestRecvReply returns whatever is currently - * buffered, so a handler that dropped its reply would otherwise pass here by - * re-presenting the previous request's status. */ -static void AssertSftpStatusReply(WOLFSSH* ssh, int reqId, word32 code) -{ - const byte* reply; - word32 replySz; - - reply = wolfSSH_SFTP_TestRecvReply(ssh, &replySz); - AssertNotNull(reply); - AssertTrue(replySz >= WOLFSSH_SFTP_HEADER + UINT32_SZ); - AssertIntEQ(reply[LENGTH_SZ], WOLFSSH_FTP_STATUS); - AssertIntEQ((int)SftpGetU32(reply + LENGTH_SZ + MSG_ID_SZ), reqId); - AssertIntEQ((int)SftpGetU32(reply + WOLFSSH_SFTP_HEADER), (int)code); -} - /* The per-session open-file-handle count is capped at WOLFSSH_MAX_SFTP_HANDLES * to bound memory and keep the linear handle lookup from becoming a CPU DoS * vector. Open exactly the cap's worth of handles (all must succeed), confirm @@ -8736,6 +8738,195 @@ static void TestSftpStartPathInsideConfineRoot(void) #endif /* !NO_WOLFSSH_SERVER && !USE_WINDOWS_API && !NO_FILESYSTEM */ +#if !defined(NO_WOLFSSH_SERVER) && defined(USE_WINDOWS_API) && \ + !defined(NO_FILESYSTEM) +/* Walks the RecvOpen CREAT/EXCL/TRUNC flag matrix on Windows, checking both + * the open result and the resulting file state for each case. */ +static void TestSftpWindowsOpenFlagMatrix(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + int rid = 500; + int reqId; + word32 idx; + word32 replySz; + const byte* reply; + const word32 hOff = WOLFSSH_SFTP_HEADER + UINT32_SZ; /* handle in reply */ + WSTAT_T st; + byte handle[WOLFSSH_HANDLE_ID_SZ]; + byte pkt[256]; + char cwd[WOLFSSH_MAX_FILENAME]; + char path[64]; + word32 pathSz; + const char content[] = "0123456789"; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + AssertNotNull(ctx); + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AssertIntEQ(wolfSSH_SFTP_TestRecvStateInit(ssh), WS_SUCCESS); + + /* unique per-process fixture name so parallel runs don't collide */ + WSNPRINTF(path, sizeof(path), "wolfssh_winflags_%lu.tmp", + (unsigned long)GetCurrentProcessId()); + pathSz = (word32)WSTRLEN(path); + + WMEMSET(cwd, 0, sizeof(cwd)); + AssertNotNull(WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1)); + AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, cwd), WS_SUCCESS); + + (void)WREMOVE(ssh->fs, path); + + /* WRITE only, no CREAT: must fail against a missing file, untouched. */ + idx = 0; + SftpPutU32(pathSz, pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, path, pathSz); idx += pathSz; + SftpPutU32(WOLFSSH_FXF_WRITE, pkt + idx); idx += UINT32_SZ; + SftpPutU32(0, pkt + idx); idx += UINT32_SZ; + reqId = rid++; + AssertTrue(wolfSSH_SFTP_RecvOpen(ssh, reqId, pkt, idx) != WS_SUCCESS); + AssertSftpStatusReply(ssh, reqId, WOLFSSH_FTP_FAILURE); + AssertTrue(WSTAT(ssh->fs, path, &st) != 0); + + /* WRITE|CREAT, no TRUNC: must create the missing file (OPEN_ALWAYS). */ + idx = 0; + SftpPutU32(pathSz, pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, path, pathSz); idx += pathSz; + SftpPutU32(WOLFSSH_FXF_WRITE | WOLFSSH_FXF_CREAT, pkt + idx); + idx += UINT32_SZ; + SftpPutU32(0, pkt + idx); idx += UINT32_SZ; + AssertIntEQ(wolfSSH_SFTP_RecvOpen(ssh, rid++, pkt, idx), WS_SUCCESS); + reply = wolfSSH_SFTP_TestRecvReply(ssh, &replySz); + AssertNotNull(reply); + AssertTrue(replySz >= hOff + WOLFSSH_HANDLE_ID_SZ); + WMEMCPY(handle, reply + hOff, WOLFSSH_HANDLE_ID_SZ); + + /* seed content through the handle just opened */ + idx = 0; + SftpPutU32(WOLFSSH_HANDLE_ID_SZ, pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, handle, WOLFSSH_HANDLE_ID_SZ); + idx += WOLFSSH_HANDLE_ID_SZ; + SftpPutU32(0, pkt + idx); idx += UINT32_SZ; /* offset hi */ + SftpPutU32(0, pkt + idx); idx += UINT32_SZ; /* offset lo */ + SftpPutU32((word32)(sizeof(content) - 1), pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, content, sizeof(content) - 1); + idx += (word32)(sizeof(content) - 1); + AssertIntEQ(wolfSSH_SFTP_RecvWrite(ssh, rid++, pkt, idx), WS_SUCCESS); + + idx = 0; + SftpPutU32(WOLFSSH_HANDLE_ID_SZ, pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, handle, WOLFSSH_HANDLE_ID_SZ); + idx += WOLFSSH_HANDLE_ID_SZ; + AssertIntEQ(wolfSSH_SFTP_RecvClose(ssh, rid++, pkt, idx), WS_SUCCESS); + + AssertIntEQ(WSTAT(ssh->fs, path, &st), 0); + AssertIntEQ((int)st.st_size, (int)(sizeof(content) - 1)); + + /* WRITE|CREAT, no TRUNC, on the existing file: must not truncate it. */ + idx = 0; + SftpPutU32(pathSz, pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, path, pathSz); idx += pathSz; + SftpPutU32(WOLFSSH_FXF_WRITE | WOLFSSH_FXF_CREAT, pkt + idx); + idx += UINT32_SZ; + SftpPutU32(0, pkt + idx); idx += UINT32_SZ; + AssertIntEQ(wolfSSH_SFTP_RecvOpen(ssh, rid++, pkt, idx), WS_SUCCESS); + reply = wolfSSH_SFTP_TestRecvReply(ssh, &replySz); + AssertNotNull(reply); + AssertTrue(replySz >= hOff + WOLFSSH_HANDLE_ID_SZ); + WMEMCPY(handle, reply + hOff, WOLFSSH_HANDLE_ID_SZ); + + AssertIntEQ(WSTAT(ssh->fs, path, &st), 0); + AssertIntEQ((int)st.st_size, (int)(sizeof(content) - 1)); + + idx = 0; + SftpPutU32(WOLFSSH_HANDLE_ID_SZ, pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, handle, WOLFSSH_HANDLE_ID_SZ); + idx += WOLFSSH_HANDLE_ID_SZ; + AssertIntEQ(wolfSSH_SFTP_RecvClose(ssh, rid++, pkt, idx), WS_SUCCESS); + + /* WRITE|CREAT|TRUNC: must truncate the existing file immediately. */ + idx = 0; + SftpPutU32(pathSz, pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, path, pathSz); idx += pathSz; + SftpPutU32(WOLFSSH_FXF_WRITE | WOLFSSH_FXF_CREAT | WOLFSSH_FXF_TRUNC, + pkt + idx); idx += UINT32_SZ; + SftpPutU32(0, pkt + idx); idx += UINT32_SZ; + AssertIntEQ(wolfSSH_SFTP_RecvOpen(ssh, rid++, pkt, idx), WS_SUCCESS); + reply = wolfSSH_SFTP_TestRecvReply(ssh, &replySz); + AssertNotNull(reply); + AssertTrue(replySz >= hOff + WOLFSSH_HANDLE_ID_SZ); + WMEMCPY(handle, reply + hOff, WOLFSSH_HANDLE_ID_SZ); + + AssertIntEQ(WSTAT(ssh->fs, path, &st), 0); + AssertIntEQ((int)st.st_size, 0); + + idx = 0; + SftpPutU32(WOLFSSH_HANDLE_ID_SZ, pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, handle, WOLFSSH_HANDLE_ID_SZ); + idx += WOLFSSH_HANDLE_ID_SZ; + AssertIntEQ(wolfSSH_SFTP_RecvClose(ssh, rid++, pkt, idx), WS_SUCCESS); + + /* WRITE|CREAT|EXCL against the existing file: must fail. */ + idx = 0; + SftpPutU32(pathSz, pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, path, pathSz); idx += pathSz; + SftpPutU32(WOLFSSH_FXF_WRITE | WOLFSSH_FXF_CREAT | WOLFSSH_FXF_EXCL, + pkt + idx); idx += UINT32_SZ; + SftpPutU32(0, pkt + idx); idx += UINT32_SZ; + reqId = rid++; + AssertTrue(wolfSSH_SFTP_RecvOpen(ssh, reqId, pkt, idx) != WS_SUCCESS); + AssertSftpStatusReply(ssh, reqId, WOLFSSH_FTP_FAILURE); + + (void)WREMOVE(ssh->fs, path); + + /* WRITE|CREAT|EXCL against a missing path: must succeed. */ + idx = 0; + SftpPutU32(pathSz, pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, path, pathSz); idx += pathSz; + SftpPutU32(WOLFSSH_FXF_WRITE | WOLFSSH_FXF_CREAT | WOLFSSH_FXF_EXCL, + pkt + idx); idx += UINT32_SZ; + SftpPutU32(0, pkt + idx); idx += UINT32_SZ; + AssertIntEQ(wolfSSH_SFTP_RecvOpen(ssh, rid++, pkt, idx), WS_SUCCESS); + reply = wolfSSH_SFTP_TestRecvReply(ssh, &replySz); + AssertNotNull(reply); + AssertTrue(replySz >= hOff + WOLFSSH_HANDLE_ID_SZ); + WMEMCPY(handle, reply + hOff, WOLFSSH_HANDLE_ID_SZ); + + idx = 0; + SftpPutU32(WOLFSSH_HANDLE_ID_SZ, pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, handle, WOLFSSH_HANDLE_ID_SZ); + idx += WOLFSSH_HANDLE_ID_SZ; + AssertIntEQ(wolfSSH_SFTP_RecvClose(ssh, rid++, pkt, idx), WS_SUCCESS); + + (void)WREMOVE(ssh->fs, path); + + /* READ|WRITE|CREAT, no TRUNC, against a missing path: must create it. */ + idx = 0; + SftpPutU32(pathSz, pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, path, pathSz); idx += pathSz; + SftpPutU32(WOLFSSH_FXF_READ | WOLFSSH_FXF_WRITE | WOLFSSH_FXF_CREAT, + pkt + idx); idx += UINT32_SZ; + SftpPutU32(0, pkt + idx); idx += UINT32_SZ; + AssertIntEQ(wolfSSH_SFTP_RecvOpen(ssh, rid++, pkt, idx), WS_SUCCESS); + reply = wolfSSH_SFTP_TestRecvReply(ssh, &replySz); + AssertNotNull(reply); + AssertTrue(replySz >= hOff + WOLFSSH_HANDLE_ID_SZ); + WMEMCPY(handle, reply + hOff, WOLFSSH_HANDLE_ID_SZ); + AssertIntEQ(WSTAT(ssh->fs, path, &st), 0); + + idx = 0; + SftpPutU32(WOLFSSH_HANDLE_ID_SZ, pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, handle, WOLFSSH_HANDLE_ID_SZ); + idx += WOLFSSH_HANDLE_ID_SZ; + AssertIntEQ(wolfSSH_SFTP_RecvClose(ssh, rid++, pkt, idx), WS_SUCCESS); + + (void)WREMOVE(ssh->fs, path); + wolfSSH_SFTP_TestRecvStateFree(ssh); + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} +#endif /* !NO_WOLFSSH_SERVER && USE_WINDOWS_API && !NO_FILESYSTEM */ + #if defined(WOLFSSL_NUCLEUS) && !defined(NO_WOLFSSH_MKTIME) static void TestNucleusMonthConversion(void) { @@ -11580,6 +11771,11 @@ int main(int argc, char** argv) /* SETSTAT/FSETSTAT apply the attributes they acknowledge */ TestSftpSetStatAttributes(); #endif + #if !defined(NO_WOLFSSH_SERVER) && defined(USE_WINDOWS_API) && \ + !defined(NO_FILESYSTEM) + /* RecvOpen's Windows open-flag matrix */ + TestSftpWindowsOpenFlagMatrix(); + #endif #if defined(WOLFSSL_NUCLEUS) && !defined(NO_WOLFSSH_MKTIME) TestNucleusMonthConversion(); #endif diff --git a/wolfssh/wolfsftp.h b/wolfssh/wolfsftp.h index a7ae5466..676912aa 100644 --- a/wolfssh/wolfsftp.h +++ b/wolfssh/wolfsftp.h @@ -356,8 +356,7 @@ WOLFSSH_LOCAL void wolfSSH_SFTP_ShowSizes(void); word32* handleSz); WOLFSSH_API int wolfSSH_TestSftpSendCap(WOLFSSH* ssh, word32 cap); WOLFSSH_API int wolfSSH_TestSftpStallPending(WOLFSSH* ssh, word32 count); - #if !defined(NO_WOLFSSH_SERVER) && !defined(USE_WINDOWS_API) && \ - !defined(NO_FILESYSTEM) + #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, word32* sz); @@ -366,7 +365,9 @@ WOLFSSH_LOCAL void wolfSSH_SFTP_ShowSizes(void); #ifndef NO_WOLFSSH_DIR WOLFSSH_API int wolfSSH_SFTP_TestDirHandleCount(WOLFSSH* ssh); #endif - WOLFSSH_API int wolfSSH_SFTP_TestInvalidateHeadFd(WOLFSSH* ssh); + #ifndef USE_WINDOWS_API + WOLFSSH_API int wolfSSH_SFTP_TestInvalidateHeadFd(WOLFSSH* ssh); + #endif #endif #if defined(WOLFSSL_NUCLEUS) && !defined(NO_WOLFSSH_MKTIME) WOLFSSH_API int wolfSSH_TestNucleusMonthFromDate(word16 d);