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@xxxxxxx>
> > ---
> > 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
> >
>
--
Gary Tierney
GPG fingerprint: 412C 0EF9 C305 68E6 B660 BDAF 706E D765 85AA 79D8
https://sks-keyservers.net/pks/lookup?op=get&search=0x706ED76585AA79D8
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Selinux mailing list Selinux@xxxxxxxxxxxxx To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx. To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
