Here is an attempt to fix ping(8) and traceroute(8) source selection.
Currently these tools do not obey route sourceaddr set by the operator. This
leads to frustration at best and erroneous diagnosis at worse on multi-homed
systems.
The "real" fix would be to rework source selection in the kernel stack but this
is a huge work which not happen overnight nor in the coming days.
In the mean time, I propose the following diff.
I removed -R (route recording) in ping(8) because it is not compatible with
sending a full IP header to the rip_output(). It should not impact anyone as RR
is most of the time ignored by routers.
Denis
Index: sbin/ping/ping.c
===================================================================
RCS file: /cvs/src/sbin/ping/ping.c,v
retrieving revision 1.245
diff -u -p -r1.245 ping.c
--- sbin/ping/ping.c 12 Jul 2021 15:09:19 -0000 1.245
+++ sbin/ping/ping.c 17 Dec 2021 20:27:31 -0000
@@ -143,16 +143,14 @@ int options;
#define F_HOSTNAME 0x0004
#define F_PINGFILLED 0x0008
#define F_QUIET 0x0010
-#define F_RROUTE 0x0020
-#define F_SO_DEBUG 0x0040
-#define F_SHOWCHAR 0x0080
-#define F_VERBOSE 0x0100
+#define F_SO_DEBUG 0x0020
+#define F_SHOWCHAR 0x0040
+#define F_VERBOSE 0x0080
/* 0x0200 */
-#define F_HDRINCL 0x0400
-#define F_TTL 0x0800
-#define F_TOS 0x1000
-#define F_AUD_RECV 0x2000
-#define F_AUD_MISS 0x4000
+#define F_TTL 0x0100
+#define F_TOS 0x0200
+#define F_AUD_RECV 0x0400
+#define F_AUD_MISS 0x0800
/* multicast options */
int moptions;
@@ -256,7 +254,6 @@ main(int argc, char *argv[])
u_char *datap, *packet;
u_char ttl = MAXTTL;
char *e, *target, hbuf[NI_MAXHOST], *source = NULL;
- char rspace[3 + 4 * NROUTES + 1]; /* record route space */
const char *errstr;
double fraction, integral, seconds;
uid_t ouid, uid;
@@ -308,7 +305,6 @@ main(int argc, char *argv[])
errstr, optarg);
break;
case 'D':
- options |= F_HDRINCL;
df = 1;
break;
case 'd':
@@ -383,7 +379,7 @@ main(int argc, char *argv[])
options |= F_QUIET;
break;
case 'R':
- options |= F_RROUTE;
+ printf("-R option is not supported anymore\n");
break;
case 's': /* size of packet to send */
datalen = strtonum(optarg, 0, maxpayload, &errstr);
@@ -393,7 +389,6 @@ main(int argc, char *argv[])
break;
#ifndef SMALL
case 'T':
- options |= F_HDRINCL;
options |= F_TOS;
errno = 0;
errstr = NULL;
@@ -509,7 +504,7 @@ main(int argc, char *argv[])
if (bind(s, from, from->sa_len) == -1)
err(1, "bind");
}
- } else if (options & F_VERBOSE) {
+ } else {
/*
* get the source address. XXX since we revoked the root
* privilege, we cannot use a raw socket for this.
@@ -711,51 +706,26 @@ main(int argc, char *argv[])
err(1, "setsockopt(IPV6_RECVHOPLIMIT)");
} else {
u_char loop = 0;
+ struct ip *ip = (struct ip *)outpackhdr;
if (options & F_TTL) {
if (IN_MULTICAST(ntohl(dst4.sin_addr.s_addr)))
moptions |= MULTICAST_TTL;
- else
- options |= F_HDRINCL;
}
- if ((options & F_RROUTE) && (options & F_HDRINCL))
- errx(1, "-R option and -D or -T, or -t to unicast"
- " destinations are incompatible");
-
- if (options & F_HDRINCL) {
- struct ip *ip = (struct ip *)outpackhdr;
-
- if (setsockopt(s, IPPROTO_IP, IP_HDRINCL,
- &optval, sizeof(optval)) == -1)
- err(1, "setsockopt(IP_HDRINCL)");
- ip->ip_v = IPVERSION;
- ip->ip_hl = sizeof(struct ip) >> 2;
- ip->ip_tos = tos;
- ip->ip_id = 0;
- ip->ip_off = htons(df ? IP_DF : 0);
- ip->ip_ttl = ttl;
- ip->ip_p = IPPROTO_ICMP;
- if (source)
- ip->ip_src = from4.sin_addr;
- else
- ip->ip_src.s_addr = INADDR_ANY;
- ip->ip_dst = dst4.sin_addr;
- }
+ if (setsockopt(s, IPPROTO_IP, IP_HDRINCL,
+ &optval, sizeof(optval)) == -1)
+ err(1, "setsockopt(IP_HDRINCL)");
- /* record route option */
- if (options & F_RROUTE) {
- if (IN_MULTICAST(ntohl(dst4.sin_addr.s_addr)))
- errx(1, "record route not valid to multicast"
- " destinations");
- memset(rspace, 0, sizeof(rspace));
- rspace[IPOPT_OPTVAL] = IPOPT_RR;
- rspace[IPOPT_OLEN] = sizeof(rspace)-1;
- rspace[IPOPT_OFFSET] = IPOPT_MINOFF;
- if (setsockopt(s, IPPROTO_IP, IP_OPTIONS,
- rspace, sizeof(rspace)) == -1)
- err(1, "record route");
- }
+ ip->ip_v = IPVERSION;
+ ip->ip_hl = sizeof(struct ip) >> 2;
+ ip->ip_tos = tos;
+ ip->ip_id = 0;
+ ip->ip_off = htons(df ? IP_DF : 0);
+ ip->ip_ttl = ttl;
+ ip->ip_p = IPPROTO_ICMP;
+ ip->ip_src = from4.sin_addr;
+ ip->ip_dst = dst4.sin_addr;
if ((moptions & MULTICAST_NOLOOP) &&
setsockopt(s, IPPROTO_IP, IP_MULTICAST_LOOP,
@@ -1166,14 +1136,12 @@ pinger(int s)
/* compute ICMP checksum here */
icp->icmp_cksum = in_cksum((u_short *)icp, cc);
- if (options & F_HDRINCL) {
- struct ip *ip = (struct ip *)outpackhdr;
+ struct ip *ip = (struct ip *)outpackhdr;
- smsgiov.iov_base = (caddr_t)outpackhdr;
- cc += sizeof(struct ip);
- ip->ip_len = htons(cc);
- ip->ip_sum = in_cksum((u_short *)outpackhdr, cc);
- }
+ smsgiov.iov_base = (caddr_t)outpackhdr;
+ cc += sizeof(struct ip);
+ ip->ip_len = htons(cc);
+ ip->ip_sum = in_cksum((u_short *)outpackhdr, cc);
}
smsgiov.iov_len = cc;
Index: usr.sbin/traceroute/traceroute.c
===================================================================
RCS file: /cvs/src/usr.sbin/traceroute/traceroute.c,v
retrieving revision 1.168
diff -u -p -r1.168 traceroute.c
--- usr.sbin/traceroute/traceroute.c 3 Sep 2021 09:13:00 -0000 1.168
+++ usr.sbin/traceroute/traceroute.c 17 Dec 2021 20:27:32 -0000
@@ -710,12 +710,14 @@ main(int argc, char *argv[])
&on, sizeof(on)) == -1)
err(6, "IP_HDRINCL");
+ /*
+ * Source selection
+ */
+ memset(&from4, 0, sizeof(from4));
if (conf->source) {
- memset(&from4, 0, sizeof(from4));
from4.sin_family = AF_INET;
if (inet_aton(conf->source, &from4.sin_addr) == 0)
errx(1, "unknown host %s", conf->source);
- ip->ip_src = from4.sin_addr;
if (ouid != 0 &&
(ntohl(from4.sin_addr.s_addr) & 0xff000000U) ==
0x7f000000U && (ntohl(to4.sin_addr.s_addr) &
@@ -725,7 +727,29 @@ main(int argc, char *argv[])
if (ouid && bind(sndsock, (struct sockaddr *)&from4,
sizeof(from4)) == -1)
err(1, "bind");
+ } else {
+ struct sockaddr_in sin;
+ int dummy;
+
+ sin = to4;
+ sin.sin_port = htons(DUMMY_PORT);
+ if ((dummy = socket(AF_INET, SOCK_DGRAM, 0)) == -1)
+ err(1, "socket");
+ if (conf->rtableid > 0 &&
+ setsockopt(dummy, SOL_SOCKET, SO_RTABLE,
+ &conf->rtableid, sizeof(conf->rtableid)) == -1)
+ err(1, "setsockopt(SO_RTABLE)");
+ if (connect(dummy, (struct sockaddr *)&sin,
+ sin.sin_len) == -1)
+ err(1, "connect");
+ len = sizeof(from4);
+ if (getsockname(dummy, (struct sockaddr *)&from4,
+ &len) == -1)
+ err(1, "getsockname");
+ close(dummy);
}
+ ip->ip_src = from4.sin_addr;
+
packetlen = datalen;
break;
case AF_INET6:
