Skip to content

Fix test_callout to dispatch via filter's bound calloutKey + add bind_ipv6 helper#307

Open
mikeagun wants to merge 1 commit into
microsoft:mainfrom
mikeagun:fix-multi-callout-per-layer
Open

Fix test_callout to dispatch via filter's bound calloutKey + add bind_ipv6 helper#307
mikeagun wants to merge 1 commit into
microsoft:mainfrom
mikeagun:fix-multi-callout-per-layer

Conversation

@mikeagun
Copy link
Copy Markdown

Description

Add support for multiple callouts per layer, add IPv6 bind callout, and populate additional bind callout fields.

Problem

fwp_engine_t::test_callout() looks up the callout by layer alone via get_callout_key_from_layer_guid_under_lock(), which returns the first fwpm_callouts entry matching the layer.

  • Real WFP dispatches each filter to the specific callout it's bound to via action.calloutKey. usersim already records that binding during add_fwpm_filter (see src/fwp_um.h:140) — test_callout just discards it.
  • The behavior is non-deterministic when more than one callout is registered at the same WFP layer, which is a normal pattern in real WFP and blocks tests for any project that does it.

Fix

Look up the callout via the filter's bound action.calloutKey instead of scanning by layer:

  -        const GUID* callout_key = get_callout_key_from_layer_guid_under_lock(&layer_guid);
  -        if (callout_key == nullptr) {
  -            return FWP_ACTION_CALLOUT_UNKNOWN;
  -        }
  -        callout = get_callout_from_key_under_lock(callout_key);
  +        callout = get_callout_from_key_under_lock(&fwpm_filter->action.calloutKey);

For tests with one callout per layer (today's status quo) the behavior is identical. For tests with multiple callouts per layer it now matches real WFP.

Additional changes (related, small).

  • Add usersim_fwp_bind_ipv6() mirroring the existing usersim_fwp_bind_ipv4().
  • In test_bind_ipv4 / test_bind_ipv6, populate COMPARTMENT_ID, IP_LOCAL_INTERFACE, and ALE_USER_ID incoming values. These are read by sock_addr-style bind callbacks but ignored by the legacy bind callback, so the addition is non-breaking.

Motivation. This surfaced while adding a BPF_PROG_TYPE_CGROUP_SOCK_ADDR bind hook (BPF_CGROUP_INET4/6_BIND) to eBPF for Windows, which coexists with the existing legacy bind hook at FWPM_LAYER_ALE_RESOURCE_ASSIGNMENT_V4/V6. Without this fix, unit tests can't reliably target one callout vs. the other when both are registered.

usersim's test_callout() was looking up the callout by layer alone
(get_callout_key_from_layer_guid_under_lock), which returned the first
callout registered at that layer. This was incorrect: each WFP filter is
bound to a specific calloutKey, and real WFP dispatches to that specific
callout. The previous behavior was non-deterministic when multiple
callouts shared a WFP layer.

Use fwpm_filter->action.calloutKey instead — it's already captured during
add_fwpm_filter(). This matches real WFP behavior and enables testing of
multi-callout-per-layer scenarios (e.g., legacy bind hook coexisting with
sock_addr-aligned bind hook at FWPM_LAYER_ALE_RESOURCE_ASSIGNMENT_V4).

Also add usersim_fwp_bind_ipv6() and populate compartment_id /
interface_luid / user_id in the bind classify parameters (fields that
the legacy bind callback ignores but are needed for sock_addr bind
context population).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant