--- Begin Message ---
- To: Debian Bug Tracking System <submit@bugs.debian.org>
- Subject: buster-pu: package iputils/3:20180629-2
- From: Noah Meyerhans <noahm@debian.org>
- Date: Mon, 13 Jan 2020 19:12:57 -0500
- Message-id: <20200114001257.GA22118@morgul.net>
Package: release.debian.org
Severity: normal
Tags: buster
User: release.debian.org@packages.debian.org
Usertags: pu
I'd like to fix https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=947921 in
buster. Ping has some issues coping with explicitly specified source
interfaces when pinging DNS names and DNS returns multiple addresses via
getaddrinfo(). Please see the commit messages and the bug report for in-depth
explanation of what's going on.
The attached diff incorporates upstream's fix for this issue.
Thanks!
noah
diff -Nru iputils-20180629/debian/changelog iputils-20180629/debian/changelog
--- iputils-20180629/debian/changelog 2018-08-03 09:53:09.000000000 -0700
+++ iputils-20180629/debian/changelog 2020-01-13 15:29:01.000000000 -0800
@@ -1,3 +1,12 @@
+iputils (3:20180629-2+deb10u1) buster; urgency=medium
+
+ * Incorporate patches from Benjamin Poirier <benjamin.poirier@gmail.com> 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 <noahm@debian.org> 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 1969-12-31 16:00:00.000000000 -0800
+++ iputils-20180629/debian/patches/ping-fix-main-loop-over-multiple-addrinfo-results.patch 2020-01-13 15:15:58.000000000 -0800
@@ -0,0 +1,84 @@
+From 769a6790437e883d72eebbabd06d33a05a0340ca Mon Sep 17 00:00:00 2001
+From: Benjamin Poirier <benjamin.poirier@gmail.com>
+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 <bpoirier@cumulusnetworks.com>
+---
+ 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 1969-12-31 16:00:00.000000000 -0800
+++ iputils-20180629/debian/patches/ping-try-next-addrinfo-on-connect-failure.patch 2020-01-13 15:19:47.000000000 -0800
@@ -0,0 +1,187 @@
+From f0ee2f4f94560953d6027c57cdc484f23e59858a Mon Sep 17 00:00:00 2001
+From: Benjamin Poirier <benjamin.poirier@gmail.com>
+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 <bpoirier@cumulusnetworks.com>
+---
+ 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 09:53:09.000000000 -0700
+++ iputils-20180629/debian/patches/series 2020-01-13 15:19:47.000000000 -0800
@@ -1 +1,3 @@
set_buildflags
+ping-fix-main-loop-over-multiple-addrinfo-results.patch
+ping-try-next-addrinfo-on-connect-failure.patch
--- End Message ---