From d5113e2f2801e517733035a8d22216b334385537 Mon Sep 17 00:00:00 2001 From: pjd Date: Wed, 3 Jul 2013 22:12:54 +0000 Subject: 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 | 53 ++++++++++++++++++++++++++++++++++++++++++++---- sbin/dhclient/dhclient.c | 27 +++++++++++++++--------- sbin/dhclient/dhcpd.h | 8 +++++--- sbin/dhclient/privsep.c | 5 ++++- 4 files changed, 75 insertions(+), 18 deletions(-) (limited to 'sbin/dhclient') diff --git a/sbin/dhclient/bpf.c b/sbin/dhclient/bpf.c index c16e477..8964279 100644 --- a/sbin/dhclient/bpf.c +++ b/sbin/dhclient/bpf.c @@ -44,6 +44,7 @@ __FBSDID("$FreeBSD$"); #include "dhcpd.h" +#include "privsep.h" #include #include @@ -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 */ diff --git a/sbin/dhclient/dhclient.c b/sbin/dhclient/dhclient.c index 43e0c16..8dc9736 100644 --- a/sbin/dhclient/dhclient.c +++ b/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 @@ again: (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 @@ cancel: 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); } } diff --git a/sbin/dhclient/dhcpd.h b/sbin/dhclient/dhcpd.h index 7747d03..479753e 100644 --- a/sbin/dhclient/dhcpd.h +++ b/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); diff --git a/sbin/dhclient/privsep.c b/sbin/dhclient/privsep.c index b42572f..a0521a6 100644 --- a/sbin/dhclient/privsep.c +++ b/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); } -- cgit v1.1