From: deraadt <deraadt@openbsd.org>
Date: Sun, 18 Oct 2015 20:15:10 +0000 (+0000)
Subject: Instead of fragile CMSG parsing, control pledge "sendfd" and "recvfd"
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=95dd115275fc900cdf835747aa31e3286dd904a7;p=openbsd

Instead of fragile CMSG parsing, control pledge "sendfd" and "recvfd"
in unp_internalize and unp_externalize.
ok kettenis guenther
---

diff --git a/sys/kern/kern_pledge.c b/sys/kern/kern_pledge.c
index 1001900a7ef..da0737821a4 100644
--- a/sys/kern/kern_pledge.c
+++ b/sys/kern/kern_pledge.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_pledge.c,v 1.54 2015/10/18 05:26:55 semarie Exp $	*/
+/*	$OpenBSD: kern_pledge.c,v 1.55 2015/10/18 20:15:10 deraadt Exp $	*/
 
 /*
  * Copyright (c) 2015 Nicholas Marriott <nicm@openbsd.org>
@@ -733,135 +733,66 @@ pledge_aftersyscall(struct proc *p, int code, int error)
 }
 
 /*
- * By default, only the advisory cmsg's can be received from the kernel,
- * such as TIMESTAMP ntpd.
- *
- * If PLEDGE_RECVFD is set SCM_RIGHTS is also allowed in for a carefully
- * selected set of descriptors (specifically to exclude directories).
- *
- * This results in a kill upon recv, if some other process on the system
- * send a SCM_RIGHTS to an open socket of some sort.  That will discourage
- * leaving such sockets lying around...
+ * Only allow reception of safe file descriptors.
  */
 int
-pledge_cmsg_recv(struct proc *p, struct mbuf *control)
+pledge_recvfd_check(struct proc *p, struct file *fp)
 {
-	struct msghdr tmp;
-	struct cmsghdr *cmsg;
-	int *fdp, fd;
-	struct file *fp;
-	int nfds, i;
+	struct vnode *vp;
+	char *vtypes[] = { VTYPE_NAMES };
 
 	if ((p->p_p->ps_flags & PS_PLEDGE) == 0)
 		return (0);
-
-	/* Scan the cmsg */
-	memset(&tmp, 0, sizeof(tmp));
-	tmp.msg_control = mtod(control, struct cmsghdr *);
-	tmp.msg_controllen = control->m_len;
-	cmsg = CMSG_FIRSTHDR(&tmp);
-
-	while (cmsg != NULL) {
-		if (cmsg->cmsg_level == SOL_SOCKET &&
-		    cmsg->cmsg_type == SCM_RIGHTS)
-			break;
-		cmsg = CMSG_NXTHDR(&tmp, cmsg);
+	if ((p->p_p->ps_pledge & PLEDGE_RECVFD) == 0) {
+		printf("recvmsg not allowed\n");
+		return pledge_fail(p, EPERM, PLEDGE_RECVFD);
 	}
 
-	/* No SCM_RIGHTS found -> OK */
-	if (cmsg == NULL)
+	switch (fp->f_type) {
+	case DTYPE_SOCKET:
+	case DTYPE_PIPE:
 		return (0);
+	case DTYPE_VNODE:
+		vp = (struct vnode *)fp->f_data;
 
-	if ((p->p_p->ps_pledge & PLEDGE_RECVFD) == 0)
-		return pledge_fail(p, EPERM, PLEDGE_RECVFD);
-
-	/* In OpenBSD, a CMSG only contains one SCM_RIGHTS.  Check it. */
-	fdp = (int *)CMSG_DATA(cmsg);
-	nfds = (cmsg->cmsg_len - CMSG_ALIGN(sizeof(*cmsg))) /
-	    sizeof(struct file *);
-	for (i = 0; i < nfds; i++) {
-		struct vnode *vp;
-
-		fd = *fdp++;
-		fp = fd_getfile(p->p_fd, fd);
-		if (fp == NULL)
-			return pledge_fail(p, EBADF, PLEDGE_RECVFD);
-
-		/* Only allow passing of sockets, pipes, and pure files */
-		switch (fp->f_type) {
-		case DTYPE_SOCKET:
-		case DTYPE_PIPE:
-			continue;
-		case DTYPE_VNODE:
-			vp = (struct vnode *)fp->f_data;
-			if (vp->v_type == VREG)
-				continue;
-			break;
-		default:
-			break;
-		}
-		return pledge_fail(p, EPERM, PLEDGE_RECVFD);
+		if (vp->v_type != VDIR)
+			return (0);
+		break;
 	}
-	return (0);
+	printf("recvfd type %d %s\n", fp->f_type, vtypes[fp->f_type]);
+	return pledge_fail(p, EPERM, PLEDGE_RECVFD);
 }
 
 /*
- * When pledged, default prevents sending of a cmsg.
- *
- * Unlike pledge_cmsg_recv pledge_cmsg_send is called with individual
- * cmsgs one per mbuf. So no need to loop or scan.
+ * Only allow sending of safe file descriptors.
  */
 int
-pledge_cmsg_send(struct proc *p, struct mbuf *control)
+pledge_sendfd_check(struct proc *p, struct file *fp)
 {
-	struct cmsghdr *cmsg;
-	int *fdp, fd;
-	struct file *fp;
-	int nfds, i;
+	struct vnode *vp;
+	char *vtypes[] = { VTYPE_NAMES };
 
 	if ((p->p_p->ps_flags & PS_PLEDGE) == 0)
 		return (0);
 
-	/* Scan the cmsg */
-	cmsg = mtod(control, struct cmsghdr *);
-
-	/* Contains no SCM_RIGHTS, so OK */
-	if (!(cmsg->cmsg_level == SOL_SOCKET &&
-	    cmsg->cmsg_type == SCM_RIGHTS))
-		return (0);
-
-	if ((p->p_p->ps_pledge & PLEDGE_SENDFD) == 0)
+	if ((p->p_p->ps_pledge & PLEDGE_SENDFD) == 0) {
+		printf("sendmsg not allowed\n");
 		return pledge_fail(p, EPERM, PLEDGE_SENDFD);
+	}
 
-	/* In OpenBSD, a CMSG only contains one SCM_RIGHTS.  Check it. */
-	fdp = (int *)CMSG_DATA(cmsg);
-	nfds = (cmsg->cmsg_len - CMSG_ALIGN(sizeof(*cmsg))) /
-	    sizeof(struct file *);
-	for (i = 0; i < nfds; i++) {
-		struct vnode *vp;
-
-		fd = *fdp++;
-		fp = fd_getfile(p->p_fd, fd);
-		if (fp == NULL)
-			return pledge_fail(p, EBADF, PLEDGE_SENDFD);
-
-		/* Only allow passing of sockets, pipes, and pure files */
-		switch (fp->f_type) {
-		case DTYPE_SOCKET:
-		case DTYPE_PIPE:
-			continue;
-		case DTYPE_VNODE:
-			vp = (struct vnode *)fp->f_data;
-			if (vp->v_type == VREG)
-				continue;
-			break;
-		default:
-			break;
-		}
-		/* Not allowed to send a bad fd type */
-		return pledge_fail(p, EPERM, PLEDGE_SENDFD);
+	switch (fp->f_type) {
+	case DTYPE_SOCKET:
+	case DTYPE_PIPE:
+		return (0);
+	case DTYPE_VNODE:
+		vp = (struct vnode *)fp->f_data;
+	
+		if (vp->v_type != VDIR)
+			return (0);
+		break;
 	}
-	return (0);
+	printf("sendfd type %d %s\n", fp->f_type, vtypes[fp->f_type]);
+	return pledge_fail(p, EPERM, PLEDGE_SENDFD);
 }
 
 int
@@ -1060,9 +991,9 @@ pledge_ioctl_check(struct proc *p, long com, void *v)
 #if NPTY > 0
 		case PTMGET:
 			if ((p->p_p->ps_pledge & PLEDGE_RPATH) == 0)
-		                break;
+				break;
 			if ((p->p_p->ps_pledge & PLEDGE_WPATH) == 0)
-		                break;
+				break;
 			if (fp->f_type != DTYPE_VNODE || vp->v_type != VCHR)
 				break;
 			if (cdevsw[major(vp->v_rdev)].d_open != ptmopen)
diff --git a/sys/kern/uipc_syscalls.c b/sys/kern/uipc_syscalls.c
index bd6793ded0f..b80c055e243 100644
--- a/sys/kern/uipc_syscalls.c
+++ b/sys/kern/uipc_syscalls.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uipc_syscalls.c,v 1.114 2015/10/18 00:04:43 deraadt Exp $	*/
+/*	$OpenBSD: uipc_syscalls.c,v 1.115 2015/10/18 20:15:10 deraadt Exp $	*/
 /*	$NetBSD: uipc_syscalls.c,v 1.19 1996/02/09 19:00:48 christos Exp $	*/
 
 /*
@@ -632,12 +632,6 @@ sendit(struct proc *p, int s, struct msghdr *mp, int flags, register_t *retsize)
 			ktrcmsghdr(p, mtod(control, char *),
 			    mp->msg_controllen);
 #endif
-
-		if (pledge_cmsg_send(p, control)) {
-			m_free(control);
-			error = EPERM;
-			goto bad;
-		}
 	} else
 		control = 0;
 #ifdef KTRACE
@@ -871,10 +865,6 @@ recvit(struct proc *p, int s, struct msghdr *mp, caddr_t namelenp,
 					mp->msg_flags |= MSG_CTRUNC;
 					i = len;
 				}
-				if (pledge_cmsg_recv(p, m)) {
-					error = EPERM;
-					goto out;
-				}
 				error = copyout(mtod(m, caddr_t), cp, i);
 				if (m->m_next)
 					i = ALIGN(i);
diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c
index c3632bde276..ebf23472d6e 100644
--- a/sys/kern/uipc_usrreq.c
+++ b/sys/kern/uipc_usrreq.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uipc_usrreq.c,v 1.88 2015/10/17 23:15:10 deraadt Exp $	*/
+/*	$OpenBSD: uipc_usrreq.c,v 1.89 2015/10/18 20:15:10 deraadt Exp $	*/
 /*	$NetBSD: uipc_usrreq.c,v 1.18 1996/02/09 19:00:50 christos Exp $	*/
 
 /*
@@ -49,6 +49,7 @@
 #include <sys/stat.h>
 #include <sys/mbuf.h>
 #include <sys/task.h>
+#include <sys/pledge.h>
 
 void	uipc_setaddr(const struct unpcb *, struct mbuf *);
 
@@ -682,6 +683,10 @@ unp_externalize(struct mbuf *rights, socklen_t controllen, int flags)
 		rp = (struct file **)CMSG_DATA(cm);
 		for (i = 0; i < nfds; i++) {
 			fp = *rp++;
+
+			error = pledge_recvfd_check(p, fp);
+			if (error)
+				break;
 			/*
 			 * No to block devices.  If passing a directory,
 			 * make sure that it is underneath the root.
@@ -844,6 +849,10 @@ morespace:
 			error = EDEADLK;
 			goto fail;
 		}
+		error = pledge_sendfd_check(p, fp);
+		if (error)
+			goto fail;
+		    
 		/* kq and systrace descriptors cannot be copied */
 		if (fp->f_type == DTYPE_KQUEUE ||
 		    fp->f_type == DTYPE_SYSTRACE) {
diff --git a/sys/sys/pledge.h b/sys/sys/pledge.h
index 7d2d46e5630..67af2b60688 100644
--- a/sys/sys/pledge.h
+++ b/sys/sys/pledge.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pledge.h,v 1.6 2015/10/18 00:04:43 deraadt Exp $	*/
+/*	$OpenBSD: pledge.h,v 1.7 2015/10/18 20:15:10 deraadt Exp $	*/
 
 /*
  * Copyright (c) 2015 Nicholas Marriott <nicm@openbsd.org>
@@ -61,8 +61,8 @@ int	pledge_namei(struct proc *, char *);
 void	pledge_aftersyscall(struct proc *, int, int);
 
 struct mbuf;
-int	pledge_cmsg_send(struct proc *p, struct mbuf *control);
-int	pledge_cmsg_recv(struct proc *p, struct mbuf *control);
+int	pledge_sendfd_check(struct proc *p, struct file *);
+int	pledge_recvfd_check(struct proc *p, struct file *);
 int	pledge_sysctl_check(struct proc *p, int namelen, int *name, void *new);
 int	pledge_chown_check(struct proc *p, uid_t, gid_t);
 int	pledge_adjtime_check(struct proc *p, const void *v);