From: kn <kn@openbsd.org>
Date: Mon, 14 Nov 2022 13:57:46 +0000 (+0000)
Subject: Preserve original order of nameservers
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=6a845ff209a3b81f32c0d04a37d071733f5948de;p=openbsd

Preserve original order of nameservers

RFC 2132 "DHCP Options and BOOTP Vendor Extensions"
3.8. Domain Name Server Option says
	Servers SHOULD be listed in order of preference.

tcpdump(8), route(8) monitor and dhcpleasectl(8) -l athn0 show servers from
the DHCP OFFER in their original order, as expected.

resolvd(8) however sorts proposals by priority and IP address before writing
them to resolv.conf(5).

But as the system resolver tries this file's `nameserver' options in the
order appearance, sorting by IP breaks DHCP's intended order and thus may
result in the wrong nameserver being queried.


Sorting by IP is done to later remove duplicates from the file.

Sort by priority alone and ensure uniqueness by iterating over the list of
of proposals and zeroeing duplicates instead to preserve any proposal's
original order.


Spotted on a public wifi OFFERing two local IPs plus 8.8.8.8 in this order
which ended up with 8.8.8.8 being the first entry in my /etc/resolv.conf.

In other words, `route nameserver lo0 2.2.2.2 1.1.1.1 1.1.1.1' now yields
	nameserver 2.2.2.2 # resolvd: lo0
	nameserver 1.1.1.1 # resolvd: lo0
rather than
	nameserver 1.1.1.1 # resolvd: lo0
	nameserver 2.2.2.2 # resolvd: lo0

Feedback OK deraadt
---

diff --git a/sbin/resolvd/resolvd.c b/sbin/resolvd/resolvd.c
index c17c6bf5561..5fb9b39a1e3 100644
--- a/sbin/resolvd/resolvd.c
+++ b/sbin/resolvd/resolvd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: resolvd.c,v 1.28 2022/09/02 09:39:55 florian Exp $	*/
+/*	$OpenBSD: resolvd.c,v 1.29 2022/11/14 13:57:46 kn Exp $	*/
 /*
  * Copyright (c) 2021 Florian Obser <florian@openbsd.org>
  * Copyright (c) 2021 Theo de Raadt <deraadt@openbsd.org>
@@ -65,7 +65,7 @@ void		route_receive(int);
 void		handle_route_message(struct rt_msghdr *, struct sockaddr **);
 void		get_rtaddrs(int, struct sockaddr *, struct sockaddr **);
 void		solicit_dns_proposals(int);
-void		regen_resolvconf(char *reason);
+void		regen_resolvconf(const char *reason);
 int		cmp(const void *, const void *);
 int		findslot(struct rdns_proposal *);
 void		zeroslot(struct rdns_proposal *);
@@ -503,17 +503,24 @@ handle_route_message(struct rt_msghdr *rtm, struct sockaddr **rti_info)
 		return;
 	}
 
-	/* Sort proposals, based upon priority and IP */
-	qsort(learning, ASR_MAXNS, sizeof(learning[0]), cmp);
+	/* Sort proposals, based upon priority */
+	if (mergesort(learning, ASR_MAXNS, sizeof(learning[0]), cmp) == -1) {
+		lwarn("mergesort");
+		return;
+	}
 
-	/* Eliminate duplicates */
+	/* Eliminate duplicate IPs per interface */
 	for (i = 0; i < ASR_MAXNS - 1; i++) {
+		int j;
+
 		if (learning[i].prio == 0)
 			continue;
-		if (learning[i].if_index == learning[i+1].if_index &&
-		    strcmp(learning[i].ip, learning[i+1].ip) == 0) {
-			zeroslot(&learning[i + 1]);
-			i--;	/* backup and re-check */
+
+		for (j = i + 1; j < ASR_MAXNS; j++) {
+			if (learning[i].if_index == learning[j].if_index &&
+			    strcmp(learning[i].ip, learning[j].ip) == 0) {
+				zeroslot(&learning[j]);
+			}
 		}
 	}
 
@@ -567,7 +574,7 @@ solicit_dns_proposals(int routesock)
 }
 
 void
-regen_resolvconf(char *why)
+regen_resolvconf(const char *why)
 {
 	struct iovec	 iov[UIO_MAXIOV];
 	int		 i, fd, len, iovcnt = 0;
@@ -694,10 +701,7 @@ cmp(const void *a, const void *b)
 {
 	const struct rdns_proposal	*rpa = a, *rpb = b;
 
-	if (rpa->prio == rpb->prio)
-		return strcmp(rpa->ip, rpb->ip);
-	else
-		return rpa->prio < rpb->prio ? -1 : 1;
+	return (rpa->prio < rpb->prio) ? -1 : (rpa->prio > rpb->prio);
 }
 
 #ifndef SMALL