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@

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 <nicm@openbsd.org>
@@ -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 <beck@openbsd.org>
@@ -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 <nicm@openbsd.org>
@@ -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 */