From 2ef36fc9e1e761aca599f822b2481a7e80f24636 Mon Sep 17 00:00:00 2001 From: Gerardo Grignoli Date: Sat, 17 Feb 2024 09:57:41 -0300 Subject: [PATCH] Restricted process and thread handles in TokenSwitchMode. Restricted ACL of the elevated process to High Integrity. --- src/gsudo/Helpers/ProcessFactory.cs | 46 ++++++++++++--- src/gsudo/{Tokens => Native}/NativeMethods.cs | 56 ++++++++++++------ src/gsudo/Native/ProcessApi.cs | 22 ++++++- src/gsudo/Native/SafeTokenHandle.cs | 15 +++++ src/gsudo/Native/TokensApi.cs | 16 +++++- .../ProcessRenderers/TokenSwitchRenderer.cs | 45 +++++++-------- src/gsudo/Tokens/PrivilegeManager.cs | 3 +- src/gsudo/Tokens/TokenSwitcher.cs | 57 +++++++++++++------ 8 files changed, 186 insertions(+), 74 deletions(-) rename src/gsudo/{Tokens => Native}/NativeMethods.cs (64%) diff --git a/src/gsudo/Helpers/ProcessFactory.cs b/src/gsudo/Helpers/ProcessFactory.cs index 1f41068a..ebb10e47 100644 --- a/src/gsudo/Helpers/ProcessFactory.cs +++ b/src/gsudo/Helpers/ProcessFactory.cs @@ -67,7 +67,7 @@ public static Process StartRedirected(string fileName, string arguments, string public static Process StartAttached(string filename, string arguments) { - Logger.Instance.Log($"Process Start: {filename} {arguments}", LogLevel.Debug ); + Logger.Instance.Log($"Process Start: {filename} {arguments}", LogLevel.Debug); var process = new Process(); process.StartInfo = new ProcessStartInfo(filename) { @@ -128,7 +128,7 @@ public static Process StartWithCredentials(string filename, string arguments, st CreateNoWindow = !InputArguments.Debug, }); } - catch(Win32Exception ex) + catch (Win32Exception ex) { if (ex.NativeErrorCode == 1326) throw new ApplicationException("The user name or password is incorrect."); @@ -332,7 +332,7 @@ private static SafeProcessHandle CreateProcessWithToken(IntPtr newToken, string return new SafeProcessHandle(processInformation.hProcess, true); } - internal static SafeProcessHandle CreateProcessAsUserWithFlags(string lpApplicationName, string args, ProcessApi.CreateProcessFlags dwCreationFlags, out PROCESS_INFORMATION pInfo) + internal static void CreateProcessForTokenReplacement(string lpApplicationName, string args, ProcessApi.CreateProcessFlags dwCreationFlags, out SafeProcessHandle processHandle, out SafeHandle threadHandle, out int processId) { var sInfoEx = new ProcessApi.STARTUPINFOEX(); sInfoEx.StartupInfo.cb = Marshal.SizeOf(sInfoEx); @@ -342,7 +342,9 @@ internal static SafeProcessHandle CreateProcessAsUserWithFlags(string lpApplicat pSec.nLength = Marshal.SizeOf(pSec); tSec.nLength = Marshal.SizeOf(tSec); - // Set more restrictive Security Descriptor + // Set a more restrictive Security Descriptor: + // - This code runs at medium integrity, so we dont have permissions to change the SDACL to High integrity level. + // - We will do that in TokenSwitcher.ReplaceProcessToken. string sddl = "D:(D;;GAFAWD;;;S-1-1-0)"; // Deny Generic-All, File-All, and Write-Dac to everyone. IntPtr sd_ptr = new IntPtr(); @@ -354,15 +356,43 @@ internal static SafeProcessHandle CreateProcessAsUserWithFlags(string lpApplicat var command = $"{lpApplicationName} {args}"; - Logger.Instance.Log($"{nameof(CreateProcessAsUserWithFlags)}: {lpApplicationName} {args}", LogLevel.Debug); + PROCESS_INFORMATION pInfo; + Logger.Instance.Log($"Creating target process: {lpApplicationName} {args}", LogLevel.Debug); if (!ProcessApi.CreateProcess(null, command, ref pSec, ref tSec, false, dwCreationFlags, IntPtr.Zero, null, ref sInfoEx, out pInfo)) { throw new Win32Exception((int)ConsoleApi.GetLastError()); } - return new SafeProcessHandle(pInfo.hProcess, true); - } + var currentProcessHandle = ProcessApi.GetCurrentProcess(); + + if (!DuplicateHandle( + currentProcessHandle, // Source process handle is the current process + pInfo.hProcess, // The handle to duplicate + currentProcessHandle, // Target process handle is also the current process + out var restrictedProcessHandle, // The duplicated handle with desired access rights + 0x1000 | 0x00100000 | 0x0001, // Desired access: PROCESS_QUERY_LIMITED_INFORMATION | SYNCHRONIZE | PROCESS_TERMINATE + false, // The handle is not inheritable + 1)) // dwOptions: auto close pInfo.hProcess. + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + if (!DuplicateHandle( + currentProcessHandle, // Source process handle is the current process + pInfo.hThread, // The thread handle to duplicate + currentProcessHandle, // Target process handle is also the current process + out var restrictedThreadHandle, // The duplicated handle with desired access rights + 0x0002, // Desired access: THREAD_SUSPEND_RESUME + false, // The handle is not inheritable + 1)) // dwOptions: auto close pInfo.hThread. + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + processHandle = new SafeProcessHandle(restrictedProcessHandle, true); + threadHandle = new Native.SafeThreadHandle(restrictedThreadHandle); + + processId = pInfo.dwProcessId; + } } } - diff --git a/src/gsudo/Tokens/NativeMethods.cs b/src/gsudo/Native/NativeMethods.cs similarity index 64% rename from src/gsudo/Tokens/NativeMethods.cs rename to src/gsudo/Native/NativeMethods.cs index e71ae796..2f0c4018 100644 --- a/src/gsudo/Tokens/NativeMethods.cs +++ b/src/gsudo/Native/NativeMethods.cs @@ -1,11 +1,10 @@ -using gsudo.Native; -using System; +using System; using System.Runtime.ConstrainedExecution; using System.Runtime.InteropServices; using System.Security.Principal; using static gsudo.Native.TokensApi; -namespace gsudo.Tokens +namespace gsudo.Native { internal static partial class NativeMethods { @@ -21,19 +20,19 @@ internal static partial class NativeMethods internal const int SE_PRIVILEGE_DISABLED = 0x00000000; internal const int ERROR_NOT_ALL_ASSIGNED = 0x00000514; - internal const UInt32 STANDARD_RIGHTS_REQUIRED = 0x000F0000; - internal const UInt32 STANDARD_RIGHTS_READ = 0x00020000; - internal const UInt32 TOKEN_ASSIGN_PRIMARY = 0x0001; - internal const UInt32 TOKEN_DUPLICATE = 0x0002; - internal const UInt32 TOKEN_IMPERSONATE = 0x0004; - internal const UInt32 TOKEN_QUERY = 0x0008; - internal const UInt32 TOKEN_QUERY_SOURCE = 0x0010; - internal const UInt32 TOKEN_ADJUST_PRIVILEGES = 0x0020; - internal const UInt32 TOKEN_ADJUST_GROUPS = 0x0040; - internal const UInt32 TOKEN_ADJUST_DEFAULT = 0x0080; - internal const UInt32 TOKEN_ADJUST_SESSIONID = 0x0100; - internal const UInt32 TOKEN_READ = (STANDARD_RIGHTS_READ | TOKEN_QUERY); - internal const UInt32 TOKEN_ALL_ACCESS = (STANDARD_RIGHTS_REQUIRED | + internal const uint STANDARD_RIGHTS_REQUIRED = 0x000F0000; + internal const uint STANDARD_RIGHTS_READ = 0x00020000; + internal const uint TOKEN_ASSIGN_PRIMARY = 0x0001; + internal const uint TOKEN_DUPLICATE = 0x0002; + internal const uint TOKEN_IMPERSONATE = 0x0004; + internal const uint TOKEN_QUERY = 0x0008; + internal const uint TOKEN_QUERY_SOURCE = 0x0010; + internal const uint TOKEN_ADJUST_PRIVILEGES = 0x0020; + internal const uint TOKEN_ADJUST_GROUPS = 0x0040; + internal const uint TOKEN_ADJUST_DEFAULT = 0x0080; + internal const uint TOKEN_ADJUST_SESSIONID = 0x0100; + internal const uint TOKEN_READ = STANDARD_RIGHTS_READ | TOKEN_QUERY; + internal const uint TOKEN_ALL_ACCESS = STANDARD_RIGHTS_REQUIRED | TOKEN_ASSIGN_PRIMARY | TOKEN_DUPLICATE | TOKEN_IMPERSONATE | @@ -42,7 +41,7 @@ internal static partial class NativeMethods TOKEN_ADJUST_PRIVILEGES | TOKEN_ADJUST_GROUPS | TOKEN_ADJUST_DEFAULT | - TOKEN_ADJUST_SESSIONID); + TOKEN_ADJUST_SESSIONID; [DllImport("kernel32.dll", CharSet = CharSet.Auto, SetLastError = true)] internal static extern IntPtr GetCurrentProcess(); @@ -65,7 +64,13 @@ internal static partial class NativeMethods [DllImport("kernel32.dll", CharSet = CharSet.Auto, SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] - internal static extern Boolean CloseHandle(IntPtr hObject); + internal static extern bool CloseHandle(IntPtr hObject); + + [DllImport("advapi32.dll", SetLastError = true)] + internal static extern bool GetKernelObjectSecurity(IntPtr Handle, uint securityInformation, IntPtr pSecurityDescriptor, uint nLength, out uint lpnLengthNeeded); + + [DllImport("advapi32.dll", SetLastError = true)] + internal static extern bool SetKernelObjectSecurity(IntPtr Handle, uint securityInformation, IntPtr pSecurityDescriptor); [StructLayout(LayoutKind.Sequential)] public struct LUID @@ -100,5 +105,20 @@ public struct TOKEN_PRIVILEGES public LUID_AND_ATTRIBUTES[] Privileges { get => privileges; set => privileges = value; } } + } + + [Flags] + internal enum SECURITY_INFORMATION : uint + { + OWNER_SECURITY_INFORMATION = 0x00000001, + GROUP_SECURITY_INFORMATION = 0x00000002, + DACL_SECURITY_INFORMATION = 0x00000004, + SACL_SECURITY_INFORMATION = 0x00000008, + UNPROTECTED_SACL_SECURITY_INFORMATION = 0x10000000, + UNPROTECTED_DACL_SECURITY_INFORMATION = 0x20000000, + PROTECTED_SACL_SECURITY_INFORMATION = 0x40000000, + PROTECTED_DACL_SECURITY_INFORMATION = 0x80000000 + } + } diff --git a/src/gsudo/Native/ProcessApi.cs b/src/gsudo/Native/ProcessApi.cs index 77bb7406..c024c818 100644 --- a/src/gsudo/Native/ProcessApi.cs +++ b/src/gsudo/Native/ProcessApi.cs @@ -120,7 +120,10 @@ public enum CreateProcessFlags : uint internal static extern bool DeleteProcThreadAttributeList(IntPtr lpAttributeList); [DllImport("kernel32.dll", SetLastError = true)] - internal static extern bool CloseHandle(IntPtr hObject); + internal static extern bool CloseHandle(IntPtr hObject); + + [DllImport("kernel32.dll", SetLastError = true)] + public static extern IntPtr LocalFree(IntPtr hMem); [DllImport("kernel32.dll", CharSet = System.Runtime.InteropServices.CharSet.Auto, SetLastError = true)] internal static extern bool GetExitCodeProcess(Microsoft.Win32.SafeHandles.SafeProcessHandle processHandle, out int exitCode); @@ -190,7 +193,9 @@ protected override bool ReleaseHandle() #region Query Process Info public const UInt32 PROCESS_QUERY_INFORMATION = 0x0400; public const UInt32 PROCESS_SET_INFORMATION = 0x0200; - + public const UInt32 READ_CONTROL = 0x00020000; + public const UInt32 WRITE_DAC = 0x40000; + [DllImport("kernel32.dll", SetLastError = true)] internal static extern IntPtr OpenProcess(UInt32 dwDesiredAccess, Boolean bInheritHandle, UInt32 dwProcessId); @@ -225,7 +230,7 @@ protected override bool ReleaseHandle() internal static extern bool CreatePipe(out SafeFileHandle hReadPipe, out SafeFileHandle hWritePipe, SECURITY_ATTRIBUTES lpPipeAttributes, int nSize); [DllImport("kernel32.dll", SetLastError = true)] - internal static extern uint ResumeThread(IntPtr hThread); + internal static extern int ResumeThread(IntPtr hThread); [DllImport("kernel32.dll", SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] @@ -233,5 +238,16 @@ protected override bool ReleaseHandle() [DllImport("kernel32.dll", SetLastError = true)] internal static extern UInt32 WaitForSingleObject(IntPtr hHandle, UInt32 dwMilliseconds); + + [DllImport("kernel32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool DuplicateHandle( + IntPtr hSourceProcessHandle, + IntPtr hSourceHandle, + IntPtr hTargetProcessHandle, + out IntPtr lpTargetHandle, + uint dwDesiredAccess, + [MarshalAs(UnmanagedType.Bool)] bool bInheritHandle, + uint dwOptions); } } diff --git a/src/gsudo/Native/SafeTokenHandle.cs b/src/gsudo/Native/SafeTokenHandle.cs index ce5fdea3..0328b5c1 100644 --- a/src/gsudo/Native/SafeTokenHandle.cs +++ b/src/gsudo/Native/SafeTokenHandle.cs @@ -21,4 +21,19 @@ protected override bool ReleaseHandle() return Native.ProcessApi.CloseHandle(base.handle); } } + + internal sealed class SafeThreadHandle : SafeHandleZeroOrMinusOneIsInvalid + { + internal SafeThreadHandle(IntPtr handle) + : base(true) + { + base.SetHandle(handle); + } + + override protected bool ReleaseHandle() + { + return Native.ProcessApi.CloseHandle(handle); + } + + } } diff --git a/src/gsudo/Native/TokensApi.cs b/src/gsudo/Native/TokensApi.cs index 7b86315d..5f6bad6d 100644 --- a/src/gsudo/Native/TokensApi.cs +++ b/src/gsudo/Native/TokensApi.cs @@ -414,7 +414,19 @@ public struct LUID #endregion [DllImport("Advapi32.dll", SetLastError = true, CharSet = CharSet.Unicode)] - internal static extern bool ConvertStringSecurityDescriptorToSecurityDescriptor(string StringSecurityDescriptor, uint StringSDRevision, out IntPtr SecurityDescriptor, out UIntPtr SecurityDescriptorSize); - + internal static extern bool ConvertStringSecurityDescriptorToSecurityDescriptor( + string StringSecurityDescriptor, + uint StringSDRevision, + out IntPtr SecurityDescriptor, + out UIntPtr SecurityDescriptorSize); + + // Additional imports for DACL manipulation + [DllImport("advapi32.dll", SetLastError = true)] + internal static extern bool ConvertSecurityDescriptorToStringSecurityDescriptor( + IntPtr SecurityDescriptor, + uint StringSDRevision, + SECURITY_INFORMATION SecurityInformation, + out IntPtr StringSecurityDescriptor, + out uint StringSecurityDescriptorLen); } } diff --git a/src/gsudo/ProcessRenderers/TokenSwitchRenderer.cs b/src/gsudo/ProcessRenderers/TokenSwitchRenderer.cs index faad4a5c..8a6214dd 100644 --- a/src/gsudo/ProcessRenderers/TokenSwitchRenderer.cs +++ b/src/gsudo/ProcessRenderers/TokenSwitchRenderer.cs @@ -4,8 +4,10 @@ using Microsoft.Win32.SafeHandles; using System; using System.Collections.Generic; +using System.ComponentModel; using System.IO; using System.Linq; +using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; @@ -19,8 +21,8 @@ class TokenSwitchRenderer : IProcessRenderer { private readonly Connection _connection; private readonly ElevationRequest _elevationRequest; - private readonly SafeProcessHandle _process; - private readonly ProcessApi.PROCESS_INFORMATION _processInformation; + private readonly SafeProcessHandle _processHandle; + private readonly SafeHandle _threadHandle; private readonly ManualResetEventSlim tokenSwitchSuccessEvent = new ManualResetEventSlim(false); internal TokenSwitchRenderer(Connection connection, ElevationRequest elevationRequest) @@ -55,9 +57,9 @@ internal TokenSwitchRenderer(Connection connection, ElevationRequest elevationRe args = elevationRequest.Arguments; } - _process = ProcessFactory.CreateProcessAsUserWithFlags(exeName, args, dwCreationFlags, out _processInformation); + ProcessFactory.CreateProcessForTokenReplacement(exeName, args, dwCreationFlags, out _processHandle, out _threadHandle, out int processId); - elevationRequest.TargetProcessId = _processInformation.dwProcessId; + elevationRequest.TargetProcessId = processId; if (!elevationRequest.NewWindow) ConsoleApi.SetConsoleCtrlHandler(ConsoleHelper.IgnoreConsoleCancelKeyPress, true); } @@ -68,7 +70,7 @@ public Task Start() { var t1 = new StreamReader(_connection.ControlStream).ConsumeOutput(HandleControlStream); - WaitHandle.WaitAny(new WaitHandle[] { tokenSwitchSuccessEvent.WaitHandle, _process.GetProcessWaitHandle(), _connection.DisconnectedWaitHandle }); + WaitHandle.WaitAny(new WaitHandle[] { tokenSwitchSuccessEvent.WaitHandle, _processHandle.GetProcessWaitHandle(), _connection.DisconnectedWaitHandle }); if (!tokenSwitchSuccessEvent.IsSet) { @@ -87,43 +89,34 @@ public Task Start() _connection.DataStream.Close(); _connection.ControlStream.Close(); - return GetResult(); - } - finally - { - ConsoleApi.SetConsoleCtrlHandler(ConsoleHelper.IgnoreConsoleCancelKeyPress, false); - } - } - - public void TerminateProcess() - { - ProcessApi.TerminateProcess(_process.DangerousGetHandle(), 0); - } + if (ProcessApi.ResumeThread(_threadHandle.DangerousGetHandle()) < 0) + throw new Win32Exception(); - public Task GetResult() - { - try - { - _ = ProcessApi.ResumeThread(_processInformation.hThread); - Native.FileApi.CloseHandle(_processInformation.hThread); + _threadHandle.Close(); if (_elevationRequest.Wait) { - _process.GetProcessWaitHandle().WaitOne(); - if (ProcessApi.GetExitCodeProcess(_process, out int exitCode)) + _processHandle.GetProcessWaitHandle().WaitOne(); + if (ProcessApi.GetExitCodeProcess(_processHandle, out int exitCode)) return Task.FromResult(exitCode); - Native.FileApi.CloseHandle(_processInformation.hProcess); + _processHandle.Close(); } return Task.FromResult(0); } finally { + _processHandle?.Close(); + _threadHandle?.Close(); ConsoleApi.SetConsoleCtrlHandler(ConsoleHelper.IgnoreConsoleCancelKeyPress, false); } } + public void TerminateProcess() + { + ProcessApi.TerminateProcess(_processHandle.DangerousGetHandle(), 0); + } enum Mode { Normal, Error}; Mode CurrentMode = Mode.Normal; diff --git a/src/gsudo/Tokens/PrivilegeManager.cs b/src/gsudo/Tokens/PrivilegeManager.cs index a6334ec9..d29d38cf 100644 --- a/src/gsudo/Tokens/PrivilegeManager.cs +++ b/src/gsudo/Tokens/PrivilegeManager.cs @@ -3,7 +3,8 @@ using System.Runtime.InteropServices; using System.Security.Principal; using static gsudo.Native.TokensApi; -using static gsudo.Tokens.NativeMethods; +using static gsudo.Native.NativeMethods; +using gsudo.Native; namespace gsudo.Tokens { diff --git a/src/gsudo/Tokens/TokenSwitcher.cs b/src/gsudo/Tokens/TokenSwitcher.cs index 2510bc6d..44fcffaf 100644 --- a/src/gsudo/Tokens/TokenSwitcher.cs +++ b/src/gsudo/Tokens/TokenSwitcher.cs @@ -3,6 +3,8 @@ using System.Runtime.InteropServices; using gsudo.Helpers; using gsudo.Native; +using static gsudo.Native.TokensApi; +using static gsudo.Native.NativeMethods; namespace gsudo.Tokens { @@ -27,28 +29,51 @@ public static void ReplaceProcessToken(ElevationRequest elevationRequest) .EnablePrivilege(Privilege.SeTcbPrivilege, false) .EnablePrivilege(Privilege.SeIncreaseQuotaPrivilege, false) .Impersonate(() => - { - IntPtr hProcess = ProcessApi.OpenProcess(ProcessApi.PROCESS_SET_INFORMATION, true, - (uint)elevationRequest.TargetProcessId); - NtDllApi.PROCESS_INFORMATION_CLASS processInformationClass = - NtDllApi.PROCESS_INFORMATION_CLASS.ProcessAccessToken; - - int res = NtDllApi.NativeMethods.NtSetInformationProcess(hProcess, processInformationClass, - ref tokenInfo, - Marshal.SizeOf()); - Logger.Instance.Log($"NtSetInformationProcess returned {res}", LogLevel.Debug); - if (res < 0) - throw new Win32Exception(); - - ProcessApi.CloseHandle(hProcess); + { + IntPtr securityDescriptor = IntPtr.Zero; + UIntPtr securityDescriptorSize = UIntPtr.Zero; + IntPtr hProcess = IntPtr.Zero; + try + { + hProcess = ProcessApi.OpenProcess(ProcessApi.PROCESS_SET_INFORMATION | ProcessApi.PROCESS_QUERY_INFORMATION | ProcessApi.READ_CONTROL | ProcessApi.WRITE_DAC, true, + (uint)elevationRequest.TargetProcessId); + + // Tighthen the security descriptor of the process to elevate, before elevating it. + string sddl = "D:(D;;GAFAWD;;;S-1-1-0)S:(ML;;NW;;;HI)"; // Deny all to everyone. SACL requires High Integrity. + Native.TokensApi.ConvertStringSecurityDescriptorToSecurityDescriptor(sddl, StringSDRevision: 1, out securityDescriptor, out securityDescriptorSize); + + // https://learn.microsoft.com/en-us/windows/win32/secauthz/low-level-security-descriptor-functions + if (!SetKernelObjectSecurity(hProcess, (uint)SECURITY_INFORMATION.DACL_SECURITY_INFORMATION, securityDescriptor)) + throw new InvalidOperationException("Failed to tighten security descriptor.", new Win32Exception()); + + // Replace the Process access token with an elevated one. + var processInformationClass = NtDllApi.PROCESS_INFORMATION_CLASS.ProcessAccessToken; + + int res = NtDllApi.NativeMethods.NtSetInformationProcess(hProcess, processInformationClass, + ref tokenInfo, Marshal.SizeOf()); + + if (res < 0) + throw new Win32Exception(); + + Logger.Instance.Log($"Process token replaced", LogLevel.Debug); + } + finally + { + if (hProcess != IntPtr.Zero) + ProcessApi.CloseHandle(hProcess); + + // Cleanup: Free the security descriptor pointer if it's not null + if (securityDescriptor != IntPtr.Zero) + ProcessApi.LocalFree(securityDescriptor); + } }); } finally { desiredToken?.Close(); } - } - + } + private static SafeTokenHandle GetDesiredToken(ElevationRequest elevationRequest) { TokenProvider tm = null;