[SSSD] [PATCH] IPA: incorrect expansion of group membership when encountering a non-POSIX group
Pavel Reichl
preichl at redhat.com
Thu Aug 14 11:20:33 UTC 2014
On 08/05/2014 05:18 PM, Michal Židek wrote:
> 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
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Hello,
please see updated patches.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-IPA-new-attribute-map-for-non-posix-groups.patch
Type: text/x-patch
Size: 8000 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140814/ad9b4dd6/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-IPA-process-non-posix-nested-groups.patch
Type: text/x-patch
Size: 6266 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140814/ad9b4dd6/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-IPA-try-to-resolve-nested-groups-as-poxix-group.patch
Type: text/x-patch
Size: 9739 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140814/ad9b4dd6/attachment-0002.bin>
More information about the sssd-devel
mailing list