Add tame(2) to file(1) and drop the old systrace(4) sandbox. tame(2) is

only applied to the child process, which requires the parent to not pass
directory file descriptors (tame("cmsg") does not allow it). Because
file(1) is already privsep, the permissions in the child can be quickly
restricted: first to "stdio cmsg getpw proc" then after the privdrop to
"stdio cmsg".

3 files changed, 44 insertions, 168 deletions

diff --git a/usr.bin/file/Makefile b/usr.bin/file/Makefile
index 5ce922e7acc..b126138bf60 100644
--- a/usr.bin/file/Makefile
+++ b/usr.bin/file/Makefile
@@ -1,7 +1,7 @@
-# $OpenBSD: Makefile,v 1.15 2015/04/27 13:52:17 nicm Exp $
+# $OpenBSD: Makefile,v 1.16 2015/10/04 07:25:59 nicm Exp $
 
 PROG=   file
-SRCS=   file.c magic-dump.c magic-load.c magic-test.c magic-common.c sandbox.c \
+SRCS=   file.c magic-dump.c magic-load.c magic-test.c magic-common.c \
 	text.c xmalloc.c
 MAN=	file.1 magic.5
 
diff --git a/usr.bin/file/file.c b/usr.bin/file/file.c
index b6816bcaeab..4fd132ef935 100644
--- a/usr.bin/file/file.c
+++ b/usr.bin/file/file.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: file.c,v 1.48 2015/10/02 18:06:27 deraadt Exp $ */
+/* $OpenBSD: file.c,v 1.49 2015/10/04 07:25:59 nicm Exp $ */
 
 /*
  * Copyright (c) 2015 Nicholas Marriott <nicm@openbsd.org>
@@ -116,7 +116,7 @@ usage(void)
 int
 main(int argc, char **argv)
 {
-	int			 opt, pair[2], fd, idx;
+	int			 opt, pair[2], fd, idx, mode;
 	char			*home;
 	struct passwd		*pw;
 	struct imsgbuf		 ibuf;
@@ -192,8 +192,10 @@ main(int argc, char **argv)
 	parent = getpid();
 	if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pair) != 0)
 		err(1, "socketpair");
-	pid = sandbox_fork(FILE_USER);
-	if (pid == 0) {
+	switch (pid = fork()) {
+	case -1:
+		err(1, "fork");
+	case 0:
 		close(pair[0]);
 		child(pair[1], parent, argc, argv);
 	}
@@ -220,10 +222,21 @@ main(int argc, char **argv)
 			fd = -1;
 			msg.error = errno;
 		} else {
-			fd = open(argv[idx], O_RDONLY|O_NONBLOCK);
-			if (fd == -1 && (errno == ENFILE || errno == EMFILE))
-				err(1, "open");
-			if (S_ISLNK(msg.sb.st_mode))
+			/*
+			 * tame(2) doesn't let us pass directory file
+			 * descriptors around - but in fact we don't need them,
+			 * so just don't open directories or symlinks (which
+			 * could be to directories).
+			 */
+			mode = msg.sb.st_mode;
+			if (!S_ISDIR(mode) && !S_ISLNK(mode)) {
+				fd = open(argv[idx], O_RDONLY|O_NONBLOCK);
+				if (fd == -1 &&
+				    (errno == ENFILE || errno == EMFILE))
+					err(1, "open");
+			} else
+				fd = -1;
+			if (S_ISLNK(mode))
 				read_link(&msg, argv[idx]);
 		}
 		send_message(&ibuf, &msg, sizeof msg, fd);
@@ -328,6 +341,7 @@ read_link(struct input_msg *msg, const char *path)
 static __dead void
 child(int fd, pid_t parent, int argc, char **argv)
 {
+	struct passwd		*pw;
 	struct magic		*m;
 	struct imsgbuf		 ibuf;
 	struct imsg		 imsg;
@@ -337,6 +351,24 @@ child(int fd, pid_t parent, int argc, char **argv)
 	int			 i, idx;
 	size_t			 len, width = 0;
 
+	if (tame("stdio cmsg getpw proc", NULL) != 0)
+		err(1, "tame");
+
+	if (geteuid() == 0) {
+		pw = getpwnam(FILE_USER);
+		if (pw == NULL)
+			errx(1, "unknown user %s", FILE_USER);
+		if (setgroups(1, &pw->pw_gid) != 0)
+			err(1, "setgroups");
+		if (setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) != 0)
+			err(1, "setresgid");
+		if (setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) != 0)
+			err(1, "setresuid");
+	}
+
+	if (tame("stdio cmsg", NULL) != 0)
+		err(1, "tame");
+
 	m = magic_load(magicfp, magicpath, cflag || Wflag);
 	if (cflag) {
 		magic_dump(m);
@@ -524,6 +556,8 @@ try_access(struct input_file *inf)
 {
 	char tmp[256] = "";
 
+	if (inf->msg->sb.st_size == 0 && S_ISREG(inf->msg->sb.st_mode))
+		return (0); /* empty file */
 	if (inf->fd != -1)
 		return (0);
 
diff --git a/usr.bin/file/sandbox.c b/usr.bin/file/sandbox.c
deleted file mode 100644
index a4039709ef1..00000000000
--- a/usr.bin/file/sandbox.c
+++ /dev/null
@@ -1,158 +0,0 @@
-/* $OpenBSD: sandbox.c,v 1.9 2015/08/23 18:31:41 guenther Exp $ */
-
-/*
- * Copyright (c) 2015 Nicholas Marriott <nicm@openbsd.org>
- *
- * Permission to use, copy, modify, and distribute this software for any
- * purpose with or without fee is hereby granted, provided that the above
- * copyright notice and this permission notice appear in all copies.
- *
- * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
- * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
- * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
- * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
- * WHATSOEVER RESULTING FROM LOSS OF MIND, USE, DATA OR PROFITS, WHETHER
- * IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING
- * OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
- */
-
-#include <sys/types.h>
-#include <sys/ioctl.h>
-#include <sys/syscall.h>
-#include <sys/wait.h>
-
-#include <dev/systrace.h>
-
-#include <errno.h>
-#include <fcntl.h>
-#include <pwd.h>
-#include <signal.h>
-#include <unistd.h>
-
-#include "file.h"
-#include "magic.h"
-#include "xmalloc.h"
-
-static const struct
-{
-	int syscallnum;
-	int action;
-} allowed_syscalls[] = {
-	{ SYS_open, SYSTR_POLICY_NEVER }, /* for strerror */
-
-	{ SYS_close, SYSTR_POLICY_PERMIT },
-	{ SYS_exit, SYSTR_POLICY_PERMIT },
-	{ SYS_fcntl, SYSTR_POLICY_PERMIT },
-	{ SYS_fstat, SYSTR_POLICY_PERMIT },
-	{ SYS_getdtablecount, SYSTR_POLICY_PERMIT },
-	{ SYS_getentropy, SYSTR_POLICY_PERMIT },
-	{ SYS_getpid, SYSTR_POLICY_PERMIT },
-	{ SYS_getrlimit, SYSTR_POLICY_PERMIT },
-	{ SYS_issetugid, SYSTR_POLICY_PERMIT },
-	{ SYS_kbind, SYSTR_POLICY_PERMIT },
-	{ SYS_madvise, SYSTR_POLICY_PERMIT },
-	{ SYS_mmap, SYSTR_POLICY_PERMIT },
-	{ SYS_mprotect, SYSTR_POLICY_PERMIT },
-	{ SYS_mquery, SYSTR_POLICY_PERMIT },
-	{ SYS_munmap, SYSTR_POLICY_PERMIT },
-	{ SYS_read, SYSTR_POLICY_PERMIT },
-	{ SYS_recvmsg, SYSTR_POLICY_PERMIT },
-	{ SYS_sendmsg, SYSTR_POLICY_PERMIT },
-	{ SYS_sigprocmask, SYSTR_POLICY_PERMIT },
-	{ SYS_write, SYSTR_POLICY_PERMIT },
-
-	{ -1, -1 }
-};
-
-static int
-sandbox_find(int syscallnum)
-{
-	int	i;
-
-	for (i = 0; allowed_syscalls[i].syscallnum != -1; i++) {
-		if (allowed_syscalls[i].syscallnum == syscallnum)
-			return (allowed_syscalls[i].action);
-	}
-	return (SYSTR_POLICY_KILL);
-}
-
-static int
-sandbox_child(const char *user)
-{
-	struct passwd	*pw;
-
-	if (geteuid() == 0) {
-		pw = getpwnam(user);
-		if (pw == NULL)
-			errx(1, "unknown user %s", user);
-		if (setgroups(1, &pw->pw_gid) != 0)
-			err(1, "setgroups");
-		if (setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) != 0)
-			err(1, "setresgid");
-		if (setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) != 0)
-			err(1, "setresuid");
-	}
-
-	if (kill(getpid(), SIGSTOP) != 0)
-		err(1, "kill(SIGSTOP)");
-	return (0);
-}
-
-int
-sandbox_fork(const char *user)
-{
-	pid_t			 pid;
-	int			 status, devfd, fd, i;
-	struct systrace_policy	 policy;
-
-	switch (pid = fork()) {
-	case -1:
-		err(1, "fork");
-	case 0:
-		return (sandbox_child(user));
-	}
-
-	/*
-	 * Wait for the child to stop itself with SIGSTOP before assigning the
-	 * policy, before that it might still be calling syscalls the policy
-	 * would block.
-	 */
-	do {
-		pid = waitpid(pid, &status, WUNTRACED);
-	} while (pid == -1 && errno == EINTR);
-	if (!WIFSTOPPED(status))
-		errx(1, "child not stopped");
-
-	devfd = open("/dev/systrace", O_RDONLY);
-	if (devfd == -1)
-		err(1, "open(\"/dev/systrace\")");
-	if (ioctl(devfd, STRIOCCLONE, &fd) == -1)
-		err(1, "ioctl(STRIOCCLONE)");
-	close(devfd);
-
-	if (ioctl(fd, STRIOCATTACH, &pid) == -1)
-		goto out;
-
-	memset(&policy, 0, sizeof policy);
-	policy.strp_op = SYSTR_POLICY_NEW;
-	policy.strp_maxents = SYS_MAXSYSCALL;
-	if (ioctl(fd, STRIOCPOLICY, &policy) == -1)
-		err(1, "ioctl(STRIOCPOLICY/NEW)");
-	policy.strp_op = SYSTR_POLICY_ASSIGN;
-	policy.strp_pid = pid;
-	if (ioctl(fd, STRIOCPOLICY, &policy) == -1)
-		err(1, "ioctl(STRIOCPOLICY/ASSIGN)");
-
-	for (i = 0; i < SYS_MAXSYSCALL; i++) {
-		policy.strp_op = SYSTR_POLICY_MODIFY;
-		policy.strp_code = i;
-		policy.strp_policy = sandbox_find(i);
-		if (ioctl(fd, STRIOCPOLICY, &policy) == -1)
-			err(1, "ioctl(STRIOCPOLICY/MODIFY)");
-	}
-
-out:
-	if (kill(pid, SIGCONT) != 0)
-		err(1, "kill(SIGCONT)");
-	return (pid);
-}