clear whitelisted-paths view in pledge.

the following diff adds a clear view of whitelisted-paths in pledge.

before, whitelisting "/usr/local/bin" path would make only "/usr/local/bin"
VNODE was present and let "/usr/local", "/usr", and "/" been ENOENT. It was a
somehow odd filesystem hierarchy, and it breaks realpath(3).

with this diff, the directories that are one of the parents of a
whitelisted-directory become visible to stat(2) related syscalls, but only
with restricted permissions: stat(2) will lie a bit, and saying they owned by
root:wheel and mode is --x--x--x. Note that only stat(2) is affected by this
"view", and the owner/mode aren't effectively changed: it is just a "lie".

while here, refactor a bit pledge_namei() in order to avoid multiple for-loop
on whitelisted-path array.

ok deraadt@

diff --git a/sys/kern/kern_pledge.c b/sys/kern/kern_pledge.c
index e78216a..626abb4 100644 (file)
--- a/sys/kern/kern_pledge.c
+++ b/sys/kern/kern_pledge.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: kern_pledge.c,v 1.59 2015/10/20 05:18:34 deraadt Exp $        */
+/*     $OpenBSD: kern_pledge.c,v 1.60 2015/10/20 06:40:00 semarie Exp $        */
 
 /*
  * Copyright (c) 2015 Nicholas Marriott <nicm@openbsd.org>
@@ -56,6 +56,7 @@
 #include "pty.h"
 
 int canonpath(const char *input, char *buf, size_t bufsize);
+int substrcmp(const char *p1, size_t s1, const char *p2, size_t s2);
 
 const u_int pledge_syscalls[SYS_MAXSYSCALL] = {
        [SYS_exit] = 0xffffffff,
@@ -657,8 +658,8 @@ pledge_namei(struct proc *p, char *origpath)
        if (p->p_p->ps_pledgepaths) {
                struct whitepaths *wl = p->p_p->ps_pledgepaths;
                char *fullpath, *builtpath = NULL, *canopath = NULL;
-               size_t builtlen = 0;
-               int i, error;
+               size_t builtlen = 0, canopathlen;
+               int i, error, pardir_found;
 
                if (origpath[0] != '/') {
                        char *cwdpath, *cwd, *bp, *bpend;
@@ -704,16 +705,42 @@ pledge_namei(struct proc *p, char *origpath)
                //    (long long)strlen(canopath));
 
                error = ENOENT;
+               canopathlen = strlen(canopath);
+               pardir_found = 0;
                for (i = 0; i < wl->wl_count && wl->wl_paths[i].name && error; i++) {
-                       if (strncmp(canopath, wl->wl_paths[i].name,
-                           wl->wl_paths[i].len - 1) == 0) {
+                       int substr = substrcmp(wl->wl_paths[i].name,
+                           wl->wl_paths[i].len - 1, canopath, canopathlen);
+
+                       //printf("pledge: check: %s [%ld] %s [%ld] = %d\n",
+                       //    wl->wl_paths[i].name, wl->wl_paths[i].len - 1,
+                       //    canopath, canopathlen,
+                       //    substr);
+
+                       /* wl_paths[i].name is a substring of canopath */
+                       if (substr == 1) {
                                u_char term = canopath[wl->wl_paths[i].len - 1];
 
                                if (term == '\0' || term == '/' ||
                                    wl->wl_paths[i].name[1] == '\0')
                                        error = 0;
+
+                       /* canopath is a substring of wl_paths[i].name */
+                       } else if (substr == 2) {
+                               u_char term = wl->wl_paths[i].name[canopathlen];
+
+                               if (canopath[1] == '\0' || term == '/')
+                                       pardir_found = 1;
                        }
                }
+               if (pardir_found)
+                       switch (p->p_pledge_syscall) {
+                       case SYS_stat:
+                       case SYS_lstat:
+                       case SYS_fstatat:
+                       case SYS_fstat:
+                               p->p_pledgenote |= TMN_STATLIE;
+                               error = 0;
+                       }
                free(canopath, M_TEMP, MAXPATHLEN);
                return (error);                 /* Don't hint why it failed */
        }
@@ -1254,3 +1281,19 @@ canonpath(const char *input, char *buf, size_t bufsize)
 
        return 0;
 }
+
+int
+substrcmp(const char *p1, size_t s1, const char *p2, size_t s2)
+{
+       size_t i;
+       for (i = 0; i < s1 || i < s2; i++) {
+               if (p1[i] != p2[i])
+                       break;
+       }
+       if (i == s1) {
+               return (1);     /* string1 is a subpath of string2 */
+       } else if (i == s2)
+               return (2);     /* string2 is a subpath of string1 */
+       else
+               return (0);     /* no subpath */
+}

diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c
index 35ad67a..2c16fb5 100644 (file)
--- a/sys/kern/vfs_syscalls.c
+++ b/sys/kern/vfs_syscalls.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: vfs_syscalls.c,v 1.231 2015/10/16 13:37:43 millert Exp $      */
+/*     $OpenBSD: vfs_syscalls.c,v 1.232 2015/10/20 06:40:00 semarie Exp $      */
 /*     $NetBSD: vfs_syscalls.c,v 1.71 1996/04/23 10:29:02 mycroft Exp $        */
 
 /*
@@ -1741,6 +1741,16 @@ dofstatat(struct proc *p, int fd, const char *path, struct stat *buf, int flag)
        vput(nd.ni_vp);
        if (error)
                return (error);
+       if (p->p_pledgenote & TMN_STATLIE) {
+               if (S_ISDIR(sb.st_mode)) {
+                       sb.st_mode &= ~ALLPERMS;
+                       sb.st_mode |= S_IXUSR | S_IXGRP | S_IXOTH;
+                       sb.st_uid = 0;
+                       sb.st_gid = 0;
+                       sb.st_gen = 0;
+               } else
+                       return (ENOENT);
+       }
        /* Don't let non-root see generation numbers (for NFS security) */
        if (suser(p, 0))
                sb.st_gen = 0;

diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index b90021e..bea8af8 100644 (file)
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: proc.h,v 1.208 2015/10/18 00:04:43 deraadt Exp $      */
+/*     $OpenBSD: proc.h,v 1.209 2015/10/20 06:40:01 semarie Exp $      */
 /*     $NetBSD: proc.h,v 1.44 1996/04/22 01:23:21 christos Exp $       */
 
 /*-
@@ -334,6 +334,7 @@ struct proc {
 #define TMN_XPATH      0x00000008
 #define TMN_FATTR      0x00000010
 #define TMN_COREDUMP   0x00000020
+#define TMN_STATLIE    0x00000040
        int     p_pledgeafter;
 #define TMA_YPLOCK     0x00000001