From efc7385c50fb22d5af58de6b047f49e5acf175d7 Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Tue, 7 Mar 2023 04:31:16 +1000 Subject: [PATCH] Fix Start-Process -Wait with -Credential (#19096) Allows a non-administrator user to be able to -Wait on a Start-Process call with a custom credential specified. --- .../commands/management/Process.cs | 245 +++++++++--------- .../commands/management/Service.cs | 16 -- .../Windows/AssignProcessToJobObject.cs | 16 ++ .../engine/Interop/Windows/CloseHandle.cs | 16 ++ 4 files changed, 151 insertions(+), 142 deletions(-) create mode 100644 src/System.Management.Automation/engine/Interop/Windows/AssignProcessToJobObject.cs create mode 100644 src/System.Management.Automation/engine/Interop/Windows/CloseHandle.cs diff --git a/src/Microsoft.PowerShell.Commands.Management/commands/management/Process.cs b/src/Microsoft.PowerShell.Commands.Management/commands/management/Process.cs index 82eccff365..b731ce905c 100644 --- a/src/Microsoft.PowerShell.Commands.Management/commands/management/Process.cs +++ b/src/Microsoft.PowerShell.Commands.Management/commands/management/Process.cs @@ -2029,7 +2029,55 @@ namespace Microsoft.PowerShell.Commands return; } - Process process = Start(startInfo); + Process process = null; + +#if !UNIX + ProcessCollection jobObject = null; + bool? jobAssigned = null; +#endif + if (startInfo.UseShellExecute) + { + process = StartWithShellExecute(startInfo); + } + else + { +#if UNIX + process = new Process() { StartInfo = startInfo }; + SetupInputOutputRedirection(process); + process.Start(); + if (process.StartInfo.RedirectStandardOutput) + { + process.BeginOutputReadLine(); + } + + if (process.StartInfo.RedirectStandardError) + { + process.BeginErrorReadLine(); + } + + if (process.StartInfo.RedirectStandardInput) + { + WriteToStandardInput(process); + } +#else + using ProcessInformation processInfo = StartWithCreateProcess(startInfo); + process = Process.GetProcessById(processInfo.ProcessId); + + // Starting a process as another user might make it impossible + // to get the process handle from the S.D.Process object. Use + // the ALL_ACCESS token from CreateProcess here to setup the + // job object assignment early if -Wait was specified. + // https://github.com/PowerShell/PowerShell/issues/17033 + if (Wait) + { + jobObject = new(); + jobAssigned = jobObject.AssignProcessToJobObject(processInfo.Process); + } + + // Resume the process now that is has been set up. + processInfo.Resume(); +#endif + } if (PassThru.IsPresent) { @@ -2054,29 +2102,21 @@ namespace Microsoft.PowerShell.Commands #if UNIX process.WaitForExit(); #else - if (_credential is not null) + _waithandle = new ManualResetEvent(false); + + // Create and start the job object. This may have + // already been done in StartWithCreateProcess. + jobObject ??= new(); + if (jobAssigned == true || (jobAssigned is null && jobObject.AssignProcessToJobObject(process.SafeHandle))) { - // If we are running as a different user, we cannot use a job object, so just wait on the process - process.WaitForExit(); + // Wait for the job object to finish + jobObject.WaitOne(_waithandle); } else { - _waithandle = new ManualResetEvent(false); - - // Create and start the job object - ProcessCollection jobObject = new(); - if (jobObject.AssignProcessToJobObject(process)) - { - // Wait for the job object to finish - jobObject.WaitOne(_waithandle); - } - else if (!process.HasExited) - { - // WinBlue: 27537 Start-Process -Wait doesn't work in a remote session on Windows 7 or lower. - process.Exited += myProcess_Exited; - process.EnableRaisingEvents = true; - process.WaitForExit(); - } + // WinBlue: 27537 Start-Process -Wait doesn't work in a remote session on Windows 7 or lower. + // A Remote session is in it's own job and nested job support was only added in Windows 8/Server 2012. + process.WaitForExit(); } #endif } @@ -2120,11 +2160,6 @@ namespace Microsoft.PowerShell.Commands #region Private Methods - /// - /// When Process exits the wait handle is set. - /// - private void myProcess_Exited(object sender, System.EventArgs e) => _waithandle?.Set(); - private string ResolveFilePath(string path) { string filepath = PathUtils.ResolveFilePath(path, this); @@ -2152,41 +2187,6 @@ namespace Microsoft.PowerShell.Commands } } - private Process Start(ProcessStartInfo startInfo) - { - Process process = null; - if (startInfo.UseShellExecute) - { - process = StartWithShellExecute(startInfo); - } - else - { -#if UNIX - process = new Process() { StartInfo = startInfo }; - SetupInputOutputRedirection(process); - process.Start(); - if (process.StartInfo.RedirectStandardOutput) - { - process.BeginOutputReadLine(); - } - - if (process.StartInfo.RedirectStandardError) - { - process.BeginErrorReadLine(); - } - - if (process.StartInfo.RedirectStandardInput) - { - WriteToStandardInput(process); - } -#else - process = StartWithCreateProcess(startInfo); -#endif - } - - return process; - } - #if UNIX private StreamWriter _outputWriter; private StreamWriter _errorWriter; @@ -2436,10 +2436,10 @@ namespace Microsoft.PowerShell.Commands /// /// This method will be used on all windows platforms, both full desktop and headless SKUs. /// - private Process StartWithCreateProcess(ProcessStartInfo startinfo) + private ProcessInformation StartWithCreateProcess(ProcessStartInfo startinfo) { ProcessNativeMethods.STARTUPINFO lpStartupInfo = new(); - SafeNativeMethods.PROCESS_INFORMATION lpProcessInformation = new(); + ProcessNativeMethods.PROCESS_INFORMATION lpProcessInformation = new(); int error = 0; GCHandle pinnedEnvironmentBlock = new(); IntPtr AddressOfEnvironmentBlock = IntPtr.Zero; @@ -2486,7 +2486,7 @@ namespace Microsoft.PowerShell.Commands try { password = (startinfo.Password == null) ? Marshal.StringToCoTaskMemUni(string.Empty) : Marshal.SecureStringToCoTaskMemUnicode(startinfo.Password); - flag = ProcessNativeMethods.CreateProcessWithLogonW(startinfo.UserName, startinfo.Domain, password, logonFlags, null, cmdLine, creationFlags, AddressOfEnvironmentBlock, startinfo.WorkingDirectory, lpStartupInfo, lpProcessInformation); + flag = ProcessNativeMethods.CreateProcessWithLogonW(startinfo.UserName, startinfo.Domain, password, logonFlags, null, cmdLine, creationFlags, AddressOfEnvironmentBlock, startinfo.WorkingDirectory, lpStartupInfo, ref lpProcessInformation); if (!flag) { error = Marshal.GetLastWin32Error(); @@ -2542,7 +2542,7 @@ namespace Microsoft.PowerShell.Commands ProcessNativeMethods.SECURITY_ATTRIBUTES lpProcessAttributes = new(); ProcessNativeMethods.SECURITY_ATTRIBUTES lpThreadAttributes = new(); - flag = ProcessNativeMethods.CreateProcess(null, cmdLine, lpProcessAttributes, lpThreadAttributes, true, creationFlags, AddressOfEnvironmentBlock, startinfo.WorkingDirectory, lpStartupInfo, lpProcessInformation); + flag = ProcessNativeMethods.CreateProcess(null, cmdLine, lpProcessAttributes, lpThreadAttributes, true, creationFlags, AddressOfEnvironmentBlock, startinfo.WorkingDirectory, lpStartupInfo, ref lpProcessInformation); if (!flag) { error = Marshal.GetLastWin32Error(); @@ -2555,11 +2555,7 @@ namespace Microsoft.PowerShell.Commands Label_03AE: - // At this point, we should have a suspended process. Get the .Net Process object, resume the process, and return. - Process result = Process.GetProcessById(lpProcessInformation.dwProcessId); - ProcessNativeMethods.ResumeThread(lpProcessInformation.hThread); - - return result; + return new ProcessInformation(lpProcessInformation); } finally { @@ -2573,7 +2569,6 @@ namespace Microsoft.PowerShell.Commands } lpStartupInfo.Dispose(); - lpProcessInformation.Dispose(); } } #endif @@ -2626,10 +2621,12 @@ namespace Microsoft.PowerShell.Commands /// Start API assigns the process to the JobObject and starts monitoring /// the child processes hosted by the process created by Start-Process cmdlet. /// - internal bool AssignProcessToJobObject(Process process) + internal bool AssignProcessToJobObject(SafeProcessHandle process) { // Add the process to the job object - bool result = NativeMethods.AssignProcessToJobObject(_jobObjectHandle, process.Handle); + bool result = Interop.Windows.AssignProcessToJobObject( + _jobObjectHandle.DangerousGetHandle(), + process.DangerousGetHandle()); return result; } @@ -2674,6 +2671,44 @@ namespace Microsoft.PowerShell.Commands } } + /// + /// ProcessInformation is a helper class that wraps the native PROCESS_INFORMATION structure + /// returned by CreateProcess or CreateProcessWithLogon. It ensures the process and thread + /// HANDLEs are disposed once it's not needed. + /// + internal sealed class ProcessInformation : IDisposable + { + public SafeProcessHandle Process { get; } + + public SafeProcessHandle Thread { get; } + + public Int32 ProcessId { get; } + + public Int32 ThreadId { get; } + + internal ProcessInformation(ProcessNativeMethods.PROCESS_INFORMATION info) + { + Process = new(info.hProcess, true); + Thread = new(info.hThread, true); + ProcessId = info.dwProcessId; + ThreadId = info.dwThreadId; + } + + public void Resume() + { + ProcessNativeMethods.ResumeThread(Thread.DangerousGetHandle()); + } + + public void Dispose() + { + Process.Dispose(); + Thread.Dispose(); + GC.SuppressFinalize(this); + } + + ~ProcessInformation() => Dispose(); + } + /// /// JOBOBJECT_BASIC_PROCESS_ID_LIST Contains the process identifier list for a job object. /// If the job is nested, the process identifier list consists of all @@ -2719,7 +2754,7 @@ namespace Microsoft.PowerShell.Commands IntPtr environmentBlock, [MarshalAs(UnmanagedType.LPWStr)] string lpCurrentDirectory, STARTUPINFO lpStartupInfo, - SafeNativeMethods.PROCESS_INFORMATION lpProcessInformation); + ref PROCESS_INFORMATION lpProcessInformation); [DllImport(PinvokeDllNames.CreateProcessDllName, CharSet = CharSet.Unicode, SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] @@ -2732,7 +2767,7 @@ namespace Microsoft.PowerShell.Commands IntPtr lpEnvironment, [MarshalAs(UnmanagedType.LPWStr)] string lpCurrentDirectory, STARTUPINFO lpStartupInfo, - SafeNativeMethods.PROCESS_INFORMATION lpProcessInformation); + ref PROCESS_INFORMATION lpProcessInformation); [DllImport(PinvokeDllNames.ResumeThreadDllName, CharSet = CharSet.Unicode, SetLastError = true)] public static extern uint ResumeThread(IntPtr threadHandle); @@ -2752,6 +2787,15 @@ namespace Microsoft.PowerShell.Commands LOGON_WITH_PROFILE = 1 } + [StructLayout(LayoutKind.Sequential)] + internal struct PROCESS_INFORMATION + { + public IntPtr hProcess; + public IntPtr hThread; + public int dwProcessId; + public int dwThreadId; + } + [StructLayout(LayoutKind.Sequential)] internal class SECURITY_ATTRIBUTES { @@ -2855,57 +2899,6 @@ namespace Microsoft.PowerShell.Commands } } - internal static class SafeNativeMethods - { - [DllImport(PinvokeDllNames.CloseHandleDllName, SetLastError = true, ExactSpelling = true)] - public static extern bool CloseHandle(IntPtr handle); - - [StructLayout(LayoutKind.Sequential)] - internal class PROCESS_INFORMATION - { - public IntPtr hProcess; - public IntPtr hThread; - public int dwProcessId; - public int dwThreadId; - - public PROCESS_INFORMATION() - { - this.hProcess = IntPtr.Zero; - this.hThread = IntPtr.Zero; - } - - /// - /// Dispose. - /// - public void Dispose() - { - Dispose(true); - } - - /// - /// Dispose. - /// - /// - private void Dispose(bool disposing) - { - if (disposing) - { - if (this.hProcess != IntPtr.Zero) - { - CloseHandle(this.hProcess); - this.hProcess = IntPtr.Zero; - } - - if (this.hThread != IntPtr.Zero) - { - CloseHandle(this.hThread); - this.hThread = IntPtr.Zero; - } - } - } - } - } - [SuppressUnmanagedCodeSecurity] internal sealed class SafeJobHandle : SafeHandleZeroOrMinusOneIsInvalid { @@ -2917,7 +2910,7 @@ namespace Microsoft.PowerShell.Commands protected override bool ReleaseHandle() { - return SafeNativeMethods.CloseHandle(base.handle); + return Interop.Windows.CloseHandle(base.handle); } } #endif diff --git a/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs b/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs index f05881751e..0253cee5e3 100644 --- a/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs +++ b/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs @@ -2776,22 +2776,6 @@ namespace Microsoft.PowerShell.Commands [DllImport("Kernel32.dll", CharSet = CharSet.Unicode)] internal static extern IntPtr CreateJobObject(IntPtr lpJobAttributes, string lpName); - /// - /// AssignProcessToJobObject API is used to assign a process to an existing job object. - /// - /// - /// A handle to the job object to which the process will be associated. - /// - /// - /// A handle to the process to associate with the job object. - /// - /// If the function succeeds, the return value is nonzero. - /// If the function fails, the return value is zero. - /// - [DllImport("Kernel32.dll", CharSet = CharSet.Unicode)] - [return: MarshalAs(UnmanagedType.Bool)] - internal static extern bool AssignProcessToJobObject(SafeHandle hJob, IntPtr hProcess); - /// /// Retrieves job state information from the job object. /// diff --git a/src/System.Management.Automation/engine/Interop/Windows/AssignProcessToJobObject.cs b/src/System.Management.Automation/engine/Interop/Windows/AssignProcessToJobObject.cs new file mode 100644 index 0000000000..1a924fb51d --- /dev/null +++ b/src/System.Management.Automation/engine/Interop/Windows/AssignProcessToJobObject.cs @@ -0,0 +1,16 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#nullable enable + +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Windows + { + [LibraryImport("Kernel32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool AssignProcessToJobObject(nint hJob, nint hProcess); + } +} diff --git a/src/System.Management.Automation/engine/Interop/Windows/CloseHandle.cs b/src/System.Management.Automation/engine/Interop/Windows/CloseHandle.cs new file mode 100644 index 0000000000..6832268272 --- /dev/null +++ b/src/System.Management.Automation/engine/Interop/Windows/CloseHandle.cs @@ -0,0 +1,16 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#nullable enable + +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Windows + { + [LibraryImport("api-ms-win-core-handle-l1-1-0.dll")] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool CloseHandle(nint hObject); + } +}