diff options
author | Simon Rozman <simon@rozman.si> | 2020-10-15 15:34:31 +0200 |
---|---|---|
committer | Jason A. Donenfeld <Jason@zx2c4.com> | 2020-10-30 16:51:00 +0100 |
commit | 8272da638e76810c8bafd7e5d1a1217b2026c7da (patch) | |
tree | 9b924dc4614a7631b582ca3cc4d0d9a8707730a6 /api | |
parent | api: arrange rundll32 a console logger (diff) | |
download | wintun-8272da638e76810c8bafd7e5d1a1217b2026c7da.tar.xz wintun-8272da638e76810c8bafd7e5d1a1217b2026c7da.zip |
api: unify security descriptors and disable for _DEBUG
When debugger is attached, CreateDirectory() with SYSTEM-only SID fails
with "This security ID may not be assigned as the owner of this object.
(Code 0x0000051B)".
Signed-off-by: Simon Rozman <simon@rozman.si>
Diffstat (limited to 'api')
-rw-r--r-- | api/api.c | 10 | ||||
-rw-r--r-- | api/api.h | 1 | ||||
-rw-r--r-- | api/driver.c | 24 | ||||
-rw-r--r-- | api/namespace.c | 23 | ||||
-rw-r--r-- | api/resource.c | 5 | ||||
-rw-r--r-- | api/resource.h | 7 |
6 files changed, 25 insertions, 45 deletions
@@ -6,6 +6,8 @@ #include "pch.h" HINSTANCE ResourceModule; +static SECURITY_ATTRIBUTES SecurityAttributesSystem = { .nLength = sizeof(SECURITY_ATTRIBUTES) }; +SECURITY_ATTRIBUTES *SecurityAttributes = NULL; WINTUN_STATUS WINAPI WintunGetVersion( @@ -54,6 +56,11 @@ DllMain(_In_ HINSTANCE hinstDLL, _In_ DWORD fdwReason, _In_ LPVOID lpvReserved) { case DLL_PROCESS_ATTACH: ResourceModule = hinstDLL; +#ifndef _DEBUG + ConvertStringSecurityDescriptorToSecurityDescriptorW( + L"O:SYD:P(A;;GA;;;SY)", SDDL_REVISION_1, &SecurityAttributesSystem.lpSecurityDescriptor, NULL); + SecurityAttributes = &SecurityAttributesSystem; +#endif AdapterInit(); NamespaceInit(); NciInit(); @@ -63,6 +70,9 @@ DllMain(_In_ HINSTANCE hinstDLL, _In_ DWORD fdwReason, _In_ LPVOID lpvReserved) NciCleanup(); NamespaceCleanup(); AdapterCleanup(); +#ifndef _DEBUG + LocalFree(SecurityAttributesSystem.lpSecurityDescriptor); +#endif break; } return TRUE; @@ -17,6 +17,7 @@ typedef _Return_type_success_(return == ERROR_SUCCESS) DWORD WINTUN_STATUS; extern HINSTANCE ResourceModule; +extern SECURITY_ATTRIBUTES *SecurityAttributes; /** * Returns the version of the Wintun driver and NDIS system currently loaded. diff --git a/api/driver.c b/api/driver.c index a4d6dc1..fffd445 100644 --- a/api/driver.c +++ b/api/driver.c @@ -301,17 +301,10 @@ InstallDriver(_In_ BOOL UpdateExisting) WCHAR RandomTempSubDirectory[MAX_PATH]; if (!PathCombineW(RandomTempSubDirectory, WindowsTempDirectory, RandomSubDirectory)) return ERROR_BUFFER_OVERFLOW; - SECURITY_ATTRIBUTES SecurityAttributes = { .nLength = sizeof(SecurityAttributes) }; - if (!ConvertStringSecurityDescriptorToSecurityDescriptorW( - L"O:SYD:P(A;;GA;;;SY)", SDDL_REVISION_1, &SecurityAttributes.lpSecurityDescriptor, NULL)) - return LOG_LAST_ERROR(L"Failed to convert security descriptor"); - DWORD Result = ERROR_SUCCESS; - if (!CreateDirectoryW(RandomTempSubDirectory, &SecurityAttributes)) - { - Result = LOG_LAST_ERROR(L"Failed to create temporary folder"); - goto cleanupFree; - } + if (!CreateDirectoryW(RandomTempSubDirectory, SecurityAttributes)) + return LOG_LAST_ERROR(L"Failed to create temporary folder"); + DWORD Result = ERROR_SUCCESS; WCHAR CatPath[MAX_PATH] = { 0 }; WCHAR SysPath[MAX_PATH] = { 0 }; WCHAR InfPath[MAX_PATH] = { 0 }; @@ -328,12 +321,9 @@ InstallDriver(_In_ BOOL UpdateExisting) LOG(WINTUN_LOG_WARN, L"Unable to install code signing certificate"); LOG(WINTUN_LOG_INFO, L"Copying resources to temporary path"); - if ((Result = ResourceCopyToFile(CatPath, &SecurityAttributes, UseWHQL ? L"wintun-whql.cat" : L"wintun.cat")) != - ERROR_SUCCESS || - (Result = ResourceCopyToFile(SysPath, &SecurityAttributes, UseWHQL ? L"wintun-whql.sys" : L"wintun.sys")) != - ERROR_SUCCESS || - (Result = ResourceCopyToFile(InfPath, &SecurityAttributes, UseWHQL ? L"wintun-whql.inf" : L"wintun.inf")) != - ERROR_SUCCESS) + if ((Result = ResourceCopyToFile(CatPath, UseWHQL ? L"wintun-whql.cat" : L"wintun.cat")) != ERROR_SUCCESS || + (Result = ResourceCopyToFile(SysPath, UseWHQL ? L"wintun-whql.sys" : L"wintun.sys")) != ERROR_SUCCESS || + (Result = ResourceCopyToFile(InfPath, UseWHQL ? L"wintun-whql.inf" : L"wintun.inf")) != ERROR_SUCCESS) { LOG(WINTUN_LOG_ERR, L"Failed to copy resources"); goto cleanupDelete; @@ -356,8 +346,6 @@ cleanupDelete: DeleteFileW(InfPath); cleanupDirectory: RemoveDirectoryW(RandomTempSubDirectory); -cleanupFree: - LocalFree(SecurityAttributes.lpSecurityDescriptor); return Result; } diff --git a/api/namespace.c b/api/namespace.c index 3d7d563..5129bdd 100644 --- a/api/namespace.c +++ b/api/namespace.c @@ -5,7 +5,6 @@ #include "pch.h" -static SECURITY_ATTRIBUTES SecurityAttributes = { .nLength = sizeof(SECURITY_ATTRIBUTES) }; static BOOL HasInitialized = FALSE; static CRITICAL_SECTION Initializing; static BCRYPT_ALG_HANDLE AlgProvider; @@ -62,36 +61,29 @@ NamespaceRuntimeInit() goto cleanupLeaveCriticalSection; } - ULONG SecDescrSize; - if (!ConvertStringSecurityDescriptorToSecurityDescriptorW( - L"O:SYD:P(A;;GA;;;SY)", 1, &SecurityAttributes.lpSecurityDescriptor, &SecDescrSize)) - { - Result = GetLastError(); - goto cleanupBCryptCloseAlgorithmProvider; - } BYTE Sid[MAX_SID_SIZE]; DWORD SidSize = MAX_SID_SIZE; if (!CreateWellKnownSid(WinLocalSystemSid, NULL, Sid, &SidSize)) { Result = GetLastError(); - goto cleanupSecurityDescriptor; + goto cleanupBCryptCloseAlgorithmProvider; } HANDLE Boundary = CreateBoundaryDescriptorW(L"Wintun", 0); if (!Boundary) { Result = GetLastError(); - goto cleanupSecurityDescriptor; + goto cleanupBCryptCloseAlgorithmProvider; } if (!AddSIDToBoundaryDescriptor(&Boundary, Sid)) { Result = GetLastError(); - goto cleanupSecurityDescriptor; + goto cleanupBCryptCloseAlgorithmProvider; } for (;;) { - if (CreatePrivateNamespaceW(&SecurityAttributes, Boundary, L"Wintun")) + if (CreatePrivateNamespaceW(SecurityAttributes, Boundary, L"Wintun")) break; Result = GetLastError(); if (Result == ERROR_ALREADY_EXISTS) @@ -102,15 +94,13 @@ NamespaceRuntimeInit() if (Result == ERROR_PATH_NOT_FOUND) continue; } - goto cleanupSecurityDescriptor; + goto cleanupBCryptCloseAlgorithmProvider; } HasInitialized = TRUE; Result = ERROR_SUCCESS; goto cleanupLeaveCriticalSection; -cleanupSecurityDescriptor: - LocalFree(SecurityAttributes.lpSecurityDescriptor); cleanupBCryptCloseAlgorithmProvider: BCryptCloseAlgorithmProvider(AlgProvider, 0); cleanupLeaveCriticalSection: @@ -149,7 +139,7 @@ NamespaceTakeMutex(_In_z_ const WCHAR *Pool) memcpy(MutexName, MutexNamePrefix, sizeof(MutexNamePrefix) - sizeof(WCHAR)); Bin2Hex(Hash, sizeof(Hash), MutexName + _countof(MutexNamePrefix) - 1); MutexName[_countof(MutexName) - 1] = 0; - Mutex = CreateMutexW(&SecurityAttributes, FALSE, MutexName); + Mutex = CreateMutexW(SecurityAttributes, FALSE, MutexName); if (!Mutex) goto cleanupPoolNorm; switch (WaitForSingleObject(Mutex, INFINITE)) @@ -187,7 +177,6 @@ NamespaceCleanup() EnterCriticalSection(&Initializing); if (HasInitialized) { - LocalFree(SecurityAttributes.lpSecurityDescriptor); BCryptCloseAlgorithmProvider(AlgProvider, 0); HasInitialized = FALSE; } diff --git a/api/resource.c b/api/resource.c index 82f0d13..61dd2e7 100644 --- a/api/resource.c +++ b/api/resource.c @@ -27,10 +27,7 @@ ResourceGetAddress(_In_z_ const WCHAR *ResourceName, _Out_ const VOID **Address, } WINTUN_STATUS -ResourceCopyToFile( - _In_z_ const WCHAR *DestinationPath, - _In_opt_ SECURITY_ATTRIBUTES *SecurityAttributes, - _In_z_ const WCHAR *ResourceName) +ResourceCopyToFile(_In_z_ const WCHAR *DestinationPath, _In_z_ const WCHAR *ResourceName) { const VOID *LockedResource; DWORD SizeResource; diff --git a/api/resource.h b/api/resource.h index 5021490..6311f2b 100644 --- a/api/resource.h +++ b/api/resource.h @@ -27,14 +27,9 @@ ResourceGetAddress(_In_z_ const WCHAR *ResourceName, _Out_ const VOID **Address, * * DestinationPath File path * - * SecurityAttributes File security attributes. May be NULL for detault. - * * ResourceName Name of the RT_RCDATA resource. Use MAKEINTRESOURCEW to locate resource by ID. * * @return ERROR_SUCCESS on success; Win32 error code otherwise. */ WINTUN_STATUS -ResourceCopyToFile( - _In_z_ const WCHAR *DestinationPath, - _In_opt_ SECURITY_ATTRIBUTES *SecurityAttributes, - _In_z_ const WCHAR *ResourceName); +ResourceCopyToFile(_In_z_ const WCHAR *DestinationPath, _In_z_ const WCHAR *ResourceName); |