ipvs: fix the checksum validations

ip_vs_in_icmp_v6() is missing checksum validation for ICMPv6
packets from clients. In fact, as for TCP/UDP we should
validate the checksum for ICMP packets only when we
mangle the packets on MASQ or on reply for tunnel.

Also, Sashiko points out that handle_response_icmp() being
common for IPv4 and IPv6 is missing the pseudo-header
calculation while validating ICMPv6 messages from real
servers which is a problem if checksum is not validated
by the hardware.

Fix the problems by creating ip_vs_checksum_common_check()
helper and use it for TCP/UDP/ICMP both for IPv4 and IPv6.
Rely on the nf_checksum() for validating the ICMP messages
but use it also for TCP and UDP.

Use correct IP offset for IP_VS_DBG_RL_PKT for TCP/UDP/SCTP.

IPVS packets (TCP/UDP/SCTP/ICMP) do not need checksum
validation on LOCAL_OUT (local clients or local real
servers) and on FORWARD (traffic from servers on LAN).
Do it only on LOCAL_IN, in case nf_checksum() is not
called on PRE_ROUTING.

Also, ip_vs_checksum_complete() can be marked static.

Fixes: 2a3b791e6e11 ("IPVS: Add/adjust Netfilter hook functions and helpers for v6")
Link: https://sashiko.dev/#/patchset/20260708180315.77413-1-ja%40ssi.bg
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 417ff51f62fc8edb80475706832e9c6008854893..d8f9ddb0fb383be028e9196678b052eb2b3537d5 100644 (file)
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -25,7 +25,9 @@
 #include <linux/netfilter.h>           /* for union nf_inet_addr */
 #include <linux/ip.h>
 #include <linux/ipv6.h>                        /* for struct ipv6hdr */
+#include <net/route.h>
 #include <net/ipv6.h>
+#include <net/ip6_fib.h>
 #if IS_ENABLED(CONFIG_NF_CONNTRACK)
 #include <net/netfilter/nf_conntrack.h>
 #endif
@@ -2066,8 +2068,6 @@ void ip_vs_nat_icmp_v6(struct sk_buff *skb, struct ip_vs_protocol *pp,
                       struct ip_vs_conn *cp, int dir);
 #endif
 
-__sum16 ip_vs_checksum_complete(struct sk_buff *skb, int offset);
-
 static inline __wsum ip_vs_check_diff4(__be32 old, __be32 new, __wsum oldsum)
 {
        __be32 diff[2] = { ~old, new };
@@ -2093,6 +2093,33 @@ static inline __wsum ip_vs_check_diff2(__be16 old, __be16 new, __wsum oldsum)
        return csum_partial(diff, sizeof(diff), oldsum);
 }
 
+static inline bool ip_vs_checksum_needed(struct sk_buff *skb, int af)
+{
+       /* Checksum unnecessary or already validated? */
+       if (skb_csum_unnecessary(skb))
+               return false;
+       /* LOCAL_OUT ? */
+       if (!skb->dev || skb->dev->flags & IFF_LOOPBACK)
+               return false;
+       /* !LOCAL_IN (FORWARD) ? */
+       if (af == AF_INET6) {
+               if (!(dst_rt6_info(skb_dst(skb))->rt6i_flags & RTF_LOCAL))
+                       return false;
+       } else {
+               if (!(skb_rtable(skb)->rt_flags & RTCF_LOCAL))
+                       return false;
+       }
+       return true;
+}
+
+static inline bool ip_vs_checksum_common_check(struct sk_buff *skb,
+                                              int offset, int proto, int af)
+{
+       if (!ip_vs_checksum_needed(skb, af))
+               return true;
+       return !nf_checksum(skb, NF_INET_LOCAL_IN, offset, proto, af);
+}
+
 /* Forget current conntrack (unconfirmed) and attach notrack entry */
 static inline void ip_vs_notrack(struct sk_buff *skb)
 {

diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index bafab93451d037c7f9b3316cd4517cfb3c2ea9c9..c8b512725e6e0c4830b637864e1cbba9e32a4dcb 100644 (file)
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -867,7 +867,7 @@ static int sysctl_nat_icmp_send(struct netns_ipvs *ipvs) { return 0; }
 
 #endif
 
-__sum16 ip_vs_checksum_complete(struct sk_buff *skb, int offset)
+static __sum16 ip_vs_checksum_complete(struct sk_buff *skb, int offset)
 {
        return csum_fold(skb_checksum(skb, offset, skb->len - offset, 0));
 }
@@ -1038,13 +1038,14 @@ static int handle_response_icmp(int af, struct sk_buff *skb,
                                unsigned int offset, unsigned int ihl,
                                unsigned int hooknum)
 {
+       int iproto = af == AF_INET6 ? IPPROTO_ICMPV6 : IPPROTO_ICMP;
        unsigned int verdict = NF_DROP;
 
        if (IP_VS_FWD_METHOD(cp) != IP_VS_CONN_F_MASQ)
                goto after_nat;
 
        /* Ensure the checksum is correct */
-       if (!skb_csum_unnecessary(skb) && ip_vs_checksum_complete(skb, ihl)) {
+       if (!ip_vs_checksum_common_check(skb, ihl, iproto, af)) {
                /* Failed checksum! */
                IP_VS_DBG_BUF(1, "Forward ICMP: failed checksum from %s!\n",
                              IP_VS_DBG_ADDR(af, snet));
@@ -1898,7 +1899,8 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related,
        verdict = NF_DROP;
 
        /* Ensure the checksum is correct */
-       if (!skb_csum_unnecessary(skb) && ip_vs_checksum_complete(skb, ihl)) {
+       if ((IP_VS_FWD_METHOD(cp) == IP_VS_CONN_F_MASQ || tunnel) &&
+           !ip_vs_checksum_common_check(skb, ihl, IPPROTO_ICMP, AF_INET)) {
                /* Failed checksum! */
                IP_VS_DBG(1, "Incoming ICMP: failed checksum from %pI4!\n",
                          &iph->saddr);
@@ -2064,6 +2066,18 @@ static int ip_vs_in_icmp_v6(struct netns_ipvs *ipvs, struct sk_buff *skb,
                goto out;
        }
 
+       verdict = NF_DROP;
+
+       /* Ensure the checksum is correct */
+       if (IP_VS_FWD_METHOD(cp) == IP_VS_CONN_F_MASQ &&
+           !ip_vs_checksum_common_check(skb, iph->len, IPPROTO_ICMPV6,
+                                        AF_INET6)) {
+               /* Failed checksum! */
+               IP_VS_DBG(1, "Incoming ICMPv6: failed checksum from %pI6c!\n",
+                         &iph->saddr);
+               goto out;
+       }
+
        /* do the statistics and put it back */
        ip_vs_in_stats(cp, skb);
 

diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
index c67317be17dfaf682449a6bfefeb4601609e199e..f6f732b7dfa863f9581976f40b2092129e4d1232 100644 (file)
--- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
@@ -11,7 +11,7 @@
 
 static int
 sctp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp,
-               unsigned int sctphoff);
+               struct ip_vs_iphdr *iph);
 
 static int
 sctp_conn_schedule(struct netns_ipvs *ipvs, int af, struct sk_buff *skb,
@@ -109,7 +109,7 @@ sctp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
                int ret;
 
                /* Some checks before mangling */
-               if (!sctp_csum_check(cp->af, skb, pp, sctphoff))
+               if (!sctp_csum_check(cp->af, skb, pp, iph))
                        return 0;
 
                /* Call application helper if needed */
@@ -157,7 +157,7 @@ sctp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
                int ret;
 
                /* Some checks before mangling */
-               if (!sctp_csum_check(cp->af, skb, pp, sctphoff))
+               if (!sctp_csum_check(cp->af, skb, pp, iph))
                        return 0;
 
                /* Call application helper if needed */
@@ -187,19 +187,22 @@ sctp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
 
 static int
 sctp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp,
-               unsigned int sctphoff)
+               struct ip_vs_iphdr *iph)
 {
+       unsigned int sctphoff = iph->len;
        struct sctphdr *sh;
        __le32 cmp, val;
 
+       if (!ip_vs_checksum_needed(skb, af))
+               return 1;
        sh = (struct sctphdr *)(skb->data + sctphoff);
        cmp = sh->checksum;
        val = sctp_compute_cksum(skb, sctphoff);
 
        if (val != cmp) {
                /* CRC failure, dump it. */
-               IP_VS_DBG_RL_PKT(0, af, pp, skb, 0,
-                               "Failed checksum for");
+               IP_VS_DBG_RL_PKT(0, af, pp, skb, iph->off,
+                                "Failed checksum for");
                return 0;
        }
        return 1;

diff --git a/net/netfilter/ipvs/ip_vs_proto_tcp.c b/net/netfilter/ipvs/ip_vs_proto_tcp.c
index f86b763efcc4dc12e1a294e35ca3986be5fad229..533fce3e5e4e415111d07b4b4ec06aa2b4387146 100644 (file)
--- a/net/netfilter/ipvs/ip_vs_proto_tcp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_tcp.c
@@ -29,7 +29,7 @@
 
 static int
 tcp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp,
-              unsigned int tcphoff);
+              struct ip_vs_iphdr *iph);
 
 static int
 tcp_conn_schedule(struct netns_ipvs *ipvs, int af, struct sk_buff *skb,
@@ -166,7 +166,7 @@ tcp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
                int ret;
 
                /* Some checks before mangling */
-               if (!tcp_csum_check(cp->af, skb, pp, tcphoff))
+               if (!tcp_csum_check(cp->af, skb, pp, iph))
                        return 0;
 
                /* Call application helper if needed */
@@ -244,7 +244,7 @@ tcp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
                int ret;
 
                /* Some checks before mangling */
-               if (!tcp_csum_check(cp->af, skb, pp, tcphoff))
+               if (!tcp_csum_check(cp->af, skb, pp, iph))
                        return 0;
 
                /*
@@ -302,41 +302,13 @@ tcp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
 
 static int
 tcp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp,
-              unsigned int tcphoff)
+              struct ip_vs_iphdr *iph)
 {
-       switch (skb->ip_summed) {
-       case CHECKSUM_NONE:
-               skb->csum = skb_checksum(skb, tcphoff, skb->len - tcphoff, 0);
-               fallthrough;
-       case CHECKSUM_COMPLETE:
-#ifdef CONFIG_IP_VS_IPV6
-               if (af == AF_INET6) {
-                       if (csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
-                                           &ipv6_hdr(skb)->daddr,
-                                           skb->len - tcphoff,
-                                           IPPROTO_TCP,
-                                           skb->csum)) {
-                               IP_VS_DBG_RL_PKT(0, af, pp, skb, 0,
-                                                "Failed checksum for");
-                               return 0;
-                       }
-               } else
-#endif
-                       if (csum_tcpudp_magic(ip_hdr(skb)->saddr,
-                                             ip_hdr(skb)->daddr,
-                                             skb->len - tcphoff,
-                                             ip_hdr(skb)->protocol,
-                                             skb->csum)) {
-                               IP_VS_DBG_RL_PKT(0, af, pp, skb, 0,
-                                                "Failed checksum for");
-                               return 0;
-                       }
-               break;
-       default:
-               /* No need to checksum. */
-               break;
+       if (!ip_vs_checksum_common_check(skb, iph->len, IPPROTO_TCP, af)) {
+               IP_VS_DBG_RL_PKT(0, af, pp, skb, iph->off,
+                                "Failed checksum for");
+               return 0;
        }
-
        return 1;
 }
 

diff --git a/net/netfilter/ipvs/ip_vs_proto_udp.c b/net/netfilter/ipvs/ip_vs_proto_udp.c
index 58f9e255927e2eb6442308457faf4692a55957e0..de3597347542eb00852d6f39bb7dd2e4dac2eb8f 100644 (file)
--- a/net/netfilter/ipvs/ip_vs_proto_udp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_udp.c
@@ -25,7 +25,7 @@
 
 static int
 udp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp,
-              unsigned int udphoff);
+              struct ip_vs_iphdr *iph);
 
 static int
 udp_conn_schedule(struct netns_ipvs *ipvs, int af, struct sk_buff *skb,
@@ -155,7 +155,7 @@ udp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
                int ret;
 
                /* Some checks before mangling */
-               if (!udp_csum_check(cp->af, skb, pp, udphoff))
+               if (!udp_csum_check(cp->af, skb, pp, iph))
                        return 0;
 
                /*
@@ -238,7 +238,7 @@ udp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
                int ret;
 
                /* Some checks before mangling */
-               if (!udp_csum_check(cp->af, skb, pp, udphoff))
+               if (!udp_csum_check(cp->af, skb, pp, iph))
                        return 0;
 
                /*
@@ -298,48 +298,20 @@ udp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
 
 static int
 udp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp,
-              unsigned int udphoff)
+              struct ip_vs_iphdr *iph)
 {
        struct udphdr _udph, *uh;
 
-       uh = skb_header_pointer(skb, udphoff, sizeof(_udph), &_udph);
+       uh = skb_header_pointer(skb, iph->len, sizeof(_udph), &_udph);
        if (uh == NULL)
                return 0;
 
-       if (uh->check != 0) {
-               switch (skb->ip_summed) {
-               case CHECKSUM_NONE:
-                       skb->csum = skb_checksum(skb, udphoff,
-                                                skb->len - udphoff, 0);
-                       fallthrough;
-               case CHECKSUM_COMPLETE:
-#ifdef CONFIG_IP_VS_IPV6
-                       if (af == AF_INET6) {
-                               if (csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
-                                                   &ipv6_hdr(skb)->daddr,
-                                                   skb->len - udphoff,
-                                                   IPPROTO_UDP,
-                                                   skb->csum)) {
-                                       IP_VS_DBG_RL_PKT(0, af, pp, skb, 0,
-                                                        "Failed checksum for");
-                                       return 0;
-                               }
-                       } else
-#endif
-                               if (csum_tcpudp_magic(ip_hdr(skb)->saddr,
-                                                     ip_hdr(skb)->daddr,
-                                                     skb->len - udphoff,
-                                                     ip_hdr(skb)->protocol,
-                                                     skb->csum)) {
-                                       IP_VS_DBG_RL_PKT(0, af, pp, skb, 0,
-                                                        "Failed checksum for");
-                                       return 0;
-                               }
-                       break;
-               default:
-                       /* No need to checksum. */
-                       break;
-               }
+       if (!uh->check)
+               return 1;
+       if (!ip_vs_checksum_common_check(skb, iph->len, IPPROTO_UDP, af)) {
+               IP_VS_DBG_RL_PKT(0, af, pp, skb, iph->off,
+                                "Failed checksum for");
+               return 0;
        }
        return 1;
 }