Re: [PATCH] net/ipv6: icmp: fix is_ineligible() to block errors for Redirect packets

[Sayooj K Karun]

You are right that netfilter can be configured to make devices behave in
non-RFC-compliant ways, so I will drop the "netfilter policy must obey
the RFC" framing from my earlier reply.

The point I should have made is that is_ineligible() is not a netfilter
function. It is the generic gate that icmpv6_send() uses to decide
whether the kernel, as an ICMPv6 originator, may emit an error for a
given trigger packet, and it is shared by all icmpv6_send() callers. It
already enforces RFC 4443 section 2.4(e.1) at exactly this spot, via
!(*tp & ICMPV6_INFOMSG_MASK), that is "do not originate an error in
response to an ICMPv6 error". My patch adds (e.2) (Redirect) right next
to it, the second rule from the same MUST NOT list. So this is not
about overriding netfilter policy; it is completing the e.1/e.2 pair at
the single point where the kernel decides ICMPv6 error eligibility.

On how it fixes the REJECT case: the two IPv6 reject paths differ in who
actually frames the ICMPv6 error. The bridge/netdev path,
nf_reject_skb_v6_unreach(), builds the packet by hand: it allocates the
skb, writes the IPv6 and ICMPv6 headers, copies in the original packet
and computes the checksum. Because it does all that itself, it has to
carry its own guard, nf_skb_is_icmp6_unreach(), the IPv6 analogue of the
nf_skb_is_icmp_unreach() you mention.

The L3 path, ip6t_REJECT / nft_reject -> nf_send_unreach6(), never frames
a packet of its own. It just calls icmpv6_send() and lets the core
ICMPv6 stack build and send the error. is_ineligible() is the gate that
core builder consults first, before it allocates or assembles anything,
so that is exactly where the e.1 suppression already lives for this path.
There is no netfilter-local guard here, and there does not need to be.

So the scenario in my commit message is the L3 path:

	ip6t_REJECT / nft_reject
	  > nf_send_unreach6()
	    > icmpv6_send() / icmp6_send()
	      > is_ineligible()  // now returns true for NDISC_REDIRECT
	        > goto out, no packet is ever built or transmitted

The patch fixes the REJECT case because the L3 reject hands packet
construction to icmp6_send(), and is_ineligible() runs at the top of
that builder, before any error skb exists. It is the same spot that
already drops e.1 today, so adding e.2 there completes the pair rather
than introducing a new override.

I also agree there is a gap to close on the netfilter side. The
bridge/netdev path never reaches is_ineligible(), and its
nf_skb_is_icmp6_unreach() guard currently checks only
ICMPV6_DEST_UNREACH, not Redirect, so it is not covered by this patch. I
will send a follow-up to netfilter-devel for nf-next extending that
guard to also skip Redirect, so both paths behave consistently.

Does that split sound right? this fix to is_ineligible() for the L3
path, plus a separate nf-next patch for the bridge/netdev reject guard?

Thanks,
Sayooj