Merge branch 'review-fixes'

.gitignore

@@ -1,3 +1,4 @@
 /bin/
 .vs/
 *.user
+*.aps

build.bat

@@ -1,5 +1,10 @@
 @echo off
 
+if [%VisualStudioVersion%]==[] (
+  echo Please launch this build script from a Visual Studio command prompt
+  exit /b 1
+)
+
 if [%1]==[] goto USAGE
 
 set CERT_THUMBPRINT=%1
@@ -8,10 +13,6 @@ set TIMESTAMP_SERVER=http://timestamp.digicert.com
 
 set ROOT=%~dp0
 
-:: Register "x64 Native Tools" environment
-
-call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Auxiliary\Build\vcvars64.bat"
-
 :: Build driver but do not sign it
 :: It's not possible to control all arguments to signtool through msbuild
 

src/containers/procregistry.cpp

@@ -80,7 +80,7 @@ TreeFreeRoutine
 // ClearDepartingParentLink()
 //
 // `Entry` is an enumerated entry in the tree.
-// `Context` is an entry that's being removed from the tree.
+// `Context` is the PID corresponding to an entry that's being removed from the tree.
 //
 // If `Entry` is a child of `Context` it needs to be updated to indicate that the parent process
 // is no longer available.
@@ -93,12 +93,10 @@ ClearDepartingParentLink
 	void *Context
 )
 {
-	auto parentEntry = (PROCESS_REGISTRY_ENTRY*)Context;
+	auto departingProcessId = (HANDLE)Context;
 
-	if (Entry->ParentEntry == parentEntry)
+	if (Entry->ParentProcessId == departingProcessId)
 	{
-		NT_ASSERT(Entry->ParentProcessId == parentEntry->ProcessId);
-
 		Entry->ParentProcessId = 0;
 		Entry->ParentEntry = NULL;
 	}
@@ -297,7 +295,7 @@ DeleteEntry
 	PROCESS_REGISTRY_ENTRY *Entry
 )
 {
-	ForEach(Context, ClearDepartingParentLink, Entry);
+	ForEach(Context, ClearDepartingParentLink, Entry->ProcessId);
 
 	InnerDeleteEntry(Context, Entry);
 }
@@ -364,7 +362,7 @@ IsEmpty
 	CONTEXT *Context
 )
 {
-	return NULL == RtlEnumerateGenericTableAvl(&Context->Tree, TRUE);
+	return FALSE != RtlIsGenericTableEmptyAvl(&Context->Tree);
 }
 
 } // namespace procregistry

src/containers/registeredimage.cpp

@@ -63,19 +63,7 @@ FindEntryExact
 	{
 		auto candidate = (REGISTERED_IMAGE_ENTRY*)entry;
 
-		if (candidate->ImageName.Length != ImageName->Length)
-		{
-			continue;
-		}
-
-		const auto equalBytes = RtlCompareMemory
-		(
-			candidate->ImageName.Buffer,
-			ImageName->Buffer,
-			ImageName->Length
-		);
-
-		if (equalBytes == ImageName->Length)
+		if (util::Equal(ImageName, &candidate->ImageName))
 		{
 			return candidate;
 		}

src/firewall/asyncbind.cpp

@@ -15,8 +15,7 @@ FailBindRequest
     HANDLE ProcessId,
     FWPS_CLASSIFY_OUT0 *ClassifyOut,
     UINT64 ClassifyHandle,
-    UINT64 FilterId,
-    bool Ipv4
+    UINT64 FilterId
 )
 {
     DbgPrint("Failing bind request from process %p\n", ProcessId);
@@ -48,36 +47,10 @@ FailBindRequest
 	ClassifyOut->actionType = FWP_ACTION_PERMIT;
 	ClassifyOut->rights &= ~FWPS_RIGHT_ACTION_WRITE;
 
-	if (Ipv4)
-	{
-		auto bindTarget = (SOCKADDR_IN*)&(bindRequest->localAddressAndPort);
-
-        IN_ADDR localhost;
-        
-        localhost.S_un.S_un_b.s_b1 = 127;
-        localhost.S_un.S_un_b.s_b2 = 0;
-        localhost.S_un.S_un_b.s_b3 = 0;
-        localhost.S_un.S_un_b.s_b4 = 1;
-
-		bindTarget->sin_addr = localhost;
-	}
-	else
-	{
-		auto bindTarget = (SOCKADDR_IN6*)&(bindRequest->localAddressAndPort);
-
-		IN6_ADDR localhost;
-		
-        localhost.u.Word[0] = 0;
-        localhost.u.Word[1] = 0;
-        localhost.u.Word[2] = 0;
-        localhost.u.Word[3] = 0;
-        localhost.u.Word[4] = 0;
-        localhost.u.Word[5] = 0;
-        localhost.u.Word[6] = 0;
-        localhost.u.Word[7] = htons(USHORT(1));
-
-		bindTarget->sin6_addr = localhost;
-	}
+    //
+    // This sets the port to 0, as well.
+    //
+    INETADDR_SETLOOPBACK((PSOCKADDR)&(bindRequest->localAddressAndPort));
 
 	FwpsApplyModifiedLayerData0(ClassifyHandle, (PVOID*)&bindRequest, 0);
 
@@ -105,7 +78,7 @@ FailPendedBindRequest
 )
 {
     const auto status = FailBindRequest(Record->ProcessId, &Record->ClassifyOut,
-        Record->ClassifyHandle, Record->FilterId, Record->Ipv4);
+        Record->ClassifyHandle, Record->FilterId);
 
     if (!status)
     {
@@ -138,8 +111,7 @@ PendBindRequest
     HANDLE ProcessId,
     void *ClassifyContext,
     UINT64 FilterId,
-    FWPS_CLASSIFY_OUT0 *ClassifyOut,
-    bool Ipv4
+    FWPS_CLASSIFY_OUT0 *ClassifyOut
 )
 {
    DbgPrint("Pending bind request from process %p\n", ProcessId);
@@ -179,7 +151,6 @@ PendBindRequest
     record->ClassifyHandle = classifyHandle;
     record->ClassifyOut = *ClassifyOut;
     record->FilterId = FilterId;
-    record->Ipv4 = Ipv4;
 
     WdfWaitLockAcquire(Context->PendedBinds.Lock, NULL);
 
@@ -196,8 +167,7 @@ FailBindRequest
     HANDLE ProcessId,
     void *ClassifyContext,
     UINT64 FilterId,
-    FWPS_CLASSIFY_OUT0 *ClassifyOut,
-    bool Ipv4
+    FWPS_CLASSIFY_OUT0 *ClassifyOut
 )
 {
 	UINT64 classifyHandle = 0;
@@ -216,7 +186,7 @@ FailBindRequest
 		return;
 	}
 
-    FailBindRequest(ProcessId, ClassifyOut, classifyHandle, FilterId, Ipv4);
+    FailBindRequest(ProcessId, ClassifyOut, classifyHandle, FilterId);
 
     FwpsReleaseClassifyHandle0(classifyHandle);
 }

src/firewall/asyncbind.h

@@ -14,8 +14,7 @@ PendBindRequest
     HANDLE ProcessId,
     void *ClassifyContext,
     UINT64 FilterId,
-    FWPS_CLASSIFY_OUT0 *ClassifyOut,
-    bool Ipv4
+    FWPS_CLASSIFY_OUT0 *ClassifyOut
 );
 
 void
@@ -24,8 +23,7 @@ FailBindRequest
     HANDLE ProcessId,
     void *ClassifyContext,
     UINT64 FilterId,
-    FWPS_CLASSIFY_OUT0 *ClassifyOut,
-    bool Ipv4
+    FWPS_CLASSIFY_OUT0 *ClassifyOut
 );
 
 void

src/firewall/blocking.cpp

@@ -124,12 +124,12 @@ PushTransactionEvent
 		return STATUS_INSUFFICIENT_RESOURCES;
 	}
 
-	InitializeListHead((LIST_ENTRY*)evt);
+	InitializeListHead(&evt->ListEntry);
 
 	evt->EventType = EventType;
 	evt->Target = Target;
 
-	InsertHeadList(TransactionEvents, (LIST_ENTRY*)evt);
+	InsertHeadList(TransactionEvents, &evt->ListEntry);
 
 	return STATUS_SUCCESS;
 }
@@ -195,13 +195,13 @@ TransactionRemovedEntry
 		return STATUS_INSUFFICIENT_RESOURCES;
 	}
 
-	InitializeListHead((LIST_ENTRY*)evt);
+	InitializeListHead(&evt->ListEntry);
 
 	evt->EventType = TRANSACTION_EVENT_TYPE::ADD_ENTRY;
 	evt->Target = Target;
 	evt->MockHead = MockHead;
 
-	InsertHeadList(TransactionEvents, (LIST_ENTRY*)evt);
+	InsertHeadList(TransactionEvents, &evt->ListEntry);
 
 	return STATUS_SUCCESS;
 }
@@ -221,7 +221,7 @@ TransactionSwappedLists
 		return STATUS_INSUFFICIENT_RESOURCES;
 	}
 
-	InitializeListHead((LIST_ENTRY*)evt);
+	InitializeListHead(&evt->ListEntry);
 
 	evt->EventType = TRANSACTION_EVENT_TYPE::SWAP_LISTS;
 
@@ -231,7 +231,7 @@ TransactionSwappedLists
 
 	util::ReparentList(&evt->BlockedTunnelConnections, BlockedTunnelConnections);
 
-	InsertHeadList(TransactionEvents, (LIST_ENTRY*)evt);
+	InsertHeadList(TransactionEvents, &evt->ListEntry);
 
 	return STATUS_SUCCESS;
 }
@@ -299,19 +299,7 @@ FindBlockConnectionsEntry
 	{
 		auto candidate = (BLOCK_CONNECTIONS_ENTRY*)entry;
 
-		if (candidate->ImageName.Length != ImageName->Length)
-		{
-			continue;
-		}
-
-		const auto equalBytes = RtlCompareMemory
-		(
-			candidate->ImageName.Buffer,
-			ImageName->Buffer,
-			ImageName->Length
-		);
-
-		if (equalBytes == ImageName->Length)
+		if (util::Equal(ImageName, &candidate->ImageName))
 		{
 			return candidate;
 		}
@@ -638,7 +626,7 @@ RemoveBlockFiltersAndEntryTx
 		return status;
 	}
 
-	RemoveEntryList((LIST_ENTRY*)Entry);
+	RemoveEntryList(&Entry->ListEntry);
 
 	return STATUS_SUCCESS;
 }
@@ -933,13 +921,13 @@ TransactionAbort
 			{
 				auto addEvent = (TRANSACTION_EVENT_ADD_ENTRY*)rawEvent;
 
-				InsertHeadList(addEvent->MockHead, (LIST_ENTRY*)addEvent->Target);
+				InsertHeadList(addEvent->MockHead, &addEvent->Target->ListEntry);
 
 				break;
 			}
 			case TRANSACTION_EVENT_TYPE::REMOVE_ENTRY:
 			{
-				RemoveEntryList((LIST_ENTRY*)evt->Target);
+				RemoveEntryList(&evt->Target->ListEntry);
 
 				ExFreePoolWithTag(evt->Target, ST_POOL_TAG);
 
@@ -1222,7 +1210,7 @@ UpdateBlockingFiltersTx2
 
 		newEntry->RefCount = entry->RefCount;
 
-		InsertTailList(&newList, (LIST_ENTRY*)newEntry);
+		InsertTailList(&newList, &newEntry->ListEntry);
 	}
 
 	//

src/firewall/callouts.cpp

@@ -89,8 +89,7 @@ ClassifyUnknownBind
 	HANDLE ProcessId,
 	UINT64 FilterId,
 	const void *ClassifyContext,
-	FWPS_CLASSIFY_OUT0 *ClassifyOut,
-	bool Ipv4
+	FWPS_CLASSIFY_OUT0 *ClassifyOut
 )
 {
 	//
@@ -103,8 +102,7 @@ ClassifyUnknownBind
 		ProcessId,
 		const_cast<void*>(ClassifyContext),
 		FilterId,
-		ClassifyOut,
-		Ipv4
+		ClassifyOut
 	);
 
 	if (NT_SUCCESS(status))
@@ -119,8 +117,7 @@ ClassifyUnknownBind
 		ProcessId,
 		const_cast<void*>(ClassifyContext),
 		FilterId,
-		ClassifyOut,
-		Ipv4
+		ClassifyOut
 	);
 }
 
@@ -210,8 +207,7 @@ CalloutClassifyBind
 				HANDLE(MetaValues->processId),
 				Filter->filterId,
 				ClassifyContext,
-				ClassifyOut,
-				FixedValues->layerId == FWPS_LAYER_ALE_BIND_REDIRECT_V4
+				ClassifyOut
 			);
 
 			break;

src/firewall/context.h

@@ -58,9 +58,6 @@ struct PENDED_BIND
 
 	// The filter that triggered the classification.
 	UINT64 FilterId;
-
-	// Whether this is an IPv4 or IPv6 bind.
-	bool Ipv4;
 };
 
 struct PENDED_BIND_MGMT

src/ioctl.cpp

@@ -365,43 +365,6 @@ DbgPrintConfiguration
     return true;
 }
 
-//
-// ClearApplySplitSetting()
-//
-// Clear splitting and notify responsible systems.
-//
-// Locks being held when called:
-//
-// Process event subsystem operation lock
-//
-bool
-NTAPI
-ClearApplySplitSetting
-(
-    procregistry::PROCESS_REGISTRY_ENTRY *Entry,
-    void *Context
-)
-{
-    auto context = (ST_DEVICE_CONTEXT *)Context;
-
-    Entry->TargetSettings.Split = ST_PROCESS_SPLIT_STATUS_OFF;
-    Entry->TargetSettings.HasFirewallState = false;
-
-    if (Entry->Settings.HasFirewallState)
-    {
-        auto status = firewall::RegisterAppBecomingUnsplitTx2(context->Firewall, &Entry->ImageName);
-
-        if (!NT_SUCCESS(status))
-        {
-            DbgPrint("Failed to update firewall 0x%X\n", status);
-
-            return false;
-        }
-    }
-
-    return true;
-}
-
 //
 // RealizeAnnounceSettingsChange()
 //

src/procmgmt/procmgmt.cpp

@@ -14,7 +14,7 @@ namespace
 // ValidateCollision()
 //
 // Find and validate existing entry in process registry that prevented the insertion
-// of a a new entry, because they share the same PID.
+// of a new entry, because they share the same PID.
 //
 bool
 ValidateCollision
@@ -54,19 +54,7 @@ ValidateCollision
         goto Approved;
     }
 
-    //
-    // Both the existing entry and the proposed entry will have lower-case
-    // imagenames so it's straight forward to compare the strings.
-    //
-
-	const auto equalBytes = RtlCompareMemory
-	(
-		existingEntry->ImageName.Buffer,
-		newEntry->ImageName.Buffer,
-		newEntry->ImageName.Length
-	);
-
-    if (equalBytes != newEntry->ImageName.Length)
+    if (!util::Equal(&existingEntry->ImageName, &newEntry->ImageName))
     {
         DbgPrint("Validate PR collision - mismatched image name\n");
 

src/util.cpp

@@ -336,4 +336,26 @@ SplittingEnabled
 		|| Status == ST_PROCESS_SPLIT_STATUS_ON_BY_INHERITANCE);
 }
 
+bool
+Equal
+(
+	const LOWER_UNICODE_STRING *a,
+	const LOWER_UNICODE_STRING *b
+)
+{
+	if (a->Length != b->Length)
+	{
+		return false;
+	}
+
+	const auto equalBytes = RtlCompareMemory
+	(
+		a->Buffer,
+		b->Buffer,
+		b->Length
+	);
+
+	return equalBytes == b->Length;
+}
+
 } // namespace util

src/util.h

@@ -118,4 +118,11 @@ SplittingEnabled
 	ST_PROCESS_SPLIT_STATUS Status
 );
 
+bool
+Equal
+(
+	const LOWER_UNICODE_STRING *a,
+	const LOWER_UNICODE_STRING *b
+);
+
 } // namespace util