Debian: Patch ping for replies with wrong source

This commit patches iputils-ping in Debian-based StarlingX so that ping
prints out ICMP echo replies with unexpected/"wrong" source IP addresses
as was done with CentOS 7's version of ping and as is done with more
recent versions of the iputils package. As an example, if the user calls
"ping 192.168.1.1" and the ICMP echo responses are sent by 192.168.1.3,
then the version of ping in Debian 11 currently assumes that no
responses have been received. This issue was discovered while running
the eBPF test cases in the kernel's samples/bpf source sub-directory.

This issue was introduced upstream with the following commit:
  5e052ada96c3 ("ping: discard packets with wrong source address")
  https://github.com/iputils/iputils/commit/5e052ada96c3

The following commit fixes this issue:
  5f6bec5ab57c ("ping: Print reply with wrong source with warning")
  https://github.com/iputils/iputils/commit/5f6bec5ab57c

Due to a number of contextual dependencies, we unfortunately cannot
directly cherry-pick the commit that fixes this issue. Hence, we
cherry-pick four patches in total. We did not opt for completely
uprevisioning the iputils package to be able to inherit bug fixes that
Debian may introduce in the future.

We checked the list of commits between the two tags 20210202 and
20211215 using the following command:

  git log --oneline refs/tags/20210202..origin/master -- 'ping/*.[ch]'

And we did not find any commits that fix issues in the commits that we
are cherry-picking with this commit.

Testing:
- An ISO image with the standard kernel was successfully incrementally
  built with this commit.
- The resulting ISO image was installed onto a qemu/KVM-based virtual
  machine and the resulting system was bootstrapped with Ansible and
  unlocked.
- The test cases shipped in the kernel repository's samples/bpf
  sub-directory were successfully executed in the virtual machine. These
  test cases make use of ping in IPv4 and IPv6 mode, which ought to be a
  good sanity test.

Partial-Bug: 1977849

Change-Id: I25b42b8de80deec86be0eb504442992863a8d833
Signed-off-by: M. Vefa Bicakci <vefa.bicakci@windriver.com>
This commit is contained in:
M. Vefa Bicakci 2022-05-30 19:49:32 +00:00
parent 414b9934e1
commit 829e0c21e7
8 changed files with 456 additions and 0 deletions

View File

@ -5,3 +5,6 @@ puppet
drbd-tools
parted
trident-installer
iputils-arping
iputils-ping
iputils-tracepath

View File

@ -67,6 +67,7 @@ ldap/ldapscripts
ldap/openldap
livepatch/kpatch
networking/ifupdown-extra
networking/iputils
networking/lldpd
networking/net-tools
ostree/initramfs-ostree

View File

@ -0,0 +1,7 @@
---
debver: 20210202-1
debname: iputils
archive: https://snapshot.debian.org/archive/debian/20220530T153822Z/pool/main/i/iputils/
revision:
dist: $STX_DIST
PKG_GITREVCOUNT: true

View File

@ -0,0 +1,120 @@
From e7384da3a54416a8f3ef6cab187452b165ef9aa1 Mon Sep 17 00:00:00 2001
From: Petr Vorel <pvorel@suse.cz>
Date: Thu, 15 Apr 2021 07:48:03 +0200
Subject: [PATCH] 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)
Signed-off-by: M. Vefa Bicakci <vefa.bicakci@windriver.com>
---
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 38012c2c262c..6fcb44f2d7f5 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 c8bbcf6a8728..86652bf6774f 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 459f63e26bbf..fcb48beb065a 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 33e6003366c3..357c39d79013 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.29.2

View File

@ -0,0 +1,37 @@
From d62d37a1b9222983ed2a284cf9d4599961d6ce8f Mon Sep 17 00:00:00 2001
From: Petr Vorel <pvorel@suse.cz>
Date: Mon, 18 Oct 2021 15:27:35 +0200
Subject: [PATCH] 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)
Signed-off-by: M. Vefa Bicakci <vefa.bicakci@windriver.com>
---
ping/ping.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ping/ping.h b/ping/ping.h
index 86652bf6774f..8fb36670b1f1 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.29.2

View File

@ -0,0 +1,134 @@
From 1a869e19893de4a561c3c9420637feccb59cfa1c Mon Sep 17 00:00:00 2001
From: Petr Vorel <pvorel@suse.cz>
Date: Mon, 18 Oct 2021 15:13:44 +0200
Subject: [PATCH] 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)
Signed-off-by: M. Vefa Bicakci <vefa.bicakci@windriver.com>
---
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 8fb36670b1f1..e357d73a9a10 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 fcb48beb065a..d0d2d8489a26 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.29.2

View File

@ -0,0 +1,150 @@
From 77241783e710b28fa898da48e9aa0bef2db7971d Mon Sep 17 00:00:00 2001
From: Petr Vorel <pvorel@suse.cz>
Date: Fri, 15 Oct 2021 17:38:51 +0200
Subject: [PATCH] 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)
Signed-off-by: M. Vefa Bicakci <vefa.bicakci@windriver.com>
---
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 6fcb44f2d7f5..3bd9e3179459 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 e357d73a9a10..a5f05f4f0e36 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 d0d2d8489a26..4712928b9c75 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 357c39d79013..03362590ce18 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.29.2

View File

@ -0,0 +1,4 @@
0001-Revert-Add-strict-pattern-matching-on-response-when-.patch
0002-ping-Move-ping_rts-multicast.patch
0003-ping-Print-reply-from-Subnet-Router-anycast-address.patch
0004-ping-Print-reply-with-wrong-source-with-warning.patch