isa/sudo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Resolve the list of gids passed in from the sudo frontend (the

Browse Source 
result of getgroups()) to names and store both the group names and
ids in the sudo_user struct.  When matching groups in the sudoers
file, match based on the names in the groups list first and
only do a gid-based match when we absolutely have to.  By matching
on the group name (as it is listed in sudoers) instead of id
(which we would have to resolve) we save a lot of group lookups
for sudoers files with a lot of groups in them.
This commit is contained in:
Todd C. Miller
2011-07-01 14:13:47 -04:00
parent 20972da410
commit 56321ec778
6 changed files with 172 additions and 104 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
plugins/sudoers/ldap.c
View File

@@ -328,7 +328,7 @@ struct sudo_ldap_handle {
LDAP *ld;
struct ldap_result *result;
char *username;
GETGROUPS_T *groups;
char **groups;
};
struct sudo_nss sudo_nss_ldap = {
@@ -989,12 +989,11 @@ sudo_ldap_build_pass1(struct passwd *pw)
sz += 12 + strlen(grp->gr_name); /* primary group */
gr_delref(grp);
}
if (strcmp(pw->pw_name, list_pw ? list_pw->pw_name : user_name) == 0) {
for (i = 0; i < user_ngroups; i++) {
if (user_groups[i] == pw->pw_gid)
if (user_gids[i] == pw->pw_gid)
continue;
if ((grp = sudo_getgrgid(user_groups[i])) != NULL) {
sz += 12 + strlen(grp->gr_name); /* supplementary group */
gr_delref(grp);
sz += 12 + strlen(user_groups[i]); /* supplementary group */
}
}
@@ -1028,14 +1027,13 @@ sudo_ldap_build_pass1(struct passwd *pw)
}
/* Append supplementary groups */
if (strcmp(pw->pw_name, list_pw ? list_pw->pw_name : user_name) == 0) {
for (i = 0; i < user_ngroups; i++) {
if (user_groups[i] == pw->pw_gid)
if (user_gids[i] == pw->pw_gid)
continue;
if ((grp = sudo_getgrgid(user_groups[i])) != NULL) {
(void) strlcat(buf, "(sudoUser=%", sz);
(void) strlcat(buf, grp->gr_name, sz);
(void) strlcat(buf, user_groups[i], sz);
(void) strlcat(buf, ")", sz);
gr_delref(grp);
}
}

75
plugins/sudoers/pwutil.c
View File

@@ -636,15 +636,55 @@ sudo_endgrent(void)
sudo_freegrcache();
}
#if defined(HAVE_GETGROUPS) && !defined(HAVE_MBR_CHECK_MEMBERSHIP)
static int
user_in_group_cached(const char *group)
{
gid_t gid = -1;
int i, retval = FALSE;
if (group[0] == '#')
gid = atoi(group + 1);
/* Check against user's primary (passwd file) group. */
if ((user_group != NULL && strcasecmp(group, user_group) == 0) ||
(group[0] == '#' && gid == user_gid)) {
retval = TRUE;
goto done;
}
/*
* If we are matching the invoking or list user and that user has a
* supplementary group vector, check it.
*/
for (i = 0; i < user_ngroups; i++) {
if (strcasecmp(group, user_groups[i]) == 0) {
retval = TRUE;
goto done;
}
}
if (group[0] == '#') {
for (i = 0; i < user_ngroups; i++) {
if (gid == user_gids[i]) {
retval = TRUE;
goto done;
}
}
}
done:
return retval;
}
#endif /* HAVE_GETGROUPS && !HAVE_MBR_CHECK_MEMBERSHIP */
int
user_in_group(struct passwd *pw, const char *group)
user_in_group_lookup(struct passwd *pw, const char *group)
{
#ifdef HAVE_MBR_CHECK_MEMBERSHIP
uuid_t gu, uu;
int ismember;
#else
char **gr_mem;
int i;
#endif
struct group *grp;
int retval = FALSE;
@@ -684,23 +724,7 @@ user_in_group(struct passwd *pw, const char *group)
}
}
#else /* HAVE_MBR_CHECK_MEMBERSHIP */
# ifdef HAVE_GETGROUPS
/*
* If we are matching the invoking or list user and that user has a
* supplementary group vector, check it.
*/
if (user_ngroups > 0 &&
strcmp(pw->pw_name, list_pw ? list_pw->pw_name : user_name) == 0) {
for (i = 0; i < user_ngroups; i++) {
if (grp->gr_gid == user_groups[i]) {
retval = TRUE;
goto done;
}
}
} else
# endif /* HAVE_GETGROUPS */
{
if (grp != NULL && grp->gr_mem != NULL) {
if (grp->gr_mem != NULL) {
for (gr_mem = grp->gr_mem; *gr_mem; gr_mem++) {
if (strcmp(*gr_mem, pw->pw_name) == 0) {
retval = TRUE;
@@ -708,7 +732,6 @@ user_in_group(struct passwd *pw, const char *group)
}
}
}
}
#endif /* HAVE_MBR_CHECK_MEMBERSHIP */
done:
@@ -716,3 +739,15 @@ done:
gr_delref(grp);
return retval;
}
int
user_in_group(struct passwd *pw, const char *group)
{
#if defined(HAVE_GETGROUPS) && !defined(HAVE_MBR_CHECK_MEMBERSHIP)
if (user_ngroups > 0 &&
strcmp(pw->pw_name, list_pw ? list_pw->pw_name : user_name) == 0) {
return user_in_group_cached(group);
}
#endif /* HAVE_GETGROUPS && !HAVE_MBR_CHECK_MEMBERSHIP */
return user_in_group_lookup(pw, group);
}

108
plugins/sudoers/set_perms.c
View File

@@ -67,7 +67,7 @@ struct perm_state {
#ifdef HAVE_SETRESUID
gid_t sgid;
#endif
GETGROUPS_T *groups;
GETGROUPS_T *gids;
int ngroups;
};
@@ -78,7 +78,7 @@ static int perm_stack_depth = 0;
/* XXX - make a runas_user struct? */
int runas_ngroups = -1;
#ifdef HAVE_GETGROUPS
GETGROUPS_T *runas_groups;
GETGROUPS_T *runas_gids;
#endif
#undef ID
@@ -146,7 +146,7 @@ set_perms(int perm)
state->egid = getegid();
state->sgid = state->egid; /* in case we are setgid */
#endif
state->groups = user_groups;
state->gids = user_gids;
state->ngroups = user_ngroups;
break;
@@ -161,15 +161,15 @@ set_perms(int perm)
state->rgid = -1;
state->egid = -1;
state->sgid = -1;
state->groups = NULL;
state->gids = NULL;
state->ngroups = -1;
break;
case PERM_USER:
state->groups = user_groups;
state->gids = user_gids;
state->ngroups = user_ngroups;
if (state->ngroups != -1 && state->groups != ostate->groups) {
if (setgroups(state->ngroups, state->groups)) {
if (state->ngroups != -1 && state->gids != ostate->gids) {
if (setgroups(state->ngroups, state->gids)) {
errstr = "setgroups()";
goto bad;
}
@@ -192,10 +192,10 @@ set_perms(int perm)
case PERM_FULL_USER:
/* headed for exec() */
state->groups = user_groups;
state->gids = user_gids;
state->ngroups = user_ngroups;
if (state->ngroups != -1 && state->groups != ostate->groups) {
if (setgroups(state->ngroups, state->groups)) {
if (state->ngroups != -1 && state->gids != ostate->gids) {
if (setgroups(state->ngroups, state->gids)) {
errstr = "setgroups()";
goto bad;
}
@@ -218,7 +218,7 @@ set_perms(int perm)
case PERM_RUNAS:
runas_setgroups();
state->groups = runas_groups;
state->gids = runas_gids;
state->ngroups = runas_ngroups;
state->rgid = -1;
@@ -238,7 +238,7 @@ set_perms(int perm)
break;
case PERM_SUDOERS:
state->groups = NULL;
state->gids = NULL;
state->ngroups = -1;
/* assumes euid == ROOT_UID, ruid == user */
@@ -266,7 +266,7 @@ set_perms(int perm)
break;
case PERM_TIMESTAMP:
state->groups = NULL;
state->gids = NULL;
state->ngroups = -1;
state->rgid = -1;
state->egid = -1;
@@ -323,8 +323,8 @@ restore_perms(void)
state->egid, state->sgid, OID(rgid), OID(egid), OID(sgid));
goto bad;
}
if (state->ngroups != -1 && state->groups != ostate->groups) {
if (setgroups(ostate->ngroups, ostate->groups)) {
if (state->ngroups != -1 && state->gids != ostate->gids) {
if (setgroups(ostate->ngroups, ostate->gids)) {
warning("setgroups()");
goto bad;
}
@@ -374,7 +374,7 @@ set_perms(int perm)
state->euid = geteuid();
state->rgid = getgid();
state->egid = getegid();
state->groups = user_groups;
state->gids = user_gids;
state->ngroups = user_ngroups;
break;
@@ -395,15 +395,15 @@ set_perms(int perm)
state->euid = ROOT_UID;
state->rgid = -1;
state->egid = -1;
state->groups = NULL;
state->gids = NULL;
state->ngroups = -1;
break;
case PERM_USER:
state->groups = user_groups;
state->gids = user_gids;
state->ngroups = user_ngroups;
if (state->ngroups != -1 && state->groups != ostate->groups) {
if (setgroups(state->ngroups, state->groups)) {
if (state->ngroups != -1 && state->gids != ostate->gids) {
if (setgroups(state->ngroups, state->gids)) {
errstr = "setgroups()";
goto bad;
}
@@ -424,10 +424,10 @@ set_perms(int perm)
case PERM_FULL_USER:
/* headed for exec() */
state->groups = user_groups;
state->gids = user_gids;
state->ngroups = user_ngroups;
if (state->ngroups != -1 && state->groups != ostate->groups) {
if (setgroups(state->ngroups, state->groups)) {
if (state->ngroups != -1 && state->gids != ostate->gids) {
if (setgroups(state->ngroups, state->gids)) {
errstr = "setgroups()";
goto bad;
}
@@ -448,7 +448,7 @@ set_perms(int perm)
case PERM_RUNAS:
runas_setgroups();
state->groups = runas_groups;
state->gids = runas_gids;
state->ngroups = runas_ngroups;
state->rgid = -1;
@@ -466,7 +466,7 @@ set_perms(int perm)
break;
case PERM_SUDOERS:
state->groups = NULL;
state->gids = NULL;
state->ngroups = -1;
/* assume euid == ROOT_UID, ruid == user */
@@ -492,7 +492,7 @@ set_perms(int perm)
break;
case PERM_TIMESTAMP:
state->groups = NULL;
state->gids = NULL;
state->ngroups = -1;
state->rgid = -1;
state->egid = -1;
@@ -552,8 +552,8 @@ restore_perms(void)
state->egid, OID(rgid), OID(egid));
goto bad;
}
if (state->ngroups != -1 && state->groups != ostate->groups) {
if (setgroups(ostate->ngroups, ostate->groups)) {
if (state->ngroups != -1 && state->gids != ostate->gids) {
if (setgroups(ostate->ngroups, ostate->gids)) {
warning("setgroups()");
goto bad;
}
@@ -619,7 +619,7 @@ set_perms(int perm)
state->euid = geteuid();
state->rgid = getgid();
state->egid = getegid();
state->groups = user_groups;
state->gids = user_gids;
state->ngroups = user_ngroups;
break;
@@ -629,15 +629,15 @@ set_perms(int perm)
state->euid = ROOT_UID;
state->rgid = -1;
state->egid = -1;
state->groups = NULL;
state->gids = NULL;
state->ngroups = -1;
break;
case PERM_USER:
state->groups = user_groups;
state->gids = user_gids;
state->ngroups = user_ngroups;
if (state->ngroups != -1 && state->groups != ostate->groups) {
if (setgroups(state->ngroups, state->groups)) {
if (state->ngroups != -1 && state->gids != ostate->gids) {
if (setgroups(state->ngroups, state->gids)) {
errstr = "setgroups()";
goto bad;
}
@@ -658,10 +658,10 @@ set_perms(int perm)
case PERM_FULL_USER:
/* headed for exec() */
state->groups = user_groups;
state->gids = user_gids;
state->ngroups = user_ngroups;
if (state->ngroups != -1 && state->groups != ostate->groups) {
if (setgroups(state->ngroups, state->groups)) {
if (state->ngroups != -1 && state->gids != ostate->gids) {
if (setgroups(state->ngroups, state->gids)) {
errstr = "setgroups()";
goto bad;
}
@@ -682,7 +682,7 @@ set_perms(int perm)
case PERM_RUNAS:
runas_setgroups();
state->groups = runas_groups;
state->gids = runas_gids;
state->ngroups = runas_ngroups;
state->rgid = -1;
@@ -700,7 +700,7 @@ set_perms(int perm)
break;
case PERM_SUDOERS:
state->groups = NULL;
state->gids = NULL;
state->ngroups = -1;
/* assume euid == ROOT_UID, ruid == user */
@@ -726,7 +726,7 @@ set_perms(int perm)
break;
case PERM_TIMESTAMP:
state->groups = NULL;
state->gids = NULL;
state->ngroups = -1;
state->rgid = -1;
state->egid = -1;
@@ -781,8 +781,8 @@ restore_perms(void)
warning("setegid(%d)", OID(egid));
goto bad;
}
if (state->ngroups != -1 && state->groups != ostate->groups) {
if (setgroups(ostate->ngroups, ostate->groups)) {
if (state->ngroups != -1 && state->gids != ostate->gids) {
if (setgroups(ostate->ngroups, ostate->gids)) {
warning("setgroups()");
goto bad;
}
@@ -832,14 +832,14 @@ set_perms(int perm)
/* Stash initial state */
state->ruid = getuid();
state->rgid = getgid();
state->groups = user_groups;
state->gids = user_gids;
state->ngroups = user_ngroups;
break;
case PERM_ROOT:
state->ruid = ROOT_UID;
state->rgid = -1;
state->groups = NULL;
state->gids = NULL;
state->ngroups = -1;
if (setuid(ROOT_UID)) {
errstr = "setuid(ROOT_UID)";
@@ -848,10 +848,10 @@ set_perms(int perm)
break;
case PERM_FULL_USER:
state->groups = user_groups;
state->gids = user_gids;
state->ngroups = user_ngroups;
if (state->ngroups != -1 && state->groups != ostate->groups) {
if (setgroups(state->ngroups, state->groups)) {
if (state->ngroups != -1 && state->gids != ostate->gids) {
if (setgroups(state->ngroups, state->gids)) {
errstr = "setgroups()";
goto bad;
}
@@ -897,8 +897,8 @@ restore_perms(void)
ostate = &perm_stack[perm_stack_depth - 2];
perm_stack_depth--;
if (state->ngroups != -1 && state->groups != ostate->groups) {
if (setgroups(ostate->ngroups, ostate->groups)) {
if (state->ngroups != -1 && state->gids != ostate->gids) {
if (setgroups(ostate->ngroups, ostate->gids)) {
warning("setgroups()");
goto bad;
}
@@ -942,20 +942,20 @@ runas_setgroups()
if (initgroups(pw->pw_name, pw->pw_gid) < 0)
log_error(USE_ERRNO|MSG_ONLY, _("unable to set runas group vector"));
# ifdef HAVE_GETGROUPS
if (runas_groups) {
efree(runas_groups);
runas_groups = NULL;
if (runas_gids) {
efree(runas_gids);
runas_gids = NULL;
}
if ((runas_ngroups = getgroups(0, NULL)) > 0) {
runas_groups = emalloc2(runas_ngroups, sizeof(GETGROUPS_T));
if (getgroups(runas_ngroups, runas_groups) < 0)
runas_gids = emalloc2(runas_ngroups, sizeof(GETGROUPS_T));
if (getgroups(runas_ngroups, runas_gids) < 0)
log_error(USE_ERRNO|MSG_ONLY, _("unable to get runas group vector"));
}
# ifdef HAVE_SETAUTHDB
aix_restoreauthdb();
# endif
} else {
if (setgroups(runas_ngroups, runas_groups) < 0)
if (setgroups(runas_ngroups, runas_gids) < 0)
log_error(USE_ERRNO|MSG_ONLY, _("unable to set runas group vector"));
# endif /* HAVE_GETGROUPS */
}

22
plugins/sudoers/sudo_nss.c
View File

@@ -204,23 +204,39 @@ sudo_read_nss(void)
#endif /* HAVE_LDAP && _PATH_NSSWITCH_CONF */
/* Reset user_groups based on passwd entry. */
/* Reset user_gids and user_groups based on passwd entry. */
static void
reset_groups(struct passwd *pw)
{
#if defined(HAVE_INITGROUPS) && defined(HAVE_GETGROUPS)
if (pw != sudo_user.pw) {
struct group *grp;
int i;
# ifdef HAVE_SETAUTHDB
aix_setauthdb(pw->pw_name);
# endif
if (initgroups(pw->pw_name, pw->pw_gid) == -1)
log_error(USE_ERRNO|MSG_ONLY, _("unable to reset group vector"));
efree(user_gids);
user_gids = NULL;
efree(user_groups);
user_groups = NULL;
if ((user_ngroups = getgroups(0, NULL)) > 0) {
user_groups = emalloc2(user_ngroups, sizeof(GETGROUPS_T));
if (getgroups(user_ngroups, user_groups) < 0)
user_gids = emalloc2(user_ngroups, sizeof(GETGROUPS_T));
if (getgroups(user_ngroups, user_gids) < 0)
log_error(USE_ERRNO|MSG_ONLY, _("unable to get group vector"));
user_groups = emalloc2(user_ngroups, sizeof(char *));
for (i = 0; i < user_ngroups; i++) {
grp = sudo_getgrgid(user_gids[i]);
if (grp != NULL) {
user_groups[i] = estrdup(grp->gr_name);
gr_delref(grp);
} else {
easprintf(&user_groups[i], "#%u",
(unsigned int) user_gids[i]);
}
}
}
# ifdef HAVE_SETAUTHDB
aix_restoreauthdb();

25
plugins/sudoers/sudoers.c
View File

@@ -101,7 +101,7 @@ static void create_admin_success_flag(void);
/* XXX */
extern int runas_ngroups;
extern GETGROUPS_T *runas_groups;
extern GETGROUPS_T *runas_gids;
/*
* Globals
@@ -647,7 +647,7 @@ sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[],
for (i = 0; i < runas_ngroups; i++) {
/* XXX - check rval */
len = snprintf(cp, glsize - (cp - gid_list), "%s%u",
i ? "," : "", (unsigned int) runas_groups[i]);
i ? "," : "", (unsigned int) runas_gids[i]);
cp += len;
}
command_info[info_len++] = gid_list;
@@ -1142,6 +1142,7 @@ sudoers_policy_version(int verbose)
static int
deserialize_info(char * const settings[], char * const user_info[])
{
struct group *grp;
char * const *cur;
const char *p;
int flags = 0;
@@ -1280,7 +1281,12 @@ deserialize_info(char * const settings[], char * const user_info[])
continue;
}
if (MATCHES(*cur, "gid=")) {
user_gid = (gid_t) atoi(*cur + sizeof("gid=") - 1);
p = *cur + sizeof("gid=") - 1;
user_gid = (gid_t) atoi(p);
if ((grp = sudo_getgrgid(user_gid)) != NULL) {
user_group = estrdup(grp->gr_name);
gr_delref(grp);
}
continue;
}
if (MATCHES(*cur, "groups=")) {
@@ -1294,12 +1300,21 @@ deserialize_info(char * const settings[], char * const user_info[])
user_ngroups++;
}
user_groups = emalloc2(user_ngroups, sizeof(GETGROUPS_T));
user_gids = emalloc2(user_ngroups, sizeof(GETGROUPS_T));
user_groups = emalloc2(user_ngroups, sizeof(char *));
user_ngroups = 0;
cp = val;
for (;;) {
/* XXX - strtol would be better here */
user_groups[user_ngroups++] = atoi(cp);
grp = sudo_getgrgid(atoi(cp));
if (grp != NULL) {
user_gids[user_ngroups] = grp->gr_gid;
user_groups[user_ngroups] = estrdup(grp->gr_name);
gr_delref(grp);
} else {
easprintf(&user_groups[user_ngroups], "#%s", cp);
}
user_ngroups++;
cp = strchr(cp, ',');
if (cp == NULL)
break;

18
plugins/sudoers/sudoers.h
View File

@@ -63,13 +63,9 @@ struct sudo_user {
char *cmnd_safe;
char *class_name;
char *krb5_ccname;
int closefrom;
int ngroups;
uid_t uid;
uid_t gid;
int lines;
int cols;
GETGROUPS_T *groups;
char *group;
char **groups;
GETGROUPS_T *gids;
char * const * env_vars;
#ifdef HAVE_SELINUX
char *role;
@@ -77,6 +73,12 @@ struct sudo_user {
#endif
char *cwd;
char *iolog_file;
int closefrom;
int ngroups;
uid_t uid;
uid_t gid;
int lines;
int cols;
#ifdef HAVE_MBR_CHECK_MEMBERSHIP
uuid_t uuid;
#endif
@@ -159,6 +161,8 @@ struct sudo_user {
#define user_uuid (sudo_user.uuid)
#define user_dir (sudo_user.pw->pw_dir)
#define user_ngroups (sudo_user.ngroups)
#define user_gids (sudo_user.gids)
#define user_group (sudo_user.group)
#define user_groups (sudo_user.groups)
#define user_tty (sudo_user.tty)
#define user_ttypath (sudo_user.ttypath)