[SSSD] [PATCHES] Support GPOs referred from other domains

Stephen Gallagher sgallagh at redhat.com
Tue May 26 19:56:35 UTC 2015


On Fri, 2015-05-22 at 13:04 +0200, Jakub Hrozek wrote:
> On Thu, May 14, 2015 at 05:58:49PM +0200, Jakub Hrozek wrote:
> > On Thu, May 14, 2015 at 11:49:17AM -0400, Stephen Gallagher wrote:
> > > On Thu, 2015-05-14 at 17:42 +0200, Jakub Hrozek wrote:
> > > > On Wed, May 06, 2015 at 02:26:30PM -0400, Stephen Gallagher 
> > > > wrote:
> > > > > Patch 0001: LDAP: Support returning referral information
> > > > > 
> > > > > Some callers may be interested in the raw referral values 
> > > > > returned 
> > > > > from
> > > > > a lookup. This patch allows interested consumers to get these 
> > > > > 
> > > > > referrals
> > > > > back and process them if they wish. It does not implement a 
> > > > > generic
> > > > > automatic following of referrals.
> > > > > 
> > > > > 
> > > > > 
> > > > > Patch 0002: AD GPO: Support processing referrals
> > > > > 
> > > > > For GPOs assigned to a site, it's possible that their 
> > > > > definition
> > > > > actually exists in another domain. To retrieve this 
> > > > > information,
> > > > > we need to follow the referral and perform a base search on
> > > > > another domain controller.
> > > > > 
> > > > > Resolves:
> > > > > https://fedorahosted.org/sssd/ticket/2645
> > > > 
> > > > Thanks a lot for the patches! I have one questions about the 
> > > > first 
> > > > patch
> > > > and two nitpicks about the second. I'm also still waiting for 
> > > > the
> > > > Coverity results, but the queue seems to be quite long..
> > > > 
> > > > > From 3f8826061d34639ddaaf245947085ea577e77fbe Mon Sep 17 
> > > > > 00:00:00 
> > > > > 2001
> > > > > From: Stephen Gallagher <sgallagh at redhat.com>
> > > > > Date: Fri, 1 May 2015 11:42:06 -0400
> > > > > Subject: [PATCH 1/2] LDAP: Support returning referral 
> > > > > information
> > > > > 
> > > > > Some callers may be interested in the raw referral values 
> > > > > returned 
> > > > > from
> > > > > a lookup. This patch allows interested consumers to get these 
> > > > > 
> > > > > referrals
> > > > > back and process them if they wish. It does not implement a 
> > > > > generic
> > > > > automatic following of referrals.
> > > > 
> > > > [...]
> > > > 
> > > > >          } else if (result == LDAP_REFERRAL) {
> > > > > -            if (refs != NULL) {
> > > > > -                for (i = 0; refs[i]; i++) {
> > > > > -                    DEBUG(SSSDBG_TRACE_LIBS, "Ref: %s\n", 
> > > > > refs[i]);
> > > > > -                }
> > > > > -                ldap_memvfree((void **) refs);
> > > > > +            ret = sdap_get_generic_ext_add_references(state, 
> > > > > 
> > > > > refs);
> > > > > +            if (ret != EOK) {
> > > > > +                DEBUG(SSSDBG_OP_FAILURE,
> > > > > +                      "sdap_get_generic_ext_add_references 
> > > > > failed: 
> > > > > %s(%d)",
> > > > > +                      sss_strerror(ret), ret);
> > > > > +                tevent_req_error(req, ret);
> > > > >              }
> > > > > -            ldap_memfree(errmsg);
> > > > > -            tevent_req_error(req, ERR_REFERRAL);
> > > > > -            return;
> > > > 
> > > > This is a change in behaviour. Previously, we would always 
> > > > return
> > > > ERR_REFERRAL and let the caller handle the error code based on 
> > > > whether
> > > > the caller ignores referrals (like the AD Provider does by 
> > > > default) 
> > > > or
> > > > not.
> > > > 
> > > > Do you think it's OK to always treat referrals as a success 
> > > > now, even
> > > > for the plain LDAP provider? If so, then I guess we can remove 
> > > > the
> > > > ERR_REFERRAL special-case and the error code as well.
> > > > 
> > > 
> > > Hmm, that's a difficult question. I'd *prefer* to always return 
> > > the
> > > reference list (as I'm doing here) and leave the decision on 
> > > whether or
> > > how to handle it up to the callers. But you're right, it's a 
> > > change in
> > > behavior right now.
> > > 
> > > That being said, it shouldn't functionally be any different, 
> > > since we
> > > pretty much ignore ERR_REFERRAL anyway (we only return the 
> > > records we
> > > actually received, if any). Or am I mistaken in that?
> > 
> > The current sdap_get_generic_ext_recv() handler looks like this:
> > 
> >     ret = sdap_get_generic_ext_recv(subreq);
> >     talloc_zfree(subreq);
> >     if (ret == ERR_REFERRAL) {
> >         if (dp_opt_get_bool(opts->basic, SDAP_REFERRALS)) {
> >             tevent_req_error(req, ret);
> >             return;
> >         } 
> >     }
> > 
> > So unless referrals are supposed to be ignored, we fail the request 
> > on
> > receiving ERR_REFERRAL. We could easily turn that into:
> > 
> >     ret = sdap_get_generic_ext_recv(subreq, mem_ctx, &ref_count, 
> > &refs);
> >     talloc_zfree(subreq);
> >     if (ref_count > 0) {
> >         if (dp_opt_get_bool(opts->basic, SDAP_REFERRALS)) {
> >             tevent_req_error(req, ret);
> >             return;
> >         } 
> >     }
> > 
> > I guess ignoring the referrals makes sense, if SDAP_REFERRALS is 
> > enabled,
> > the referral chasing would follow legitimate referrals. The 
> > referrals we
> > receive in LDAP results processing are just noise more or less...
> 
> Bump..


Sorry for the delay; two new patches attached.

This patch fixes the two missing error checks in the AD GPO code as
well as making several changes to the general LDAP support of
referrals.

While I was looking through this, I discovered a bug that resulted in
referrals for more information not being returned to the caller (this
is different from referral-as-final-result). I tweaked the code so that
this would come back as well. I also added some extra debugging at
level 9 so we can see when these occur, what they were and that they
were ignored.

As discussed above, I just changed the referral check around
sdap_get_generic_ext_recv() to check for ref_count > 0 instead of
ERR_REFERRALS. I didn't remove that completely from the system just in
case we decide to use the error for something else involving referrals
in the future.

I retested these patches against the problematic cross-realm GPO case,
which worked successfully.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-AD-GPO-Support-processing-referrals.patch
Type: text/x-patch
Size: 29728 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150526/e98e407b/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-LDAP-Support-returning-referral-information.patch
Type: text/x-patch
Size: 9198 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150526/e98e407b/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: This is a digitally signed message part
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150526/e98e407b/attachment.sig>


More information about the sssd-devel mailing list