fix user profile unload

contrib/win32/win32compat/misc_internal.h

@@ -66,7 +66,8 @@ void invalid_parameter_handler(const wchar_t *, const wchar_t *, const wchar_t *
 void to_lower_case(char *s);
 void to_wlower_case(wchar_t *s);
 HANDLE get_user_token(const char* user, int impersonation);
-int load_user_profile(HANDLE user_token, char* user);
+HANDLE load_user_profile(HANDLE user_token, char* user);
+void unload_user_profile(HANDLE user_token, HANDLE profile);
 int create_directory_withsddl(wchar_t *path, wchar_t *sddl, BOOL check_permissions);
 int is_absolute_path(const char *);
 int file_in_chroot_jail(HANDLE);

contrib/win32/win32compat/signal_internal.h

@@ -13,6 +13,9 @@ struct _children {
 	 */
 	HANDLE handles[MAX_CHILDREN];
 	DWORD process_id[MAX_CHILDREN];
+	/* per-child loaded profile and token used to unload it (NULL if none) */
+	HANDLE profiles[MAX_CHILDREN];
+	HANDLE tokens[MAX_CHILDREN];
 	/* total children */
 	DWORD num_children;
 	/* #zombies */
@@ -23,6 +26,7 @@ struct _children {
 
 int sw_initialize();
 int register_child(HANDLE child, DWORD pid);
+int register_child_profile(DWORD pid, HANDLE profile, HANDLE user_token);
 int sw_remove_child_at_index(DWORD index);
 int sw_child_to_zombie(DWORD index);
 void sw_cleanup_child_zombies();

contrib/win32/win32compat/signal_sigchld.c

@@ -51,18 +51,42 @@ register_child(HANDLE child, DWORD pid)
 		first_zombie_index = children.num_children - children.num_zombies;
 		children.handles[children.num_children] = children.handles[first_zombie_index];
 		children.process_id[children.num_children] = children.process_id[first_zombie_index];
+		children.profiles[children.num_children] = children.profiles[first_zombie_index];
+		children.tokens[children.num_children] = children.tokens[first_zombie_index];
 
 		children.handles[first_zombie_index] = child;
 		children.process_id[first_zombie_index] = pid;
+		children.profiles[first_zombie_index] = NULL;
+		children.tokens[first_zombie_index] = NULL;
 	} else {
 		children.handles[children.num_children] = child;
 		children.process_id[children.num_children] = pid;
+		children.profiles[children.num_children] = NULL;
+		children.tokens[children.num_children] = NULL;
 	}
 
 	children.num_children++;
 	return 0;
 }
 
+/* associate a loaded profile and its unload token with a registered child */
+int
+register_child_profile(DWORD pid, HANDLE profile, HANDLE user_token)
+{
+	DWORD i;
+
+	for (i = 0; i < children.num_children; i++) {
+		if (children.process_id[i] == pid) {
+			children.profiles[i] = profile;
+			children.tokens[i] = user_token;
+			return 0;
+		}
+	}
+
+	errno = ECHILD;
+	return -1;
+}
+
 int
 sw_remove_child_at_index(DWORD index)
 {
@@ -76,22 +100,39 @@ sw_remove_child_at_index(DWORD index)
 	}
 
 	CloseHandle(children.handles[index]);
+	/* unload the profile associated with this child, if any */
+	if (children.profiles[index] != NULL) {
+		unload_user_profile(children.tokens[index], children.profiles[index]);
+		children.profiles[index] = NULL;
+	}
+	if (children.tokens[index] != NULL) {
+		CloseHandle(children.tokens[index]);
+		children.tokens[index] = NULL;
+	}
 	if (children.num_zombies == 0) {
 		children.handles[index] = children.handles[children.num_children - 1];
 		children.process_id[index] = children.process_id[children.num_children - 1];
+		children.profiles[index] = children.profiles[children.num_children - 1];
+		children.tokens[index] = children.tokens[children.num_children - 1];
 	} else {
 		/* if its a zombie */
 		if (index >= (children.num_children - children.num_zombies)) {
 			children.handles[index] = children.handles[children.num_children - 1];
 			children.process_id[index] = children.process_id[children.num_children - 1];
+			children.profiles[index] = children.profiles[children.num_children - 1];
+			children.tokens[index] = children.tokens[children.num_children - 1];
 			children.num_zombies--;
 		} else {
 			last_non_zombie = children.num_children - children.num_zombies - 1;
 			children.handles[index] = children.handles[last_non_zombie];
 			children.process_id[index] = children.process_id[last_non_zombie];
+			children.profiles[index] = children.profiles[last_non_zombie];
+			children.tokens[index] = children.tokens[last_non_zombie];
 
 			children.handles[last_non_zombie] = children.handles[children.num_children - 1];
 			children.process_id[last_non_zombie] = children.process_id[children.num_children - 1];
+			children.profiles[last_non_zombie] = children.profiles[children.num_children - 1];
+			children.tokens[last_non_zombie] = children.tokens[children.num_children - 1];
 		}
 	}
 
@@ -103,7 +144,7 @@ int
 sw_child_to_zombie(DWORD index)
 {
 	DWORD last_non_zombie, zombie_pid;
-	HANDLE zombie_handle;
+	HANDLE zombie_handle, zombie_profile, zombie_token;
 
 	debug4("zombie'ing child at index %d, %d zombies of %d", index,
 		children.num_zombies, children.num_children);
@@ -118,10 +159,16 @@ sw_child_to_zombie(DWORD index)
 		/* swap */
 		zombie_pid = children.process_id[index];
 		zombie_handle = children.handles[index];
+		zombie_profile = children.profiles[index];
+		zombie_token = children.tokens[index];
 		children.handles[index] = children.handles[last_non_zombie];
 		children.process_id[index] = children.process_id[last_non_zombie];
+		children.profiles[index] = children.profiles[last_non_zombie];
+		children.tokens[index] = children.tokens[last_non_zombie];
 		children.handles[last_non_zombie] = zombie_handle;
 		children.process_id[last_non_zombie] = zombie_pid;
+		children.profiles[last_non_zombie] = zombie_profile;
+		children.tokens[last_non_zombie] = zombie_token;
 	}
 	children.num_zombies++;
 	return 0;

contrib/win32/win32compat/spawn-ext.c

@@ -1,5 +1,6 @@
 #include <Windows.h>
 #include "misc_internal.h"
+#include "signal_internal.h"
 #include "inc\unistd.h"
 #include "debug.h"
 
@@ -14,26 +15,67 @@ __posix_spawn_asuser(pid_t *pidp, const char *path, const posix_spawn_file_actio
 	int r = -1;
 	/* use token generated from password auth if already present */
 	HANDLE user_token = NULL;
-
-	if (password_auth_token)
+	HANDLE profile = NULL;
+	HANDLE profile_token = NULL;
+	pid_t pid = 0;
+	BOOLEAN token_from_sspi = FALSE;
+	BOOLEAN token_from_password_auth = FALSE;
+
+	if (password_auth_token) {
 		user_token = password_auth_token;
-	else if (sspi_auth_user) 
+		token_from_password_auth = TRUE;
+	}
+	else if (sspi_auth_user) {
 		user_token = sspi_auth_user;
+		token_from_sspi = TRUE;
+	}
 
-	if (!user_token && (user_token = get_user_token(user, 1)) == NULL) {
-		error("unable to get security token for user %s", user);
-		errno = EOTHER;
-		return -1;
+	if (!user_token) {
+		if ((user_token = get_user_token(user, 1)) == NULL) {
+			error("unable to get security token for user %s", user);
+			errno = EOTHER;
+			return -1;
+		}
 	}
 	/*
 	 * This identity check decides whether to load both the user's profile and
 	 * environment block.
 	 */
 	BOOLEAN is_privsep = (strcmp(user, "sshd") == 0);
-	if (!is_privsep)
-		load_user_profile(user_token, user);
+	if (!is_privsep) {
+		profile = load_user_profile(user_token, user);
+		if (profile != NULL) {
+			/* least-privilege token copy kept for unload at child exit */
+			if (!DuplicateHandle(GetCurrentProcess(), user_token, GetCurrentProcess(),
+			    &profile_token, TOKEN_QUERY | TOKEN_IMPERSONATE | TOKEN_DUPLICATE, FALSE, 0)) {
+				debug3("%s: DuplicateHandle() failed with error %d.", __FUNCTION__, GetLastError());
+				profile_token = NULL;
+			}
+		}
+	}
+
+	r = posix_spawn_internal(&pid, path, file_actions, attrp, argv, envp, user_token, TRUE, !is_privsep);
 
-	r = posix_spawn_internal(pidp, path, file_actions, attrp, argv, envp, user_token, TRUE, !is_privsep);
-	CloseHandle(user_token);
+	if (pidp && r == 0)
+		*pidp = pid;
+
+	if (profile != NULL) {
+		BOOLEAN child_has_profile = FALSE;
+		if (r == 0 && profile_token != NULL)
+			child_has_profile = (register_child_profile(pid, profile, profile_token) == 0);
+
+		if (!child_has_profile) {
+			unload_user_profile(profile_token ? profile_token : user_token, profile);
+			if (profile_token)
+				CloseHandle(profile_token);
+		}
+	}
+
+	/* token ownership: SSPI token is freed by the GSS credential, not here */
+	if (!token_from_sspi) {
+		CloseHandle(user_token);
+		if (token_from_password_auth)
+			password_auth_token = NULL;
+	}
 	return r;
-}
+}

contrib/win32/win32compat/win32_usertoken_utils.c

@@ -395,19 +395,19 @@ get_user_token(const char* user, int impersonation) {
 	return token;
 }
 
-int 
+HANDLE 
 load_user_profile(HANDLE user_token, char* user)
 {
 	wchar_t * user_utf16 = NULL;
 
 	if (!am_system()) {
 	    debug("Not running as SYSTEM: skipping loading user profile");
-	    return 0;
+	    return NULL;
 	}
 
 	if ((user_utf16 = utf8_to_utf16(user)) == NULL) {
 		fatal("out of memory");
-		return -1;
+		return NULL;
 	}
 
 	/* note: user string will normalized form output of get_passwd() */
@@ -432,15 +432,31 @@ load_user_profile(HANDLE user_token, char* user)
 	EnablePrivilege("SeBackupPrivilege", 1);
 	EnablePrivilege("SeRestorePrivilege", 1);
 	if (LoadUserProfileW(user_token, &profileInfo) == FALSE) {
-		debug3("%s: LoadUserProfileW() failed for user %S with error %d.", __FUNCTION__, GetLastError());
+		debug3("%s: LoadUserProfileW() failed for user %S with error %d.", __FUNCTION__, user_name, GetLastError());
+		profileInfo.hProfile = NULL;
 	}
 	EnablePrivilege("SeBackupPrivilege", 0);
 	EnablePrivilege("SeRestorePrivilege", 0);
 
 	if (user_utf16)
 		free(user_utf16);
 
-	return 0;
+	return profileInfo.hProfile;
+}
+
+/* unload the loaded user profile (if any) when the token can no longer be used */
+void
+unload_user_profile(HANDLE user_token, HANDLE profile)
+{
+	if (profile == NULL || profile == INVALID_HANDLE_VALUE)
+		return;
+
+	EnablePrivilege("SeBackupPrivilege", 1);
+	EnablePrivilege("SeRestorePrivilege", 1);
+	if (UnloadUserProfile(user_token, profile) == FALSE)
+		debug3("%s: UnloadUserProfile() failed with error %d.", __FUNCTION__, GetLastError());
+	EnablePrivilege("SeBackupPrivilege", 0);
+	EnablePrivilege("SeRestorePrivilege", 0);
 }
 
 

regress/pesterTests/SSH.Tests.ps1

@@ -228,6 +228,8 @@ Describe "E2E scenarios for ssh client" -Tags "CI" {
             $o | Should Be `$env:computername
         }
         It "$tC.$tI - exiting ssh session exits sshd session child processes" -skip:$skip {
+            # Let any prior session teardown settle so the baseline is accurate.
+            Start-Sleep -Seconds 2
             $sshdPidCountBefore = (Get-Process -Name sshd* | Select-Object -ExpandProperty Id).Count
             ssh test_target "echo '`$env:computername'"
             Start-Sleep -Seconds 2

regress/pesterTests/SSHD.Tests.ps1

@@ -43,7 +43,7 @@ Describe "E2E scenarios for sshd" -Tags "CI" {
             $sshdPidCountBefore = (Get-Process -Name sshd* | Select-Object -ExpandProperty Id).Count
             # Start ssh process (do not authenticate)
             $sshProc = Start-Process -FilePath ssh -ArgumentList "-l $user test_target" -PassThru
-            Start-Sleep -Seconds 2
+            Start-Sleep -Seconds 4
             $sshdPidsCountWithConn = (Get-Process -Name sshd* | Select-Object -ExpandProperty Id).Count
             # Wait for LoginGraceTime to expire, accounting for jitter
             Start-Sleep -Seconds 14

regress/pesterTests/UserProfileUnload.Tests.ps1

@@ -0,0 +1,73 @@
+Import-Module $PSScriptRoot\CommonUtils.psm1 -Force
+
+# Regression test for PowerShell/Win32-OpenSSH#1694: sshd must unload the user
+# registry hive after a session ends, otherwise open handles on NTUSER.DAT /
+# UsrClass.dat leak and the user profile can no longer be deleted.
+Describe "User profile hive is unloaded after a session (issue #1694)" -Tags "CI" {
+    BeforeAll {
+        if ($OpenSSHTestInfo -eq $null) {
+            Throw "`$OpenSSHTestInfo is null. Please run Set-OpenSSHTestEnvironment to set test environments."
+        }
+
+        $server = $OpenSSHTestInfo["Target"]
+        $port = $OpenSSHTestInfo["Port"]
+        $user = $OpenSSHTestInfo["PasswdUser"]
+        $password = $OpenSSHTestInfo["TestAccountPW"]
+
+        # The profile is only loaded (and therefore unloaded) when sshd runs as
+        # SYSTEM. Skip when the test daemon runs under another account.
+        $skip = $true
+        $svcName = $OpenSSHTestInfo["SshdServiceName"]
+        if ($svcName) {
+            $svc = Get-CimInstance Win32_Service -Filter "Name='$svcName'" -ErrorAction SilentlyContinue
+            if ($svc -and $svc.StartName -match '^(LocalSystem|NT AUTHORITY\\SYSTEM)$') { $skip = $false }
+        } else {
+            # task-based test daemon is registered as SYSTEM
+            $skip = $false
+        }
+
+        # resolve the user's SID so we can look for its loaded hive under HKEY_USERS
+        $userSid = $null
+        try {
+            $userSid = (New-Object System.Security.Principal.NTAccount($user)).Translate(
+                [System.Security.Principal.SecurityIdentifier]).Value
+        } catch {
+            $skip = $true
+        }
+
+        Add-PasswordSetting -Pass $password
+    }
+
+    AfterAll {
+        Remove-PasswordSetting
+    }
+
+    It "loads the user hive during a session and unloads it after the child exits" -skip:$skip {
+        # Keep a session alive in a background job so the loaded hive is observable.
+        # A job has no console, so ssh uses SSH_ASKPASS for the password.
+        $job = Start-Job -ScriptBlock {
+            param($port, $server, $user)
+            ssh -p $port -o "StrictHostKeyChecking=no" "$user@$server" "powershell -NoProfile -Command Start-Sleep -Seconds 30"
+        } -ArgumentList $port, $server, $user
+
+        # Wait for auth + profile load (slow on CI). Enumerate HKEY_USERS via reg.exe,
+        # which needs only enumerate rights, unlike opening the ACL'd hive key.
+        $loaded = $false
+        for ($i = 0; $i -lt 30 -and -not $loaded; $i++) {
+            Start-Sleep -Seconds 1
+            $loaded = [bool]((reg query HKU 2>$null) -match [regex]::Escape($userSid))
+        }
+        $loaded | Should Be $true
+
+        Stop-Job $job
+        Remove-Job $job -Force
+
+        # The session child is reaped asynchronously; wait for the unload to complete.
+        $loaded = $true
+        for ($i = 0; $i -lt 30 -and $loaded; $i++) {
+            Start-Sleep -Seconds 1
+            $loaded = [bool]((reg query HKU 2>$null) -match [regex]::Escape($userSid))
+        }
+        $loaded | Should Be $false
+    }
+}