Add CONNECT_AUTHORIZATION readonly context test#5230
Merged
saxena-anurag merged 5 commits intoMay 15, 2026
Merged
Conversation
Alan-Jowett
requested review from
LakshK98,
dthaler,
matthewige,
mikeagun,
mtfriesen,
poornagmsft,
saxena-anurag and
shankarseal
as code owners
Alan-Jowett
force-pushed
the
issue-5213-connect-auth-readonly-test
branch
from
9d79e79 to
bca9d04
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a regression test for CONNECT_AUTHORIZATION read-only context enforcement by introducing a new sample eBPF program and wiring a socket test to exercise it across connection variants.
Changes:
- Added a new sample
cgroup_connect_authorization_readonly.cprogram that mutatesctx->user_portinconnect_authorization. - Added a new templated socket test case intended to validate that this context mutation is rejected for IPv4/IPv6 and TCP/UDP.
- Registered the new sample source in the sample project filters.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
tests/socket/socket_tests.cpp |
Adds the new socket regression test case that loads and attaches the readonly-context sample program. |
tests/sample/sample.vcxproj.filters |
Places the new sample source under the Visual Studio project’s source-file filter. |
tests/sample/cgroup_connect_authorization_readonly.c |
Adds the eBPF sample that mutates ctx->user_port from connect_authorization4/6. |
Alan-Jowett
force-pushed
the
issue-5213-connect-auth-readonly-test
branch
from
06f5f62 to
691a1a9
Compare
Add a socket test that covers the netebpfext path which rejects CONNECT_AUTHORIZATION programs that mutate read-only sock_addr context. Introduce a dedicated sample program that modifies the destination port while returning a soft permit, and add it to the sample project filters. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Make the readonly CONNECT_AUTHORIZATION regression test explicitly assert a blocked connection instead of relying on the default expected_result. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Alan-Jowett
force-pushed
the
issue-5213-connect-auth-readonly-test
branch
from
691a1a9 to
7e797db
Compare
mikeagun
previously approved these changes
May 13, 2026
Contributor
|
@Alan-Jowett this was discussed in Monday triage meeting that the extension should ignore modification of any read only fields in ctx (unless the modification is done to the ctx data). This behavior already exists for |
saxena-anurag
requested changes
May 14, 2026
…TION Instead of overriding the verdict to REJECT when a CONNECT_AUTHORIZATION program modifies destination IP/port, restore the original context and proceed with the program's actual verdict. This matches Linux behavior where writes to read-only context fields are silently ignored. Update the readonly context test to expect the connection to succeed (allow) instead of being blocked. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolve conflict in net_ebpf_ext_sock_addr.c: keep the silent-ignore behavior for CONNECT_AUTHORIZATION read-only context mutations and adopt upstream's renamed logging macros (NET_EBPF_EXT_ -> EBPF_EXT_). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
Requested changes have been made. |
dthaler
reviewed
May 14, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dthaler
approved these changes
May 14, 2026
mikeagun
approved these changes
May 14, 2026
saxena-anurag
approved these changes
May 15, 2026
| @@ -1667,21 +1668,27 @@ _net_ebpf_extension_sock_addr_process_verdict(_Inout_ void* program_context, int | |||
| context->redirected = redirected; | |||
| context->address_changed = address_changed; | |||
Contributor
There was a problem hiding this comment.
[non-blocking] I think there is an existing issue with cgroup\connect hook where the extension does not "restore" the read only fields before invoking next BPF program.
Filed #5271
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add a socket-level regression test for CONNECT_AUTHORIZATION read-only context enforcement.
This introduces a dedicated sample program that mutates
ctx->user_portfrom aconnect_authorizationprogram while returningBPF_SOCK_ADDR_VERDICT_PROCEED_SOFT, plus a templated socket test that expects the connection to be blocked for IPv4/IPv6 and TCP/UDP.Fixes #5213.
Testing
Built:
msbuild /m /p:Configuration=Debug /p:Platform=x64 /p:SolutionDir=Q:\ebpf-for-windows\ tests\sample\sample.vcxprojmsbuild /m /p:Configuration=Debug /p:Platform=x64 /p:SolutionDir=Q:\ebpf-for-windows\ tests\socket\socket_tests.vcxprojAdded new socket coverage in
connection_test_connect_authorization_readonly_context.If new tests were added:
Local runtime note: in this environment,
socket_tests.exefails atbpf_object__load(obj)for both the new test and the existing baselineconnection_test_connect_authorizationcase, so the new runtime path could not be confirmed end-to-end here.Documentation
No documentation changes.
Installation
No installer impact.