From a08ea1b14d34d7e363bfa38528049c034b4c0620 Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Wed, 10 Aug 2016 10:56:05 -0600 Subject: [PATCH] Set runas_pw early and adjust runaslist_matches() to deal. Since we now set runas_default early there is no need to call update_defaults with SETDEF_RUNAS after sudoers has been parsed. --- doc/sudoers.cat | 14 ++++----- doc/sudoers.man.in | 7 ++--- doc/sudoers.mdoc.in | 7 ++--- plugins/sudoers/match.c | 16 ++++++++-- plugins/sudoers/parse.c | 3 +- plugins/sudoers/sudoers.c | 59 ++++++++++++----------------------- plugins/sudoers/testsudoers.c | 23 +++++++------- 7 files changed, 59 insertions(+), 70 deletions(-) diff --git a/doc/sudoers.cat b/doc/sudoers.cat index d60ae6898..9c6a9cc81 100644 --- a/doc/sudoers.cat +++ b/doc/sudoers.cat @@ -456,12 +456,12 @@ SSUUDDOOEERRSS FFIILLEE FFOORRMMAATT not an error to use the -= operator to remove an element that does not exist in a list. - Defaults entries are parsed in the following order: generic, host and - user Defaults first, then runas Defaults and finally command defaults. - If there are multiple Defaults settings of the same type, the last - matching setting is used. The following Defaults settings are parsed - before all others since they may affect subsequent entries: _f_q_d_n, - _g_r_o_u_p___p_l_u_g_i_n, _r_u_n_a_s___d_e_f_a_u_l_t, _s_u_d_o_e_r_s___l_o_c_a_l_e. + Defaults entries are parsed in the following order: generic, host, user + and runas Defaults first, then command defaults. If there are multiple + Defaults settings of the same type, the last matching setting is used. + The following Defaults settings are parsed before all others since they + may affect subsequent entries: _f_q_d_n, _g_r_o_u_p___p_l_u_g_i_n, _r_u_n_a_s___d_e_f_a_u_l_t, + _s_u_d_o_e_r_s___l_o_c_a_l_e. See _S_U_D_O_E_R_S _O_P_T_I_O_N_S for a list of supported Defaults parameters. @@ -2543,4 +2543,4 @@ DDIISSCCLLAAIIMMEERR file distributed with ssuuddoo or https://www.sudo.ws/license.html for complete details. -Sudo 1.8.18 August 9, 2016 Sudo 1.8.18 +Sudo 1.8.18 August 10, 2016 Sudo 1.8.18 diff --git a/doc/sudoers.man.in b/doc/sudoers.man.in index 5ac07fc4e..ba88c7643 100644 --- a/doc/sudoers.man.in +++ b/doc/sudoers.man.in @@ -21,7 +21,7 @@ .\" Agency (DARPA) and Air Force Research Laboratory, Air Force .\" Materiel Command, USAF, under agreement number F39502-99-1-0512. .\" -.TH "SUDOERS" "5" "August 9, 2016" "Sudo @PACKAGE_VERSION@" "File Formats Manual" +.TH "SUDOERS" "5" "August 10, 2016" "Sudo @PACKAGE_VERSION@" "File Formats Manual" .nh .if n .ad l .SH "NAME" @@ -954,9 +954,8 @@ It is not an error to use the operator to remove an element that does not exist in a list. .PP -Defaults entries are parsed in the following order: generic, host -and user Defaults first, then runas Defaults and finally command -defaults. +Defaults entries are parsed in the following order: generic, host, +user and runas Defaults first, then command defaults. If there are multiple Defaults settings of the same type, the last matching setting is used. The following Defaults settings are parsed before all others since diff --git a/doc/sudoers.mdoc.in b/doc/sudoers.mdoc.in index bf82a7277..33c4813eb 100644 --- a/doc/sudoers.mdoc.in +++ b/doc/sudoers.mdoc.in @@ -19,7 +19,7 @@ .\" Agency (DARPA) and Air Force Research Laboratory, Air Force .\" Materiel Command, USAF, under agreement number F39502-99-1-0512. .\" -.Dd August 9, 2016 +.Dd August 10, 2016 .Dt SUDOERS @mansectform@ .Os Sudo @PACKAGE_VERSION@ .Sh NAME @@ -908,9 +908,8 @@ It is not an error to use the operator to remove an element that does not exist in a list. .Pp -Defaults entries are parsed in the following order: generic, host -and user Defaults first, then runas Defaults and finally command -defaults. +Defaults entries are parsed in the following order: generic, host, +user and runas Defaults first, then command defaults. If there are multiple Defaults settings of the same type, the last matching setting is used. The following Defaults settings are parsed before all others since diff --git a/plugins/sudoers/match.c b/plugins/sudoers/match.c index fda2e33c3..d405e623e 100644 --- a/plugins/sudoers/match.c +++ b/plugins/sudoers/match.c @@ -153,10 +153,17 @@ runaslist_matches(const struct member_list *user_list, int group_matched = UNSPEC; debug_decl(runaslist_matches, SUDOERS_DEBUG_MATCH) - if (runas_pw != NULL) { + /* + * Skip checking runas user if it is the same as the invoking user + * and a runas group was specified. + * This logic assumes that we cache and refcount passwd structs. + */ + if (!(runas_pw == sudo_user.pw && runas_gr != NULL)) { /* If no runas user or runas group listed in sudoers, use default. */ - if (user_list == NULL && group_list == NULL) - debug_return_int(userpw_matches(def_runas_default, runas_pw->pw_name, runas_pw)); + if (user_list == NULL && group_list == NULL) { + debug_return_int(userpw_matches(def_runas_default, + runas_pw->pw_name, runas_pw)); + } if (user_list != NULL) { TAILQ_FOREACH_REVERSE(m, user_list, member_list, entries) { @@ -204,6 +211,9 @@ runaslist_matches(const struct member_list *user_list, } } + /* + * Skip checking runas group if none was specified. + */ if (runas_gr != NULL) { if (user_matched == UNSPEC) { if (runas_pw == NULL || strcmp(runas_pw->pw_name, user_name) == 0) diff --git a/plugins/sudoers/parse.c b/plugins/sudoers/parse.c index d35188ecc..0fde088aa 100644 --- a/plugins/sudoers/parse.c +++ b/plugins/sudoers/parse.c @@ -132,7 +132,8 @@ sudo_file_setdefs(struct sudo_nss *nss) if (nss->handle == NULL) debug_return_int(-1); - if (!update_defaults(SETDEF_GENERIC|SETDEF_HOST|SETDEF_USER, false)) + if (!update_defaults(SETDEF_GENERIC|SETDEF_HOST|SETDEF_USER|SETDEF_RUNAS, + false)) debug_return_int(-1); debug_return_int(0); } diff --git a/plugins/sudoers/sudoers.c b/plugins/sudoers/sudoers.c index 6b99046c7..67dd6b275 100644 --- a/plugins/sudoers/sudoers.c +++ b/plugins/sudoers/sudoers.c @@ -205,32 +205,7 @@ sudoers_policy_init(void *info, char * const envp[]) goto cleanup; } - /* XXX - collect post-sudoers parse settings into a function */ - - /* - * Set runas passwd/group entries based on command line or sudoers. - * Note that if runas_group was specified without runas_user we - * defer setting runas_pw so the match routines know to ignore it. - */ - /* XXX - qpm4u does more here as it may have already set runas_pw */ - if (runas_group != NULL) { - if (!set_runasgr(runas_group, false)) - goto cleanup; - if (runas_user != NULL) { - if (!set_runaspw(runas_user, false)) - goto cleanup; - } - } else { - if (!set_runaspw(runas_user ? runas_user : def_runas_default, false)) - goto cleanup; - } - - if (!update_defaults(SETDEF_RUNAS, false)) { - log_warningx(SLOG_SEND_MAIL|SLOG_NO_STDERR, - N_("problem with defaults entries")); - } - - /* Set login class if applicable. */ + /* Set login class if applicable (after sudoers is parsed). */ if (set_loginclass(runas_pw ? runas_pw : sudo_user.pw)) rval = true; @@ -389,13 +364,6 @@ sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[], } } - /* If only a group was specified, set runas_pw based on invoking user. */ - if (runas_pw == NULL) { - if (!set_runaspw(user_name, false)) { - goto done; - } - } - /* * Look up the timestamp dir owner if one is specified. */ @@ -707,8 +675,8 @@ init_vars(char * const envp[]) } /* - * Get a local copy of the user's struct passwd if we don't already - * have one. + * Get a local copy of the user's passwd struct and group list if we + * don't already have them. */ if (sudo_user.pw == NULL) { if ((sudo_user.pw = sudo_getpwnam(user_name)) == NULL) { @@ -727,12 +695,10 @@ init_vars(char * const envp[]) unknown_user = true; } } - - /* - * Get group list and store initialize permissions. - */ if (user_group_list == NULL) user_group_list = sudo_get_grlist(sudo_user.pw); + + /* Store initialize permissions so we can restore them later. */ if (!set_perms(PERM_INITIAL)) debug_return_bool(false); @@ -758,6 +724,21 @@ init_vars(char * const envp[]) debug_return_bool(false); } + /* + * Set runas passwd/group entries based on command line or sudoers. + * Note that if runas_group was specified without runas_user we + * run the command as the invoking user. + */ + if (runas_group != NULL) { + if (!set_runasgr(runas_group, false)) + debug_return_bool(false); + if (!set_runaspw(runas_user ? runas_user : user_name, false)) + debug_return_bool(false); + } else { + if (!set_runaspw(runas_user ? runas_user : def_runas_default, false)) + debug_return_bool(false); + } + debug_return_bool(true); } diff --git a/plugins/sudoers/testsudoers.c b/plugins/sudoers/testsudoers.c index ff19d04a0..fdf45bc15 100644 --- a/plugins/sudoers/testsudoers.c +++ b/plugins/sudoers/testsudoers.c @@ -261,6 +261,17 @@ main(int argc, char *argv[]) /* Allocate space for data structures in the parser. */ init_parser("sudoers", false); + /* + * Set runas passwd/group entries based on command line or sudoers. + * Note that if runas_group was specified without runas_user we + * run the command as the invoking user. + */ + if (runas_group != NULL) { + set_runasgr(runas_group); + set_runaspw(runas_user ? runas_user : user_name); + } else + set_runaspw(runas_user ? runas_user : def_runas_default); + sudoers_setlocale(SUDOERS_LOCALE_SUDOERS, NULL); if (sudoersparse() != 0 || parse_error) { parse_error = true; @@ -277,18 +288,6 @@ main(int argc, char *argv[]) (void) fputs(" (problem with defaults entries)", stdout); puts("."); - /* - * Set runas passwd/group entries based on command line or sudoers. - * Note that if runas_group was specified without runas_user we - * defer setting runas_pw so the match routines know to ignore it. - */ - if (runas_group != NULL) { - set_runasgr(runas_group); - if (runas_user != NULL) - set_runaspw(runas_user); - } else - set_runaspw(runas_user ? runas_user : def_runas_default); - if (dflag) { (void) putchar('\n'); dump_sudoers(); -- 2.50.1