Version in base suite: 20180629-2 Base version: iputils_20180629-2 Target version: iputils_20180629-2+deb10u1 Base file: /srv/ftp-master.debian.org/ftp/pool/main/i/iputils/iputils_20180629-2.dsc Target file: /srv/ftp-master.debian.org/policy/pool/main/i/iputils/iputils_20180629-2+deb10u1.dsc changelog | 9 patches/ping-fix-main-loop-over-multiple-addrinfo-results.patch | 84 ++++ patches/ping-try-next-addrinfo-on-connect-failure.patch | 187 ++++++++++ patches/series | 2 4 files changed, 282 insertions(+) diff -Nru iputils-20180629/debian/changelog iputils-20180629/debian/changelog --- iputils-20180629/debian/changelog 2018-08-03 16:53:09.000000000 +0000 +++ iputils-20180629/debian/changelog 2020-01-13 23:29:01.000000000 +0000 @@ -1,3 +1,12 @@ +iputils (3:20180629-2+deb10u1) buster; urgency=medium + + * Incorporate patches from Benjamin Poirier to + correct an issue in which ping would improperly exit with a failure code + when there were untried addresses still available in the getaddrinfo() + library call return value. (Closes: #947921) + + -- Noah Meyerhans Mon, 13 Jan 2020 15:29:01 -0800 + iputils (3:20180629-2) unstable; urgency=medium * Upstream no longer maintains the RELNOTES or README files in a useful diff -Nru iputils-20180629/debian/patches/ping-fix-main-loop-over-multiple-addrinfo-results.patch iputils-20180629/debian/patches/ping-fix-main-loop-over-multiple-addrinfo-results.patch --- iputils-20180629/debian/patches/ping-fix-main-loop-over-multiple-addrinfo-results.patch 1970-01-01 00:00:00.000000000 +0000 +++ iputils-20180629/debian/patches/ping-fix-main-loop-over-multiple-addrinfo-results.patch 2020-01-13 23:29:01.000000000 +0000 @@ -0,0 +1,84 @@ +From 769a6790437e883d72eebbabd06d33a05a0340ca Mon Sep 17 00:00:00 2001 +From: Benjamin Poirier +Date: Thu, 26 Dec 2019 10:44:03 +0900 +Subject: [PATCH 1/2] ping: fix main loop over multiple addrinfo results + +Despite what the log of commit f68eec0eafad ("ping: perform dual-stack ping +by default") says, main() was not designed to loop over multiple addresses +returned by getaddrinfo(). This is apparent because until commit +db11bc96a68c ("ping: make command to return from main()"), ping{4,6}_run() +never returned (they always exited). After commit db11bc96a68c, we +encounter unexpected situations if getaddrinfo returns multiple results and +ping{4,6}_run() return != 0. + +For example (notice echo reply is not received): + + root@vsid:/src/iputils# ./builddir/ping/ping -w1 google.com + PING google.com(nrt12s22-in-x0e.1e100.net (2404:6800:4004:80c::200e)) 56 data bytes + + --- google.com ping statistics --- + 1 packets transmitted, 0 received, 100% packet loss, time 0ms + + PING (216.58.197.142) 56(84) bytes of data. + + --- ping statistics --- + 1 packets transmitted, 0 received, 100% packet loss, time -1002ms + + root@vsid:/src/iputils# + +Establish the following convention: + +* return value >= 0 -> exit with this code (same behavior as before commit + db11bc96a68c) + +* return value < 0 -> go on to next addrinfo result + +The second case will be used in the following patch. + +Fixes: db11bc96a68c ("ping: make command to return from main()") +Signed-off-by: Benjamin Poirier +--- + ping.c | 6 +++++- + ping6_common.c | 1 + + 2 files changed, 6 insertions(+), 1 deletion(-) + +diff --git a/ping.c b/ping.c +index 733477f..4d2de0f 100644 +--- a/ping.c ++++ b/ping.c +@@ -528,8 +528,11 @@ main(int argc, char **argv) + exit(2); + } + +- if (status == 0) ++ if (status >= 0) + break; ++ /* status < 0 means to go on to next addrinfo result, there ++ * better be one. */ ++ assert(ai->ai_next); + } + + freeaddrinfo(result); +@@ -537,6 +540,7 @@ main(int argc, char **argv) + return status; + } + ++/* return >= 0: exit with this code, < 0: go on to next addrinfo result */ + int ping4_run(int argc, char **argv, struct addrinfo *ai, socket_st *sock) + { + static const struct addrinfo hints = { .ai_family = AF_INET, .ai_protocol = IPPROTO_UDP, .ai_flags = getaddrinfo_flags }; +diff --git a/ping6_common.c b/ping6_common.c +index 5991c2a..1f88e26 100644 +--- a/ping6_common.c ++++ b/ping6_common.c +@@ -600,6 +600,7 @@ int niquery_option_handler(const char *opt_arg) + return ret; + } + ++/* return >= 0: exit with this code, < 0: go on to next addrinfo result */ + int ping6_run(int argc, char **argv, struct addrinfo *ai, struct socket_st *sock) + { + static const struct addrinfo hints = { .ai_family = AF_INET6, .ai_flags = getaddrinfo_flags }; +-- +2.25.0.rc0 + diff -Nru iputils-20180629/debian/patches/ping-try-next-addrinfo-on-connect-failure.patch iputils-20180629/debian/patches/ping-try-next-addrinfo-on-connect-failure.patch --- iputils-20180629/debian/patches/ping-try-next-addrinfo-on-connect-failure.patch 1970-01-01 00:00:00.000000000 +0000 +++ iputils-20180629/debian/patches/ping-try-next-addrinfo-on-connect-failure.patch 2020-01-13 23:29:01.000000000 +0000 @@ -0,0 +1,187 @@ +From f0ee2f4f94560953d6027c57cdc484f23e59858a Mon Sep 17 00:00:00 2001 +From: Benjamin Poirier +Date: Wed, 25 Dec 2019 13:33:12 +0900 +Subject: [PATCH 2/2] ping: try next addrinfo on connect failure +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +On hosts that have routing rules matching on the outgoing interface [1], +getaddrinfo() may return results sorted in a suboptimal order because it is +not aware of the network interface passed to ping via the "-I" option. In +particular, address reachability detection may fail and getaddrinfo() will +return ipv6 results first, even though the only routes available are ipv4. + +Improve user experience by trying next addrinfo entry if we encounter a +failure at connect() time because of missing or unreachable routes. + +[1] For example, on switches running Cumulus Linux, the default VRF is used +for front ports and a "mgmt" VRF is used for the management interface, which +also handles all DNS traffic. (VRFs apply different routing rules based on +the iif/oif, ie. influenced by SO_BINDTODEVICE.) In the default vrf, it's +possible to ping an ipv4 address via the mgmt vrf by specifying "-I mgmt". +However, that will fail if the target host is specified by name, has a AAAA +record and there is no ipv6 route to it. + +Since libc commit 5ddb5bf5fb, getaddrinfo() does a udp connect to result +addresses to check if there is a route to them. This is to implement +RFC3484 ยง6 Rule 1 ("Avoid unusable destinations") which is part of the +algorithm to order results. getaddrinfo() is unaware of ping's "-I" option +and tries to connect its socket via the default vrf, which has no ipv6 route +to the target host (and, in fact, no ipv4 route either). Following this +failure, getaddrinfo() returns results ordered according to +/etc/gai.conf (Rule 6) - by default, ipv6 first. + +ping tries only the first entry returned by getaddrinfo() and fails to +connect to it because there is no ipv6 route to the host, even in the mgmt +vrf. However, if getaddrinfo() had ordered the ipv4 result first or ping +had tried the next addrinfo entry (the ipv4 one), ping could connect a udp +socket to it and later successfully exchange icmp messages with it. + +Example: + + cumulus@act-5812-10:~$ ip vrf list + Name Table + ----------------------- + mgmt 1001 + cumulus@act-5812-10:~$ ip vrf identify + cumulus@act-5812-10:~$ # --> default vrf + cumulus@act-5812-10:~$ + cumulus@act-5812-10:~$ ip rule + 99: from all to 10.230.0.53 ipproto udp dport 53 lookup mgmt + 99: from all to 10.20.249.1 ipproto udp dport 53 lookup mgmt + 1000: from all lookup [l3mdev-table] + 32765: from all lookup local + 32766: from all lookup main + 32767: from all lookup default + + cumulus@act-5812-10:~$ ip route + + cumulus@act-5812-10:~$ ip -6 route + ::1 dev lo proto kernel metric 256 pref medium + + cumulus@act-5812-10:~$ ip route show vrf mgmt + default via 10.230.130.1 dev eth0 + unreachable default metric 4278198272 + 10.230.130.0/24 dev eth0 proto kernel scope link src 10.230.130.211 + 127.0.0.0/8 dev mgmt proto kernel scope link src 127.0.0.1 + + cumulus@act-5812-10:~$ ip -6 route show vrf mgmt + ::1 dev mgmt proto kernel metric 256 pref medium + anycast fe80:: dev eth0 proto kernel metric 0 pref medium + fe80::/64 dev eth0 proto kernel metric 256 pref medium + ff00::/8 dev eth0 metric 256 pref medium + unreachable default dev lo metric 4278198272 pref medium + + cumulus@act-5812-10:~$ host google.com + google.com has address 172.217.0.46 + google.com has IPv6 address 2607:f8b0:4005:802::200e + google.com mail is handled by 30 alt2.aspmx.l.google.com. + google.com mail is handled by 40 alt3.aspmx.l.google.com. + google.com mail is handled by 20 alt1.aspmx.l.google.com. + google.com mail is handled by 10 aspmx.l.google.com. + google.com mail is handled by 50 alt4.aspmx.l.google.com. + +Success with numeric address + + cumulus@act-5812-10:~$ ping -n -c1 -I mgmt 172.217.0.46 + ping: Warning: source address might be selected on device other than mgmt. + PING 172.217.0.46 (172.217.0.46) from 10.230.130.211 mgmt: 56(84) bytes of data. + 64 bytes from 172.217.0.46: icmp_seq=1 ttl=51 time=4.68 ms + + --- 172.217.0.46 ping statistics --- + 1 packets transmitted, 1 received, 0% packet loss, time 0ms + rtt min/avg/max/mdev = 4.675/4.675/4.675/0.000 ms + +Failure with host by name + + cumulus@act-5812-10:~$ ping -n -c1 -I mgmt google.com + connect: No route to host + +Success when running in the mgmt vrf because getaddrinfo()'s address +reachability test is effective and ipv4 result(s) are ordered first. + + cumulus@act-5812-10:~$ ip vrf exec mgmt ping -n -c1 google.com + PING google.com (172.217.0.46) 56(84) bytes of data. + 64 bytes from 172.217.0.46: icmp_seq=1 ttl=51 time=4.65 ms + + --- google.com ping statistics --- + 1 packets transmitted, 1 received, 0% packet loss, time 0ms + rtt min/avg/max/mdev = 4.650/4.650/4.650/0.000 ms + +For demonstration purposes, the following configuration allows to reproduce +a similar problem. Starting from a host with a vanilla configuration, +default ipv4 route using eth0, no ipv6 global routes: + + root@vsid:~# ip route + default via 192.168.15.1 dev eth0 + 192.168.15.0/24 dev eth0 proto kernel scope link src 192.168.15.100 + + root@vsid:~# ip -6 route + ::1 dev lo proto kernel metric 256 pref medium + fe80::/64 dev eth0 proto kernel metric 256 pref medium + + root@vsid:~# ip rou flush table main + + root@vsid:~# ip rou add table 1 192.168.15.0/24 dev eth0 + + root@vsid:~# ip rou add table 1 default via 192.168.15.1 + + root@vsid:~# ip rule + 0: from all lookup local + 32766: from all lookup main + 32767: from all lookup default + root@vsid:~# ip rule add pref 1 to 192.168.15.1 ipproto udp dport 53 lookup 1 + root@vsid:~# ip rule add pref 2 oif eth0 lookup 1 + root@vsid:~# ping -c1 -I eth0 google.com + + ping: connect: Network is unreachable + +With the current patch + + root@vsid:~# /src/iputils/builddir/ping/ping -c1 -I eth0 google.com + PING (172.217.174.110) from 192.168.15.100 eth0: 56(84) bytes of data. + 64 bytes from nrt12s28-in-f14.1e100.net (172.217.174.110): icmp_seq=1 ttl=53 time=11.3 ms + + --- ping statistics --- + 1 packets transmitted, 1 received, 0% packet loss, time 0ms + rtt min/avg/max/mdev = 11.313/11.313/11.313/0.000 ms + +Signed-off-by: Benjamin Poirier +--- + ping.c | 3 +++ + ping6_common.c | 4 ++++ + 2 files changed, 7 insertions(+) + +diff --git a/ping.c b/ping.c +index 4d2de0f..a4d18c5 100644 +--- a/ping.c ++++ b/ping.c +@@ -671,6 +671,9 @@ int ping4_run(int argc, char **argv, struct addrinfo *ai, socket_st *sock) + perror("connect"); + exit(2); + } ++ } else if ((errno == EHOSTUNREACH || errno == ENETUNREACH) && ai->ai_next) { ++ close(probe_fd); ++ return -1; + } else { + perror("connect"); + exit(2); +diff --git a/ping6_common.c b/ping6_common.c +index 1f88e26..3b63337 100644 +--- a/ping6_common.c ++++ b/ping6_common.c +@@ -787,6 +787,10 @@ int ping6_run(int argc, char **argv, struct addrinfo *ai, struct socket_st *sock + + firsthop.sin6_port = htons(1025); + if (connect(probe_fd, (struct sockaddr*)&firsthop, sizeof(firsthop)) == -1) { ++ if ((errno == EHOSTUNREACH || errno == ENETUNREACH) && ai->ai_next) { ++ close(probe_fd); ++ return -1; ++ } + perror("connect"); + exit(2); + } +-- +2.25.0.rc0 + diff -Nru iputils-20180629/debian/patches/series iputils-20180629/debian/patches/series --- iputils-20180629/debian/patches/series 2018-08-03 16:53:09.000000000 +0000 +++ iputils-20180629/debian/patches/series 2020-01-13 23:29:01.000000000 +0000 @@ -1 +1,3 @@ set_buildflags +ping-fix-main-loop-over-multiple-addrinfo-results.patch +ping-try-next-addrinfo-on-connect-failure.patch