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

Pavel Reichl preichl at redhat.com
Thu Jul 10 14:38:40 UTC 2014


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.

Regards,
Pavel Reichl 

On Tue, 2014-07-08 at 09:57 +0200, Sumit Bose wrote:
> On Mon, Jul 07, 2014 at 10:25:23PM +0200, Jakub Hrozek wrote:
> > On Mon, Jul 07, 2014 at 07:03:01PM +0200, Pavel Reichl wrote:
> > > On Mon, 2014-06-23 at 11:41 +0200, Sumit Bose wrote:
> > > > On Mon, Jun 23, 2014 at 11:20:52AM +0200, Jakub Hrozek wrote:
> > > > > On Tue, Jun 17, 2014 at 06:30:31PM +0200, Pavel Reichl wrote:
> > > > > > Hello,
> > > > > > 
> > > > > > please see attached patch.
> > > > > > 
> > > > > > Regards,
> > > > > > 
> > > > > > PR
> > > > > 
> > > > > The patch solves the problem, but I think one part should be improved:
> > > > > 
> > > > > > @@ -875,7 +893,13 @@ static void sdap_ad_tokengroups_initgr_mapping_done(struct tevent_req *subreq)
> > > > > >          domain = find_subdomain_by_sid(get_domains_head(state->domain), sid);
> > > > > >          if (domain == NULL) {
> > > > > >              DEBUG(SSSDBG_MINOR_FAILURE, "Domain not found for SID %s\n", sid);
> > > > > > -            continue;
> > > > > > +            if (state->domain->parent == NULL &&
> > > > > > +                state->domain->subdomains == NULL) {
> > > > > > +                domain = state->domain;
> > > > > > +                DEBUG(SSSDBG_TRACE_FUNC, "Using domain %s\n", domain->name);
> > > > > > +            } else {
> > > > > > +                continue;
> > > > > > +            }
> > > > > >          }
> > > > > 
> > > > > I think this is a bit dangerous. I wonder if we should have some
> > > > > modification of find_subdomain_by_sid that would return the first
> > > > > configured domain if no subdomain provider was configured or if no
> > > > > domains had a SID. This could be a separate function.
> > > > 
> > > > This sounds even more dangerous to me.
> > > > 
> > > > > 
> > > > > 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 sufficient
solution?

> bye,
> Sumit
> 
> > 
> > > > which is the case for the IPA and AD provider.
> > > > 
> > > 
> > > > What about adding an explicit check if the running id provider is the
> > > > plain LDAP provider?
> > > Would that be acceptable with you Jakub?
> > 
> > It's a bit hackish but I don't see any other way.
> > 
> > > 
> > > >  As an alternative the LDAP provider can add a
> > > > special value in the id member of the sss_dom_info struct and then
> > > > find_subdomain_by_sid can handle this case specially?
> > > > 
> > > > bye,
> > > > Sumit
> > > > 
> > > > > _______________________________________________
> > > > > sssd-devel mailing list
> > > > > sssd-devel at lists.fedorahosted.org
> > > > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > > > _______________________________________________
> > > > sssd-devel mailing list
> > > > sssd-devel at lists.fedorahosted.org
> > > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > > 
> > > 
> > _______________________________________________
> > sssd-devel mailing list
> > sssd-devel at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

-------------- 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: 9928 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140710/8e0012d6/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: 1674 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140710/8e0012d6/attachment-0001.bin>


More information about the sssd-devel mailing list