Hi,
two notes about vmd with this diff:
1. "vmm" pledge can be !fdpledged as well as it already pre-opens the
/dev/vmm fd for ioctls. I added the following chunk on top of your
diff and it works as expected:
---snip---
if ((p->p_p->ps_pledge & PLEDGE_VMM)) {
#if NVMM > 0
- if ((fp->f_type == DTYPE_VNODE) &&
+ if (!fdpledged &&
+ (fp->f_type == DTYPE_VNODE) &&
(vp->v_type == VCHR) &&
(cdevsw[major(vp->v_rdev)].d_open == vmmopen)) {
error = pledge_ioctl_vmm(p, com);
if (error == 0)
return 0;
}
-#endif
+#endif /* NVMM > 0 */
}
---snap---
2. vmd calls openpty() in the pledged parent whenever a new VM is
started - effectively doing ioctls on post-pledge fds. I will
probably solve this by opening the pty in the non-pledged "priv"
process, and do some additional passing, but then I'll also have to
give up its chroot to access /dev/.
vmd: ioctl 40287401 post-pledge fd 12
vmd(51681): syscall 54 "tty"
Reyk
On Tue, Jan 24, 2017 at 07:36:47PM -0700, Theo de Raadt wrote:
> Here is the proposed ioctl lock-down policy for file descriptors
> allocated in a process before pledge(2).
>
> The manual page diff is first, that explains the direction this is
> going.
>
> The other supporting code has been commited already, so feel free
> to take this for a ride and let's see what programs need further
> modification to cope with it.
>
> Index: lib/libc/sys/pledge.2
> ===================================================================
> RCS file: /cvs/src/lib/libc/sys/pledge.2,v
> retrieving revision 1.39
> diff -u -p -u -r1.39 pledge.2
> --- lib/libc/sys/pledge.2 23 Jan 2017 07:19:39 -0000 1.39
> +++ lib/libc/sys/pledge.2 24 Jan 2017 10:04:31 -0000
> @@ -43,6 +43,16 @@ Subsequent calls to
> .Fn pledge
> can reduce the abilities further, but abilities can never be regained.
> .Pp
> +New file descriptors created by a pledged process are annotated internally
> +to permit a greater set of
> +.Xr ioctl 2
> +operations.
> +File descriptors obtained via
> +.Xr dup 2
> +or
> +.Xr recvmsg 2
> +retain the annotation from the process who originally opened them.
> +.Pp
> A process which attempts a restricted operation is killed with an uncatchable
> .Dv SIGABRT ,
> delivering a core file if possible.
> Index: sys/sys/pledge.h
> ===================================================================
> RCS file: /cvs/src/sys/sys/pledge.h,v
> retrieving revision 1.30
> diff -u -p -u -r1.30 pledge.h
> --- sys/sys/pledge.h 23 Jan 2017 04:25:05 -0000 1.30
> +++ sys/sys/pledge.h 23 Jan 2017 11:40:58 -0000
> @@ -126,7 +126,7 @@ int pledge_adjtime(struct proc *p, const
> int pledge_sendit(struct proc *p, const void *to);
> int pledge_sockopt(struct proc *p, int set, int level, int optname);
> int pledge_socket(struct proc *p, int domain, int state);
> -int pledge_ioctl(struct proc *p, long com, struct file *);
> +int pledge_ioctl(struct proc *p, long com, struct file *, int, int);
> int pledge_ioctl_drm(struct proc *p, long com, dev_t device);
> int pledge_ioctl_vmm(struct proc *p, long com);
> int pledge_flock(struct proc *p);
> Index: sys/kern/kern_pledge.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_pledge.c,v
> retrieving revision 1.192
> diff -u -p -u -r1.192 kern_pledge.c
> --- sys/kern/kern_pledge.c 23 Jan 2017 05:49:24 -0000 1.192
> +++ sys/kern/kern_pledge.c 24 Jan 2017 08:19:03 -0000
> @@ -66,6 +66,7 @@
>
> #include "audio.h"
> #include "pf.h"
> +#include "bpfilter.h"
> #include "pty.h"
>
> #if defined(__amd64__) || defined(__i386__)
> @@ -538,6 +539,22 @@ sys_pledge(struct proc *p, void *v, regi
> }
>
> if (SCARG(uap, request)) {
> +#ifdef DIAGNOSTIC
> + if (!ISSET(pr->ps_flags, PS_PLEDGE)) {
> + struct filedesc *fdp = p->p_fd;
> + int fd;
> +
> + fdplock(fdp);
> + for (fd = 0; fd <= fdp->fd_lastfile; fd++) {
> + if (fdp->fd_ofiles[fd] &&
> + (fdp->fd_ofileflags[fd] & UF_PLEDGED))
> + printf("%s: fd %d was set!\n",
> + pr->ps_comm, fd);
> + fdp->fd_ofileflags[fd] &= ~UF_PLEDGED;
> + }
> + fdpunlock(fdp);
> + }
> +#endif
> pr->ps_pledge = flags;
> pr->ps_flags |= PS_PLEDGE;
> }
> @@ -1102,7 +1119,7 @@ pledge_sendit(struct proc *p, const void
> }
>
> int
> -pledge_ioctl(struct proc *p, long com, struct file *fp)
> +pledge_ioctl(struct proc *p, long com, struct file *fp, int fd, int
> fdpledged)
> {
> struct vnode *vp = NULL;
> int error = EPERM;
> @@ -1137,30 +1154,32 @@ pledge_ioctl(struct proc *p, long com, s
> }
> }
>
> +#if NBPFILTER > 0
> if ((p->p_p->ps_pledge & PLEDGE_BPF)) {
> switch (com) {
> - case BIOCGSTATS: /* bpf: tcpdump privsep on ^C */
> - if (fp->f_type == DTYPE_VNODE &&
> - fp->f_ops->fo_ioctl == vn_ioctl)
> + case BIOCGSTATS: /* tcpdump, pflogd */
> + if (!fdpledged &&
> + fp->f_type == DTYPE_VNODE &&
> + vp->v_type == VCHR &&
> + cdevsw[major(vp->v_rdev)].d_open == bpfopen)
> return (0);
> break;
> }
> }
> +#endif /* NBPFILTER > 0 */
>
> if ((p->p_p->ps_pledge & PLEDGE_TAPE)) {
> switch (com) {
> - case MTIOCGET:
> + case MTIOCGET: /* pax */
> case MTIOCTOP:
> - /* for pax(1) and such, checking tapes... */
> if (fp->f_type == DTYPE_VNODE &&
> (vp->v_type == VCHR || vp->v_type == VBLK))
> return (0);
> - break;
> }
> }
>
> - if ((p->p_p->ps_pledge & PLEDGE_DRM)) {
> #if NDRM > 0
> + if ((p->p_p->ps_pledge & PLEDGE_DRM)) {
> if ((fp->f_type == DTYPE_VNODE) &&
> (vp->v_type == VCHR) &&
> (cdevsw[major(vp->v_rdev)].d_open == drmopen)) {
> @@ -1168,11 +1187,11 @@ pledge_ioctl(struct proc *p, long com, s
> if (error == 0)
> return 0;
> }
> -#endif /* NDRM > 0 */
> }
> +#endif /* NDRM > 0 */
>
> - if ((p->p_p->ps_pledge & PLEDGE_AUDIO)) {
> #if NAUDIO > 0
> + if ((p->p_p->ps_pledge & PLEDGE_AUDIO)) {
> switch (com) {
> case AUDIO_GETPOS:
> case AUDIO_GETPAR:
> @@ -1184,19 +1203,22 @@ pledge_ioctl(struct proc *p, long com, s
> cdevsw[major(vp->v_rdev)].d_open == audioopen)
> return (0);
> }
> -#endif /* NAUDIO > 0 */
> }
> +#endif /* NAUDIO > 0 */
>
> if ((p->p_p->ps_pledge & PLEDGE_DISKLABEL)) {
> switch (com) {
> - case DIOCGDINFO:
> - case DIOCGPDINFO:
> - case DIOCRLDINFO:
> + case DIOCRLDINFO: /* change operations, etc. */
> case DIOCWDINFO:
> case BIOCDISK:
> case BIOCINQ:
> case BIOCINSTALLBOOT:
> case BIOCVOL:
> + if (fdpledged)
> + break;
> + /* FALLTHROUGH */
> + case DIOCGDINFO: /* read operations, etc. */
> + case DIOCGPDINFO:
> if (fp->f_type == DTYPE_VNODE &&
> ((vp->v_type == VCHR &&
> cdevsw[major(vp->v_rdev)].d_type == D_DISK) ||
> @@ -1213,10 +1235,10 @@ pledge_ioctl(struct proc *p, long com, s
> }
> }
>
> - if ((p->p_p->ps_pledge & PLEDGE_PF)) {
> #if NPF > 0
> + if ((p->p_p->ps_pledge & PLEDGE_PF)) {
> switch (com) {
> - case DIOCADDRULE:
> + case DIOCADDRULE: /* relayd */
> case DIOCGETSTATUS:
> case DIOCNATLOOK:
> case DIOCRADDTABLES:
> @@ -1225,41 +1247,46 @@ pledge_ioctl(struct proc *p, long com, s
> case DIOCRCLRTSTATS:
> case DIOCRGETTSTATS:
> case DIOCRSETADDRS:
> - case DIOCXBEGIN:
> + case DIOCXBEGIN: /* relayd, proxies */
> case DIOCXCOMMIT:
> case DIOCKILLSRCNODES:
> - if ((fp->f_type == DTYPE_VNODE) &&
> + case DIOCRGETASTATS: /* bgpd */
> + case DIOCRDELADDRS:
> + case DIOCRADDADDRS:
> + if (!fdpledged &&
> + (fp->f_type == DTYPE_VNODE) &&
> (vp->v_type == VCHR) &&
> (cdevsw[major(vp->v_rdev)].d_open == pfopen))
> return (0);
> break;
> }
> -#endif
> }
> +#endif /* NPF > 0 */
>
> if ((p->p_p->ps_pledge & PLEDGE_TTY)) {
> switch (com) {
> #if NPTY > 0
> - case PTMGET:
> - if ((p->p_p->ps_pledge & PLEDGE_RPATH) == 0)
> - break;
> - if ((p->p_p->ps_pledge & PLEDGE_WPATH) == 0)
> - break;
> - if (fp->f_type != DTYPE_VNODE || vp->v_type != VCHR)
> - break;
> - if (cdevsw[major(vp->v_rdev)].d_open != ptmopen)
> - break;
> - return (0);
> + case PTMGET: /* tmux */
> + if (!fdpledged &&
> + (p->p_p->ps_pledge & PLEDGE_RPATH) &&
> + (p->p_p->ps_pledge & PLEDGE_WPATH) &&
> + fp->f_type == DTYPE_VNODE && vp->v_type == VCHR &&
> + cdevsw[major(vp->v_rdev)].d_open == ptmopen)
> + return (0);
> + break;
> #endif /* NPTY > 0 */
> - case TIOCSTI: /* ksh? csh? */
> - if ((p->p_p->ps_pledge & PLEDGE_PROC) &&
> + case TIOCSTI: /* csh */
> + if (!fdpledged &&
> + (p->p_p->ps_pledge & PLEDGE_PROC) &&
> fp->f_type == DTYPE_VNODE && (vp->v_flag & VISTTY))
> return (0);
> break;
> case TIOCSPGRP:
> - if ((p->p_p->ps_pledge & PLEDGE_PROC) == 0)
> - break;
> - /* FALLTHROUGH */
> + /* cannot do !fdpledged test */
> + if ((p->p_p->ps_pledge & PLEDGE_PROC) &&
> + fp->f_type == DTYPE_VNODE && (vp->v_flag & VISTTY))
> + return (0);
> + break;
> case TIOCFLUSH: /* getty, telnet */
> case TIOCGPGRP:
> case TIOCGETA:
> @@ -1267,7 +1294,7 @@ pledge_ioctl(struct proc *p, long com, s
> if (fp->f_type == DTYPE_VNODE && (vp->v_flag & VISTTY))
> return (0);
> return (ENOTTY);
> - case TIOCSWINSZ:
> + case TIOCSWINSZ: /* tmux, script */
> case TIOCEXT: /* mail, libedit .. */
> case TIOCCBRK: /* cu */
> case TIOCSBRK: /* cu */
> @@ -1275,7 +1302,7 @@ pledge_ioctl(struct proc *p, long com, s
> case TIOCSDTR: /* cu */
> case TIOCEXCL: /* cu */
> case TIOCSETA: /* cu, ... */
> - case TIOCSETAW: /* cu, ... */
> + case TIOCSETAW: /* tmux, cu, ksh ... */
> case TIOCSETAF: /* tcsetattr TCSAFLUSH, script */
> case TIOCSCTTY: /* forkpty(3), login_tty(3), ... */
> if (fp->f_type == DTYPE_VNODE && (vp->v_flag & VISTTY))
> @@ -1296,7 +1323,7 @@ pledge_ioctl(struct proc *p, long com, s
> case SIOCGNBRINFO_IN6:
> case SIOCGIFINFO_IN6:
> case SIOCGIFMEDIA:
> - if (fp->f_type == DTYPE_SOCKET)
> + if (!fdpledged && fp->f_type == DTYPE_SOCKET)
> return (0);
> break;
> }
> @@ -1311,9 +1338,11 @@ pledge_ioctl(struct proc *p, long com, s
> if (error == 0)
> return 0;
> }
> -#endif
> +#endif /* NVMM > 0 */
> }
>
> + if (fdpledged)
> + printf("%s: ioctl %08lx post-pledge fd %d\n", p->p_p->ps_comm,
> com, fd);
> return pledge_fail(p, error, PLEDGE_TTY);
> }
>
> Index: sys/kern/sys_generic.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/sys_generic.c,v
> retrieving revision 1.114
> diff -u -p -u -r1.114 sys_generic.c
> --- sys/kern/sys_generic.c 24 Jan 2017 00:58:55 -0000 1.114
> +++ sys/kern/sys_generic.c 24 Jan 2017 11:23:11 -0000
> @@ -390,17 +390,20 @@ sys_ioctl(struct proc *p, void *v, regis
> syscallarg(void *) data;
> } */ *uap = v;
> struct file *fp;
> - struct filedesc *fdp;
> + struct filedesc *fdp = p->p_fd;
> u_long com = SCARG(uap, com);
> int error;
> u_int size;
> caddr_t data, memp;
> - int tmp;
> + int tmp, pl;
> #define STK_PARAMS 128
> long long stkbuf[STK_PARAMS / sizeof(long long)];
>
> - fdp = p->p_fd;
> + fdplock(fdp);
> fp = fd_getfile_mode(fdp, SCARG(uap, fd), FREAD|FWRITE);
> + if (fp)
> + pl = fdp->fd_ofileflags[SCARG(uap, fd)] & UF_PLEDGED;
> + fdpunlock(fdp);
>
> if (fp == NULL)
> return (EBADF);
> @@ -412,7 +415,7 @@ sys_ioctl(struct proc *p, void *v, regis
> return (EINVAL);
> }
>
> - error = pledge_ioctl(p, com, fp);
> + error = pledge_ioctl(p, com, fp, SCARG(uap, fd), pl);
> if (error)
> return (error);
>
>
--
