Repository navigation
wolfsshd: start a Windows session with the user's environment - #1380
soulmachine wants to merge 2 commits into
Conversation
CreateProcessAsUserW got a NULL environment, so the child inherited the service's (LocalSystem) block: USERPROFILE pointed at the system profile, APPDATA and LOCALAPPDATA were SYSTEM's, and the user PATH was missing. Tools installed per user were not found, and those that were found read their settings from the wrong profile. Build the block from the user's token with CreateEnvironmentBlock and pass it with CREATE_UNICODE_ENVIRONMENT. The profile is already loaded by wolfSSHD_GetHomeDirectory, so user variables resolve. If the block cannot be built, log the Windows error and fall back to the inherited environment, which is the behaviour before this change. Tested on Windows 11 (MSVC v143, wolfSSL v5.9.4-stable): over wolfsshd, USERPROFILE is C:\Users\Administrator, and per-user tools on the user PATH (mise shims for claude and codex) resolve and run, in exec and PTY sessions.
|
Can one of the admins verify this patch? |
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are reported for the Windows session environment changes.
No open issues 🎉
What changed in this PR
This PR updates Windows shell and exec sessions in wolfsshd to use the authenticated user’s environment instead of inheriting the service account’s variables.
Changes:
- Builds a user environment block and passes it to process creation with Unicode environment support.
- Logs environment creation failures and falls back to the inherited service environment.
- Releases the environment block during cleanup.
| File | Description |
|---|---|
apps/wolfsshd/wolfsshd.c |
Adds user-specific environment creation, process integration, and cleanup for Windows sessions. |
Review effort: Balanced
|
Thanks, this checks out on my Windows 11 VM: with the PR, the session gets the user's |
|
More test evidence from a second machine: Windows 11 (build 26200), the same fork base ( Before (unpatched build, same machine): After (this PR), end-to-end suite, 13/13: The patch changes only the environment passed to |
CreateEnvironmentBlock(..., FALSE) expands the registry values before it adds ProgramFiles and the other process variables, so PSModulePath came through as "%ProgramFiles%\WindowsPowerShell\Modules;..." and the (Arm) ProgramFiles variables were missing. Build the block with TRUE, as Win32-OpenSSH does, so those variables are set first. TRUE also carries the service account's own user PATH (...\config\systemprofile\AppData\Local\Microsoft\WindowsApps), which Win32-OpenSSH drops by rebuilding PATH in the session. Here PATH is taken from the FALSE block instead: system PATH plus the user's. Tested on Windows 11 x64, MSVC v143: PSModulePath expands, PATH has no systemprofile entry, USERPROFILE, APPDATA, LOCALAPPDATA and TEMP are the user's, and no other variable differs from the FALSE block.
|
Thanks for testing it.
I pushed d31063f, which does the same thing in one place: it builds the block with
My end-to-end suite passes 15/15 on d31063f. It now also asserts that If you'd rather stay with plain |
Summary
On Windows,
wolfsshdstarts the session withCreateProcessAsUserW(token, ..., lpEnvironment = NULL, ...), so the child inherits the service's environment (LocalSystem), not the user's.USERPROFILE,APPDATAandLOCALAPPDATApoint atC:\Windows\system32\config\systemprofile,USERNAMEis the machine account, and the user's ownPATHentries are missing. Per-user tools are not found, and the ones that are found read their settings from the wrong profile.The profile is already loaded before this point, so the block resolves the user's registry variables. The block is built with
TRUE, as in Win32-OpenSSH, soPSModulePathexpands and the(Arm)ProgramFiles variables are set. OnlyPATHcomes from theFALSEblock. This gives the same result as Win32-OpenSSH's later PATH rebuild.userenv.libis already linked by everywolfsshdproject configuration.Evidence
Windows 11 Pro x64, MSVC v143 (VS 2022 Build Tools), wolfSSL v5.9.4-stable,
wolfsshdrunning as a LocalSystem service, logging in as a local administrator with anssh-ed25519key:No automated test is added: the Windows sshd tests in
windows-check.ymldo not cover the session environment yet. I'm happy to add a PowerShell check there (exececho %USERPROFILE%and assert it is not the system profile) if you want one.Merge Danger
Door: two-way
A revert restores the old inherited environment.
Blast Radius: Windows sessions
Only the Windows shell and exec path changes. A session now sees the user's variables instead of the service's, so a setup that relied on the service's environment leaking into sessions would notice. If
CreateEnvironmentBlockfails, the session still starts as before, and the error is logged.