* [PATCH nf-next v2 1/1] netfilter: conntrack: skip verification of zero UDP checksum [not found] <20220405234739.269371-2-kevmitch@arista.com> @ 2022-04-08 4:33 ` Kevin Mitchell 2022-04-27 14:36 ` Pablo Neira Ayuso 0 siblings, 1 reply; 4+ messages in thread From: Kevin Mitchell @ 2022-04-08 4:33 UTC (permalink / raw) Cc: kevmitch, gal, Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal, David S. Miller, Jakub Kicinski, Paolo Abeni, Hideaki YOSHIFUJI, David Ahern, netfilter-devel, coreteam, netdev, linux-kernel 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 <kevmitch@arista.com> --- 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 + */ + if (skb->protocol == htons(ETH_P_IPV6)) + return true; + + udp_hdr = skb_header_pointer(skb, dataoff, + sizeof(_udp_hdr), + &_udp_hdr); + if (!udp_hdr || udp_hdr->check) + return true; + + return false; + } + case IPPROTO_GRE: + /* Protocols with other integrity checks. */ case IPPROTO_AH: case IPPROTO_ESP: @@ -19,9 +41,6 @@ static inline bool nf_reject_verify_csum(__u8 proto) /* Protocols with partial checksums. */ case IPPROTO_UDPLITE: case IPPROTO_DCCP: - - /* Protocols with optional checksums. */ - case IPPROTO_GRE: return false; } return true; diff --git a/net/ipv4/netfilter/nf_reject_ipv4.c b/net/ipv4/netfilter/nf_reject_ipv4.c index 4eed5afca392..6c46d4e8ab84 100644 --- a/net/ipv4/netfilter/nf_reject_ipv4.c +++ b/net/ipv4/netfilter/nf_reject_ipv4.c @@ -82,6 +82,7 @@ struct sk_buff *nf_reject_skb_v4_unreach(struct net *net, unsigned int len; __wsum csum; u8 proto; + int dataoff; if (!nf_reject_iphdr_validate(oldskb)) return NULL; @@ -99,10 +100,11 @@ struct sk_buff *nf_reject_skb_v4_unreach(struct net *net, if (pskb_trim_rcsum(oldskb, ntohs(ip_hdr(oldskb)->tot_len))) return NULL; + dataoff = ip_hdrlen(oldskb); proto = ip_hdr(oldskb)->protocol; if (!skb_csum_unnecessary(oldskb) && - nf_reject_verify_csum(proto) && + nf_reject_verify_csum(oldskb, dataoff, proto) && nf_ip_checksum(oldskb, hook, ip_hdrlen(oldskb), proto)) return NULL; @@ -312,6 +314,7 @@ void nf_send_unreach(struct sk_buff *skb_in, int code, int hook) { struct iphdr *iph = ip_hdr(skb_in); u8 proto = iph->protocol; + int dataoff = ip_hdrlen(skb_in); if (iph->frag_off & htons(IP_OFFSET)) return; @@ -320,12 +323,13 @@ void nf_send_unreach(struct sk_buff *skb_in, int code, int hook) nf_reject_fill_skb_dst(skb_in) < 0) return; - if (skb_csum_unnecessary(skb_in) || !nf_reject_verify_csum(proto)) { + if (skb_csum_unnecessary(skb_in) || + !nf_reject_verify_csum(skb_in, dataoff, proto)) { icmp_send(skb_in, ICMP_DEST_UNREACH, code, 0); return; } - if (nf_ip_checksum(skb_in, hook, ip_hdrlen(skb_in), proto) == 0) + if (nf_ip_checksum(skb_in, hook, dataoff, proto) == 0) icmp_send(skb_in, ICMP_DEST_UNREACH, code, 0); } EXPORT_SYMBOL_GPL(nf_send_unreach); diff --git a/net/ipv6/netfilter/nf_reject_ipv6.c b/net/ipv6/netfilter/nf_reject_ipv6.c index dffeaaaadcde..f61d4f18e1cf 100644 --- a/net/ipv6/netfilter/nf_reject_ipv6.c +++ b/net/ipv6/netfilter/nf_reject_ipv6.c @@ -31,7 +31,7 @@ static bool nf_reject_v6_csum_ok(struct sk_buff *skb, int hook) if (thoff < 0 || thoff >= skb->len || (fo & htons(~0x7)) != 0) return false; - if (!nf_reject_verify_csum(proto)) + if (!nf_reject_verify_csum(skb, thoff, proto)) return true; return nf_ip6_checksum(skb, hook, thoff, proto) == 0; @@ -388,7 +388,7 @@ static bool reject6_csum_ok(struct sk_buff *skb, int hook) if (thoff < 0 || thoff >= skb->len || (fo & htons(~0x7)) != 0) return false; - if (!nf_reject_verify_csum(proto)) + if (!nf_reject_verify_csum(skb, thoff, proto)) return true; return nf_ip6_checksum(skb, hook, thoff, proto) == 0; -- 2.35.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH nf-next v2 1/1] netfilter: conntrack: skip verification of zero UDP checksum 2022-04-08 4:33 ` [PATCH nf-next v2 1/1] netfilter: conntrack: skip verification of zero UDP checksum Kevin Mitchell @ 2022-04-27 14:36 ` Pablo Neira Ayuso 2022-04-30 3:40 ` [PATCH nf-next v3] " Kevin Mitchell 0 siblings, 1 reply; 4+ messages in thread From: Pablo Neira Ayuso @ 2022-04-27 14:36 UTC (permalink / raw) To: Kevin Mitchell Cc: gal, Jozsef Kadlecsik, Florian Westphal, David S. Miller, Jakub Kicinski, Paolo Abeni, Hideaki YOSHIFUJI, David Ahern, netfilter-devel, coreteam, netdev, linux-kernel 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 <kevmitch@arista.com> > --- > 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. ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH nf-next v3] netfilter: conntrack: skip verification of zero UDP checksum 2022-04-27 14:36 ` Pablo Neira Ayuso @ 2022-04-30 3:40 ` Kevin Mitchell 2022-05-09 6:06 ` Pablo Neira Ayuso 0 siblings, 1 reply; 4+ messages in thread From: Kevin Mitchell @ 2022-04-30 3:40 UTC (permalink / raw) Cc: kevmitch, gal, Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Hideaki YOSHIFUJI, David Ahern, netfilter-devel, coreteam, netdev, linux-kernel The checksum is optional for UDP packets. 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. Signed-off-by: Kevin Mitchell <kevmitch@arista.com> --- include/net/netfilter/nf_reject.h | 21 +++++++++++++++++---- net/ipv4/netfilter/nf_reject_ipv4.c | 10 +++++++--- net/ipv6/netfilter/nf_reject_ipv6.c | 4 ++-- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/include/net/netfilter/nf_reject.h b/include/net/netfilter/nf_reject.h index 9051c3a0c8e7..7c669792fb9c 100644 --- a/include/net/netfilter/nf_reject.h +++ b/include/net/netfilter/nf_reject.h @@ -5,12 +5,28 @@ #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; + + udp_hdr = skb_header_pointer(skb, dataoff, + sizeof(_udp_hdr), + &_udp_hdr); + if (!udp_hdr || udp_hdr->check) + return true; + + return false; + } + case IPPROTO_GRE: + /* Protocols with other integrity checks. */ case IPPROTO_AH: case IPPROTO_ESP: @@ -19,9 +35,6 @@ static inline bool nf_reject_verify_csum(__u8 proto) /* Protocols with partial checksums. */ case IPPROTO_UDPLITE: case IPPROTO_DCCP: - - /* Protocols with optional checksums. */ - case IPPROTO_GRE: return false; } return true; diff --git a/net/ipv4/netfilter/nf_reject_ipv4.c b/net/ipv4/netfilter/nf_reject_ipv4.c index 4eed5afca392..6c46d4e8ab84 100644 --- a/net/ipv4/netfilter/nf_reject_ipv4.c +++ b/net/ipv4/netfilter/nf_reject_ipv4.c @@ -82,6 +82,7 @@ struct sk_buff *nf_reject_skb_v4_unreach(struct net *net, unsigned int len; __wsum csum; u8 proto; + int dataoff; if (!nf_reject_iphdr_validate(oldskb)) return NULL; @@ -99,10 +100,11 @@ struct sk_buff *nf_reject_skb_v4_unreach(struct net *net, if (pskb_trim_rcsum(oldskb, ntohs(ip_hdr(oldskb)->tot_len))) return NULL; + dataoff = ip_hdrlen(oldskb); proto = ip_hdr(oldskb)->protocol; if (!skb_csum_unnecessary(oldskb) && - nf_reject_verify_csum(proto) && + nf_reject_verify_csum(oldskb, dataoff, proto) && nf_ip_checksum(oldskb, hook, ip_hdrlen(oldskb), proto)) return NULL; @@ -312,6 +314,7 @@ void nf_send_unreach(struct sk_buff *skb_in, int code, int hook) { struct iphdr *iph = ip_hdr(skb_in); u8 proto = iph->protocol; + int dataoff = ip_hdrlen(skb_in); if (iph->frag_off & htons(IP_OFFSET)) return; @@ -320,12 +323,13 @@ void nf_send_unreach(struct sk_buff *skb_in, int code, int hook) nf_reject_fill_skb_dst(skb_in) < 0) return; - if (skb_csum_unnecessary(skb_in) || !nf_reject_verify_csum(proto)) { + if (skb_csum_unnecessary(skb_in) || + !nf_reject_verify_csum(skb_in, dataoff, proto)) { icmp_send(skb_in, ICMP_DEST_UNREACH, code, 0); return; } - if (nf_ip_checksum(skb_in, hook, ip_hdrlen(skb_in), proto) == 0) + if (nf_ip_checksum(skb_in, hook, dataoff, proto) == 0) icmp_send(skb_in, ICMP_DEST_UNREACH, code, 0); } EXPORT_SYMBOL_GPL(nf_send_unreach); diff --git a/net/ipv6/netfilter/nf_reject_ipv6.c b/net/ipv6/netfilter/nf_reject_ipv6.c index dffeaaaadcde..f61d4f18e1cf 100644 --- a/net/ipv6/netfilter/nf_reject_ipv6.c +++ b/net/ipv6/netfilter/nf_reject_ipv6.c @@ -31,7 +31,7 @@ static bool nf_reject_v6_csum_ok(struct sk_buff *skb, int hook) if (thoff < 0 || thoff >= skb->len || (fo & htons(~0x7)) != 0) return false; - if (!nf_reject_verify_csum(proto)) + if (!nf_reject_verify_csum(skb, thoff, proto)) return true; return nf_ip6_checksum(skb, hook, thoff, proto) == 0; @@ -388,7 +388,7 @@ static bool reject6_csum_ok(struct sk_buff *skb, int hook) if (thoff < 0 || thoff >= skb->len || (fo & htons(~0x7)) != 0) return false; - if (!nf_reject_verify_csum(proto)) + if (!nf_reject_verify_csum(skb, thoff, proto)) return true; return nf_ip6_checksum(skb, hook, thoff, proto) == 0; -- 2.35.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH nf-next v3] netfilter: conntrack: skip verification of zero UDP checksum 2022-04-30 3:40 ` [PATCH nf-next v3] " Kevin Mitchell @ 2022-05-09 6:06 ` Pablo Neira Ayuso 0 siblings, 0 replies; 4+ messages in thread From: Pablo Neira Ayuso @ 2022-05-09 6:06 UTC (permalink / raw) To: Kevin Mitchell Cc: gal, Jozsef Kadlecsik, Florian Westphal, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Hideaki YOSHIFUJI, David Ahern, netfilter-devel, coreteam, netdev, linux-kernel On Fri, Apr 29, 2022 at 08:40:27PM -0700, Kevin Mitchell wrote: > The checksum is optional for UDP packets. 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. Applied. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-05-09 6:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20220405234739.269371-2-kevmitch@arista.com>
2022-04-08 4:33 ` [PATCH nf-next v2 1/1] netfilter: conntrack: skip verification of zero UDP checksum Kevin Mitchell
2022-04-27 14:36 ` Pablo Neira Ayuso
2022-04-30 3:40 ` [PATCH nf-next v3] " Kevin Mitchell
2022-05-09 6:06 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).