From: semarie <semarie@openbsd.org>
Date: Tue, 20 Oct 2015 06:40:00 +0000 (+0000)
Subject: clear whitelisted-paths view in pledge.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=9edc603d532065ffbf5fbddc3275fc67c61646cb;p=openbsd

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 e78216a7fda..626abb413ed 100644
--- 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 35ad67a9f85..2c16fb5019b 100644
--- 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 b90021e7836..bea8af81667 100644
--- 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