diff --git a/.github/workflows/windows-check.yml b/.github/workflows/windows-check.yml index aed2420da..2f65cdb58 100644 --- a/.github/workflows/windows-check.yml +++ b/.github/workflows/windows-check.yml @@ -88,6 +88,12 @@ jobs: timeout-minutes: 2 run: .\sshd_login_grace_test.ps1 -SshdExe "$env:SSHD_PATH" + - name: Test wolfsshd -D option parsing on Windows + working-directory: ${{ github.workspace }}\wolfssh\apps\wolfsshd\test + shell: pwsh + timeout-minutes: 2 + run: .\sshd_dash_d_test.ps1 -SshdExe "$env:SSHD_PATH" + # Build and run the self-contained unit tests with the MSVC AddressSanitizer. # This is the only job that executes wolfSSH tests under a sanitizer on # Windows, where the USE_WINDOWS_API console code (e.g. wolfSSH_DoOSC) is @@ -220,3 +226,90 @@ jobs: if ($LASTEXITCODE -ne 0) { throw "$t failed under ASAN (exit $LASTEXITCODE)" } } + # Build and run the autotools regression tests under MSYS2 MinGW64. MinGW + # defines _WIN32, so wolfssh/settings.h turns on USE_WINDOWS_API and the + # Windows-only coverage in tests/regress.c (TestSftpWindowsOpenFlagMatrix, + # which walks the RecvOpen CREAT/EXCL/TRUNC/APPEND matrix against the + # CreateFile() disposition table) actually compiles and runs. The MSVC + # solution has no regress project and regress.c does not build with cl (it + # uses arpa/inet.h and unistd.h), so without this job that matrix runs in no + # CI at all. wolfsshd is left out of the build: its autotools path is not + # MinGW-clean and the MSVC solution already covers it. + mingw-regress: + name: MSYS2 MinGW64 regression tests + runs-on: windows-latest + timeout-minutes: 40 + + # Pinned rather than left on wolfssl/wolfssl's default branch, so an + # upstream wolfSSL change can't silently break this job. Matches the + # ref singlethread-check.yml and x509-interop.yml already pin to. + env: + WOLFSSL_REF: v5.9.1-stable + + defaults: + run: + shell: msys2 {0} + + steps: + - name: Set up MSYS2 MinGW64 + uses: msys2/setup-msys2@v2 + with: + msystem: MINGW64 + update: false + install: >- + base-devel + autotools + git + mingw-w64-x86_64-gcc + mingw-w64-x86_64-pkgconf + + - name: Checkout wolfssl + uses: actions/checkout@v4 + with: + repository: wolfssl/wolfssl + ref: ${{ env.WOLFSSL_REF }} + path: wolfssl + + - name: Build and install wolfssl + working-directory: wolfssl + run: | + ./autogen.sh + # --enable-all pulls in --enable-crl-monitor, which wolfSSL's configure + # rejects on MinGW (it is limited to linux, OS X, and freebsd). Turn it + # back off explicitly; wolfSSH does not use the CRL monitor. + ./configure --enable-all --disable-crl-monitor \ + --enable-static --disable-shared \ + --prefix="$HOME/wolfssl-install" + make -j$(nproc) + make install + + - name: Checkout wolfssh + uses: actions/checkout@v4 + with: + path: wolfssh + + - name: Build and run the regression tests + working-directory: wolfssh + run: | + # Run autoreconf directly instead of ./autogen.sh: for a git checkout + # autogen.sh exports WARNINGS="all,error", turning autotools warnings + # into errors that the MSYS2 automake can trip on. + autoreconf -ivf + # wolfssl is a static archive here, so its Windows socket (ws2_32) and + # certificate store (crypt32) references are only resolved when this + # configure's own AC_CHECK_LIB and later link steps pull them in too. + ./configure --enable-sftp \ + CPPFLAGS="-I$HOME/wolfssl-install/include" \ + LDFLAGS="-L$HOME/wolfssl-install/lib" \ + LIBS="-lws2_32 -lcrypt32" + # MinGW's EXEEXT is ".exe", so the check_PROGRAMS targets automake + # generates are tests/regress.test.exe and tests/unit.test.exe, not + # the extension-less names make would use on a POSIX host. + make -j$(nproc) tests/regress.test.exe tests/unit.test.exe + ./tests/regress.test.exe + ./tests/unit.test.exe + + - name: Show config.log on failure + if: failure() + working-directory: wolfssh + run: cat config.log diff --git a/apps/wolfssh/common.c b/apps/wolfssh/common.c index fd5c1d57d..0092fd8c9 100644 --- a/apps/wolfssh/common.c +++ b/apps/wolfssh/common.c @@ -257,7 +257,10 @@ static int AppendKeyToFile(const char* filename, const char* name, if (ret == WS_SUCCESS) { const int needsNewline = AppendNeedsNewline(filename); - ret = WFOPEN(NULL, &f, filename, "a"); + /* Binary mode: text mode would translate '\n' to CRLF on Windows, + * and known_hosts is conventionally LF-terminated regardless of + * platform. */ + ret = WFOPEN(NULL, &f, filename, "ab"); if (ret == 0 && f != WBADFILE) { /* Check the write and the close so a failed or truncated entry * (for example on a full disk) is reported rather than appearing @@ -721,7 +724,6 @@ int ClientSetEcho(int type) #else static int echoInit = 0; static DWORD originalTerm; - static CONSOLE_SCREEN_BUFFER_INFO screenOrig; HANDLE stdinHandle = GetStdHandle(STD_INPUT_HANDLE); if (!echoInit) { if (GetConsoleMode(stdinHandle, &originalTerm) == 0) { diff --git a/apps/wolfsshd/test/sshd_dash_d_test.ps1 b/apps/wolfsshd/test/sshd_dash_d_test.ps1 new file mode 100644 index 000000000..9be75fe4c --- /dev/null +++ b/apps/wolfsshd/test/sshd_dash_d_test.ps1 @@ -0,0 +1,115 @@ +#!/usr/bin/env pwsh +# +# Windows regression test for wolfsshd -D (foreground) option parsing. +# +# On Windows, StartSSHD() rebuilds argv from GetCommandLineW(); a bug in that +# path left -D (foreground) mode parsing the raw wide command line, so -f and +# -p were silently ignored and the daemon fell back to its compiled-in +# defaults. This test starts wolfsshd with -D and a config file at a +# non-default path whose Port line differs from the -p value, then checks that +# the listener comes up on the -p port. That only happens if -D mode parsed +# both -f (to find the config) and -p (to override the config's Port). +# +# No Windows user account or authorized key is required: the check is that the +# daemon binds the requested port, not that a session authenticates. +# +# Usage: +# pwsh sshd_dash_d_test.ps1 -SshdExe [-Port N] [-ConfPort N] +# (SshdExe also accepts the SSHD_PATH environment variable.) + +param( + [string]$SshdExe = $env:SSHD_PATH, + [int]$Port = 22335, + [int]$ConfPort = 22336 +) + +$ErrorActionPreference = "Stop" +$exitCode = 1 + +$scriptDir = Split-Path -Parent $MyInvocation.MyCommand.Path +$repoRoot = (Resolve-Path (Join-Path $scriptDir "..\..\..")).Path +$keyPath = (Resolve-Path (Join-Path $repoRoot "keys\server-key.pem")).Path +$confFile = Join-Path $scriptDir "sshd_config_test_dash_d" +$authFile = Join-Path $scriptDir "authorized_keys_test_dash_d" + +if (-not $SshdExe -or -not (Test-Path $SshdExe)) { + Write-Host "ERROR: wolfsshd.exe not found (pass -SshdExe or set SSHD_PATH)" + exit 1 +} + +if ($Port -eq $ConfPort) { + Write-Host "ERROR: -Port and -ConfPort must differ so the test can tell them apart" + exit 1 +} + +# The config's Port is deliberately not the port we probe. If -p is parsed it +# wins (wolfsshd only reads the config Port when none was given on the command +# line), so a listener on $Port proves the -p override took effect. +@" +Port $ConfPort +Protocol 2 +PermitRootLogin yes +PasswordAuthentication yes +UseDNS no +HostKey $keyPath +AuthorizedKeysFile $authFile +"@ | Out-File -FilePath $confFile -Encoding ASCII + +"" | Out-File -FilePath $authFile -Encoding ASCII + +# -D selects the non-service (foreground) path on Windows. +$sshd = Start-Process -FilePath $SshdExe ` + -ArgumentList "-D", "-f", "`"$confFile`"", "-p", "$Port" ` + -NoNewWindow -PassThru + +try { + $up = $false + for ($i = 0; $i -lt 20; $i++) { + if ($sshd.HasExited) { + throw "wolfsshd exited early (code $($sshd.ExitCode)); -D option parsing likely failed" + } + try { + $probe = New-Object System.Net.Sockets.TcpClient + $probe.Connect("127.0.0.1", $Port) + $probe.Close() + $up = $true + break + } + catch { + Start-Sleep -Milliseconds 500 + } + } + if (-not $up) { + throw "wolfsshd did not listen on the -p port $Port; -D did not honor -f/-p" + } + + # The config Port must not have been used: nothing should answer there. + $confBound = $false + try { + $probe = New-Object System.Net.Sockets.TcpClient + $probe.Connect("127.0.0.1", $ConfPort) + $probe.Close() + $confBound = $true + } + catch { + # expected: no listener on the config Port + } + if ($confBound) { + throw "wolfsshd listened on the config Port $ConfPort; -p override was not applied" + } + + Write-Host "PASS: -D mode parsed -f and -p (listening on $Port, not $ConfPort)" + $exitCode = 0 +} +catch { + Write-Host "FAIL: $_" + $exitCode = 1 +} +finally { + if ($sshd -and -not $sshd.HasExited) { + Stop-Process -Id $sshd.Id -Force -ErrorAction SilentlyContinue + } + Remove-Item -Path $confFile, $authFile -Force -ErrorAction SilentlyContinue +} + +exit $exitCode diff --git a/src/internal.c b/src/internal.c index 10769aef4..c63d54165 100644 --- a/src/internal.c +++ b/src/internal.c @@ -22616,6 +22616,9 @@ int wolfSSH_CleanPath(WOLFSSH* ssh, char* in, int inSz) byte found; char *path; void *heap = NULL; +#if defined(WOLFSSL_NUCLEUS) || defined(USE_WINDOWS_API) + int j; +#endif if (in == NULL || inSz <= 0) { return WS_BAD_ARGUMENT; @@ -22715,19 +22718,16 @@ int wolfSSH_CleanPath(WOLFSSH* ssh, char* in, int inSz) } /* clean up any multiple drive listed i.e. A:/A: */ - { - int i,j; - sz = (long)WSTRLEN(path); - for (i = 0, j = 0; i < sz; i++) { - if (path[i] == ':') { - if (j == 0) j = i; - else { - /* @TODO only checking once */ - WMEMMOVE(path, path + i - WS_DRIVE_SIZE, - sz - i + WS_DRIVE_SIZE); - path[sz - i + WS_DRIVE_SIZE] = '\0'; - break; - } + sz = (long)WSTRLEN(path); + for (i = 0, j = 0; i < sz; i++) { + if (path[i] == ':') { + if (j == 0) j = i; + else { + /* @TODO only checking once */ + WMEMMOVE(path, path + i - WS_DRIVE_SIZE, + sz - i + WS_DRIVE_SIZE); + path[sz - i + WS_DRIVE_SIZE] = '\0'; + break; } } } diff --git a/src/port.c b/src/port.c index 077791f24..6886f77c1 100644 --- a/src/port.c +++ b/src/port.c @@ -925,4 +925,32 @@ char* wstrncat(char* s1, const char* s2, size_t n) return NULL; } + +#ifdef USE_WINDOWS_API +/* strsep() equivalent for platforms whose C library does not provide the + * BSD extension (MSVCRT/MinGW). Splits *s1 on the first character found + * in delim, NUL-terminates the token in place, and advances *s1 past it + * (NULL when no delimiter remains). Returns the start of the token, or + * NULL if *s1 was already NULL. */ +char* wstrsep(char** s1, const char* delim) +{ + char* start = *s1; + char* p; + + if (start == NULL) + return NULL; + + for (p = start; *p != '\0'; p++) { + if (WSTRCHR(delim, *p) != NULL) { + *p = '\0'; + *s1 = p + 1; + return start; + } + } + + *s1 = NULL; + return start; +} +#endif /* USE_WINDOWS_API */ + #endif /* WSTRING_USER */ diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 88cca98f8..3f1d48246 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -2622,7 +2622,6 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) word32 idx = 0; DWORD desiredAccess = 0; DWORD creationDisp = 0; - DWORD flagsAndAttrs = 0; int ret = WS_SUCCESS; int rc; int fileHandleOpened = 0; @@ -3037,7 +3036,7 @@ int wolfSSH_SFTP_RecvOpenDir(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) DWORD drives, mask; UINT driveType; char driveName[] = " :\\"; - int i; + word32 i; WMEMSET(ssh->driveList, 0, sizeof ssh->driveList); ssh->driveListCount = 0; @@ -10401,11 +10400,16 @@ int wolfSSH_SFTP_Put(WOLFSSH* ssh, char* from, char* to, byte resume, break; /* either at end of file or error */ } #else /* USE_WINDOWS_API */ + /* ReadFile() wants a DWORD* out param; state->rSz is + * an int shared with the WFREAD() branch above. */ + DWORD wRSz = 0; + if (ReadFile(state->fileHandle, state->r, - WOLFSSH_MAX_SFTP_RW, &state->rSz, + WOLFSSH_MAX_SFTP_RW, &wRSz, &state->offset) == 0) { break; /* either at end of file or error */ } + state->rSz = (int)wRSz; #endif /* USE_WINDOWS_API */ } sz = wolfSSH_SFTP_SendWritePacket(ssh, diff --git a/src/wolfterm.c b/src/wolfterm.c index 17512d0fe..786cee4d0 100644 --- a/src/wolfterm.c +++ b/src/wolfterm.c @@ -374,6 +374,8 @@ static int wolfSSH_DoOSC(WOLFSSH* ssh, WOLFSSH_HANDLE handle, byte* buf, * not saved to escBuf and escState is never set to WS_ESC_OSC, so there is * no resume path. Returning WS_SUCCESS lets the caller advance past the * sequence and reset escState cleanly. */ + WOLFSSH_UNUSED(handle); + if (*idx >= bufSz) { /* missing the OSC command byte, drop the sequence */ return WS_SUCCESS; diff --git a/tests/regress.c b/tests/regress.c index eca4fd729..aacc173d9 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -32,7 +32,11 @@ #include #include -#include +#ifndef _WIN32 + #include +#else + #include +#endif #include #include #include @@ -7935,6 +7939,13 @@ static void TestClientBuffersIdempotent(void) } #endif +/* Windows has no /dev/null; the null device there is "NUL". */ +#ifdef USE_WINDOWS_API + #define TEST_NULL_DEVICE "NUL" +#else + #define TEST_NULL_DEVICE "/dev/null" +#endif + /* Simulate Ctrl+D (stdin EOF) during password prompt; expect failure but no crash. */ static void TestPasswordEofNoCrash(void) { @@ -7949,7 +7960,7 @@ static void TestPasswordEofNoCrash(void) savedStdin = dup(STDIN_FILENO); AssertTrue(savedStdin >= 0); - devNull = open("/dev/null", O_RDONLY); + devNull = open(TEST_NULL_DEVICE, O_RDONLY); AssertTrue(devNull >= 0); AssertTrue(dup2(devNull, STDIN_FILENO) >= 0); @@ -11914,6 +11925,22 @@ static int KnownHostsCheckCapture(const byte* pubKey, word32 pubKeySz, } +/* setenv()/unsetenv() are POSIX and have no MSVCRT equivalent; _putenv_s() + * matches their (name, value) shape and success/failure return closely + * enough for this test's own HOME juggling. WMKDIR is not an option here: + * it is only defined when wolfssh/port.h is built with SFTP, SCP, or sshd + * support, and this test compiles whenever WOLFSSL_BASE64_ENCODE is set, + * independent of those. */ +#ifdef USE_WINDOWS_API + #define TEST_SETENV(n,v) _putenv_s((n), (v)) + #define TEST_UNSETENV(n) _putenv_s((n), "") + #define TEST_MKDIR(p,m) _mkdir((p)) +#else + #define TEST_SETENV(n,v) setenv((n), (v), 1) + #define TEST_UNSETENV(n) unsetenv((n)) + #define TEST_MKDIR(p,m) mkdir((p), (m)) +#endif + /* known_hosts is a text file and POSIX lets its last line end without a * newline, and a file written on Windows ends its lines with CRLF. Match the * last entry with a trailing newline, without one, and with CRLF line @@ -11978,9 +12005,9 @@ static void TestKnownHostsLastEntry(void) (void)rmdir(homeDir); /* Use a single flag to avoid duplicate errors below. */ - ready = (mkdir(homeDir, 0700) == 0) - && (mkdir(sshDir, 0700) == 0) - && (setenv("HOME", homeDir, 1) == 0); + ready = (TEST_MKDIR(homeDir, 0700) == 0) + && (TEST_MKDIR(sshDir, 0700) == 0) + && (TEST_SETENV("HOME", homeDir) == 0); AssertTrue(ready); /* A regression falls through to the "add it to known hosts?" prompt, so @@ -11988,7 +12015,7 @@ static void TestKnownHostsLastEntry(void) * Check each step, otherwise a failure here leaves the prompt reading * the real stdin. */ savedStdin = dup(STDIN_FILENO); - devNull = open("/dev/null", O_RDONLY); + devNull = open(TEST_NULL_DEVICE, O_RDONLY); ready = ready && (savedStdin >= 0) && (devNull >= 0) && (dup2(devNull, STDIN_FILENO) >= 0); AssertTrue(ready); @@ -12043,11 +12070,11 @@ static void TestKnownHostsLastEntry(void) } if (savedHome != NULL) { - AssertIntEQ(setenv("HOME", savedHome, 1), 0); + AssertIntEQ(TEST_SETENV("HOME", savedHome), 0); WFREE(savedHome, NULL, 0); } else { - unsetenv("HOME"); + TEST_UNSETENV("HOME"); } (void)remove(hostsPath); diff --git a/wolfssh/port.h b/wolfssh/port.h index 24abb2fba..46f6268db 100644 --- a/wolfssh/port.h +++ b/wolfssh/port.h @@ -628,7 +628,6 @@ extern "C" { #define WSTRNCMP(s1,s2,n) strncmp((s1),(s2),(n)) #define WSTRSPN(s1,s2) strspn((s1),(s2)) #define WSTRCSPN(s1,s2) strcspn((s1),(s2)) - #define WSTRSEP(s,d) strsep((s),(d)) #define WSTRCAT(s1,s2) strcat((s1),(s2)) #define WSTRCPY(s1,s2) strcpy((s1),(s2)) @@ -641,6 +640,14 @@ extern "C" { #define WSTRDUP(s,h,t) wstrdup((s),(h),(t)) #define WSTRCHR(s,c) strchr((s),(c)) + #ifndef USE_WINDOWS_API + #define WSTRSEP(s,d) strsep((s),(d)) + #else + /* strsep() is a BSD extension not provided by MSVCRT/MinGW */ + WOLFSSH_API char* wstrsep(char** s1, const char* delim); + #define WSTRSEP(s,d) wstrsep((s),(d)) + #endif + #ifdef USE_WINDOWS_API #define WSTRNCPY(s1,s2,n) strncpy_s((s1),(n),(s2),(n)) #define WSTRNCASECMP(s1,s2,n) _strnicmp((s1),(s2),(n)) diff --git a/wolfssh/test.h b/wolfssh/test.h index a03f43686..a258075cc 100644 --- a/wolfssh/test.h +++ b/wolfssh/test.h @@ -401,7 +401,7 @@ static INLINE int mygetopt(int argc, char** argv, const char* optstring) } -#ifdef USE_WINDOWS_API +#if defined(USE_WINDOWS_API) && defined(_MSC_VER) #pragma warning(push) #pragma warning(disable:4996) /* For Windows builds, disable compiler warnings for: @@ -563,7 +563,7 @@ static INLINE void build_addr(SOCKADDR_IN_T* addr, const char* peer, } #endif /* WOLFSSH_NUCLEUS */ -#ifdef USE_WINDOWS_API +#if defined(USE_WINDOWS_API) && defined(_MSC_VER) #pragma warning(pop) #endif