From tmoestl@gmx.net Thu Apr 5 15:01:40 2001 Date: Thu, 5 Apr 2001 20:06:59 +0200 From: Thomas Moestl To: Robert Watson Subject: p_cansignal Hello, here's my version of p_cansingal. It is a pretty 1-to-1 port of the OpenBSD one, with the exception of different return values, jail check and equivalence check. This is more permissive in the respect that it allows SIGCONT to be delivered in a session, which was previously checked in the CANSIGNAL macro in kern_sig.c. I don't think there is a problem with that. CANSIGNAL can go away now because of that. My CANSIGIO version (in kern/sig.c) also has a prison check. CANSIGIO is currently using separate checks because we only have a ucred here (we need a session, and thus the proc pointer, for p_cansignal). Additionally, I have taken the last check from OpenBSD, which seems to be a little more permissive in that it also checks the saved uids, but as the XXX comment (which is also taken from OpenBSD) says, this should not make a difference, since we have checked for P_SUGID before (and there should be no way to alter the saved uid that does not set P_SUIGID). Also, p_cansingal might better be placed in another header. For convenience, a link to the OpenBSD version: http://www.openbsd.org/cgi-bin/cvsweb/src/sys/kern/kern_sig.c?rev=1.42&content-type=text/x-cvsweb-markup Please give this one a bit of testing, as I use slightly different versions of this (cap_check instead of suser_xxx) and have other changes that may mask problems (although I don't expect that). Nothing should change with respect to locking, because there was no place where this was used without a session check (i.e. only CANSIGNAL was used). - thomas [ Part 2: "Attached Text" ] Index: sys/kern/kern_prot.c =================================================================== RCS file: /home/ncvs/src/sys/kern/kern_prot.c,v retrieving revision 1.78 diff -u -r1.78 kern_prot.c --- sys/kern/kern_prot.c 2001/03/29 22:59:44 1.78 +++ sys/kern/kern_prot.c 2001/04/05 18:00:13 @@ -1001,48 +1001,67 @@ return (u_cansee(p1->p_ucred, p2->p_ucred)); } -static int -p_cankill(struct proc *p1, struct proc *p2, int *privused) +/* + * Can process p1 send the signal signum to process p2? + */ +int +p_cansignal(struct proc *p1, struct proc *p2, int signum) { - int error; - - if (privused != NULL) - *privused = 0; - + int error; + if (p1 == p2) return (0); if ((error = prison_check(p1->p_ucred, p2->p_ucred))) return (error); - if (p1->p_cred->p_ruid == p2->p_cred->p_ruid) + if (!suser_xxx(NULL, p1, PRISON_ROOT)) return (0); - if (p1->p_ucred->cr_uid == p2->p_cred->p_ruid) - return (0); + + if (signum == SIGCONT && p1->p_session == p2->p_session) + return (0); /* SIGCONT in session */ + /* + * Using kill(), only certain signals can be sent to setugid + * child processes + * * XXX should a process be able to affect another process * acting as the same uid (i.e., a userland nfsd or the like?) - */ - if (p1->p_cred->p_ruid == p2->p_ucred->cr_uid) - return (0); - if (p1->p_ucred->cr_uid == p2->p_ucred->cr_uid) - return (0); - - if (!suser_xxx(0, p1, PRISON_ROOT)) { - if (privused != NULL) - *privused = 1; - return (0); - } - -#ifdef CAPABILITIES - if (!cap_check_xxx(0, p1, CAP_KILL, PRISON_ROOT)) { - if (privused != NULL) - *privused = 1; - return (0); - } -#endif - - return (EPERM); + */ + if (p2->p_flag & P_SUGID) { + switch (signum) { + case 0: + case SIGKILL: + case SIGINT: + case SIGTERM: + case SIGSTOP: + case SIGTTIN: + case SIGTTOU: + case SIGTSTP: + case SIGHUP: + case SIGUSR1: + case SIGUSR2: + if (p1->p_cred->p_ruid == p2->p_cred->p_ruid || + p1->p_ucred->cr_uid == p2->p_cred->p_ruid || + p1->p_cred->p_ruid == p2->p_ucred->cr_uid || + p1->p_ucred->cr_uid == p2->p_ucred->cr_uid) + return (0); + } + return (EPERM); + } + + /* + * XXX: because the P_SUGID test exists, this has extra tests which + * could be removed. + */ + if (p1->p_cred->p_ruid == p2->p_cred->p_ruid || + p1->p_cred->p_ruid == p2->p_cred->p_svuid || + p1->p_ucred->cr_uid == p2->p_cred->p_ruid || + p1->p_ucred->cr_uid == p2->p_cred->p_svuid || + p1->p_cred->p_ruid == p2->p_ucred->cr_uid || + p1->p_ucred->cr_uid == p2->p_ucred->cr_uid) + return (0); + return (EPERM); } static int @@ -1132,9 +1151,6 @@ case P_CAN_SEE: return (p_cansee(p1, p2, privused)); - case P_CAN_KILL: - return (p_cankill(p1, p2, privused)); - case P_CAN_SCHED: return (p_cansched(p1, p2, privused)); Index: sys/kern/kern_sig.c =================================================================== RCS file: /home/ncvs/src/sys/kern/kern_sig.c,v retrieving revision 1.114 diff -u -r1.114 kern_sig.c --- sys/kern/kern_sig.c 2001/04/03 01:39:23 1.114 +++ sys/kern/kern_sig.c 2001/04/05 17:43:12 @@ -98,13 +98,6 @@ "Log processes quitting on abnormal signals to syslog(3)"); /* - * Can process p, with pcred pc, send the signal sig to process q? - */ -#define CANSIGNAL(p, q, sig) \ - (!p_can(p, q, P_CAN_KILL, NULL) || \ - ((sig) == SIGCONT && (q)->p_session == (p)->p_session)) - -/* * Policy -- Can real uid ruid with ucred uc send a signal to process q? */ #define CANSIGIO(ruid, uc, q) \ @@ -910,7 +903,7 @@ * XXX: this locking needs work.. specifically the * session checks.. */ - if (!CANSIGNAL(cp, p, sig)) + if (p_cansignal(cp, p, sig)) continue; nfound++; if (sig) { @@ -945,7 +938,7 @@ } mtx_unlock_spin(&sched_lock); /* XXX: locking b0rked */ - if (!CANSIGNAL(cp, p, sig)) + if (p_cansignal(cp, p, sig)) continue; nfound++; if (sig) { @@ -979,7 +972,7 @@ if ((p = pfind(uap->pid)) == NULL) return (ESRCH); /* XXX: locking b0rked */ - if (!CANSIGNAL(cp, p, uap->signum)) + if (p_cansignal(cp, p, uap->signum)) return (EPERM); if (uap->signum) { PROC_LOCK(p); Index: sys/sys/proc.h =================================================================== RCS file: /home/ncvs/src/sys/sys/proc.h,v retrieving revision 1.157 diff -u -r1.157 proc.h --- sys/sys/proc.h 2001/03/28 11:52:55 1.157 +++ sys/sys/proc.h 2001/04/05 17:45:56 @@ -329,7 +329,6 @@ #define P_MAGIC 0xbeefface #define P_CAN_SEE 1 -#define P_CAN_KILL 2 #define P_CAN_SCHED 3 #define P_CAN_DEBUG 4 @@ -530,6 +529,7 @@ void mi_switch __P((void)); int p_can __P((struct proc *p1, struct proc *p2, int operation, int *privused)); +int p_cansignal __P((struct proc *p1, struct proc *p2, int signum)); int p_trespass __P((struct proc *p1, struct proc *p2)); void procinit __P((void)); void proc_reparent __P((struct proc *child, struct proc *newparent));