Skip to content

bpf:wireguard: delivery host packets to bpf_host for ingress policies#42892

Merged
julianwiedmann merged 1 commit intomainfrom
pr/smagnani96/wg-local-redirect
Jan 9, 2026
Merged

bpf:wireguard: delivery host packets to bpf_host for ingress policies#42892
julianwiedmann merged 1 commit intomainfrom
pr/smagnani96/wg-local-redirect

Conversation

@smagnani96
Copy link
Copy Markdown
Contributor

@smagnani96 smagnani96 commented Nov 20, 2025

This effectively restores HostFw for WireGuard.
Prior to this, we were erroneously always returning to the stack all
packets destined for local host, skipping the HostFw policies if enabled.

With this patch, packets for local host will always be delivered to
cilium_net@egress, similarly to what we do in bpf_overlay after decap.
With HostFw enabled, the to-host program in cilium_host@ingress will
then enforce policies.

This patch does not affect packets for local endpoint:

  • With BPF Host Routing: will be directly delivered to the pod, tail
    calling into its ep->lxc_id function to enforce ingress policies.
  • Without BPF Host Routing: will return to stack, which then goes to its
    to-container installed program to match ingress policies.

Trying to pull-in the whole host_firewall.h and policy.h would require
to set bpf_wireguard similarly as we do for bpf_host, meaning assigning
an endpoint ID, otherwise host policies would block all host related
packets (ep id == 0). For this reason, we decide here to go through cilium_host.

From a bpf test perspective:

  • no changes for packet to/from local endpoint INGRESS/EGRESS
  • no changes for packet from local host EGRESS
  • packet to local host INGRESS: differently than bpf_host, in WireGuard
    we always redirect to cilium_host@ingress.

While fixing this bits, let's move the superfluous revalidate_data
post NodePort inside the NodePort code, as not needed otherwise.

@smagnani96 smagnani96 self-assigned this Nov 20, 2025
@smagnani96 smagnani96 added kind/bug This is a bug in the Cilium logic. area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. release-note/misc This PR makes changes that have no direct user impact. feature/wireguard Relates to Cilium's Wireguard feature labels Nov 20, 2025
@smagnani96 smagnani96 force-pushed the pr/smagnani96/wg-local-redirect branch 3 times, most recently from c41c7b9 to 2405bac Compare November 21, 2025 12:20
@smagnani96
Copy link
Copy Markdown
Contributor Author

/test

@julianwiedmann julianwiedmann self-requested a review November 21, 2025 14:36
@smagnani96 smagnani96 added the needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch label Nov 21, 2025
@smagnani96 smagnani96 marked this pull request as ready for review November 21, 2025 16:30
@smagnani96 smagnani96 requested review from a team as code owners November 21, 2025 16:30
@smagnani96 smagnani96 requested a review from rgo3 November 21, 2025 16:30
@smagnani96 smagnani96 force-pushed the pr/smagnani96/wg-local-redirect branch from 2405bac to 97aa39d Compare November 25, 2025 15:55
@smagnani96
Copy link
Copy Markdown
Contributor Author

fyi: given I'll backport this to v1.18 once finished, bpf tests have not been written in scapy-like language.

@smagnani96 smagnani96 force-pushed the pr/smagnani96/wg-local-redirect branch from 97aa39d to a624bbc Compare November 25, 2025 22:46
@smagnani96 smagnani96 changed the base branch from main to pr/smagnani96/tc-nodeport-l3-bpf November 25, 2025 22:46
Base automatically changed from pr/smagnani96/tc-nodeport-l3-bpf to main November 26, 2025 07:26
@smagnani96 smagnani96 force-pushed the pr/smagnani96/wg-local-redirect branch from a624bbc to 5bf395b Compare November 26, 2025 10:08
@julianwiedmann
Copy link
Copy Markdown
Member

For the last patch:

Given we've done a legimitate lookup, let's make use of that identity.

I believe that's not accurate - bpf_wireguard currently still respects secctx_from_ipcache, and only resolves the source identity when BPF Host-Routing is enabled.

So if we now want to always forward an identity, then we also need to unconditionally resolve it (which feels like a reasonable choice to me).

@smagnani96 smagnani96 force-pushed the pr/smagnani96/wg-local-redirect branch from 5bf395b to 6b489a1 Compare November 27, 2025 15:14
@smagnani96
Copy link
Copy Markdown
Contributor Author

smagnani96 commented Nov 27, 2025

Many many thanks Julian. Your input was essential here. It took me a while, but I think we're close now.

I believe that's not accurate - bpf_wireguard currently still respects secctx_from_ipcache, and only resolves the source identity when BPF Host-Routing is enabled.

So if we now want to always forward an identity, then we also need to unconditionally resolve it (which feels like a reasonable choice to me).

I moved the whole removal of secctx_from_ipcache and setting the identity into separate PRs.

But with Wireguard we do have the case (when BPF Host Routing is disabled) where a local pod's traffic first passes through netfilter, and only in to-netdev gets redirected to cilium_wg. And so the from-wireguard path also needs to traverse netfilter, and can't unconditionally redirect straight to the pod.

It was not easy for me, but I think now I understand it slightly better.
So I've updated PR accordingly, adding new codepaths and comments to both the code and commit. Will update PR description as well 🙏🏼

Will need to update the BPF test accordingly, doing it right now.

@smagnani96 smagnani96 force-pushed the pr/smagnani96/wg-local-redirect branch 3 times, most recently from f70bf2f to 105e10e Compare November 27, 2025 17:47
@smagnani96 smagnani96 force-pushed the pr/smagnani96/wg-local-redirect branch from 6547155 to a4ecfef Compare December 18, 2025 10:46
@smagnani96 smagnani96 requested review from a team as code owners December 18, 2025 11:42
@smagnani96 smagnani96 requested a review from brlbil December 18, 2025 11:42
@smagnani96 smagnani96 added the dont-merge/discussion A discussion is ongoing and should be resolved before merging, regardless of reviews & tests status. label Dec 18, 2025
@smagnani96
Copy link
Copy Markdown
Contributor Author

smagnani96 commented Dec 18, 2025

I tried to enable HostFirewall in CI in the unique config in which we have WireGuard+NativeRouting+NodeEncryption.
This should be the minimal test to make sure this PR restores HostFw with WireGuard (see #43374): the CLI host-firewall-ingress test already exists (--include-unsafe-tests), we just need a valid config to run it.

EDIT: branched off CI tests into a separate PR, which will be used for testing this feature. (#43450)

@smagnani96 smagnani96 force-pushed the pr/smagnani96/wg-local-redirect branch from 9227d89 to f45ba7b Compare December 19, 2025 19:26
@smagnani96 smagnani96 removed request for a team and brlbil December 19, 2025 19:26
@joestringer joestringer added release-note/bug This PR fixes an issue in a previous release of Cilium. and removed ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. labels Jan 5, 2026
@smagnani96
Copy link
Copy Markdown
Contributor Author

Cherry-picked this commit and enabled testing in wireguard-3 config #43450.
Test can't be merged, as I did a trick to only run it after upgrade, otherwise would obviously fail because w/o this HostFW is broken.
ci-l7, and in particular host-firewall-{ingress,egress} tests, successfully passed https://github.com/cilium/cilium/actions/runs/20824455539/job/59823165980#step:30:515.
Going to rebase this and re-run CI, then it can be merge IMHO.

This effectively restores HostFw for WireGuard.
Prior to this, we were erroneously always returning to the stack all
packets destined for local host, skipping the HostFw policies if enabled.

With this patch, packets for local host will always be delivered to
cilium_net@egress, similarly to what we do in bpf_overlay after decap.
With HostFw enabled, the to-host program in `cilium_host@ingress` will
then enforce policies.

This patch does not affect packets for local endpoint:
* With BPF Host Routing: will be directly delivered to the pod, tail
  calling into its ep->lxc_id function to enforce ingress policies.
* Without BPF Host Routing: will return to stack, which then goes to its
  to-container installed program to match ingress policies.

Trying to pull-in the whole `host_firewall.h` and `policy.h` would require
to set bpf_wireguard similarly as we do for bpf_host, meaning assigning
an endpoint ID, otherwise host policies would block all host related
packets (ep id == 0). For this reason, we decide here to go through cilium_host.

From a bpf test perspective:
* no changes for packet to/from local endpoint INGRESS/EGRESS
* no changes for packet from local host EGRESS
* packet to local host INGRESS: differently than bpf_host, in WireGuard
                                we always redirect to cilium_host@ingress.

While fixing this bits, let's move the superfluous `revalidate_data`
post NodePort inside the NodePort code, as not needed otherwise.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
@smagnani96 smagnani96 force-pushed the pr/smagnani96/wg-local-redirect branch from f45ba7b to 3b9daea Compare January 9, 2026 14:10
@smagnani96
Copy link
Copy Markdown
Contributor Author

/test

@smagnani96 smagnani96 removed the dont-merge/discussion A discussion is ongoing and should be resolved before merging, regardless of reviews & tests status. label Jan 9, 2026
@julianwiedmann julianwiedmann added this pull request to the merge queue Jan 9, 2026
@maintainer-s-little-helper maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Jan 9, 2026
Merged via the queue into main with commit 88e28e1 Jan 9, 2026
389 of 395 checks passed
@julianwiedmann julianwiedmann deleted the pr/smagnani96/wg-local-redirect branch January 9, 2026 18:14
@github-actions github-actions bot added the backport-done/1.18 The backport for Cilium 1.18.x for this PR is done. label Jan 12, 2026
@giorio94 giorio94 removed the needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch label Jan 19, 2026
smagnani96 added a commit that referenced this pull request Jan 21, 2026
In previous PR #39239, we enabled
marking the decrypted packet in bpf_wireguard with MARK_MAGIC_DECRYPT
only when ENABLE_IDENTITY_MARK is enabled. This is because w/o ENABLE_IDENTITY_MARK,
marking the packet causes issues with AWS, probably due to overlapping marks.

Now that #42892 is merged, we either
deliver the packet to the endpoint or to cilium_host, so we can always safely mark
the decrypted packet.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. backport/author The backport will be carried out by the author of the PR. backport-done/1.18 The backport for Cilium 1.18.x for this PR is done. feature/wireguard Relates to Cilium's Wireguard feature kind/bug This is a bug in the Cilium logic. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.

Projects

No open projects
Status: Released

Development

Successfully merging this pull request may close these issues.

6 participants