matthew@ points out that it's a bad idea to rely on the non-privileged,

network-facing process to properly fill out the interface name and
rdomain on which operations are to be performed.

Instead, always use the interface name and rdomain discovered before
forking and dropping privs. Lets all the imsg structs to drop a
couple of members.

ok matthew@ henning@

3 files changed, 16 insertions, 43 deletions

diff --git a/sbin/dhclient/dhclient.c b/sbin/dhclient/dhclient.c
index c2a375072f1..bc52c309010 100644
--- a/sbin/dhclient/dhclient.c
+++ b/sbin/dhclient/dhclient.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: dhclient.c,v 1.353 2015/02/05 23:56:06 krw Exp $	*/
+/*	$OpenBSD: dhclient.c,v 1.354 2015/02/06 06:47:29 krw Exp $	*/
 
 /*
  * Copyright 2004 Henning Brauer <henning@openbsd.org>
@@ -1987,8 +1987,6 @@ fork_privchld(int fd, int fd2)
 	 */
 	if (quit != SIGTERM) {
 		memset(&imsg, 0, sizeof(imsg));
-		strlcpy(imsg.ifname, ifi->name, sizeof(imsg.ifname));
-		imsg.rdomain = ifi->rdomain;
 		imsg.addr = active_addr;
 		priv_cleanup(&imsg);
 	}
@@ -2354,7 +2352,6 @@ write_file(char *path, int flags, mode_t mode, uid_t uid, gid_t gid,
 		return;
 	}
 
-	imsg.rdomain = ifi->rdomain;
 	imsg.len = sz;
 	imsg.flags = flags;
 	imsg.mode = mode;
@@ -2380,7 +2377,7 @@ priv_write_file(struct imsg_write_file *imsg)
 	int fd;
 
 	if ((strcmp("/etc/resolv.conf", imsg->path) == 0) &&
-	    !resolv_conf_priority(imsg->rdomain))
+	    !resolv_conf_priority(ifi->rdomain))
 		return;
 
 	fd = open(imsg->path, imsg->flags, imsg->mode);
diff --git a/sbin/dhclient/kroute.c b/sbin/dhclient/kroute.c
index e3e252944c2..668fa110099 100644
--- a/sbin/dhclient/kroute.c
+++ b/sbin/dhclient/kroute.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kroute.c,v 1.70 2015/01/31 03:13:04 krw Exp $	*/
+/*	$OpenBSD: kroute.c,v 1.71 2015/02/06 06:47:29 krw Exp $	*/
 
 /*
  * Copyright 2012 Kenneth R Westerback <krw@openbsd.org>
@@ -33,7 +33,7 @@ struct in_addr active_addr;
 int	create_route_label(struct sockaddr_rtlabel *);
 int	check_route_label(struct sockaddr_rtlabel *);
 void	populate_rti_info(struct sockaddr **, struct rt_msghdr *);
-void	delete_route(int, int, struct rt_msghdr *);
+void	delete_route(int, struct rt_msghdr *);
 
 #define	ROUTE_LABEL_NONE		1
 #define	ROUTE_LABEL_NOT_DHCLIENT	2
@@ -56,8 +56,6 @@ flush_routes(char *ifname, int rdomain)
 
 	memset(&imsg, 0, sizeof(imsg));
 
-	strlcpy(imsg.ifname, ifname, sizeof(imsg.ifname));
-	imsg.rdomain = rdomain;
 	imsg.zapzombies = 1;
 
 	rslt = imsg_compose(unpriv_ibuf, IMSG_FLUSH_ROUTES, 0, 0, -1,
@@ -87,7 +85,7 @@ priv_flush_routes(struct imsg_flush_routes *imsg)
 	mib[3] = AF_INET;
 	mib[4] = NET_RT_FLAGS;
 	mib[5] = RTF_GATEWAY;
-	mib[6] = imsg->rdomain;
+	mib[6] = ifi->rdomain;
 
 	while (1) {
 		if (sysctl(mib, 7, NULL, &needed, NULL, 0) == -1) {
@@ -136,11 +134,11 @@ priv_flush_routes(struct imsg_flush_routes *imsg)
 		switch (check_route_label(sa_rl)) {
 		case ROUTE_LABEL_DHCLIENT_OURS:
 			/* Always delete routes we labeled. */
-			delete_route(s, imsg->rdomain, rtm);
+			delete_route(s, rtm);
 			break;
 		case ROUTE_LABEL_DHCLIENT_DEAD:
 			if (imsg->zapzombies)
-				delete_route(s, imsg->rdomain, rtm);
+				delete_route(s, rtm);
 			break;
 		case ROUTE_LABEL_DHCLIENT_LIVE:
 		case ROUTE_LABEL_DHCLIENT_UNKNOWN:
@@ -153,9 +151,9 @@ priv_flush_routes(struct imsg_flush_routes *imsg)
 			if (if_indextoname(rtm->rtm_index, ifname) &&
 			    sa_in &&
 			    sa_in->sin_addr.s_addr == INADDR_ANY &&
-			    rtm->rtm_tableid == imsg->rdomain &&
-			    strcmp(imsg->ifname, ifname) == 0)
-				delete_route(s, imsg->rdomain, rtm);
+			    rtm->rtm_tableid == ifi->rdomain &&
+			    strcmp(ifi->name, ifname) == 0)
+				delete_route(s, rtm);
 			break;
 		default:
 			break;
@@ -175,7 +173,6 @@ add_route(int rdomain, struct in_addr dest, struct in_addr netmask,
 
 	memset(&imsg, 0, sizeof(imsg));
 
-	imsg.rdomain = rdomain;
 	imsg.dest = dest;
 	imsg.gateway = gateway;
 	imsg.netmask = netmask;
@@ -208,7 +205,7 @@ priv_add_route(struct imsg_add_route *imsg)
 
 	rtm.rtm_version = RTM_VERSION;
 	rtm.rtm_type = RTM_ADD;
-	rtm.rtm_tableid = imsg->rdomain;
+	rtm.rtm_tableid = ifi->rdomain;
 	rtm.rtm_priority = RTP_NONE;
 	rtm.rtm_msglen = sizeof(rtm);
 	rtm.rtm_addrs = imsg->addrs;
@@ -322,8 +319,6 @@ delete_address(char *ifname, int rdomain, struct in_addr addr)
 	/* Note the address we are deleting for RTM_DELADDR filtering! */
 	deleting.s_addr = addr.s_addr;
 
-	strlcpy(imsg.ifname, ifname, sizeof(imsg.ifname));
-	imsg.rdomain = rdomain;
 	imsg.addr = addr;
 
 	rslt = imsg_compose(unpriv_ibuf, IMSG_DELETE_ADDRESS, 0, 0 , -1, &imsg,
@@ -349,8 +344,7 @@ priv_delete_address(struct imsg_delete_address *imsg)
 		error("socket open failed: %s", strerror(errno));
 
 	memset(&ifaliasreq, 0, sizeof(ifaliasreq));
-	strncpy(ifaliasreq.ifra_name, imsg->ifname,
-	    sizeof(ifaliasreq.ifra_name));
+	strncpy(ifaliasreq.ifra_name, ifi->name, sizeof(ifaliasreq.ifra_name));
 
 	in = (struct sockaddr_in *)&ifaliasreq.ifra_addr;
 	in->sin_family = AF_INET;
@@ -386,8 +380,6 @@ add_address(char *ifname, int rdomain, struct in_addr addr,
 	/* Note the address we are adding for RTM_NEWADDR filtering! */
 	adding = addr;
 
-	strlcpy(imsg.ifname, ifname, sizeof(imsg.ifname));
-	imsg.rdomain = rdomain;
 	imsg.addr = addr;
 	imsg.mask = mask;
 
@@ -421,8 +413,7 @@ priv_add_address(struct imsg_add_address *imsg)
 		error("socket open failed: %s", strerror(errno));
 
 	memset(&ifaliasreq, 0, sizeof(ifaliasreq));
-	strncpy(ifaliasreq.ifra_name, imsg->ifname,
-	    sizeof(ifaliasreq.ifra_name));
+	strncpy(ifaliasreq.ifra_name, ifi->name, sizeof(ifaliasreq.ifra_name));
 
 	/* The actual address in ifra_addr. */
 	in = (struct sockaddr_in *)&ifaliasreq.ifra_addr;
@@ -458,8 +449,6 @@ sendhup(struct client_lease *active)
 
 	memset(&imsg, 0, sizeof(imsg));
 
-	strlcpy(imsg.ifname, ifi->name, sizeof(imsg.ifname));
-	imsg.rdomain = ifi->rdomain;
 	if (active)
 		imsg.addr = active->address;
 
@@ -481,7 +470,6 @@ priv_cleanup(struct imsg_hup *imsg)
 	struct imsg_delete_address dimsg;
 
 	memset(&fimsg, 0, sizeof(fimsg));
-	fimsg.rdomain = imsg->rdomain;
 	fimsg.zapzombies = 0;	/* Only zapzombies when binding a lease. */
 	priv_flush_routes(&fimsg);
 
@@ -489,8 +477,6 @@ priv_cleanup(struct imsg_hup *imsg)
 		return;
 
 	memset(&dimsg, 0, sizeof(dimsg));
-	strlcpy(dimsg.ifname, imsg->ifname, sizeof(imsg->ifname));
-	dimsg.rdomain = imsg->rdomain;
 	dimsg.addr = imsg->addr;
 	priv_delete_address(&dimsg);
 }
@@ -668,13 +654,13 @@ populate_rti_info(struct sockaddr **rti_info, struct rt_msghdr *rtm)
 }
 
 void
-delete_route(int s, int rdomain, struct rt_msghdr *rtm)
+delete_route(int s, struct rt_msghdr *rtm)
 {
 	static int seqno;
 	ssize_t rlen;
 
 	rtm->rtm_type = RTM_DELETE;
-	rtm->rtm_tableid = rdomain;
+	rtm->rtm_tableid = ifi->rdomain;
 	rtm->rtm_seq = seqno++;
 
 	rlen = write(s, (char *)rtm, rtm->rtm_msglen);
diff --git a/sbin/dhclient/privsep.h b/sbin/dhclient/privsep.h
index eae657aafff..294d410f561 100644
--- a/sbin/dhclient/privsep.h
+++ b/sbin/dhclient/privsep.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: privsep.h,v 1.24 2015/01/16 06:39:56 deraadt Exp $ */
+/*	$OpenBSD: privsep.h,v 1.25 2015/02/06 06:47:29 krw Exp $ */
 
 /*
  * Copyright (c) 2004 Henning Brauer <henning@openbsd.org>
@@ -31,21 +31,15 @@ enum imsg_code {
 };
 
 struct imsg_delete_address {
-	char	ifname[IFNAMSIZ];
-	int	rdomain;
 	struct	in_addr addr;
 };
 
 struct imsg_add_address {
-	char	ifname[IFNAMSIZ];
-	int	rdomain;
 	struct	in_addr	addr;
 	struct	in_addr mask;
 };
 
 struct imsg_flush_routes {
-	char	ifname[IFNAMSIZ];
-	int	rdomain;
 	int	zapzombies;
 };
 
@@ -53,20 +47,16 @@ struct imsg_add_route {
 	struct in_addr	dest;
 	struct in_addr	netmask;
 	struct in_addr	gateway;
-	int		rdomain;
 	int		addrs;
 	int		flags;
 };
 
 struct imsg_hup {
-	char	ifname[IFNAMSIZ];
-	int	rdomain;
 	struct	in_addr addr;
 };
 
 struct imsg_write_file {
 	char	path[PATH_MAX];
-	int	rdomain;
 	int	flags;
 	mode_t	mode;
 	size_t	len;