Skip to content

Implement sock_addr bind hook usermode tests#5300

Draft
mikeagun wants to merge 5 commits into
microsoft:mainfrom
mikeagun:align-bind-hook-333-tests
Draft

Implement sock_addr bind hook usermode tests#5300
mikeagun wants to merge 5 commits into
microsoft:mainfrom
mikeagun:align-bind-hook-333-tests

Conversation

@mikeagun

@mikeagun mikeagun commented May 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Follow-up to #333
Closes #5327

Adds usermode unit tests and fuzzer coverage for the sock_addr-aligned bind hook
(BPF_CGROUP_INET4/6_BIND) introduced in PR #5272.

Depends on:

Changes:

  • usersim bump to pick up usersim_fwp_bind_ipv6() and fixed multi-callout-per-layer
    dispatch in test_callout()
  • netebpfext_unit tests (6 test cases): verdict dispatch, IPv4/IPv6 context population,
    bpf_sock_addr_get_network_context at bind layer, set_redirect_context rejection
  • Fuzzer coverage: test_cgroup_inet4_bind / test_cgroup_inet6_bind in
    netebpfext_fuzzer

Testing

This PR adds usersim tests for the bind hook.

If new tests were added:

  • Unit tests are added.
  • Driver tests are added.

Documentation

N/A

Installation

N/A

@mikeagun mikeagun mentioned this pull request May 20, 2026
Merged
2 tasks
@mikeagun
mikeagun force-pushed the align-bind-hook-333-tests branch 9 times, most recently from 18342c0 to 6b6f416 Compare May 28, 2026 01:40
@mikeagun mikeagun mentioned this pull request May 29, 2026
Open
mikeagun pushed a commit to mikeagun/ebpf-for-windows that referenced this pull request Jun 8, 2026
Fix listen fuzzer stack-buffer-overflow: scope hook client by program…
5f42c54 
… type

The netebpfext fuzzer's hook client attaches to ALL hook providers via the
generic EBPF_HOOK_EXTENSION_IID NPI, including providers for different program
types that share a WFP layer. With the listen hook in this PR plus the bind
hook that recently landed on main, a fuzzer iteration that selects
BPF_PROG_TYPE_CGROUP_SOCK_ADDR and invokes test_cgroup_inet4_listen would also
match the CGROUP_SOCK_ADDR bind callout (same WFP layer scenario as
ALE_RESOURCE_ASSIGNMENT_V4 vs the legacy bind callout). The same stack-buffer
-overflow class of bug applies symmetrically: the listen path's
classify dispatch could hit the bind callout's context size mismatch.

Fix: add a desired_program_type field to netebpfext_helper_base_client_context_t
and filter in _hook_client_attach_provider. The helper constructor accepts an
optional bpf_prog_type_t, auto-derives the program type GUID from the
registered program_info providers, and sets desired_program_type before
registering the hook client. This is a general solution that prevents
cross-type context mismatches for any future hooks that share a WFP layer.

This change mirrors the equivalent fix applied to the bind tests PR (microsoft#5300)
so the two PRs land with consistent listen+bind fuzzer safety regardless of
merge order.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mikeagun
mikeagun force-pushed the align-bind-hook-333-tests branch from 6b6f416 to 254961e Compare June 8, 2026 18:49
mikeagun pushed a commit to mikeagun/ebpf-for-windows that referenced this pull request Jun 8, 2026
Fix listen fuzzer stack-buffer-overflow: scope hook client by program…
3681ade 
… type

The netebpfext fuzzer's hook client attaches to ALL hook providers via the
generic EBPF_HOOK_EXTENSION_IID NPI, including providers for different program
types that share a WFP layer. With the listen hook in this PR plus the bind
hook that recently landed on main, a fuzzer iteration that selects
BPF_PROG_TYPE_CGROUP_SOCK_ADDR and invokes test_cgroup_inet4_listen would also
match the CGROUP_SOCK_ADDR bind callout (same WFP layer scenario as
ALE_RESOURCE_ASSIGNMENT_V4 vs the legacy bind callout). The same stack-buffer
-overflow class of bug applies symmetrically: the listen path's
classify dispatch could hit the bind callout's context size mismatch.

Fix: add a desired_program_type field to netebpfext_helper_base_client_context_t
and filter in _hook_client_attach_provider. The helper constructor accepts an
optional bpf_prog_type_t, auto-derives the program type GUID from the
registered program_info providers, and sets desired_program_type before
registering the hook client. This is a general solution that prevents
cross-type context mismatches for any future hooks that share a WFP layer.

This change mirrors the equivalent fix applied to the bind tests PR (microsoft#5300)
so the two PRs land with consistent listen+bind fuzzer safety regardless of
merge order.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mikeagun
mikeagun force-pushed the align-bind-hook-333-tests branch from 254961e to 32da892 Compare June 8, 2026 21:17
mikeagun pushed a commit to mikeagun/ebpf-for-windows that referenced this pull request Jun 9, 2026
Fix listen fuzzer stack-buffer-overflow: scope hook client by program…
70046dd 
… type

The netebpfext fuzzer's hook client attaches to ALL hook providers via the
generic EBPF_HOOK_EXTENSION_IID NPI, including providers for different program
types that share a WFP layer. With the listen hook in this PR plus the bind
hook that recently landed on main, a fuzzer iteration that selects
BPF_PROG_TYPE_CGROUP_SOCK_ADDR and invokes test_cgroup_inet4_listen would also
match the CGROUP_SOCK_ADDR bind callout (same WFP layer scenario as
ALE_RESOURCE_ASSIGNMENT_V4 vs the legacy bind callout). The same stack-buffer
-overflow class of bug applies symmetrically: the listen path's
classify dispatch could hit the bind callout's context size mismatch.

Fix: add a desired_program_type field to netebpfext_helper_base_client_context_t
and filter in _hook_client_attach_provider. The helper constructor accepts an
optional bpf_prog_type_t, auto-derives the program type GUID from the
registered program_info providers, and sets desired_program_type before
registering the hook client. This is a general solution that prevents
cross-type context mismatches for any future hooks that share a WFP layer.

This change mirrors the equivalent fix applied to the bind tests PR (microsoft#5300)
so the two PRs land with consistent listen+bind fuzzer safety regardless of
merge order.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mikeagun pushed a commit to mikeagun/ebpf-for-windows that referenced this pull request Jun 10, 2026
Fix listen fuzzer stack-buffer-overflow: scope hook client by program…
14791f3 
… type

The netebpfext fuzzer's hook client attaches to ALL hook providers via the
generic EBPF_HOOK_EXTENSION_IID NPI, including providers for different program
types that share a WFP layer. With the listen hook in this PR plus the bind
hook that recently landed on main, a fuzzer iteration that selects
BPF_PROG_TYPE_CGROUP_SOCK_ADDR and invokes test_cgroup_inet4_listen would also
match the CGROUP_SOCK_ADDR bind callout (same WFP layer scenario as
ALE_RESOURCE_ASSIGNMENT_V4 vs the legacy bind callout). The same stack-buffer
-overflow class of bug applies symmetrically: the listen path's
classify dispatch could hit the bind callout's context size mismatch.

Fix: add a desired_program_type field to netebpfext_helper_base_client_context_t
and filter in _hook_client_attach_provider. The helper constructor accepts an
optional bpf_prog_type_t, auto-derives the program type GUID from the
registered program_info providers, and sets desired_program_type before
registering the hook client. This is a general solution that prevents
cross-type context mismatches for any future hooks that share a WFP layer.

This change mirrors the equivalent fix applied to the bind tests PR (microsoft#5300)
so the two PRs land with consistent listen+bind fuzzer safety regardless of
merge order.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mikeagun
mikeagun force-pushed the align-bind-hook-333-tests branch from 32da892 to b03dab7 Compare June 10, 2026 00:53
@mikeagun
mikeagun marked this pull request as ready for review June 10, 2026 16:15
dthaler
dthaler reviewed Jun 10, 2026
View reviewed changes
Comment thread tests/netebpfext_unit/netebpf_ext_helper.h Outdated
Comment thread tests/netebpfext_unit/netebpfext_unit.cpp
mikeagun pushed a commit to mikeagun/ebpf-for-windows that referenced this pull request Jun 19, 2026
Make netebpf_ext_helper test helpers accept const fwp_classify_parame…
8cd6558 
…ters_t*

Update all 12 test_* helpers in netebpf_ext_helper.h to take a
const fwp_classify_parameters_t* (was non-const). Body uses const_cast
to bridge to the underlying non-const usersim_fwp_* API. This enforces
the immutability discipline at the wrapper boundary so callers cannot
accidentally rely on per-call mutation of the shared parameters object
(e.g., the sock_addr_invoke_bind test reuses one parameters object
across multiple v4/v6 calls; without const, a mutation inside one call
could silently affect the next).

Scope: all helpers in this file for in-file consistency. Touched helpers
(unchanged behavior, signature only):
  test_bind_ipv4 / test_bind_ipv6
  test_cgroup_inet4_bind / test_cgroup_inet6_bind
  test_cgroup_inet4_recv_accept / test_cgroup_inet6_recv_accept
  test_cgroup_inet4_connect / test_cgroup_inet6_connect
  test_cgroup_inet4_connect_authorization / test_cgroup_inet6_connect_authorization
  test_sock_ops_v4 / test_sock_ops_v6 (flow_id out-param remains non-const)

Addresses review comments from @dthaler on PR microsoft#5300:
- thread PRRT_kwDOFTPWZc6ImFRO (tests/netebpfext_unit/netebpf_ext_helper.h:60)
- thread PRRT_kwDOFTPWZc6ImGRk (FYI explaining v4/v6 reuse rationale)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mikeagun pushed a commit to mikeagun/ebpf-for-windows that referenced this pull request Jun 19, 2026
Fix listen fuzzer stack-buffer-overflow: scope hook client by program…
1e5589a 
… type

The netebpfext fuzzer's hook client attaches to ALL hook providers via the
generic EBPF_HOOK_EXTENSION_IID NPI, including providers for different program
types that share a WFP layer. With the listen hook in this PR plus the bind
hook that recently landed on main, a fuzzer iteration that selects
BPF_PROG_TYPE_CGROUP_SOCK_ADDR and invokes test_cgroup_inet4_listen would also
match the CGROUP_SOCK_ADDR bind callout (same WFP layer scenario as
ALE_RESOURCE_ASSIGNMENT_V4 vs the legacy bind callout). The same stack-buffer
-overflow class of bug applies symmetrically: the listen path's
classify dispatch could hit the bind callout's context size mismatch.

Fix: add a desired_program_type field to netebpfext_helper_base_client_context_t
and filter in _hook_client_attach_provider. The helper constructor accepts an
optional bpf_prog_type_t, auto-derives the program type GUID from the
registered program_info providers, and sets desired_program_type before
registering the hook client. This is a general solution that prevents
cross-type context mismatches for any future hooks that share a WFP layer.

This change mirrors the equivalent fix applied to the bind tests PR (microsoft#5300)
so the two PRs land with consistent listen+bind fuzzer safety regardless of
merge order.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
shankarseal
shankarseal reviewed Jun 19, 2026
View reviewed changes
Comment thread tests/netebpfext_unit/netebpf_ext_helper.cpp Outdated
shankarseal
shankarseal reviewed Jun 19, 2026
View reviewed changes
Comment thread tests/netebpfext_unit/netebpf_ext_helper.cpp Outdated
@Alan-Jowett

Alan-Jowett commented Jul 1, 2026

Copy link
Copy Markdown
Member

Please resolve the merge conflicts and comments. Will review after that.

Alan-Jowett
Alan-Jowett reviewed Jul 7, 2026
View reviewed changes
Comment thread external/usersim Outdated
Michael Agun and others added 2 commits July 13, 2026 15:27
Add usermode tests and fuzzer coverage for sock_addr bind hook
045a5b3 
Adds netebpfext_unit tests (6 test cases) and fuzzer coverage for the
CGROUP_INET4/6_BIND sock_addr-aligned bind hook.

Requires usersim bump for usersim_fwp_bind_ipv6() API and fixed
test_callout() dispatch by filter calloutKey.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Make netebpf_ext_helper test helpers accept const fwp_classify_parame…
42ccd0a 
…ters_t*

Update all 12 test_* helpers in netebpf_ext_helper.h to take a
const fwp_classify_parameters_t* (was non-const). Body uses const_cast
to bridge to the underlying non-const usersim_fwp_* API. This enforces
the immutability discipline at the wrapper boundary so callers cannot
accidentally rely on per-call mutation of the shared parameters object
(e.g., the sock_addr_invoke_bind test reuses one parameters object
across multiple v4/v6 calls; without const, a mutation inside one call
could silently affect the next).

Scope: all helpers in this file for in-file consistency. Touched helpers
(unchanged behavior, signature only):
  test_bind_ipv4 / test_bind_ipv6
  test_cgroup_inet4_bind / test_cgroup_inet6_bind
  test_cgroup_inet4_recv_accept / test_cgroup_inet6_recv_accept
  test_cgroup_inet4_connect / test_cgroup_inet6_connect
  test_cgroup_inet4_connect_authorization / test_cgroup_inet6_connect_authorization
  test_sock_ops_v4 / test_sock_ops_v6 (flow_id out-param remains non-const)

Addresses review comments from @dthaler on PR microsoft#5300:
- thread PRRT_kwDOFTPWZc6ImFRO (tests/netebpfext_unit/netebpf_ext_helper.h:60)
- thread PRRT_kwDOFTPWZc6ImGRk (FYI explaining v4/v6 reuse rationale)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mikeagun
mikeagun force-pushed the align-bind-hook-333-tests branch from 8cd6558 to 42ccd0a Compare July 14, 2026 02:42
Alan-Jowett
Alan-Jowett reviewed Jul 15, 2026
View reviewed changes
Comment thread tests/netebpfext_unit/netebpf_ext_helper.h Outdated
// (FWPM_LAYER_ALE_RESOURCE_ASSIGNMENT_V4). usersim's test_callout uses the filter's bound
// calloutKey to dispatch to the correct callout, so this routes to whichever bind hook
// currently has an attached client (legacy or CGROUP_SOCK_ADDR).
return usersim_fwp_bind_ipv4(const_cast<fwp_classify_parameters_t*>(parameters));

@Alan-Jowett Alan-Jowett Jul 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this helper is still ambiguous and does not prove that the CGROUP_SOCK_ADDR bind callout is what runs here.

Provenance

  • This helper calls usersim_fwp_bind_ipv4() here.
  • usersim_fwp_bind_ipv4() goes through fwp_engine_t::test_bind_ipv4(), which invokes test_callout(FWPS_LAYER_ALE_RESOURCE_ASSIGNMENT_V4, FWPM_LAYER_ALE_RESOURCE_ASSIGNMENT_V4, _default_sublayer, ...) (external/usersim/src/fwp_um.cpp:133-151).
  • usersim then selects a filter by only (layerKey, subLayerKey) in get_fwpm_filter_with_context_under_lock() (external/usersim/src/fwp_um.h:290-298).
  • Both the legacy bind filter (netebpfext/net_ebpf_ext_bind.c:22-32) and the new sock_addr bind filter (netebpfext/net_ebpf_ext_sock_addr.c:605-617) register on FWPM_LAYER_ALE_RESOURCE_ASSIGNMENT_V4 with the default sublayer.

Because both filters share the same (layer, sublayer) key, usersim_fwp_bind_ipv4() does not uniquely select net_ebpf_extension_sock_addr_bind_classify(). That means test_cgroup_inet4_bind() can pass while still exercising the legacy bind path instead of the new sock_addr bind path. Could we add a dedicated usersim entry point, or otherwise select the sock_addr bind callout explicitly?

@mikeagun mikeagun Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current code it will dispatch correctly because the tests only attach the relevant callouts, but that is a good point that there is a latent bug that could trigger selection of the wrong classify function with future code changes.

I'll create a usersim PR to enable explicit selection of the correct callout.

Alan-Jowett
Alan-Jowett previously approved these changes Jul 15, 2026
View reviewed changes
Select the sock_addr bind callout explicitly in netebpfext bind tests
a53f989 
The netebpfext bind test helpers routed through usersim_fwp_bind_ipv4() /
usersim_fwp_bind_ipv6(), which select a WFP filter by (layer, sublayer)
first-match. The legacy bind hook and the CGROUP_SOCK_ADDR bind hook both
register filters at ALE_RESOURCE_ASSIGNMENT with the default sublayer, so
when both are registered that first-match could dispatch to the wrong
callout -- a sock_addr bind test could pass while exercising the legacy bind
path.

Route the bind helpers through the new usersim_fwp_bind_ipv4/ipv6_by_callout()
entry points, which select the filter by its bound callout key:
test_cgroup_inet4/6_bind() target the CGROUP_SOCK_ADDR bind callout and
test_bind_ipv4/ipv6() target the legacy bind callout.

Add sock_addr_bind_callout_disambiguation, which co-registers both bind hooks
and verifies each bind entry dispatches to its intended callout.

Addresses review feedback from @Alan-Jowett on microsoft#5300.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c2ba0624-5e1d-4385-9972-eec1b8c595f1
@mikeagun
mikeagun marked this pull request as draft July 15, 2026 21:50
@mikeagun mikeagun mentioned this pull request Jul 15, 2026
Draft
Michael Agun and others added 2 commits July 15, 2026 15:50
Bump external/usersim for bind classify by callout key
8d251bb 
Point external/usersim at the branch adding usersim_fwp_bind_ipv4/ipv6_by_callout(),
used by the netebpfext bind tests to select a bind callout explicitly.

Temporary: this points at the fork branch pending the usersim change landing
upstream, after which external/usersim will be re-pointed to usersim main.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c2ba0624-5e1d-4385-9972-eec1b8c595f1
Temporarily point external/usersim submodule URL at the fork
a01968f 
usersim_fwp_bind_ipv4/ipv6_by_callout() is not yet in usersim upstream. Point
the external/usersim submodule URL at the mikeagun/usersim fork so CI can
resolve the submodule commit while microsoft/usersim#315 is in review. Revert
to https://github.com/microsoft/usersim.git once microsoft#315 lands.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c2ba0624-5e1d-4385-9972-eec1b8c595f1
@mikeagun
mikeagun force-pushed the align-bind-hook-333-tests branch from 91c73a7 to a01968f Compare July 15, 2026 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dthaler dthaler dthaler left review comments
@Alan-Jowett Alan-Jowett Alan-Jowett left review comments
@LakshK98 LakshK98 Awaiting requested review from LakshK98 LakshK98 is a code owner
@matthewige matthewige Awaiting requested review from matthewige matthewige is a code owner
@mtfriesen mtfriesen Awaiting requested review from mtfriesen mtfriesen is a code owner
@poornagmsft poornagmsft Awaiting requested review from poornagmsft poornagmsft is a code owner
@saxena-anurag saxena-anurag Awaiting requested review from saxena-anurag saxena-anurag is a code owner
@shankarseal shankarseal Awaiting requested review from shankarseal shankarseal is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees

@saxena-anurag saxena-anurag

@shankarseal shankarseal

Labels

None yet

Projects

eBPF for Windows Triage
Status: Todo

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add usersim based tests for new cgroup bind hook

5 participants

@mikeagun @Alan-Jowett @dthaler @shankarseal @saxena-anurag