From 2419b5e354d826aafef5300d70e6406a18d73d0d Mon Sep 17 00:00:00 2001 From: Michael Agun Date: Tue, 11 Aug 2026 15:46:16 -0700 Subject: [PATCH 1/3] Add WFP filter and callout enumeration to the mock The user-mode WFP mock could add and delete filters, callouts, sub-layers and providers, but could not enumerate any of them, so code that discovers WFP objects rather than remembering them could not be exercised at all. Add the enumeration APIs for filters and callouts: FwpmFilterCreateEnumHandle0 / FwpmFilterEnum0 / FwpmFilterDestroyEnumHandle0 FwpmCalloutCreateEnumHandle0 / FwpmCalloutEnum0 / FwpmCalloutDestroyEnumHandle0 plus FwpmFreeMemory0, which callers need to release an enumeration result. Enumerations are snapshots taken when the enum handle is created, matching the documented behaviour that a WFP enumerator "is not live" and does not reflect changes made after the handle exists. That is what makes the common "enumerate everything, then delete each entry" pattern terminate. The enumeration templates honour providerKey and layerKey filtering. Results are returned as a single allocation laid out as [pointers][objects][GUIDs] so that one FwpmFreeMemory0 call frees the whole batch, as real WFP requires, and the enumeration cursor only advances over entries the caller actually received. Two fidelity problems had to be fixed for enumeration to return usable objects: * FWPM_FILTER0::providerKey and FWPM_CALLOUT0::providerKey are pointers into caller memory, and the mock stored the structures by value. Every stored object therefore aliased whatever the caller happened to pass, and reading providerKey later -- which enumeration by provider must do -- could dereference memory the caller had already released. Both are now deep-copied into the stored entry, with the stored pointer re-bound to that copy. * Deletes of a missing object returned STATUS_INVALID_PARAMETER or STATUS_NOT_FOUND (0xC0000225), where real WFP returns FWP_E_FILTER_NOT_FOUND (0x80320003), FWP_E_CALLOUT_NOT_FOUND (0x80320001) and FWP_E_SUBLAYER_NOT_FOUND (0x80320007). Callers that distinguish "already gone", which is benign, from a genuine failure could not do so, so their not-found handling was silently dead under the mock. --- src/Source.def | 7 ++ src/fwp_um.cpp | 190 +++++++++++++++++++++++++++++++++++++- src/fwp_um.h | 243 +++++++++++++++++++++++++++++++++++++++++++++---- src/tags.h | 1 + 4 files changed, 420 insertions(+), 21 deletions(-) diff --git a/src/Source.def b/src/Source.def index c09998c..e87ed94 100644 --- a/src/Source.def +++ b/src/Source.def @@ -9,11 +9,18 @@ EXPORTS IoFileObjectType DATA FwpmCalloutAdd0 + FwpmCalloutCreateEnumHandle0 FwpmCalloutDeleteByKey0 + FwpmCalloutDestroyEnumHandle0 + FwpmCalloutEnum0 FwpmEngineClose0 FwpmEngineOpen0 FwpmFilterAdd0 + FwpmFilterCreateEnumHandle0 FwpmFilterDeleteById0 + FwpmFilterDestroyEnumHandle0 + FwpmFilterEnum0 + FwpmFreeMemory0 FwpmProviderAdd0 FwpmProviderDeleteByKey0 FwpmSubLayerAdd0 diff --git a/src/fwp_um.cpp b/src/fwp_um.cpp index 658f225..6fd961b 100644 --- a/src/fwp_um.cpp +++ b/src/fwp_um.cpp @@ -543,10 +543,194 @@ _IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS FwpmFilterDeleteById0(_In_ HANDLE en if (engine.remove_fwpm_filter(id)) { return STATUS_SUCCESS; } else { - return STATUS_INVALID_PARAMETER; + // Real WFP reports a missing filter as FWP_E_FILTER_NOT_FOUND. Callers distinguish "already gone" + // (benign) from a genuine failure, so returning a generic status here would hide that distinction. + return (NTSTATUS)FWP_E_FILTER_NOT_FOUND; } } +// Enumeration results are returned as a single allocation laid out as [array of N pointers][N objects][N GUIDs], +// so a caller frees the whole result with one FwpmFreeMemory0 call, as real WFP requires. The trailing GUID slots +// back the providerKey pointers of the returned objects, which would otherwise dangle once this batch is freed. +static _Ret_maybenull_ uint8_t* +_fwpm_enum_allocate_block(size_t count, size_t object_size, _Out_ size_t* object_offset, _Out_ size_t* guid_offset) +{ + *object_offset = count * sizeof(void*); + *guid_offset = *object_offset + count * object_size; + + return (uint8_t*)cxplat_allocate( + CXPLAT_POOL_FLAG_NON_PAGED, *guid_offset + count * sizeof(GUID), USERSIM_TAG_FWPM_ENUM); +} + +_IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS FwpmFilterCreateEnumHandle0( + _In_ HANDLE engine_handle, + _In_opt_ const FWPM_FILTER_ENUM_TEMPLATE0* enum_template, + _Out_ HANDLE* enum_handle) +{ + if (cxplat_fault_injection_inject_fault()) { + *enum_handle = NULL; + return STATUS_NO_MEMORY; + } + + auto& engine = *reinterpret_cast(engine_handle); + + *enum_handle = (HANDLE)(uintptr_t)engine.create_fwpm_filter_enum_handle(enum_template); + return STATUS_SUCCESS; +} + +_IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS FwpmFilterEnum0( + _In_ HANDLE engine_handle, + _In_ HANDLE enum_handle, + _In_ uint32_t num_entries_requested, + _Outptr_result_buffer_(*num_entries_returned) FWPM_FILTER0*** entries, + _Out_ uint32_t* num_entries_returned) +{ + if (cxplat_fault_injection_inject_fault()) { + return STATUS_NO_MEMORY; + } + + auto& engine = *reinterpret_cast(engine_handle); + + std::vector batch; + if (!engine.next_fwpm_filter_enum_entries((uint64_t)(uintptr_t)enum_handle, num_entries_requested, batch)) { + return STATUS_INVALID_HANDLE; + } + + *entries = nullptr; + *num_entries_returned = 0; + if (batch.empty()) { + // Real WFP returns success with zero entries at the end of an enumeration. + return STATUS_SUCCESS; + } + + size_t object_offset; + size_t guid_offset; + uint8_t* block = _fwpm_enum_allocate_block(batch.size(), sizeof(FWPM_FILTER0), &object_offset, &guid_offset); + if (block == nullptr) { + // The entries were already taken from the snapshot, so put the cursor back; otherwise this batch would be + // skipped for good and a caller that retried would silently see fewer objects than exist. + engine.rewind_fwpm_filter_enum((uint64_t)(uintptr_t)enum_handle, batch.size()); + return STATUS_NO_MEMORY; + } + + auto pointers = (FWPM_FILTER0**)block; + auto objects = (FWPM_FILTER0*)(block + object_offset); + auto guids = (GUID*)(block + guid_offset); + + for (size_t index = 0; index < batch.size(); index++) { + objects[index] = batch[index].filter; + guids[index] = batch[index].provider_key; + if (objects[index].providerKey != nullptr) { + objects[index].providerKey = &guids[index]; + } + pointers[index] = &objects[index]; + } + + *entries = pointers; + *num_entries_returned = (uint32_t)batch.size(); + return STATUS_SUCCESS; +} + +_IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS + FwpmFilterDestroyEnumHandle0(_In_ HANDLE engine_handle, _Inout_ HANDLE enum_handle) +{ + auto& engine = *reinterpret_cast(engine_handle); + + if (!engine.destroy_fwpm_filter_enum_handle((uint64_t)(uintptr_t)enum_handle)) { + return STATUS_INVALID_HANDLE; + } + return STATUS_SUCCESS; +} + +_IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS FwpmCalloutCreateEnumHandle0( + _In_ HANDLE engine_handle, + _In_opt_ const FWPM_CALLOUT_ENUM_TEMPLATE0* enum_template, + _Out_ HANDLE* enum_handle) +{ + if (cxplat_fault_injection_inject_fault()) { + *enum_handle = NULL; + return STATUS_NO_MEMORY; + } + + auto& engine = *reinterpret_cast(engine_handle); + + *enum_handle = (HANDLE)(uintptr_t)engine.create_fwpm_callout_enum_handle(enum_template); + return STATUS_SUCCESS; +} + +_IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS FwpmCalloutEnum0( + _In_ HANDLE engine_handle, + _In_ HANDLE enum_handle, + _In_ uint32_t num_entries_requested, + _Outptr_result_buffer_(*num_entries_returned) FWPM_CALLOUT0*** entries, + _Out_ uint32_t* num_entries_returned) +{ + if (cxplat_fault_injection_inject_fault()) { + return STATUS_NO_MEMORY; + } + + auto& engine = *reinterpret_cast(engine_handle); + + std::vector batch; + if (!engine.next_fwpm_callout_enum_entries((uint64_t)(uintptr_t)enum_handle, num_entries_requested, batch)) { + return STATUS_INVALID_HANDLE; + } + + *entries = nullptr; + *num_entries_returned = 0; + if (batch.empty()) { + return STATUS_SUCCESS; + } + + size_t object_offset; + size_t guid_offset; + uint8_t* block = _fwpm_enum_allocate_block(batch.size(), sizeof(FWPM_CALLOUT0), &object_offset, &guid_offset); + if (block == nullptr) { + // See the filter case above: the cursor must not advance past a batch the caller never received. + engine.rewind_fwpm_callout_enum((uint64_t)(uintptr_t)enum_handle, batch.size()); + return STATUS_NO_MEMORY; + } + + auto pointers = (FWPM_CALLOUT0**)block; + auto objects = (FWPM_CALLOUT0*)(block + object_offset); + auto guids = (GUID*)(block + guid_offset); + + for (size_t index = 0; index < batch.size(); index++) { + objects[index] = batch[index].callout; + guids[index] = batch[index].provider_key; + if (objects[index].providerKey != nullptr) { + objects[index].providerKey = &guids[index]; + } + pointers[index] = &objects[index]; + } + + *entries = pointers; + *num_entries_returned = (uint32_t)batch.size(); + return STATUS_SUCCESS; +} + +_IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS + FwpmCalloutDestroyEnumHandle0(_In_ HANDLE engine_handle, _Inout_ HANDLE enum_handle) +{ + auto& engine = *reinterpret_cast(engine_handle); + + if (!engine.destroy_fwpm_callout_enum_handle((uint64_t)(uintptr_t)enum_handle)) { + return STATUS_INVALID_HANDLE; + } + return STATUS_SUCCESS; +} + +void NTAPI +FwpmFreeMemory0(_Inout_ void** p) +{ + if (p == nullptr || *p == nullptr) { + return; + } + + cxplat_free(*p, CXPLAT_POOL_FLAG_NON_PAGED, USERSIM_TAG_FWPM_ENUM); + *p = nullptr; +} + _IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS FwpmTransactionBegin0(_In_ _Acquires_lock_(_Curr_) HANDLE engine_handle, _In_ uint32_t flags) { @@ -626,7 +810,7 @@ _IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS FwpmCalloutDeleteByKey0(_In_ HANDLE auto& engine = *reinterpret_cast(engine_handle); if (!engine.remove_fwpm_callout(key)) { - return STATUS_NOT_FOUND; + return (NTSTATUS)FWP_E_CALLOUT_NOT_FOUND; } return STATUS_SUCCESS; } @@ -707,7 +891,7 @@ _IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS auto& engine = *reinterpret_cast(engine_handle); if (!engine.remove_fwpm_sub_layer(sub_layer_key)) { - return STATUS_NOT_FOUND; + return (NTSTATUS)FWP_E_SUBLAYER_NOT_FOUND; } return STATUS_SUCCESS; } diff --git a/src/fwp_um.h b/src/fwp_um.h index 9558e85..5587fea 100644 --- a/src/fwp_um.h +++ b/src/fwp_um.h @@ -7,10 +7,41 @@ #include #include +#include typedef std::unique_lock exclusive_lock_t; typedef std::shared_lock shared_lock_t; +// A WFP filter as stored by the mock engine. +// +// FWPM_FILTER0::providerKey is a pointer into caller-owned memory. Storing the FWPM_FILTER0 by value would +// therefore alias whatever the caller happened to pass, and any later read of providerKey (for example when +// enumerating filters by provider) would dereference memory the caller may already have released. The mock +// deep-copies the GUID into the entry and re-points the stored filter at its own copy. std::unordered_map is +// node-based, so the address of provider_key remains stable across rehashing. +typedef struct _fwpm_filter_entry +{ + FWPM_FILTER0 filter; + GUID provider_key; +} fwpm_filter_entry_t; + +// A WFP callout as stored by the mock engine. FWPM_CALLOUT0::providerKey has the same caller-owned-pointer +// problem as FWPM_FILTER0::providerKey, and is deep-copied for the same reason. +typedef struct _fwpm_callout_entry +{ + FWPM_CALLOUT0 callout; + GUID provider_key; +} fwpm_callout_entry_t; + +// An in-progress enumeration. Real WFP enumerations are snapshots taken when the enum handle is created, so +// objects deleted while an enumeration is open are still returned and objects added are not. The mock models +// that explicitly, which also makes the common "enumerate everything, then delete each entry" pattern safe. +template struct fwpm_enum_state_t +{ + std::vector entries; + size_t next_index = 0; +}; + typedef class fwp_engine_t { public: @@ -30,7 +61,16 @@ typedef class fwp_engine_t { exclusive_lock_t l(lock); uint32_t id = next_id++; - fwpm_callouts.insert({id, *callout}); + auto& stored = fwpm_callouts.insert({id, fwpm_callout_entry_t{*callout, {}}}).first->second; + + // Re-point the stored callout at the entry's own copy of the provider key (see fwpm_callout_entry_t). + if (callout->providerKey != nullptr) { + stored.provider_key = *callout->providerKey; + stored.callout.providerKey = &stored.provider_key; + } else { + stored.callout.providerKey = nullptr; + } + return id; } @@ -45,8 +85,8 @@ typedef class fwp_engine_t remove_fwpm_callout(_In_ const GUID* key) { exclusive_lock_t l(lock); - for (auto& [first, callout] : fwpm_callouts) { - if (memcmp(&callout.calloutKey, key, sizeof(GUID)) == 0) { + for (auto& [first, entry] : fwpm_callouts) { + if (memcmp(&entry.callout.calloutKey, key, sizeof(GUID)) == 0) { return fwpm_callouts.erase(first) == 1; } } @@ -135,7 +175,15 @@ typedef class fwp_engine_t { exclusive_lock_t l(lock); id = next_id++; - fwpm_filters.insert({id, *filter}); + auto& stored = fwpm_filters.insert({id, fwpm_filter_entry_t{*filter, {}}}).first->second; + + // Re-point the stored filter at the entry's own copy of the provider key (see fwpm_filter_entry_t). + if (filter->providerKey != nullptr) { + stored.provider_key = *filter->providerKey; + stored.filter.providerKey = &stored.provider_key; + } else { + stored.filter.providerKey = nullptr; + } callout = get_fwps_callout(&filter->action.calloutKey); CXPLAT_DEBUG_ASSERT(callout != nullptr); @@ -161,9 +209,9 @@ typedef class fwp_engine_t if (it.first == id) { // May be null if the callout function has already been unregistered (e.g., during driver // unload); in that case WFP delivers no delete notification (handled below). - callout = get_fwps_callout(&it.second.action.calloutKey); + callout = get_fwps_callout(&it.second.filter.action.calloutKey); fwps_filter.filterId = id; - fwps_filter.context = it.second.rawContext; + fwps_filter.context = it.second.filter.rawContext; break; } } @@ -218,6 +266,122 @@ typedef class fwp_engine_t return fwpm_filters.size(); } + // Creates a snapshot of the filters matching the (optional) enumeration template, and returns a handle to it. + // A null template matches every filter, as it does in real WFP. + _Requires_lock_not_held_(this->lock) uint64_t + create_fwpm_filter_enum_handle(_In_opt_ const FWPM_FILTER_ENUM_TEMPLATE0* enum_template) + { + exclusive_lock_t l(lock); + uint64_t handle = next_enum_handle++; + auto& state = fwpm_filter_enums[handle]; + for (auto& [id, entry] : fwpm_filters) { + if (!provider_key_matches(entry.filter.providerKey, enum_template ? enum_template->providerKey : nullptr)) { + continue; + } + if (enum_template != nullptr && !is_null_guid(enum_template->layerKey) && + memcmp(&entry.filter.layerKey, &enum_template->layerKey, sizeof(GUID)) != 0) { + continue; + } + + state.entries.push_back(entry); + rebind_filter_entry(state.entries.back()); + } + return handle; + } + + // Copies up to 'requested' entries from the snapshot into 'out', advancing the enumeration cursor. + _Requires_lock_not_held_(this->lock) bool next_fwpm_filter_enum_entries( + uint64_t handle, uint32_t requested, _Inout_ std::vector& out) + { + exclusive_lock_t l(lock); + auto it = fwpm_filter_enums.find(handle); + if (it == fwpm_filter_enums.end()) { + return false; + } + + auto& state = it->second; + while (out.size() < requested && state.next_index < state.entries.size()) { + out.push_back(state.entries[state.next_index++]); + rebind_filter_entry(out.back()); + } + return true; + } + + _Requires_lock_not_held_(this->lock) bool destroy_fwpm_filter_enum_handle(uint64_t handle) + { + exclusive_lock_t l(lock); + return fwpm_filter_enums.erase(handle) == 1; + } + + // Rewinds the filter enumeration cursor by 'count' entries. Used when a batch was taken from the snapshot but + // could not be handed to the caller, so those entries are enumerated again rather than silently skipped. + _Requires_lock_not_held_(this->lock) void rewind_fwpm_filter_enum(uint64_t handle, size_t count) + { + exclusive_lock_t l(lock); + auto it = fwpm_filter_enums.find(handle); + if (it != fwpm_filter_enums.end()) { + auto& state = it->second; + state.next_index -= (count < state.next_index) ? count : state.next_index; + } + } + + // Creates a snapshot of the callouts matching the (optional) enumeration template, and returns a handle to it. + _Requires_lock_not_held_(this->lock) uint64_t + create_fwpm_callout_enum_handle(_In_opt_ const FWPM_CALLOUT_ENUM_TEMPLATE0* enum_template) + { + exclusive_lock_t l(lock); + uint64_t handle = next_enum_handle++; + auto& state = fwpm_callout_enums[handle]; + for (auto& [id, entry] : fwpm_callouts) { + if (!provider_key_matches( + entry.callout.providerKey, enum_template ? enum_template->providerKey : nullptr)) { + continue; + } + if (enum_template != nullptr && !is_null_guid(enum_template->layerKey) && + memcmp(&entry.callout.applicableLayer, &enum_template->layerKey, sizeof(GUID)) != 0) { + continue; + } + + state.entries.push_back(entry); + rebind_callout_entry(state.entries.back()); + } + return handle; + } + + _Requires_lock_not_held_(this->lock) bool next_fwpm_callout_enum_entries( + uint64_t handle, uint32_t requested, _Inout_ std::vector& out) + { + exclusive_lock_t l(lock); + auto it = fwpm_callout_enums.find(handle); + if (it == fwpm_callout_enums.end()) { + return false; + } + + auto& state = it->second; + while (out.size() < requested && state.next_index < state.entries.size()) { + out.push_back(state.entries[state.next_index++]); + rebind_callout_entry(out.back()); + } + return true; + } + + _Requires_lock_not_held_(this->lock) bool destroy_fwpm_callout_enum_handle(uint64_t handle) + { + exclusive_lock_t l(lock); + return fwpm_callout_enums.erase(handle) == 1; + } + + // Rewinds the callout enumeration cursor by 'count' entries. See rewind_fwpm_filter_enum. + _Requires_lock_not_held_(this->lock) void rewind_fwpm_callout_enum(uint64_t handle, size_t count) + { + exclusive_lock_t l(lock); + auto it = fwpm_callout_enums.find(handle); + if (it != fwpm_callout_enums.end()) { + auto& state = it->second; + state.next_index -= (count < state.next_index) ? count : state.next_index; + } + } + _Requires_lock_not_held_(this->lock) void add_fwpm_provider(_In_ const FWPM_PROVIDER* provider) { UNREFERENCED_PARAMETER(provider); @@ -305,6 +469,45 @@ typedef class fwp_engine_t } private: + // Re-points a copied entry's providerKey at its own GUID copy. A byte-wise copy of an entry would otherwise + // leave providerKey aliasing the GUID inside the entry it was copied from (see fwpm_filter_entry_t). + static void + rebind_filter_entry(_Inout_ fwpm_filter_entry_t& entry) + { + if (entry.filter.providerKey != nullptr) { + entry.filter.providerKey = &entry.provider_key; + } + } + + static void + rebind_callout_entry(_Inout_ fwpm_callout_entry_t& entry) + { + if (entry.callout.providerKey != nullptr) { + entry.callout.providerKey = &entry.provider_key; + } + } + + static bool + is_null_guid(_In_ const GUID& guid) + { + static const GUID null_guid = {}; + return memcmp(&guid, &null_guid, sizeof(GUID)) == 0; + } + + // Applies an enumeration template's providerKey filter. A null template key matches every object, including + // objects with no provider; a non-null template key matches only objects tagged with that exact provider. + static bool + provider_key_matches(_In_opt_ const GUID* object_key, _In_opt_ const GUID* template_key) + { + if (template_key == nullptr) { + return true; + } + if (object_key == nullptr) { + return false; + } + return memcmp(object_key, template_key, sizeof(GUID)) == 0; + } + _Requires_lock_not_held_(this->lock) FWP_ACTION_TYPE test_callout( uint16_t layer_id, _In_ const GUID& layer_guid, @@ -320,9 +523,9 @@ typedef class fwp_engine_t _Ret_maybenull_ const FWPM_FILTER* get_fwpm_filter_with_context_under_lock(_In_ const GUID& layer_guid) { - for (auto& [first, filter] : fwpm_filters) { - if (memcmp(&filter.layerKey, &layer_guid, sizeof(GUID)) == 0 && filter.rawContext != 0) { - return &filter; + for (auto& [first, entry] : fwpm_filters) { + if (memcmp(&entry.filter.layerKey, &layer_guid, sizeof(GUID)) == 0 && entry.filter.rawContext != 0) { + return &entry.filter; } } return nullptr; @@ -331,10 +534,11 @@ typedef class fwp_engine_t _Ret_maybenull_ const FWPM_FILTER* get_fwpm_filter_with_context_under_lock(_In_ const GUID& layer_guid, _In_ const GUID& sublayer_guid) { - for (auto& [first, filter] : fwpm_filters) { - if (memcmp(&filter.layerKey, &layer_guid, sizeof(GUID)) == 0 && - memcmp(&filter.subLayerKey, &sublayer_guid, sizeof(GUID)) == 0 && filter.rawContext != 0) { - return &filter; + for (auto& [first, entry] : fwpm_filters) { + if (memcmp(&entry.filter.layerKey, &layer_guid, sizeof(GUID)) == 0 && + memcmp(&entry.filter.subLayerKey, &sublayer_guid, sizeof(GUID)) == 0 && + entry.filter.rawContext != 0) { + return &entry.filter; } } return nullptr; @@ -343,9 +547,9 @@ typedef class fwp_engine_t _Ret_maybenull_ const GUID* get_callout_key_from_layer_guid_under_lock(_In_ const GUID* layer_guid) { - for (auto& [first, callout] : fwpm_callouts) { - if (callout.applicableLayer == *layer_guid) { - return &callout.calloutKey; + for (auto& [first, entry] : fwpm_callouts) { + if (entry.callout.applicableLayer == *layer_guid) { + return &entry.callout.calloutKey; } } return nullptr; @@ -378,10 +582,13 @@ typedef class fwp_engine_t std::shared_mutex lock; uint32_t next_id = 1; uint32_t next_flow_id = 1; + uint64_t next_enum_handle = 1; uint32_t _filter_delete_failure_count = 0; // Test-only WFP filter delete fault-injection counter. std::unordered_map fwps_callouts; - std::unordered_map fwpm_callouts; - std::unordered_map fwpm_filters; + std::unordered_map fwpm_callouts; + std::unordered_map fwpm_filters; + std::unordered_map> fwpm_filter_enums; + std::unordered_map> fwpm_callout_enums; std::unordered_map fwpm_sub_layers; std::unordered_map fwpm_flow_contexts; GUID _default_sublayer = {}; diff --git a/src/tags.h b/src/tags.h index b1a0f46..31bdba6 100644 --- a/src/tags.h +++ b/src/tags.h @@ -6,6 +6,7 @@ #define USERSIM_TAG_ACCOUNT_NAME 'ansu' #define USERSIM_TAG_ETW_PROVIDER 'pesu' #define USERSIM_TAG_FWPS_CONNECT_REQUEST0 'cfsu' +#define USERSIM_TAG_FWPM_ENUM 'efsu' #define USERSIM_TAG_HANDLE 'ahsu' #define USERSIM_TAG_IO_WORK_ITEM 'wisu' #define USERSIM_TAG_MDL 'dmsu' From 59caf220a9911464e975d53e331336b5e6798da1 Mon Sep 17 00:00:00 2001 From: Michael Agun Date: Wed, 12 Aug 2026 10:50:19 -0700 Subject: [PATCH 2/3] Model WFP object identity and referential integrity The mock accepted any FwpmProviderAdd and deleted any object on request, so two behaviours that real WFP relies on could not be observed by a test: * A provider was not modelled at all -- add_fwpm_provider and remove_fwpm_provider were no-ops, so FwpmProviderAdd always succeeded and could never report FWP_E_ALREADY_EXISTS. * Nothing tracked references, so a callout, sub-layer or provider could be deleted while a filter still pointed at it, where real WFP returns FWP_E_IN_USE. Together these are the mechanism behind a real class of driver bug: a filter whose delete fails keeps its callout, sub-layer and provider alive, those objects outlive the driver, and the next start fails when it tries to add a provider that is already there. A mock that cannot represent that cannot regression-test it. Store providers, and reject a duplicate add. Add reference checks so deleting a referenced callout, sub-layer or provider returns FWP_E_IN_USE, and fold the not-found and in-use cases into single delete_fwpm_* methods so the check and the erase happen under one lock acquisition. FWPM_SUBLAYER0::providerKey is a caller-owned pointer like the filter and callout cases, and the new provider reference check dereferences it, so it is now deep-copied into the stored entry as those already are. Also drops a stray second fault injection in FwpmProviderDeleteByKey0 that returned STATUS_NOT_FOUND after the removal had already happened. --- src/fwp_um.cpp | 23 +++--- src/fwp_um.h | 190 ++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 181 insertions(+), 32 deletions(-) diff --git a/src/fwp_um.cpp b/src/fwp_um.cpp index 6fd961b..de63e11 100644 --- a/src/fwp_um.cpp +++ b/src/fwp_um.cpp @@ -809,10 +809,7 @@ _IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS FwpmCalloutDeleteByKey0(_In_ HANDLE auto& engine = *reinterpret_cast(engine_handle); - if (!engine.remove_fwpm_callout(key)) { - return (NTSTATUS)FWP_E_CALLOUT_NOT_FOUND; - } - return STATUS_SUCCESS; + return engine.delete_fwpm_callout(key); } _IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS FwpmEngineOpen0( @@ -844,7 +841,11 @@ _IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS auto& engine = *reinterpret_cast(engine_handle); - engine.add_fwpm_provider(provider); + if (!engine.add_fwpm_provider(provider)) { + // A provider with this key already exists. Real WFP reports this rather than silently accepting the add, + // and it is how a caller discovers that a previous instance's provider outlived it. + return (NTSTATUS)FWP_E_ALREADY_EXISTS; + } UNREFERENCED_PARAMETER(sd); return STATUS_SUCCESS; @@ -858,12 +859,7 @@ _IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS FwpmProviderDeleteByKey0(_In_ HANDLE auto& engine = *reinterpret_cast(engine_handle); - engine.remove_fwpm_provider(key); - if (cxplat_fault_injection_inject_fault()) { - return STATUS_NOT_FOUND; - } - - return STATUS_SUCCESS; + return engine.delete_fwpm_provider(key); } _IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS @@ -890,10 +886,7 @@ _IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS auto& engine = *reinterpret_cast(engine_handle); - if (!engine.remove_fwpm_sub_layer(sub_layer_key)) { - return (NTSTATUS)FWP_E_SUBLAYER_NOT_FOUND; - } - return STATUS_SUCCESS; + return engine.delete_fwpm_sub_layer(sub_layer_key); } _IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS FwpmEngineClose0(_Inout_ HANDLE engine_handle) diff --git a/src/fwp_um.h b/src/fwp_um.h index 5587fea..52df6f6 100644 --- a/src/fwp_um.h +++ b/src/fwp_um.h @@ -33,6 +33,15 @@ typedef struct _fwpm_callout_entry GUID provider_key; } fwpm_callout_entry_t; +// A WFP sub-layer as stored by the mock engine. FWPM_SUBLAYER0::providerKey has the same caller-owned-pointer +// problem as the filter and callout cases, and is deep-copied for the same reason: the reference checks that back +// FWP_E_IN_USE dereference it. +typedef struct _fwpm_sub_layer_entry +{ + FWPM_SUBLAYER0 sub_layer; + GUID provider_key; +} fwpm_sub_layer_entry_t; + // An in-progress enumeration. Real WFP enumerations are snapshots taken when the enum handle is created, so // objects deleted while an enumeration is open are still returned and objects added are not. The mock models // that explicitly, which also makes the common "enumerate everything, then delete each entry" pattern safe. @@ -81,17 +90,32 @@ typedef class fwp_engine_t return fwpm_callouts.erase(id) == 1; } - bool - remove_fwpm_callout(_In_ const GUID* key) + _Requires_lock_not_held_(this->lock) NTSTATUS delete_fwpm_callout(_In_ const GUID* key) { exclusive_lock_t l(lock); + + // Report a missing object as not-found even if some filter still carries the key: an object that does not + // exist cannot be in use. + size_t id = 0; + bool found = false; for (auto& [first, entry] : fwpm_callouts) { if (memcmp(&entry.callout.calloutKey, key, sizeof(GUID)) == 0) { - return fwpm_callouts.erase(first) == 1; + id = first; + found = true; + break; } } - return false; + if (!found) { + return (NTSTATUS)FWP_E_CALLOUT_NOT_FOUND; + } + + if (is_callout_referenced_under_lock(key)) { + return (NTSTATUS)FWP_E_IN_USE; + } + + fwpm_callouts.erase(id); + return STATUS_SUCCESS; } uint32_t @@ -382,23 +406,65 @@ typedef class fwp_engine_t } } - _Requires_lock_not_held_(this->lock) void add_fwpm_provider(_In_ const FWPM_PROVIDER* provider) + // Adds a provider, rejecting a duplicate as real WFP does. Object identity matters here: a provider that + // outlives the driver that created it is what makes a subsequent FwpmProviderAdd fail, so a mock that always + // accepts the add cannot reproduce that class of bug. + // + // Unlike FWPM_FILTER0/FWPM_CALLOUT0, FWPM_PROVIDER0::providerKey is a GUID by value, so the stored copy owns + // its own key and needs no re-binding. The remaining pointer members (displayData strings, serviceName, + // providerData) are still shallow copies of caller memory, which is safe only because nothing reads them: + // deep-copy them before adding any accessor or enumerator that hands a stored provider back to a caller. + _Requires_lock_not_held_(this->lock) bool add_fwpm_provider(_In_ const FWPM_PROVIDER* provider) { - UNREFERENCED_PARAMETER(provider); - return; + exclusive_lock_t l(lock); + if (get_fwpm_provider_under_lock(&provider->providerKey) != nullptr) { + return false; + } + + fwpm_providers.insert({next_id++, *provider}); + return true; } - _Requires_lock_not_held_(this->lock) void remove_fwpm_provider(_In_ const GUID* key) + _Requires_lock_not_held_(this->lock) NTSTATUS delete_fwpm_provider(_In_ const GUID* key) { - UNREFERENCED_PARAMETER(key); - return; + exclusive_lock_t l(lock); + + size_t id = 0; + bool found = false; + for (auto& [first, provider] : fwpm_providers) { + if (memcmp(&provider.providerKey, key, sizeof(GUID)) == 0) { + id = first; + found = true; + break; + } + } + + if (!found) { + return (NTSTATUS)FWP_E_PROVIDER_NOT_FOUND; + } + + if (is_provider_referenced_under_lock(key)) { + return (NTSTATUS)FWP_E_IN_USE; + } + + fwpm_providers.erase(id); + return STATUS_SUCCESS; } _Requires_lock_not_held_(this->lock) uint32_t add_fwpm_sub_layer(_In_ const FWPM_SUBLAYER0* sub_layer) { exclusive_lock_t l(lock); uint32_t id = next_id++; - fwpm_sub_layers.insert({id, *sub_layer}); + auto& stored = fwpm_sub_layers.insert({id, fwpm_sub_layer_entry_t{*sub_layer, {}}}).first->second; + + // Re-point the stored sub-layer at the entry's own copy of the provider key (see fwpm_sub_layer_entry_t). + if (sub_layer->providerKey != nullptr) { + stored.provider_key = *sub_layer->providerKey; + stored.sub_layer.providerKey = &stored.provider_key; + } else { + stored.sub_layer.providerKey = nullptr; + } + return id; } @@ -408,16 +474,30 @@ typedef class fwp_engine_t return fwpm_sub_layers.erase(id) == 1; } - _Requires_lock_not_held_(this->lock) bool remove_fwpm_sub_layer(_In_ const GUID* key) + _Requires_lock_not_held_(this->lock) NTSTATUS delete_fwpm_sub_layer(_In_ const GUID* key) { exclusive_lock_t l(lock); - for (auto& [first, sub_layer] : fwpm_sub_layers) { - if (memcmp(&sub_layer.subLayerKey, key, sizeof(GUID)) == 0) { - return fwpm_sub_layers.erase(first) == 1; + + size_t id = 0; + bool found = false; + for (auto& [first, entry] : fwpm_sub_layers) { + if (memcmp(&entry.sub_layer.subLayerKey, key, sizeof(GUID)) == 0) { + id = first; + found = true; + break; } } - return false; + if (!found) { + return (NTSTATUS)FWP_E_SUBLAYER_NOT_FOUND; + } + + if (is_sub_layer_referenced_under_lock(key)) { + return (NTSTATUS)FWP_E_IN_USE; + } + + fwpm_sub_layers.erase(id); + return STATUS_SUCCESS; } FWP_ACTION_TYPE @@ -544,6 +624,81 @@ typedef class fwp_engine_t return nullptr; } + // Reference checks backing FWP_E_IN_USE. Real WFP refuses to delete an object that another object still + // points at, which is the mechanism by which a filter that could not be deleted keeps its callout, sub-layer + // and provider alive across a driver unload. Without this the mock would happily delete a referenced object + // and no test could observe that failure mode. + static bool + is_callout_action(FWP_ACTION_TYPE action_type) + { + return action_type == FWP_ACTION_CALLOUT_TERMINATING || action_type == FWP_ACTION_CALLOUT_INSPECTION || + action_type == FWP_ACTION_CALLOUT_UNKNOWN; + } + + bool + is_callout_referenced_under_lock(_In_ const GUID* callout_key) + { + for (auto& [first, entry] : fwpm_filters) { + // FWPM_ACTION0::calloutKey shares a union with filterType, so it only holds a callout key when the + // action is a callout action. Comparing it for any other action type would match unrelated bytes and + // report a spurious FWP_E_IN_USE. + if (!is_callout_action(entry.filter.action.type)) { + continue; + } + if (memcmp(&entry.filter.action.calloutKey, callout_key, sizeof(GUID)) == 0) { + return true; + } + } + return false; + } + + bool + is_sub_layer_referenced_under_lock(_In_ const GUID* sub_layer_key) + { + for (auto& [first, entry] : fwpm_filters) { + if (memcmp(&entry.filter.subLayerKey, sub_layer_key, sizeof(GUID)) == 0) { + return true; + } + } + return false; + } + + // A provider is referenced by any filter, callout or sub-layer tagged with it. + bool + is_provider_referenced_under_lock(_In_ const GUID* provider_key) + { + for (auto& [first, entry] : fwpm_filters) { + if (entry.filter.providerKey != nullptr && + memcmp(entry.filter.providerKey, provider_key, sizeof(GUID)) == 0) { + return true; + } + } + for (auto& [first, entry] : fwpm_callouts) { + if (entry.callout.providerKey != nullptr && + memcmp(entry.callout.providerKey, provider_key, sizeof(GUID)) == 0) { + return true; + } + } + for (auto& [first, entry] : fwpm_sub_layers) { + if (entry.sub_layer.providerKey != nullptr && + memcmp(entry.sub_layer.providerKey, provider_key, sizeof(GUID)) == 0) { + return true; + } + } + return false; + } + + _Ret_maybenull_ const FWPM_PROVIDER* + get_fwpm_provider_under_lock(_In_ const GUID* provider_key) + { + for (auto& [first, provider] : fwpm_providers) { + if (memcmp(&provider.providerKey, provider_key, sizeof(GUID)) == 0) { + return &provider; + } + } + return nullptr; + } + _Ret_maybenull_ const GUID* get_callout_key_from_layer_guid_under_lock(_In_ const GUID* layer_guid) { @@ -589,7 +744,8 @@ typedef class fwp_engine_t std::unordered_map fwpm_filters; std::unordered_map> fwpm_filter_enums; std::unordered_map> fwpm_callout_enums; - std::unordered_map fwpm_sub_layers; + std::unordered_map fwpm_sub_layers; + std::unordered_map fwpm_providers; std::unordered_map fwpm_flow_contexts; GUID _default_sublayer = {}; GUID _connect_v4_sublayer = {}; From 6213ba2a20851882d3c1f1c6ccd2650edfb7a010 Mon Sep 17 00:00:00 2001 From: Michael Agun Date: Thu, 13 Aug 2026 23:01:16 -0700 Subject: [PATCH 3/3] Fix WFP mock object identity and callout unregister status FWPM_FILTER0::filterId was never populated when a filter was added, so every filter returned by FwpmFilterEnum carried an id of zero. A caller that did not add the filter itself -- the only kind of caller enumeration exists for -- had no usable handle: FwpmFilterDeleteById reported FWP_E_FILTER_NOT_FOUND, and a caller that treats "already gone" as success silently deleted nothing. Real WFP assigns the id on add and reports it through enumeration. FwpsCalloutUnregisterById0 returned STATUS_INVALID_PARAMETER for an identifier that matches no registered callout, where real WFP returns STATUS_FWP_CALLOUT_NOT_FOUND. Cleanup paths unregister every callout they may have registered and treat "not found" as success, so the generic status turned a benign no-op into an apparent failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5196c1b8-883f-440e-8846-e5fd79db2f5d --- src/fwp_um.cpp | 5 ++++- src/fwp_um.h | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/fwp_um.cpp b/src/fwp_um.cpp index de63e11..0f11700 100644 --- a/src/fwp_um.cpp +++ b/src/fwp_um.cpp @@ -945,7 +945,10 @@ _IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS FwpsCalloutUnregisterById0(_In_ cons if (engine.remove_fwps_callout(callout_id)) { return STATUS_SUCCESS; } else { - return STATUS_INVALID_PARAMETER; + // Real WFP reports an unregistered run-time identifier as FWP_E_CALLOUT_NOT_FOUND. Callers unregister + // callouts on cleanup paths where some were never registered, and they treat "not found" as success, so a + // generic status here would turn a benign no-op into an apparent failure. + return (NTSTATUS)FWP_E_CALLOUT_NOT_FOUND; } } diff --git a/src/fwp_um.h b/src/fwp_um.h index 52df6f6..5347af6 100644 --- a/src/fwp_um.h +++ b/src/fwp_um.h @@ -201,6 +201,11 @@ typedef class fwp_engine_t id = next_id++; auto& stored = fwpm_filters.insert({id, fwpm_filter_entry_t{*filter, {}}}).first->second; + // Record the assigned run-time identifier in the stored filter. Real WFP populates FWPM_FILTER0::filterId + // on add and reports it through enumeration, and it is the only handle a caller that did not add the + // filter itself has for deleting it. + stored.filter.filterId = id; + // Re-point the stored filter at the entry's own copy of the provider key (see fwpm_filter_entry_t). if (filter->providerKey != nullptr) { stored.provider_key = *filter->providerKey;