This is a multi-part message in MIME format.
--------------040303080105050106020809
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

hi,

sysctl_proc_find() is used for both looking up a process by its pid
and performing "access" checks on it for three different callers.

attached diff:

  - moves 'nice' access semantics to secmodel code,
  - makes sysctl_proc_find() just lookup the process,
  - use KAUTH_PROCESS_CANSEE requests to determine if the caller is
    allowed to view the target process' corename, stop flags, and
    rlimits.
  - use explicit kauth(9) calls with KAUTH_PROCESS_CORENAME,
    KAUTH_REQ_PROCESS_RESOURCE_NICE, KAUTH_REQ_PROCESS_RESOURCE_RLIMIT,
    and KAUTH_PROCESS_STOPFLAG when modifying the aforementioned.

notes:

  - the semantics were copied verbatim from the sysctl code. I don't
    know if they are correct. for example, sys_setrlimit() allows
    modifying only the current process; no idea what the semantics
    should be for non-root user changing rlimits on not-current process.
    same goes for corename.

  - sysctl_proc_find()'s existence isn't justified. ideally, we can just
    use pfind(). however, pfind() should also use KAUTH_PROCESS_CANSEE,
    so I'll do that some other time.

-e.

--------------040303080105050106020809
Content-Type: text/plain;
 name="res.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="res.diff"

Index: secmodel/bsd44/secmodel_bsd44_suser.c
===================================================================
RCS file: /usr/cvs/src/sys/secmodel/bsd44/secmodel_bsd44_suser.c,v
retrieving revision 1.17
diff -u -p -r1.17 secmodel_bsd44_suser.c
--- secmodel/bsd44/secmodel_bsd44_suser.c	28 Nov 2006 17:27:10 -0000	1.17
+++ secmodel/bsd44/secmodel_bsd44_suser.c	13 Dec 2006 02:34:59 -0000
@@ -250,30 +250,112 @@ secmodel_bsd44_suser_process_cb(kauth_cr
 		result = KAUTH_RESULT_ALLOW;
 		break;
 
+	case KAUTH_PROCESS_CORENAME:
+		result = KAUTH_RESULT_ALLOW;
+
+		if (isroot)
+			break;
+
+		/*
+		 * suid proc of ours or proc not ours
+		 */
+		if (kauth_cred_getuid(cred) != kauth_cred_getuid(p->p_cred) ||
+		    kauth_cred_getuid(cred) != kauth_cred_getsvuid(p->p_cred))
+			result = KAUTH_RESULT_DENY;
+
+		/*
+		 * sgid proc has sgid back to us temporarily
+		 */
+		else if (kauth_cred_getgid(p->p_cred) != kauth_cred_getsvgid(p->p_cred))
+			result = KAUTH_RESULT_DENY;
+
+		/*
+		 * our rgid must be in target's group list (ie,
+		 * sub-processes started by a sgid process)
+		 */
+		else {
+			int ismember = 0;
+
+			if (kauth_cred_ismember_gid(cred,
+			    kauth_cred_getgid(p->p_cred), &ismember) != 0 ||
+			    !ismember)
+				result = KAUTH_RESULT_DENY;
+		}
+		break;
+
 	case KAUTH_PROCESS_RESOURCE:
 		switch ((u_long)arg1) {
 		case KAUTH_REQ_PROCESS_RESOURCE_NICE:
-			if (isroot)
+			if (isroot) {
+				result = KAUTH_RESULT_ALLOW;
+				break;
+			}
+
+			if (kauth_cred_geteuid(cred) !=
+			    kauth_cred_geteuid(p->p_cred) &&
+			    kauth_cred_getuid(cred) !=
+			    kauth_cred_geteuid(p->p_cred)) {
+				result = KAUTH_RESULT_DENY;
+				break;
+			}
+
+			if ((u_long)arg2 >= p->p_nice)
 				result = KAUTH_RESULT_ALLOW;
-			else if ((u_long)arg2 >= p->p_nice)
-				result = KAUTH_RESULT_ALLOW; 
+
 			break;
 
-		case KAUTH_REQ_PROCESS_RESOURCE_RLIMIT:
-			if (isroot)
+		case KAUTH_REQ_PROCESS_RESOURCE_RLIMIT: {
+			struct rlimit *new_rlimit;
+			u_long which;
+
+			if (isroot) {
 				result = KAUTH_RESULT_ALLOW;
-			else {
-				struct rlimit *new_rlimit;
-				u_long which;
+				break;
+			}
 
-				new_rlimit = arg2;
-				which = (u_long)arg3;
+			/*
+			 * suid proc of ours or proc not ours
+			 */
+			if (kauth_cred_getuid(cred) !=
+			    kauth_cred_getuid(p->p_cred) ||
+			    kauth_cred_getuid(cred) !=
+			    kauth_cred_getsvuid(p->p_cred)) {
+				result = KAUTH_RESULT_DENY;
+				break;
+			}
 
-				if (new_rlimit->rlim_max <=
-				    p->p_rlimit[which].rlim_max)
-					result = KAUTH_RESULT_ALLOW;
+			/*
+			 * sgid proc has sgid back to us temporarily
+			 */
+			else if (kauth_cred_getgid(p->p_cred) !=
+			    kauth_cred_getsvgid(p->p_cred)) {
+				result = KAUTH_RESULT_DENY;
+				break;
 			}
+
+			/*
+			 * our rgid must be in target's group list (ie,
+			 * sub-processes started by a sgid process)
+			 */
+			else {
+				int ismember = 0;
+
+				if (kauth_cred_ismember_gid(cred,
+				    kauth_cred_getgid(p->p_cred),
+				    &ismember) != 0 || !ismember) {
+					result = KAUTH_RESULT_DENY;
+					break;
+				}
+			}
+
+			new_rlimit = arg2;
+			which = (u_long)arg3;
+
+			if (new_rlimit->rlim_max <=
+			    p->p_rlimit[which].rlim_max)
+				result = KAUTH_RESULT_ALLOW;
 			break;
+			}
 
 		default:
 			result = KAUTH_RESULT_DEFER;
@@ -286,6 +368,39 @@ secmodel_bsd44_suser_process_cb(kauth_cr
 			result = KAUTH_RESULT_ALLOW;
 		break;
 
+	case KAUTH_PROCESS_STOPFLAG:
+		result = KAUTH_RESULT_ALLOW;
+
+		if (isroot)
+			break;
+
+		/*
+		 * suid proc of ours or proc not ours
+		 */
+		if (kauth_cred_getuid(cred) != kauth_cred_getuid(p->p_cred) ||
+		    kauth_cred_getuid(cred) != kauth_cred_getsvuid(p->p_cred))
+			result = KAUTH_RESULT_DENY;
+
+		/*
+		 * sgid proc has sgid back to us temporarily
+		 */
+		else if (kauth_cred_getgid(p->p_cred) != kauth_cred_getsvgid(p->p_cred))
+			result = KAUTH_RESULT_DENY;
+
+		/*
+		 * our rgid must be in target's group list (ie,
+		 * sub-processes started by a sgid process)
+		 */
+		else {
+			int ismember = 0;
+
+			if (kauth_cred_ismember_gid(cred,
+			    kauth_cred_getgid(p->p_cred), &ismember) != 0 ||
+			    !ismember)
+				result = KAUTH_RESULT_DENY;
+		}
+		break;
+
 	default:
 		result = KAUTH_RESULT_DEFER;
 		break;
Index: kern/kern_resource.c
===================================================================
RCS file: /usr/cvs/src/sys/kern/kern_resource.c,v
retrieving revision 1.110
diff -u -p -r1.110 kern_resource.c
--- kern/kern_resource.c	7 Dec 2006 20:04:31 -0000	1.110
+++ kern/kern_resource.c	13 Dec 2006 04:04:49 -0000
@@ -197,18 +197,13 @@ donice(struct lwp *l, struct proc *chgp,
 	kauth_cred_t cred = l->l_cred;
 	int s;
 
-	if (kauth_cred_geteuid(cred) && kauth_cred_getuid(cred) &&
-	    kauth_cred_geteuid(cred) != kauth_cred_geteuid(chgp->p_cred) &&
-	    kauth_cred_getuid(cred) != kauth_cred_geteuid(chgp->p_cred))
-		return (EPERM);
 	if (n > PRIO_MAX)
 		n = PRIO_MAX;
 	if (n < PRIO_MIN)
 		n = PRIO_MIN;
 	n += NZERO;
-	if (n < chgp->p_nice && kauth_authorize_process(cred,
-	    KAUTH_PROCESS_RESOURCE, chgp, (void *)KAUTH_REQ_PROCESS_RESOURCE_NICE,
-	    (void *)(u_long)n, NULL))
+	if (kauth_authorize_process(cred, KAUTH_PROCESS_RESOURCE, chgp,
+	    (void *)KAUTH_REQ_PROCESS_RESOURCE_NICE, KAUTH_ARG(n), NULL))
 		return (EACCES);
 	chgp->p_nice = n;
 	SCHED_LOCK(s);
@@ -261,10 +256,10 @@ dosetrlimit(struct lwp *l, struct proc *
 		 */
 		return (EINVAL);
 	}
-	if (limp->rlim_max > alimp->rlim_max && (error =
-	    kauth_authorize_process(l->l_cred, KAUTH_PROCESS_RESOURCE,
-	    p, (void *)KAUTH_REQ_PROCESS_RESOURCE_RLIMIT, limp,
-	    (void *)(u_long)which)))
+	error = kauth_authorize_process(l->l_cred, KAUTH_PROCESS_RESOURCE,
+	    p, KAUTH_ARG(KAUTH_REQ_PROCESS_RESOURCE_RLIMIT), limp,
+	    KAUTH_ARG(which));
+	if (error)
 			return (error);
 
 	if (p->p_limit->p_refcnt > 1 &&
@@ -573,39 +568,6 @@ sysctl_proc_findproc(struct lwp *l, stru
 		ptmp = l->l_proc;
 	else if ((ptmp = pfind(pid)) == NULL)
 		error = ESRCH;
-	else {
-		boolean_t isroot = kauth_authorize_generic(l->l_cred,
-		    KAUTH_GENERIC_ISSUSER, NULL) == 0;
-		/*
-		 * suid proc of ours or proc not ours
-		 */
-		if (kauth_cred_getuid(l->l_cred) !=
-		    kauth_cred_getuid(ptmp->p_cred) ||
-		    kauth_cred_getuid(l->l_cred) !=
-		    kauth_cred_getsvuid(ptmp->p_cred))
-			error = isroot ? 0 : EPERM;
-
-		/*
-		 * sgid proc has sgid back to us temporarily
-		 */
-		else if (kauth_cred_getgid(ptmp->p_cred) !=
-		    kauth_cred_getsvgid(ptmp->p_cred))
-			error = isroot ? 0 : EPERM;
-
-		/*
-		 * our rgid must be in target's group list (ie,
-		 * sub-processes started by a sgid process)
-		 */
-		else {
-			int ismember = 0;
-
-			if (kauth_cred_ismember_gid(l->l_cred,
-			    kauth_cred_getgid(ptmp->p_cred), &ismember) != 0 ||
-			    !ismember) {
-				error = isroot ? 0 : EPERM;
-			}
-		}
-	}
 
 	*p2 = ptmp;
 	return (error);
@@ -641,6 +603,12 @@ sysctl_proc_corename(SYSCTLFN_ARGS)
 	if (error)
 		return (error);
 
+	/* XXX this should be in p_find() */
+	error = kauth_authorize_process(l->l_cred, KAUTH_PROCESS_CANSEE,
+	    ptmp, NULL, NULL, NULL);
+	if (error)
+		return (error);
+
 	cname = PNBUF_GET();
 	/*
 	 * let them modify a temporary copy of the core name
@@ -659,9 +627,10 @@ sysctl_proc_corename(SYSCTLFN_ARGS)
 		goto done;
 	}
 
-	if (kauth_authorize_process(l->l_cred, KAUTH_PROCESS_CORENAME,
-	    ptmp, NULL, NULL, NULL) != 0)
-		return (EPERM);
+	error = kauth_authorize_process(l->l_cred, KAUTH_PROCESS_CORENAME,
+	    ptmp, cname, NULL, NULL);
+	if (error)
+		return (error);
 
 	/*
 	 * no error yet and cname now has the new core name in it.
@@ -722,6 +691,12 @@ sysctl_proc_stop(SYSCTLFN_ARGS)
 	if (error)
 		return (error);
 
+	/* XXX this should be in p_find() */
+	error = kauth_authorize_process(l->l_cred, KAUTH_PROCESS_CANSEE,
+	    ptmp, NULL, NULL, NULL);
+	if (error)
+		return (error);
+
 	switch (rnode->sysctl_num) {
 	case PROC_PID_STOPFORK:
 		f = P_STOPFORK;
@@ -743,6 +718,11 @@ sysctl_proc_stop(SYSCTLFN_ARGS)
 	if (error || newp == NULL)
 		return (error);
 
+	error = kauth_authorize_process(l->l_cred, KAUTH_PROCESS_STOPFLAG,
+	    ptmp, KAUTH_ARG(f), NULL, NULL);
+	if (error)
+		return (error);
+
 	if (i)
 		ptmp->p_flag |= f;
 	else
@@ -782,6 +762,12 @@ sysctl_proc_plimit(SYSCTLFN_ARGS)
 	if (error)
 		return (error);
 
+	/* XXX this should be in p_find() */
+	error = kauth_authorize_process(l->l_cred, KAUTH_PROCESS_CANSEE,
+	    ptmp, NULL, NULL, NULL);
+	if (error)
+		return (error);
+
 	node = *rnode;
 	memcpy(&alim, &ptmp->p_rlimit[limitno], sizeof(alim));
 	if (which == PROC_PID_LIMIT_TYPE_HARD)

--------------040303080105050106020809--