From ed2f5cc225365268acd36eab1562ab02a1c3e89d Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Thu, 24 Jun 2021 12:12:13 +0200 Subject: [PATCH] api: don't auto-elevate There's no longer a need to do this for every API call. This only exists now for the pnp guid reuse workaround hack. Signed-off-by: Jason A. Donenfeld --- api/adapter.c | 57 +++++++---------------------- api/elevate.c | 95 ++++-------------------------------------------- api/elevate.h | 4 -- api/entry.c | 43 +++++++++++++++++++++- api/entry.h | 1 + api/namespace.c | 4 +- api/rundll32_i.c | 12 +----- api/session.c | 14 ++----- 8 files changed, 69 insertions(+), 161 deletions(-) diff --git a/api/adapter.c b/api/adapter.c index c15a420..3134b6d 100644 --- a/api/adapter.c +++ b/api/adapter.c @@ -668,18 +668,13 @@ WintunFreeAdapter(_In_ WINTUN_ADAPTER *Adapter) _Return_type_success_(return != NULL) WINTUN_ADAPTER *WINAPI WintunOpenAdapter(_In_z_ const WCHAR *Pool, _In_z_ const WCHAR *Name) { - if (!ElevateToSystem()) - { - LOG(WINTUN_LOG_ERR, L"Failed to impersonate SYSTEM user"); - return NULL; - } DWORD LastError; WINTUN_ADAPTER *Adapter = NULL; HANDLE Mutex = NamespaceTakePoolMutex(Pool); if (!Mutex) { LastError = LOG(WINTUN_LOG_ERR, L"Failed to take %s pool mutex", Pool); - goto cleanupToken; + goto cleanup; } HDEVINFO DevInfo = SetupDiGetClassDevsExW(&GUID_DEVCLASS_NET, NULL, NULL, DIGCF_PRESENT, NULL, NULL, NULL); @@ -759,8 +754,7 @@ cleanupDevInfo: SetupDiDestroyDeviceInfoList(DevInfo); cleanupMutex: NamespaceReleaseMutex(Mutex); -cleanupToken: - RevertToSelf(); +cleanup: SetLastError(LastError); return Adapter; } @@ -1840,11 +1834,6 @@ _Return_type_success_(return != NULL) WINTUN_ADAPTER *WINAPI WintunCreateAdapter _In_opt_ const GUID *RequestedGUID, _Out_opt_ BOOL *RebootRequired) { - if (!ElevateToSystem()) - { - LOG(WINTUN_LOG_ERR, L"Failed to impersonate SYSTEM user"); - return NULL; - } BOOL DummyRebootRequired; if (!RebootRequired) RebootRequired = &DummyRebootRequired; @@ -1855,12 +1844,11 @@ _Return_type_success_(return != NULL) WINTUN_ADAPTER *WINAPI WintunCreateAdapter { Adapter = CreateAdapterViaRundll32(Pool, Name, RequestedGUID, RebootRequired); LastError = Adapter ? ERROR_SUCCESS : GetLastError(); - goto cleanupToken; + goto cleanup; } Adapter = CreateAdapter(Pool, Name, RequestedGUID, RebootRequired); LastError = Adapter ? ERROR_SUCCESS : GetLastError(); -cleanupToken: - RevertToSelf(); +cleanup: return RET_ERROR(Adapter, LastError); } @@ -1869,11 +1857,6 @@ _Return_type_success_(return != FALSE) BOOL WINAPI WintunDeleteAdapter( _In_ BOOL ForceCloseSessions, _Out_opt_ BOOL *RebootRequired) { - if (!ElevateToSystem()) - { - LOG(WINTUN_LOG_ERR, L"Failed to impersonate SYSTEM user"); - return FALSE; - } BOOL DummyRebootRequired; if (!RebootRequired) RebootRequired = &DummyRebootRequired; @@ -1883,14 +1866,14 @@ _Return_type_success_(return != FALSE) BOOL WINAPI WintunDeleteAdapter( { LastError = DeleteAdapterViaRundll32(Adapter, ForceCloseSessions, RebootRequired) ? ERROR_SUCCESS : GetLastError(); - goto cleanupToken; + goto cleanup; } HANDLE Mutex = NamespaceTakePoolMutex(Adapter->Pool); if (!Mutex) { LastError = LOG(WINTUN_LOG_ERR, L"Failed to take %s pool mutex", Adapter->Pool); - goto cleanupToken; + goto cleanup; } HDEVINFO DevInfo; @@ -1930,8 +1913,7 @@ cleanupDevInfo: SetupDiDestroyDeviceInfoList(DevInfo); cleanupMutex: NamespaceReleaseMutex(Mutex); -cleanupToken: - RevertToSelf(); +cleanup: return RET_ERROR(TRUE, LastError); } @@ -1990,12 +1972,6 @@ cleanupMutex: _Return_type_success_(return != FALSE) BOOL WINAPI WintunDeletePoolDriver(_In_z_ const WCHAR *Pool, _Out_opt_ BOOL *RebootRequired) { - if (!ElevateToSystem()) - { - LOG(WINTUN_LOG_ERR, L"Failed to impersonate SYSTEM user"); - return FALSE; - } - BOOL DummyRebootRequired; if (!RebootRequired) RebootRequired = &DummyRebootRequired; @@ -2005,20 +1981,20 @@ _Return_type_success_(return != FALSE) BOOL WINAPI if (MAYBE_WOW64 && NativeMachine != IMAGE_FILE_PROCESS) { LastError = DeletePoolDriverViaRundll32(Pool, RebootRequired) ? ERROR_SUCCESS : GetLastError(); - goto cleanupToken; + goto cleanup; } if (!DeleteAllOurAdapters(Pool, RebootRequired)) { LastError = GetLastError(); - goto cleanupToken; + goto cleanup; } HANDLE DriverInstallationLock = NamespaceTakeDriverInstallationMutex(); if (!DriverInstallationLock) { LastError = LOG(WINTUN_LOG_ERR, L"Failed to take driver installation mutex"); - goto cleanupToken; + goto cleanup; } HDEVINFO DeviceInfoSet = SetupDiGetClassDevsW(&GUID_DEVCLASS_NET, NULL, NULL, 0); if (!DeviceInfoSet) @@ -2060,25 +2036,19 @@ cleanupDeviceInfoSet: SetupDiDestroyDeviceInfoList(DeviceInfoSet); cleanupDriverInstallationLock: NamespaceReleaseMutex(DriverInstallationLock); -cleanupToken: - RevertToSelf(); +cleanup: return RET_ERROR(TRUE, LastError); } _Return_type_success_(return != FALSE) BOOL WINAPI WintunEnumAdapters(_In_z_ const WCHAR *Pool, _In_ WINTUN_ENUM_CALLBACK Func, _In_ LPARAM Param) { - if (!ElevateToSystem()) - { - LOG(WINTUN_LOG_ERR, L"Failed to impersonate SYSTEM user"); - return FALSE; - } DWORD LastError = ERROR_SUCCESS; HANDLE Mutex = NamespaceTakePoolMutex(Pool); if (!Mutex) { LastError = LOG(WINTUN_LOG_ERR, L"Failed to take %s pool mutex", Pool); - goto cleanupToken; + goto cleanup; } HDEVINFO DevInfo = SetupDiGetClassDevsExW(&GUID_DEVCLASS_NET, NULL, NULL, DIGCF_PRESENT, NULL, NULL, NULL); if (DevInfo == INVALID_HANDLE_VALUE) @@ -2112,7 +2082,6 @@ _Return_type_success_(return != FALSE) BOOL WINAPI SetupDiDestroyDeviceInfoList(DevInfo); cleanupMutex: NamespaceReleaseMutex(Mutex); -cleanupToken: - RevertToSelf(); +cleanup: return RET_ERROR(TRUE, LastError); } diff --git a/api/elevate.c b/api/elevate.c index 2daed6b..61cd7ee 100644 --- a/api/elevate.c +++ b/api/elevate.c @@ -9,19 +9,19 @@ #include #include -_Return_type_success_(return != FALSE) BOOL ElevateToSystem(void) +static _Return_type_success_(return != FALSE) BOOL ElevateToSystem(void) { HANDLE CurrentProcessToken, ThreadToken, ProcessSnapshot, WinlogonProcess, WinlogonToken, DuplicatedToken; PROCESSENTRY32W ProcessEntry = { .dwSize = sizeof(PROCESSENTRY32W) }; BOOL Ret; DWORD LastError = ERROR_SUCCESS; TOKEN_PRIVILEGES Privileges = { .PrivilegeCount = 1, .Privileges = { { .Attributes = SE_PRIVILEGE_ENABLED } } }; - CHAR LocalSystemSid[0x400]; + CHAR LocalSystemSid[MAX_SID_SIZE]; DWORD RequiredBytes = sizeof(LocalSystemSid); struct { TOKEN_USER MaybeLocalSystem; - CHAR LargeEnoughForLocalSystem[0x400]; + CHAR LargeEnoughForLocalSystem[MAX_SID_SIZE]; } TokenUserBuffer; Ret = CreateWellKnownSid(WinLocalSystemSid, NULL, &LocalSystemSid, &RequiredBytes); @@ -46,7 +46,7 @@ _Return_type_success_(return != FALSE) BOOL ElevateToSystem(void) goto cleanup; } if (EqualSid(TokenUserBuffer.MaybeLocalSystem.User.Sid, LocalSystemSid)) - return TRUE; + return ImpersonateSelf(SecurityImpersonation); Ret = LookupPrivilegeValueW(NULL, SE_DEBUG_NAME, &Privileges.Privileges[0].Luid); if (!Ret) { @@ -122,81 +122,6 @@ cleanup: return FALSE; } -_Return_type_success_(return != NULL) HANDLE GetPrimarySystemTokenFromThread(void) -{ - HANDLE CurrentToken, DuplicatedToken; - BOOL Ret; - DWORD LastError; - TOKEN_PRIVILEGES Privileges = { .PrivilegeCount = 1, .Privileges = { { .Attributes = SE_PRIVILEGE_ENABLED } } }; - CHAR LocalSystemSid[0x400]; - DWORD RequiredBytes = sizeof(LocalSystemSid); - struct - { - TOKEN_USER MaybeLocalSystem; - CHAR LargeEnoughForLocalSystem[0x400]; - } TokenUserBuffer; - - Ret = CreateWellKnownSid(WinLocalSystemSid, NULL, &LocalSystemSid, &RequiredBytes); - if (!Ret) - { - LastError = LOG_LAST_ERROR(L"Failed to create SID"); - return NULL; - } - Ret = OpenThreadToken( - GetCurrentThread(), TOKEN_QUERY | TOKEN_ADJUST_PRIVILEGES | TOKEN_DUPLICATE, FALSE, &CurrentToken); - if (!Ret && GetLastError() == ERROR_NO_TOKEN) - Ret = OpenProcessToken( - GetCurrentProcess(), TOKEN_QUERY | TOKEN_ADJUST_PRIVILEGES | TOKEN_DUPLICATE, &CurrentToken); - if (!Ret) - { - LastError = LOG_LAST_ERROR(L"Failed to open token"); - return NULL; - } - Ret = GetTokenInformation(CurrentToken, TokenUser, &TokenUserBuffer, sizeof(TokenUserBuffer), &RequiredBytes); - if (!Ret) - { - LastError = LOG_LAST_ERROR(L"Failed to get token information"); - goto cleanup; - } - if (!EqualSid(TokenUserBuffer.MaybeLocalSystem.User.Sid, LocalSystemSid)) - { - LOG(WINTUN_LOG_ERR, L"Not SYSTEM"); - LastError = ERROR_ACCESS_DENIED; - goto cleanup; - } - Ret = LookupPrivilegeValueW(NULL, SE_ASSIGNPRIMARYTOKEN_NAME, &Privileges.Privileges[0].Luid); - if (!Ret) - { - LastError = LOG_LAST_ERROR(L"Failed to lookup privilege value"); - goto cleanup; - } - Ret = AdjustTokenPrivileges(CurrentToken, FALSE, &Privileges, 0, NULL, NULL); - if (!Ret) - { - LastError = LOG_LAST_ERROR(L"Failed to adjust token privileges"); - goto cleanup; - } - Ret = DuplicateTokenEx( - CurrentToken, - TOKEN_QUERY | TOKEN_DUPLICATE | TOKEN_ASSIGN_PRIMARY, - NULL, - SecurityImpersonation, - TokenPrimary, - &DuplicatedToken); - if (!Ret) - { - LastError = LOG_LAST_ERROR(L"Failed to duplicate token"); - goto cleanup; - } - CloseHandle(CurrentToken); - return DuplicatedToken; - -cleanup: - CloseHandle(CurrentToken); - SetLastError(LastError); - return NULL; -} - _Return_type_success_(return != FALSE) BOOL ImpersonateService(_In_z_ WCHAR *ServiceName, _In_ HANDLE *OriginalToken) { HANDLE ThreadToken, ServiceProcess, ServiceToken, DuplicatedToken; @@ -212,20 +137,14 @@ _Return_type_success_(return != FALSE) BOOL ImpersonateService(_In_z_ WCHAR *Ser GetLastError() != ERROR_NO_TOKEN) return FALSE; + if (!ElevateToSystem()) + goto cleanup; + if (!LookupPrivilegeValueW(NULL, SE_DEBUG_NAME, &Privileges.Privileges[0].Luid)) { LastError = LOG_LAST_ERROR(L"Failed to lookup privilege value"); goto cleanup; } - if (!*OriginalToken) - { - RevertToSelf(); - if (!ImpersonateSelf(SecurityImpersonation)) - { - LastError = LOG_LAST_ERROR(L"Failed to impersonate self"); - goto cleanup; - } - } if (!OpenThreadToken(GetCurrentThread(), TOKEN_ADJUST_PRIVILEGES, FALSE, &ThreadToken)) { LastError = LOG_LAST_ERROR(L"Failed to open thread token"); diff --git a/api/elevate.h b/api/elevate.h index 511f23d..04367b4 100644 --- a/api/elevate.h +++ b/api/elevate.h @@ -7,10 +7,6 @@ #include -_Return_type_success_(return != FALSE) BOOL ElevateToSystem(void); - -_Return_type_success_(return != NULL) HANDLE GetPrimarySystemTokenFromThread(void); - _Return_type_success_(return != FALSE) BOOL ImpersonateService(_In_z_ WCHAR *ServiceName, _In_ HANDLE *OriginalToken); _Return_type_success_(return != FALSE) BOOL RestoreToken(_In_ HANDLE OriginalToken); diff --git a/api/entry.c b/api/entry.c index 23d20bc..7402f5c 100644 --- a/api/entry.c +++ b/api/entry.c @@ -21,6 +21,7 @@ HINSTANCE ResourceModule; HANDLE ModuleHeap; SECURITY_ATTRIBUTES SecurityAttributes = { .nLength = sizeof(SECURITY_ATTRIBUTES) }; +BOOL IsLocalSystem; static FARPROC WINAPI DelayedLoadLibraryHook(unsigned dliNotify, PDelayLoadInfo pdli) @@ -35,6 +36,41 @@ DelayedLoadLibraryHook(unsigned dliNotify, PDelayLoadInfo pdli) const PfnDliHook __pfnDliNotifyHook2 = DelayedLoadLibraryHook; +static BOOL +InitializeSecurityObjects(void) +{ + BYTE LocalSystemSid[MAX_SID_SIZE]; + DWORD RequiredBytes = sizeof(LocalSystemSid); + HANDLE CurrentProcessToken; + struct + { + TOKEN_USER MaybeLocalSystem; + CHAR LargeEnoughForLocalSystem[MAX_SID_SIZE]; + } TokenUserBuffer; + BOOL Ret = FALSE; + + if (!CreateWellKnownSid(WinLocalSystemSid, NULL, LocalSystemSid, &RequiredBytes)) + return FALSE; + + if (!OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &CurrentProcessToken)) + return FALSE; + + if (!GetTokenInformation(CurrentProcessToken, TokenUser, &TokenUserBuffer, sizeof(TokenUserBuffer), &RequiredBytes)) + goto cleanupProcessToken; + + IsLocalSystem = EqualSid(TokenUserBuffer.MaybeLocalSystem.User.Sid, LocalSystemSid); + Ret = ConvertStringSecurityDescriptorToSecurityDescriptorW( + IsLocalSystem ? L"O:SYD:P(A;;GA;;;SY)(A;;GA;;;BA)S:(ML;;NWNRNX;;;HI)" + : L"O:BAD:P(A;;GA;;;SY)(A;;GA;;;BA)S:(ML;;NWNRNX;;;HI)", + SDDL_REVISION_1, + &SecurityAttributes.lpSecurityDescriptor, + NULL); + +cleanupProcessToken: + CloseHandle(CurrentProcessToken); + return Ret; +} + BOOL APIENTRY DllMain(_In_ HINSTANCE hinstDLL, _In_ DWORD fdwReason, _In_ LPVOID lpvReserved) { @@ -47,8 +83,11 @@ DllMain(_In_ HINSTANCE hinstDLL, _In_ DWORD fdwReason, _In_ LPVOID lpvReserved) ModuleHeap = HeapCreate(0, 0, 0); if (!ModuleHeap) return FALSE; - ConvertStringSecurityDescriptorToSecurityDescriptorW( - L"O:SYD:P(A;;GA;;;SY)", SDDL_REVISION_1, &SecurityAttributes.lpSecurityDescriptor, NULL); + if (!InitializeSecurityObjects()) + { + HeapDestroy(ModuleHeap); + return FALSE; + } AdapterInit(); NamespaceInit(); break; diff --git a/api/entry.h b/api/entry.h index 65405b7..9f6b0de 100644 --- a/api/entry.h +++ b/api/entry.h @@ -29,3 +29,4 @@ extern HINSTANCE ResourceModule; extern HANDLE ModuleHeap; extern SECURITY_ATTRIBUTES SecurityAttributes; +extern BOOL IsLocalSystem; diff --git a/api/namespace.c b/api/namespace.c index 34ade2c..735e585 100644 --- a/api/namespace.c +++ b/api/namespace.c @@ -59,8 +59,8 @@ static _Return_type_success_(return != FALSE) BOOL NamespaceRuntimeInit(void) } BYTE Sid[MAX_SID_SIZE]; - DWORD SidSize = MAX_SID_SIZE; - if (!CreateWellKnownSid(WinLocalSystemSid, NULL, Sid, &SidSize)) + DWORD SidSize = sizeof(Sid); + if (!CreateWellKnownSid(IsLocalSystem ? WinLocalSystemSid : WinBuiltinAdministratorsSid, NULL, Sid, &SidSize)) { LastError = LOG_LAST_ERROR(L"Failed to create SID"); goto cleanupBCryptCloseAlgorithmProvider; diff --git a/api/rundll32_i.c b/api/rundll32_i.c index 01ba6b6..17598da 100644 --- a/api/rundll32_i.c +++ b/api/rundll32_i.c @@ -176,23 +176,15 @@ static _Return_type_success_(return != FALSE) BOOL ExecuteRunDll32( .hStdOutput = StreamWStdout, .hStdError = StreamWStderr }; PROCESS_INFORMATION pi; - HANDLE ProcessToken = GetPrimarySystemTokenFromThread(); - if (!ProcessToken) - { - LastError = LOG(WINTUN_LOG_ERR, L"Failed to get primary system token from thread"); - goto cleanupThreads; - } - if (!CreateProcessAsUserW(ProcessToken, RunDll32Path, CommandLine, NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi)) + if (!CreateProcessW(RunDll32Path, CommandLine, NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi)) { LastError = LOG_LAST_ERROR(L"Failed to create process: %s", CommandLine); - goto cleanupToken; + goto cleanupThreads; } LastError = ERROR_SUCCESS; WaitForSingleObject(pi.hProcess, INFINITE); CloseHandle(pi.hProcess); CloseHandle(pi.hThread); -cleanupToken: - CloseHandle(ProcessToken); cleanupThreads: if (ThreadStderr) { diff --git a/api/session.c b/api/session.c index 763dd83..d266d5d 100644 --- a/api/session.c +++ b/api/session.c @@ -77,7 +77,7 @@ _Return_type_success_(return != NULL) TUN_SESSION *WINAPI if (!Session) { LastError = GetLastError(); - goto out; + goto cleanup; } const ULONG RingSize = TUN_RING_SIZE(Capacity); BYTE *AllocatedRegion = VirtualAlloc(0, (size_t)RingSize * 2, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE); @@ -86,18 +86,13 @@ _Return_type_success_(return != NULL) TUN_SESSION *WINAPI LastError = LOG_LAST_ERROR(L"Failed to allocate ring memory (requested size: 0x%zx)", (size_t)RingSize * 2); goto cleanupRings; } - if (!ElevateToSystem()) - { - LastError = LOG(WINTUN_LOG_ERR, L"Failed to impersonate SYSTEM user"); - goto cleanupAllocatedRegion; - } Session->Descriptor.Send.RingSize = RingSize; Session->Descriptor.Send.Ring = (TUN_RING *)AllocatedRegion; Session->Descriptor.Send.TailMoved = CreateEventW(&SecurityAttributes, FALSE, FALSE, NULL); if (!Session->Descriptor.Send.TailMoved) { LastError = LOG_LAST_ERROR(L"Failed to create send event"); - goto cleanupToken; + goto cleanupAllocatedRegion; } Session->Descriptor.Receive.RingSize = RingSize; @@ -129,7 +124,6 @@ _Return_type_success_(return != NULL) TUN_SESSION *WINAPI LastError = LOG_LAST_ERROR(L"Failed to register rings"); goto cleanupHandle; } - RevertToSelf(); Session->Capacity = Capacity; (void)InitializeCriticalSectionAndSpinCount(&Session->Receive.Lock, LOCK_SPIN_COUNT); (void)InitializeCriticalSectionAndSpinCount(&Session->Send.Lock, LOCK_SPIN_COUNT); @@ -140,13 +134,11 @@ cleanupReceiveTailMoved: CloseHandle(Session->Descriptor.Receive.TailMoved); cleanupSendTailMoved: CloseHandle(Session->Descriptor.Send.TailMoved); -cleanupToken: - RevertToSelf(); cleanupAllocatedRegion: VirtualFree(AllocatedRegion, 0, MEM_RELEASE); cleanupRings: Free(Session); -out: +cleanup: SetLastError(LastError); return NULL; }