netdev - [PATCH v2] icmp: fix icmp_ndo_send address translation for reply direction

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250825203826.3231093-1-fabian@blaese.de>
Date: Mon, 25 Aug 2025 22:38:26 +0200
From: Fabian Bläse <fabian@...ese.de>
To: netdev@...r.kernel.org
Cc: netfilter-devel@...r.kernel.org,
	Fabian Bläse <fabian@...ese.de>,
	"Jason A. Donenfeld" <Jason@...c4.com>
Subject: [PATCH v2] icmp: fix icmp_ndo_send address translation for reply direction

The icmp_ndo_send function was originally introduced to ensure proper
rate limiting when icmp_send is called by a network device driver,
where the packet's source address may have already been transformed
by NAT or MASQUERADE.

However, the implementation only considered the IP_CT_DIR_ORIGINAL case
and incorrectly applies the same logic to packets in reply direction.

Therefore, an SNAT rule in the original direction causes icmp_ndo_send to
translate the source IP of reply-direction packets, even though no
translation is required. The source address is translated to the sender
address of the original direction, because the original tuple's source
address is used.

On the other hand, icmp_ndo_send incorrectly misses translating the
source address of packets in reply-direction, leading to incorrect rate
limiting. The generated ICMP error is translated by netfilter at a later
stage, therefore the ICMP error is sent correctly.

Fix this by translating the address based on the connection direction:
- CT_DIR_ORIGINAL: Use the original tuple's source address
  (unchanged from current behavior)
- CT_DIR_REPLY: Use the reply tuple's source address
  (fixing the incorrect translation)

Fixes: 0b41713b6066 ("icmp: introduce helper for nat'd source address in network device context")

Signed-off-by: Fabian Bläse <fabian@...ese.de>
Cc: Jason A. Donenfeld <Jason@...c4.com>
---
Changes v1->v2:
- Implement fix for ICMPv6 as well
---
 net/ipv4/icmp.c     | 14 ++++++++++++--
 net/ipv6/ip6_icmp.c | 14 ++++++++++++--
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 2ffe73ea644f..a4fb0bc7c4cf 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -803,7 +803,13 @@ void icmp_ndo_send(struct sk_buff *skb_in, int type, int code, __be32 info)
 	__be32 orig_ip;
 
 	ct = nf_ct_get(skb_in, &ctinfo);
-	if (!ct || !(ct->status & IPS_SRC_NAT)) {
+	if (!ct) {
+		__icmp_send(skb_in, type, code, info, &opts);
+		return;
+	}
+
+	if ( !(ct->status & IPS_SRC_NAT && CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL)
+		&& !(ct->status & IPS_DST_NAT && CTINFO2DIR(ctinfo) == IP_CT_DIR_REPLY)) {
 		__icmp_send(skb_in, type, code, info, &opts);
 		return;
 	}
@@ -818,7 +824,11 @@ void icmp_ndo_send(struct sk_buff *skb_in, int type, int code, __be32 info)
 		goto out;
 
 	orig_ip = ip_hdr(skb_in)->saddr;
-	ip_hdr(skb_in)->saddr = ct->tuplehash[0].tuple.src.u3.ip;
+	if (CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL) {
+		ip_hdr(skb_in)->saddr = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.u3.ip;
+	} else {
+		ip_hdr(skb_in)->saddr = ct->tuplehash[IP_CT_DIR_REPLY].tuple.src.u3.ip;
+	}
 	__icmp_send(skb_in, type, code, info, &opts);
 	ip_hdr(skb_in)->saddr = orig_ip;
 out:
diff --git a/net/ipv6/ip6_icmp.c b/net/ipv6/ip6_icmp.c
index 9e3574880cb0..c6078694311c 100644
--- a/net/ipv6/ip6_icmp.c
+++ b/net/ipv6/ip6_icmp.c
@@ -58,7 +58,13 @@ void icmpv6_ndo_send(struct sk_buff *skb_in, u8 type, u8 code, __u32 info)
 	struct nf_conn *ct;
 
 	ct = nf_ct_get(skb_in, &ctinfo);
-	if (!ct || !(ct->status & IPS_SRC_NAT)) {
+	if (!ct) {
+		__icmpv6_send(skb_in, type, code, info, &parm);
+		return;
+	}
+
+	if ( !(ct->status & IPS_SRC_NAT && CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL)
+		&& !(ct->status & IPS_DST_NAT && CTINFO2DIR(ctinfo) == IP_CT_DIR_REPLY)) {
 		__icmpv6_send(skb_in, type, code, info, &parm);
 		return;
 	}
@@ -73,7 +79,11 @@ void icmpv6_ndo_send(struct sk_buff *skb_in, u8 type, u8 code, __u32 info)
 		goto out;
 
 	orig_ip = ipv6_hdr(skb_in)->saddr;
-	ipv6_hdr(skb_in)->saddr = ct->tuplehash[0].tuple.src.u3.in6;
+	if (CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL) {
+		ipv6_hdr(skb_in)->saddr = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.u3.in6;
+	} else {
+		ipv6_hdr(skb_in)->saddr = ct->tuplehash[IP_CT_DIR_REPLY].tuple.src.u3.in6;
+	}
 	__icmpv6_send(skb_in, type, code, info, &parm);
 	ipv6_hdr(skb_in)->saddr = orig_ip;
 out:
-- 
2.50.1