ping: Print reply with wrong source with warning

Plus some follow-up fixes:
- ping: Print reply from Subnet-Router anycast address
- Revert "Add strict pattern matching on response when pattern was provided

Resolves: RHEL-12789, RHEL-13480
This commit is contained in:
Jan Macku 2024-08-28 14:24:04 +02:00
parent a760ddd03d
commit 6995bcac62
5 changed files with 442 additions and 0 deletions

View File

@ -0,0 +1,119 @@
From 7d62be648f03adba22bcccd2eb3506d51169b85d Mon Sep 17 00:00:00 2001
From: Petr Vorel <pvorel@suse.cz>
Date: Thu, 15 Apr 2021 07:48:03 +0200
Subject: [PATCH 1/3] Revert "Add strict pattern matching on response when
pattern was provided"
This reverts commit f7710a17c4d5994313a64583f511bcdb9559f2a9.
Commit broke report of truncated packets:
$ ping -c2 -s100 google.com
PING google.com (142.250.185.238) 100(128) bytes of data.
Running ping from both s20161105 (which does not contain f7710a1) and
reverted f7710a1 on master reports truncated packets:
$ ping -c2 -s100 google.com
PING google.com (142.250.185.238) 100(128) bytes of data.
76 bytes from fra16s53-in-f14.1e100.net (142.250.185.238): icmp_seq=1 ttl=116 (truncated)
76 bytes from fra16s53-in-f14.1e100.net (142.250.185.238): icmp_seq=2 ttl=116 (truncated)
There was unreachable code in gather_statistics() because
contains_pattern_in_payload() added in f7710a1 always found a mismatch
first. Due that all of these did not work:
* updating counters for statistics generation
* keeping track of timestamps and time-of-flight using the first section
of the payload
* checking for duplicate replies and report them
* printing basic info about the reply
* printing "(truncated)" if the reply was truncated
* checking the checksum
* validating the rest of the payload (bytes 17 and above) against the
ICMP request that was sent, and report any differences
Fixes: f7710a1 ("Add strict pattern matching on response when pattern was provided")
Closes: https://github.com/iputils/iputils/issues/320
Closes: https://github.com/iputils/iputils/pull/331
Reported-by: Paul Swirhun <paulswirhun@gmail.com>
Suggested-by: Paul Swirhun <paulswirhun@gmail.com>
Reviewed-by: Noah Meyerhans <noahm@debian.org>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
(cherry picked from commit dff5d82dadab1b04400b2f9e1eb10a0d124868ed)
---
ping/ping.c | 2 --
ping/ping.h | 1 -
ping/ping6_common.c | 2 --
ping/ping_common.c | 18 ------------------
4 files changed, 23 deletions(-)
diff --git a/ping/ping.c b/ping/ping.c
index 1c98733..0655bf4 100644
--- a/ping/ping.c
+++ b/ping/ping.c
@@ -1549,8 +1549,6 @@ int ping4_parse_reply(struct ping_rts *rts, struct socket_st *sock,
return 1;
if (!is_ours(rts, sock, icp->un.echo.id))
return 1; /* 'Twas not our ECHO */
- if (!contains_pattern_in_payload(rts, (uint8_t *)(icp + 1)))
- return 1; /* 'Twas really not our ECHO */
if (gather_statistics(rts, (uint8_t *)icp, sizeof(*icp), cc,
ntohs(icp->un.echo.sequence),
reply_ttl, 0, tv, pr_addr(rts, from, sizeof *from),
diff --git a/ping/ping.h b/ping/ping.h
index c8bbcf6..86652bf 100644
--- a/ping/ping.h
+++ b/ping/ping.h
@@ -380,7 +380,6 @@ int is_ours(struct ping_rts *rts, socket_st *sock, uint16_t id);
extern int pinger(struct ping_rts *rts, ping_func_set_st *fset, socket_st *sock);
extern void sock_setbufs(struct ping_rts *rts, socket_st *, int alloc);
extern void setup(struct ping_rts *rts, socket_st *);
-extern int contains_pattern_in_payload(struct ping_rts *rts, uint8_t *ptr);
extern int main_loop(struct ping_rts *rts, ping_func_set_st *fset, socket_st*,
uint8_t *packet, int packlen);
extern int finish(struct ping_rts *rts);
diff --git a/ping/ping6_common.c b/ping/ping6_common.c
index 459f63e..fcb48be 100644
--- a/ping/ping6_common.c
+++ b/ping/ping6_common.c
@@ -823,8 +823,6 @@ int ping6_parse_reply(struct ping_rts *rts, socket_st *sock,
return 1;
if (!is_ours(rts, sock, icmph->icmp6_id))
return 1;
- if (!contains_pattern_in_payload(rts, (uint8_t *)(icmph + 1)))
- return 1; /* 'Twas really not our ECHO */
if (gather_statistics(rts, (uint8_t *)icmph, sizeof(*icmph), cc,
ntohs(icmph->icmp6_seq),
hops, 0, tv, pr_addr(rts, from, sizeof *from),
diff --git a/ping/ping_common.c b/ping/ping_common.c
index 33e6003..357c39d 100644
--- a/ping/ping_common.c
+++ b/ping/ping_common.c
@@ -553,24 +553,6 @@ void setup(struct ping_rts *rts, socket_st *sock)
}
}
-/*
- * Return 0 if pattern in payload point to be ptr did not match the pattern that was sent
- */
-int contains_pattern_in_payload(struct ping_rts *rts, uint8_t *ptr)
-{
- size_t i;
- uint8_t *cp, *dp;
-
- /* check the data */
- cp = ((u_char *)ptr) + sizeof(struct timeval);
- dp = &rts->outpack[8 + sizeof(struct timeval)];
- for (i = sizeof(struct timeval); i < rts->datalen; ++i, ++cp, ++dp) {
- if (*cp != *dp)
- return 0;
- }
- return 1;
-}
-
int main_loop(struct ping_rts *rts, ping_func_set_st *fset, socket_st *sock,
uint8_t *packet, int packlen)
{
--
2.46.0

View File

@ -0,0 +1,36 @@
From 1c66917a079e95f00e9a1f2af1329c65501e8a60 Mon Sep 17 00:00:00 2001
From: Petr Vorel <pvorel@suse.cz>
Date: Mon, 18 Oct 2021 15:27:35 +0200
Subject: [PATCH 1/4] ping: Move ping_rts->multicast
as it's used for both IPv4 and IPv6.
Signed-off-by: Petr Vorel <pvorel@suse.cz>
(cherry picked from commit 7a4ec7532871772cb22a3b2c42f6006f95f8b263)
---
ping/ping.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ping/ping.h b/ping/ping.h
index 703296d..a5f05f4 100644
--- a/ping/ping.h
+++ b/ping/ping.h
@@ -192,6 +192,7 @@ struct ping_rts {
struct sockaddr_in6 source6;
struct sockaddr_in6 whereto6;
struct sockaddr_in6 firsthop6;
+ int multicast;
/* Used only in ping.c */
int ts_type;
@@ -201,7 +202,6 @@ struct ping_rts {
int optlen;
int settos; /* Set TOS, Precedence or other QOS options */
int broadcast_pings;
- int multicast;
struct sockaddr_in source;
/* Used only in ping_common.c */
--
2.46.0

View File

@ -0,0 +1,133 @@
From aa75473cac4a37cd673da3a66904878efcfdbd6f Mon Sep 17 00:00:00 2001
From: Petr Vorel <pvorel@suse.cz>
Date: Mon, 18 Oct 2021 15:13:44 +0200
Subject: [PATCH 2/3] ping: Print reply from Subnet-Router anycast address
by detecting Subnet-Router address for 64 bit prefix and suppress
address comparison check.
5e052ad ("ping: discard packets with wrong source address") correctly
hid replies with wrong source address to comply RFC 1122 (Section
3.2.1.3: "The IP source address in an ICMP Echo Reply MUST be the same
as the specific-destination address").
While change in 5e052ad works for broadcast and multicast addresses and
some of anycast addresses, it does not work for (at least) Subnet-Router
anycast address):
# VETH1_IPV6=fd00:dead:beef:1234::1
# VPEER1_IPV6=fd00:dead:beef:1234::2
# ip netns add ns-ipv6
# ip li add name veth1 type veth peer name vpeer1
# ip -6 addr add $VETH1_IPV6/64 dev veth1
# ip li set dev veth1 up
# ip li set dev vpeer1 netns ns-ipv6
# ip netns exec ns-ipv6 ip li set dev lo up
# ip netns exec ns-ipv6 ip -6 addr add $VPEER1_IPV6/64 dev vpeer1
# ip netns exec ns-ipv6 ip li set vpeer1 up
# ip netns exec ns-ipv6 ip -6 route add default dev vpeer1 via $VETH1_IPV6
# sysctl -w net.ipv6.conf.all.forwarding=1
$ ping -c1 ff02::1 # anycast - all nodes
PING ff02::1(ff02::1) 56 data bytes
64 bytes from fe80::9c9c:ffff:fe14:e9d2%vpeer1: icmp_seq=1 ttl=64 time=0.064 ms
$ ping -c1 ff02::2 # anycast - all routers
PING ff02::2(ff02::2) 56 data bytes
64 bytes from fe80::5496:9ff:fef5:8f01%vpeer1: icmp_seq=1 ttl=64 time=0.088 ms
$ ping -c1 -W5 fd00:dead:beef:1234:: # Subnet-Router anycast
PING fd00:dead:beef:1234::(fd00:dead:beef:1234::) 56 data bytes
Subnet-Router anycast address works for both busybox ping (without
printing the real source address) and fping:
$ busybox ping -c1 fd00:dead:beef:1234::
PING fd00:dead:beef:1234:: (fd00:dead:beef:1234::): 56 data bytes
64 bytes from fd00:dead:beef:1234::1: seq=0 ttl=64 time=0.122 ms
$ fping -c1 fd00:dead:beef:1234::
[<- fd00:dead:beef:1234::1]fd00:dead:beef:1234:: : [0], 64 bytes, 0.096 ms (0.096 avg, 0% loss)
RFC 4291 specifies Subnet-Router anycast address as [1]:
The Subnet-Router anycast address is predefined. Its format is as
follows:
| n bits | 128-n bits |
+------------------------------------------------+----------------+
| subnet prefix | 00000000000000 |
+------------------------------------------------+----------------+
The "subnet prefix" in an anycast address is the prefix that
identifies a specific link. This anycast address is syntactically
the same as a unicast address for an interface on the link with the
interface identifier set to zero.
=> to detect Subnet-Router anycast address we need to know prefix, which
we don't know, thus detect it for prefix 64 (the default IPv6 prefix).
[1] https://datatracker.ietf.org/doc/html/rfc4291#section-2.6.1
Fixes: 5e052ad ("ping: discard packets with wrong source address")
Closes: https://github.com/iputils/iputils/issues/371
Reported-by: Tim Sandquist
Signed-off-by: Petr Vorel <pvorel@suse.cz>
(cherry picked from commit 15a5e5c7aace5a7a782ff802988e04ed4c1148a5)
---
ping/ping.h | 1 +
ping/ping6_common.c | 12 +++++++++++-
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/ping/ping.h b/ping/ping.h
index 86652bf..f26fdac 100644
--- a/ping/ping.h
+++ b/ping/ping.h
@@ -212,6 +212,7 @@ struct ping_rts {
#endif
/* Used only in ping6_common.c */
+ int subnet_router_anycast; /* Subnet-Router anycast (RFC 4291) */
struct sockaddr_in6 firsthop;
unsigned char cmsgbuf[4096];
size_t cmsglen;
diff --git a/ping/ping6_common.c b/ping/ping6_common.c
index fcb48be..d0d2d84 100644
--- a/ping/ping6_common.c
+++ b/ping/ping6_common.c
@@ -101,6 +101,7 @@ int ping6_run(struct ping_rts *rts, int argc, char **argv, struct addrinfo *ai,
struct socket_st *sock)
{
int hold, packlen;
+ size_t i;
unsigned char *packet;
char *target;
struct icmp6_filter filter;
@@ -247,6 +248,15 @@ int ping6_run(struct ping_rts *rts, int argc, char **argv, struct addrinfo *ai,
rts->pmtudisc = IPV6_PMTUDISC_DO;
}
+ /* detect Subnet-Router anycast at least for the default prefix 64 */
+ rts->subnet_router_anycast = 1;
+ for (i = 8; i < sizeof(struct in6_addr); i++) {
+ if (rts->whereto6.sin6_addr.s6_addr[i]) {
+ rts->subnet_router_anycast = 0;
+ break;
+ }
+ }
+
if (rts->pmtudisc >= 0) {
if (setsockopt(sock->fd, IPPROTO_IPV6, IPV6_MTU_DISCOVER, &rts->pmtudisc,
sizeof rts->pmtudisc) == -1)
@@ -818,7 +828,7 @@ int ping6_parse_reply(struct ping_rts *rts, socket_st *sock,
}
if (icmph->icmp6_type == ICMP6_ECHO_REPLY) {
- if (!rts->multicast &&
+ if (!rts->multicast && !rts->subnet_router_anycast &&
memcmp(&from->sin6_addr.s6_addr, &rts->whereto6.sin6_addr.s6_addr, 16))
return 1;
if (!is_ours(rts, sock, icmph->icmp6_id))
--
2.46.0

View File

@ -0,0 +1,149 @@
From ea2808dc199b89c12dd3b3a968e67bc0f522d399 Mon Sep 17 00:00:00 2001
From: Petr Vorel <pvorel@suse.cz>
Date: Fri, 15 Oct 2021 17:38:51 +0200
Subject: [PATCH 3/3] ping: Print reply with wrong source with warning
5e052ad ("ping: discard packets with wrong source address") correctly
hid replies with wrong source address to comply RFC 1122 (Section
3.2.1.3: "The IP source address in an ICMP Echo Reply MUST be the same
as the specific-destination address").
This caused to hide reply when pinging Subnet-Router anycast address.
Although it was fixed in the previous commit, relax this to admit the
reply but print warning "DIFFERENT ADDRESS!". ping is diagnostic program,
with insisting on RFC we force people to use tcpdump to see replies.
Link: https://github.com/iputils/iputils/issues/371
Reviewed-by: Matteo Croce <mcroce@microsoft.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
(cherry picked from commit 5f6bec5ab57cc8beaa78f5756a0ffbdf01f28d36)
---
ping/ping.c | 10 ++++++----
ping/ping.h | 3 ++-
ping/ping6_common.c | 13 ++++++++-----
ping/ping_common.c | 6 +++++-
4 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/ping/ping.c b/ping/ping.c
index 0655bf4..81ee7c8 100644
--- a/ping/ping.c
+++ b/ping/ping.c
@@ -1504,6 +1504,7 @@ int ping4_parse_reply(struct ping_rts *rts, struct socket_st *sock,
int reply_ttl;
uint8_t *opts, *tmp_ttl;
int olen;
+ int wrong_source = 0;
/* Check the IP header */
ip = (struct iphdr *)buf;
@@ -1544,15 +1545,16 @@ int ping4_parse_reply(struct ping_rts *rts, struct socket_st *sock,
csfailed = in_cksum((unsigned short *)icp, cc, 0);
if (icp->type == ICMP_ECHOREPLY) {
- if (!rts->broadcast_pings && !rts->multicast &&
- from->sin_addr.s_addr != rts->whereto.sin_addr.s_addr)
- return 1;
if (!is_ours(rts, sock, icp->un.echo.id))
return 1; /* 'Twas not our ECHO */
+
+ if (!rts->broadcast_pings && !rts->multicast &&
+ from->sin_addr.s_addr != rts->whereto.sin_addr.s_addr)
+ wrong_source = 1;
if (gather_statistics(rts, (uint8_t *)icp, sizeof(*icp), cc,
ntohs(icp->un.echo.sequence),
reply_ttl, 0, tv, pr_addr(rts, from, sizeof *from),
- pr_echo_reply, rts->multicast)) {
+ pr_echo_reply, rts->multicast, wrong_source)) {
fflush(stdout);
return 0;
}
diff --git a/ping/ping.h b/ping/ping.h
index f26fdac..703296d 100644
--- a/ping/ping.h
+++ b/ping/ping.h
@@ -389,7 +389,8 @@ extern void common_options(int ch);
extern int gather_statistics(struct ping_rts *rts, uint8_t *icmph, int icmplen,
int cc, uint16_t seq, int hops,
int csfailed, struct timeval *tv, char *from,
- void (*pr_reply)(uint8_t *ptr, int cc), int multicast);
+ void (*pr_reply)(uint8_t *ptr, int cc), int multicast,
+ int wrong_source);
extern void print_timestamp(struct ping_rts *rts);
void fill(struct ping_rts *rts, char *patp, unsigned char *packet, size_t packet_size);
diff --git a/ping/ping6_common.c b/ping/ping6_common.c
index d0d2d84..4712928 100644
--- a/ping/ping6_common.c
+++ b/ping/ping6_common.c
@@ -802,6 +802,7 @@ int ping6_parse_reply(struct ping_rts *rts, socket_st *sock,
struct cmsghdr *c;
struct icmp6_hdr *icmph;
int hops = -1;
+ int wrong_source = 0;
for (c = CMSG_FIRSTHDR(msg); c; c = CMSG_NXTHDR(msg, c)) {
if (c->cmsg_level != IPPROTO_IPV6)
@@ -828,16 +829,18 @@ int ping6_parse_reply(struct ping_rts *rts, socket_st *sock,
}
if (icmph->icmp6_type == ICMP6_ECHO_REPLY) {
- if (!rts->multicast && !rts->subnet_router_anycast &&
- memcmp(&from->sin6_addr.s6_addr, &rts->whereto6.sin6_addr.s6_addr, 16))
- return 1;
if (!is_ours(rts, sock, icmph->icmp6_id))
return 1;
+
+ if (!rts->multicast && !rts->subnet_router_anycast &&
+ memcmp(&from->sin6_addr.s6_addr, &rts->whereto6.sin6_addr.s6_addr, 16))
+ wrong_source = 1;
+
if (gather_statistics(rts, (uint8_t *)icmph, sizeof(*icmph), cc,
ntohs(icmph->icmp6_seq),
hops, 0, tv, pr_addr(rts, from, sizeof *from),
pr_echo_reply,
- rts->multicast)) {
+ rts->multicast, wrong_source)) {
fflush(stdout);
return 0;
}
@@ -850,7 +853,7 @@ int ping6_parse_reply(struct ping_rts *rts, socket_st *sock,
seq,
hops, 0, tv, pr_addr(rts, from, sizeof *from),
pr_niquery_reply,
- rts->multicast))
+ rts->multicast, 0))
return 0;
} else {
int nexthdr;
diff --git a/ping/ping_common.c b/ping/ping_common.c
index 357c39d..0336259 100644
--- a/ping/ping_common.c
+++ b/ping/ping_common.c
@@ -711,7 +711,8 @@ int main_loop(struct ping_rts *rts, ping_func_set_st *fset, socket_st *sock,
int gather_statistics(struct ping_rts *rts, uint8_t *icmph, int icmplen,
int cc, uint16_t seq, int hops,
int csfailed, struct timeval *tv, char *from,
- void (*pr_reply)(uint8_t *icmph, int cc), int multicast)
+ void (*pr_reply)(uint8_t *icmph, int cc), int multicast,
+ int wrong_source)
{
int dupflag = 0;
long triptime = 0;
@@ -804,10 +805,13 @@ restamp:
printf(_(" time=%ld.%03ld ms"), triptime / 1000,
triptime % 1000);
}
+
if (dupflag && (!multicast || rts->opt_verbose))
printf(_(" (DUP!)"));
if (csfailed)
printf(_(" (BAD CHECKSUM!)"));
+ if (wrong_source)
+ printf(_(" (DIFFERENT ADDRESS!)"));
/* check the data */
cp = ((unsigned char *)ptr) + sizeof(struct timeval);
--
2.46.0

View File

@ -24,6 +24,10 @@ Patch002: 002-arping-Fix-1s-delay-on-exit-for-unsolicited-arpings.patch
Patch003: 003-arping-Fix-unsolicited-ARP-regressions-on-c-1.patch Patch003: 003-arping-Fix-unsolicited-ARP-regressions-on-c-1.patch
Patch004: 004-arping-fix-typo-in-error-checking.patch Patch004: 004-arping-fix-typo-in-error-checking.patch
Patch005: 005-arping-exit-0-if-running-in-deadline-mode-and-we-see.patch Patch005: 005-arping-exit-0-if-running-in-deadline-mode-and-we-see.patch
Patch006: 006-Revert-Add-strict-pattern-matching-on-response-when-.patch
Patch010: 007-ping-Move-ping_rts-multicast.patch
Patch007: 008-ping-Print-reply-from-Subnet-Router-anycast-address.patch
Patch008: 009-ping-Print-reply-with-wrong-source-with-warning.patch
# Downstream-only patches # Downstream-only patches
Patch100: 100-iputils-ifenslave.patch Patch100: 100-iputils-ifenslave.patch
@ -137,6 +141,7 @@ install -cp ifenslave.8 ${RPM_BUILD_ROOT}%{_mandir}/man8/
* Wed Aug 28 2024 Jan Macku <jamacku@redhat.com> - 20210202-10 * Wed Aug 28 2024 Jan Macku <jamacku@redhat.com> - 20210202-10
- arping: Fix 1s delay on exit for unsolicited arpings (RHEL-34110) - arping: Fix 1s delay on exit for unsolicited arpings (RHEL-34110)
- arping: exit 0 if running in deadline mode and we see replies (RHEL-27718) - arping: exit 0 if running in deadline mode and we see replies (RHEL-27718)
- ping: Print reply with wrong source with warning & some follow-up fixes (RHEL-12789, RHEL-13480)
* Wed May 03 2023 Jan Macku <jamacku@redhat.com> - 20210202-9 * Wed May 03 2023 Jan Macku <jamacku@redhat.com> - 20210202-9
- ping: Remove 'unsupported IPv6' warning on disabled IPv6 (rhbz#2152511) - ping: Remove 'unsupported IPv6' warning on disabled IPv6 (rhbz#2152511)