From 11caa94b7cb9c636f62fcb3c06542fcd1ee368a7 Mon Sep 17 00:00:00 2001 From: Vladimir Date: Tue, 8 Nov 2022 17:03:34 +0100 Subject: [PATCH 1/3] Cache update verdict function --- include/pm_common.h | 21 +++++-- pm_kext/col/verdict_cache.c | 105 +++++++++++++++++++++++++++----- pm_kext/include/pm_callouts.h | 1 + pm_kext/include/verdict_cache.h | 23 +++++-- pm_kext/src/pm_callouts.c | 15 +++-- pm_kext/src/pm_kernel.c | 69 ++++++--------------- pm_kext_glue_dll/src/pm_api.c | 12 ++-- 7 files changed, 158 insertions(+), 88 deletions(-) diff --git a/include/pm_common.h b/include/pm_common.h index 30397f4..50d2f69 100644 --- a/include/pm_common.h +++ b/include/pm_common.h @@ -40,7 +40,6 @@ typedef struct { UINT32 packetSize; } PortmasterPacketInfo; - /* * Packet Info Flags */ @@ -172,9 +171,18 @@ typedef struct { */ typedef struct { UINT32 id; - UINT32 len; //preset with maxlen of payload from caller -> set with acutal len of payload from receiver + UINT32 len; // preset with maxlen of payload from caller -> set with acutal len of payload from receiver } PortmasterPayload; +typedef struct { + UINT32 localIP[4]; // Source Address, only srcIP[0] if IPv4 + UINT32 remoteIP[4]; // Destination Address + UINT16 localPort; // Source Port + UINT16 remotePort; // Destination port + UINT8 ipV6; // True: IPv6, False: IPv4 + UINT8 protocol; // Protocol (UDP, TCP, ...) + verdict_t verdict; // New verdict +} VerdictUpdateInfo; /* * Currently unused returncodes @@ -196,11 +204,11 @@ typedef struct { #define SIOCTL_TYPE 40000 -#define IOCTL_HELLO \ - CTL_CODE(SIOCTL_TYPE, 0x800, METHOD_BUFFERED, FILE_READ_DATA|FILE_WRITE_DATA) //FILE_ANY_ACCESS +#define IOCTL_VERSION \ + CTL_CODE(SIOCTL_TYPE, 0x800, METHOD_BUFFERED, FILE_READ_DATA|FILE_WRITE_DATA) #define IOCTL_RECV_VERDICT_REQ_POLL \ - CTL_CODE(SIOCTL_TYPE, 0x801, METHOD_BUFFERED, FILE_READ_DATA|FILE_WRITE_DATA) + CTL_CODE(SIOCTL_TYPE, 0x801, METHOD_BUFFERED, FILE_READ_DATA|FILE_WRITE_DATA) // Not used #define IOCTL_RECV_VERDICT_REQ \ CTL_CODE(SIOCTL_TYPE, 0x802, METHOD_BUFFERED, FILE_READ_DATA|FILE_WRITE_DATA) @@ -214,6 +222,9 @@ typedef struct { #define IOCTL_CLEAR_CACHE \ CTL_CODE(SIOCTL_TYPE, 0x805, METHOD_BUFFERED, FILE_READ_DATA|FILE_WRITE_DATA) +#define IOCTL_UPDATE_VERDICT \ + CTL_CODE(SIOCTL_TYPE, 0x806, METHOD_BUFFERED, FILE_READ_DATA|FILE_WRITE_DATA) + /****************************************************************************/ /* MISC */ /****************************************************************************/ diff --git a/pm_kext/col/verdict_cache.c b/pm_kext/col/verdict_cache.c index f2fc046..8e396a5 100644 --- a/pm_kext/col/verdict_cache.c +++ b/pm_kext/col/verdict_cache.c @@ -27,12 +27,13 @@ #define uthash_fatal #define HASH_NO_STDINT 1 #include "uthash.h" - +#pragma warning(push) +#pragma warning(disable: 4127) // warning C4127: conditional expression is constant -> if generated by macro HASH_FIND typedef struct { UINT32 localIP[4]; - UINT16 localPort; UINT32 remoteIP[4]; + UINT16 localPort; UINT16 remotePort; UINT8 protocol; } VerdictCacheKey; @@ -75,6 +76,16 @@ static VerdictCacheKey getCacheKey(PortmasterPacketInfo *info) { return key; } +static VerdictCacheKey getCacheKeyFromUpdateInfo(VerdictUpdateInfo *info) { + VerdictCacheKey key = {0}; + memcpy(key.localIP, info->localIP, sizeof(UINT32) * 4); + key.localPort = info->localPort; + memcpy(key.remoteIP, info->remoteIP, sizeof(UINT32) * 4); + key.remotePort = info->remotePort; + key.protocol = info->protocol; + return key; +} + static VerdictCacheKey getCacheRedirectKey(PortmasterPacketInfo *info) { VerdictCacheKey key = {0}; memcpy(key.localIP, info->localIP, sizeof(UINT32) * 4); @@ -93,7 +104,7 @@ static VerdictCacheKey getCacheRedirectKey(PortmasterPacketInfo *info) { * @return error code * */ -int verdictCacheCreate(UINT32 maxSize, void **verdictCache) { +int verdictCacheCreate(UINT32 maxSize, VerdictCache **verdictCache) { if (maxSize == 0) { return 1; } @@ -193,12 +204,76 @@ static void resetItem(VerdictCache *verdictCache, VerdictCacheItem *item) { memset(item, 0, sizeof(VerdictCacheItem)); } +static void verdictCacheUpdateFromItem(VerdictCache *verdictCache, VerdictCacheItem *item, verdict_t newVerdict) { + verdict_t oldVerdict = item->verdict; + + if(oldVerdict != newVerdict) { + // Remove old redirect + if(oldVerdict == PORTMASTER_VERDICT_REDIR_DNS || oldVerdict == PORTMASTER_VERDICT_REDIR_TUNNEL) { + if(item->hhRedirect.key != NULL) { + HASH_DELETE(hhRedirect, verdictCache->mapRedirect, item); + } + memset(&item->redirectKey, 0, sizeof(VerdictCacheKey)); + } + + // Set new verdict and create redirect if needed + item->verdict = newVerdict; + if(newVerdict == PORTMASTER_VERDICT_REDIR_DNS || newVerdict == PORTMASTER_VERDICT_REDIR_TUNNEL) { + item->redirectKey = getCacheRedirectKey(item->packetInfo); + HASH_ADD(hhRedirect, verdictCache->mapRedirect, redirectKey, sizeof(VerdictCacheKey), item); + } + + INFO("verdictCacheUpdate verdict updated %s: %d -> %d", printPacketInfo(item->packetInfo), oldVerdict, newVerdict); + } +} + +/** + * @brief Update verdict for specific item in the cache + * + * @par verdictCache = verdict cache to use + * @par item = item from the verdict cache + * @par newVerdict = verdict to save + */ +int verdictCacheUpdate(VerdictCache *verdictCache, VerdictUpdateInfo *info) { + if (verdictCache == NULL || info == NULL) { + ERR("verdictCacheUpdate NULL pointer exception VerdictCache=0p%Xp, VerdictUpdateInfo=0p%Xp", verdictCache, info); + return 1; + } + + VerdictCacheItem *item = NULL; + VerdictCacheKey key = getCacheKeyFromUpdateInfo(info); + + // Lock to check verdict cache. + KLOCK_QUEUE_HANDLE lockHandle = {0}; + KeAcquireInStackQueuedSpinLock(&verdictCache->lock, &lockHandle); + + int rc = 0; + + // Search for verdict + HASH_FIND(hh, verdictCache->map, &key, sizeof(VerdictCacheKey), item); + if(item == NULL) { + ERR("verdictCacheUpdate failed to update verdict, item not found"); + rc = 2; + } + + // Update verdict + if(rc == 0) { + verdict_t newVerdict = info->verdict; + verdictCacheUpdateFromItem(verdictCache, item, newVerdict); + } + + KeReleaseInStackQueuedSpinLock(&lockHandle); + + return rc; +} + /** * @brief Adds verdict to cache * - * @par verdictCache = verdict cache to use - * @par packet_info = pointer to packet_info - * @par verdict = verdict to save + * @par verdictCache = verdict cache to use + * @par packet_info = pointer to packet_info + * @par verdict = verdict to save + * @par removedPacketInfo = old packet info that was removed from the cache (can be NULL) * @return error code * */ @@ -218,12 +293,11 @@ int verdictCacheAdd(VerdictCache *verdictCache, PortmasterPacketInfo *packetInfo KeAcquireInStackQueuedSpinLock(&verdictCache->lock, &lockHandle); int rc = 0; - - #pragma warning(suppress : 4127) // warning C4127: conditional expression is constant -> if generated by macro HASH_FIND(hh, verdictCache->map, &key, sizeof(VerdictCacheKey), newItem); if(newItem != NULL) { - // already in - INFO("addVerdict packet was already in"); + // Already in + INFO("verdictCacheAdd packet was already in the verdict cache. Updating the verdict..."); + verdictCacheUpdateFromItem(verdictCache, newItem, verdict); rc = 3; } @@ -234,8 +308,7 @@ int verdictCacheAdd(VerdictCache *verdictCache, PortmasterPacketInfo *packetInfo } else { VerdictCacheItem *item = getOldestAccessTimeItem(verdictCache); if(item == NULL) { - ERR("addVerdict failed to find free element"); - KeReleaseInStackQueuedSpinLock(&lockHandle); + ERR("verdictCacheAdd failed to find free element"); rc = 2; } else { *removedPacketInfo = item->packetInfo; @@ -246,18 +319,18 @@ int verdictCacheAdd(VerdictCache *verdictCache, PortmasterPacketInfo *packetInfo } if(rc == 0) { - // Set key + // Add to verdict map newItem->key = key; newItem->packetInfo = packetInfo; newItem->verdict = verdict; newItem->lastAccessed = cacheAccessCounter; HASH_ADD(hh, verdictCache->map, key, sizeof(VerdictCacheKey), newItem); + // Add to redirect map if needed if(verdict == PORTMASTER_VERDICT_REDIR_DNS || verdict == PORTMASTER_VERDICT_REDIR_TUNNEL) { newItem->redirectKey = getCacheRedirectKey(packetInfo); // insert only if we dont have already item with the same key VerdictCacheItem *redirectItem = NULL; - #pragma warning(suppress : 4127) // warning C4127: conditional expression is constant -> if generated by macro HASH_FIND(hhRedirect, verdictCache->mapRedirect, &newItem->redirectKey, sizeof(VerdictCacheKey), redirectItem); if(redirectItem == NULL) { HASH_ADD(hhRedirect, verdictCache->mapRedirect, redirectKey, sizeof(VerdictCacheKey), newItem); @@ -290,7 +363,6 @@ static verdict_t checkVerdict(VerdictCache *verdictCache, PortmasterPacketInfo * VerdictCacheItem *item = NULL; VerdictCacheKey key = getCacheKey(packetInfo); - #pragma warning(suppress : 4127) // warning C4127: conditional expression is constant -> if generated by macro HASH_FIND(hh, verdictCache->map, &key, sizeof(VerdictCacheKey), item); if(item == NULL) { @@ -325,7 +397,6 @@ static verdict_t checkReverseRedirect(VerdictCache *verdictCache, PortmasterPack VerdictCacheItem *item = NULL; VerdictCacheKey key = getCacheRedirectKey(packetInfo); - #pragma warning(suppress : 4127) // warning C4127: conditional expression is constant -> if generated by macro HASH_FIND(hhRedirect, verdictCache->mapRedirect, &key, sizeof(VerdictCacheKey), item); if(item == NULL) { return PORTMASTER_VERDICT_GET; @@ -367,3 +438,5 @@ verdict_t verdictCacheGet(VerdictCache *verdictCache, PortmasterPacketInfo *pack KeReleaseInStackQueuedSpinLock(&lockHandle); return verdict; } + +#pragma warning(pop) \ No newline at end of file diff --git a/pm_kext/include/pm_callouts.h b/pm_kext/include/pm_callouts.h index 63e44a9..231cd9b 100644 --- a/pm_kext/include/pm_callouts.h +++ b/pm_kext/include/pm_callouts.h @@ -65,6 +65,7 @@ NTSTATUS genericFlowDelete(UINT16 layerId, UINT32 calloutId, UINT64 flowContext) void respondWithVerdict(UINT32 id, verdict_t verdict); PacketCache* getPacketCache(); +int updateVerdict(VerdictUpdateInfo*); void clearCache(); diff --git a/pm_kext/include/verdict_cache.h b/pm_kext/include/verdict_cache.h index e0183a3..7a9e0c4 100644 --- a/pm_kext/include/verdict_cache.h +++ b/pm_kext/include/verdict_cache.h @@ -41,7 +41,7 @@ int verdictCacheCreate(UINT32 maxSize, VerdictCache **verdict_cache); /** * @brief Remove all items from verdict cache * - * @par verdict_cache = verdict_cache to use + * @par verdict_cache = VerdictCache to use * @par freeData = callback function that is executed for each item before delete were the data of the item can be deleted * */ @@ -56,23 +56,34 @@ void verdictCacheClear(VerdictCache *verdictCache, void(*freeData)(PortmasterPac */ int verdictCacheTeardown(VerdictCache *verdictCache, void(*freeData)(PortmasterPacketInfo*, verdict_t)); +/** + * @brief Updates a verdict that is already in the cache + * + * @par verdict_cache = VerdictCache to use + * @par info = pointer to verdictUpdateInfo + * @return error code + * + */ +int verdictCacheUpdate(VerdictCache *verdictCache, VerdictUpdateInfo *info); + /** * @brief Adds verdict to cache * - * @par verdict_cache = verdict_cache to use - * @par packet_info = pointer to packet_info + * @par verdictCache = VerdictCache to use + * @par packetInfo = pointer to PacketInfo * @par verdict = verdict to save * @return error code * */ int verdictCacheAdd(VerdictCache *verdictCache, PortmasterPacketInfo *packetInfo, verdict_t verdict, PortmasterPacketInfo **removedPacketInfo); + /** * @brief returns the verdict of a packet if inside the cache, with redirect info if available * - * @par verdict_cache = verdict_cache to use - * @par packet_info = pointer to packet_info - * @par redir_info = double pointer to packet_info (return value) + * @par verdictCache = VerdictCache to use + * @par packetInfo = pointer to PacketInfo + * @par redirInfo = double pointer to packetInfo (return value) * @par verdict = pointer to verdict (return value) * @return error code * diff --git a/pm_kext/src/pm_callouts.c b/pm_kext/src/pm_callouts.c index 7c9cbe9..672cc98 100644 --- a/pm_kext/src/pm_callouts.c +++ b/pm_kext/src/pm_callouts.c @@ -105,7 +105,6 @@ void respondWithVerdict(UINT32 id, verdict_t verdict) { size_t packetLength = 0; int rc = packetCacheRetrieve(packetCache, id, &packetInfo, &packet, &packetLength); - if (rc != 0) { // packet id was not in packet cache INFO("received verdict response for unknown packet id: %u", id); @@ -142,7 +141,7 @@ void respondWithVerdict(UINT32 id, verdict_t verdict) { //Handle Packet according to Verdict switch (verdict) { case PORTMASTER_VERDICT_DROP: - INFO("PORTMASTER_VERDICT_DROP: %s", printPacketInfo(packetInfo)); + // INFO("PORTMASTER_VERDICT_DROP: %s", printPacketInfo(packetInfo)); portmasterFree(packet); return; case PORTMASTER_VERDICT_BLOCK: @@ -151,7 +150,7 @@ void respondWithVerdict(UINT32 id, verdict_t verdict) { portmasterFree(packet); return; case PORTMASTER_VERDICT_ACCEPT: - DEBUG("PORTMASTER_VERDICT_ACCEPT: %s", printPacketInfo(packetInfo)); + // DEBUG("PORTMASTER_VERDICT_ACCEPT: %s", printPacketInfo(packetInfo)); break; // ACCEPT case PORTMASTER_VERDICT_REDIR_DNS: INFO("PORTMASTER_VERDICT_REDIR_DNS: %s", printPacketInfo(packetInfo)); @@ -195,6 +194,14 @@ PacketCache* getPacketCache() { return packetCache; } +int updateVerdict(VerdictUpdateInfo *info) { + if(info->ipV6) { + return verdictCacheUpdate(verdictCacheV6, info); + } else { + return verdictCacheUpdate(verdictCacheV4, info); + } +} + /****************************************************************** * Classify Functions ******************************************************************/ @@ -321,7 +328,7 @@ FWP_ACTION_TYPE classifySingle( return FWP_ACTION_BLOCK; case PORTMASTER_VERDICT_ACCEPT: - INFO("PORTMASTER_VERDICT_ACCEPT: %s", printPacketInfo(packetInfo)); + // INFO("PORTMASTER_VERDICT_ACCEPT: %s", printPacketInfo(packetInfo)); return FWP_ACTION_PERMIT; case PORTMASTER_VERDICT_REDIR_DNS: diff --git a/pm_kext/src/pm_kernel.c b/pm_kext/src/pm_kernel.c index 86ea160..eeaf855 100644 --- a/pm_kext/src/pm_kernel.c +++ b/pm_kext/src/pm_kernel.c @@ -69,9 +69,6 @@ NTSTATUS driverDeviceControl(__in PDEVICE_OBJECT pDeviceObject, __inout PIRP Ir NTSTATUS InitDriverObject(DRIVER_OBJECT *driverObject, UNICODE_STRING *registryPath, WDFDRIVER *driver, WDFDEVICE *device); -// shared mem to communicate between callout -> device_control -> userland -char globalBuf[256]; - // Global IO Queue for communicating PRKQUEUE globalIOQueue; static LARGE_INTEGER ioQueueTimeout; @@ -274,54 +271,17 @@ NTSTATUS driverDeviceControl(__in PDEVICE_OBJECT pDeviceObject, __inout PIRP Irp PIO_STACK_LOCATION pIoStackLocation = IoGetCurrentIrpStackLocation(Irp); int IoControlCode = pIoStackLocation->Parameters.DeviceIoControl.IoControlCode; switch(IoControlCode) { -#ifdef DEBUG_ON - //Hello World with ke-us shared memory "Irp->AssociatedIrp.SystemBuffer" - case IOCTL_HELLO: { - INFO("IOCTL HELLO"); - const PCHAR welcome = "Hello from kerneland."; - long n100nsTimeCount = 70000000; - // FIXME: warning C4996: 'RtlConvertLongToLargeInteger': was declared deprecated - LARGE_INTEGER li = { .QuadPart = -1 * n100nsTimeCount }; //WTF? - NTSTATUS rc = KeDelayExecutionThread( - UserMode, //KPROCESSOR_MODE WaitMode, KernelMode - true, //Alterable - &li //Unit: 100ns - ); - INFO("Message received : %s", pBuf); - rc = KeDelayExecutionThread( - KernelMode, //KPROCESSOR_MODE WaitMode, - false, //Alterable - &li //Unit: 100ns - ); - RtlZeroMemory(pBuf, pIoStackLocation->Parameters.DeviceIoControl.InputBufferLength); - //Copy message from kernel to pBuf, so that it can be evaluated in userland - RtlCopyMemory(pBuf, welcome, strlen(welcome) ); - //Finish the I/O operation by simply completing the packet and returning - //the same status as in the packet itself. + case IOCTL_VERSION: { + char *versionBuffer = (char*)pBuf; + versionBuffer[0] = PM_VERSION_MAJOR; + versionBuffer[1] = PM_VERSION_MINOR; + versionBuffer[2] = PM_VERSION_REVISION; + versionBuffer[3] = PM_VERSION_BUILD; Irp->IoStatus.Status = STATUS_SUCCESS; - Irp->IoStatus.Information = strlen(welcome); - IoCompleteRequest(Irp,IO_NO_INCREMENT); + Irp->IoStatus.Information = 4; + IoCompleteRequest(Irp, IO_NO_INCREMENT); return STATUS_SUCCESS; } - - case IOCTL_RECV_VERDICT_REQ_POLL: { - DEBUG("IOCTL_RECV_VERDICT_REQ_POLL"); - size_t length = strlen(globalBuf); - if (length > 0) { - RtlZeroMemory(pBuf, pIoStackLocation->Parameters.DeviceIoControl.InputBufferLength); - //Copy message from kernel to pBuf, so that it can be evaluated in userland - RtlCopyMemory(pBuf, globalBuf, strlen(globalBuf) ); - Irp->IoStatus.Status = STATUS_SUCCESS; - Irp->IoStatus.Information = strlen(globalBuf); - IoCompleteRequest(Irp,IO_NO_INCREMENT); - RtlZeroMemory(globalBuf, sizeof(globalBuf)); - return STATUS_SUCCESS; - } - Irp->IoStatus.Status = STATUS_TIMEOUT; - IoCompleteRequest(Irp,IO_NO_INCREMENT); - return STATUS_TIMEOUT; - } -#endif case IOCTL_RECV_VERDICT_REQ: { DEBUG("IOCTL_RECV_VERDICT_REQ"); PLIST_ENTRY ple = KeRemoveQueue( @@ -343,7 +303,7 @@ NTSTATUS driverDeviceControl(__in PDEVICE_OBJECT pDeviceObject, __inout PIRP Irp INFO("List was empty or not-> STATUS_USER_APC"); Irp->IoStatus.Status = STATUS_USER_APC; Irp->IoStatus.Information = 0; - IoCompleteRequest(Irp,IO_NO_INCREMENT); + IoCompleteRequest(Irp, IO_NO_INCREMENT); return STATUS_USER_APC; } @@ -351,7 +311,7 @@ NTSTATUS driverDeviceControl(__in PDEVICE_OBJECT pDeviceObject, __inout PIRP Irp { DataEntry *dentry = (DataEntry*)CONTAINING_RECORD(ple, DataEntry, entry); - int size= sizeof(PortmasterPacketInfo); + int size = sizeof(PortmasterPacketInfo); RtlZeroMemory(pBuf, pIoStackLocation->Parameters.DeviceIoControl.InputBufferLength); //Copy message from kernel to pBuf, so that it can be evaluated in userland @@ -450,7 +410,14 @@ NTSTATUS driverDeviceControl(__in PDEVICE_OBJECT pDeviceObject, __inout PIRP Irp case IOCTL_CLEAR_CACHE: { clearCache(); Irp->IoStatus.Status = STATUS_SUCCESS; - IoCompleteRequest(Irp,IO_NO_INCREMENT); + IoCompleteRequest(Irp, IO_NO_INCREMENT); + return STATUS_SUCCESS; + } + case IOCTL_UPDATE_VERDICT: { + VerdictUpdateInfo *verdictUpdateInfo = (VerdictUpdateInfo*)pBuf; + updateVerdict(verdictUpdateInfo); + Irp->IoStatus.Status = STATUS_SUCCESS; + IoCompleteRequest(Irp, IO_NO_INCREMENT); return STATUS_SUCCESS; } default: { diff --git a/pm_kext_glue_dll/src/pm_api.c b/pm_kext_glue_dll/src/pm_api.c index b137c55..cac84bc 100644 --- a/pm_kext_glue_dll/src/pm_api.c +++ b/pm_kext_glue_dll/src/pm_api.c @@ -188,13 +188,13 @@ extern _EXPORT UINT32 PortmasterClearCache() { */ extern _EXPORT UINT32 PortmasterRecvVerdictRequestHello(PortmasterPacketInfo *packetInfo) { int rc = 0; - char *welcome = "Hello from userland."; - DWORD dwBytesRead = 0; - char ReadBuffer[50] = {0}; + // char *welcome = "Hello from userland."; + // DWORD dwBytesRead = 0; + // char ReadBuffer[50] = {0}; - DeviceIoControl(deviceHandle, IOCTL_HELLO, welcome, (DWORD)strlen(welcome), ReadBuffer, sizeof(ReadBuffer), &dwBytesRead, NULL); - INFO("Message received from kerneland : %s", ReadBuffer); - INFO("Bytes read : %d", dwBytesRead); + // DeviceIoControl(deviceHandle, IOCTL_HELLO, welcome, (DWORD)strlen(welcome), ReadBuffer, sizeof(ReadBuffer), &dwBytesRead, NULL); + // INFO("Message received from kerneland : %s", ReadBuffer); + // INFO("Bytes read : %d", dwBytesRead); return rc; } From 609336f8bccd8c34b34fd037b7d8f8a8dbafaf81 Mon Sep 17 00:00:00 2001 From: Vladimir Date: Wed, 9 Nov 2022 10:38:42 +0100 Subject: [PATCH 2/3] Packet cache size increase and discard too old request --- include/pm_common.h | 9 +++---- pm_kext/col/packet_cache.c | 55 +++++++++++++++++++++++++------------- pm_kext/src/pm_callouts.c | 6 ++--- 3 files changed, 43 insertions(+), 27 deletions(-) diff --git a/include/pm_common.h b/include/pm_common.h index 50d2f69..a6cda05 100644 --- a/include/pm_common.h +++ b/include/pm_common.h @@ -147,15 +147,14 @@ static const char* VERDICT_NAMES[] = { "PORTMASTER_VERDICT_ERROR", * CACHE SIZES for packet and verdict cache * Packet cache: * - One entry can be as big as the MTU - eg. 1500 Bytes. - * - But normally it will be much smaller, eg. a TCP SYN packet. - * - A size of 512 with a mean entry size of 750 Bytes would result in a max space requirement of about 380KB. + * - A size of 1024 with a mean entry size of 750 Bytes would result in a max space requirement of about 760KB. * - This cache is quickly emptied, but is not purged, so errors in Portmaster could result in dead entries. * Verdict cache: * - On entry has about 50 Bytes. - * - A size of 1024 would result in a max space requirements of about 50KB. + * - A size of 1024 would result in a requirements of about 50KB which is allocated on initialization. * - This cache is not emptied or purged, it will pretty much always be at max capacity. */ -#define PM_PACKET_CACHE_SIZE 512 +#define PM_PACKET_CACHE_SIZE 1024 #define PM_VERDICT_CACHE_SIZE 1024 /* @@ -171,7 +170,7 @@ typedef struct { */ typedef struct { UINT32 id; - UINT32 len; // preset with maxlen of payload from caller -> set with acutal len of payload from receiver + UINT32 len; // preset with maxlen of payload from caller -> set with actual len of payload from receiver } PortmasterPayload; typedef struct { diff --git a/pm_kext/col/packet_cache.c b/pm_kext/col/packet_cache.c index e2ff233..5238281 100644 --- a/pm_kext/col/packet_cache.c +++ b/pm_kext/col/packet_cache.c @@ -192,22 +192,31 @@ static PacketCacheItem* getPacketFromID(PacketCache *packetCache, UINT32 packetI int packetCacheRetrieve(PacketCache *packetCache, UINT32 packetID, PortmasterPacketInfo **packetInfo, void **packet, size_t *packetLength) { DEBUG("retrieve_packet called"); + int rc = 0; KLOCK_QUEUE_HANDLE lockHandle = {0}; KeAcquireInStackQueuedSpinLock(&packetCache->lock, &lockHandle); PacketCacheItem *item = getPacketFromID(packetCache, packetID); + + // Check if entry was already overwritten + bool cacheFilledMoreThenOnce = (packetCache->nextPacketID - 1) > packetCache->maxSize; + if(cacheFilledMoreThenOnce && packetID <= (packetCache->nextPacketID - packetCache->maxSize - 1)) { + DEBUG("Requested packet was overwritten: %d", packetID); + rc = 1; + } - if(item != NULL) { - *packetInfo = item->packetInfo; - *packet = item->packet; - *packetLength = item->packetLength; - memset(item, 0, sizeof(PacketCacheItem)); + if(rc == 0) { + if(item != NULL) { + *packetInfo = item->packetInfo; + *packet = item->packet; + *packetLength = item->packetLength; + memset(item, 0, sizeof(PacketCacheItem)); + } else { + rc = 2; + } } - KeReleaseInStackQueuedSpinLock(&lockHandle); - if(item == NULL) { - return 1; - } - return 0; + + return rc; } /** @@ -221,19 +230,27 @@ int packetCacheRetrieve(PacketCache *packetCache, UINT32 packetID, PortmasterPac */ int packetCacheGet(PacketCache *packetCache, uint32_t packetID, void **packet, size_t *packetLength) { DEBUG("packetCacheGet called"); - + int rc = 0; KLOCK_QUEUE_HANDLE lockHandle = {0}; KeAcquireInStackQueuedSpinLock(&packetCache->lock, &lockHandle); - PacketCacheItem *item = getPacketFromID(packetCache, packetID); - if(item != NULL) { - *packet = item->packet; - *packetLength = item->packetLength; + // Check if entry was already overwritten + bool cacheFilledMoreThenOnce = (packetCache->nextPacketID - 1) > packetCache->maxSize; + if(cacheFilledMoreThenOnce && packetID <= (packetCache->nextPacketID - packetCache->maxSize - 1)) { + DEBUG("Requested packet was overwritten: %d", packetID); + rc = 1; } - KeReleaseInStackQueuedSpinLock(&lockHandle); - if(item == NULL) { - return 1; + if(rc == 0) { + PacketCacheItem *item = getPacketFromID(packetCache, packetID); + if(item != NULL) { + *packet = item->packet; + *packetLength = item->packetLength; + } else { + rc = 2; + } } - return 0; + KeReleaseInStackQueuedSpinLock(&lockHandle); + + return rc; } diff --git a/pm_kext/src/pm_callouts.c b/pm_kext/src/pm_callouts.c index 672cc98..76eba13 100644 --- a/pm_kext/src/pm_callouts.c +++ b/pm_kext/src/pm_callouts.c @@ -141,7 +141,7 @@ void respondWithVerdict(UINT32 id, verdict_t verdict) { //Handle Packet according to Verdict switch (verdict) { case PORTMASTER_VERDICT_DROP: - // INFO("PORTMASTER_VERDICT_DROP: %s", printPacketInfo(packetInfo)); + INFO("PORTMASTER_VERDICT_DROP: %s", printPacketInfo(packetInfo)); portmasterFree(packet); return; case PORTMASTER_VERDICT_BLOCK: @@ -150,7 +150,7 @@ void respondWithVerdict(UINT32 id, verdict_t verdict) { portmasterFree(packet); return; case PORTMASTER_VERDICT_ACCEPT: - // DEBUG("PORTMASTER_VERDICT_ACCEPT: %s", printPacketInfo(packetInfo)); + DEBUG("PORTMASTER_VERDICT_ACCEPT: %s", printPacketInfo(packetInfo)); break; // ACCEPT case PORTMASTER_VERDICT_REDIR_DNS: INFO("PORTMASTER_VERDICT_REDIR_DNS: %s", printPacketInfo(packetInfo)); @@ -328,7 +328,7 @@ FWP_ACTION_TYPE classifySingle( return FWP_ACTION_BLOCK; case PORTMASTER_VERDICT_ACCEPT: - // INFO("PORTMASTER_VERDICT_ACCEPT: %s", printPacketInfo(packetInfo)); + INFO("PORTMASTER_VERDICT_ACCEPT: %s", printPacketInfo(packetInfo)); return FWP_ACTION_PERMIT; case PORTMASTER_VERDICT_REDIR_DNS: From 4d91978d4f8479c9e2cf51d63d5a41ca42274aac Mon Sep 17 00:00:00 2001 From: Vladimir Date: Thu, 10 Nov 2022 17:23:55 +0100 Subject: [PATCH 3/3] small readability improvements --- pm_kext/col/packet_cache.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/pm_kext/col/packet_cache.c b/pm_kext/col/packet_cache.c index 5238281..4d9e926 100644 --- a/pm_kext/col/packet_cache.c +++ b/pm_kext/col/packet_cache.c @@ -33,7 +33,7 @@ typedef struct PacketCacheItem { typedef struct { PacketCacheItem *packets; UINT32 maxSize; - UINT32 nextPacketID; + INT64 nextPacketID; // INT64 so we can easy check for overwrites KSPIN_LOCK lock; } PacketCache; @@ -132,7 +132,7 @@ uint32_t packetCacheRegister(PacketCache* packetCache, PortmasterPacketInfo *pac KLOCK_QUEUE_HANDLE lockHandle = {0}; KeAcquireInStackQueuedSpinLock(&packetCache->lock, &lockHandle); - UINT32 packetIndex = getIndexFromPacketID(packetCache, packetCache->nextPacketID); + UINT32 packetIndex = getIndexFromPacketID(packetCache, (UINT32)packetCache->nextPacketID); PacketCacheItem *newItem = &packetCache->packets[packetIndex]; if(newItem->packetInfo != NULL && newItem->packet != NULL) { @@ -141,7 +141,7 @@ uint32_t packetCacheRegister(PacketCache* packetCache, PortmasterPacketInfo *pac memset(newItem, 0, sizeof(PacketCacheItem)); } - newItem->packetID = packetCache->nextPacketID; + newItem->packetID = (UINT32)packetCache->nextPacketID; newItem->packetInfo = packetInfo; newItem->packet = packet; newItem->packetLength = packetLength; @@ -195,15 +195,15 @@ int packetCacheRetrieve(PacketCache *packetCache, UINT32 packetID, PortmasterPac int rc = 0; KLOCK_QUEUE_HANDLE lockHandle = {0}; KeAcquireInStackQueuedSpinLock(&packetCache->lock, &lockHandle); - PacketCacheItem *item = getPacketFromID(packetCache, packetID); - // Check if entry was already overwritten - bool cacheFilledMoreThenOnce = (packetCache->nextPacketID - 1) > packetCache->maxSize; - if(cacheFilledMoreThenOnce && packetID <= (packetCache->nextPacketID - packetCache->maxSize - 1)) { + // Check if entry was overwritten + if((INT64)packetID <= (packetCache->nextPacketID - (INT64)packetCache->maxSize - 1)) { DEBUG("Requested packet was overwritten: %d", packetID); rc = 1; } + PacketCacheItem *item = getPacketFromID(packetCache, packetID); + if(rc == 0) { if(item != NULL) { *packetInfo = item->packetInfo; @@ -234,9 +234,8 @@ int packetCacheGet(PacketCache *packetCache, uint32_t packetID, void **packet, s KLOCK_QUEUE_HANDLE lockHandle = {0}; KeAcquireInStackQueuedSpinLock(&packetCache->lock, &lockHandle); - // Check if entry was already overwritten - bool cacheFilledMoreThenOnce = (packetCache->nextPacketID - 1) > packetCache->maxSize; - if(cacheFilledMoreThenOnce && packetID <= (packetCache->nextPacketID - packetCache->maxSize - 1)) { + // Check if entry was overwritten + if((INT64)packetID <= (packetCache->nextPacketID - (INT64)packetCache->maxSize - 1)) { DEBUG("Requested packet was overwritten: %d", packetID); rc = 1; }