Re: ping(8) picking up another ping's echo replies

[Edgar Fuß <ef%math.uni-bonn.de@localhost>]

> 3. Ignore the ping(8) problem and teach the check_ping monitoring plugin to 
>    return CRITICAL if it finds "sendto: host is down" in ping's stderr.
> I went for this one for now.
While this somewhat mitigates the problem, it does not fix it. I still got 
check_ping to believe a host is up that's powered off, disconnected, and in 
my drawer.

> Nevertheless, I think that's a problem with ping(8) t[ha]t should be fixed.
I guess the cleanest solution would be a PRNG shared by all ping(8) instances 
and nobody else. I guess we don't have something like that, do we?

I looked at what others do. Both FreeBSD and iputils are using the PID. 
I think we should either revert to that (for me, the OpenBSD "fix" to use a 
random number instead trades in usability for some rather abstract idea of 
security) or at least superficially check the sender's address to match what 
we expect as in the attached patch (if sending to a multicast address, accept 
anything, otherwise the replying address must not have any bits set which are 
clear in the address we sent to).

Opinions?

Index: ping.c
===================================================================
RCS file: /cvsroot/src/sbin/ping/ping.c,v
retrieving revision 1.102.2.2
diff -u -p -r1.102.2.2 ping.c
--- ping.c	11 Jul 2017 21:16:07 -0000	1.102.2.2
+++ ping.c	13 Feb 2018 16:32:22 -0000
@@ -1030,8 +1030,17 @@ pr_pack(u_char *buf,
 	dumped = 0;
 	net_len = tot_len - hlen;
 	icp = (struct icmp *)(buf + hlen);
+	/*
+	 * Unlesswe sent to a multicast address, the replying address must not
+	 * have any bits set which are clear in the (potentionally broadcast)
+	 * address we sent to
+	 */
 	if (icp->icmp_type == ICMP_ECHOREPLY
-	    && icp->icmp_id == ident) {
+	    && icp->icmp_id == ident
+	    && ((pingflags | F_MCAST) ||
+	        (((uint32_t)whereto.sin_addr.s_addr
+	        | (uint32_t)from->sin_addr.s_addr)
+	        == (uint32_t)whereto.sin_addr.s_addr))) {
 		struct timespec tv;
 
 		if (icp->icmp_seq == htons((u_int16_t)(ntransmitted-1)))