diff --git a/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs b/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs index 8ef5167d70edc..03740e8fcea22 100644 --- a/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs +++ b/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.ProcessOptions.cs @@ -42,7 +42,13 @@ internal static partial class StartupInfoOptions internal const int STARTF_USESHOWWINDOW = 0x00000001; internal const int STARTF_USESTDHANDLES = 0x00000100; internal const int CREATE_UNICODE_ENVIRONMENT = 0x00000400; + internal const int EXTENDED_STARTUPINFO_PRESENT = 0x00080000; internal const int CREATE_NO_WINDOW = 0x08000000; } + + internal static partial class ProcThreadAttribute + { + internal const int HANDLE_LIST = 131074; + } } } diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateProcess.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateProcess.cs index 834ff2e730d93..7589f5320a644 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateProcess.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateProcess.cs @@ -21,10 +21,21 @@ internal static unsafe partial bool CreateProcess( int dwCreationFlags, IntPtr lpEnvironment, string? lpCurrentDirectory, - ref STARTUPINFO lpStartupInfo, + ref STARTUPINFOEX lpStartupInfo, ref PROCESS_INFORMATION lpProcessInformation ); + [LibraryImport(Libraries.Kernel32, EntryPoint = "InitializeProcThreadAttributeList", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static unsafe partial bool InitializeProcThreadAttributeList(byte* lpAttributeList, uint dwAttributeCount, uint dwFlags, nuint* lpSize); + + [LibraryImport(Libraries.Kernel32, EntryPoint = "UpdateProcThreadAttribute", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static unsafe partial bool UpdateProcThreadAttribute(byte* lpAttributeList, uint dwFlags, nuint Attribute, Span lpValue, nuint cbSize, void* lpPreviousValue, nuint* lpReturnSize); + + [LibraryImport(Libraries.Kernel32, EntryPoint = "DeleteProcThreadAttributeList", SetLastError = true)] + internal static unsafe partial void DeleteProcThreadAttributeList(byte* lpAttributeList); + [StructLayout(LayoutKind.Sequential)] internal struct PROCESS_INFORMATION { @@ -56,5 +67,12 @@ internal struct STARTUPINFO internal IntPtr hStdOutput; internal IntPtr hStdError; } + + [StructLayout(LayoutKind.Sequential)] + internal unsafe struct STARTUPINFOEX + { + internal STARTUPINFO StartupInfo; + internal byte* AttributeList; + } } } diff --git a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs index bd58edc786fba..a726eb517d5c3 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -231,6 +231,7 @@ public ProcessStartInfo(string fileName, System.Collections.Generic.IEnumerable< [System.ComponentModel.EditorAttribute("System.Diagnostics.Design.StartFileNameEditor, System.Design, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a", "System.Drawing.Design.UITypeEditor, System.Drawing, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a")] [System.Diagnostics.CodeAnalysis.AllowNullAttribute] public string FileName { get { throw null; } set { } } + public bool InheritHandles { get { throw null; } [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] set { } } [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] public bool LoadUserProfile { get { throw null; } set { } } [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] diff --git a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx index 6a2f970d6aec8..3eed5255cac34 100644 --- a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx @@ -306,6 +306,12 @@ The Process object must have the UseShellExecute property set to false in order to use environment variables. + + The Process object must have the UseShellExecute property set to false in order to disable handle inheritance. + + + The Process object must have the InheritHandles property set to true in order to start a process as a user. + UseShellExecute is not supported on this platform. diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs index 9570370179179..8a918e010eedd 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs @@ -47,6 +47,9 @@ private unsafe bool StartWithShellExecuteEx(ProcessStartInfo startInfo) if (startInfo._environmentVariables != null) throw new InvalidOperationException(SR.CantUseEnvVars); + if (!startInfo.InheritHandles) + throw new InvalidOperationException(SR.CantDisableHandleInheritanceAndUseShellExecute); + string arguments = startInfo.BuildArguments(); fixed (char* fileName = startInfo.FileName.Length > 0 ? startInfo.FileName : null) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs index 06a2bd51d6d40..a11dbd40c4348 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs @@ -16,7 +16,9 @@ namespace System.Diagnostics { public partial class Process : IDisposable { - private static readonly object s_createProcessLock = new object(); + // We use ReaderWriterLock instead of the newer ReaderWriterLockSlim for more fairness between the + // read and write sides of the lock. + private static readonly ReaderWriterLock s_createProcessLock = new(); private string? _processName; @@ -426,6 +428,44 @@ private void SetWorkingSetLimitsCore(IntPtr? newMin, IntPtr? newMax, out IntPtr /// Starts the process using the supplied start info. /// The start info with which to start the process. private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo) + { + // Take a snapshot of these properties so that concurrent modification of the ProcessStartInfo + // does not effect the correctness of the handle inheritance. + bool inheritHandles = startInfo.InheritHandles; + bool hasRedirection = startInfo.RedirectStandardInput || startInfo.RedirectStandardOutput || startInfo.RedirectStandardError; + + if (inheritHandles && hasRedirection) + { + // Take the writer side of the lock to synchronize all redirect pipe handle creations and CreateProcess + // calls. We do not want one process to inherit the handles created concurrently for another + // process, as that will impact the ownership and lifetimes of those handles now inherited + // into multiple child processes. + s_createProcessLock.AcquireWriterLock(Timeout.Infinite); + try + { + return StartWithCreateProcessCore(startInfo, inheritHandles, hasRedirection); + } + finally + { + s_createProcessLock.ReleaseWriterLock(); + } + } + else + { + s_createProcessLock.AcquireReaderLock(Timeout.Infinite); + try + { + return StartWithCreateProcessCore(startInfo, inheritHandles, hasRedirection); + } + finally + { + s_createProcessLock.ReleaseReaderLock(); + } + + } + } + + private unsafe bool StartWithCreateProcessCore(ProcessStartInfo startInfo, bool inheritHandles, bool hasRedirection) { // See knowledge base article Q190351 for an explanation of the following code. Noteworthy tricky points: // * The handles are duplicated as non-inheritable before they are passed to CreateProcess so @@ -436,10 +476,11 @@ private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo) var commandLine = new ValueStringBuilder(stackalloc char[256]); BuildCommandLine(startInfo, ref commandLine); - Interop.Kernel32.STARTUPINFO startupInfo = default; + Interop.Kernel32.STARTUPINFOEX startupInfo = default; Interop.Kernel32.PROCESS_INFORMATION processInfo = default; Interop.Kernel32.SECURITY_ATTRIBUTES unused_SecAttrs = default; SafeProcessHandle procSH = new SafeProcessHandle(); + byte* lpAttributeList = null; // handles used in parent process SafeFileHandle? parentInputPipeHandle = null; @@ -449,183 +490,222 @@ private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo) SafeFileHandle? parentErrorPipeHandle = null; SafeFileHandle? childErrorPipeHandle = null; - // Take a global lock to synchronize all redirect pipe handle creations and CreateProcess - // calls. We do not want one process to inherit the handles created concurrently for another - // process, as that will impact the ownership and lifetimes of those handles now inherited - // into multiple child processes. - lock (s_createProcessLock) + try { - try + startupInfo.StartupInfo.cb = sizeof(Interop.Kernel32.STARTUPINFO); + + int numberOfHandles = 0; + Span handlesToInherit = stackalloc nint[3]; + + // set up the streams + if (hasRedirection) { - startupInfo.cb = sizeof(Interop.Kernel32.STARTUPINFO); + if (startInfo.RedirectStandardInput) + { + CreatePipe(out parentInputPipeHandle, out childInputPipeHandle, true); + handlesToInherit[numberOfHandles++] = childInputPipeHandle.DangerousGetHandle(); + } + else + { + childInputPipeHandle = new SafeFileHandle(Interop.Kernel32.GetStdHandle(Interop.Kernel32.HandleTypes.STD_INPUT_HANDLE), false); + } - // set up the streams - if (startInfo.RedirectStandardInput || startInfo.RedirectStandardOutput || startInfo.RedirectStandardError) + if (startInfo.RedirectStandardOutput) { - if (startInfo.RedirectStandardInput) - { - CreatePipe(out parentInputPipeHandle, out childInputPipeHandle, true); - } - else - { - childInputPipeHandle = new SafeFileHandle(Interop.Kernel32.GetStdHandle(Interop.Kernel32.HandleTypes.STD_INPUT_HANDLE), false); - } + CreatePipe(out parentOutputPipeHandle, out childOutputPipeHandle, false); + handlesToInherit[numberOfHandles++] = childOutputPipeHandle.DangerousGetHandle(); + } + else + { + childOutputPipeHandle = new SafeFileHandle(Interop.Kernel32.GetStdHandle(Interop.Kernel32.HandleTypes.STD_OUTPUT_HANDLE), false); + } - if (startInfo.RedirectStandardOutput) - { - CreatePipe(out parentOutputPipeHandle, out childOutputPipeHandle, false); - } - else - { - childOutputPipeHandle = new SafeFileHandle(Interop.Kernel32.GetStdHandle(Interop.Kernel32.HandleTypes.STD_OUTPUT_HANDLE), false); - } + if (startInfo.RedirectStandardError) + { + CreatePipe(out parentErrorPipeHandle, out childErrorPipeHandle, false); + handlesToInherit[numberOfHandles++] = childErrorPipeHandle.DangerousGetHandle(); + } + else + { + childErrorPipeHandle = new SafeFileHandle(Interop.Kernel32.GetStdHandle(Interop.Kernel32.HandleTypes.STD_ERROR_HANDLE), false); + } - if (startInfo.RedirectStandardError) - { - CreatePipe(out parentErrorPipeHandle, out childErrorPipeHandle, false); - } - else - { - childErrorPipeHandle = new SafeFileHandle(Interop.Kernel32.GetStdHandle(Interop.Kernel32.HandleTypes.STD_ERROR_HANDLE), false); - } + startupInfo.StartupInfo.hStdInput = childInputPipeHandle.DangerousGetHandle(); + startupInfo.StartupInfo.hStdOutput = childOutputPipeHandle.DangerousGetHandle(); + startupInfo.StartupInfo.hStdError = childErrorPipeHandle.DangerousGetHandle(); - startupInfo.hStdInput = childInputPipeHandle.DangerousGetHandle(); - startupInfo.hStdOutput = childOutputPipeHandle.DangerousGetHandle(); - startupInfo.hStdError = childErrorPipeHandle.DangerousGetHandle(); + startupInfo.StartupInfo.dwFlags = Interop.Advapi32.StartupInfoOptions.STARTF_USESTDHANDLES; + } - startupInfo.dwFlags = Interop.Advapi32.StartupInfoOptions.STARTF_USESTDHANDLES; - } + if (startInfo.WindowStyle != ProcessWindowStyle.Normal) + { + startupInfo.StartupInfo.wShowWindow = (short)GetShowWindowFromWindowStyle(startInfo.WindowStyle); + startupInfo.StartupInfo.dwFlags |= Interop.Advapi32.StartupInfoOptions.STARTF_USESHOWWINDOW; + } - if (startInfo.WindowStyle != ProcessWindowStyle.Normal) - { - startupInfo.wShowWindow = (short)GetShowWindowFromWindowStyle(startInfo.WindowStyle); - startupInfo.dwFlags |= Interop.Advapi32.StartupInfoOptions.STARTF_USESHOWWINDOW; - } + // set up the creation flags parameter + int creationFlags = 0; + if (startInfo.CreateNoWindow) creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_NO_WINDOW; - // set up the creation flags parameter - int creationFlags = 0; - if (startInfo.CreateNoWindow) creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_NO_WINDOW; + // set up the environment block parameter + string? environmentBlock = null; + if (startInfo._environmentVariables != null) + { + creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_UNICODE_ENVIRONMENT; + environmentBlock = GetEnvironmentVariablesBlock(startInfo._environmentVariables!); + } - // set up the environment block parameter - string? environmentBlock = null; - if (startInfo._environmentVariables != null) + string? workingDirectory = startInfo.WorkingDirectory; + if (workingDirectory.Length == 0) + { + workingDirectory = null; + } + + bool retVal; + int errorCode = 0; + + if (startInfo.UserName.Length != 0) + { + if (startInfo.Password != null && startInfo.PasswordInClearText != null) { - creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_UNICODE_ENVIRONMENT; - environmentBlock = GetEnvironmentVariablesBlock(startInfo._environmentVariables!); + throw new ArgumentException(SR.CantSetDuplicatePassword); } - - string? workingDirectory = startInfo.WorkingDirectory; - if (workingDirectory.Length == 0) + if (!inheritHandles) { - workingDirectory = null; + throw new InvalidOperationException(SR.CantDisableHandleInheritanceAndUseUserName); } - bool retVal; - int errorCode = 0; + Interop.Advapi32.LogonFlags logonFlags = (Interop.Advapi32.LogonFlags)0; + if (startInfo.LoadUserProfile && startInfo.UseCredentialsForNetworkingOnly) + { + throw new ArgumentException(SR.CantEnableConflictingLogonFlags, nameof(startInfo)); + } + else if (startInfo.LoadUserProfile) + { + logonFlags = Interop.Advapi32.LogonFlags.LOGON_WITH_PROFILE; + } + else if (startInfo.UseCredentialsForNetworkingOnly) + { + logonFlags = Interop.Advapi32.LogonFlags.LOGON_NETCREDENTIALS_ONLY; + } - if (startInfo.UserName.Length != 0) + fixed (char* passwordInClearTextPtr = startInfo.PasswordInClearText ?? string.Empty) + fixed (char* environmentBlockPtr = environmentBlock) + fixed (char* commandLinePtr = &commandLine.GetPinnableReference(terminate: true)) { - if (startInfo.Password != null && startInfo.PasswordInClearText != null) - { - throw new ArgumentException(SR.CantSetDuplicatePassword); - } + IntPtr passwordPtr = (startInfo.Password != null) ? + Marshal.SecureStringToGlobalAllocUnicode(startInfo.Password) : IntPtr.Zero; - Interop.Advapi32.LogonFlags logonFlags = (Interop.Advapi32.LogonFlags)0; - if (startInfo.LoadUserProfile && startInfo.UseCredentialsForNetworkingOnly) - { - throw new ArgumentException(SR.CantEnableConflictingLogonFlags, nameof(startInfo)); - } - else if (startInfo.LoadUserProfile) - { - logonFlags = Interop.Advapi32.LogonFlags.LOGON_WITH_PROFILE; - } - else if (startInfo.UseCredentialsForNetworkingOnly) + try { - logonFlags = Interop.Advapi32.LogonFlags.LOGON_NETCREDENTIALS_ONLY; + retVal = Interop.Advapi32.CreateProcessWithLogonW( + startInfo.UserName, + startInfo.Domain, + (passwordPtr != IntPtr.Zero) ? passwordPtr : (IntPtr)passwordInClearTextPtr, + logonFlags, + null, // we don't need this since all the info is in commandLine + commandLinePtr, + creationFlags, + (IntPtr)environmentBlockPtr, + workingDirectory, + ref startupInfo.StartupInfo, // pointer to STARTUPINFO + ref processInfo // pointer to PROCESS_INFORMATION + ); + if (!retVal) + errorCode = Marshal.GetLastWin32Error(); } - - fixed (char* passwordInClearTextPtr = startInfo.PasswordInClearText ?? string.Empty) - fixed (char* environmentBlockPtr = environmentBlock) - fixed (char* commandLinePtr = &commandLine.GetPinnableReference(terminate: true)) + finally { - IntPtr passwordPtr = (startInfo.Password != null) ? - Marshal.SecureStringToGlobalAllocUnicode(startInfo.Password) : IntPtr.Zero; - - try - { - retVal = Interop.Advapi32.CreateProcessWithLogonW( - startInfo.UserName, - startInfo.Domain, - (passwordPtr != IntPtr.Zero) ? passwordPtr : (IntPtr)passwordInClearTextPtr, - logonFlags, - null, // we don't need this since all the info is in commandLine - commandLinePtr, - creationFlags, - (IntPtr)environmentBlockPtr, - workingDirectory, - ref startupInfo, // pointer to STARTUPINFO - ref processInfo // pointer to PROCESS_INFORMATION - ); - if (!retVal) - errorCode = Marshal.GetLastWin32Error(); - } - finally - { - if (passwordPtr != IntPtr.Zero) - Marshal.ZeroFreeGlobalAllocUnicode(passwordPtr); - } + if (passwordPtr != IntPtr.Zero) + Marshal.ZeroFreeGlobalAllocUnicode(passwordPtr); } } - else + } + else + { + if (!inheritHandles && numberOfHandles != 0) { - fixed (char* environmentBlockPtr = environmentBlock) - fixed (char* commandLinePtr = &commandLine.GetPinnableReference(terminate: true)) + nuint attributeListSize = 0; + bool shouldBeFalse = Interop.Kernel32.InitializeProcThreadAttributeList(null, 1, 0, &attributeListSize); + Debug.Assert(!shouldBeFalse); + Debug.Assert(attributeListSize > 0); + Debug.Assert(attributeListSize < 1024); + + byte* newList = stackalloc byte[(int)attributeListSize]; + if (!Interop.Kernel32.InitializeProcThreadAttributeList(newList, 1, 0, &attributeListSize)) { - retVal = Interop.Kernel32.CreateProcess( - null, // we don't need this since all the info is in commandLine - commandLinePtr, // pointer to the command line string - ref unused_SecAttrs, // address to process security attributes, we don't need to inherit the handle - ref unused_SecAttrs, // address to thread security attributes. - true, // handle inheritance flag - creationFlags, // creation flags - (IntPtr)environmentBlockPtr, // pointer to new environment block - workingDirectory, // pointer to current directory name - ref startupInfo, // pointer to STARTUPINFO - ref processInfo // pointer to PROCESS_INFORMATION - ); - if (!retVal) - errorCode = Marshal.GetLastWin32Error(); + throw new Win32Exception(); + } + lpAttributeList = newList; + + if (!Interop.Kernel32.UpdateProcThreadAttribute( + lpAttributeList, + 0, + Interop.Advapi32.ProcThreadAttribute.HANDLE_LIST, + handlesToInherit, + (nuint)MemoryMarshal.AsBytes(handlesToInherit.Slice(0, numberOfHandles)).Length, + null, + null)) + { + throw new Win32Exception(); } - } - if (processInfo.hProcess != IntPtr.Zero && processInfo.hProcess != new IntPtr(-1)) - Marshal.InitHandle(procSH, processInfo.hProcess); - if (processInfo.hThread != IntPtr.Zero && processInfo.hThread != new IntPtr(-1)) - Interop.Kernel32.CloseHandle(processInfo.hThread); + startupInfo.AttributeList = lpAttributeList; + startupInfo.StartupInfo.cb = sizeof(Interop.Kernel32.STARTUPINFOEX); + creationFlags |= Interop.Advapi32.StartupInfoOptions.EXTENDED_STARTUPINFO_PRESENT; + } - if (!retVal) + fixed (char* environmentBlockPtr = environmentBlock) + fixed (char* commandLinePtr = &commandLine.GetPinnableReference(terminate: true)) { - string nativeErrorMessage = errorCode == Interop.Errors.ERROR_BAD_EXE_FORMAT || errorCode == Interop.Errors.ERROR_EXE_MACHINE_TYPE_MISMATCH - ? SR.InvalidApplication - : GetErrorMessage(errorCode); - - throw CreateExceptionForErrorStartingProcess(nativeErrorMessage, errorCode, startInfo.FileName, workingDirectory); + retVal = Interop.Kernel32.CreateProcess( + null, // we don't need this since all the info is in commandLine + commandLinePtr, // pointer to the command line string + ref unused_SecAttrs, // address to process security attributes, we don't need to inherit the handle + ref unused_SecAttrs, // address to thread security attributes. + inheritHandles || numberOfHandles != 0, // handle inheritance flag + creationFlags, // creation flags + (IntPtr)environmentBlockPtr, // pointer to new environment block + workingDirectory, // pointer to current directory name + ref startupInfo, // pointer to STARTUPINFOEX + ref processInfo // pointer to PROCESS_INFORMATION + ); + if (!retVal) + errorCode = Marshal.GetLastWin32Error(); } } - catch + + if (processInfo.hProcess != IntPtr.Zero && processInfo.hProcess != new IntPtr(-1)) + Marshal.InitHandle(procSH, processInfo.hProcess); + if (processInfo.hThread != IntPtr.Zero && processInfo.hThread != new IntPtr(-1)) + Interop.Kernel32.CloseHandle(processInfo.hThread); + + if (!retVal) { - parentInputPipeHandle?.Dispose(); - parentOutputPipeHandle?.Dispose(); - parentErrorPipeHandle?.Dispose(); - procSH.Dispose(); - throw; + string nativeErrorMessage = errorCode == Interop.Errors.ERROR_BAD_EXE_FORMAT || errorCode == Interop.Errors.ERROR_EXE_MACHINE_TYPE_MISMATCH + ? SR.InvalidApplication + : GetErrorMessage(errorCode); + + throw CreateExceptionForErrorStartingProcess(nativeErrorMessage, errorCode, startInfo.FileName, workingDirectory); } - finally + } + catch + { + parentInputPipeHandle?.Dispose(); + parentOutputPipeHandle?.Dispose(); + parentErrorPipeHandle?.Dispose(); + procSH.Dispose(); + throw; + } + finally + { + if (lpAttributeList != null) { - childInputPipeHandle?.Dispose(); - childOutputPipeHandle?.Dispose(); - childErrorPipeHandle?.Dispose(); + Interop.Kernel32.DeleteProcThreadAttributeList(lpAttributeList); } + childInputPipeHandle?.Dispose(); + childOutputPipeHandle?.Dispose(); + childErrorPipeHandle?.Dispose(); } if (startInfo.RedirectStandardInput) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Unix.cs index 9664815dcde79..b72b6fd727175 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Unix.cs @@ -52,5 +52,16 @@ public SecureString? Password get { throw new PlatformNotSupportedException(SR.Format(SR.ProcessStartSingleFeatureNotSupported, nameof(Password))); } set { throw new PlatformNotSupportedException(SR.Format(SR.ProcessStartSingleFeatureNotSupported, nameof(Password))); } } + + /// + /// If true, the child process inherits handles from this process that were opened as + /// inheritable. + /// + public bool InheritHandles + { + get { return true; } + [SupportedOSPlatform("windows")] + set { throw new PlatformNotSupportedException(SR.Format(SR.ProcessStartSingleFeatureNotSupported, nameof(InheritHandles))); } + } } } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Windows.cs index 8869c1809341f..e1530b0efa762 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Windows.cs @@ -46,5 +46,16 @@ public string Domain [CLSCompliant(false)] [SupportedOSPlatform("windows")] public SecureString? Password { get; set; } + + /// + /// If true, the child process inherits handles from this process that were opened as + /// inheritable. + /// + public bool InheritHandles + { + get; + [SupportedOSPlatform("windows")] + set; + } = true; } } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs index 728e52f520208..f32136e77d9b0 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs @@ -1014,6 +1014,32 @@ public void Password_GetSetUnix_ThrowsPlatformNotSupportedException() Assert.Throws(() => info.Password = new SecureString()); } + [Fact] + public void InheritHandles_GetDefaultValue_ReturnsExpected() + { + var info = new ProcessStartInfo(); + Assert.True(info.InheritHandles); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public void InheritHandles_Set_GetReturnsExpected() + { + var info = new ProcessStartInfo(); + Assert.True(info.InheritHandles); + info.InheritHandles = false; + Assert.False(info.InheritHandles); + } + + [Fact] + [PlatformSpecific(TestPlatforms.AnyUnix)] + public void InheritHandles_SetUnix_Throws() + { + var info = new ProcessStartInfo(); + Assert.Throws(() => info.InheritHandles = true); + Assert.Throws(() => info.InheritHandles = false); + } + [Theory] [InlineData(null)] [InlineData("")] diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStreamReadTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStreamReadTests.cs index 55a38beec9089..8531df1e51715 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStreamReadTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStreamReadTests.cs @@ -16,11 +16,18 @@ namespace System.Diagnostics.Tests { public class ProcessStreamReadTests : ProcessTestBase { - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void TestSyncErrorStream() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] + [InlineData(false)] + public void TestSyncErrorStream(bool inheritHandles) { Process p = CreateProcessPortable(RemotelyInvokable.ErrorProcessBody); p.StartInfo.RedirectStandardError = true; + if (OperatingSystem.IsWindows()) + { + // Ensure redirecting works regardless of the InheritHandles setting. + p.StartInfo.InheritHandles = inheritHandles; + } p.Start(); string expected = RemotelyInvokable.TestConsoleApp + " started error stream" + Environment.NewLine + RemotelyInvokable.TestConsoleApp + " closed error stream" + Environment.NewLine; @@ -86,11 +93,18 @@ public void TestAsyncErrorStream_SynchronizingObject(bool invokeRequired) Assert.Equal(invokeRequired ? 3 : 0, invokeCalled); } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void TestSyncOutputStream() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] + [InlineData(false)] + public void TestSyncOutputStream(bool inheritHandles) { Process p = CreateProcessPortable(RemotelyInvokable.StreamBody); p.StartInfo.RedirectStandardOutput = true; + if (OperatingSystem.IsWindows()) + { + // Ensure redirecting works regardless of the InheritHandles setting. + p.StartInfo.InheritHandles = inheritHandles; + } p.Start(); string s = p.StandardOutput.ReadToEnd(); Assert.True(p.WaitForExit(WaitInMS)); @@ -380,13 +394,20 @@ public async Task ReadAsync_OutputStreams_Cancel_RespondsQuickly() } } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void TestSyncStreams() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] + [InlineData(false)] + public void TestSyncStreams(bool inheritHandles) { const string expected = "This string should come as output"; Process p = CreateProcessPortable(RemotelyInvokable.ReadLine); p.StartInfo.RedirectStandardInput = true; p.StartInfo.RedirectStandardOutput = true; + if (OperatingSystem.IsWindows()) + { + // Ensure redirecting works regardless of the InheritHandles setting. + p.StartInfo.InheritHandles = inheritHandles; + } p.OutputDataReceived += (s, e) => { Assert.Equal(expected, e.Data); }; p.Start(); using (StreamWriter writer = p.StandardInput) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index 7f66696c6b275..29f8c63299a4b 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -15,6 +15,7 @@ using Xunit; using Microsoft.DotNet.RemoteExecutor; using Microsoft.DotNet.XUnitExtensions; +using Microsoft.Win32.SafeHandles; namespace System.Diagnostics.Tests { @@ -1031,6 +1032,11 @@ private string WriteScriptFile(string directory, string name, int returnValue) return filename; } + private static bool FileHandleIsValid(SafeFileHandle fileHandle) + { + throw new PlatformNotSupportedException(); + } + private static string StartAndReadToEnd(string filename, string[] arguments) { var psi = new ProcessStartInfo diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs index 6fb91c0f4e113..893eceaca1ffc 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs @@ -1,8 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; using System.IO; +using Microsoft.Win32.SafeHandles; namespace System.Diagnostics.Tests { @@ -15,5 +15,10 @@ private string WriteScriptFile(string directory, string name, int returnValue) File.WriteAllText(filename, $"exit {returnValue}"); return filename; } + + private static bool FileHandleIsValid(SafeFileHandle fileHandle) + { + return Interop.Kernel32.GetFileType(fileHandle) == Interop.Kernel32.FileTypes.FILE_TYPE_DISK; + } } } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index 6907a43b63040..92d8852b0863d 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -5,6 +5,7 @@ using System.ComponentModel; using System.Diagnostics; using System.DirectoryServices.ActiveDirectory; +using System.Globalization; using System.IO; using System.IO.Pipes; using System.Linq; @@ -254,6 +255,31 @@ public void ProcessStart_UseShellExecute_OnWindows_OpenMissingFile_Throws() fileToOpen); } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows))] + public void ProcessStart_UseShellExecute_DisableInheritHandles_ThrowsInvalidOperationException() + { + var psi = new ProcessStartInfo + { + FileName = "cmd.exe", + UseShellExecute = true, + InheritHandles = false, + }; + AssertExtensions.Throws(() => Process.Start(psi)); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows))] + public void ProcessStart_UseUserName_DisableInheritHandles_ThrowsInvalidOperationException() + { + var psi = new ProcessStartInfo + { + FileName = "cmd.exe", + InheritHandles = false, + UserName = "Administrator", + PasswordInClearText = "password", + }; + AssertExtensions.Throws(() => Process.Start(psi)); + } + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.HasWindowsShell))] [InlineData(true)] [InlineData(false)] @@ -2766,5 +2792,94 @@ private static void ExecuteWithRetryOnLinux(Action test) test(); } } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [PlatformSpecific(TestPlatforms.Windows)] + [InlineData(true, false)] + [InlineData(false, false)] + [InlineData(true, true)] + [InlineData(false, true)] + public void ProcessStart_InheritHandles_InfluencesTheInheritanceOfHandles(bool inheritHandles, bool redirectStreams) + { + using (var tmpFile = File.Open(GetTestFilePath(), FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.ReadWrite | FileShare.Inheritable)) + { + tmpFile.WriteByte(42); + tmpFile.Flush(); + tmpFile.Position = 0; + + var options = new RemoteInvokeOptions(); + options.StartInfo.InheritHandles = inheritHandles; + if (redirectStreams) + { + options.StartInfo.RedirectStandardInput = true; + options.StartInfo.RedirectStandardOutput = true; + options.StartInfo.RedirectStandardError = true; + } + string handleStr = tmpFile.SafeFileHandle.DangerousGetHandle().ToString(CultureInfo.InvariantCulture); + + using (RemoteInvokeHandle handle = RemoteExecutor.Invoke(static (string handleStr, string inheritHandlesStr, string redirectStreamsStr) => + { + nint handle = nint.Parse(handleStr); + bool inheritHandles = bool.Parse(inheritHandlesStr); + bool redirectStreams = bool.Parse(redirectStreamsStr); + // set ownsHandle to false to prevent trying to close a potentially invalid handle + var fileHandle = new SafeFileHandle(handle, ownsHandle: false); + + if (redirectStreams) + { + Assert.Equal("input", Console.ReadLine()); + Console.WriteLine("output"); + Console.Error.WriteLine("error"); + } + + // To avoid asserts when trying to read from an invalid handle, + // ensure that what we have is at least a normal file. + bool handleIsValid = FileHandleIsValid(fileHandle); + if (inheritHandles) + { + Assert.True(handleIsValid); + } + else if (!handleIsValid) + { + return RemoteExecutor.SuccessExitCode; + } + + byte[] buf = new byte[100]; + long bytesRead; + try + { + bytesRead = RandomAccess.Read(fileHandle, buf.AsSpan(), 0); + } + catch (Exception) when (!inheritHandles) + { + // It it hard to predict what could go wrong when we are reading from a random + // file handle. + return RemoteExecutor.SuccessExitCode; + } + + if (inheritHandles) + { + Assert.Equal(1, bytesRead); + Assert.Equal(42, buf[0]); + } + else + { + // Hypothetically there could be a handle with the same value in this process + // as in the parent process. Make sure that we are looking at a different file. + Assert.False(bytesRead == 1 && buf[0] == 42); + } + + return RemoteExecutor.SuccessExitCode; + }, handleStr, inheritHandles.ToString(), redirectStreams.ToString(), options)) + { + if (redirectStreams) + { + handle.Process.StandardInput.WriteLine("input"); + Assert.Equal("output", handle.Process.StandardOutput.ReadLine()); + Assert.Equal("error", handle.Process.StandardError.ReadLine()); + } + } + } + } } } diff --git a/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj b/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj index a69d3eb237205..3925bf824c336 100644 --- a/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj +++ b/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj @@ -50,6 +50,10 @@ Link="Common\Interop\Windows\Kernel32\Interop.LoadLibrary.cs" /> + +