[SSSD] [PATCH] IPA: incorrect expansion of group membership when encountering a non-POSIX group

Michal Židek mzidek at redhat.com
Tue Aug 5 15:18:44 UTC 2014


On 07/16/2014 05:18 PM, Pavel Reichl wrote:
> Hello,
>
> please see attached patches.
>
> Regards,
>
> Pavel Reichl
>

Hi,

Patch 1: LGTM

Patch 2:

> @@ -962,9 +977,40 @@ static void sdap_initgr_nested_search(struct tevent_req *subreq)
>                                                           groups[0]);
>           state->groups_cur++;
>       } else {
> -        DEBUG(SSSDBG_OP_FAILURE,
> -              "Search for group %s, returned %zu results. Skipping\n",
> -               state->group_dns[state->cur], count);
> +        if (state->try_as_non_posix || count != 0) {
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This condition was a little difficult for me to understand
without additional explanation. Could you add some more descriptive
comments to this (and the else) branch?

> +            /* We can't get the group as posix nor non-posix. Succumb on
> +             * this one and try to continue with another.
> +             */
> +            DEBUG(SSSDBG_OP_FAILURE,
> +                  "Search for group %s, returned %zu results. Skipping\n",
> +                  state->group_dns[state->cur], count);
> +        } else {
> +            /* Try to get the group as non-posix. */
> +            state->try_as_non_posix = true;
> +
> +            DEBUG(SSSDBG_OP_FAILURE,
> +                  "Search for group %s, returned %zu results. "
> +                  "Trying to get it as non-posix\n",
> +                  state->group_dns[state->cur], count);
> +
> +            subreq = sdap_get_generic_send(state, state->ev,
> +                                           state->opts, state->sh,
> +                                           state->group_dns[state->cur],
> +                                           LDAP_SCOPE_BASE,
> +                                           state->np_grp_filter,
> +                                           state->np_grp_attrs,
> +                                           state->opts->np_group_map,
> +                                           SDAP_OPTS_NP_GROUP,
> +                                           dp_opt_get_int(state->opts->basic,
> +                                                          SDAP_SEARCH_TIMEOUT),
> +                                           false);
> +            if (!subreq) {
> +                tevent_req_error(req, ENOMEM);
> +                return;
> +            }
> +            tevent_req_set_callback(subreq, sdap_initgr_nested_search, req);
> +            return;
> +        }
>       }
>
>       state->cur++;
> @@ -972,6 +1018,7 @@ static void sdap_initgr_nested_search(struct tevent_req *subreq)
>        * memberOf which might not be only groups, but permissions, etc.
>        * Use state->groups_cur for group index cap */
>       if (state->cur < state->memberof->num_values) {
> +        state->try_as_non_posix = false;
>           subreq = sdap_get_generic_send(state, state->ev,
>                                          state->opts, state->sh,
>                                          state->group_dns[state->cur],
> @@ -2656,6 +2703,7 @@ struct tevent_req *sdap_get_initgr_send(TALLOC_CTX *memctx,
>       state->conn = conn;
>       state->name = name;
>       state->grp_attrs = grp_attrs;
> +    state->np_grp_attrs = np_grp_attrs;
>       state->orig_user = NULL;
>       state->timeout = dp_opt_get_int(state->opts->basic, SDAP_SEARCH_TIMEOUT);
>       state->user_base_iter = 0;
> -- 1.9.3
>
>

I found some problems while testing these patches.
Let us have hierarchy like this:

GROUP    | MEMBERS (users and groups)
-------------------------------------
pos1     | ipauser1, nonpos2
nonpos2  | ipauser2, pos3
pos3     | ipauser3,

Groups pos1 and pos3 are posix groups, nonpos2 is normal group.

(note: output is sanitized  uid, gid and non relevant groups are deleted)

This is correct (these command are given in order as shown):

# id ipauser3
groups=234600001(pos1),234600003(pos3)

# id ipauser2
groups=234600001(pos1)

# getent group pos1
pos1:*:234600001:ipauser3,ipauser2,ipauser1

Now I clear the cache and do:

# getent group pos1
pos1:*:234600001:ipauser1

Users ipauser2 and ipauser3 are not shown, which is
wrong.

When I try hierarchy like this:

GROUP     | MEMBERS (users and groups)
-------------------------------------
ppos1     | pnonpos, ppos2
pnonpos   | ipauser4,
ppos2     | ipauser5,

Groups ppos1 and ppos2 are posix groups, pnonpos is normal group.

# getent group ppos1
ppos1:*:234600007:ipauser5

So ipauser4 was not returned and it should be. It is only
returned when I do 'id ipauser4' first.

So when searching for users of groups (getgrnam) the
non-posix group cuts the hierarchy if the user data is
not cached already. This is still potential
security issue IMO. If you deny access to group
pos1, the user ipauser2 and ipauser3 will be able
to enter.

So the bug is still there. On top of that we have
inconsistent behaviour when commands like 'id'
precede 'getent group', (in other words, we have
different results of getgrnam with clear and
filled cache) which is weird.

Michal



More information about the sssd-devel mailing list