Change to secmodel_bsd44_suser.c

[Elad Efrat <elad%NetBSD.org@localhost>]

Hi,

Please review the attached diff. I removed explicit KAUTH_RESULT_DENY
assignments in secmodel_bsd44_suser.c, so there's a single, default
deny in each listener, and "allow" is done explicitly.

Given this is a change in critical code, I'd appreciate careful review.
Of course, any other input is welcome. :)

Thanks,

-e.

Index: secmodel_bsd44_suser.c
===================================================================
RCS file: /usr/cvs/src/sys/secmodel/bsd44/secmodel_bsd44_suser.c,v
retrieving revision 1.50
diff -u -p -r1.50 secmodel_bsd44_suser.c
--- secmodel_bsd44_suser.c      2 Feb 2008 21:04:41 -0000       1.50
+++ secmodel_bsd44_suser.c      5 Feb 2008 20:02:38 -0000
@@ -467,7 +467,7 @@ secmodel_bsd44_suser_process_cb(kauth_cr
                             kauth_cred_getuid(p->p_cred) ||
                            kauth_cred_getuid(cred) !=
                             kauth_cred_getsvuid(p->p_cred)))
-                               result = KAUTH_RESULT_DENY;
+                               break;
                        else
                                result = KAUTH_RESULT_ALLOW;
 
@@ -494,7 +494,6 @@ secmodel_bsd44_suser_process_cb(kauth_cr
 
                if ((p->p_traceflag & KTRFAC_PERSISTENT) ||
                    (p->p_flag & PK_SUGID)) {
-                       result = KAUTH_RESULT_DENY;
                        break;
                }
 
@@ -506,7 +505,6 @@ secmodel_bsd44_suser_process_cb(kauth_cr
                        break;
                }
 
-               result = KAUTH_RESULT_DENY;
                break;
                }
 
@@ -520,7 +518,6 @@ secmodel_bsd44_suser_process_cb(kauth_cr
                }
 
                if (req == KAUTH_REQ_PROCESS_PROCFS_CTL) {
-                       result = KAUTH_RESULT_DENY;
                        break;
                }
 
@@ -531,7 +528,6 @@ secmodel_bsd44_suser_process_cb(kauth_cr
                        if (kauth_cred_getuid(cred) !=
                            kauth_cred_getuid(p->p_cred) ||
                            ISSET(p->p_flag, PK_SUGID)) {
-                               result = KAUTH_RESULT_DENY;
                                break;
                        }
                        /*FALLTHROUGH*/
@@ -575,7 +571,6 @@ secmodel_bsd44_suser_process_cb(kauth_cr
                        if (kauth_cred_getuid(cred) !=
                            kauth_cred_getuid(p->p_cred) ||
                            ISSET(p->p_flag, PK_SUGID)) {
-                               result = KAUTH_RESULT_DENY;
                                break;
                        }
 
@@ -603,15 +598,8 @@ secmodel_bsd44_suser_process_cb(kauth_cr
                }
 
        case KAUTH_PROCESS_CORENAME:
-               result = KAUTH_RESULT_ALLOW;
-
-               if (isroot)
-                       break;
-
-               if (proc_uidmatch(cred, p->p_cred) != 0) {
-                       result = KAUTH_RESULT_DENY;
-                       break;
-               }
+               if (isroot || proc_uidmatch(cred, p->p_cred) == 0)
+                       result = KAUTH_RESULT_ALLOW;
 
                break;
 
@@ -624,7 +612,7 @@ secmodel_bsd44_suser_process_cb(kauth_cr
                 * processes, maxproc is the limit.
                 */
                if (__predict_false((lnprocs >= maxproc - 5) && !isroot))
-                       result = KAUTH_RESULT_DENY;
+                       break;
                else
                        result = KAUTH_RESULT_ALLOW;
 
@@ -636,7 +624,7 @@ secmodel_bsd44_suser_process_cb(kauth_cr
                     kauth_cred_getuid(cred) ||
                     ISSET(p->p_flag, PK_SUGID)) &&
                    !isroot)
-                       result = KAUTH_RESULT_DENY;
+                       break;
                else
                        result = KAUTH_RESULT_ALLOW;
 
@@ -652,7 +640,6 @@ secmodel_bsd44_suser_process_cb(kauth_cr
                    kauth_cred_geteuid(p->p_cred) &&
                    kauth_cred_getuid(cred) !=
                    kauth_cred_geteuid(p->p_cred)) {
-                       result = KAUTH_RESULT_DENY;
                        break;
                }
 
@@ -678,7 +665,6 @@ secmodel_bsd44_suser_process_cb(kauth_cr
 
                        if ((p != curlwp->l_proc) &&
                            (proc_uidmatch(cred, p->p_cred) != 0)) {
-                               result = KAUTH_RESULT_DENY;
                                break;
                        }
 
@@ -750,13 +736,8 @@ secmodel_bsd44_suser_process_cb(kauth_cr
                break;
 
        case KAUTH_PROCESS_STOPFLAG:
-               result = KAUTH_RESULT_ALLOW;
-
-               if (isroot)
-                       break;
-
-               if (proc_uidmatch(cred, p->p_cred) != 0) {
-                       result = KAUTH_RESULT_DENY;
+               if (isroot || proc_uidmatch(cred, p->p_cred) == 0) {
+                       result = KAUTH_RESULT_ALLOW;
                        break;
                }
                break;