Normalize and correct sock_addr verdict handling

[D. Michael Agun (mikeagun)]

Description

Normalize verdict and WFP action handling across all sock_addr classify callbacks.

• Initialize every callback with BPF_SOCK_ADDR_VERDICT_PROCEED_SOFT.
• Skip eBPF program invocation when FWPS_RIGHT_ACTION_WRITE is absent, preserving the existing WFP action rather than issuing a veto.
• Use the soft-permit default for exits that occur before eBPF program invocation, including filter-context validation and classify-handle acquisition failures.
• Normalize program invocation results:
  • EBPF_SUCCESS uses the returned or accumulated program verdict.
  • EBPF_OBJECT_NOT_FOUND preserves the current verdict.
  • Other invocation failures produce BPF_SOCK_ADDR_VERDICT_REJECT.
• Centralize write-right checks, active filter-context validation, and program-result handling.
• Keep classify callback bodies verdict-only and translate verdicts to WFP actions from their exit paths:
  • PROCEED_SOFT returns FWP_ACTION_PERMIT while retaining the write right.
  • PROCEED_HARD returns FWP_ACTION_PERMIT and clears the write right.
  • Reject and unknown verdicts return FWP_ACTION_BLOCK and clear the write right.
• Ensure listen and receive-accept invocation failures produce a non-overridable block.
• Preserve connect-authorization cache cleanup when write rights are absent and preserve cached hard/reject verdicts when no authorization program matches.
• Preserve connect-redirect's specialized behavior: ordinary soft decisions return CONTINUE, while redirects and cached rejections return terminating PERMIT.
• Only process the connect-redirect verdict cache after the socket context has been initialized, avoiding invalid zero-context cache operations on early exits.
• bug fix: restore connect after a reject in connect hook to preserve reject decision -- previously after a destination rewrite+reject the connection would be allowed (the cached decision would have the unapplied redirect destination).

Closes #5544.

Testing

• Debug x64 drivers\netebpfext build.
• Debug x64 tests\netebpfext_unit build.
• netebpfext_unit: 494 assertions in 25 test cases.

[Copilot]

🔵 Needs a closer look

It changes kernel networking classification logic (verdict/action/right handling) and warrants careful human validation despite only minor nits found.

Pull request overview

This PR normalizes sock_addr WFP classify callback behavior in netebpfext by standardizing default verdicts, write-right handling, and program-invocation result mapping, then translating final verdicts to WFP actions in a consistent exit path.

Changes:

• Introduces shared helpers to centralize FWPS_RIGHT_ACTION_WRITE checks, active filter-context validation, and program-result→verdict normalization.
• Updates sock_addr classify callbacks to default to BPF_SOCK_ADDR_VERDICT_PROCEED_SOFT, skip program invocation without write rights, and apply verdicts via common exit logic.
• Refactors CONNECT_REDIRECT to apply action decisions via a dedicated helper and prevents verdict-cache operations before the socket context is initialized.

File summaries

File	Description
netebpfext/net_ebpf_ext_sock_addr.c	Centralizes sock_addr verdict/action handling and normalizes classification behavior across callbacks.

Review details

Suppressed comments (2)

netebpfext/net_ebpf_ext_sock_addr.c:2541

• incoming_metadata_values is used later (e.g., transportEndpointHandle and field copy), so UNREFERENCED_PARAMETER(incoming_metadata_values) is inaccurate and should be removed to avoid confusion.

    UNREFERENCED_PARAMETER(incoming_metadata_values);

netebpfext/net_ebpf_ext_sock_addr.c:2353

• incoming_metadata_values is referenced later (e.g., used for transportEndpointHandle logging and passed into _net_ebpf_extension_sock_addr_copy_wfp_connection_fields), so UNREFERENCED_PARAMETER(incoming_metadata_values) is inaccurate and should be removed to avoid confusion.

    UNREFERENCED_PARAMETER(incoming_metadata_values);

• Files reviewed: 1/1 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Moderate regression-test gaps remain for write-right handling and fail-closed listen errors.

Review details

Suppressed comments (2)

netebpfext/net_ebpf_ext_sock_addr.c:30

• These new gates implement the PR's key FWPS_RIGHT_ACTION_WRITE behavior, but the existing netebpfext_unit coverage never supplies a classify output with the write right revoked (the tests explicitly note that this state cannot be inspected). There is therefore no regression test that dispatch is skipped and the pre-existing action is preserved, especially for connect-authorization cache cleanup; add a simulator or integration case covering both outcomes.

#define BAIL_ON_NO_WRITE_RIGHT(classify_output, action_write_allowed)                                                  \
    do {                                                                                                               \
        (action_write_allowed) = (((classify_output)->rights & FWPS_RIGHT_ACTION_WRITE) != 0);                         \
        if (!(action_write_allowed)) {                                                                                 \
            EBPF_EXT_LOG_MESSAGE(                                                                                      \

netebpfext/net_ebpf_ext_sock_addr.c:2280

• The new failure mapping for the listen callback is not covered by the existing sock_addr tests: sock_addr_listen_invoke exercises soft permit, reject, and hard permit, but unlike sock_addr_invoke it never sets SOCK_ADDR_TEST_ACTION_FAILURE. Add IPv4/IPv6 assertions that a listen program invocation failure becomes a block with the write right revoked, so this required fail-closed behavior cannot regress.

    program_result = net_ebpf_extension_hook_expand_stack_and_invoke_programs(
        sock_addr_ctx, &filter_context->base, &program_verdict);
    verdict = _net_ebpf_extension_sock_addr_get_program_verdict(program_result, program_verdict, verdict);

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Moderate findings require coverage for write-right handling and listen invocation failures.

Review details

Suppressed comments (2)

netebpfext/net_ebpf_ext_sock_addr.c:32

• This new gate implements the PR's key FWPS_RIGHT_ACTION_WRITE contract, but the existing sock_addr tests never drive a classify output with the write right revoked or verify that the eBPF callback is skipped and the prior action is preserved; the test suite even notes that this state is not asserted directly at tests/netebpfext_unit/netebpfext_unit.cpp:287-293. Add a usersim/integration case (including connect-authorization cache cleanup) so this central behavior cannot regress.

#define BAIL_ON_NO_WRITE_RIGHT(classify_output, action_write_allowed)                          \
    do {                                                                                       \
        (action_write_allowed) = (((classify_output)->rights & FWPS_RIGHT_ACTION_WRITE) != 0); \
        if (!(action_write_allowed)) {                                                         \
            EBPF_EXT_LOG_MESSAGE(                                                              \

netebpfext/net_ebpf_ext_sock_addr.c:2309

• The new fail-closed mapping for listen invocation errors is not covered by the existing sock_addr_listen_invoke test: it exercises soft permit, reject, and hard permit, but never SOCK_ADDR_TEST_ACTION_FAILURE. Add IPv4 and IPv6 assertions that an invocation failure reaches this path and produces a block with the write right revoked.

    program_result = net_ebpf_extension_hook_expand_stack_and_invoke_programs(
        sock_addr_ctx, &filter_context->base, &program_verdict);
    NET_EBPF_EXT_LOG_SOCK_ADDR_PROGRAM_INVOCATION_FAILURE(program_result);
    effective_verdict =
        _net_ebpf_extension_sock_addr_get_effective_verdict(program_result, program_verdict, effective_verdict);

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The changes affect verdict and WFP action behavior across multiple classify paths, warranting final human review.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite