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@

diff --git a/sys/kern/kern_pledge.c b/sys/kern/kern_pledge.c
index 5094593..546d938 100644 (file)
--- 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 45dd646..40c3834 100644 (file)
--- 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 7daa79f..6598f81 100644 (file)
--- 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 01cd786..6a25840 100644 (file)
--- 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 910cc35..578efb0 100644 (file)
--- 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 */