[SSSD] [PATCH] ad: support cross domain membership

Pavel Březina pbrezina at redhat.com
Fri Oct 25 08:55:15 UTC 2013


On 10/24/2013 08:40 PM, Jakub Hrozek wrote:
> On Wed, Oct 02, 2013 at 04:13:32PM +0200, Pavel Březina wrote:
>> On 10/01/2013 09:54 PM, Jakub Hrozek wrote:
>>> On Tue, Sep 24, 2013 at 03:17:47PM +0200, Pavel Březina wrote:
>>>> On 09/24/2013 01:32 PM, Jakub Hrozek wrote:
>>>>> On Wed, Sep 11, 2013 at 02:40:14PM +0200, Pavel Březina wrote:
>>>>>> https://fedorahosted.org/sssd/ticket/2064
>>>>>>
>>>>>> These patch set depends on: [PATCH] ad: store group in correct
>>>>>> tree on initgroups via tokenGroups
>>>>>>
>>>>>> You can also pull it with all dependencies from my repository:
>>>>>> fedorapeople.org:public_git/sssd.git #ad-groups
>>>>>>
>>>>>> The fundamental changes in this patch set are: - lookup groups
>>>>>> in global catalog - pick up member domain from its originalDN
>>>>>
>>>>> >From 0273d17f24eac7b60dfc0515a9e3b97ad16d1199 Mon Sep 17
>>>>>> 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
>>>>>> <pbrezina at redhat.com> Date: Mon, 9 Sep 2013 15:52:03 +0200
>>>>>> Subject: [PATCH 1/9] ad: shortcut if possible during get object
>>>>>> by ID or SID
>>>>>>
>>>>>> When getByID or getBySID comes from responder, the request
>>>>>> doesn't necessarily have to contain correct domain, since
>>>>>> responder iterates over all domains until it finds a match.
>>>>>>
>>>>>> Every domain has its own ID range, so we can simply shortcut
>>>>>> if domain does not match and avoid LDAP round trip. Responder
>>>>>> will continue with next domain until it finds the correct one.
>>>>>
>>>>> This patch seems OK to me, but I'd like a second look from
>>>>> someone who understands the ranges better (which is probably
>>>>> Sumit)
>>>>>
>>>>> >From f74d4637980438032649dfbf079fa6c839862586 Mon Sep 17
>>>>>> 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
>>>>>> <pbrezina at redhat.com> Date: Tue, 10 Sep 2013 10:40:06 +0200
>>>>>> Subject: [PATCH 2/9] ad: simplify get_conn_list()
>>>>>>
>>>>>> It was originally design to return list of connection objects,
>>>>>> it really always work with only one connection.
>>>>>
>>>>> I'd like to review this patch and the following along with my
>>>>> patches to look up POSIX IDs in GC, they touch the same code.
>>>>>
>>>>> >From ad5dc9e7557ef605fc5d7fc759e5cb6c2f9a148c Mon Sep 17
>>>>>> 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
>>>>>> <pbrezina at redhat.com> Date: Tue, 10 Sep 2013 14:45:50 +0200
>>>>>> Subject: [PATCH 4/9] sdap_domain_add(): fix possible memory
>>>>>> leak
>>>>>
>>>>> ACK.
>>>>>
>>>>> >From 9f2c212e01700289d70002c8c39b732ca6c11cee Mon Sep 17
>>>>>> 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
>>>>>> <pbrezina at redhat.com> Date: Tue, 10 Sep 2013 14:45:52 +0200
>>>>>> Subject: [PATCH 5/9] sdap: store base dn in sdap_domain
>>>>>>
>>>>>> Groups may contain members from different domains. Remembering
>>>>>> base dn in domain object gives us the ability to simply lookup
>>>>>> correct domain by comparing object dn with domain base dn.
>>>>>>
>>>>>> Resolves: https://fedorahosted.org/sssd/ticket/2064
>>>>>
>>>>> I haven't tested these patches yet.
>>>>
>>>> I'm sending rebased version of my patches.
>>>>
>>>> [PATCH 4/9] sdap_domain_add(): fix possible memory leak was removed
>>> >from the patch set since recent Sumit's patch removed the code I
>>>> fixed :-)
>>>
>>> Hi,
>>>
>>> can you check if patches #6 and #7 still apply after the recent
>>> changes in 1.11 ? We actually do use the LDAP fallback now..
>>
>> They won't apply, but I think it is quite all right to just skip them.
>> The purpose of these patches was to always contact GC for get_group and
>> initgroups.
>>
>> We always contact GC first at the moment and having LDAP as fallback is
>> fine for groups. If there will be a member from different domain, we
>> will just fail - but if there won't be foreign member it will work.
>
> Hi,
>
> In general the patches look good to me but I think the first two can be
> simplified.
>
> Instead of magically trying to find the base DN from the entry's
> original DN, I think we can simplify it to:
>
> struct sdap_domain *
> sdap_domain_get_by_dn(struct sdap_options *opts,
>                        const char *dn)
> {
>      struct sdap_domain *sditer = NULL;
>
>      DLIST_FOR_EACH(sditer, opts->sdom) {
>          if (sss_ldap_dn_in_search_bases(tmp_ctx, dn, sditer->search_bases, NULL) ||
>              sss_ldap_dn_in_search_bases(tmp_ctx, dn, sditer->user_search_bases, NULL) ||
>              sss_ldap_dn_in_search_bases(tmp_ctx, dn, sditer->group_search_bases, NULL)) {
>              return sditer;
>          }
>      }
>
>      return NULL;
> }
>
> Then you won't need to store the base DN, no need for the magic with strstr
> and the code will work even for custom search bases.

Hi,
new patches are attached. I just added all of the search bases.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ad-shortcut-if-possible-during-get-object-by-ID-or-S.patch
Type: text/x-patch
Size: 3366 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20131025/f513a567/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-sdap-add-sdap_domain_get_by_dn.patch
Type: text/x-patch
Size: 2719 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20131025/f513a567/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-ghosts-pick-correct-domain-for-every-member.patch
Type: text/x-patch
Size: 3364 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20131025/f513a567/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-sdap_fill_memberships-pick-correct-domain-for-every-.patch
Type: text/x-patch
Size: 3232 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20131025/f513a567/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-nested-groups-pick-correct-domain-for-cache-lookups.patch
Type: text/x-patch
Size: 2870 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20131025/f513a567/attachment-0004.bin>


More information about the sssd-devel mailing list