|
TCP/UDP checksum validation for CHECKSUM_COMPLETE is broken
before the git history.
Expecting skb->csum to cover data starting from the protocol
header is wrong. As IPVS works at the IP layer, the csum for
the IP header is not subtracted yet.
ip_vs_in_icmp_v6() is missing checksum validation for ICMPv6
packets from clients.
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.
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@xxxxxx>
---
include/net/ip_vs.h | 5 ++-
net/netfilter/ipvs/ip_vs_core.c | 63 ++++++++++++++++++++++++++--
net/netfilter/ipvs/ip_vs_proto_tcp.c | 35 ++--------------
net/netfilter/ipvs/ip_vs_proto_udp.c | 39 +++--------------
4 files changed, 71 insertions(+), 71 deletions(-)
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 49297fec448a..fd18b1cd6471 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -2065,8 +2065,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 };
@@ -2092,6 +2090,9 @@ static inline __wsum ip_vs_check_diff2(__be16 old, __be16
new, __wsum oldsum)
return csum_partial(diff, sizeof(diff), oldsum);
}
+bool ip_vs_checksum_common_check(int af, struct sk_buff *skb, int proto,
+ int offset);
+
/* 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 c9c88c99d07b..ceb1cbf33dd7 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -867,11 +867,56 @@ 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));
}
+/**
+ * ip_vs_checksum_common_check - validate checksum for TCP/UDP/ICMP
+ * @af: AF_INET/AF_INET6
+ * @skb: socket buffer
+ * @proto: IPPROTO_xxx
+ * @offset: offset of protocol header
+ */
+bool ip_vs_checksum_common_check(int af, struct sk_buff *skb, int proto,
+ int offset)
+{
+ __wsum csum;
+
+ if (skb_csum_unnecessary(skb))
+ return true;
+ if (skb->ip_summed == CHECKSUM_NONE) {
+ csum = skb_checksum(skb, offset, skb->len - offset, 0);
+ } else if (skb->ip_summed == CHECKSUM_COMPLETE) {
+ /* IPVS works at IP layer, so skb->csum covers data from
+ * IP header, strip it up to the protocol header
+ */
+ csum = csum_sub(skb->csum, skb_checksum(skb, 0, offset, 0));
+ } else {
+ /* No need to checksum. */
+ return true;
+ }
+#ifdef CONFIG_IP_VS_IPV6
+ if (af == AF_INET6) {
+ if (csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
+ &ipv6_hdr(skb)->daddr,
+ skb->len - offset, proto,
+ csum))
+ return false;
+ } else
+#endif
+ if (proto == IPPROTO_ICMP)
+ return !csum_fold(csum);
+ else if (csum_tcpudp_magic(ip_hdr(skb)->saddr,
+ ip_hdr(skb)->daddr,
+ skb->len - offset, proto,
+ csum))
+ return false;
+
+ return true;
+}
+
static inline enum ip_defrag_users ip_vs_defrag_user(unsigned int hooknum)
{
if (NF_INET_LOCAL_IN == hooknum)
@@ -1039,12 +1084,13 @@ static int handle_response_icmp(int af, struct sk_buff
*skb,
unsigned int hooknum)
{
unsigned int verdict = NF_DROP;
+ int iproto = af == AF_INET6 ? IPPROTO_ICMPV6 : IPPROTO_ICMP;
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(af, skb, iproto, ihl)) {
/* Failed checksum! */
IP_VS_DBG_BUF(1, "Forward ICMP: failed checksum from %s!\n",
IP_VS_DBG_ADDR(af, snet));
@@ -1898,7 +1944,7 @@ 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_checksum_common_check(AF_INET, skb, IPPROTO_ICMP, ihl)) {
/* Failed checksum! */
IP_VS_DBG(1, "Incoming ICMP: failed checksum from %pI4!\n",
&iph->saddr);
@@ -2064,6 +2110,17 @@ 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_checksum_common_check(AF_INET6, skb, IPPROTO_ICMPV6,
+ iph->len)) {
+ /* 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_tcp.c
b/net/netfilter/ipvs/ip_vs_proto_tcp.c
index 8cc0a8ce6241..147cf01708ff 100644
--- a/net/netfilter/ipvs/ip_vs_proto_tcp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_tcp.c
@@ -304,39 +304,10 @@ static int
tcp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp,
unsigned int tcphoff)
{
- 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(af, skb, IPPROTO_TCP, tcphoff)) {
+ IP_VS_DBG_RL_PKT(0, af, pp, skb, 0, "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 f9de632e38cd..d10713ca74f7 100644
--- a/net/netfilter/ipvs/ip_vs_proto_udp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_udp.c
@@ -306,40 +306,11 @@ udp_csum_check(int af, struct sk_buff *skb, struct
ip_vs_protocol *pp,
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(af, skb, IPPROTO_UDP, udphoff)) {
+ IP_VS_DBG_RL_PKT(0, af, pp, skb, 0, "Failed checksum for");
+ return 0;
}
return 1;
}
--
2.55.0
|
