Hesham/wintun
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Set deny-all DACL instead of removing symlink on halting

Browse Source 
Deleting symbolic link on device removal only still makes it possible to
open it from the real path.

Setting the deny-all DACL instead is a more reliable way of preventing
clients reopening the device when it is being removed.

Signed-off-by: Sergei Mileshin <msvsysproger@gmail.com>
Signed-off-by: Simon Rozman <simon@rozman.si>
This commit is contained in:
Sergei Mileshin 2019-06-17 05:05:45 +03:00 committed by Simon Rozman
parent 2e744ebde5
commit a530bb1b84
2 changed files with 27 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
undocumented.h
View File

@ -29,3 +29,5 @@ typedef struct _SYSTEM_HANDLE_INFORMATION_EX
} SYSTEM_HANDLE_INFORMATION_EX, *PSYSTEM_HANDLE_INFORMATION_EX;
extern NTSTATUS ZwQuerySystemInformation(SYSTEM_INFORMATION_CLASS SystemInformationClass, PVOID SystemInformation, ULONG SystemInformationLength, ULONG *ReturnLength);
extern POBJECT_TYPE *IoDeviceObjectType;

38
wintun.c
View File

@ -107,8 +107,6 @@ typedef struct _TUN_CTX {
} PacketQueue;
NDIS_HANDLE NBLPool;
ULONG NetLuidIndex;
} TUN_CTX;
static UINT NdisVersion;
@ -1076,7 +1074,6 @@ static NDIS_STATUS TunInitializeEx(NDIS_HANDLE MiniportAdapterHandle, NDIS_HANDL
InterlockedExchange((LONG *)&ctx->State, TUN_STATE_INITIALIZING);
InterlockedExchange((LONG *)&ctx->PowerState, NdisDeviceStateD0);
ctx->MiniportAdapterHandle = MiniportAdapterHandle;
ctx->NetLuidIndex = (ULONG)MiniportInitParameters->NetLuid.Info.NetLuidIndex;
ctx->Statistics.Header.Type = NDIS_OBJECT_TYPE_DEFAULT;
ctx->Statistics.Header.Revision = NDIS_STATISTICS_INFO_REVISION_1;
@ -1240,6 +1237,29 @@ cleanup_NdisDeregisterDeviceEx:
return status;
}
_IRQL_requires_max_(PASSIVE_LEVEL)
static NTSTATUS TunDeviceSetDenyAllDacl(_In_ DEVICE_OBJECT *device_object)
{
NTSTATUS status;
SECURITY_DESCRIPTOR sd;
ACL acl;
HANDLE handle;
if (!NT_SUCCESS(status = RtlCreateSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION)))
return status;
if (!NT_SUCCESS(status = RtlCreateAcl(&acl, sizeof(ACL), ACL_REVISION)))
return status;
if (!NT_SUCCESS(status = RtlSetDaclSecurityDescriptor(&sd, TRUE, &acl, FALSE)))
return status;
if (!NT_SUCCESS(status = ObOpenObjectByPointer(device_object, OBJ_KERNEL_HANDLE, NULL, WRITE_DAC, *IoDeviceObjectType, KernelMode, &handle)))
return status;
status = ZwSetSecurityObject(handle, DACL_SECURITY_INFORMATION, &sd);
ZwClose(handle);
return status;
}
_IRQL_requires_max_(PASSIVE_LEVEL)
static void TunForceHandlesClosed(_Inout_ TUN_CTX *ctx)
{
@ -1321,16 +1341,8 @@ static void TunHaltEx(NDIS_HANDLE MiniportAdapterContext, NDIS_HALT_ACTION HaltA
for (IRP *pending_irp; (pending_irp = IoCsqRemoveNextIrp(&ctx->Device.ReadQueue.Csq, NULL)) != NULL;)
TunCompleteRequest(ctx, pending_irp, STATUS_FILE_FORCED_CLOSED, IO_NO_INCREMENT);
/* It's a bit annoying to reconstruct this here, but it's better than storing it, and
* although we could just get it from ndishandle+288, that's probably a bit dirty. */
WCHAR symbolic_name[sizeof(L"\\DosDevices\\" TUN_DEVICE_NAME) / sizeof(WCHAR) + 10/*MAXULONG as string*/] = { 0 };
UNICODE_STRING unicode_symbolic_name;
TunInitUnicodeString(&unicode_symbolic_name, symbolic_name);
RtlUnicodeStringPrintf(&unicode_symbolic_name, L"\\DosDevices\\" TUN_DEVICE_NAME, ctx->NetLuidIndex);
/* We first get rid of the symbolic link, to prevent userspace from accidently reopening
* this while we're waiting for the refcount to drop to zero. It might still be possible to
* open it from the real path, in which case, maybe we should consider setting a deny-all DACL. */
IoDeleteSymbolicLink(&unicode_symbolic_name);
/* Setting a deny-all DACL we prevent userspace to open the device by symlink after TunForceHandlesClosed(). */
TunDeviceSetDenyAllDacl(ctx->Device.Object);
if (InterlockedGet64(&ctx->Device.RefCount))
TunForceHandlesClosed(ctx);