policy: return ICMP "Destination unreachable" on ipv4 ingress policy denials by antonipp · Pull Request #42068 · cilium/cilium

antonipp · 2025-10-08T09:57:06Z

Description

Follow-up to #41406. See #41859 for the full list of TODOs.

This PR adds the ICMP response functionality to ingress policy denials.
The functionality works for local and remote endpoints with/without tunneling and with/without eBPF Host Routing.

Testing

Created a new unit test
Performed integration tests: backported the PR to Cilium 1.18.2, then created a pod with an ingress policy denying all traffic. Then tried to reach this pod from multiple different places and confirmed ping was returning "Packet filtered":
```
 # ping 10.130.112.4
 PING 10.130.112.4 (10.130.112.4) 56(84) bytes of data.
 From 10.130.112.4 icmp_seq=1 Packet filtered
 From 10.130.112.4 icmp_seq=2 Packet filtered
 [...]
```
- Local pod from same node [legacy routing]
- Pod from a different node in the same cluster [legacy routing]
- Pod in a different cluster [legacy routing]
- Local pod from same node [eBPF host routing]
- Pod from a different node in the same cluster [eBPF host routing]
- Pod in a different cluster [eBPF host routing]

Set up Cilium with tunneling in kind and ran the connectivity test suite:

 $ cilium connectivity test
 [...]
 ✅ [cilium-test-1] All 77 tests (1087 actions) successful, 46 tests skipped, 1 scenarios skipped.

policy: add option to return ICMP "Destination unreachable" on ipv4 ingress policy denials

antonipp · 2025-10-08T10:30:16Z

/test

julianwiedmann

(Unfortunately I don't have a quick way to set up a real cluster with tunneling rn)

Have you tried using the kind-based development setup?

julianwiedmann · 2025-10-15T11:42:46Z

bpf/bpf_lxc.c

+#if defined(TUNNEL_MODE)
+		if (from_tunnel) {
+			struct remote_endpoint_info *info;
+			struct trace_ctx trace = {
+				.reason = TRACE_REASON_POLICY,
+				.monitor = TRACE_PAYLOAD_LEN,
+			};
+
+			info = lookup_ip4_remote_endpoint(ip4->daddr, 0);
+			if (info && info->flag_has_tunnel_ep) {
+				ret = encap_and_redirect_lxc(ctx, info, SECLABEL_IPV4,
+							     info->sec_identity, &trace,
+							     bpf_htons(ETH_P_IP));
+				if (ret == CTX_ACT_REDIRECT) {
+					update_metrics(ctx_full_len(ctx), metric_dir,
+						       __DROP_REASON(verdict));
+					return ret;
+				}
+				if (IS_ERR(ret))
+					goto drop_err;
+				if (!revalidate_data(ctx, &data, &data_end, &ip4)) {
+					ret = DROP_INVALID;
+					goto drop_err;
+				}
+			}
+		}
+#endif /* TUNNEL_MODE */
+
+		if (is_defined(ENABLE_HOST_ROUTING)) {
+			ret = fib_redirect_v4(ctx, ETH_HLEN, ip4, false, false, &ext_err, &oif);
+			if (fib_ok(ret)) {
+				update_metrics(ctx_full_len(ctx), metric_dir,
+					       __DROP_REASON(verdict));
+				return ret;
+			}
+		} else {
+			ret = redirect_self(ctx);
+			if (!IS_ERR(ret)) {
+				update_metrics(ctx_full_len(ctx), metric_dir,
+					       __DROP_REASON(verdict));
+				return ret;
+			}


For reference - I'd expect us to follow this path here 1-to-1, as if the ICMP was genuinely sent out by the pod and needs forwarding to its destination. To the point where this might have to be a shared helper.

Unless there's cases in there which don't apply for an ICMP reply?

Yeah, you are right, it was a bit weird re-creating this logic. I refactored the code to extract it to a shared helper function ipv4_forward_to_destination which is now called in both places. I also set up kind and tested the tunneling scenario (updated the PR description accordingly)

antonipp · 2025-10-21T09:26:55Z

/test

This expands the functionality enabled by --policy-deny-response=icmp from egress-only to egress and ingress. Now ICMP responses can be sent for ingress policy denials as well. Signed-off-by: Anton Ippolitov <anton.ippolitov@datadoghq.com>

antonipp · 2025-10-21T15:34:19Z

/test

julianwiedmann

Thank you! Had a quick first look to find my bearings, see some comments below.

Noting down the full(?) matrix of possible packet paths:

arrival via {to-container, tailcall program}
origin is {self, local pod, local host, local proxy, remote pod/host, world}

For the local delivery paths, we'll need to have a good eye on what expectations redirect_ep() has, to determine whether it can use bpf_redirect_peer().

julianwiedmann · 2025-10-29T11:02:17Z

bpf/bpf_lxc.c

-		ret = redirect_self(ctx);

-		if (!IS_ERR(ret)) {
-			update_metrics(ctx_full_len(ctx), METRIC_EGRESS, __DROP_REASON(verdict));
+		info = lookup_ip4_remote_endpoint(ip4->daddr, 0);


Ah, interesting that we can also use ipv4_forward_to_destination() for the egress-deny path! I'd suggest to do that in a follow-up PR though, so we can evaluate it on its own.

julianwiedmann · 2025-10-29T11:05:50Z

bpf/bpf_lxc.c

+	__u8 metric_dir = (__u8)ctx_load_meta(ctx, CB_POLICY_DENY_DIR);
+	__u32 src_label = metric_dir == METRIC_EGRESS ?
+			  SECLABEL_IPV4 : ctx_load_meta(ctx, CB_POLICY_DENY_SRC_ID);
+	__u32 dst_sec_identity = WORLD_IPV4_ID;


Hm we previously applied network policy for the packet, so we must have already determined the peer's identity. Ideally we're re-using that identity here, and not try to resolve it again.

That allows us to be consistent, and handles cases where the actual identity is different from what's in the ipcache (relevant for eg. proxy connections).

julianwiedmann · 2025-10-29T11:07:02Z

bpf/bpf_lxc.c

+		ret = ipv4_forward_to_destination(ctx, ip4, NULL, &dst_sec_identity,
+						  &ext_err, &ct_state_local, CT_NEW,
+						  info, &trace, false, false, false, 0, 0);


Oh it's going to be fun figuring out all the little corner-cases from passing "default" values here :)

julianwiedmann · 2025-10-29T11:08:35Z

bpf/bpf_lxc.c

+ipv4_forward_to_destination(struct __ctx_buff *ctx,
+			    struct iphdr *ip4,
+			    struct ipv4_ct_tuple *tuple,
+			    const __u32 *dst_sec_identity,


The dst_sec_identity can be a regular __u32 now I think, we're not writing to it from this function.

julianwiedmann · 2025-10-29T11:10:44Z

bpf/bpf_lxc.c

-static __always_inline int handle_ipv4_from_lxc(struct __ctx_buff *ctx, __u32 *dst_sec_identity,
-						__s8 *ext_err)
+static __always_inline int
+ipv4_forward_to_destination(struct __ctx_buff *ctx,
+			    struct iphdr *ip4,


Please split this big patch into (at least) two parts, so it's easier to review (and partially revert if needed):

pure refactor, only splitting off the ipv4_forward_to_destination() logic.

adding all the new pieces for the ingress-deny ICMP message.

github-actions · 2025-11-29T02:07:53Z

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

github-actions · 2026-01-02T02:12:45Z

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

antonipp · 2026-02-11T08:05:11Z

Closing this one, since I handed over my work on this to Alex #44184

maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 8, 2025

antonipp force-pushed the ai/icmp-drop-ingress-ipv4 branch from 128984c to 1ba7880 Compare October 8, 2025 10:14

antonipp marked this pull request as ready for review October 8, 2025 10:30

antonipp requested review from a team as code owners October 8, 2025 10:30

antonipp requested review from fristonio, hemanthmalla and ldelossa October 8, 2025 10:30

julianwiedmann self-requested a review October 8, 2025 11:19

maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Oct 9, 2025

julianwiedmann reviewed Oct 15, 2025

View reviewed changes

antonipp force-pushed the ai/icmp-drop-ingress-ipv4 branch 2 times, most recently from 1ccf465 to 84836e6 Compare October 21, 2025 09:16

antonipp force-pushed the ai/icmp-drop-ingress-ipv4 branch from 84836e6 to 48d74f0 Compare October 21, 2025 15:34

hemanthmalla approved these changes Oct 21, 2025

View reviewed changes

julianwiedmann self-requested a review October 23, 2025 07:06

julianwiedmann reviewed Oct 29, 2025

View reviewed changes

antonipp mentioned this pull request Nov 19, 2025

Network Policy traffic drop response feature improvements #41859

Open

7 tasks

github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Nov 29, 2025

antonipp marked this pull request as draft December 2, 2025 10:28

github-actions bot removed the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Dec 3, 2025

julianwiedmann mentioned this pull request Dec 9, 2025

bpf: lxc: split forwarding section from handle_ipv*_from_lxc() #43226

Merged

github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Jan 2, 2026

joestringer added the dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs label Jan 8, 2026

github-actions bot removed the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Jan 9, 2026

aanm removed the dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs label Jan 14, 2026

41ks mentioned this pull request Feb 4, 2026

policy: return ICMP "Destination unreachable" on ipv4 ingress policy denials #44184

Draft

6 tasks

antonipp closed this Feb 11, 2026

antonipp deleted the ai/icmp-drop-ingress-ipv4 branch February 11, 2026 08:05

Conversation

antonipp commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Uh oh!

antonipp commented Oct 8, 2025

Uh oh!

julianwiedmann left a comment

Choose a reason for hiding this comment

Uh oh!

julianwiedmann Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antonipp Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

antonipp commented Oct 21, 2025

Uh oh!

antonipp commented Oct 21, 2025

Uh oh!

julianwiedmann left a comment

Choose a reason for hiding this comment

Uh oh!

julianwiedmann Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

julianwiedmann Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

julianwiedmann Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

julianwiedmann Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

julianwiedmann Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 29, 2025

Uh oh!

github-actions bot commented Jan 2, 2026

Uh oh!

antonipp commented Feb 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

antonipp commented Oct 8, 2025 •

edited

Loading

julianwiedmann Oct 15, 2025 •

edited

Loading