Use LH_FOREACH_REV when checking permission and short-circuit on

the first non-UNSPEC hit we get for the command.  This means that
instead of cycling through the all the parsed sudoers entries we
start at the end and work backwards and quit after the first positive
or negative match.

diff --git a/match.c b/match.c
index 7dd8a6552cd43a99797ee82b79104243efc01bf5..324e527eaf2db67d53e8a144e5a5fa14dd771ffe 100644 (file)
--- a/match.c
+++ b/match.c
@@ -111,7 +111,7 @@ userlist_matches(pw, list)
     struct alias *a;
     int rval, matched = UNSPEC;
 
-    for (m = list->first; m != NULL; m = m->next) {
+    LH_FOREACH_REV(list, m) {
        switch (m->type) {
            case ALL:
                matched = !m->negated;
@@ -137,6 +137,8 @@ userlist_matches(pw, list)
                    matched = !m->negated;
                break;
        }
+       if (matched != UNSPEC)
+           break;
     }
     return(matched);
 }
@@ -157,7 +159,7 @@ runaslist_matches(list)
     if (list == NULL)
        return(userpw_matches(def_runas_default, runas_pw->pw_name, runas_pw));
 
-    for (m = list->first; m != NULL; m = m->next) {
+    LH_FOREACH_REV(list, m) {
        switch (m->type) {
            case ALL:
                matched = !m->negated;
@@ -183,6 +185,8 @@ runaslist_matches(list)
                    matched = !m->negated;
                break;
        }
+       if (matched != UNSPEC)
+           break;
     }
     return(matched);
 }
@@ -199,7 +203,7 @@ hostlist_matches(list)
     struct alias *a;
     int rval, matched = UNSPEC;
 
-    for (m = list->first; m != NULL; m = m->next) {
+    LH_FOREACH_REV(list, m) {
        switch (m->type) {
            case ALL:
                matched = !m->negated;
@@ -225,6 +229,8 @@ hostlist_matches(list)
                    matched = !m->negated;
                break;
        }
+       if (matched != UNSPEC)
+           break;
     }
     return(matched);
 }
@@ -272,10 +278,12 @@ cmndlist_matches(list)
     struct member *m;
     int rval, matched = UNSPEC;
 
-    for (m = list->first; m != NULL; m = m->next) {
+    LH_FOREACH_REV(list, m) {
        rval = cmnd_matches(m);
-       if (rval != UNSPEC)
+       if (rval != UNSPEC) {
            matched = m->negated ? !rval : rval;
+           break;
+       }
     }
     return(matched);
 }

diff --git a/parse.c b/parse.c
index 2433005ebf08345923fcf1695d5e1069421ecd27..36c14a696078f7c7f8d9ba56a23ad6648aed0d95 100644 (file)
--- a/parse.c
+++ b/parse.c
@@ -110,15 +110,13 @@ sudoers_lookup(pwflag)
        CLR(validated, FLAG_NO_USER);
        CLR(validated, FLAG_NO_HOST);
        match = DENY;
-       /* XXX - it should be faster to start from the bottom and
-                work our way up and then just stop at the first match. */
-       LH_FOREACH_FWD(&userspecs, us) {
+       LH_FOREACH_REV(&userspecs, us) {
            if (userlist_matches(sudo_user.pw, &us->users) != ALLOW)
                continue;
-           LH_FOREACH_FWD(&us->privileges, priv) {
+           LH_FOREACH_REV(&us->privileges, priv) {
                if (hostlist_matches(&priv->hostlist) != ALLOW)
                    continue;
-               LH_FOREACH_FWD(&priv->cmndlist, cs) {
+               LH_FOREACH_REV(&priv->cmndlist, cs) {
                    /* Only check the command when listing another user. */
                    if (user_uid == 0 || list_pw == NULL ||
                        user_uid == list_pw->pw_uid ||
@@ -127,9 +125,12 @@ sudoers_lookup(pwflag)
                    if ((pwcheck == any && nopass != TRUE) ||
                        (pwcheck == all && nopass == TRUE))
                        nopass = cs->tags.nopasswd;
+                   if (match == ALLOW)
+                       goto matched_pseudo;
                }
            }
        }
+       matched_pseudo:
        if (match == ALLOW || user_uid == 0) {
            /* User has an entry for this host. */
            CLR(validated, VALIDATE_NOT_OK);
@@ -145,34 +146,34 @@ sudoers_lookup(pwflag)
     /* Need to be runas user while stat'ing things. */
     set_perms(PERM_RUNAS);
 
-    /* XXX - it should be faster to start from the bottom and
-            work our way up and then just stop at the first match. */
     match = UNSPEC;
-    LH_FOREACH_FWD(&userspecs, us) {
+    LH_FOREACH_REV(&userspecs, us) {
        if (userlist_matches(sudo_user.pw, &us->users) != ALLOW)
            continue;
        CLR(validated, FLAG_NO_USER);
-       LH_FOREACH_FWD(&us->privileges, priv) {
+       LH_FOREACH_REV(&us->privileges, priv) {
            host_match = hostlist_matches(&priv->hostlist);
-           if (host_match == UNSPEC)
-               continue;
            if (host_match == ALLOW)
                CLR(validated, FLAG_NO_HOST);
+           else
+               continue;
            runas = NULL;
-           LH_FOREACH_FWD(&priv->cmndlist, cs) {
+           LH_FOREACH_REV(&priv->cmndlist, cs) {
                if (!LH_EMPTY(&cs->runaslist))
                    runas = &cs->runaslist;
                runas_match = runaslist_matches(runas);
-               if (runas_match != UNSPEC) {
+               if (runas_match == ALLOW) {
                    cmnd_match = cmnd_matches(cs->cmnd);
-                   if (cmnd_match != UNSPEC)
-                       match = host_match && runas_match && cmnd_match;
-                   if (match == ALLOW)
+                   if (cmnd_match != UNSPEC) {
+                       match = cmnd_match;
                        tags = &cs->tags;
+                       goto matched2;
+                   }
                }
            }
        }
     }
+    matched2:
     if (match == ALLOW) {
        CLR(validated, VALIDATE_NOT_OK);
        SET(validated, VALIDATE_OK);
@@ -433,28 +434,31 @@ display_cmnd(v, pw)
 #endif
     if (rval != 0 && !def_ignore_local_sudoers) {
        match = NULL;
-       LH_FOREACH_FWD(&userspecs, us) {
+       LH_FOREACH_REV(&userspecs, us) {
            if (userlist_matches(pw, &us->users) != ALLOW)
                continue;
 
-           LH_FOREACH_FWD(&us->privileges, priv) {
+           LH_FOREACH_REV(&us->privileges, priv) {
                host_match = hostlist_matches(&priv->hostlist);
-               if (host_match == UNSPEC)
+               if (host_match != ALLOW)
                    continue;
                runas = NULL;
-               LH_FOREACH_FWD(&priv->cmndlist, cs) {
+               LH_FOREACH_REV(&priv->cmndlist, cs) {
                    if (!LH_EMPTY(&cs->runaslist) != NULL)
                        runas = &cs->runaslist;
                    runas_match = runaslist_matches(runas);
-                   if (runas_match != UNSPEC) {
+                   if (runas_match == ALLOW) {
                        cmnd_match = cmnd_matches(cs->cmnd);
-                       if (cmnd_match != UNSPEC)
+                       if (cmnd_match != UNSPEC) {
                            match = host_match && runas_match ?
                                cs->cmnd : NULL;
+                           goto matched;
+                       }
                    }
                }
            }
        }
+       matched:
        if (match != NULL && !match->negated) {
            printf("%s%s%s\n", safe_cmnd, user_args ? " " : "",
                user_args ? user_args : "");

diff --git a/testsudoers.c b/testsudoers.c
index 0d6ffaf5a94085e00d6ef657d0d9d8f590e13ce5..9cb5b63e4a8bfe5c540d014b1a2e65036671b9aa 100644 (file)
--- a/testsudoers.c
+++ b/testsudoers.c
@@ -269,20 +269,20 @@ main(argc, argv)
     /* This loop must match the one in sudoers_lookup() */
     printf("\nEntries for user %s:\n", user_name);
     matched = UNSPEC;
-    LH_FOREACH_FWD(&userspecs, us) {
-       if (userlist_matches(sudo_user.pw, &us->users) != TRUE)
+    LH_FOREACH_REV(&userspecs, us) {
+       if (userlist_matches(sudo_user.pw, &us->users) != ALLOW)
            continue;
-       LH_FOREACH_FWD(&us->privileges, priv) {
+       LH_FOREACH_REV(&us->privileges, priv) {
            putchar('\n');
            print_privilege(priv); /* XXX */
            putchar('\n');
-           if (hostlist_matches(&priv->hostlist) == TRUE) {
+           if (hostlist_matches(&priv->hostlist) == ALLOW) {
                puts("\thost  matched");
                runas = NULL;
-               LH_FOREACH_FWD(&priv->cmndlist, cs) {
+               LH_FOREACH_REV(&priv->cmndlist, cs) {
                    if (!LH_EMPTY(&cs->runaslist))
                        runas = &cs->runaslist;
-                   if (runaslist_matches(runas) == TRUE) {
+                   if (runaslist_matches(runas) == ALLOW) {
                        puts("\trunas matched");
                        rval = cmnd_matches(cs->cmnd);
                        if (rval != UNSPEC)
@@ -291,11 +291,12 @@ main(argc, argv)
                            rval == DENY ? "denied" : "unmatched");
                    }
                }
-           } else puts("\thost  unmatched");
+           } else
+               puts("\thost  unmatched");
        }
     }
-    printf("\nCommand %s\n", matched == TRUE ? "allowed" :
-       matched == FALSE ? "denied" : "unmatched");
+    printf("\nCommand %s\n", matched == ALLOW ? "allowed" :
+       matched == DENY ? "denied" : "unmatched");
 
     exit(0);
 }