On 2022-02-18 12:17 +10, Jonathan Matthew <[email protected]> wrote:
> The only thing ping uses to determine whether a received icmp echo reply
> packet is a
> response to one of its requests is the 16 bit icmp ident field. If you ping
> enough
> stuff at the same time, eventually you'll have two concurrent pings using the
> same ident,
> and they will both see each other's replies. Since we do tricky MAC stuff on
> the ping
> payload, this results in signature mismatches that look like this:
>
> PING 172.23.94.210 (172.23.94.210): 56 data bytes
> 64 bytes from 172.23.94.210: icmp_seq=0 ttl=253 time=0.820 ms
> 64 bytes from 172.23.94.210: icmp_seq=1 ttl=253 time=0.419 ms
> 64 bytes from 172.23.94.210: icmp_seq=2 ttl=253 time=0.369 ms
> signature mismatch!
> 64 bytes from 172.23.94.210: icmp_seq=3 ttl=253 time=0.273 ms
>
> --- 172.23.94.210 ping statistics ---
> 4 packets transmitted, 5 packets received, -- somebody's duplicating packets!
> round-trip min/avg/max/std-dev = 0.273/0.376/0.820/0.265 ms
>
> ping is counting the packet with the signature mismatch as a reply it
> received, and it
> prints a misleading message about duplicated packets because it got more
> replies than
> the number of requests it sent.
>
> I think it would be more helpful not to count signature mismatch packets as
> replies.
> If you're actually getting corrupted replies, I'd say that's more like packet
> loss
> than normal operation. If you're getting extra replies due to ident
> collisions, this
> will result in ping sending and receiving the expected number of packets.
>
> Printing the source address and sequence number on signature mismatches would
> also help.
> I would have figured this out much quicker had ping told me the mismatch
> packets were
> from a completely different source. For example:
>
> PING 172.23.94.210 (172.23.94.210): 56 data bytes
> 64 bytes from 172.23.94.210: icmp_seq=0 ttl=253 time=2.645 ms
> 64 bytes from 172.23.94.210: icmp_seq=1 ttl=253 time=1.360 ms
> 64 bytes from 172.23.94.210: icmp_seq=2 ttl=253 time=0.506 ms
> 64 bytes from 172.23.94.210: icmp_seq=3 ttl=253 time=0.615 ms
> signature mismatch from 10.138.79.45: icmp_seq=0
> 64 bytes from 172.23.94.210: icmp_seq=4 ttl=253 time=0.431 ms
>
> --- 172.23.94.210 ping statistics ---
> 5 packets transmitted, 5 packets received, 0.0% packet loss
> round-trip min/avg/max/std-dev = 0.431/1.111/2.645/0.835 ms
>
> ok?
OK florian
I think we can go further and also check the from address in the echo
reply case, like this.
If something on the path is so confused as to answer to our pings with
the wrong source address I think it's tcpdump time...
Feel free to put this in at the same time if you agree.
diff --git sbin/ping/ping.c sbin/ping/ping.c
index 6fa634bca3e..e47baa8912c 100644
--- sbin/ping/ping.c
+++ sbin/ping/ping.c
@@ -181,6 +181,9 @@ char *hostname;
int ident; /* random number to identify our packets */
int v6flag; /* are we ping6? */
+struct sockaddr_in dst4;
+struct sockaddr_in6 dst6;
+
/* counters */
int64_t npackets; /* max packets to transmit */
int64_t nreceived; /* # of packets we got back */
@@ -243,8 +246,8 @@ main(int argc, char *argv[])
struct addrinfo hints, *res;
struct itimerval itimer;
struct sockaddr *from, *dst;
- struct sockaddr_in from4, dst4;
- struct sockaddr_in6 from6, dst6;
+ struct sockaddr_in from4;
+ struct sockaddr_in6 from6;
struct cmsghdr *scmsg = NULL;
struct in6_pktinfo *pktinfo = NULL;
struct icmp6_filter filt;
@@ -1285,6 +1288,13 @@ pr_pack(u_char *buf, int cc, struct msghdr *mhdr)
}
if (echo_reply) {
+ if (v6flag) {
+ if (memcmp(&dst6, from, sizeof(dst6)) != 0)
+ return; /* 'Twas not our ECHO */
+ } else {
+ if (memcmp(&dst4, from, sizeof(dst4)) != 0)
+ return; /* 'Twas not our ECHO */
+ }
++nreceived;
if (cc >= ECHOLEN + ECHOTMLEN) {
SIPHASH_CTX ctx;
@@ -1302,7 +1312,10 @@ pr_pack(u_char *buf, int cc, struct msghdr *mhdr)
if (timingsafe_memcmp(mac, &payload.mac,
sizeof(mac)) != 0) {
- printf("signature mismatch!\n");
+ printf("signature mismatch from %s: "
+ "icmp_seq=%u\n", pr_addr(from, fromlen),
+ ntohs(seq));
+ --nreceived;
return;
}
timinginfo=1;
>
> Index: ping.c
> ===================================================================
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.245
> diff -u -p -r1.245 ping.c
> --- ping.c 12 Jul 2021 15:09:19 -0000 1.245
> +++ ping.c 18 Feb 2022 01:52:22 -0000
> @@ -1302,7 +1302,10 @@ pr_pack(u_char *buf, int cc, struct msgh
>
> if (timingsafe_memcmp(mac, &payload.mac,
> sizeof(mac)) != 0) {
> - printf("signature mismatch!\n");
> + printf("signature mismatch from %s: "
> + "icmp_seq=%u\n", pr_addr(from, fromlen),
> + ntohs(seq));
> + --nreceived;
> return;
> }
> timinginfo=1;
>
--
I'm not entirely sure you are real.
