[PATCH 0/1] UDP traceroute packets with no checksum

[PATCH 0/1] UDP traceroute packets with no checksum

[Kevin Mitchell]

We have run into some trouble with third-party devices whose traceroute
implementation sends UDP packets with empty checksums. Since the
checksum is optional for UDP inside an IPv4 packet, this is perfectly
legal, even if slightly strange. However, we found that such a packet
ends up getting silently ignored by our switch instead of eliciting the
expected ICMP response. The reason is that we handle UDP traceroute
packets are with the iptables rule

-A INPUT -p udp -m udp --dport 33434:33636 -j REJECT --reject-with icmp-port-unreachable

Such packets therefore get an iptables reject normally eliciting the
desired ICMP_DEST_UNREACH response from nf_send_unreach(). However, it
seems that before sending the ICMP response, this function determines
whether it needs to verify the checksum on the incoming packet by
checking if skb->ip_summed == CHECKSUM_UNNECESSARY or
skb->csum_valid. If this is not the case, then it proceeds to verify the
checksum. However, no provision is made here for an empty checksum,
which of course will fail to verify. In this case, netfilter declines
to send the ICMP response on the assumption that the packet is garbage
and the source address cannot be trusted.

As our front panel ports do not perform layer 4 hardware checksum
verification before sending packets to the kernel, the struct skb passed
to netif_recieve_skb by the driver are unconditionally marked
CHECKSUM_NONE.

The udp_error function makes accommodation for UDP packets with empty
checksum by declining to run nf_checksum on them and instead indicating
the packet acceptable. Since nf_checksum already alters the
skb->ip_summed member by marking it as CHECKSUM_COMPLETE when there is a
checksum to verify, it seems reasonable that udp_error similarly mark it
as CHECKSUM_UNNECESSARY when it determines there is no checksum. This is
supported by the note in skbuff.h that explicitly suggests such a value
for packets with empty checksum.

It seems that this issue was almost fixed by 7fc38225363d ("netfilter:
reject: skip csum verification for protocols that don't support it"),
which addressed protocols other than UDP. I considered adding a
IPPROTO_UDP branch to the nf_reject_verify_csum switch statement,
however it would necessarily be more complex than all of the existing
cases since it would require extracting the UDP header and checking the
checksum value. I'm open to doing that as well if requested, but it
doesn't appear to be necessary from what I can tell with the simpler
one-liner I have proposed here.

Finally, I have made no attempt to distinguish between IPv4 and IPv6 in
spite of the fact that empty checksums are not strictly allowed for UDP
inside IPv6. However, this decision to be more permissive appears to
have already been made since udp_error already declared such packets
error free. The aforementioned skbuff.h comment also explicitly mentions
setting empty checksum UDP in IPv6 as CHECKSUM_UNNECESSARY.

[PATCH 1/1] netfilter: conntrack: mark UDP zero checksum as CHECKSUM_UNNECESSARY

[Kevin Mitchell]

The udp_error function verifies the checksum of incoming UDP packets if
one is set. This has the desirable side effect of setting skb->ip_summed
to CHECKSUM_COMPLETE, signalling that this verification need not be
repeated further up the stack.

Conversely, when the UDP checksum is empty, which is perfectly legal (at least
inside IPv4), udp_error previously left no trace that the checksum had been
deemed acceptable.

This was a problem in particular for nf_reject_ipv4, which verifies the
checksum in nf_send_unreach() before sending ICMP_DEST_UNREACH. It makes
no accommodation for zero UDP checksums unless they are already marked
as CHECKSUM_UNNECESSARY.

This commit ensures packets with empty UDP checksum are marked as
CHECKSUM_UNNECESSARY, which is explicitly recommended in skbuff.h.

Signed-off-by: Kevin Mitchell <kevmitch@arista.com>
---
 net/netfilter/nf_conntrack_proto_udp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c
index 3b516cffc779..12f793d8fe0c 100644
--- a/net/netfilter/nf_conntrack_proto_udp.c
+++ b/net/netfilter/nf_conntrack_proto_udp.c
@@ -63,8 +63,10 @@ static bool udp_error(struct sk_buff *skb,
 	}
 
 	/* Packet with no checksum */
-	if (!hdr->check)
+	if (!hdr->check) {
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
 		return false;
+	}
 
 	/* Checksum invalid? Ignore.
 	 * We skip checking packets on the outgoing path
-- 
2.34.1

Re: [PATCH 1/1] netfilter: conntrack: mark UDP zero checksum as CHECKSUM_UNNECESSARY

[Florian Westphal]

Kevin Mitchell <kevmitch@arista.com> wrote:
> The udp_error function verifies the checksum of incoming UDP packets if
> one is set. This has the desirable side effect of setting skb->ip_summed
> to CHECKSUM_COMPLETE, signalling that this verification need not be
> repeated further up the stack.

Acked-by: Florian Westphal <fw@strlen.de>

Re: [PATCH 1/1] netfilter: conntrack: mark UDP zero checksum as CHECKSUM_UNNECESSARY

[Pablo Neira Ayuso]

On Fri, Jan 14, 2022 at 08:00:50PM -0800, Kevin Mitchell wrote:
> The udp_error function verifies the checksum of incoming UDP packets if
> one is set. This has the desirable side effect of setting skb->ip_summed
> to CHECKSUM_COMPLETE, signalling that this verification need not be
> repeated further up the stack.
> 
> Conversely, when the UDP checksum is empty, which is perfectly legal (at least
> inside IPv4), udp_error previously left no trace that the checksum had been
> deemed acceptable.
> 
> This was a problem in particular for nf_reject_ipv4, which verifies the
> checksum in nf_send_unreach() before sending ICMP_DEST_UNREACH. It makes
> no accommodation for zero UDP checksums unless they are already marked
> as CHECKSUM_UNNECESSARY.
> 
> This commit ensures packets with empty UDP checksum are marked as
> CHECKSUM_UNNECESSARY, which is explicitly recommended in skbuff.h.

Applied to nf-next