From 28ed51b441f5828081597e25da731de0699276df Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Sat, 5 Jan 2008 18:27:18 +0000 Subject: [PATCH] Improve chaining of multiple sudoers sources by passing in the previous return value to the next in the chain --- ldap.c | 72 ++++++++++++++++++++++++++++++------------------------ parse.c | 17 +++++++------ sudo.c | 30 +++++++++++------------ sudo.h | 4 +-- sudo_nss.h | 2 +- 5 files changed, 67 insertions(+), 58 deletions(-) diff --git a/ldap.c b/ldap.c index bf2d8c0d5..d4baa202f 100644 --- a/ldap.c +++ b/ldap.c @@ -585,7 +585,8 @@ sudo_ldap_check_runas(ld, entry) } /* - * Walk through search results and return TRUE if we have a command match. + * Walk through search results and return TRUE if we have a command match, + * FALSE if disallowed and UNSPEC if not matched. */ int sudo_ldap_check_command(ld, entry, setenv_implied) @@ -595,7 +596,7 @@ sudo_ldap_check_command(ld, entry, setenv_implied) { struct berval **bv, **p; char *allowed_cmnd, *allowed_args, *val; - int foundbang, ret = FALSE; + int foundbang, ret = UNSPEC; if (!entry) return(ret); @@ -604,7 +605,7 @@ sudo_ldap_check_command(ld, entry, setenv_implied) if (bv == NULL) return(ret); - for (p = bv; *p != NULL && ret >= 0; p++) { + for (p = bv; *p != NULL && ret != FALSE; p++) { val = (*p)->bv_val; /* Match against ALL ? */ if (!strcmp(val, "ALL")) { @@ -635,7 +636,7 @@ sudo_ldap_check_command(ld, entry, setenv_implied) * If allowed (no bang) set ret but keep on checking. * If disallowed (bang), exit loop. */ - ret = foundbang ? -1 : TRUE; + ret = foundbang ? FALSE : TRUE; } DPRINTF(("ldap sudoCommand '%s' ... %s", val, ret == TRUE ? "MATCH!" : "not"), 2); @@ -645,8 +646,7 @@ sudo_ldap_check_command(ld, entry, setenv_implied) ldap_value_free_len(bv); /* more cleanup */ - /* return TRUE if we found at least one ALLOW and no DENY */ - return(ret > 0); + return(ret); } /* @@ -1523,27 +1523,28 @@ sudo_ldap_setdefs(nss) * like sudoers_lookup() - only LDAP style */ int -sudo_ldap_lookup(nss, pwflag) +sudo_ldap_lookup(nss, ret, pwflag) struct sudo_nss *nss; + int ret; int pwflag; { LDAP *ld = (LDAP *) nss->handle; - LDAPMessage *entry = NULL, *result = NULL; /* used for searches */ - char *filt; /* used to parse attributes */ - int do_netgr, rc, ret; /* temp/final return values */ + LDAPMessage *entry = NULL, *result = NULL; + char *filt; + int do_netgr, rc, matched; int setenv_implied; - int ldap_user_matches = FALSE, ldap_host_matches = FALSE; /* flags */ + int ldap_user_matches = FALSE, ldap_host_matches = FALSE; struct passwd *pw = list_pw ? list_pw : sudo_user.pw; if (ld == NULL) - return(VALIDATE_NOT_OK | FLAG_NO_HOST | FLAG_NO_USER); + return(ret); if (pwflag) { int doauth = UNSPEC; enum def_tupple pwcheck = (pwflag == -1) ? never : sudo_defs_table[pwflag].sd_un.tuple; - for (ret = 0, do_netgr = 0; !ret && do_netgr < 2; do_netgr++) { + for (matched = 0, do_netgr = 0; !matched && do_netgr < 2; do_netgr++) { filt = do_netgr ? estrdup("sudoUser=+*") : sudo_ldap_build_pass1(pw); rc = ldap_search_ext_s(ld, ldap_conf.base, LDAP_SCOPE_SUBTREE, filt, NULL, 0, NULL, NULL, NULL, -1, &result); @@ -1566,7 +1567,7 @@ sudo_ldap_lookup(nss, pwflag) if (user_uid == 0 || list_pw == NULL || user_uid == list_pw->pw_uid || sudo_ldap_check_command(ld, entry, NULL)) { - ret = 1; + matched = 1; break; /* end foreach */ } } @@ -1574,8 +1575,9 @@ sudo_ldap_lookup(nss, pwflag) ldap_msgfree(result); result = NULL; } - if (ret || user_uid == 0) { - ret = VALIDATE_OK; + if (matched || user_uid == 0) { + SET(ret, VALIDATE_OK); + CLR(ret, VALIDATE_NOT_OK); if (def_authenticate) { switch (pwcheck) { case always: @@ -1612,7 +1614,7 @@ sudo_ldap_lookup(nss, pwflag) * try to match them against the username. */ setenv_implied = FALSE; - for (ret = 0, do_netgr = 0; !ret && do_netgr < 2; do_netgr++) { + for (matched = 0, do_netgr = 0; !matched && do_netgr < 2; do_netgr++) { filt = do_netgr ? estrdup("sudoUser=+*") : sudo_ldap_build_pass1(pw); DPRINTF(("ldap search '%s'", filt), 1); rc = ldap_search_ext_s(ld, ldap_conf.base, LDAP_SCOPE_SUBTREE, filt, @@ -1634,19 +1636,26 @@ sudo_ldap_lookup(nss, pwflag) sudo_ldap_check_host(ld, entry) && /* remember that host matched */ (ldap_host_matches = TRUE) && - /* verify command match */ - sudo_ldap_check_command(ld, entry, &setenv_implied) && /* verify runas match */ - sudo_ldap_check_runas(ld, entry) + sudo_ldap_check_runas(ld, entry) && + /* verify command match */ + (rc = sudo_ldap_check_command(ld, entry, &setenv_implied)) != UNSPEC ) { /* We have a match! */ - DPRINTF(("Perfect Match!"), 1); - /* pick up any options */ - if (setenv_implied) - def_setenv = TRUE; - sudo_ldap_parse_options(ld, entry); - /* make sure we don't reenter loop */ - ret = VALIDATE_OK; + DPRINTF(("Command %sallowed", rc == TRUE ? "" : "NOT "), 1); + matched = TRUE; + if (rc == TRUE) { + /* pick up any options */ + if (setenv_implied) + def_setenv = TRUE; + sudo_ldap_parse_options(ld, entry); + /* make sure we don't reenter loop */ + SET(ret, VALIDATE_OK); + CLR(ret, VALIDATE_NOT_OK); + } else { + SET(ret, VALIDATE_NOT_OK); + CLR(ret, VALIDATE_OK); + } /* break from inside for loop */ break; } @@ -1662,14 +1671,13 @@ done: if (!ISSET(ret, VALIDATE_OK)) { /* we do not have a match */ - ret = VALIDATE_NOT_OK; if (pwflag && list_pw == NULL) SET(ret, FLAG_NO_CHECK); - else if (!ldap_user_matches) - SET(ret, FLAG_NO_USER); - else if (!ldap_host_matches) - SET(ret, FLAG_NO_HOST); } + if (ldap_user_matches) + CLR(ret, FLAG_NO_USER); + if (ldap_host_matches) + CLR(ret, FLAG_NO_HOST); DPRINTF(("sudo_ldap_lookup(%d)=0x%02x", pwflag, ret), 1); return(ret); diff --git a/parse.c b/parse.c index 5d65e333b..bcaf00420 100644 --- a/parse.c +++ b/parse.c @@ -188,19 +188,17 @@ sudo_file_setdefs(nss) * allowed to run the specified command on this host as the target user. */ int -sudo_file_lookup(nss, pwflag) +sudo_file_lookup(nss, validated, pwflag) struct sudo_nss *nss; + int validated; int pwflag; { - int validated, match, host_match, runas_match, cmnd_match; + int match, host_match, runas_match, cmnd_match; struct cmndspec *cs; struct cmndtag *tags = NULL; struct privilege *priv; struct userspec *us; - /* Assume the worst. */ - validated = VALIDATE_NOT_OK | FLAG_NO_HOST | FLAG_NO_USER; - if (nss->handle == NULL) return(validated); @@ -243,9 +241,9 @@ sudo_file_lookup(nss, pwflag) matched_pseudo: if (match == ALLOW || user_uid == 0) { /* User has an entry for this host. */ - CLR(validated, VALIDATE_NOT_OK); SET(validated, VALIDATE_OK); - } + } else if (match == DENY) + SET(validated, VALIDATE_NOT_OK); if (pwcheck == always && def_authenticate) SET(validated, FLAG_CHECK_USER); else if (pwcheck == never || nopass == TRUE) @@ -283,8 +281,8 @@ sudo_file_lookup(nss, pwflag) } matched2: if (match == ALLOW) { - CLR(validated, VALIDATE_NOT_OK); SET(validated, VALIDATE_OK); + CLR(validated, VALIDATE_NOT_OK); if (tags != NULL) { if (tags->nopasswd != UNSPEC) def_authenticate = !tags->nopasswd; @@ -293,6 +291,9 @@ sudo_file_lookup(nss, pwflag) if (tags->setenv != UNSPEC) def_setenv = tags->setenv; } + } else if (match == DENY) { + SET(validated, VALIDATE_NOT_OK); + CLR(validated, VALIDATE_OK); } set_perms(PERM_ROOT); return(validated); diff --git a/sudo.c b/sudo.c index a6e2823fc..a34391997 100644 --- a/sudo.c +++ b/sudo.c @@ -158,7 +158,7 @@ main(argc, argv, envp) char **argv; char **envp; { - int sources = 0, validated = 0; + int sources = 0, validated; int fd, cmnd_status, sudo_mode, pwflag, rc = 0; sigaction_t sa; #if defined(SUDO_DEVEL) && defined(__OpenBSD__) @@ -332,17 +332,22 @@ main(argc, argv, envp) cmnd_status = set_cmnd(sudo_mode); + validated = FLAG_NO_USER | FLAG_NO_HOST; tq_foreach_fwd(snl, nss) { - rc = nss->lookup(nss, pwflag); + validated = nss->lookup(nss, validated, pwflag); - /* XXX - rethink this logic */ - if (validated == 0 || ISSET(rc, VALIDATE_OK)) - validated = rc; - else if (ISSET(rc, VALIDATE_NOT_OK) && ISSET(validated, VALIDATE_NOT_OK)) - validated |= rc; +/* + VALIDATE_OK + VALIDATE_NOT_OK + FLAG_CHECK_USER + FLAG_NO_USER + FLAG_NO_HOST + FLAG_NO_CHECK +*/ /* Handle [NOTFOUND=return] */ - if (!ISSET(rc, VALIDATE_OK) && nss->ret_notfound) + /* XXX - no longer a valid check due to inheriting validated */ + if (!ISSET(validated, VALIDATE_OK) && nss->ret_notfound) break; } if (safe_cmnd == NULL) @@ -504,13 +509,12 @@ main(argc, argv, envp) NewArgv[0] = "sh"; NewArgv[1] = safe_cmnd; execv(_PATH_BSHELL, NewArgv); - } - warning("unable to execute %s", safe_cmnd); + } warning("unable to execute %s", safe_cmnd); exit(127); } else if (ISSET(validated, FLAG_NO_USER) || (validated & FLAG_NO_HOST)) { log_auth(validated, 1); exit(1); - } else if (ISSET(validated, VALIDATE_NOT_OK)) { + } else { if (def_path_info) { /* * We'd like to not leak path info at all here, but that can @@ -530,10 +534,6 @@ main(argc, argv, envp) log_auth(validated, 1); } exit(1); - } else { - /* should never get here */ - log_auth(validated, 1); - exit(1); } exit(0); /* not reached */ } diff --git a/sudo.h b/sudo.h index c03059de9..507d8e469 100644 --- a/sudo.h +++ b/sudo.h @@ -230,7 +230,7 @@ void verify_user __P((struct passwd *, char *)); int sudo_ldap_open __P((struct sudo_nss *)); int sudo_ldap_close __P((struct sudo_nss *)); int sudo_ldap_setdefs __P((struct sudo_nss *)); -int sudo_ldap_lookup __P((struct sudo_nss *, int)); +int sudo_ldap_lookup __P((struct sudo_nss *, int, int)); int sudo_ldap_parse __P((struct sudo_nss *)); void sudo_ldap_display_privs __P((struct sudo_nss *, struct passwd *)); int sudo_ldap_display_cmnd __P((struct sudo_nss *, struct passwd *)); @@ -239,7 +239,7 @@ int sudo_ldap_display_cmnd __P((struct sudo_nss *, struct passwd *)); int sudo_file_open __P((struct sudo_nss *)); int sudo_file_close __P((struct sudo_nss *)); int sudo_file_setdefs __P((struct sudo_nss *)); -int sudo_file_lookup __P((struct sudo_nss *, int)); +int sudo_file_lookup __P((struct sudo_nss *, int, int)); int sudo_file_parse __P((struct sudo_nss *)); void sudo_file_display_privs __P((struct sudo_nss *, struct passwd *)); int sudo_file_display_cmnd __P((struct sudo_nss *, struct passwd *)); diff --git a/sudo_nss.h b/sudo_nss.h index 6cfe86c32..46a522ce9 100644 --- a/sudo_nss.h +++ b/sudo_nss.h @@ -24,7 +24,7 @@ struct sudo_nss { int (*close) __P((struct sudo_nss *nss)); int (*parse) __P((struct sudo_nss *nss)); int (*setdefs) __P((struct sudo_nss *nss)); - int (*lookup) __P((struct sudo_nss *nss, int)); + int (*lookup) __P((struct sudo_nss *nss, int, int)); void (*display_privs) __P((struct sudo_nss *nss, struct passwd *)); int (*display_cmnd) __P((struct sudo_nss *nss, struct passwd *)); void *handle; -- 2.40.0