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>

api/api.c

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

api/api.h

@@ -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.

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 (!CreateDirectoryW(RandomTempSubDirectory, SecurityAttributes))
-    if (!ConvertStringSecurityDescriptorToSecurityDescriptorW(
+        return LOG_LAST_ERROR(L"Failed to create temporary folder");
-            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;
-    }
 
+    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")) !=
+    if ((Result = ResourceCopyToFile(CatPath, UseWHQL ? L"wintun-whql.cat" : L"wintun.cat")) != ERROR_SUCCESS ||
-            ERROR_SUCCESS ||
+        (Result = ResourceCopyToFile(SysPath, UseWHQL ? L"wintun-whql.sys" : L"wintun.sys")) != ERROR_SUCCESS ||
-        (Result = ResourceCopyToFile(SysPath, &SecurityAttributes, UseWHQL ? L"wintun-whql.sys" : L"wintun.sys")) !=
+        (Result = ResourceCopyToFile(InfPath, UseWHQL ? L"wintun-whql.inf" : L"wintun.inf")) != ERROR_SUCCESS)
-            ERROR_SUCCESS ||
-        (Result = ResourceCopyToFile(InfPath, &SecurityAttributes, 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;
 }
 

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

api/resource.c

@@ -27,10 +27,7 @@ ResourceGetAddress(_In_z_ const WCHAR *ResourceName, _Out_ const VOID **Address,
 }
 
 WINTUN_STATUS
-ResourceCopyToFile(
+ResourceCopyToFile(_In_z_ const WCHAR *DestinationPath, _In_z_ const WCHAR *ResourceName)
-    _In_z_ const WCHAR *DestinationPath,
-    _In_opt_ SECURITY_ATTRIBUTES *SecurityAttributes,
-    _In_z_ const WCHAR *ResourceName)
 {
     const VOID *LockedResource;
     DWORD SizeResource;

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(
+ResourceCopyToFile(_In_z_ const WCHAR *DestinationPath, _In_z_ const WCHAR *ResourceName);
-    _In_z_ const WCHAR *DestinationPath,
-    _In_opt_ SECURITY_ATTRIBUTES *SecurityAttributes,
-    _In_z_ const WCHAR *ResourceName);