From 6995bcac620d2cb934826f79066763d94486ba86 Mon Sep 17 00:00:00 2001 From: Jan Macku Date: Wed, 28 Aug 2024 14:24:04 +0200 Subject: [PATCH] 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 --- ...t-pattern-matching-on-response-when-.patch | 119 ++++++++++++++ 007-ping-Move-ping_rts-multicast.patch | 36 +++++ ...y-from-Subnet-Router-anycast-address.patch | 133 ++++++++++++++++ ...reply-with-wrong-source-with-warning.patch | 149 ++++++++++++++++++ iputils.spec | 5 + 5 files changed, 442 insertions(+) create mode 100644 006-Revert-Add-strict-pattern-matching-on-response-when-.patch create mode 100644 007-ping-Move-ping_rts-multicast.patch create mode 100644 008-ping-Print-reply-from-Subnet-Router-anycast-address.patch create mode 100644 009-ping-Print-reply-with-wrong-source-with-warning.patch diff --git a/006-Revert-Add-strict-pattern-matching-on-response-when-.patch b/006-Revert-Add-strict-pattern-matching-on-response-when-.patch new file mode 100644 index 0000000..5329b7e --- /dev/null +++ b/006-Revert-Add-strict-pattern-matching-on-response-when-.patch @@ -0,0 +1,119 @@ +From 7d62be648f03adba22bcccd2eb3506d51169b85d Mon Sep 17 00:00:00 2001 +From: Petr Vorel +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 +Suggested-by: Paul Swirhun +Reviewed-by: Noah Meyerhans +Signed-off-by: Petr Vorel +(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 + diff --git a/007-ping-Move-ping_rts-multicast.patch b/007-ping-Move-ping_rts-multicast.patch new file mode 100644 index 0000000..8b28601 --- /dev/null +++ b/007-ping-Move-ping_rts-multicast.patch @@ -0,0 +1,36 @@ +From 1c66917a079e95f00e9a1f2af1329c65501e8a60 Mon Sep 17 00:00:00 2001 +From: Petr Vorel +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 +(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 + diff --git a/008-ping-Print-reply-from-Subnet-Router-anycast-address.patch b/008-ping-Print-reply-from-Subnet-Router-anycast-address.patch new file mode 100644 index 0000000..b8768cd --- /dev/null +++ b/008-ping-Print-reply-from-Subnet-Router-anycast-address.patch @@ -0,0 +1,133 @@ +From aa75473cac4a37cd673da3a66904878efcfdbd6f Mon Sep 17 00:00:00 2001 +From: Petr Vorel +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 +(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 + diff --git a/009-ping-Print-reply-with-wrong-source-with-warning.patch b/009-ping-Print-reply-with-wrong-source-with-warning.patch new file mode 100644 index 0000000..cb74bd7 --- /dev/null +++ b/009-ping-Print-reply-with-wrong-source-with-warning.patch @@ -0,0 +1,149 @@ +From ea2808dc199b89c12dd3b3a968e67bc0f522d399 Mon Sep 17 00:00:00 2001 +From: Petr Vorel +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 +Signed-off-by: Petr Vorel +(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 + diff --git a/iputils.spec b/iputils.spec index 92791fc..38afed1 100644 --- a/iputils.spec +++ b/iputils.spec @@ -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 Patch004: 004-arping-fix-typo-in-error-checking.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 Patch100: 100-iputils-ifenslave.patch @@ -137,6 +141,7 @@ install -cp ifenslave.8 ${RPM_BUILD_ROOT}%{_mandir}/man8/ * Wed Aug 28 2024 Jan Macku - 20210202-10 - 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) +- ping: Print reply with wrong source with warning & some follow-up fixes (RHEL-12789, RHEL-13480) * Wed May 03 2023 Jan Macku - 20210202-9 - ping: Remove 'unsupported IPv6' warning on disabled IPv6 (rhbz#2152511)