[SSSD] [PATCH] LDAP: tokengroups do not work with id_provider=ldap

Pavel Reichl preichl at redhat.com
Thu Jul 17 15:41:23 UTC 2014


On Thu, 2014-07-17 at 07:59 +0200, Jakub Hrozek wrote:
> On 10 Jul 2014, at 16:38, Pavel Reichl <preichl at redhat.com> wrote:
> 
> > Hello,
> > 
> > please see attached patches. 
> > 
> > I found out that if we go with approach introduced in previous version
> > (in case of LDAP provider assume SID comes from default domain) this can
> > lead to resolutions of SIDs like S-1-5-32-545 and also SIDs of non-posix
> > groups which in case of disabled id mapping leads to failure which will
> > end request prematurely. 2nd patch should make SID resolution more
> > resilient to handle this.
> > 
> 
> The only complaint I have about the code is that the domain NULL check is not needed, I actually think we should fail if there is a NULL domain in the provider code, the provider handler should already take care of finding the right domain. If not, we’re in big trouble anyway.
> 
> Otherwise the code solves the problem — I can’t say it looks great. That’s not your fault Pavel, just yet another reminder that we need to work on the LDAP provider refactoring no later than 1.13.
> 
> I tested with both id_provider=ldap and =ad using both POSIX attributes and ID mapping. Both worked fine in my testing.
> 
> Can you re-send the patches without the domain check? Then I’ll ack.

Sure, attached patches address your and Lukas' comments.

[snip]
> >>>>>> Anyhow, find_subdomain_by_sid is misnamed, we routinely use the function
> >>>>>> to find the primary domain.
> >>>>> 
> >>>>> I think find_subdomain_by_sid() does what the name says and of course it
> >>>>> can return the primary domain as long as the SID of the domain is know
> >>>  ^^^^^^
> >>> fwiw, this was my concern, the function is named "find_subdomain" yet it
> >>> can find both main domain and subdomain. But I won't bikeshed any further.
> >> 
> >> ah, sorry, now I see your point. I agree that the name misleading but I
> >> think this can be fixed after the release.
> > 

Would 's/find_subdomain_by_sid/find_domain_by_sid/' be a sufficient
solution?

[snip]

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-LDAP-tokengroups-do-not-work-with-id_provider-ldap.patch
Type: text/x-patch
Size: 9907 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140717/e087512a/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-SDAP-Continue-resolving-SID-even-if-some-fail.patch
Type: text/x-patch
Size: 1672 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140717/e087512a/attachment-0001.bin>


More information about the sssd-devel mailing list