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

hi,

attached diff does the following:

  - reorganize the code: first, switch statement to check if it's
    possible to do the request as far as kernel stability etc. goes.
    then a kauth(9) check to enforce security semantics (this replaces
    several kauth(9) calls in the second switch. last, the second
    switch statement to dispatch the request, after all aspects have
    been checked.

  - add kauth(9) KAUTH_PROCESS_CANSEE call right after the pfind()
    to enforce curtain policy. with XXX comment because these should
    not be sprinkled all around the code.

comments?

-e.


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

Index: secmodel/bsd44/secmodel_bsd44_suser.c
===================================================================
RCS file: /usr/cvs/src/sys/secmodel/bsd44/secmodel_bsd44_suser.c,v
retrieving revision 1.22
diff -u -p -r1.22 secmodel_bsd44_suser.c
--- secmodel/bsd44/secmodel_bsd44_suser.c	26 Dec 2006 10:43:44 -0000	1.22
+++ secmodel/bsd44/secmodel_bsd44_suser.c	24 Dec 2006 16:19:06 -0000
@@ -305,7 +305,42 @@ secmodel_bsd44_suser_process_cb(kauth_cr
 		break;
 		}
 
-	case KAUTH_PROCESS_CANPTRACE:
+	case KAUTH_PROCESS_CANPTRACE: {
+		switch ((u_long)arg1) {
+		case PT_ATTACH:
+		case PT_WRITE_I:
+		case PT_WRITE_D:
+		case PT_READ_I:
+		case PT_READ_D:
+		case PT_IO:
+		case PT_GETREGS:
+		case PT_SETREGS:
+		case PT_GETFPREGS:
+		case PT_SETFPREGS:
+		PTRACE_MACHDEP_REQUEST_CASES
+			if (isroot) {
+				result = KAUTH_RESULT_ALLOW;
+				break;
+			}
+
+			if (kauth_cred_getuid(cred) !=
+			    kauth_cred_getuid(p->p_cred) ||
+			    ISSET(p->p_flag, P_SUGID)) {
+				result = KAUTH_RESULT_DENY;
+				break;
+			}
+
+			result = KAUTH_RESULT_ALLOW;
+			break;
+
+		default:
+		        result = KAUTH_RESULT_ALLOW;
+		        break;
+		}
+
+		break;
+		}
+
 	case KAUTH_PROCESS_CANSYSTRACE:
 		if (isroot) {
 			result = KAUTH_RESULT_ALLOW;
Index: kern/sys_process.c
===================================================================
RCS file: /usr/cvs/src/sys/kern/sys_process.c,v
retrieving revision 1.118
diff -u -p -r1.118 sys_process.c
--- kern/sys_process.c	6 Dec 2006 18:54:02 -0000	1.118
+++ kern/sys_process.c	24 Dec 2006 16:38:46 -0000
@@ -148,6 +148,12 @@ sys_ptrace(struct lwp *l, void *v, regis
 		/* Find the process we're supposed to be operating on. */
 		if ((t = pfind(SCARG(uap, pid))) == NULL)
 			return (ESRCH);
+
+		/* XXX elad - this should be in pfind(). */
+		error = kauth_authorize_process(l->l_cred, KAUTH_PROCESS_CANSEE,
+		    t, NULL, NULL, NULL);
+		if (error)
+			return (ESRCH);
 	}
 
 	/* Can't trace a process that's currently exec'ing. */
@@ -181,16 +187,7 @@ sys_ptrace(struct lwp *l, void *v, regis
 			return (EBUSY);
 
 		/*
-		 *	(4) the security model prevents it, or
-		 */
-		error = kauth_authorize_process(l->l_cred,
-		    KAUTH_PROCESS_CANPTRACE, t, KAUTH_ARG(SCARG(uap, req)),
-		    NULL, NULL);
-		if (error)
-			return (error);
-
-		/*
-		 *	(5) the tracer is chrooted, and its root directory is
+		 *	(4) the tracer is chrooted, and its root directory is
 		 *	    not at or above the root directory of the tracee
 		 */
 		if (!proc_isunder(t, l))
@@ -201,18 +198,7 @@ sys_ptrace(struct lwp *l, void *v, regis
 	case  PT_READ_D:
 	case  PT_WRITE_I:
 	case  PT_WRITE_D:
-	case  PT_CONTINUE:
 	case  PT_IO:
-	case  PT_KILL:
-	case  PT_DETACH:
-	case  PT_LWPINFO:
-	case  PT_SYSCALL:
-#ifdef COREDUMP
-	case  PT_DUMPCORE:
-#endif
-#ifdef PT_STEP
-	case  PT_STEP:
-#endif
 #ifdef PT_GETREGS
 	case  PT_GETREGS:
 #endif
@@ -225,11 +211,29 @@ sys_ptrace(struct lwp *l, void *v, regis
 #ifdef PT_SETFPREGS
 	case  PT_SETFPREGS:
 #endif
-
 #ifdef __HAVE_PTRACE_MACHDEP
 	PTRACE_MACHDEP_REQUEST_CASES
 #endif
+		/*
+		 * You can't read/write the memory or registers of a process
+		 * if the tracer is chrooted, and its root directory is not at
+		 * or above the root directory of the tracee.
+		 */
+		if (!proc_isunder(t, l))
+			return (EPERM);
+		/*FALLTHROUGH*/
 
+	case  PT_CONTINUE:
+	case  PT_KILL:
+	case  PT_DETACH:
+	case  PT_LWPINFO:
+	case  PT_SYSCALL:
+#ifdef COREDUMP
+	case  PT_DUMPCORE:
+#endif
+#ifdef PT_STEP
+	case  PT_STEP:
+#endif
 		/*
 		 * You can't do what you want to the process if:
 		 *	(1) It's not being traced at all,
@@ -268,6 +272,11 @@ sys_ptrace(struct lwp *l, void *v, regis
 		return (EINVAL);
 	}
 
+	error = kauth_authorize_process(l->l_cred, KAUTH_PROCESS_CANPTRACE,
+	    t, KAUTH_ARG(SCARG(uap, req)), NULL, NULL);
+	if (error)
+		return (error);
+
 	/* Do single-step fixup if needed. */
 	FIX_SSTEP(t);
 
@@ -319,15 +328,6 @@ sys_ptrace(struct lwp *l, void *v, regis
 		uio.uio_rw = write ? UIO_WRITE : UIO_READ;
 		UIO_SETUP_SYSSPACE(&uio);
 
-		error = kauth_authorize_process(l->l_cred,
-		    KAUTH_PROCESS_CANPTRACE, t, KAUTH_ARG(SCARG(uap, req)),
-		    NULL, NULL);
-		if (error)
-			return (error);
-
-		if (!proc_isunder(t, l))
-			return (EPERM);
-
 		error = process_domem(l, lt, &uio);
 		if (!write)
 			*retval = tmp;
@@ -370,15 +370,6 @@ sys_ptrace(struct lwp *l, void *v, regis
 		uio.uio_resid = piod.piod_len;
 		uio.uio_vmspace = vm;
 
-		error = kauth_authorize_process(l->l_cred,
-		    KAUTH_PROCESS_CANPTRACE, t, KAUTH_ARG(SCARG(uap, req)),
-		    NULL, NULL);
-		if (error)
-			return (error);
-
-		if (!proc_isunder(t, l))
-			return (EPERM);
-
 		error = process_domem(l, lt, &uio);
 		piod.piod_len -= uio.uio_resid;
 		(void) copyout(&piod, SCARG(uap, addr), sizeof(piod));
@@ -592,15 +583,6 @@ sys_ptrace(struct lwp *l, void *v, regis
 			uio.uio_rw = write ? UIO_WRITE : UIO_READ;
 			uio.uio_vmspace = vm;
 
-			error = kauth_authorize_process(l->l_cred,
-			    KAUTH_PROCESS_CANPTRACE, t, 
-			    KAUTH_ARG(SCARG(uap, req)), NULL, NULL);
-			if (error)
-				return (error);
-
-			if (!proc_isunder(t, l))
-				return (EPERM);
-
 			error = process_doregs(l, lt, &uio);
 			uvmspace_free(vm);
 			return error;
@@ -640,15 +622,6 @@ sys_ptrace(struct lwp *l, void *v, regis
 			uio.uio_rw = write ? UIO_WRITE : UIO_READ;
 			uio.uio_vmspace = vm;
 
-			error = kauth_authorize_process(l->l_cred,
-			    KAUTH_PROCESS_CANPTRACE, t,
-			    KAUTH_ARG(SCARG(uap, req)), NULL, NULL);
-			if (error)
-				return (error);
-
-			if (!proc_isunder(t, l))
-				return (EPERM);
-
 			error = process_dofpregs(l, lt, &uio);
 			uvmspace_free(vm);
 			return error;
@@ -657,15 +630,6 @@ sys_ptrace(struct lwp *l, void *v, regis
 
 #ifdef __HAVE_PTRACE_MACHDEP
 	PTRACE_MACHDEP_REQUEST_CASES
-		error = kauth_authorize_process(l->l_cred,
-		    KAUTH_PROCESS_CANPTRACE, t, KAUTH_ARG(SCARG(uap, req)),
-		    NULL, NULL);
-		if (error)
-			return (error);
-
-		if (!proc_isunder(t, l))
-			return (EPERM);
-
 		return (ptrace_machdep_dorequest(l, lt,
 		    SCARG(uap, req), SCARG(uap, addr),
 		    SCARG(uap, data)));

--------------080504000609000203050305--