MFp4 @229481:

Currently it was allowed to send any UDP packets from unprivileged process and
possibly any packets because /dev/bpf was open for writing.

Move sending packets to privileged process. Unprivileged process has no longer
access to not connected UDP socket and has only access to /dev/bpf in read-only
mode.

Reviewed by:	brooks
Sponsored by:	The FreeBSD Foundation

sbin/dhclient/bpf.c

@@ -44,6 +44,7 @@
 __FBSDID("$FreeBSD$");
 
 #include "dhcpd.h"
+#include "privsep.h"
 #include <sys/ioctl.h>
 #include <sys/uio.h>
 
@@ -260,23 +261,67 @@ if_register_receive(struct interface_info *info)
 }
 
 void
-send_packet(struct interface_info *interface, struct dhcp_packet *raw,
-    size_t len, struct in_addr from, struct in_addr to)
+send_packet_unpriv(int privfd, struct dhcp_packet *raw, size_t len,
+    struct in_addr from, struct in_addr to)
+{
+	struct imsg_hdr hdr;
+	struct buf *buf;
+	int errs;
+
+	hdr.code = IMSG_SEND_PACKET;
+	hdr.len = sizeof(hdr) +
+	    sizeof(size_t) + len +
+	    sizeof(from) + sizeof(to);
+
+	if ((buf = buf_open(hdr.len)) == NULL)
+		error("buf_open: %m");
+
+	errs = 0;
+	errs += buf_add(buf, &hdr, sizeof(hdr));
+	errs += buf_add(buf, &len, sizeof(len));
+	errs += buf_add(buf, raw, len);
+	errs += buf_add(buf, &from, sizeof(from));
+	errs += buf_add(buf, &to, sizeof(to));
+	if (errs)
+		error("buf_add: %m");
+
+	if (buf_close(privfd, buf) == -1)
+		error("buf_close: %m");
+}
+
+void
+send_packet_priv(struct interface_info *interface, struct imsg_hdr *hdr, int fd)
 {
 	unsigned char buf[256];
 	struct iovec iov[2];
 	struct msghdr msg;
+	struct dhcp_packet raw;
+	size_t len;
+	struct in_addr from, to;
 	int result, bufp = 0;
 
+	if (hdr->len < sizeof(*hdr) + sizeof(size_t))
+		error("corrupted message received");
+	buf_read(fd, &len, sizeof(len));
+	if (hdr->len != sizeof(*hdr) + sizeof(size_t) + len +
+	    sizeof(from) + sizeof(to)) {
+		error("corrupted message received");
+	}
+	if (len > sizeof(raw))
+		error("corrupted message received");
+	buf_read(fd, &raw, len);
+	buf_read(fd, &from, sizeof(from));
+	buf_read(fd, &to, sizeof(to));
+
 	/* Assemble the headers... */
 	if (to.s_addr == INADDR_BROADCAST)
 		assemble_hw_header(interface, buf, &bufp);
 	assemble_udp_ip_header(buf, &bufp, from.s_addr, to.s_addr,
-	    htons(REMOTE_PORT), (unsigned char *)raw, len);
+	    htons(REMOTE_PORT), (unsigned char *)&raw, len);
 
 	iov[0].iov_base = buf;
 	iov[0].iov_len = bufp;
-	iov[1].iov_base = raw;
+	iov[1].iov_base = &raw;
 	iov[1].iov_len = len;
 
 	/* Fire it off */

sbin/dhclient/dhclient.c

@@ -455,11 +455,19 @@ main(int argc, char *argv[])
 	if (gethostname(hostname, sizeof(hostname)) < 0)
 		hostname[0] = '\0';
 
+	/* set up the interface */
+	discover_interfaces(ifi);
+
 	if (pipe(pipe_fd) == -1)
 		error("pipe");
 
 	fork_privchld(pipe_fd[0], pipe_fd[1]);
 
+	close(ifi->ufdesc);
+	ifi->ufdesc = -1;
+	close(ifi->wfdesc);
+	ifi->wfdesc = -1;
+
 	close(pipe_fd[0]);
 	privfd = pipe_fd[1];
 
@@ -479,9 +487,6 @@ main(int argc, char *argv[])
 	if (shutdown(routefd, SHUT_WR) < 0)
 		error("can't shutdown route socket: %m");
 
-	/* set up the interface */
-	discover_interfaces(ifi);
-
 	if (chroot(_PATH_VAREMPTY) == -1)
 		error("chroot");
 	if (chdir("/") == -1)
@@ -1236,8 +1241,8 @@ send_discover(void *ipp)
 	    (int)ip->client->interval);
 
 	/* Send out a packet. */
-	send_packet(ip, &ip->client->packet, ip->client->packet_length,
-	    inaddr_any, inaddr_broadcast);
+	send_packet_unpriv(privfd, &ip->client->packet,
+	    ip->client->packet_length, inaddr_any, inaddr_broadcast);
 
 	add_timeout(cur_time + ip->client->interval, send_discover, ip);
 }
@@ -1461,8 +1466,8 @@ send_request(void *ipp)
 	    REMOTE_PORT);
 
 	/* Send out a packet. */
-	send_packet(ip, &ip->client->packet, ip->client->packet_length,
-	    from, to);
+	send_packet_unpriv(privfd, &ip->client->packet,
+	    ip->client->packet_length, from, to);
 
 	add_timeout(cur_time + ip->client->interval, send_request, ip);
 }
@@ -1476,8 +1481,8 @@ send_decline(void *ipp)
 	    inet_ntoa(inaddr_broadcast), REMOTE_PORT);
 
 	/* Send out a packet. */
-	send_packet(ip, &ip->client->packet, ip->client->packet_length,
-	    inaddr_any, inaddr_broadcast);
+	send_packet_unpriv(privfd, &ip->client->packet,
+	    ip->client->packet_length, inaddr_any, inaddr_broadcast);
 }
 
 void
@@ -2698,6 +2703,8 @@ fork_privchld(int fd, int fd2)
 	dup2(nullfd, STDERR_FILENO);
 	close(nullfd);
 	close(fd2);
+	close(ifi->rfdesc);
+	ifi->rfdesc = -1;
 
 	for (;;) {
 		pfd[0].fd = fd;
@@ -2709,6 +2716,6 @@ fork_privchld(int fd, int fd2)
 		if (nfds == 0 || !(pfd[0].revents & POLLIN))
 			continue;
 
-		dispatch_imsg(fd);
+		dispatch_imsg(ifi, fd);
 	}
 }

sbin/dhclient/dhcpd.h

@@ -300,8 +300,10 @@ struct hash_bucket	*new_hash_bucket(void);
 int if_register_bpf(struct interface_info *, int);
 void if_register_send(struct interface_info *);
 void if_register_receive(struct interface_info *);
-void send_packet(struct interface_info *, struct dhcp_packet *, size_t,
-    struct in_addr, struct in_addr);
+void send_packet_unpriv(int, struct dhcp_packet *, size_t, struct in_addr,
+    struct in_addr);
+struct imsg_hdr;
+void send_packet_priv(struct interface_info *, struct imsg_hdr *, int);
 ssize_t receive_packet(struct interface_info *, unsigned char *, size_t,
     struct sockaddr_in *, struct hardware *);
 
@@ -435,4 +437,4 @@ struct buf	*buf_open(size_t);
 int		 buf_add(struct buf *, void *, size_t);
 int		 buf_close(int, struct buf *);
 ssize_t		 buf_read(int, void *, size_t);
-void		 dispatch_imsg(int);
+void		 dispatch_imsg(struct interface_info *, int);

sbin/dhclient/privsep.c

@@ -101,7 +101,7 @@ buf_read(int sock, void *buf, size_t nbytes)
 }
 
 void
-dispatch_imsg(int fd)
+dispatch_imsg(struct interface_info *ifi, int fd)
 {
 	struct imsg_hdr		 hdr;
 	char			*medium, *reason, *filename,
@@ -232,6 +232,9 @@ dispatch_imsg(int fd)
 		if (buf_close(fd, buf) == -1)
 			error("buf_close: %m");
 		break;
+	case IMSG_SEND_PACKET:
+		send_packet_priv(ifi, &hdr, fd);
+		break;
 	default:
 		error("received unknown message, code %d", hdr.code);
 	}