[SSSD] [PATCHES][PRELIMINARY] ldap_*_search_base doesn't fully limit the group / netgroup

Jan Zeleny jzeleny at redhat.com
Tue Nov 22 18:56:46 UTC 2011


Pavel Březina <pbrezina at redhat.com> wrote:
> https://fedorahosted.org/sssd/ticket/960
> 
> I'm sending the fix for groups first because I want this to be ACKed
> before I start working on netgroups.
> 
> Current behaviour is that if any of the search bases contain filter,
> than dereference will be turned off and single step approach will be used.
> 
> Algorithm for determining the search base:
>     1. output_filter = ""
>     1. String compare of memberdn and basedn
>        (calculates with scope as well)
>     2. If compare is true and filter != "", append filter to
>        output_filter (|)
>     3. return true and output_filter if it is possible that memberdn
>        belongs to basedn
> The output_filter is then appended (&) in the actual filter.
> 
> There is probably one bug, when you have several search bases when one
> is a generalization of the other but with more restrictive filter.
> 
> For example (LDIF attached):
> ldap_group_search_base =
>     cn=QA,ou=Groups,dc=brq,dc=redhat,dc=com?sub??
>     cn=DEV,ou=Groups,dc=brq,dc=redhat,dc=com?sub?
> 
> ldap_user_search_base =
>     cn=NewHires,ou=People,dc=brq,dc=redhat,dc=com?sub??        (A)
>     ou=People,dc=brq,dc=redhat,dc=com?sub?(&(uid=u1)(uid=u5))  (B)
> 
> GroupA (direct or indirect) members in LDIF are:
>     u1, u3 (from B), u4 (from A)
> 
> Expected result might be u4 (it is currently the actual result).
> However, B is a contradiction and the filter contains this
> contradiction*) so the actual result should be empty membership. But the
> result is:
> getent group groupA
> groupA:*:10002:u4
> 
> * calling ldap_search_ext with
> [(&(|(&(uid=u1)(uid=u3)))(objectclass=posixAccount))][cn=u4,cn=NewHires,ou=
> People,dc=brq,dc=redhat,dc=com]
> 
> Does anyone know what am I missing?
> 
> Thank you,
> Pavel.
> 
> [PATCH 1/3] Function that compares dn to base dn (thanks Jan)
> [PATCH 2/3] Fixes some bugs in previous function and changes it's
> behaviour to follow my needs
> [PATCH 3/3] Fixes the group processing


Nack,
first of all, please squash the first two patches. Originally I hoped the 
functionality in my helper function would be complete. Apparently I was wrong.

In the hepler function, in case the resulting filter was NULL, but there 
wasn't null in the *_filter, you set NULL there. I don't think that's correct, 
you have potential for loosing some allocated memory. If anything, I believe 
talloc_zero() would be better. More importantly, please double check that this 
scenario doesn't happen (i.e. that you always initialize the filter variable 
to NULL).

In the last patch you call either tevent_req_done() or tevent_req_error() and 
then return EOK. That's certainly not correct. Just returning appropriate 
return code (EOK, EAGAIN => EOK, other=>ret) should do the trick I think.

I tried to find out why the correct behavior occurs, so far no success. I'll 
continue with my investigation tomorrow.

Jan



More information about the sssd-devel mailing list