a few fixes to systrace

- add an exec message so that whenever a set-uid/gid process
  exec's a new image which we may control, the exec does not
  go by unnoticed.

- take special care to check for P_SUGIDEXEC as well as
  P_SUGID, corresponding to the same changes that were made in
  the ptrace code a while ago

ok niels@, sturm@; thanks to naddy for testing

4 files changed, 132 insertions, 35 deletions

diff --git a/sys/dev/systrace.c b/sys/dev/systrace.c
index 6faadffc21b..0d19bf5bb23 100644
--- a/sys/dev/systrace.c
+++ b/sys/dev/systrace.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: systrace.c,v 1.34 2003/10/21 05:24:40 jmc Exp $	*/
+/*	$OpenBSD: systrace.c,v 1.35 2004/06/23 05:16:35 marius Exp $	*/
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
  * All rights reserved.
@@ -117,7 +117,7 @@ struct str_process {
 	uid_t saveuid;
 	gid_t setegid;
 	gid_t savegid;
-
+	
 	struct str_message msg;
 };
 
@@ -703,8 +703,9 @@ systrace_redirect(int code, struct proc *p, void *v, register_t *retval)
 	 */
 	if (fst->issuser) {
 		maycontrol = 1;
-		issuser =1 ;
-	} else if (!(p->p_flag & P_SUGID)) {
+		issuser = 1;
+	} else if (!ISSET(p->p_flag, P_SUGID) &&
+	    !ISSET(p->p_flag, P_SUGIDEXEC)) {
 		maycontrol = fst->p_ruid == p->p_cred->p_ruid &&
 		    fst->p_rgid == p->p_cred->p_rgid;
 	}
@@ -816,7 +817,7 @@ systrace_redirect(int code, struct proc *p, void *v, register_t *retval)
 
 	systrace_replacefree(strp);
 
-	if (p->p_flag & P_SUGID) {
+	if (ISSET(p->p_flag, P_SUGID) || ISSET(p->p_flag, P_SUGIDEXEC)) {
 		if ((fst = strp->parent) == NULL || !fst->issuser) {
 			systrace_unlock();
 			return (error);
@@ -948,7 +949,6 @@ systrace_answer(struct str_process *strp, struct systrace_answer *ans)
 		SET(strp->flags, STR_PROC_SETEGID);
 		strp->setegid = ans->stra_setegid;
 	}
-	
 
 	/* Clearing the flag indicates to the process that it woke up */
 	CLR(strp->flags, STR_PROC_WAITANSWER);
@@ -1162,14 +1162,15 @@ systrace_attach(struct fsystrace *fst, pid_t pid)
 	 *	    gave us setuid/setgid privs (unless
 	 *	    you're root), or...
 	 *
-	 *      [Note: once P_SUGID gets set in execve(), it stays
-	 *	set until the process does another execve(). Hence
+	 *      [Note: once P_SUGID or P_SUGIDEXEC gets set in execve(),
+	 *      it stays set until the process does another execve(). Hence
 	 *	this prevents a setuid process which revokes its
 	 *	special privileges using setuid() from being
 	 *	traced. This is good security.]
 	 */
 	if ((proc->p_cred->p_ruid != p->p_cred->p_ruid ||
-		ISSET(proc->p_flag, P_SUGID)) &&
+		ISSET(proc->p_flag, P_SUGID) ||
+		ISSET(proc->p_flag, P_SUGIDEXEC)) &&
 	    (error = suser(p, 0)) != 0)
 		goto out;
 
@@ -1190,6 +1191,40 @@ systrace_attach(struct fsystrace *fst, pid_t pid)
 	return (error);
 }
 
+void
+systrace_execve(char *path, struct proc *p)
+{
+       struct str_process *strp;
+       struct fsystrace *fst;
+       struct str_msg_execve *msg_execve;
+
+       do { 
+               systrace_lock();
+               strp = p->p_systrace;
+               if (strp == NULL) {
+                       systrace_unlock();
+                       return;
+               }
+
+               msg_execve = &strp->msg.msg_data.msg_execve;
+               fst = strp->parent;
+               lockmgr(&fst->lock, LK_EXCLUSIVE, NULL, p);
+               systrace_unlock();
+
+               /*
+                * susers will get the execve call anyway.  Also, if
+                * we're not allowed to control the process, escape.
+                */
+               if (fst->issuser ||
+                   fst->p_ruid != p->p_cred->p_ruid ||
+                   fst->p_rgid != p->p_cred->p_rgid) {
+                       lockmgr(&fst->lock, LK_RELEASE, NULL, p);
+                       return;
+               }
+	       strlcpy(msg_execve->path, path, MAXPATHLEN);
+       } while (systrace_make_msg(strp, SYSTR_MSG_EXECVE) != 0);
+}
+
 /* Prepare to replace arguments */
 
 int
@@ -1295,7 +1330,6 @@ systrace_replace(struct str_process *strp, size_t argsize, register_t args[])
 int
 systrace_fname(struct str_process *strp, caddr_t kdata, size_t len)
 {
-
 	if (strp->nfname >= SYSTR_MAXFNAME || len < 2)
 		return EINVAL;
 
@@ -1326,6 +1360,7 @@ systrace_namei(struct nameidata *ndp)
 	struct fsystrace *fst;
 	struct componentname *cnp = &ndp->ni_cnd;
 	size_t i;
+	int hamper = 0;
 
 	systrace_lock();
 	strp = cnp->cn_proc->p_systrace;
@@ -1334,18 +1369,22 @@ systrace_namei(struct nameidata *ndp)
 		lockmgr(&fst->lock, LK_EXCLUSIVE, NULL, curproc);
 		systrace_unlock();
 
-		for (i = 0; i < strp->nfname; i++) {
+		for (i = 0; i < strp->nfname; i++)
 			if (strcmp(cnp->cn_pnbuf, strp->fname[i]) == 0) {
-				/* ELOOP if namei() tries to readlink */
-				ndp->ni_loopcnt = MAXSYMLINKS;
-				cnp->cn_flags &= ~FOLLOW;
-				cnp->cn_flags |= NOFOLLOW;
+				hamper = 1;
 				break;
 			}
-		}
+
 		lockmgr(&fst->lock, LK_RELEASE, NULL, curproc);
 	} else
 		systrace_unlock();
+
+	if (hamper) {
+		/* ELOOP if namei() tries to readlink */
+		ndp->ni_loopcnt = MAXSYMLINKS;
+		cnp->cn_flags &= ~FOLLOW;
+		cnp->cn_flags |= NOFOLLOW;
+	}
 }
 
 struct str_process *
@@ -1554,7 +1593,11 @@ systrace_make_msg(struct str_process *strp, int type)
 	struct str_message *msg = &strp->msg;
 	struct fsystrace *fst = strp->parent;
 	struct proc *p = strp->proc;
-	int st;
+	int st, pri;
+
+	pri = PWAIT|PCATCH;
+	if (type == SYSTR_MSG_EXECVE)
+		pri &= ~PCATCH;
 
 	msg->msg_seqnr = ++strp->seqnr;
 	msg->msg_type = type;
@@ -1578,7 +1621,7 @@ systrace_make_msg(struct str_process *strp, int type)
 	lockmgr(&fst->lock, LK_RELEASE, NULL, p);
 
 	while (1) {
-		st = tsleep(strp, PWAIT | PCATCH, "systrmsg", 0);
+		st = tsleep(strp, pri, "systrmsg", 0);
 		if (st != 0)
 			return (ERESTART);
 		/* If we detach, then everything is permitted */
diff --git a/sys/dev/systrace.h b/sys/dev/systrace.h
index b7d7de9cff1..e221ec32b13 100644
--- a/sys/dev/systrace.h
+++ b/sys/dev/systrace.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: systrace.h,v 1.15 2003/10/08 16:30:01 sturm Exp $	*/
+/*	$OpenBSD: systrace.h,v 1.16 2004/06/23 05:16:35 marius Exp $	*/
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
  * All rights reserved.
@@ -45,6 +45,10 @@ struct str_msg_ugid {
 	gid_t gid;
 };
 
+struct str_msg_execve {
+	char path[MAXPATHLEN];
+};
+
 #define SYSTR_MAX_POLICIES	64
 #define SYSTR_MAXARGS		64
 #define SYSTR_MAXFNAME		8
@@ -69,6 +73,7 @@ struct str_msg_child {
 #define SYSTR_MSG_CHILD		4
 #define SYSTR_MSG_UGID		5
 #define SYSTR_MSG_POLICYFREE	6
+#define SYSTR_MSG_EXECVE	7
 
 #define SYSTR_MSG_NOPROCESS(x) \
 	((x)->msg.msg_type == SYSTR_MSG_CHILD || \
@@ -84,6 +89,7 @@ struct str_message {
 		struct str_msg_ugid msg_ugid;
 		struct str_msg_ask msg_ask;
 		struct str_msg_child msg_child;
+		struct str_msg_execve msg_execve;
 	} msg_data;
 };
 
@@ -146,17 +152,17 @@ struct systrace_replace {
 	int32_t strr_flags[SYSTR_MAXARGS];
 };
 
-#define STRIOCCLONE	_IOR('s', 100, int)
-#define SYSTR_CLONE	STRIOCCLONE
-#define STRIOCATTACH	_IOW('s', 101, pid_t)
-#define STRIOCDETACH	_IOW('s', 102, pid_t)
-#define STRIOCANSWER	_IOW('s', 103, struct systrace_answer)
-#define STRIOCIO	_IOWR('s', 104, struct systrace_io)
-#define STRIOCPOLICY	_IOWR('s', 105, struct systrace_policy)
-#define STRIOCGETCWD	_IOW('s', 106, pid_t)
-#define STRIOCRESCWD	_IO('s', 107)
-#define STRIOCREPORT	_IOW('s', 108, pid_t)
-#define STRIOCREPLACE	_IOW('s', 109, struct systrace_replace)
+#define STRIOCCLONE		_IOR('s', 100, int)
+#define SYSTR_CLONE		STRIOCCLONE
+#define STRIOCATTACH		_IOW('s', 101, pid_t)
+#define STRIOCDETACH		_IOW('s', 102, pid_t)
+#define STRIOCANSWER		_IOW('s', 103, struct systrace_answer)
+#define STRIOCIO		_IOWR('s', 104, struct systrace_io)
+#define STRIOCPOLICY		_IOWR('s', 105, struct systrace_policy)
+#define STRIOCGETCWD		_IOW('s', 106, pid_t)
+#define STRIOCRESCWD		_IO('s', 107)
+#define STRIOCREPORT		_IOW('s', 108, pid_t)
+#define STRIOCREPLACE		_IOW('s', 109, struct systrace_replace)
 
 #define SYSTR_POLICY_ASK	0
 #define SYSTR_POLICY_PERMIT	1
@@ -197,9 +203,10 @@ struct fsystrace {
 /* Internal prototypes */
 
 void systrace_namei(struct nameidata *);
-int systrace_redirect(int, struct proc *, void *, register_t *);
+int  systrace_redirect(int, struct proc *, void *, register_t *);
 void systrace_exit(struct proc *);
 void systrace_fork(struct proc *, struct proc *);
+void systrace_execve(char *, struct proc *);
 
 #endif /* _KERNEL */
 #endif /* _SYSTRACE_H_ */
diff --git a/sys/kern/exec_script.c b/sys/kern/exec_script.c
index 4ad3847350a..e0af44c583d 100644
--- a/sys/kern/exec_script.c
+++ b/sys/kern/exec_script.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: exec_script.c,v 1.19 2004/05/14 04:00:33 tedu Exp $	*/
+/*	$OpenBSD: exec_script.c,v 1.20 2004/06/23 05:16:35 marius Exp $	*/
 /*	$NetBSD: exec_script.c,v 1.13 1996/02/04 02:15:06 christos Exp $	*/
 
 /*
@@ -46,6 +46,12 @@
 
 #include <sys/exec_script.h>
 
+#include "systrace.h"
+
+#if NSYSTRACE > 0
+#include <dev/systrace.h>
+#endif
+
 #if defined(SETUIDSCRIPTS) && !defined(FDSCRIPTS)
 #define FDSCRIPTS		/* Need this for safe set-id scripts. */
 #endif
@@ -214,8 +220,13 @@ check_shell:
 	if ((epp->ep_flags & EXEC_HASFD) == 0) {
 #endif
 		/* normally can't fail, but check for it if diagnostic */
-		error = copyinstr(epp->ep_name, *tmpsap++, MAXPATHLEN,
+#if NSYSTRACE > 0
+		error = copystr(epp->ep_name, *tmpsap++, MAXPATHLEN,
 		    (size_t *)0);
+#else
+		error = copyinstr(epp->ep_name, *tmpsap++, MAXPATHLEN,
+		    (size_t *)0);		
+#endif
 #ifdef DIAGNOSTIC
 		if (error != 0)
 			panic("exec_script: copyinstr couldn't fail");
diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
index 0075f15bf0f..c7fb1aad5d3 100644
--- a/sys/kern/kern_exec.c
+++ b/sys/kern/kern_exec.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_exec.c,v 1.86 2004/06/11 12:57:36 mickey Exp $	*/
+/*	$OpenBSD: kern_exec.c,v 1.87 2004/06/23 05:16:35 marius Exp $	*/
 /*	$NetBSD: kern_exec.c,v 1.75 1996/02/09 18:59:28 christos Exp $	*/
 
 /*-
@@ -66,6 +66,12 @@
 
 #include <dev/rndvar.h>
 
+#include "systrace.h"
+
+#if NSYSTRACE > 0
+#include <dev/systrace.h>
+#endif
+
 /*
  * Map the shared signal code.
  */
@@ -255,6 +261,12 @@ sys_execve(p, v, retval)
 	struct vmspace *vm = p->p_vmspace;
 	char **tmpfap;
 	extern struct emul emul_native;
+#if NSYSTRACE > 0
+	int wassugid =
+	    ISSET(p->p_flag, P_SUGID) || ISSET(p->p_flag, P_SUGIDEXEC);
+	char pathbuf[MAXPATHLEN];
+	size_t pathbuflen;
+#endif
 
 	/*
 	 * Cheap solution to complicated problems.
@@ -262,13 +274,25 @@ sys_execve(p, v, retval)
 	 */
 	p->p_flag |= P_INEXEC;
 
+#if NSYSTRACE > 0
+	error = copyinstr(SCARG(uap, path), pathbuf, MAXPATHLEN, &pathbuflen);
+	if (error != 0)
+		goto clrflag;
+
+	NDINIT(&nid, LOOKUP, NOFOLLOW, UIO_SYSSPACE, pathbuf, p);
+#else
 	/* init the namei data to point the file user's program name */
 	NDINIT(&nid, LOOKUP, NOFOLLOW, UIO_USERSPACE, SCARG(uap, path), p);
+#endif
 
 	/*
 	 * initialize the fields of the exec package.
 	 */
+#if NSYSTRACE > 0
+	pack.ep_name = pathbuf;
+#else
 	pack.ep_name = (char *)SCARG(uap, path);
+#endif
 	pack.ep_hdr = malloc(exec_maxhdrsz, M_EXEC, M_WAITOK);
 	pack.ep_hdrlen = exec_maxhdrsz;
 	pack.ep_hdrvalid = 0;
@@ -622,7 +646,16 @@ sys_execve(p, v, retval)
 	if (KTRPOINT(p, KTR_EMUL))
 		ktremul(p, p->p_emul->e_name);
 #endif
+
 	p->p_flag &= ~P_INEXEC;
+
+#if NSYSTRACE > 0
+	if (ISSET(p->p_flag, P_SYSTRACE) &&
+	    wassugid && !ISSET(p->p_flag, P_SUGID) &&
+	    !ISSET(p->p_flag, P_SUGIDEXEC))
+		systrace_execve(pathbuf, p);
+#endif
+
 	return (0);
 
 bad:
@@ -642,8 +675,11 @@ bad:
 	pool_put(&namei_pool, nid.ni_cnd.cn_pnbuf);
 	uvm_km_free_wakeup(exec_map, (vaddr_t) argp, NCARGS);
 
-freehdr:
+ freehdr:
 	free(pack.ep_hdr, M_EXEC);
+#if NSYSTRACE > 0
+ clrflag:
+#endif
 	p->p_flag &= ~P_INEXEC;
 	return (error);