From 10e5d4e708ec78c6ed83c16e15c4475b1042721d Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Fri, 31 Aug 2007 01:21:26 +0000 Subject: [PATCH] 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. --- match.c | 18 +++++++++++++----- parse.c | 48 ++++++++++++++++++++++++++---------------------- testsudoers.c | 19 ++++++++++--------- 3 files changed, 49 insertions(+), 36 deletions(-) diff --git a/match.c b/match.c index 7dd8a6552..324e527ea 100644 --- 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 2433005eb..36c14a696 100644 --- 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 0d6ffaf5a..9cb5b63e4 100644 --- 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); } -- 2.40.0