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 1001900..da07378 100644 (file)
--- 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 bd6793d..b80c055 100644 (file)
--- 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 c3632bd..ebf2347 100644 (file)
--- 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 7d2d46e..67af2b6 100644 (file)
--- 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);