From 9dfd71d84e7615be3460d3218da4f57cd1899dd3 Mon Sep 17 00:00:00 2001 From: beck Date: Sat, 11 Aug 2018 16:16:07 +0000 Subject: [PATCH] Get rid of PLEDGE_STAT, which was a hack used for unveil. We use UNVEIL_INSPECT instead in the unveil flags for the same purpose, and now add traversed vnodes of a path with UNVEIL_INSPECT instead of with 0 flags and voodoo in unveil_flagmatch. This allows us to uncontort the logic of unveil_flagmatch a bunch. helpful review and ok from semarie@ --- sys/kern/kern_pledge.c | 14 +++++++------- sys/kern/kern_unveil.c | 37 +++++++++++++++++-------------------- sys/kern/vfs_syscalls.c | 14 +++++++------- sys/sys/namei.h | 4 +++- sys/sys/pledge.h | 3 +-- 5 files changed, 35 insertions(+), 37 deletions(-) diff --git a/sys/kern/kern_pledge.c b/sys/kern/kern_pledge.c index 5094593de5d..546d9389ad4 100644 --- a/sys/kern/kern_pledge.c +++ b/sys/kern/kern_pledge.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_pledge.c,v 1.239 2018/08/02 15:34:07 rob Exp $ */ +/* $OpenBSD: kern_pledge.c,v 1.240 2018/08/11 16:16:07 beck Exp $ */ /* * Copyright (c) 2015 Nicholas Marriott @@ -608,14 +608,14 @@ pledge_namei(struct proc *p, struct nameidata *ni, char *origpath) switch (p->p_pledge_syscall) { case SYS_access: /* tzset() needs this. */ - if ((ni->ni_pledge == (PLEDGE_RPATH | PLEDGE_STAT)) && + if (ni->ni_pledge == PLEDGE_RPATH && strcmp(path, "/etc/localtime") == 0) { ni->ni_cnd.cn_flags |= BYPASSUNVEIL; return (0); } /* when avoiding YP mode, getpw* functions touch this */ - if (ni->ni_pledge == (PLEDGE_RPATH | PLEDGE_STAT) && + if (ni->ni_pledge == PLEDGE_RPATH && strcmp(path, "/var/run/ypbind.lock") == 0) { if (p->p_p->ps_pledge & PLEDGE_GETPW) { ni->ni_cnd.cn_flags |= BYPASSUNVEIL; @@ -713,7 +713,7 @@ pledge_namei(struct proc *p, struct nameidata *ni, char *origpath) break; case SYS_readlink: /* Allow /etc/malloc.conf for malloc(3). */ - if ((ni->ni_pledge == (PLEDGE_RPATH | PLEDGE_STAT)) && + if ((ni->ni_pledge == PLEDGE_RPATH) && strcmp(path, "/etc/malloc.conf") == 0) { ni->ni_cnd.cn_flags |= BYPASSUNVEIL; return (0); @@ -721,7 +721,7 @@ pledge_namei(struct proc *p, struct nameidata *ni, char *origpath) break; case SYS_stat: /* DNS needs /etc/resolv.conf. */ - if ((ni->ni_pledge == (PLEDGE_RPATH | PLEDGE_STAT)) && + if ((ni->ni_pledge == PLEDGE_RPATH) && (p->p_p->ps_pledge & PLEDGE_DNS) && strcmp(path, "/etc/resolv.conf") == 0) { ni->ni_cnd.cn_flags |= BYPASSUNVEIL; @@ -732,9 +732,9 @@ pledge_namei(struct proc *p, struct nameidata *ni, char *origpath) /* * Ensure each flag of ni_pledge has counterpart allowing it in - * ps_pledge. discard PLEDGE_STAT as it is unveil(2) stuff. + * ps_pledge. */ - if ((ni->ni_pledge & ~PLEDGE_STAT) & ~p->p_p->ps_pledge) + if (ni->ni_pledge & ~p->p_p->ps_pledge) return (pledge_fail(p, EPERM, (ni->ni_pledge & ~p->p_p->ps_pledge))); /* continue, and check unveil if present */ diff --git a/sys/kern/kern_unveil.c b/sys/kern/kern_unveil.c index 45dd646febc..40c383489b6 100644 --- a/sys/kern/kern_unveil.c +++ b/sys/kern/kern_unveil.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_unveil.c,v 1.12 2018/08/07 15:07:54 deraadt Exp $ */ +/* $OpenBSD: kern_unveil.c,v 1.13 2018/08/11 16:16:07 beck Exp $ */ /* * Copyright (c) 2017-2018 Bob Beck @@ -378,9 +378,15 @@ void unveil_add_traversed_vnodes(struct proc *p, struct nameidata *ndp) { /* - * add the traversed vnodes with 0 flags if they - * are not already present. + * Add the traversed vnodes with the UNVEIL_INSPECT flag + * if they are not already present to allow traversal + * operations such as access and stat. This lets + * TOCTOU fans that call access on all components of + * an unveil'ed path before the final operation + * work. */ + struct unveil *uv; + if (ndp->ni_tvpsize) { size_t i; @@ -389,7 +395,8 @@ unveil_add_traversed_vnodes(struct proc *p, struct nameidata *ndp) if (unveil_lookup(vp, p) == NULL) { vref(vp); vp->v_uvcount++; - unveil_add_vnode(p->p_p, vp); + uv = unveil_add_vnode(p->p_p, vp); + uv->uv_flags = UNVEIL_INSPECT; } } } @@ -533,26 +540,11 @@ int unveil_flagmatch(struct nameidata *ni, u_char flags) { if (flags == 0) { - /* XXX Fix this, you can do it better */ - if (ni->ni_pledge & PLEDGE_STAT) { -#ifdef DEBUG_UNVEIL - printf("allowing stat/accesss for 0 flags"); -#endif - SET(ni->ni_pledge, PLEDGE_STATLIE); - return 1; - } #ifdef DEBUG_UNVEIL printf("All operations forbidden for 0 flags\n"); #endif return 0; } - if (ni->ni_pledge & PLEDGE_STAT) { -#ifdef DEBUG_UNVEIL - printf("Allowing stat for nonzero flags\n"); -#endif - CLR(ni->ni_pledge, PLEDGE_STATLIE); - return 1; - } if (ni->ni_unveil & UNVEIL_READ) { if ((flags & UNVEIL_READ) == 0) { #ifdef DEBUG_UNVEIL @@ -585,6 +577,11 @@ unveil_flagmatch(struct nameidata *ni, u_char flags) return 0; } } + if (ni->ni_unveil & UNVEIL_INSPECT) { +#ifdef DEBUG_UNVEIL + printf("any unveil allows UNVEIL_INSPECT\n"); +#endif + } return 1; } @@ -602,7 +599,7 @@ unveil_check_component(struct proc *p, struct nameidata *ni, struct vnode *dp) (uv = unveil_lookup(dp, p)) != NULL) { /* if directory flags match, it's a match */ if (unveil_flagmatch(ni, uv->uv_flags)) { - if (uv->uv_flags) { + if (uv->uv_flags & UNVEIL_USERSET) { ni->ni_unveil_match = uv; #ifdef DEBUG_UNVEIL printf("unveil: %s(%d): component directory match" diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c index 7daa79f56f9..6598f81e667 100644 --- a/sys/kern/vfs_syscalls.c +++ b/sys/kern/vfs_syscalls.c @@ -1,4 +1,4 @@ -/* $OpenBSD: vfs_syscalls.c,v 1.301 2018/08/05 14:23:57 beck Exp $ */ +/* $OpenBSD: vfs_syscalls.c,v 1.302 2018/08/11 16:16:07 beck Exp $ */ /* $NetBSD: vfs_syscalls.c,v 1.71 1996/04/23 10:29:02 mycroft Exp $ */ /* @@ -1809,8 +1809,8 @@ dofaccessat(struct proc *p, int fd, const char *path, int amode, int flag) } NDINITAT(&nd, LOOKUP, FOLLOW | LOCKLEAF, UIO_USERSPACE, fd, path, p); - nd.ni_pledge = PLEDGE_RPATH | PLEDGE_STAT; - nd.ni_unveil = 0; /* XXX fix this when we fix PLEDGE_STAT */ + nd.ni_pledge = PLEDGE_RPATH; + nd.ni_unveil = UNVEIL_INSPECT; if ((error = namei(&nd)) != 0) goto out; vp = nd.ni_vp; @@ -1880,8 +1880,8 @@ dofstatat(struct proc *p, int fd, const char *path, struct stat *buf, int flag) follow = (flag & AT_SYMLINK_NOFOLLOW) ? NOFOLLOW : FOLLOW; NDINITAT(&nd, LOOKUP, follow | LOCKLEAF, UIO_USERSPACE, fd, path, p); - nd.ni_pledge = PLEDGE_RPATH | PLEDGE_STAT; - nd.ni_unveil = 0; + nd.ni_pledge = PLEDGE_RPATH; + nd.ni_unveil = UNVEIL_INSPECT; if ((error = namei(&nd)) != 0) return (error); error = vn_stat(nd.ni_vp, &sb, p); @@ -1989,8 +1989,8 @@ doreadlinkat(struct proc *p, int fd, const char *path, char *buf, struct nameidata nd; NDINITAT(&nd, LOOKUP, NOFOLLOW | LOCKLEAF, UIO_USERSPACE, fd, path, p); - nd.ni_pledge = PLEDGE_RPATH | PLEDGE_STAT; - nd.ni_unveil = 0; + nd.ni_pledge = PLEDGE_RPATH; + nd.ni_unveil = UNVEIL_INSPECT; if ((error = namei(&nd)) != 0) return (error); vp = nd.ni_vp; diff --git a/sys/sys/namei.h b/sys/sys/namei.h index 01cd786c9ed..6a25840319e 100644 --- a/sys/sys/namei.h +++ b/sys/sys/namei.h @@ -1,4 +1,4 @@ -/* $OpenBSD: namei.h,v 1.36 2018/08/05 14:23:57 beck Exp $ */ +/* $OpenBSD: namei.h,v 1.37 2018/08/11 16:16:07 beck Exp $ */ /* $NetBSD: namei.h,v 1.11 1996/02/09 18:25:20 christos Exp $ */ /* @@ -257,5 +257,7 @@ struct nchstats { #define UNVEIL_WRITE 0x02 #define UNVEIL_CREATE 0x04 #define UNVEIL_EXEC 0x08 +#define UNVEIL_USERSET 0x0F +#define UNVEIL_INSPECT 0x80 #endif /* !_SYS_NAMEI_H_ */ diff --git a/sys/sys/pledge.h b/sys/sys/pledge.h index 910cc351ea3..578efb0615b 100644 --- a/sys/sys/pledge.h +++ b/sys/sys/pledge.h @@ -1,4 +1,4 @@ -/* $OpenBSD: pledge.h,v 1.37 2018/07/13 09:25:23 beck Exp $ */ +/* $OpenBSD: pledge.h,v 1.38 2018/08/11 16:16:07 beck Exp $ */ /* * Copyright (c) 2015 Nicholas Marriott @@ -68,7 +68,6 @@ * to track program behaviours which have been observed. */ #define PLEDGE_USERSET 0x0fffffffffffffffULL -#define PLEDGE_STAT 0x2000000000000000ULL /* XXX this is a stat */ #define PLEDGE_STATLIE 0x4000000000000000ULL #define PLEDGE_YPACTIVE 0x8000000000000000ULL /* YP use detected and allowed */ -- 2.20.1