All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 net 0/5] icmp: account for NAT when sending icmps from ndo layer
@ 2020-02-11 19:47 Jason A. Donenfeld
  2020-02-11 19:47 ` [PATCH v4 net 1/5] icmp: introduce helper for nat'd source address in network device context Jason A. Donenfeld
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Jason A. Donenfeld @ 2020-02-11 19:47 UTC (permalink / raw)
  To: netdev, davem; +Cc: Jason A. Donenfeld, netfilter-devel

The ICMP routines use the source address for two reasons:

1. Rate-limiting ICMP transmissions based on source address, so
   that one source address cannot provoke a flood of replies. If
   the source address is wrong, the rate limiting will be
   incorrectly applied.

2. Choosing the interface and hence new source address of the
   generated ICMP packet. If the original packet source address
   is wrong, ICMP replies will be sent from the wrong source
   address, resulting in either a misdelivery, infoleak, or just
   general network admin confusion.

Most of the time, the icmp_send and icmpv6_send routines can just reach
down into the skb's IP header to determine the saddr. However, if
icmp_send or icmpv6_send is being called from a network device driver --
there are a few in the tree -- then it's possible that by the time
icmp_send or icmpv6_send looks at the packet, the packet's source
address has already been transformed by SNAT or MASQUERADE or some other
transformation that CONNTRACK knows about. In this case, the packet's
source address is most certainly the *wrong* source address to be used
for the purpose of ICMP replies.

Rather, the source address we want to use for ICMP replies is the
original one, from before the transformation occurred.

Fortunately, it's very easy to just ask CONNTRACK if it knows about this
packet, and if so, how to fix it up. The saddr is the only field in the
header we need to fix up, for the purposes of the subsequent processing
in the icmp_send and icmpv6_send functions, so we do the lookup very
early on, so that the rest of the ICMP machinery can progress as usual.

Changes v3->v4:
- Add back the skb_shared checking, since the previous assumption isn't
  actually true [Eric]. This implies dropping the additional patches v3 had
  for removing skb_share_check from various drivers. We can revisit that
  general set of ideas later, but that's probably better suited as a net-next
  patchset rather than this stable one which is geared at fixing bugs. So,
  this implements things in the safe conservative way.

Changes v2->v3:
- Add selftest to ensure this actually does what we want and never regresses.
- Check the size of the skb header before operating on it.
- Use skb_ensure_writable to ensure we can modify the cloned skb [Florian].
- Conditionalize this on IPS_SRC_NAT so we don't do anything unnecessarily
  [Florian].
- It turns out that since we're calling these from the xmit path,
  skb_share_check isn't required, so remove that [Florian]. This simplifes the
  code a bit too. **The supposition here is that skbs passed to ndo_start_xmit
  are _never_ shared. If this is not correct NOW IS THE TIME TO PIPE UP, for
  doom awaits us later.**
- While investigating the shared skb business, several drivers appeared to be
  calling it incorrectly in the xmit path, so this series also removes those
  unnecessary calls, based on the supposition mentioned in the previous point.

Changes v1->v2:
- icmpv6 takes subtly different types than icmpv4, like u32 instead of be32,
  u8 instead of int.
- Since we're technically writing to the skb, we need to make sure it's not
  a shared one [Dave, 2017].
- Restore the original skb data after icmp_send returns. All current users
  are freeing the packet right after, so it doesn't matter, but future users
  might not.
- Remove superfluous route lookup in sunvnet [Dave].
- Use NF_NAT instead of NF_CONNTRACK for condition [Florian].
- Include this cover letter [Dave].

Cc: netdev@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org

Jason A. Donenfeld (5):
  icmp: introduce helper for nat'd source address in network device
    context
  gtp: use icmp_ndo_send helper
  sunvnet: use icmp_ndo_send helper
  wireguard: device: use icmp_ndo_send helper
  xfrm: interface: use icmp_ndo_send helper

 drivers/net/ethernet/sun/sunvnet_common.c  | 23 +++------------
 drivers/net/gtp.c                          |  4 +--
 drivers/net/wireguard/device.c             |  4 +--
 include/linux/icmpv6.h                     |  6 ++++
 include/net/icmp.h                         |  6 ++++
 net/ipv4/icmp.c                            | 33 +++++++++++++++++++++
 net/ipv6/ip6_icmp.c                        | 34 ++++++++++++++++++++++
 net/xfrm/xfrm_interface.c                  |  6 ++--
 tools/testing/selftests/wireguard/netns.sh | 11 +++++++
 9 files changed, 101 insertions(+), 26 deletions(-)

-- 
2.25.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v4 net 1/5] icmp: introduce helper for nat'd source address in network device context
  2020-02-11 19:47 [PATCH v4 net 0/5] icmp: account for NAT when sending icmps from ndo layer Jason A. Donenfeld
@ 2020-02-11 19:47 ` Jason A. Donenfeld
  2020-02-11 19:47 ` [PATCH v4 net 2/5] gtp: use icmp_ndo_send helper Jason A. Donenfeld
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Jason A. Donenfeld @ 2020-02-11 19:47 UTC (permalink / raw)
  To: netdev, davem; +Cc: Jason A. Donenfeld, Florian Westphal

This introduces a helper function to be called only by network drivers
that wraps calls to icmp[v6]_send in a conntrack transformation, in case
NAT has been used. We don't want to pollute the non-driver path, though,
so we introduce this as a helper to be called by places that actually
make use of this, as suggested by Florian.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Florian Westphal <fw@strlen.de>
---
 include/linux/icmpv6.h |  6 ++++++
 include/net/icmp.h     |  6 ++++++
 net/ipv4/icmp.c        | 33 +++++++++++++++++++++++++++++++++
 net/ipv6/ip6_icmp.c    | 34 ++++++++++++++++++++++++++++++++++
 4 files changed, 79 insertions(+)

diff --git a/include/linux/icmpv6.h b/include/linux/icmpv6.h
index ef1cbb5f454f..93338fd54af8 100644
--- a/include/linux/icmpv6.h
+++ b/include/linux/icmpv6.h
@@ -31,6 +31,12 @@ static inline void icmpv6_send(struct sk_buff *skb,
 }
 #endif
 
+#if IS_ENABLED(CONFIG_NF_NAT)
+void icmpv6_ndo_send(struct sk_buff *skb_in, u8 type, u8 code, __u32 info);
+#else
+#define icmpv6_ndo_send icmpv6_send
+#endif
+
 extern int				icmpv6_init(void);
 extern int				icmpv6_err_convert(u8 type, u8 code,
 							   int *err);
diff --git a/include/net/icmp.h b/include/net/icmp.h
index 5d4bfdba9adf..9ac2d2672a93 100644
--- a/include/net/icmp.h
+++ b/include/net/icmp.h
@@ -43,6 +43,12 @@ static inline void icmp_send(struct sk_buff *skb_in, int type, int code, __be32
 	__icmp_send(skb_in, type, code, info, &IPCB(skb_in)->opt);
 }
 
+#if IS_ENABLED(CONFIG_NF_NAT)
+void icmp_ndo_send(struct sk_buff *skb_in, int type, int code, __be32 info);
+#else
+#define icmp_ndo_send icmp_send
+#endif
+
 int icmp_rcv(struct sk_buff *skb);
 int icmp_err(struct sk_buff *skb, u32 info);
 int icmp_init(void);
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 18068ed42f25..f369e7ce685b 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -748,6 +748,39 @@ out:;
 }
 EXPORT_SYMBOL(__icmp_send);
 
+#if IS_ENABLED(CONFIG_NF_NAT)
+#include <net/netfilter/nf_conntrack.h>
+void icmp_ndo_send(struct sk_buff *skb_in, int type, int code, __be32 info)
+{
+	struct sk_buff *cloned_skb = NULL;
+	enum ip_conntrack_info ctinfo;
+	struct nf_conn *ct;
+	__be32 orig_ip;
+
+	ct = nf_ct_get(skb_in, &ctinfo);
+	if (!ct || !(ct->status & IPS_SRC_NAT)) {
+		icmp_send(skb_in, type, code, info);
+		return;
+	}
+
+	if (skb_shared(skb_in))
+		skb_in = cloned_skb = skb_clone(skb_in, GFP_ATOMIC);
+
+	if (unlikely(!skb_in || skb_network_header(skb_in) < skb_in->head ||
+	    (skb_network_header(skb_in) + sizeof(struct iphdr)) >
+	    skb_tail_pointer(skb_in) || skb_ensure_writable(skb_in,
+	    skb_network_offset(skb_in) + sizeof(struct iphdr))))
+		goto out;
+
+	orig_ip = ip_hdr(skb_in)->saddr;
+	ip_hdr(skb_in)->saddr = ct->tuplehash[0].tuple.src.u3.ip;
+	icmp_send(skb_in, type, code, info);
+	ip_hdr(skb_in)->saddr = orig_ip;
+out:
+	consume_skb(cloned_skb);
+}
+EXPORT_SYMBOL(icmp_ndo_send);
+#endif
 
 static void icmp_socket_deliver(struct sk_buff *skb, u32 info)
 {
diff --git a/net/ipv6/ip6_icmp.c b/net/ipv6/ip6_icmp.c
index 02045494c24c..e0086758b6ee 100644
--- a/net/ipv6/ip6_icmp.c
+++ b/net/ipv6/ip6_icmp.c
@@ -45,4 +45,38 @@ void icmpv6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info)
 	rcu_read_unlock();
 }
 EXPORT_SYMBOL(icmpv6_send);
+
+#if IS_ENABLED(CONFIG_NF_NAT)
+#include <net/netfilter/nf_conntrack.h>
+void icmpv6_ndo_send(struct sk_buff *skb_in, u8 type, u8 code, __u32 info)
+{
+	struct sk_buff *cloned_skb = NULL;
+	enum ip_conntrack_info ctinfo;
+	struct in6_addr orig_ip;
+	struct nf_conn *ct;
+
+	ct = nf_ct_get(skb_in, &ctinfo);
+	if (!ct || !(ct->status & IPS_SRC_NAT)) {
+		icmpv6_send(skb_in, type, code, info);
+		return;
+	}
+
+	if (skb_shared(skb_in))
+		skb_in = cloned_skb = skb_clone(skb_in, GFP_ATOMIC);
+
+	if (unlikely(!skb_in || skb_network_header(skb_in) < skb_in->head ||
+	    (skb_network_header(skb_in) + sizeof(struct ipv6hdr)) >
+	    skb_tail_pointer(skb_in) || skb_ensure_writable(skb_in,
+	    skb_network_offset(skb_in) + sizeof(struct ipv6hdr))))
+		goto out;
+
+	orig_ip = ipv6_hdr(skb_in)->saddr;
+	ipv6_hdr(skb_in)->saddr = ct->tuplehash[0].tuple.src.u3.in6;
+	icmpv6_send(skb_in, type, code, info);
+	ipv6_hdr(skb_in)->saddr = orig_ip;
+out:
+	consume_skb(cloned_skb);
+}
+EXPORT_SYMBOL(icmpv6_ndo_send);
+#endif
 #endif
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v4 net 2/5] gtp: use icmp_ndo_send helper
  2020-02-11 19:47 [PATCH v4 net 0/5] icmp: account for NAT when sending icmps from ndo layer Jason A. Donenfeld
  2020-02-11 19:47 ` [PATCH v4 net 1/5] icmp: introduce helper for nat'd source address in network device context Jason A. Donenfeld
@ 2020-02-11 19:47 ` Jason A. Donenfeld
  2020-02-11 19:47 ` [PATCH v4 net 3/5] sunvnet: " Jason A. Donenfeld
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Jason A. Donenfeld @ 2020-02-11 19:47 UTC (permalink / raw)
  To: netdev, davem; +Cc: Jason A. Donenfeld, Harald Welte

Because gtp is calling icmp from network device context, it should use
the ndo helper so that the rate limiting applies correctly.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Harald Welte <laforge@gnumonks.org>
---
 drivers/net/gtp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index af07ea760b35..672cd2caf2fb 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -546,8 +546,8 @@ static int gtp_build_skb_ip4(struct sk_buff *skb, struct net_device *dev,
 	    mtu < ntohs(iph->tot_len)) {
 		netdev_dbg(dev, "packet too big, fragmentation needed\n");
 		memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
-		icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
-			  htonl(mtu));
+		icmp_ndo_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
+			      htonl(mtu));
 		goto err_rt;
 	}
 
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v4 net 3/5] sunvnet: use icmp_ndo_send helper
  2020-02-11 19:47 [PATCH v4 net 0/5] icmp: account for NAT when sending icmps from ndo layer Jason A. Donenfeld
  2020-02-11 19:47 ` [PATCH v4 net 1/5] icmp: introduce helper for nat'd source address in network device context Jason A. Donenfeld
  2020-02-11 19:47 ` [PATCH v4 net 2/5] gtp: use icmp_ndo_send helper Jason A. Donenfeld
@ 2020-02-11 19:47 ` Jason A. Donenfeld
  2020-02-11 19:47 ` [PATCH v4 net 4/5] wireguard: device: " Jason A. Donenfeld
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Jason A. Donenfeld @ 2020-02-11 19:47 UTC (permalink / raw)
  To: netdev, davem; +Cc: Jason A. Donenfeld, Shannon Nelson

Because sunvnet is calling icmp from network device context, it should use
the ndo helper so that the rate limiting applies correctly. While we're
at it, doing the additional route lookup before calling icmp_ndo_send is
superfluous, since this is the job of the icmp code in the first place.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/sun/sunvnet_common.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/sun/sunvnet_common.c b/drivers/net/ethernet/sun/sunvnet_common.c
index c23ce838ff63..8dc6c9ff22e1 100644
--- a/drivers/net/ethernet/sun/sunvnet_common.c
+++ b/drivers/net/ethernet/sun/sunvnet_common.c
@@ -1350,27 +1350,12 @@ sunvnet_start_xmit_common(struct sk_buff *skb, struct net_device *dev,
 		if (vio_version_after_eq(&port->vio, 1, 3))
 			localmtu -= VLAN_HLEN;
 
-		if (skb->protocol == htons(ETH_P_IP)) {
-			struct flowi4 fl4;
-			struct rtable *rt = NULL;
-
-			memset(&fl4, 0, sizeof(fl4));
-			fl4.flowi4_oif = dev->ifindex;
-			fl4.flowi4_tos = RT_TOS(ip_hdr(skb)->tos);
-			fl4.daddr = ip_hdr(skb)->daddr;
-			fl4.saddr = ip_hdr(skb)->saddr;
-
-			rt = ip_route_output_key(dev_net(dev), &fl4);
-			if (!IS_ERR(rt)) {
-				skb_dst_set(skb, &rt->dst);
-				icmp_send(skb, ICMP_DEST_UNREACH,
-					  ICMP_FRAG_NEEDED,
-					  htonl(localmtu));
-			}
-		}
+		if (skb->protocol == htons(ETH_P_IP))
+			icmp_ndo_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
+				      htonl(localmtu));
 #if IS_ENABLED(CONFIG_IPV6)
 		else if (skb->protocol == htons(ETH_P_IPV6))
-			icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, localmtu);
+			icmpv6_ndo_send(skb, ICMPV6_PKT_TOOBIG, 0, localmtu);
 #endif
 		goto out_dropped;
 	}
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v4 net 4/5] wireguard: device: use icmp_ndo_send helper
  2020-02-11 19:47 [PATCH v4 net 0/5] icmp: account for NAT when sending icmps from ndo layer Jason A. Donenfeld
                   ` (2 preceding siblings ...)
  2020-02-11 19:47 ` [PATCH v4 net 3/5] sunvnet: " Jason A. Donenfeld
@ 2020-02-11 19:47 ` Jason A. Donenfeld
  2020-02-11 19:47 ` [PATCH v4 net 5/5] xfrm: interface: " Jason A. Donenfeld
  2020-02-13 22:19 ` [PATCH v4 net 0/5] icmp: account for NAT when sending icmps from ndo layer David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: Jason A. Donenfeld @ 2020-02-11 19:47 UTC (permalink / raw)
  To: netdev, davem; +Cc: Jason A. Donenfeld

Because wireguard is calling icmp from network device context, it should
use the ndo helper so that the rate limiting applies correctly.  This
commit adds a small test to the wireguard test suite to ensure that the
new functions continue doing the right thing in the context of
wireguard. It does this by setting up a condition that will definately
evoke an icmp error message from the driver, but along a nat'd path.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/net/wireguard/device.c             |  4 ++--
 tools/testing/selftests/wireguard/netns.sh | 11 +++++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireguard/device.c b/drivers/net/wireguard/device.c
index 16b19824b9ad..43db442b1373 100644
--- a/drivers/net/wireguard/device.c
+++ b/drivers/net/wireguard/device.c
@@ -203,9 +203,9 @@ static netdev_tx_t wg_xmit(struct sk_buff *skb, struct net_device *dev)
 err:
 	++dev->stats.tx_errors;
 	if (skb->protocol == htons(ETH_P_IP))
-		icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_UNREACH, 0);
+		icmp_ndo_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_UNREACH, 0);
 	else if (skb->protocol == htons(ETH_P_IPV6))
-		icmpv6_send(skb, ICMPV6_DEST_UNREACH, ICMPV6_ADDR_UNREACH, 0);
+		icmpv6_ndo_send(skb, ICMPV6_DEST_UNREACH, ICMPV6_ADDR_UNREACH, 0);
 	kfree_skb(skb);
 	return ret;
 }
diff --git a/tools/testing/selftests/wireguard/netns.sh b/tools/testing/selftests/wireguard/netns.sh
index f5ab1cda8bb5..138d46b3f330 100755
--- a/tools/testing/selftests/wireguard/netns.sh
+++ b/tools/testing/selftests/wireguard/netns.sh
@@ -24,6 +24,7 @@
 set -e
 
 exec 3>&1
+export LANG=C
 export WG_HIDE_KEYS=never
 netns0="wg-test-$$-0"
 netns1="wg-test-$$-1"
@@ -297,7 +298,17 @@ ip1 -4 rule add table main suppress_prefixlength 0
 n1 ping -W 1 -c 100 -f 192.168.99.7
 n1 ping -W 1 -c 100 -f abab::1111
 
+# Have ns2 NAT into wg0 packets from ns0, but return an icmp error along the right route.
+n2 iptables -t nat -A POSTROUTING -s 10.0.0.0/24 -d 192.168.241.0/24 -j SNAT --to 192.168.241.2
+n0 iptables -t filter -A INPUT \! -s 10.0.0.0/24 -i vethrs -j DROP # Manual rpfilter just to be explicit.
+n2 bash -c 'printf 1 > /proc/sys/net/ipv4/ip_forward'
+ip0 -4 route add 192.168.241.1 via 10.0.0.100
+n2 wg set wg0 peer "$pub1" remove
+[[ $(! n0 ping -W 1 -c 1 192.168.241.1 || false) == *"From 10.0.0.100 icmp_seq=1 Destination Host Unreachable"* ]]
+
 n0 iptables -t nat -F
+n0 iptables -t filter -F
+n2 iptables -t nat -F
 ip0 link del vethrc
 ip0 link del vethrs
 ip1 link del wg0
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v4 net 5/5] xfrm: interface: use icmp_ndo_send helper
  2020-02-11 19:47 [PATCH v4 net 0/5] icmp: account for NAT when sending icmps from ndo layer Jason A. Donenfeld
                   ` (3 preceding siblings ...)
  2020-02-11 19:47 ` [PATCH v4 net 4/5] wireguard: device: " Jason A. Donenfeld
@ 2020-02-11 19:47 ` Jason A. Donenfeld
  2020-02-13 22:19 ` [PATCH v4 net 0/5] icmp: account for NAT when sending icmps from ndo layer David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: Jason A. Donenfeld @ 2020-02-11 19:47 UTC (permalink / raw)
  To: netdev, davem; +Cc: Jason A. Donenfeld, Nicolas Dichtel, Steffen Klassert

Because xfrmi is calling icmp from network device context, it should use
the ndo helper so that the rate limiting applies correctly.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_interface.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/xfrm/xfrm_interface.c b/net/xfrm/xfrm_interface.c
index dc651a628dcf..3361e3ac5714 100644
--- a/net/xfrm/xfrm_interface.c
+++ b/net/xfrm/xfrm_interface.c
@@ -300,10 +300,10 @@ xfrmi_xmit2(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
 			if (mtu < IPV6_MIN_MTU)
 				mtu = IPV6_MIN_MTU;
 
-			icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
+			icmpv6_ndo_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
 		} else {
-			icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
-				  htonl(mtu));
+			icmp_ndo_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
+				      htonl(mtu));
 		}
 
 		dst_release(dst);
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 net 0/5] icmp: account for NAT when sending icmps from ndo layer
  2020-02-11 19:47 [PATCH v4 net 0/5] icmp: account for NAT when sending icmps from ndo layer Jason A. Donenfeld
                   ` (4 preceding siblings ...)
  2020-02-11 19:47 ` [PATCH v4 net 5/5] xfrm: interface: " Jason A. Donenfeld
@ 2020-02-13 22:19 ` David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2020-02-13 22:19 UTC (permalink / raw)
  To: Jason; +Cc: netdev, netfilter-devel

From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: Tue, 11 Feb 2020 20:47:04 +0100

> The ICMP routines use the source address for two reasons:
> 
> 1. Rate-limiting ICMP transmissions based on source address, so
>    that one source address cannot provoke a flood of replies. If
>    the source address is wrong, the rate limiting will be
>    incorrectly applied.
> 
> 2. Choosing the interface and hence new source address of the
>    generated ICMP packet. If the original packet source address
>    is wrong, ICMP replies will be sent from the wrong source
>    address, resulting in either a misdelivery, infoleak, or just
>    general network admin confusion.
> 
> Most of the time, the icmp_send and icmpv6_send routines can just reach
> down into the skb's IP header to determine the saddr. However, if
> icmp_send or icmpv6_send is being called from a network device driver --
> there are a few in the tree -- then it's possible that by the time
> icmp_send or icmpv6_send looks at the packet, the packet's source
> address has already been transformed by SNAT or MASQUERADE or some other
> transformation that CONNTRACK knows about. In this case, the packet's
> source address is most certainly the *wrong* source address to be used
> for the purpose of ICMP replies.
> 
> Rather, the source address we want to use for ICMP replies is the
> original one, from before the transformation occurred.
 ...

Series applied, thank you.


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-02-13 22:19 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-11 19:47 [PATCH v4 net 0/5] icmp: account for NAT when sending icmps from ndo layer Jason A. Donenfeld
2020-02-11 19:47 ` [PATCH v4 net 1/5] icmp: introduce helper for nat'd source address in network device context Jason A. Donenfeld
2020-02-11 19:47 ` [PATCH v4 net 2/5] gtp: use icmp_ndo_send helper Jason A. Donenfeld
2020-02-11 19:47 ` [PATCH v4 net 3/5] sunvnet: " Jason A. Donenfeld
2020-02-11 19:47 ` [PATCH v4 net 4/5] wireguard: device: " Jason A. Donenfeld
2020-02-11 19:47 ` [PATCH v4 net 5/5] xfrm: interface: " Jason A. Donenfeld
2020-02-13 22:19 ` [PATCH v4 net 0/5] icmp: account for NAT when sending icmps from ndo layer David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.