X Tutup
Skip to content

Fix Start-Process -Wait with -Credential #19096

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PaulHigin merged 1 commit into from
Mar 6, 2023
Merged

Fix Start-Process -Wait with -Credential #19096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
247 changes: 120 additions & 127 deletions src/Microsoft.PowerShell.Commands.Management/commands/management/Process.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2014,7 +2014,55 @@ protected override void BeginProcessing()
string targetMessage = StringUtil.Format(ProcessResources.StartProcessTarget, startInfo.FileName, startInfo.Arguments.Trim());
if (!ShouldProcess(targetMessage)) { 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)
{
Expand All @@ -2039,29 +2087,21 @@ protected override void BeginProcessing()
#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
}
Expand Down Expand Up @@ -2105,11 +2145,6 @@ private void Dispose(bool isDisposing)

#region Private Methods

/// <summary>
/// When Process exits the wait handle is set.
/// </summary>
private void myProcess_Exited(object sender, System.EventArgs e) => _waithandle?.Set();

private string ResolveFilePath(string path)
{
string filepath = PathUtils.ResolveFilePath(path, this);
Expand Down Expand Up @@ -2137,41 +2172,6 @@ private static void LoadEnvironmentVariable(ProcessStartInfo startinfo, IDiction
}
}

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;
Expand Down Expand Up @@ -2421,10 +2421,10 @@ private void SetStartupInfo(ProcessStartInfo startinfo, ref ProcessNativeMethods
/// <summary>
/// This method will be used on all windows platforms, both full desktop and headless SKUs.
/// </summary>
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;
Expand Down Expand Up @@ -2471,7 +2471,7 @@ private Process StartWithCreateProcess(ProcessStartInfo startinfo)
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();
Expand Down Expand Up @@ -2527,7 +2527,7 @@ private Process StartWithCreateProcess(ProcessStartInfo startinfo)

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();
Expand All @@ -2540,11 +2540,7 @@ private Process StartWithCreateProcess(ProcessStartInfo startinfo)

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
{
Expand All @@ -2558,7 +2554,6 @@ private Process StartWithCreateProcess(ProcessStartInfo startinfo)
}

lpStartupInfo.Dispose();
lpProcessInformation.Dispose();
}
}
#endif
Expand Down Expand Up @@ -2611,10 +2606,12 @@ internal ProcessCollection()
/// Start API assigns the process to the JobObject and starts monitoring
/// the child processes hosted by the process created by Start-Process cmdlet.
/// </summary>
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;
}

Expand Down Expand Up @@ -2659,6 +2656,44 @@ internal void WaitOne(ManualResetEvent waitHandleToUse)
}
}

/// <summary>
/// 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.
/// </summary>
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();
}

/// <summary>
/// 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
Expand Down Expand Up @@ -2704,7 +2739,7 @@ internal static extern bool CreateProcessWithLogonW(string userName,
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)]
Expand All @@ -2717,7 +2752,7 @@ public static extern bool CreateProcess([MarshalAs(UnmanagedType.LPWStr)] string
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);
Expand All @@ -2737,6 +2772,15 @@ internal enum LogonFlags
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
{
Expand Down Expand Up @@ -2840,57 +2884,6 @@ public void Dispose()
}
}

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;
}

/// <summary>
/// Dispose.
/// </summary>
public void Dispose()
{
Dispose(true);
}

/// <summary>
/// Dispose.
/// </summary>
/// <param name="disposing"></param>
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
{
Expand All @@ -2902,7 +2895,7 @@ internal SafeJobHandle(IntPtr jobHandle)

protected override bool ReleaseHandle()
{
return SafeNativeMethods.CloseHandle(base.handle);
return Interop.Windows.CloseHandle(base.handle);
}
}
#endif
Expand Down Expand Up @@ -2971,7 +2964,7 @@ public override void GetObjectData(
base.GetObjectData(info, context);

ArgumentNullException.ThrowIfNull(info);

info.AddValue("ProcessName", _processName);
}
#endregion Serialization
Expand Down
16 changes: 0 additions & 16 deletions src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2776,22 +2776,6 @@ byte[] lpSecurityDescriptor
[DllImport("Kernel32.dll", CharSet = CharSet.Unicode)]
internal static extern IntPtr CreateJobObject(IntPtr lpJobAttributes, string lpName);

/// <summary>
/// AssignProcessToJobObject API is used to assign a process to an existing job object.
/// </summary>
/// <param name="hJob">
/// A handle to the job object to which the process will be associated.
/// </param>
/// <param name="hProcess">
/// A handle to the process to associate with the job object.
/// </param>
/// <returns>If the function succeeds, the return value is nonzero.
/// If the function fails, the return value is zero.
/// </returns>
[DllImport("Kernel32.dll", CharSet = CharSet.Unicode)]
[return: MarshalAs(UnmanagedType.Bool)]
internal static extern bool AssignProcessToJobObject(SafeHandle hJob, IntPtr hProcess);

/// <summary>
/// Retrieves job state information from the job object.
/// </summary>
Expand Down
Loading
X Tutup