| Message ID | 1469654706-26930-1-git-send-email-gary.tierney@gmx.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
Any specific reason why this is being ignored? On 07/27/2016 11:25 PM, Gary Tierney wrote: > semanage-login supports login mappings using the %group syntax, but > genhomedircon does not expand groups to the users belonging to them. > > This commit adds support for generating home directory contexts for login > mappings using the group syntax and adds error reporting for handling cases > where there is ambiguity due to a user belonging to multiple groups mapped by > semanage-login. If a login mapping is added for the user which belongs to > multiple groups it will take precedence and resolve the ambiguity issue. > > Signed-off-by: Gary Tierney <gary.tierney@gmx.com> > --- > libsemanage/src/genhomedircon.c | 319 +++++++++++++++++++++++++++++++--------- > 1 file changed, 247 insertions(+), 72 deletions(-) > > diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c > index c5ea436..2955b19 100644 > --- a/libsemanage/src/genhomedircon.c > +++ b/libsemanage/src/genhomedircon.c > @@ -48,6 +48,8 @@ > #include <errno.h> > #include <unistd.h> > #include <regex.h> > +#include <grp.h> > +#include <search.h> > > /* paths used in get_home_dirs() */ > #define PATH_ETC_USERADD "/etc/default/useradd" > @@ -98,6 +100,10 @@ typedef struct user_entry { > char *prefix; > char *home; > char *level; > + > + // The login identifier that was used > + // in semanage-login / seusers > + char *login; > struct user_entry *next; > } genhomedircon_user_entry_t; > > @@ -486,6 +492,11 @@ static int USER_CONTEXT_PRED(const char *string) > return (int)(strstr(string, TEMPLATE_USER) != NULL); > } > > +static int STR_COMPARATOR(const void *a, const void *b) > +{ > + return strcmp((const char *) a, (const char *) b); > +} > + > /* make_tempate > * @param s the settings holding the paths to various files > * @param pred function pointer to function to use as filter for slurp > @@ -652,6 +663,24 @@ static int write_user_context(genhomedircon_settings_t * s, FILE * out, > return write_replacements(s, out, tpl, repl); > } > > +static int seuser_sort_func(const void *arg1, const void *arg2) > +{ > + const semanage_seuser_t **u1 = (const semanage_seuser_t **) arg1; > + const semanage_seuser_t **u2 = (const semanage_seuser_t **) arg2;; > + const char *name1 = semanage_seuser_get_name(*u1); > + const char *name2 = semanage_seuser_get_name(*u2); > + > + if (name1[0] == '%' && name2[0] == '%') { > + return 0; > + } else if (name1[0] == '%') { > + return 1; > + } else if (name2[0] == '%') { > + return -1; > + } > + > + return strcmp(name1, name2); > +} > + > static int user_sort_func(semanage_user_t ** arg1, semanage_user_t ** arg2) > { > return strcmp(semanage_user_get_name(*arg1), > @@ -665,7 +694,8 @@ static int name_user_cmp(char *key, semanage_user_t ** val) > > static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, > const char *u, const char *g, const char *sen, > - const char *pre, const char *h, const char *l) > + const char *pre, const char *h, const char *l, > + const char *ln) > { > genhomedircon_user_entry_t *temp = NULL; > char *name = NULL; > @@ -675,6 +705,7 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, > char *prefix = NULL; > char *home = NULL; > char *level = NULL; > + char *lname = NULL; > > temp = malloc(sizeof(genhomedircon_user_entry_t)); > if (!temp) > @@ -700,6 +731,9 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, > level = strdup(l); > if (!level) > goto cleanup; > + lname = strdup(ln); > + if (!lname) > + goto cleanup; > > temp->name = name; > temp->uid = uid; > @@ -708,6 +742,7 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, > temp->prefix = prefix; > temp->home = home; > temp->level = level; > + temp->login = lname; > temp->next = (*list); > (*list) = temp; > > @@ -721,6 +756,7 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, > free(prefix); > free(home); > free(level); > + free(lname); > free(temp); > return STATUS_ERR; > } > @@ -741,6 +777,7 @@ static void pop_user_entry(genhomedircon_user_entry_t ** list) > free(temp->prefix); > free(temp->home); > free(temp->level); > + free(temp->login); > free(temp); > } > > @@ -790,7 +827,8 @@ static int setup_fallback_user(genhomedircon_settings_t * s) > > if (push_user_entry(&(s->fallback), FALLBACK_NAME, > FALLBACK_UIDGID, FALLBACK_UIDGID, > - seuname, prefix, "", level) != 0) > + seuname, prefix, "", level, > + FALLBACK_NAME) != 0) > errors = STATUS_ERR; > semanage_user_key_free(key); > if (u) > @@ -806,6 +844,201 @@ static int setup_fallback_user(genhomedircon_settings_t * s) > return errors; > } > > +static genhomedircon_user_entry_t *find_user(genhomedircon_user_entry_t *head, > + const char *name) > +{ > + for(; head; head = head->next) { > + if (strcmp(head->name, name) == 0) { > + return head; > + } > + } > + > + return NULL; > +} > + > +static int add_user(genhomedircon_settings_t * s, > + genhomedircon_user_entry_t **head, > + semanage_user_t *user, > + const char *name, > + const char *sename, > + const char *selogin) > +{ > + if (selogin[0] == '%') { > + genhomedircon_user_entry_t *orig = find_user(*head, name); > + if (orig != NULL && orig->login[0] == '%') { > + ERR(s->h_semanage, "User %s is already mapped to" > + "group %s, but also belongs to group %s. Add an" > + "explicit mapping for this user to" > + "override group mappings.", > + name, orig->login + 1, selogin + 1); > + return STATUS_ERR; > + } else if (orig != NULL) { > + // user mappings take precedence > + return STATUS_SUCCESS; > + } > + } > + > + int retval = STATUS_ERR; > + > + char *rbuf = NULL; > + long rbuflen; > + struct passwd pwstorage, *pwent = NULL; > + const char *prefix = NULL; > + const char *level = NULL; > + char uid[11]; > + char gid[11]; > + > + /* Allocate space for the getpwnam_r buffer */ > + rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); > + if (rbuflen <= 0) > + goto cleanup; > + rbuf = malloc(rbuflen); > + if (rbuf == NULL) > + goto cleanup; > + > + if (user) { > + prefix = semanage_user_get_prefix(user); > + level = semanage_user_get_mlslevel(user); > + > + if (!level) { > + level = FALLBACK_LEVEL; > + } > + } else { > + prefix = name; > + level = FALLBACK_LEVEL; > + } > + > + retval = getpwnam_r(name, &pwstorage, rbuf, rbuflen, &pwent); > + if (retval != 0 || pwent == NULL) { > + if (retval != 0 && retval != ENOENT) { > + goto cleanup; > + } > + > + WARN(s->h_semanage, > + "user %s not in password file", name); > + retval = STATUS_SUCCESS; > + goto cleanup; > + } > + > + int len = strlen(pwent->pw_dir) -1; > + for(; len > 0 && pwent->pw_dir[len] == '/'; len--) { > + pwent->pw_dir[len] = '\0'; > + } > + > + if (strcmp(pwent->pw_dir, "/") == 0) { > + /* don't relabel / genhomdircon checked to see if root > + * was the user and if so, set his home directory to > + * /root */ > + retval = STATUS_SUCCESS; > + goto cleanup; > + } > + > + if (ignore(pwent->pw_dir)) { > + retval = STATUS_SUCCESS; > + goto cleanup; > + } > + > + len = snprintf(uid, sizeof(uid), "%u", pwent->pw_uid); > + if (len < 0 || len >= (int)sizeof(uid)) { > + goto cleanup; > + } > + > + len = snprintf(gid, sizeof(gid), "%u", pwent->pw_gid); > + if (len < 0 || len >= (int)sizeof(gid)) { > + goto cleanup; > + } > + > + retval = push_user_entry(head, name, uid, gid, sename, prefix, > + pwent->pw_dir, level, selogin); > +cleanup: > + free(rbuf); > + return retval; > +} > + > +static int get_group_users(genhomedircon_settings_t * s, > + genhomedircon_user_entry_t **head, > + semanage_user_t *user, > + const char *sename, > + const char *selogin) > +{ > + int retval = STATUS_ERR; > + > + long grbuflen; > + char *grbuf = NULL; > + struct group grstorage, *group = NULL; > + > + long prbuflen; > + char *pwbuf = NULL; > + struct passwd pwstorage, *pw = NULL; > + > + grbuflen = sysconf(_SC_GETGR_R_SIZE_MAX); > + if (grbuflen <= 0) > + goto cleanup; > + grbuf = malloc(grbuflen); > + if (grbuf == NULL) > + goto cleanup; > + > + const char *grname = selogin + 1; > + > + if (getgrnam_r(grname, &grstorage, grbuf, > + (size_t) grbuflen, &group) != 0) { > + goto cleanup; > + } > + > + if (group == NULL) { > + ERR(s->h_semanage, "Can't find group named %s\n", grname); > + goto cleanup; > + } > + > + size_t nmembers = 0; > + char **members = group->gr_mem; > + > + while (*members != NULL) { > + nmembers++; > + members++; > + } > + > + for (unsigned int i = 0; i < nmembers; i++) { > + const char *uname = group->gr_mem[i]; > + > + if (add_user(s, head, user, uname, sename, selogin) < 0) { > + goto cleanup; > + } > + } > + > + prbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); > + if (prbuflen <= 0) > + goto cleanup; > + pwbuf = malloc(prbuflen); > + if (pwbuf == NULL) > + goto cleanup; > + > + setpwent(); > + while ((retval = getpwent_r(&pwstorage, pwbuf, prbuflen, &pw)) == 0) { > + // skip users who also have this group as their > + // primary group > + if (lfind(pw->pw_name, group->gr_mem, &nmembers, > + sizeof(char *), &STR_COMPARATOR)) { > + continue; > + } > + > + if (group->gr_gid == pw->pw_gid) { > + if (add_user(s, head, user, pw->pw_name, > + sename, selogin) < 0) { > + goto cleanup; > + } > + } > + } > + > + retval = STATUS_SUCCESS; > +cleanup: > + endpwent(); > + free(pwbuf); > + free(grbuf); > + > + return retval; > +} > + > static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s, > int *errors) > { > @@ -817,14 +1050,7 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s, > semanage_user_t **u = NULL; > const char *name = NULL; > const char *seuname = NULL; > - const char *prefix = NULL; > - const char *level = NULL; > - char uid[11]; > - char gid[11]; > - struct passwd pwstorage, *pwent = NULL; > unsigned int i; > - long rbuflen; > - char *rbuf = NULL; > int retval; > > *errors = 0; > @@ -838,94 +1064,43 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s, > nusers = 0; > } > > + qsort(seuser_list, nseusers, sizeof(semanage_seuser_t *), > + &seuser_sort_func); > qsort(user_list, nusers, sizeof(semanage_user_t *), > (int (*)(const void *, const void *))&user_sort_func); > > - /* Allocate space for the getpwnam_r buffer */ > - rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); > - if (rbuflen <= 0) > - goto cleanup; > - rbuf = malloc(rbuflen); > - if (rbuf == NULL) > - goto cleanup; > - > for (i = 0; i < nseusers; i++) { > seuname = semanage_seuser_get_sename(seuser_list[i]); > name = semanage_seuser_get_name(seuser_list[i]); > > - if (strcmp(name,"root") && strcmp(seuname, s->fallback->sename) == 0) > - continue; > - > if (strcmp(name, DEFAULT_LOGIN) == 0) > continue; > > if (strcmp(name, TEMPLATE_SEUSER) == 0) > continue; > > - /* %groupname syntax */ > - if (name[0] == '%') > - continue; > - > /* find the user structure given the name */ > - u = bsearch(seuname, user_list, nusers, sizeof(semanage_user_t *), > + u = bsearch(seuname, user_list, nusers, > + sizeof(semanage_user_t *), > (int (*)(const void *, const void *)) > &name_user_cmp); > - if (u) { > - prefix = semanage_user_get_prefix(*u); > - level = semanage_user_get_mlslevel(*u); > - if (!level) > - level = FALLBACK_LEVEL; > - } else { > - prefix = name; > - level = FALLBACK_LEVEL; > - } > - > - retval = getpwnam_r(name, &pwstorage, rbuf, rbuflen, &pwent); > - if (retval != 0 || pwent == NULL) { > - if (retval != 0 && retval != ENOENT) { > - *errors = STATUS_ERR; > - goto cleanup; > - } > - > - WARN(s->h_semanage, > - "user %s not in password file", name); > - continue; > - } > > - int len = strlen(pwent->pw_dir) -1; > - for(; len > 0 && pwent->pw_dir[len] == '/'; len--) { > - pwent->pw_dir[len] = '\0'; > + /* %groupname syntax */ > + if (name[0] == '%') { > + retval = get_group_users(s, &head, *u, seuname, > + name); > + } else { > + retval = add_user(s, &head, *u, name, > + seuname, name); > } > > - if (strcmp(pwent->pw_dir, "/") == 0) { > - /* don't relabel / genhomdircon checked to see if root > - * was the user and if so, set his home directory to > - * /root */ > - continue; > - } > - if (ignore(pwent->pw_dir)) > - continue; > - > - len = snprintf(uid, sizeof(uid), "%u", pwent->pw_uid); > - if (len < 0 || len >= (int)sizeof(uid)) { > + if (retval != 0) { > *errors = STATUS_ERR; > goto cleanup; > } > - len = snprintf(gid, sizeof(gid), "%u", pwent->pw_gid); > - if (len < 0 || len >= (int)sizeof(gid)) { > - *errors = STATUS_ERR; > - goto cleanup; > - } > - > - if (push_user_entry(&head, name, uid, gid, seuname, > - prefix, pwent->pw_dir, level) != STATUS_SUCCESS) { > - *errors = STATUS_ERR; > - break; > - } > } > > cleanup: > - free(rbuf); > if (*errors) { > for (; head; pop_user_entry(&head)) { > /* the pop function takes care of all the cleanup >
On 08/15/2016 02:32 PM, Dominick Grift wrote: > Any specific reason why this is being ignored? > It is not being ignored. We've had a bunch of patches recently and we haven't had a chance to test this one yet. Jim > On 07/27/2016 11:25 PM, Gary Tierney wrote: >> semanage-login supports login mappings using the %group syntax, but >> genhomedircon does not expand groups to the users belonging to them. >> >> This commit adds support for generating home directory contexts for login >> mappings using the group syntax and adds error reporting for handling cases >> where there is ambiguity due to a user belonging to multiple groups mapped by >> semanage-login. If a login mapping is added for the user which belongs to >> multiple groups it will take precedence and resolve the ambiguity issue. >> >> Signed-off-by: Gary Tierney <gary.tierney@gmx.com> >> --- >> libsemanage/src/genhomedircon.c | 319 +++++++++++++++++++++++++++++++--------- >> 1 file changed, 247 insertions(+), 72 deletions(-) >> >> diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c >> index c5ea436..2955b19 100644 >> --- a/libsemanage/src/genhomedircon.c >> +++ b/libsemanage/src/genhomedircon.c >> @@ -48,6 +48,8 @@ >> #include <errno.h> >> #include <unistd.h> >> #include <regex.h> >> +#include <grp.h> >> +#include <search.h> >> >> /* paths used in get_home_dirs() */ >> #define PATH_ETC_USERADD "/etc/default/useradd" >> @@ -98,6 +100,10 @@ typedef struct user_entry { >> char *prefix; >> char *home; >> char *level; >> + >> + // The login identifier that was used >> + // in semanage-login / seusers >> + char *login; >> struct user_entry *next; >> } genhomedircon_user_entry_t; >> >> @@ -486,6 +492,11 @@ static int USER_CONTEXT_PRED(const char *string) >> return (int)(strstr(string, TEMPLATE_USER) != NULL); >> } >> >> +static int STR_COMPARATOR(const void *a, const void *b) >> +{ >> + return strcmp((const char *) a, (const char *) b); >> +} >> + >> /* make_tempate >> * @param s the settings holding the paths to various files >> * @param pred function pointer to function to use as filter for slurp >> @@ -652,6 +663,24 @@ static int write_user_context(genhomedircon_settings_t * s, FILE * out, >> return write_replacements(s, out, tpl, repl); >> } >> >> +static int seuser_sort_func(const void *arg1, const void *arg2) >> +{ >> + const semanage_seuser_t **u1 = (const semanage_seuser_t **) arg1; >> + const semanage_seuser_t **u2 = (const semanage_seuser_t **) arg2;; >> + const char *name1 = semanage_seuser_get_name(*u1); >> + const char *name2 = semanage_seuser_get_name(*u2); >> + >> + if (name1[0] == '%' && name2[0] == '%') { >> + return 0; >> + } else if (name1[0] == '%') { >> + return 1; >> + } else if (name2[0] == '%') { >> + return -1; >> + } >> + >> + return strcmp(name1, name2); >> +} >> + >> static int user_sort_func(semanage_user_t ** arg1, semanage_user_t ** arg2) >> { >> return strcmp(semanage_user_get_name(*arg1), >> @@ -665,7 +694,8 @@ static int name_user_cmp(char *key, semanage_user_t ** val) >> >> static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, >> const char *u, const char *g, const char *sen, >> - const char *pre, const char *h, const char *l) >> + const char *pre, const char *h, const char *l, >> + const char *ln) >> { >> genhomedircon_user_entry_t *temp = NULL; >> char *name = NULL; >> @@ -675,6 +705,7 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, >> char *prefix = NULL; >> char *home = NULL; >> char *level = NULL; >> + char *lname = NULL; >> >> temp = malloc(sizeof(genhomedircon_user_entry_t)); >> if (!temp) >> @@ -700,6 +731,9 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, >> level = strdup(l); >> if (!level) >> goto cleanup; >> + lname = strdup(ln); >> + if (!lname) >> + goto cleanup; >> >> temp->name = name; >> temp->uid = uid; >> @@ -708,6 +742,7 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, >> temp->prefix = prefix; >> temp->home = home; >> temp->level = level; >> + temp->login = lname; >> temp->next = (*list); >> (*list) = temp; >> >> @@ -721,6 +756,7 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, >> free(prefix); >> free(home); >> free(level); >> + free(lname); >> free(temp); >> return STATUS_ERR; >> } >> @@ -741,6 +777,7 @@ static void pop_user_entry(genhomedircon_user_entry_t ** list) >> free(temp->prefix); >> free(temp->home); >> free(temp->level); >> + free(temp->login); >> free(temp); >> } >> >> @@ -790,7 +827,8 @@ static int setup_fallback_user(genhomedircon_settings_t * s) >> >> if (push_user_entry(&(s->fallback), FALLBACK_NAME, >> FALLBACK_UIDGID, FALLBACK_UIDGID, >> - seuname, prefix, "", level) != 0) >> + seuname, prefix, "", level, >> + FALLBACK_NAME) != 0) >> errors = STATUS_ERR; >> semanage_user_key_free(key); >> if (u) >> @@ -806,6 +844,201 @@ static int setup_fallback_user(genhomedircon_settings_t * s) >> return errors; >> } >> >> +static genhomedircon_user_entry_t *find_user(genhomedircon_user_entry_t *head, >> + const char *name) >> +{ >> + for(; head; head = head->next) { >> + if (strcmp(head->name, name) == 0) { >> + return head; >> + } >> + } >> + >> + return NULL; >> +} >> + >> +static int add_user(genhomedircon_settings_t * s, >> + genhomedircon_user_entry_t **head, >> + semanage_user_t *user, >> + const char *name, >> + const char *sename, >> + const char *selogin) >> +{ >> + if (selogin[0] == '%') { >> + genhomedircon_user_entry_t *orig = find_user(*head, name); >> + if (orig != NULL && orig->login[0] == '%') { >> + ERR(s->h_semanage, "User %s is already mapped to" >> + "group %s, but also belongs to group %s. Add an" >> + "explicit mapping for this user to" >> + "override group mappings.", >> + name, orig->login + 1, selogin + 1); >> + return STATUS_ERR; >> + } else if (orig != NULL) { >> + // user mappings take precedence >> + return STATUS_SUCCESS; >> + } >> + } >> + >> + int retval = STATUS_ERR; >> + >> + char *rbuf = NULL; >> + long rbuflen; >> + struct passwd pwstorage, *pwent = NULL; >> + const char *prefix = NULL; >> + const char *level = NULL; >> + char uid[11]; >> + char gid[11]; >> + >> + /* Allocate space for the getpwnam_r buffer */ >> + rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); >> + if (rbuflen <= 0) >> + goto cleanup; >> + rbuf = malloc(rbuflen); >> + if (rbuf == NULL) >> + goto cleanup; >> + >> + if (user) { >> + prefix = semanage_user_get_prefix(user); >> + level = semanage_user_get_mlslevel(user); >> + >> + if (!level) { >> + level = FALLBACK_LEVEL; >> + } >> + } else { >> + prefix = name; >> + level = FALLBACK_LEVEL; >> + } >> + >> + retval = getpwnam_r(name, &pwstorage, rbuf, rbuflen, &pwent); >> + if (retval != 0 || pwent == NULL) { >> + if (retval != 0 && retval != ENOENT) { >> + goto cleanup; >> + } >> + >> + WARN(s->h_semanage, >> + "user %s not in password file", name); >> + retval = STATUS_SUCCESS; >> + goto cleanup; >> + } >> + >> + int len = strlen(pwent->pw_dir) -1; >> + for(; len > 0 && pwent->pw_dir[len] == '/'; len--) { >> + pwent->pw_dir[len] = '\0'; >> + } >> + >> + if (strcmp(pwent->pw_dir, "/") == 0) { >> + /* don't relabel / genhomdircon checked to see if root >> + * was the user and if so, set his home directory to >> + * /root */ >> + retval = STATUS_SUCCESS; >> + goto cleanup; >> + } >> + >> + if (ignore(pwent->pw_dir)) { >> + retval = STATUS_SUCCESS; >> + goto cleanup; >> + } >> + >> + len = snprintf(uid, sizeof(uid), "%u", pwent->pw_uid); >> + if (len < 0 || len >= (int)sizeof(uid)) { >> + goto cleanup; >> + } >> + >> + len = snprintf(gid, sizeof(gid), "%u", pwent->pw_gid); >> + if (len < 0 || len >= (int)sizeof(gid)) { >> + goto cleanup; >> + } >> + >> + retval = push_user_entry(head, name, uid, gid, sename, prefix, >> + pwent->pw_dir, level, selogin); >> +cleanup: >> + free(rbuf); >> + return retval; >> +} >> + >> +static int get_group_users(genhomedircon_settings_t * s, >> + genhomedircon_user_entry_t **head, >> + semanage_user_t *user, >> + const char *sename, >> + const char *selogin) >> +{ >> + int retval = STATUS_ERR; >> + >> + long grbuflen; >> + char *grbuf = NULL; >> + struct group grstorage, *group = NULL; >> + >> + long prbuflen; >> + char *pwbuf = NULL; >> + struct passwd pwstorage, *pw = NULL; >> + >> + grbuflen = sysconf(_SC_GETGR_R_SIZE_MAX); >> + if (grbuflen <= 0) >> + goto cleanup; >> + grbuf = malloc(grbuflen); >> + if (grbuf == NULL) >> + goto cleanup; >> + >> + const char *grname = selogin + 1; >> + >> + if (getgrnam_r(grname, &grstorage, grbuf, >> + (size_t) grbuflen, &group) != 0) { >> + goto cleanup; >> + } >> + >> + if (group == NULL) { >> + ERR(s->h_semanage, "Can't find group named %s\n", grname); >> + goto cleanup; >> + } >> + >> + size_t nmembers = 0; >> + char **members = group->gr_mem; >> + >> + while (*members != NULL) { >> + nmembers++; >> + members++; >> + } >> + >> + for (unsigned int i = 0; i < nmembers; i++) { >> + const char *uname = group->gr_mem[i]; >> + >> + if (add_user(s, head, user, uname, sename, selogin) < 0) { >> + goto cleanup; >> + } >> + } >> + >> + prbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); >> + if (prbuflen <= 0) >> + goto cleanup; >> + pwbuf = malloc(prbuflen); >> + if (pwbuf == NULL) >> + goto cleanup; >> + >> + setpwent(); >> + while ((retval = getpwent_r(&pwstorage, pwbuf, prbuflen, &pw)) == 0) { >> + // skip users who also have this group as their >> + // primary group >> + if (lfind(pw->pw_name, group->gr_mem, &nmembers, >> + sizeof(char *), &STR_COMPARATOR)) { >> + continue; >> + } >> + >> + if (group->gr_gid == pw->pw_gid) { >> + if (add_user(s, head, user, pw->pw_name, >> + sename, selogin) < 0) { >> + goto cleanup; >> + } >> + } >> + } >> + >> + retval = STATUS_SUCCESS; >> +cleanup: >> + endpwent(); >> + free(pwbuf); >> + free(grbuf); >> + >> + return retval; >> +} >> + >> static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s, >> int *errors) >> { >> @@ -817,14 +1050,7 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s, >> semanage_user_t **u = NULL; >> const char *name = NULL; >> const char *seuname = NULL; >> - const char *prefix = NULL; >> - const char *level = NULL; >> - char uid[11]; >> - char gid[11]; >> - struct passwd pwstorage, *pwent = NULL; >> unsigned int i; >> - long rbuflen; >> - char *rbuf = NULL; >> int retval; >> >> *errors = 0; >> @@ -838,94 +1064,43 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s, >> nusers = 0; >> } >> >> + qsort(seuser_list, nseusers, sizeof(semanage_seuser_t *), >> + &seuser_sort_func); >> qsort(user_list, nusers, sizeof(semanage_user_t *), >> (int (*)(const void *, const void *))&user_sort_func); >> >> - /* Allocate space for the getpwnam_r buffer */ >> - rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); >> - if (rbuflen <= 0) >> - goto cleanup; >> - rbuf = malloc(rbuflen); >> - if (rbuf == NULL) >> - goto cleanup; >> - >> for (i = 0; i < nseusers; i++) { >> seuname = semanage_seuser_get_sename(seuser_list[i]); >> name = semanage_seuser_get_name(seuser_list[i]); >> >> - if (strcmp(name,"root") && strcmp(seuname, s->fallback->sename) == 0) >> - continue; >> - >> if (strcmp(name, DEFAULT_LOGIN) == 0) >> continue; >> >> if (strcmp(name, TEMPLATE_SEUSER) == 0) >> continue; >> >> - /* %groupname syntax */ >> - if (name[0] == '%') >> - continue; >> - >> /* find the user structure given the name */ >> - u = bsearch(seuname, user_list, nusers, sizeof(semanage_user_t *), >> + u = bsearch(seuname, user_list, nusers, >> + sizeof(semanage_user_t *), >> (int (*)(const void *, const void *)) >> &name_user_cmp); >> - if (u) { >> - prefix = semanage_user_get_prefix(*u); >> - level = semanage_user_get_mlslevel(*u); >> - if (!level) >> - level = FALLBACK_LEVEL; >> - } else { >> - prefix = name; >> - level = FALLBACK_LEVEL; >> - } >> - >> - retval = getpwnam_r(name, &pwstorage, rbuf, rbuflen, &pwent); >> - if (retval != 0 || pwent == NULL) { >> - if (retval != 0 && retval != ENOENT) { >> - *errors = STATUS_ERR; >> - goto cleanup; >> - } >> - >> - WARN(s->h_semanage, >> - "user %s not in password file", name); >> - continue; >> - } >> >> - int len = strlen(pwent->pw_dir) -1; >> - for(; len > 0 && pwent->pw_dir[len] == '/'; len--) { >> - pwent->pw_dir[len] = '\0'; >> + /* %groupname syntax */ >> + if (name[0] == '%') { >> + retval = get_group_users(s, &head, *u, seuname, >> + name); >> + } else { >> + retval = add_user(s, &head, *u, name, >> + seuname, name); >> } >> >> - if (strcmp(pwent->pw_dir, "/") == 0) { >> - /* don't relabel / genhomdircon checked to see if root >> - * was the user and if so, set his home directory to >> - * /root */ >> - continue; >> - } >> - if (ignore(pwent->pw_dir)) >> - continue; >> - >> - len = snprintf(uid, sizeof(uid), "%u", pwent->pw_uid); >> - if (len < 0 || len >= (int)sizeof(uid)) { >> + if (retval != 0) { >> *errors = STATUS_ERR; >> goto cleanup; >> } >> - len = snprintf(gid, sizeof(gid), "%u", pwent->pw_gid); >> - if (len < 0 || len >= (int)sizeof(gid)) { >> - *errors = STATUS_ERR; >> - goto cleanup; >> - } >> - >> - if (push_user_entry(&head, name, uid, gid, seuname, >> - prefix, pwent->pw_dir, level) != STATUS_SUCCESS) { >> - *errors = STATUS_ERR; >> - break; >> - } >> } >> >> cleanup: >> - free(rbuf); >> if (*errors) { >> for (; head; pop_user_entry(&head)) { >> /* the pop function takes care of all the cleanup >> > > > > > _______________________________________________ > Selinux mailing list > Selinux@tycho.nsa.gov > To unsubscribe, send email to Selinux-leave@tycho.nsa.gov. > To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov. >
On 07/27/2016 05:25 PM, Gary Tierney wrote: > semanage-login supports login mappings using the %group syntax, but > genhomedircon does not expand groups to the users belonging to them. > > This commit adds support for generating home directory contexts for login > mappings using the group syntax and adds error reporting for handling cases > where there is ambiguity due to a user belonging to multiple groups mapped by > semanage-login. If a login mapping is added for the user which belongs to > multiple groups it will take precedence and resolve the ambiguity issue. Sorry for the long delay in responding. One question/comment below. > Signed-off-by: Gary Tierney <gary.tierney@gmx.com> > --- > libsemanage/src/genhomedircon.c | 319 +++++++++++++++++++++++++++++++--------- > 1 file changed, 247 insertions(+), 72 deletions(-) > > diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c > index c5ea436..2955b19 100644 > --- a/libsemanage/src/genhomedircon.c > +++ b/libsemanage/src/genhomedircon.c > @@ -838,94 +1064,43 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s, > nusers = 0; > } > > + qsort(seuser_list, nseusers, sizeof(semanage_seuser_t *), > + &seuser_sort_func); > qsort(user_list, nusers, sizeof(semanage_user_t *), > (int (*)(const void *, const void *))&user_sort_func); > > - /* Allocate space for the getpwnam_r buffer */ > - rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); > - if (rbuflen <= 0) > - goto cleanup; > - rbuf = malloc(rbuflen); > - if (rbuf == NULL) > - goto cleanup; > - > for (i = 0; i < nseusers; i++) { > seuname = semanage_seuser_get_sename(seuser_list[i]); > name = semanage_seuser_get_name(seuser_list[i]); > > - if (strcmp(name,"root") && strcmp(seuname, s->fallback->sename) == 0) > - continue; > - This appears to change the behavior of genhomedircon in general, not just with respect to %group handling. Was this intentional? I'm not necessarily opposed to this change, but I am unclear on the implications. It seems that previously genhomedircon would not generate file_contexts.homedirs entries for users who were mapped to the fallback seuser, with an exception for root for /root labeling. With this change, they will have entries added. > if (strcmp(name, DEFAULT_LOGIN) == 0) > continue; > > if (strcmp(name, TEMPLATE_SEUSER) == 0) > continue; > > - /* %groupname syntax */ > - if (name[0] == '%') > - continue; > - > /* find the user structure given the name */ > - u = bsearch(seuname, user_list, nusers, sizeof(semanage_user_t *), > + u = bsearch(seuname, user_list, nusers, > + sizeof(semanage_user_t *), > (int (*)(const void *, const void *)) > &name_user_cmp); > - if (u) { > - prefix = semanage_user_get_prefix(*u); > - level = semanage_user_get_mlslevel(*u); > - if (!level) > - level = FALLBACK_LEVEL; > - } else { > - prefix = name; > - level = FALLBACK_LEVEL; > - } > - > - retval = getpwnam_r(name, &pwstorage, rbuf, rbuflen, &pwent); > - if (retval != 0 || pwent == NULL) { > - if (retval != 0 && retval != ENOENT) { > - *errors = STATUS_ERR; > - goto cleanup; > - } > - > - WARN(s->h_semanage, > - "user %s not in password file", name); > - continue; > - } > > - int len = strlen(pwent->pw_dir) -1; > - for(; len > 0 && pwent->pw_dir[len] == '/'; len--) { > - pwent->pw_dir[len] = '\0'; > + /* %groupname syntax */ > + if (name[0] == '%') { > + retval = get_group_users(s, &head, *u, seuname, > + name); > + } else { > + retval = add_user(s, &head, *u, name, > + seuname, name); > } > > - if (strcmp(pwent->pw_dir, "/") == 0) { > - /* don't relabel / genhomdircon checked to see if root > - * was the user and if so, set his home directory to > - * /root */ > - continue; > - } > - if (ignore(pwent->pw_dir)) > - continue; > - > - len = snprintf(uid, sizeof(uid), "%u", pwent->pw_uid); > - if (len < 0 || len >= (int)sizeof(uid)) { > + if (retval != 0) { > *errors = STATUS_ERR; > goto cleanup; > } > - len = snprintf(gid, sizeof(gid), "%u", pwent->pw_gid); > - if (len < 0 || len >= (int)sizeof(gid)) { > - *errors = STATUS_ERR; > - goto cleanup; > - } > - > - if (push_user_entry(&head, name, uid, gid, seuname, > - prefix, pwent->pw_dir, level) != STATUS_SUCCESS) { > - *errors = STATUS_ERR; > - break; > - } > } > > cleanup: > - free(rbuf); > if (*errors) { > for (; head; pop_user_entry(&head)) { > /* the pop function takes care of all the cleanup >
On Mon, Aug 15, 2016 at 03:58:44PM -0400, Stephen Smalley wrote: > On 07/27/2016 05:25 PM, Gary Tierney wrote: > > semanage-login supports login mappings using the %group syntax, but > > genhomedircon does not expand groups to the users belonging to them. > > > > This commit adds support for generating home directory contexts for login > > mappings using the group syntax and adds error reporting for handling cases > > where there is ambiguity due to a user belonging to multiple groups mapped by > > semanage-login. If a login mapping is added for the user which belongs to > > multiple groups it will take precedence and resolve the ambiguity issue. > > Sorry for the long delay in responding. One question/comment below. > > > Signed-off-by: Gary Tierney <gary.tierney@gmx.com> > > --- > > libsemanage/src/genhomedircon.c | 319 +++++++++++++++++++++++++++++++--------- > > 1 file changed, 247 insertions(+), 72 deletions(-) > > > > diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c > > index c5ea436..2955b19 100644 > > --- a/libsemanage/src/genhomedircon.c > > +++ b/libsemanage/src/genhomedircon.c > > @@ -838,94 +1064,43 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s, > > nusers = 0; > > } > > > > + qsort(seuser_list, nseusers, sizeof(semanage_seuser_t *), > > + &seuser_sort_func); > > qsort(user_list, nusers, sizeof(semanage_user_t *), > > (int (*)(const void *, const void *))&user_sort_func); > > > > - /* Allocate space for the getpwnam_r buffer */ > > - rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); > > - if (rbuflen <= 0) > > - goto cleanup; > > - rbuf = malloc(rbuflen); > > - if (rbuf == NULL) > > - goto cleanup; > > - > > for (i = 0; i < nseusers; i++) { > > seuname = semanage_seuser_get_sename(seuser_list[i]); > > name = semanage_seuser_get_name(seuser_list[i]); > > > > - if (strcmp(name,"root") && strcmp(seuname, s->fallback->sename) == 0) > > - continue; > > - > > This appears to change the behavior of genhomedircon in general, not > just with respect to %group handling. Was this intentional? > I'm not necessarily opposed to this change, but I am unclear on the > implications. It seems that previously genhomedircon would not generate > file_contexts.homedirs entries for users who were mapped to the fallback > seuser, with an exception for root for /root labeling. With this > change, they will have entries added. I just realized, this might actually be required. I dont remember if I checked what happens to %{USERID} or %{USERNAME} entries if they map to the fallback. Are fcontexts with those entries done anyway or are they skipped? cuz skipped would be bad. -- Jason > > if (strcmp(name, DEFAULT_LOGIN) == 0) > > continue; > > > > if (strcmp(name, TEMPLATE_SEUSER) == 0) > > continue; > > > > - /* %groupname syntax */ > > - if (name[0] == '%') > > - continue; > > - > > /* find the user structure given the name */ > > - u = bsearch(seuname, user_list, nusers, sizeof(semanage_user_t *), > > + u = bsearch(seuname, user_list, nusers, > > + sizeof(semanage_user_t *), > > (int (*)(const void *, const void *)) > > &name_user_cmp); > > - if (u) { > > - prefix = semanage_user_get_prefix(*u); > > - level = semanage_user_get_mlslevel(*u); > > - if (!level) > > - level = FALLBACK_LEVEL; > > - } else { > > - prefix = name; > > - level = FALLBACK_LEVEL; > > - } > > - > > - retval = getpwnam_r(name, &pwstorage, rbuf, rbuflen, &pwent); > > - if (retval != 0 || pwent == NULL) { > > - if (retval != 0 && retval != ENOENT) { > > - *errors = STATUS_ERR; > > - goto cleanup; > > - } > > - > > - WARN(s->h_semanage, > > - "user %s not in password file", name); > > - continue; > > - } > > > > - int len = strlen(pwent->pw_dir) -1; > > - for(; len > 0 && pwent->pw_dir[len] == '/'; len--) { > > - pwent->pw_dir[len] = '\0'; > > + /* %groupname syntax */ > > + if (name[0] == '%') { > > + retval = get_group_users(s, &head, *u, seuname, > > + name); > > + } else { > > + retval = add_user(s, &head, *u, name, > > + seuname, name); > > } > > > > - if (strcmp(pwent->pw_dir, "/") == 0) { > > - /* don't relabel / genhomdircon checked to see if root > > - * was the user and if so, set his home directory to > > - * /root */ > > - continue; > > - } > > - if (ignore(pwent->pw_dir)) > > - continue; > > - > > - len = snprintf(uid, sizeof(uid), "%u", pwent->pw_uid); > > - if (len < 0 || len >= (int)sizeof(uid)) { > > + if (retval != 0) { > > *errors = STATUS_ERR; > > goto cleanup; > > } > > - len = snprintf(gid, sizeof(gid), "%u", pwent->pw_gid); > > - if (len < 0 || len >= (int)sizeof(gid)) { > > - *errors = STATUS_ERR; > > - goto cleanup; > > - } > > - > > - if (push_user_entry(&head, name, uid, gid, seuname, > > - prefix, pwent->pw_dir, level) != STATUS_SUCCESS) { > > - *errors = STATUS_ERR; > > - break; > > - } > > } > > > > cleanup: > > - free(rbuf); > > if (*errors) { > > for (; head; pop_user_entry(&head)) { > > /* the pop function takes care of all the cleanup > > > > _______________________________________________ > Selinux mailing list > Selinux@tycho.nsa.gov > To unsubscribe, send email to Selinux-leave@tycho.nsa.gov. > To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
On Tue, Aug 16, 2016 at 09:16:24PM +0800, Jason Zaman wrote: > On Mon, Aug 15, 2016 at 03:58:44PM -0400, Stephen Smalley wrote: > > On 07/27/2016 05:25 PM, Gary Tierney wrote: > > > semanage-login supports login mappings using the %group syntax, but > > > genhomedircon does not expand groups to the users belonging to them. > > > > > > This commit adds support for generating home directory contexts for login > > > mappings using the group syntax and adds error reporting for handling cases > > > where there is ambiguity due to a user belonging to multiple groups mapped by > > > semanage-login. If a login mapping is added for the user which belongs to > > > multiple groups it will take precedence and resolve the ambiguity issue. > > > > Sorry for the long delay in responding. One question/comment below. > > > > > Signed-off-by: Gary Tierney <gary.tierney@gmx.com> > > > --- > > > libsemanage/src/genhomedircon.c | 319 +++++++++++++++++++++++++++++++--------- > > > 1 file changed, 247 insertions(+), 72 deletions(-) > > > > > > diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c > > > index c5ea436..2955b19 100644 > > > --- a/libsemanage/src/genhomedircon.c > > > +++ b/libsemanage/src/genhomedircon.c > > > @@ -838,94 +1064,43 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s, > > > nusers = 0; > > > } > > > > > > + qsort(seuser_list, nseusers, sizeof(semanage_seuser_t *), > > > + &seuser_sort_func); > > > qsort(user_list, nusers, sizeof(semanage_user_t *), > > > (int (*)(const void *, const void *))&user_sort_func); > > > > > > - /* Allocate space for the getpwnam_r buffer */ > > > - rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); > > > - if (rbuflen <= 0) > > > - goto cleanup; > > > - rbuf = malloc(rbuflen); > > > - if (rbuf == NULL) > > > - goto cleanup; > > > - > > > for (i = 0; i < nseusers; i++) { > > > seuname = semanage_seuser_get_sename(seuser_list[i]); > > > name = semanage_seuser_get_name(seuser_list[i]); > > > > > > - if (strcmp(name,"root") && strcmp(seuname, s->fallback->sename) == 0) > > > - continue; > > > - > > > > This appears to change the behavior of genhomedircon in general, not > > just with respect to %group handling. Was this intentional? > > I'm not necessarily opposed to this change, but I am unclear on the > > implications. It seems that previously genhomedircon would not generate > > file_contexts.homedirs entries for users who were mapped to the fallback > > seuser, with an exception for root for /root labeling. With this > > change, they will have entries added. > > I just realized, this might actually be required. I dont remember if I > checked what happens to %{USERID} or %{USERNAME} entries if they map to > the fallback. Are fcontexts with those entries done anyway or are they > skipped? cuz skipped would be bad. Actually no %{USERNAME} and %{USERID} are fine, they map to [^/]+ and [0-9]+ respectively in the fallback so there should be no problems there. But as far as I can tell having duplicate entries should not cause any problems and i'd much rather have %group stuff working. ideally tho, no dups would be preferable. I will also test this patch, been meaning to but was investigating other issues and forgot. -- Jason > > > > if (strcmp(name, DEFAULT_LOGIN) == 0) > > > continue; > > > > > > if (strcmp(name, TEMPLATE_SEUSER) == 0) > > > continue; > > > > > > - /* %groupname syntax */ > > > - if (name[0] == '%') > > > - continue; > > > - > > > /* find the user structure given the name */ > > > - u = bsearch(seuname, user_list, nusers, sizeof(semanage_user_t *), > > > + u = bsearch(seuname, user_list, nusers, > > > + sizeof(semanage_user_t *), > > > (int (*)(const void *, const void *)) > > > &name_user_cmp); > > > - if (u) { > > > - prefix = semanage_user_get_prefix(*u); > > > - level = semanage_user_get_mlslevel(*u); > > > - if (!level) > > > - level = FALLBACK_LEVEL; > > > - } else { > > > - prefix = name; > > > - level = FALLBACK_LEVEL; > > > - } > > > - > > > - retval = getpwnam_r(name, &pwstorage, rbuf, rbuflen, &pwent); > > > - if (retval != 0 || pwent == NULL) { > > > - if (retval != 0 && retval != ENOENT) { > > > - *errors = STATUS_ERR; > > > - goto cleanup; > > > - } > > > - > > > - WARN(s->h_semanage, > > > - "user %s not in password file", name); > > > - continue; > > > - } > > > > > > - int len = strlen(pwent->pw_dir) -1; > > > - for(; len > 0 && pwent->pw_dir[len] == '/'; len--) { > > > - pwent->pw_dir[len] = '\0'; > > > + /* %groupname syntax */ > > > + if (name[0] == '%') { > > > + retval = get_group_users(s, &head, *u, seuname, > > > + name); > > > + } else { > > > + retval = add_user(s, &head, *u, name, > > > + seuname, name); > > > } > > > > > > - if (strcmp(pwent->pw_dir, "/") == 0) { > > > - /* don't relabel / genhomdircon checked to see if root > > > - * was the user and if so, set his home directory to > > > - * /root */ > > > - continue; > > > - } > > > - if (ignore(pwent->pw_dir)) > > > - continue; > > > - > > > - len = snprintf(uid, sizeof(uid), "%u", pwent->pw_uid); > > > - if (len < 0 || len >= (int)sizeof(uid)) { > > > + if (retval != 0) { > > > *errors = STATUS_ERR; > > > goto cleanup; > > > } > > > - len = snprintf(gid, sizeof(gid), "%u", pwent->pw_gid); > > > - if (len < 0 || len >= (int)sizeof(gid)) { > > > - *errors = STATUS_ERR; > > > - goto cleanup; > > > - } > > > - > > > - if (push_user_entry(&head, name, uid, gid, seuname, > > > - prefix, pwent->pw_dir, level) != STATUS_SUCCESS) { > > > - *errors = STATUS_ERR; > > > - break; > > > - } > > > } > > > > > > cleanup: > > > - free(rbuf); > > > if (*errors) { > > > for (; head; pop_user_entry(&head)) { > > > /* the pop function takes care of all the cleanup > > > > > > > _______________________________________________ > > Selinux mailing list > > Selinux@tycho.nsa.gov > > To unsubscribe, send email to Selinux-leave@tycho.nsa.gov. > > To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
Hi Stephen, Replied inline below. On Mon, Aug 15, 2016 at 03:58:44PM -0400, Stephen Smalley wrote: > On 07/27/2016 05:25 PM, Gary Tierney wrote: > > semanage-login supports login mappings using the %group syntax, but > > genhomedircon does not expand groups to the users belonging to them. > > > > This commit adds support for generating home directory contexts for login > > mappings using the group syntax and adds error reporting for handling cases > > where there is ambiguity due to a user belonging to multiple groups mapped by > > semanage-login. If a login mapping is added for the user which belongs to > > multiple groups it will take precedence and resolve the ambiguity issue. > > Sorry for the long delay in responding. One question/comment below. > > > Signed-off-by: Gary Tierney <gary.tierney@gmx.com> > > --- > > libsemanage/src/genhomedircon.c | 319 +++++++++++++++++++++++++++++++--------- > > 1 file changed, 247 insertions(+), 72 deletions(-) > > > > diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c > > index c5ea436..2955b19 100644 > > --- a/libsemanage/src/genhomedircon.c > > +++ b/libsemanage/src/genhomedircon.c > > @@ -838,94 +1064,43 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s, > > nusers = 0; > > } > > > > + qsort(seuser_list, nseusers, sizeof(semanage_seuser_t *), > > + &seuser_sort_func); > > qsort(user_list, nusers, sizeof(semanage_user_t *), > > (int (*)(const void *, const void *))&user_sort_func); > > > > - /* Allocate space for the getpwnam_r buffer */ > > - rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); > > - if (rbuflen <= 0) > > - goto cleanup; > > - rbuf = malloc(rbuflen); > > - if (rbuf == NULL) > > - goto cleanup; > > - > > for (i = 0; i < nseusers; i++) { > > seuname = semanage_seuser_get_sename(seuser_list[i]); > > name = semanage_seuser_get_name(seuser_list[i]); > > > > - if (strcmp(name,"root") && strcmp(seuname, s->fallback->sename) == 0) > > - continue; > > - > > This appears to change the behavior of genhomedircon in general, not > just with respect to %group handling. Was this intentional? > I'm not necessarily opposed to this change, but I am unclear on the > implications. It seems that previously genhomedircon would not generate > file_contexts.homedirs entries for users who were mapped to the fallback > seuser, with an exception for root for /root labeling. With this > change, they will have entries added. > Hmm, yes, you're right. This is a mistake. We do need to skip this conditional however, so we can check if a user already has a mapping when we're expanding the members of a group. To prevent this from happening it should suffice to check if the user that we're writing contexts for shares an sename with the __default__ mapping, and if so, simply skip writing contexts for that user. We can do this check inside write_gen_home_dir_contexts() like so: https://github.com/garyttierney/selinux/commit/ebdb7f225ddbd5f311b2db75f68e2896285a5090#diff-b298746a257be78548f69d5d296dcd09R1140 An alternative fix would be to prevent these users from being added to the `genhomedircon_user_entry_t` stack in the first place, and have get_group_users() search the seusers list for a login mapping which matches the group members username, skipping the group member if a match is found. Any thoughts on either of these solutions? > > if (strcmp(name, DEFAULT_LOGIN) == 0) > > continue; > > > > if (strcmp(name, TEMPLATE_SEUSER) == 0) > > continue; > > > > - /* %groupname syntax */ > > - if (name[0] == '%') > > - continue; > > - > > /* find the user structure given the name */ > > - u = bsearch(seuname, user_list, nusers, sizeof(semanage_user_t *), > > + u = bsearch(seuname, user_list, nusers, > > + sizeof(semanage_user_t *), > > (int (*)(const void *, const void *)) > > &name_user_cmp); > > - if (u) { > > - prefix = semanage_user_get_prefix(*u); > > - level = semanage_user_get_mlslevel(*u); > > - if (!level) > > - level = FALLBACK_LEVEL; > > - } else { > > - prefix = name; > > - level = FALLBACK_LEVEL; > > - } > > - > > - retval = getpwnam_r(name, &pwstorage, rbuf, rbuflen, &pwent); > > - if (retval != 0 || pwent == NULL) { > > - if (retval != 0 && retval != ENOENT) { > > - *errors = STATUS_ERR; > > - goto cleanup; > > - } > > - > > - WARN(s->h_semanage, > > - "user %s not in password file", name); > > - continue; > > - } > > > > - int len = strlen(pwent->pw_dir) -1; > > - for(; len > 0 && pwent->pw_dir[len] == '/'; len--) { > > - pwent->pw_dir[len] = '\0'; > > + /* %groupname syntax */ > > + if (name[0] == '%') { > > + retval = get_group_users(s, &head, *u, seuname, > > + name); > > + } else { > > + retval = add_user(s, &head, *u, name, > > + seuname, name); > > } > > > > - if (strcmp(pwent->pw_dir, "/") == 0) { > > - /* don't relabel / genhomdircon checked to see if root > > - * was the user and if so, set his home directory to > > - * /root */ > > - continue; > > - } > > - if (ignore(pwent->pw_dir)) > > - continue; > > - > > - len = snprintf(uid, sizeof(uid), "%u", pwent->pw_uid); > > - if (len < 0 || len >= (int)sizeof(uid)) { > > + if (retval != 0) { > > *errors = STATUS_ERR; > > goto cleanup; > > } > > - len = snprintf(gid, sizeof(gid), "%u", pwent->pw_gid); > > - if (len < 0 || len >= (int)sizeof(gid)) { > > - *errors = STATUS_ERR; > > - goto cleanup; > > - } > > - > > - if (push_user_entry(&head, name, uid, gid, seuname, > > - prefix, pwent->pw_dir, level) != STATUS_SUCCESS) { > > - *errors = STATUS_ERR; > > - break; > > - } > > } > > > > cleanup: > > - free(rbuf); > > if (*errors) { > > for (; head; pop_user_entry(&head)) { > > /* the pop function takes care of all the cleanup > > >
On 08/16/2016 11:59 AM, Gary Tierney wrote: > Hi Stephen, > > Replied inline below. > > On Mon, Aug 15, 2016 at 03:58:44PM -0400, Stephen Smalley wrote: >> On 07/27/2016 05:25 PM, Gary Tierney wrote: >>> semanage-login supports login mappings using the %group syntax, >>> but genhomedircon does not expand groups to the users belonging >>> to them. >>> >>> This commit adds support for generating home directory contexts >>> for login mappings using the group syntax and adds error >>> reporting for handling cases where there is ambiguity due to a >>> user belonging to multiple groups mapped by semanage-login. If >>> a login mapping is added for the user which belongs to multiple >>> groups it will take precedence and resolve the ambiguity >>> issue. >> >> Sorry for the long delay in responding. One question/comment >> below. >> >>> Signed-off-by: Gary Tierney <gary.tierney@gmx.com> --- >>> libsemanage/src/genhomedircon.c | 319 >>> +++++++++++++++++++++++++++++++--------- 1 file changed, 247 >>> insertions(+), 72 deletions(-) >>> >>> diff --git a/libsemanage/src/genhomedircon.c >>> b/libsemanage/src/genhomedircon.c index c5ea436..2955b19 >>> 100644 --- a/libsemanage/src/genhomedircon.c +++ >>> b/libsemanage/src/genhomedircon.c @@ -838,94 +1064,43 @@ static >>> genhomedircon_user_entry_t *get_users(genhomedircon_settings_t >>> * s, nusers = 0; } >>> >>> + qsort(seuser_list, nseusers, sizeof(semanage_seuser_t *), + >>> &seuser_sort_func); qsort(user_list, nusers, >>> sizeof(semanage_user_t *), (int (*)(const void *, const void >>> *))&user_sort_func); >>> >>> - /* Allocate space for the getpwnam_r buffer */ - rbuflen = >>> sysconf(_SC_GETPW_R_SIZE_MAX); - if (rbuflen <= 0) - goto >>> cleanup; - rbuf = malloc(rbuflen); - if (rbuf == NULL) - goto >>> cleanup; - for (i = 0; i < nseusers; i++) { seuname = >>> semanage_seuser_get_sename(seuser_list[i]); name = >>> semanage_seuser_get_name(seuser_list[i]); >>> >>> - if (strcmp(name,"root") && strcmp(seuname, >>> s->fallback->sename) == 0) - continue; - >> >> This appears to change the behavior of genhomedircon in general, >> not just with respect to %group handling. Was this intentional? >> I'm not necessarily opposed to this change, but I am unclear on >> the implications. It seems that previously genhomedircon would >> not generate file_contexts.homedirs entries for users who were >> mapped to the fallback seuser, with an exception for root for >> /root labeling. With this change, they will have entries added. >> > > Hmm, yes, you're right. This is a mistake. We do need to skip this > conditional however, so we can check if a user already has a > mapping when we're expanding the members of a group. > > To prevent this from happening it should suffice to check if the > user that we're writing contexts for shares an sename with the > __default__ mapping, and if so, simply skip writing contexts for > that user. We can do this check inside > write_gen_home_dir_contexts() like so: > > https://github.com/garyttierney/selinux/commit/ebdb7f225ddbd5f311b2db75f68e2896285a5090#diff-b298746a257be78548f69d5d296dcd09R1140 > > An alternative fix would be to prevent these users from being > added to the `genhomedircon_user_entry_t` stack in the first place, > and have get_group_users() search the seusers list for a login > mapping which matches the group members username, skipping the > group member if a match is found. > > Any thoughts on either of these solutions? I'm actually inclined to just take your patch as is, removing this test altogether, but I wanted to check whether this might have undesirable side effects. I am unclear on the correctness of the current code before your patch, e.g. what if the Linux user/login name is mapped to the same seuser as the fallback but has a more specific level within the authorized range for that seuser? The fact that we already have an exception for root suggests that something is wrong with this test. At worst, removing the test may lead to some unnecessary (but harmless) entries in file_contexts.homedirs, but only if there is an entry in seusers for that Linux user/login in the first place, and they can obviously address that by deleting the user's entry if they do not truly need to map them to a different seuser and/or level.
On Tue, Aug 16, 2016 at 01:13:02PM -0400, Stephen Smalley wrote: >On 08/16/2016 11:59 AM, Gary Tierney wrote: >> Hi Stephen, >> >> Replied inline below. >> >> On Mon, Aug 15, 2016 at 03:58:44PM -0400, Stephen Smalley wrote: >>> On 07/27/2016 05:25 PM, Gary Tierney wrote: >>>> semanage-login supports login mappings using the %group syntax, >>>> but genhomedircon does not expand groups to the users belonging >>>> to them. >>>> >>>> This commit adds support for generating home directory contexts >>>> for login mappings using the group syntax and adds error >>>> reporting for handling cases where there is ambiguity due to a >>>> user belonging to multiple groups mapped by semanage-login. If >>>> a login mapping is added for the user which belongs to multiple >>>> groups it will take precedence and resolve the ambiguity >>>> issue. >>> >>> Sorry for the long delay in responding. One question/comment >>> below. >>> >>>> Signed-off-by: Gary Tierney <gary.tierney@gmx.com> --- >>>> libsemanage/src/genhomedircon.c | 319 >>>> +++++++++++++++++++++++++++++++--------- 1 file changed, 247 >>>> insertions(+), 72 deletions(-) >>>> >>>> diff --git a/libsemanage/src/genhomedircon.c >>>> b/libsemanage/src/genhomedircon.c index c5ea436..2955b19 >>>> 100644 --- a/libsemanage/src/genhomedircon.c +++ >>>> b/libsemanage/src/genhomedircon.c @@ -838,94 +1064,43 @@ static >>>> genhomedircon_user_entry_t *get_users(genhomedircon_settings_t >>>> * s, nusers = 0; } >>>> >>>> + qsort(seuser_list, nseusers, sizeof(semanage_seuser_t *), + >>>> &seuser_sort_func); qsort(user_list, nusers, >>>> sizeof(semanage_user_t *), (int (*)(const void *, const void >>>> *))&user_sort_func); >>>> >>>> - /* Allocate space for the getpwnam_r buffer */ - rbuflen = >>>> sysconf(_SC_GETPW_R_SIZE_MAX); - if (rbuflen <= 0) - goto >>>> cleanup; - rbuf = malloc(rbuflen); - if (rbuf == NULL) - goto >>>> cleanup; - for (i = 0; i < nseusers; i++) { seuname = >>>> semanage_seuser_get_sename(seuser_list[i]); name = >>>> semanage_seuser_get_name(seuser_list[i]); >>>> >>>> - if (strcmp(name,"root") && strcmp(seuname, >>>> s->fallback->sename) == 0) - continue; - >>> >>> This appears to change the behavior of genhomedircon in general, >>> not just with respect to %group handling. Was this intentional? >>> I'm not necessarily opposed to this change, but I am unclear on >>> the implications. It seems that previously genhomedircon would >>> not generate file_contexts.homedirs entries for users who were >>> mapped to the fallback seuser, with an exception for root for >>> /root labeling. With this change, they will have entries added. >>> >> >> Hmm, yes, you're right. This is a mistake. We do need to skip this >> conditional however, so we can check if a user already has a >> mapping when we're expanding the members of a group. >> >> To prevent this from happening it should suffice to check if the >> user that we're writing contexts for shares an sename with the >> __default__ mapping, and if so, simply skip writing contexts for >> that user. We can do this check inside >> write_gen_home_dir_contexts() like so: >> >> https://github.com/garyttierney/selinux/commit/ebdb7f225ddbd5f311b2db75f68e2896285a5090#diff-b298746a257be78548f69d5d296dcd09R1140 >> >> An alternative fix would be to prevent these users from being >> added to the `genhomedircon_user_entry_t` stack in the first place, >> and have get_group_users() search the seusers list for a login >> mapping which matches the group members username, skipping the >> group member if a match is found. >> >> Any thoughts on either of these solutions? > >I'm actually inclined to just take your patch as is, removing this >test altogether, but I wanted to check whether this might have >undesirable side effects. I am unclear on the correctness of the >current code before your patch, e.g. what if the Linux user/login name >is mapped to the same seuser as the fallback but has a more specific >level within the authorized range for that seuser? The fact that we >already have an exception for root suggests that something is wrong >with this test. At worst, removing the test may lead to some >unnecessary (but harmless) entries in file_contexts.homedirs, but only >if there is an entry in seusers for that Linux user/login in the first >place, and they can obviously address that by deleting the user's >entry if they do not truly need to map them to a different seuser >and/or level. > Yes, the level is something I hadn't given much consideration. As far as I can tell the only consequences would be a difference in generated homedir contexts for users who had expected adding a mapping to the fallback user with a more specific level like you mentioned to work. There doesn't seem to be any useful VCS history attached to the line of code responsible for this test, but it seems to me like it exists solely to avoid unnecessarily creating homedir contexts for logins mapped to the fallback user (since the exception for root makes sure that root will always have contexts generated), but it creates the issue with the level mentioned above. It seems like it might also create an issue with users whose home directories aren't under /home (or LU_HOMEDIRECTORY), since they wont be matched by the fallback users homedir regexp. Since an incorrect level could be a security issue, I think this is definitely something which should be resolved. Now that this patch addresses these 2 separate issues, would it be worth breaking this into 2 commits? If all is OK with the new fallback user behavior, of course.
On Wed, Jul 27, 2016 at 10:25:06PM +0100, Gary Tierney wrote: > semanage-login supports login mappings using the %group syntax, but > genhomedircon does not expand groups to the users belonging to them. > > This commit adds support for generating home directory contexts for login > mappings using the group syntax and adds error reporting for handling cases > where there is ambiguity due to a user belonging to multiple groups mapped by > semanage-login. If a login mapping is added for the user which belongs to > multiple groups it will take precedence and resolve the ambiguity issue. > > Signed-off-by: Gary Tierney <gary.tierney@gmx.com> Thanks for this, it was on my todo list so glad you did it for me :D. Some *super* minor issues below. And feel free to add: Reviewed-By: Jason Zaman <jason@perfinion.com> > --- > libsemanage/src/genhomedircon.c | 319 +++++++++++++++++++++++++++++++--------- > 1 file changed, 247 insertions(+), 72 deletions(-) > > diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c > index c5ea436..2955b19 100644 > --- a/libsemanage/src/genhomedircon.c > +++ b/libsemanage/src/genhomedircon.c > @@ -48,6 +48,8 @@ > #include <errno.h> > #include <unistd.h> > #include <regex.h> > +#include <grp.h> > +#include <search.h> > > /* paths used in get_home_dirs() */ > #define PATH_ETC_USERADD "/etc/default/useradd" > @@ -98,6 +100,10 @@ typedef struct user_entry { > char *prefix; > char *home; > char *level; > + > + // The login identifier that was used > + // in semanage-login / seusers the comment makes the struct annoying to read. its obvious enough without it. > + char *login; > struct user_entry *next; > } genhomedircon_user_entry_t; > > @@ -486,6 +492,11 @@ static int USER_CONTEXT_PRED(const char *string) > return (int)(strstr(string, TEMPLATE_USER) != NULL); > } > > +static int STR_COMPARATOR(const void *a, const void *b) > +{ > + return strcmp((const char *) a, (const char *) b); > +} > + > /* make_tempate > * @param s the settings holding the paths to various files > * @param pred function pointer to function to use as filter for slurp > @@ -652,6 +663,24 @@ static int write_user_context(genhomedircon_settings_t * s, FILE * out, > return write_replacements(s, out, tpl, repl); > } > > +static int seuser_sort_func(const void *arg1, const void *arg2) > +{ > + const semanage_seuser_t **u1 = (const semanage_seuser_t **) arg1; > + const semanage_seuser_t **u2 = (const semanage_seuser_t **) arg2;; > + const char *name1 = semanage_seuser_get_name(*u1); > + const char *name2 = semanage_seuser_get_name(*u2); > + > + if (name1[0] == '%' && name2[0] == '%') { > + return 0; > + } else if (name1[0] == '%') { > + return 1; > + } else if (name2[0] == '%') { > + return -1; > + } > + > + return strcmp(name1, name2); > +} > + > static int user_sort_func(semanage_user_t ** arg1, semanage_user_t ** arg2) > { > return strcmp(semanage_user_get_name(*arg1), > @@ -665,7 +694,8 @@ static int name_user_cmp(char *key, semanage_user_t ** val) > > static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, > const char *u, const char *g, const char *sen, > - const char *pre, const char *h, const char *l) > + const char *pre, const char *h, const char *l, > + const char *ln) > { > genhomedircon_user_entry_t *temp = NULL; > char *name = NULL; > @@ -675,6 +705,7 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, > char *prefix = NULL; > char *home = NULL; > char *level = NULL; > + char *lname = NULL; > > temp = malloc(sizeof(genhomedircon_user_entry_t)); > if (!temp) > @@ -700,6 +731,9 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, > level = strdup(l); > if (!level) > goto cleanup; > + lname = strdup(ln); > + if (!lname) > + goto cleanup; > > temp->name = name; > temp->uid = uid; > @@ -708,6 +742,7 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, > temp->prefix = prefix; > temp->home = home; > temp->level = level; > + temp->login = lname; > temp->next = (*list); > (*list) = temp; > > @@ -721,6 +756,7 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, > free(prefix); > free(home); > free(level); > + free(lname); > free(temp); > return STATUS_ERR; > } > @@ -741,6 +777,7 @@ static void pop_user_entry(genhomedircon_user_entry_t ** list) > free(temp->prefix); > free(temp->home); > free(temp->level); > + free(temp->login); > free(temp); > } > > @@ -790,7 +827,8 @@ static int setup_fallback_user(genhomedircon_settings_t * s) > > if (push_user_entry(&(s->fallback), FALLBACK_NAME, > FALLBACK_UIDGID, FALLBACK_UIDGID, > - seuname, prefix, "", level) != 0) > + seuname, prefix, "", level, > + FALLBACK_NAME) != 0) > errors = STATUS_ERR; > semanage_user_key_free(key); > if (u) > @@ -806,6 +844,201 @@ static int setup_fallback_user(genhomedircon_settings_t * s) > return errors; > } > > +static genhomedircon_user_entry_t *find_user(genhomedircon_user_entry_t *head, > + const char *name) > +{ > + for(; head; head = head->next) { > + if (strcmp(head->name, name) == 0) { > + return head; > + } > + } > + > + return NULL; > +} > + > +static int add_user(genhomedircon_settings_t * s, > + genhomedircon_user_entry_t **head, > + semanage_user_t *user, > + const char *name, > + const char *sename, > + const char *selogin) > +{ > + if (selogin[0] == '%') { > + genhomedircon_user_entry_t *orig = find_user(*head, name); > + if (orig != NULL && orig->login[0] == '%') { > + ERR(s->h_semanage, "User %s is already mapped to" > + "group %s, but also belongs to group %s. Add an" > + "explicit mapping for this user to" > + "override group mappings.", > + name, orig->login + 1, selogin + 1); > + return STATUS_ERR; > + } else if (orig != NULL) { > + // user mappings take precedence > + return STATUS_SUCCESS; > + } > + } > + > + int retval = STATUS_ERR; > + > + char *rbuf = NULL; > + long rbuflen; > + struct passwd pwstorage, *pwent = NULL; > + const char *prefix = NULL; > + const char *level = NULL; > + char uid[11]; > + char gid[11]; > + > + /* Allocate space for the getpwnam_r buffer */ > + rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); > + if (rbuflen <= 0) > + goto cleanup; > + rbuf = malloc(rbuflen); > + if (rbuf == NULL) > + goto cleanup; > + > + if (user) { > + prefix = semanage_user_get_prefix(user); > + level = semanage_user_get_mlslevel(user); > + > + if (!level) { > + level = FALLBACK_LEVEL; > + } > + } else { > + prefix = name; > + level = FALLBACK_LEVEL; > + } > + > + retval = getpwnam_r(name, &pwstorage, rbuf, rbuflen, &pwent); > + if (retval != 0 || pwent == NULL) { > + if (retval != 0 && retval != ENOENT) { > + goto cleanup; > + } > + > + WARN(s->h_semanage, > + "user %s not in password file", name); > + retval = STATUS_SUCCESS; > + goto cleanup; > + } > + > + int len = strlen(pwent->pw_dir) -1; > + for(; len > 0 && pwent->pw_dir[len] == '/'; len--) { > + pwent->pw_dir[len] = '\0'; > + } > + > + if (strcmp(pwent->pw_dir, "/") == 0) { > + /* don't relabel / genhomdircon checked to see if root > + * was the user and if so, set his home directory to > + * /root */ > + retval = STATUS_SUCCESS; > + goto cleanup; > + } > + > + if (ignore(pwent->pw_dir)) { > + retval = STATUS_SUCCESS; > + goto cleanup; > + } > + > + len = snprintf(uid, sizeof(uid), "%u", pwent->pw_uid); > + if (len < 0 || len >= (int)sizeof(uid)) { > + goto cleanup; > + } > + > + len = snprintf(gid, sizeof(gid), "%u", pwent->pw_gid); > + if (len < 0 || len >= (int)sizeof(gid)) { > + goto cleanup; > + } > + > + retval = push_user_entry(head, name, uid, gid, sename, prefix, > + pwent->pw_dir, level, selogin); > +cleanup: > + free(rbuf); > + return retval; > +} > + > +static int get_group_users(genhomedircon_settings_t * s, > + genhomedircon_user_entry_t **head, > + semanage_user_t *user, > + const char *sename, > + const char *selogin) > +{ > + int retval = STATUS_ERR; > + > + long grbuflen; > + char *grbuf = NULL; > + struct group grstorage, *group = NULL; > + > + long prbuflen; > + char *pwbuf = NULL; > + struct passwd pwstorage, *pw = NULL; > + > + grbuflen = sysconf(_SC_GETGR_R_SIZE_MAX); > + if (grbuflen <= 0) > + goto cleanup; > + grbuf = malloc(grbuflen); > + if (grbuf == NULL) > + goto cleanup; > + > + const char *grname = selogin + 1; > + > + if (getgrnam_r(grname, &grstorage, grbuf, > + (size_t) grbuflen, &group) != 0) { > + goto cleanup; > + } > + > + if (group == NULL) { > + ERR(s->h_semanage, "Can't find group named %s\n", grname); > + goto cleanup; > + } > + > + size_t nmembers = 0; > + char **members = group->gr_mem; > + > + while (*members != NULL) { > + nmembers++; > + members++; > + } > + > + for (unsigned int i = 0; i < nmembers; i++) { My machine threw an error with this, move the unsigned int i to the top of the func instead. genhomedircon.c: In function ‘get_group_users’: genhomedircon.c:1001:2: error: ‘for’ loop initial declarations are only allowed in C99 or C11 mode > + const char *uname = group->gr_mem[i]; > + > + if (add_user(s, head, user, uname, sename, selogin) < 0) { > + goto cleanup; > + } > + } > + > + prbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); > + if (prbuflen <= 0) > + goto cleanup; > + pwbuf = malloc(prbuflen); > + if (pwbuf == NULL) > + goto cleanup; > + > + setpwent(); > + while ((retval = getpwent_r(&pwstorage, pwbuf, prbuflen, &pw)) == 0) { > + // skip users who also have this group as their > + // primary group > + if (lfind(pw->pw_name, group->gr_mem, &nmembers, > + sizeof(char *), &STR_COMPARATOR)) { > + continue; > + } > + > + if (group->gr_gid == pw->pw_gid) { > + if (add_user(s, head, user, pw->pw_name, > + sename, selogin) < 0) { > + goto cleanup; > + } > + } > + } > + > + retval = STATUS_SUCCESS; > +cleanup: > + endpwent(); > + free(pwbuf); > + free(grbuf); > + > + return retval; > +} > + > static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s, > int *errors) > { > @@ -817,14 +1050,7 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s, > semanage_user_t **u = NULL; > const char *name = NULL; > const char *seuname = NULL; > - const char *prefix = NULL; > - const char *level = NULL; > - char uid[11]; > - char gid[11]; > - struct passwd pwstorage, *pwent = NULL; > unsigned int i; > - long rbuflen; > - char *rbuf = NULL; > int retval; > > *errors = 0; > @@ -838,94 +1064,43 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s, > nusers = 0; > } > > + qsort(seuser_list, nseusers, sizeof(semanage_seuser_t *), > + &seuser_sort_func); > qsort(user_list, nusers, sizeof(semanage_user_t *), > (int (*)(const void *, const void *))&user_sort_func); > > - /* Allocate space for the getpwnam_r buffer */ > - rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); > - if (rbuflen <= 0) > - goto cleanup; > - rbuf = malloc(rbuflen); > - if (rbuf == NULL) > - goto cleanup; > - > for (i = 0; i < nseusers; i++) { > seuname = semanage_seuser_get_sename(seuser_list[i]); > name = semanage_seuser_get_name(seuser_list[i]); > > - if (strcmp(name,"root") && strcmp(seuname, s->fallback->sename) == 0) > - continue; > - > if (strcmp(name, DEFAULT_LOGIN) == 0) > continue; > > if (strcmp(name, TEMPLATE_SEUSER) == 0) > continue; > > - /* %groupname syntax */ > - if (name[0] == '%') > - continue; > - > /* find the user structure given the name */ > - u = bsearch(seuname, user_list, nusers, sizeof(semanage_user_t *), > + u = bsearch(seuname, user_list, nusers, > + sizeof(semanage_user_t *), Undo this one, its just code formatting and adds noise to the patch. > (int (*)(const void *, const void *)) > &name_user_cmp); > - if (u) { > - prefix = semanage_user_get_prefix(*u); > - level = semanage_user_get_mlslevel(*u); > - if (!level) > - level = FALLBACK_LEVEL; > - } else { > - prefix = name; > - level = FALLBACK_LEVEL; > - } > - > - retval = getpwnam_r(name, &pwstorage, rbuf, rbuflen, &pwent); > - if (retval != 0 || pwent == NULL) { > - if (retval != 0 && retval != ENOENT) { > - *errors = STATUS_ERR; > - goto cleanup; > - } > - > - WARN(s->h_semanage, > - "user %s not in password file", name); > - continue; > - } > > - int len = strlen(pwent->pw_dir) -1; > - for(; len > 0 && pwent->pw_dir[len] == '/'; len--) { > - pwent->pw_dir[len] = '\0'; > + /* %groupname syntax */ > + if (name[0] == '%') { > + retval = get_group_users(s, &head, *u, seuname, > + name); > + } else { > + retval = add_user(s, &head, *u, name, > + seuname, name); > } > > - if (strcmp(pwent->pw_dir, "/") == 0) { > - /* don't relabel / genhomdircon checked to see if root > - * was the user and if so, set his home directory to > - * /root */ > - continue; > - } > - if (ignore(pwent->pw_dir)) > - continue; > - > - len = snprintf(uid, sizeof(uid), "%u", pwent->pw_uid); > - if (len < 0 || len >= (int)sizeof(uid)) { > + if (retval != 0) { > *errors = STATUS_ERR; > goto cleanup; > } > - len = snprintf(gid, sizeof(gid), "%u", pwent->pw_gid); > - if (len < 0 || len >= (int)sizeof(gid)) { > - *errors = STATUS_ERR; > - goto cleanup; > - } > - > - if (push_user_entry(&head, name, uid, gid, seuname, > - prefix, pwent->pw_dir, level) != STATUS_SUCCESS) { > - *errors = STATUS_ERR; > - break; > - } > } > > cleanup: > - free(rbuf); > if (*errors) { > for (; head; pop_user_entry(&head)) { > /* the pop function takes care of all the cleanup > -- > 2.7.4 > > _______________________________________________ > Selinux mailing list > Selinux@tycho.nsa.gov > To unsubscribe, send email to Selinux-leave@tycho.nsa.gov. > To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
On 08/16/2016 07:09 PM, Gary Tierney wrote: > On Tue, Aug 16, 2016 at 01:13:02PM -0400, Stephen Smalley wrote: >> On 08/16/2016 11:59 AM, Gary Tierney wrote: >>> Hi Stephen, >>> >>> Replied inline below. >>> >>> On Mon, Aug 15, 2016 at 03:58:44PM -0400, Stephen Smalley >>> wrote: >>>> On 07/27/2016 05:25 PM, Gary Tierney wrote: >>>>> semanage-login supports login mappings using the %group >>>>> syntax, but genhomedircon does not expand groups to the >>>>> users belonging to them. >>>>> >>>>> This commit adds support for generating home directory >>>>> contexts for login mappings using the group syntax and adds >>>>> error reporting for handling cases where there is ambiguity >>>>> due to a user belonging to multiple groups mapped by >>>>> semanage-login. If a login mapping is added for the user >>>>> which belongs to multiple groups it will take precedence >>>>> and resolve the ambiguity issue. >>>> >>>> Sorry for the long delay in responding. One >>>> question/comment below. >>>> >>>>> Signed-off-by: Gary Tierney <gary.tierney@gmx.com> --- >>>>> libsemanage/src/genhomedircon.c | 319 >>>>> +++++++++++++++++++++++++++++++--------- 1 file changed, >>>>> 247 insertions(+), 72 deletions(-) >>>>> >>>>> diff --git a/libsemanage/src/genhomedircon.c >>>>> b/libsemanage/src/genhomedircon.c index c5ea436..2955b19 >>>>> 100644 --- a/libsemanage/src/genhomedircon.c +++ >>>>> b/libsemanage/src/genhomedircon.c @@ -838,94 +1064,43 @@ >>>>> static genhomedircon_user_entry_t >>>>> *get_users(genhomedircon_settings_t * s, nusers = 0; } >>>>> >>>>> + qsort(seuser_list, nseusers, sizeof(semanage_seuser_t >>>>> *), + &seuser_sort_func); qsort(user_list, nusers, >>>>> sizeof(semanage_user_t *), (int (*)(const void *, const >>>>> void *))&user_sort_func); >>>>> >>>>> - /* Allocate space for the getpwnam_r buffer */ - >>>>> rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); - if (rbuflen >>>>> <= 0) - goto cleanup; - rbuf = malloc(rbuflen); - >>>>> if (rbuf == NULL) - goto cleanup; - for (i = 0; i < >>>>> nseusers; i++) { seuname = >>>>> semanage_seuser_get_sename(seuser_list[i]); name = >>>>> semanage_seuser_get_name(seuser_list[i]); >>>>> >>>>> - if (strcmp(name,"root") && strcmp(seuname, >>>>> s->fallback->sename) == 0) - continue; - >>>> >>>> This appears to change the behavior of genhomedircon in >>>> general, not just with respect to %group handling. Was this >>>> intentional? I'm not necessarily opposed to this change, but >>>> I am unclear on the implications. It seems that previously >>>> genhomedircon would not generate file_contexts.homedirs >>>> entries for users who were mapped to the fallback seuser, >>>> with an exception for root for /root labeling. With this >>>> change, they will have entries added. >>>> >>> >>> Hmm, yes, you're right. This is a mistake. We do need to skip >>> this conditional however, so we can check if a user already has >>> a mapping when we're expanding the members of a group. >>> >>> To prevent this from happening it should suffice to check if >>> the user that we're writing contexts for shares an sename with >>> the __default__ mapping, and if so, simply skip writing >>> contexts for that user. We can do this check inside >>> write_gen_home_dir_contexts() like so: >>> >>> https://github.com/garyttierney/selinux/commit/ebdb7f225ddbd5f311b2db75f68e2896285a5090#diff-b298746a257be78548f69d5d296dcd09R1140 >>> >>> >>> >>> An alternative fix would be to prevent these users from being >>> added to the `genhomedircon_user_entry_t` stack in the first >>> place, and have get_group_users() search the seusers list for a >>> login mapping which matches the group members username, >>> skipping the group member if a match is found. >>> >>> Any thoughts on either of these solutions? >> >> I'm actually inclined to just take your patch as is, removing >> this test altogether, but I wanted to check whether this might >> have undesirable side effects. I am unclear on the correctness >> of the current code before your patch, e.g. what if the Linux >> user/login name is mapped to the same seuser as the fallback but >> has a more specific level within the authorized range for that >> seuser? The fact that we already have an exception for root >> suggests that something is wrong with this test. At worst, >> removing the test may lead to some unnecessary (but harmless) >> entries in file_contexts.homedirs, but only if there is an entry >> in seusers for that Linux user/login in the first place, and they >> can obviously address that by deleting the user's entry if they >> do not truly need to map them to a different seuser and/or >> level. >> > > Yes, the level is something I hadn't given much consideration. As > far as I can tell the only consequences would be a difference in > generated homedir contexts for users who had expected adding a > mapping to the fallback user with a more specific level like you > mentioned to work. > > There doesn't seem to be any useful VCS history attached to the > line of code responsible for this test, but it seems to me like it > exists solely to avoid unnecessarily creating homedir contexts for > logins mapped to the fallback user (since the exception for root > makes sure that root will always have contexts generated), but it > creates the issue with the level mentioned above. It seems like it > might also create an issue with users whose home directories aren't > under /home (or LU_HOMEDIRECTORY), since they wont be matched by > the fallback users homedir regexp. Since an incorrect level could > be a security issue, I think this is definitely something which > should be resolved. > > Now that this patch addresses these 2 separate issues, would it be > worth breaking this into 2 commits? If all is OK with the new > fallback user behavior, of course. Sure, especially if you are going to re-spin it anyway for the other comments.
diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c index c5ea436..2955b19 100644 --- a/libsemanage/src/genhomedircon.c +++ b/libsemanage/src/genhomedircon.c @@ -48,6 +48,8 @@ #include <errno.h> #include <unistd.h> #include <regex.h> +#include <grp.h> +#include <search.h> /* paths used in get_home_dirs() */ #define PATH_ETC_USERADD "/etc/default/useradd" @@ -98,6 +100,10 @@ typedef struct user_entry { char *prefix; char *home; char *level; + + // The login identifier that was used + // in semanage-login / seusers + char *login; struct user_entry *next; } genhomedircon_user_entry_t; @@ -486,6 +492,11 @@ static int USER_CONTEXT_PRED(const char *string) return (int)(strstr(string, TEMPLATE_USER) != NULL); } +static int STR_COMPARATOR(const void *a, const void *b) +{ + return strcmp((const char *) a, (const char *) b); +} + /* make_tempate * @param s the settings holding the paths to various files * @param pred function pointer to function to use as filter for slurp @@ -652,6 +663,24 @@ static int write_user_context(genhomedircon_settings_t * s, FILE * out, return write_replacements(s, out, tpl, repl); } +static int seuser_sort_func(const void *arg1, const void *arg2) +{ + const semanage_seuser_t **u1 = (const semanage_seuser_t **) arg1; + const semanage_seuser_t **u2 = (const semanage_seuser_t **) arg2;; + const char *name1 = semanage_seuser_get_name(*u1); + const char *name2 = semanage_seuser_get_name(*u2); + + if (name1[0] == '%' && name2[0] == '%') { + return 0; + } else if (name1[0] == '%') { + return 1; + } else if (name2[0] == '%') { + return -1; + } + + return strcmp(name1, name2); +} + static int user_sort_func(semanage_user_t ** arg1, semanage_user_t ** arg2) { return strcmp(semanage_user_get_name(*arg1), @@ -665,7 +694,8 @@ static int name_user_cmp(char *key, semanage_user_t ** val) static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, const char *u, const char *g, const char *sen, - const char *pre, const char *h, const char *l) + const char *pre, const char *h, const char *l, + const char *ln) { genhomedircon_user_entry_t *temp = NULL; char *name = NULL; @@ -675,6 +705,7 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, char *prefix = NULL; char *home = NULL; char *level = NULL; + char *lname = NULL; temp = malloc(sizeof(genhomedircon_user_entry_t)); if (!temp) @@ -700,6 +731,9 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, level = strdup(l); if (!level) goto cleanup; + lname = strdup(ln); + if (!lname) + goto cleanup; temp->name = name; temp->uid = uid; @@ -708,6 +742,7 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, temp->prefix = prefix; temp->home = home; temp->level = level; + temp->login = lname; temp->next = (*list); (*list) = temp; @@ -721,6 +756,7 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n, free(prefix); free(home); free(level); + free(lname); free(temp); return STATUS_ERR; } @@ -741,6 +777,7 @@ static void pop_user_entry(genhomedircon_user_entry_t ** list) free(temp->prefix); free(temp->home); free(temp->level); + free(temp->login); free(temp); } @@ -790,7 +827,8 @@ static int setup_fallback_user(genhomedircon_settings_t * s) if (push_user_entry(&(s->fallback), FALLBACK_NAME, FALLBACK_UIDGID, FALLBACK_UIDGID, - seuname, prefix, "", level) != 0) + seuname, prefix, "", level, + FALLBACK_NAME) != 0) errors = STATUS_ERR; semanage_user_key_free(key); if (u) @@ -806,6 +844,201 @@ static int setup_fallback_user(genhomedircon_settings_t * s) return errors; } +static genhomedircon_user_entry_t *find_user(genhomedircon_user_entry_t *head, + const char *name) +{ + for(; head; head = head->next) { + if (strcmp(head->name, name) == 0) { + return head; + } + } + + return NULL; +} + +static int add_user(genhomedircon_settings_t * s, + genhomedircon_user_entry_t **head, + semanage_user_t *user, + const char *name, + const char *sename, + const char *selogin) +{ + if (selogin[0] == '%') { + genhomedircon_user_entry_t *orig = find_user(*head, name); + if (orig != NULL && orig->login[0] == '%') { + ERR(s->h_semanage, "User %s is already mapped to" + "group %s, but also belongs to group %s. Add an" + "explicit mapping for this user to" + "override group mappings.", + name, orig->login + 1, selogin + 1); + return STATUS_ERR; + } else if (orig != NULL) { + // user mappings take precedence + return STATUS_SUCCESS; + } + } + + int retval = STATUS_ERR; + + char *rbuf = NULL; + long rbuflen; + struct passwd pwstorage, *pwent = NULL; + const char *prefix = NULL; + const char *level = NULL; + char uid[11]; + char gid[11]; + + /* Allocate space for the getpwnam_r buffer */ + rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); + if (rbuflen <= 0) + goto cleanup; + rbuf = malloc(rbuflen); + if (rbuf == NULL) + goto cleanup; + + if (user) { + prefix = semanage_user_get_prefix(user); + level = semanage_user_get_mlslevel(user); + + if (!level) { + level = FALLBACK_LEVEL; + } + } else { + prefix = name; + level = FALLBACK_LEVEL; + } + + retval = getpwnam_r(name, &pwstorage, rbuf, rbuflen, &pwent); + if (retval != 0 || pwent == NULL) { + if (retval != 0 && retval != ENOENT) { + goto cleanup; + } + + WARN(s->h_semanage, + "user %s not in password file", name); + retval = STATUS_SUCCESS; + goto cleanup; + } + + int len = strlen(pwent->pw_dir) -1; + for(; len > 0 && pwent->pw_dir[len] == '/'; len--) { + pwent->pw_dir[len] = '\0'; + } + + if (strcmp(pwent->pw_dir, "/") == 0) { + /* don't relabel / genhomdircon checked to see if root + * was the user and if so, set his home directory to + * /root */ + retval = STATUS_SUCCESS; + goto cleanup; + } + + if (ignore(pwent->pw_dir)) { + retval = STATUS_SUCCESS; + goto cleanup; + } + + len = snprintf(uid, sizeof(uid), "%u", pwent->pw_uid); + if (len < 0 || len >= (int)sizeof(uid)) { + goto cleanup; + } + + len = snprintf(gid, sizeof(gid), "%u", pwent->pw_gid); + if (len < 0 || len >= (int)sizeof(gid)) { + goto cleanup; + } + + retval = push_user_entry(head, name, uid, gid, sename, prefix, + pwent->pw_dir, level, selogin); +cleanup: + free(rbuf); + return retval; +} + +static int get_group_users(genhomedircon_settings_t * s, + genhomedircon_user_entry_t **head, + semanage_user_t *user, + const char *sename, + const char *selogin) +{ + int retval = STATUS_ERR; + + long grbuflen; + char *grbuf = NULL; + struct group grstorage, *group = NULL; + + long prbuflen; + char *pwbuf = NULL; + struct passwd pwstorage, *pw = NULL; + + grbuflen = sysconf(_SC_GETGR_R_SIZE_MAX); + if (grbuflen <= 0) + goto cleanup; + grbuf = malloc(grbuflen); + if (grbuf == NULL) + goto cleanup; + + const char *grname = selogin + 1; + + if (getgrnam_r(grname, &grstorage, grbuf, + (size_t) grbuflen, &group) != 0) { + goto cleanup; + } + + if (group == NULL) { + ERR(s->h_semanage, "Can't find group named %s\n", grname); + goto cleanup; + } + + size_t nmembers = 0; + char **members = group->gr_mem; + + while (*members != NULL) { + nmembers++; + members++; + } + + for (unsigned int i = 0; i < nmembers; i++) { + const char *uname = group->gr_mem[i]; + + if (add_user(s, head, user, uname, sename, selogin) < 0) { + goto cleanup; + } + } + + prbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); + if (prbuflen <= 0) + goto cleanup; + pwbuf = malloc(prbuflen); + if (pwbuf == NULL) + goto cleanup; + + setpwent(); + while ((retval = getpwent_r(&pwstorage, pwbuf, prbuflen, &pw)) == 0) { + // skip users who also have this group as their + // primary group + if (lfind(pw->pw_name, group->gr_mem, &nmembers, + sizeof(char *), &STR_COMPARATOR)) { + continue; + } + + if (group->gr_gid == pw->pw_gid) { + if (add_user(s, head, user, pw->pw_name, + sename, selogin) < 0) { + goto cleanup; + } + } + } + + retval = STATUS_SUCCESS; +cleanup: + endpwent(); + free(pwbuf); + free(grbuf); + + return retval; +} + static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s, int *errors) { @@ -817,14 +1050,7 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s, semanage_user_t **u = NULL; const char *name = NULL; const char *seuname = NULL; - const char *prefix = NULL; - const char *level = NULL; - char uid[11]; - char gid[11]; - struct passwd pwstorage, *pwent = NULL; unsigned int i; - long rbuflen; - char *rbuf = NULL; int retval; *errors = 0; @@ -838,94 +1064,43 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s, nusers = 0; } + qsort(seuser_list, nseusers, sizeof(semanage_seuser_t *), + &seuser_sort_func); qsort(user_list, nusers, sizeof(semanage_user_t *), (int (*)(const void *, const void *))&user_sort_func); - /* Allocate space for the getpwnam_r buffer */ - rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); - if (rbuflen <= 0) - goto cleanup; - rbuf = malloc(rbuflen); - if (rbuf == NULL) - goto cleanup; - for (i = 0; i < nseusers; i++) { seuname = semanage_seuser_get_sename(seuser_list[i]); name = semanage_seuser_get_name(seuser_list[i]); - if (strcmp(name,"root") && strcmp(seuname, s->fallback->sename) == 0) - continue; - if (strcmp(name, DEFAULT_LOGIN) == 0) continue; if (strcmp(name, TEMPLATE_SEUSER) == 0) continue; - /* %groupname syntax */ - if (name[0] == '%') - continue; - /* find the user structure given the name */ - u = bsearch(seuname, user_list, nusers, sizeof(semanage_user_t *), + u = bsearch(seuname, user_list, nusers, + sizeof(semanage_user_t *), (int (*)(const void *, const void *)) &name_user_cmp); - if (u) { - prefix = semanage_user_get_prefix(*u); - level = semanage_user_get_mlslevel(*u); - if (!level) - level = FALLBACK_LEVEL; - } else { - prefix = name; - level = FALLBACK_LEVEL; - } - - retval = getpwnam_r(name, &pwstorage, rbuf, rbuflen, &pwent); - if (retval != 0 || pwent == NULL) { - if (retval != 0 && retval != ENOENT) { - *errors = STATUS_ERR; - goto cleanup; - } - - WARN(s->h_semanage, - "user %s not in password file", name); - continue; - } - int len = strlen(pwent->pw_dir) -1; - for(; len > 0 && pwent->pw_dir[len] == '/'; len--) { - pwent->pw_dir[len] = '\0'; + /* %groupname syntax */ + if (name[0] == '%') { + retval = get_group_users(s, &head, *u, seuname, + name); + } else { + retval = add_user(s, &head, *u, name, + seuname, name); } - if (strcmp(pwent->pw_dir, "/") == 0) { - /* don't relabel / genhomdircon checked to see if root - * was the user and if so, set his home directory to - * /root */ - continue; - } - if (ignore(pwent->pw_dir)) - continue; - - len = snprintf(uid, sizeof(uid), "%u", pwent->pw_uid); - if (len < 0 || len >= (int)sizeof(uid)) { + if (retval != 0) { *errors = STATUS_ERR; goto cleanup; } - len = snprintf(gid, sizeof(gid), "%u", pwent->pw_gid); - if (len < 0 || len >= (int)sizeof(gid)) { - *errors = STATUS_ERR; - goto cleanup; - } - - if (push_user_entry(&head, name, uid, gid, seuname, - prefix, pwent->pw_dir, level) != STATUS_SUCCESS) { - *errors = STATUS_ERR; - break; - } } cleanup: - free(rbuf); if (*errors) { for (; head; pop_user_entry(&head)) { /* the pop function takes care of all the cleanup
semanage-login supports login mappings using the %group syntax, but genhomedircon does not expand groups to the users belonging to them. This commit adds support for generating home directory contexts for login mappings using the group syntax and adds error reporting for handling cases where there is ambiguity due to a user belonging to multiple groups mapped by semanage-login. If a login mapping is added for the user which belongs to multiple groups it will take precedence and resolve the ambiguity issue. Signed-off-by: Gary Tierney <gary.tierney@gmx.com> --- libsemanage/src/genhomedircon.c | 319 +++++++++++++++++++++++++++++++--------- 1 file changed, 247 insertions(+), 72 deletions(-)