2022-04-27 15:06:50

by Pablo Neira Ayuso

[permalink] [raw]
Subject: Re: [PATCH nf-next v2 1/1] netfilter: conntrack: skip verification of zero UDP checksum

On Thu, Apr 07, 2022 at 09:33:40PM -0700, Kevin Mitchell wrote:
> The checksum is optional for UDP packets in IPv4. However nf_reject
> would previously require a valid checksum to elicit a response such as
> ICMP_DEST_UNREACH.
>
> Add some logic to nf_reject_verify_csum to determine if a UDP packet has
> a zero checksum and should therefore not be verified. Explicitly require
> a valid checksum for IPv6 consistent RFC 2460 and with the non-netfilter
> stack (see udp6_csum_zero_error).
>
> Signed-off-by: Kevin Mitchell <[email protected]>
> ---
> include/net/netfilter/nf_reject.h | 27 +++++++++++++++++++++++----
> net/ipv4/netfilter/nf_reject_ipv4.c | 10 +++++++---
> net/ipv6/netfilter/nf_reject_ipv6.c | 4 ++--
> 3 files changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/include/net/netfilter/nf_reject.h b/include/net/netfilter/nf_reject.h
> index 9051c3a0c8e7..f248c1ff8b22 100644
> --- a/include/net/netfilter/nf_reject.h
> +++ b/include/net/netfilter/nf_reject.h
> @@ -5,12 +5,34 @@
> #include <linux/types.h>
> #include <uapi/linux/in.h>
>
> -static inline bool nf_reject_verify_csum(__u8 proto)
> +static inline bool nf_reject_verify_csum(struct sk_buff *skb, int dataoff,
> + __u8 proto)
> {
> /* Skip protocols that don't use 16-bit one's complement checksum
> * of the entire payload.
> */
> switch (proto) {
> + /* Protocols with optional checksums. */
> + case IPPROTO_UDP: {
> + const struct udphdr *udp_hdr;
> + struct udphdr _udp_hdr;
> +
> + /* Checksum is required in IPv6
> + * see RFC 2460 section 8.1
> + */

Right, but follow up work say otherwise?

https://www.rfc-editor.org/rfc/rfc6935
https://www.rfc-editor.org/rfc/rfc6936

Moreover, conntrack and NAT already allow for UDP zero checksum in IPv6.

I'm inclined to stick to the existing behaviour for consistency, ie.
allow for zero checksum in IPv6 UDP.