
From tmoestl@gmx.net Thu Apr  5 15:01:40 2001
Date: Thu, 5 Apr 2001 20:06:59 +0200
From: Thomas Moestl <tmoestl@gmx.net>
To: Robert Watson <rwatson@FreeBSD.org>
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));
