From 528bd53c0cc743655a6747db3a4d672c6098ca67 Mon Sep 17 00:00:00 2001 From: marius eriksen Date: Wed, 23 Jun 2004 05:16:36 +0000 Subject: 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 --- sys/dev/systrace.c | 79 ++++++++++++++++++++++++++++++++++++++------------ sys/dev/systrace.h | 33 ++++++++++++--------- sys/kern/exec_script.c | 15 ++++++++-- sys/kern/kern_exec.c | 40 +++++++++++++++++++++++-- 4 files changed, 132 insertions(+), 35 deletions(-) (limited to 'sys') 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 * 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 * 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 +#include "systrace.h" + +#if NSYSTRACE > 0 +#include +#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 +#include "systrace.h" + +#if NSYSTRACE > 0 +#include +#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); -- cgit v1.2.3