[SSSD] [PATCH] ad: store group in correct tree on initgroups via tokenGroups

Pavel Březina pbrezina at redhat.com
Fri Sep 27 09:47:57 UTC 2013


On 09/26/2013 09:11 PM, Jakub Hrozek wrote:
> On Wed, Sep 25, 2013 at 05:21:30PM +0200, Jakub Hrozek wrote:
>> On Wed, Sep 25, 2013 at 02:55:22PM +0200, Pavel Březina wrote:
>>> On 09/25/2013 01:07 PM, Jakub Hrozek wrote:
>>>> On Tue, Sep 24, 2013 at 03:14:59PM +0200, Pavel Březina wrote:
>>>>> On 09/21/2013 07:41 PM, Jakub Hrozek wrote:
>>>>>> On Fri, Sep 06, 2013 at 02:30:54PM +0200, Pavel Březina wrote:
>>>>>>> https://fedorahosted.org/sssd/ticket/2066
>>>>>>>
>>>>>>> Please note, that these patches has to be applied atop
>>>>>>> 0004-util-add-find_subdomain_by_sid.patch from my simple
>>>>>>> provider patches. You can also pull it from branch ad-groups at
>>>>>>> my fedorapeople repo.
>>>>>>>
>>>>>>> I still don't know why AD returns Domain Users from root domain
>>>>>>> in tokenGroups, even though subaduser is not member of DU at ad.pb
>>>>>>> (AD bug perhaps?), but with these patches we store it
>>>>>>> correctly.
>>>>>>>
>>>>>>
>>>>>> Did you check if the SID of root domain's Domain Users is also
>>>>>> returned in the PAC?
>>>>>>
>>>>>>> This should be also a step forward to #2064.
>>>>>>
>>>>>> >From 3b07596669cac3ec57e7a20e210b6ccd1fc5fa3b Mon Sep 17
>>>>>>> 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
>>>>>>> <pbrezina at redhat.com> Date: Fri, 6 Sep 2013 11:08:56 +0200
>>>>>>> Subject: [PATCH 1/4] util: add get_domains_head()
>>>>>>>
>>>>>>> This function will return head of the domain list.
>>>>>>>
>>>>>>> Resolves: https://fedorahosted.org/sssd/ticket/2066 ---
>>>>>>> src/util/domain_info_utils.c | 13 +++++++++++++ src/util/util.h
>>>>>>> |  2 ++ 2 files changed, 15 insertions(+)
>>>>>>>
>>>>>>> diff --git a/src/util/domain_info_utils.c
>>>>>>> b/src/util/domain_info_utils.c index
>>>>>>> f9d9057a811f3e08c451d7f6b44bc14202559962..65f3c21f0ad3de2d560347260d317fa21b3e69b3
>>>>>>> 100644 --- a/src/util/domain_info_utils.c +++
>>>>>>> b/src/util/domain_info_utils.c @@ -27,6 +27,19 @@ /* the
>>>>>>> directory domain - realm mappings are written to */ #define
>>>>>>> KRB5_MAPPING_DIR PUBCONF_PATH"/krb5.include.d"
>>>>>>>
>>>>>>> +struct sss_domain_info *get_domains_head(struct
>>>>>>> sss_domain_info *domain) +{ +    struct sss_domain_info *dom =
>>>>>>> NULL; + +    /* get to the top level domain */ +    for (dom =
>>>>>>> domain; dom->parent != NULL; dom = dom->parent); + +    /*
>>>>>>> proceed to the list head */ +    for (; dom->prev != NULL; dom
>>>>>>> = dom->prev);
>>>>>>
>>>>>> Can we currently have a setup with multiple top level domains?
>>>>>
>>>>> No. We create separate sssd_be process for each top level domain
>>>>> and I just checked that be_ctx->domain contains only one top level
>>>>> domain in multi-domain environment.
>>>>
>>>> OK, then what is the point of the loop that proceeds to the list head
>>>> if there can only be one domain?
>>>
>>> state->domain is not necessarily equal to be_ctx->domain in this case. I
>>> would have pass it to _send as parameter and create state->be_domain for
>>> example.
>>>
>>> I'm not quite sure why I've gone this path. I remember that I've
>>> actually created state->be_domain at first but then switched to
>>> get_domains_head().
>>>
>>> I'm also quite surprised that get_domains_head() is called at only one
>>> place. I thought I used it more :-)
>
> Pavel, please file a ticket, assign it to yourself and check the code
> out once again during the 1.11.2 time. I don't see a problem with the
> current code that could cause a bug, but we should keep the incoming
> code clean.
>
> Ack to the current patches on condition you'd check the the logic again.

https://fedorahosted.org/sssd/ticket/2103




More information about the sssd-devel mailing list