On Tue, Jan 12, 2016 at 10:30:29AM +0100, Petr Cech wrote:
> On 01/06/2016 02:19 PM, Jakub Hrozek wrote:
> >On Wed, Jan 06, 2016 at 11:03:45AM +0100, Sumit Bose wrote:
> >>On Wed, Jan 06, 2016 at 10:47:13AM +0100, Pavel Březina wrote:
> >>>On 01/05/2016 05:33 PM, Jakub Hrozek wrote:
> >>>>On Tue, Jan 05, 2016 at 02:12:51PM +0100, Pavel Březina wrote:
> >>>>>On 12/14/2015 03:36 PM, Petr Cech wrote:
> >>>>>>Hi all,
> >>>>>>
> >>>>>>there is patch for
https://fedorahosted.org/sssd/ticket/2791
attached.
> >>>>>>
> >>>>>>Result of patch:
> >>>>>>
> >>>>>>The message:
> >>>>>>Dec 14 14:16:11 vm-058-166 sssd[be[uma.dev]]: dereference
processing
> >>>>>>failed : Input/output error
> >>>>>>is replaced by
> >>>>>>Dec 14 15:29:26 vm-058-166 sssd[be[uma.dev]]: LDAP server
claims to
> >>>>>>support deref, but deref search failed. Disabling deref for
further
> >>>>>>requests. You can permanently disable deref by setting
> >>>>>>ldap_deref_threshold to 0 in domain configuration.
> >>>>>>
> >>>>>>But I am little afraid of this patch. Why? I tested this
work on RHEL 6
> >>>>>>how it is described in the ticket.
> >>>>>>If I tried to set
> >>>>>> ldap_deref_threshold = 0
> >>>>>>it was still falling into bad case and printed I/O error.
So, if we
> >>>>>>considered patch is right, there is question if advice in
new message is
> >>>>>>valid for IPA 3.x.
> >>>>>>
> >>>>>>Regards
> >>>>>>
> >>>>>>Petr
> >>>>>
> >>>>>This threshold parameter applies for groups membership, not for
views where
> >>>>>dereference is always used in current code.
> >>>>>
> >>>>>The question is, why does sdap_x_deref_search_send fail with
EIO?
> >>>>>Dereference should be supported with IPA...
> >>>>
> >>>>Because the dereferenced attribute doesn't exist on the server
IIRC.
> >>>
> >>>Then the debug message is misleading since dereference is in fact
working.
> >>>We should return different error code if possible (ENOENT?) and this
case
> >>>should be handled in views code.
> >>
> >>Unfortunately there is no specific LDAP error code for this case, see
> >>e.g. comments in a50b229c8ea1e22c9efa677760b94d8c48c3ec89. Different
> >>versions of 389ds return LDAP_UNAVAILABLE_CRITICAL_EXTENSION or
> >>LDAP_PROTOCOL_ERROR, not sure what OpenLDAP does.
> >>
> >>I think the error handling code should be moved to the callers because
> >>only they know how to handle different error conditions.
> >
> >+1
> >
> >we can also translate the LDAP-specific error codes into sssd private
> >error codes instead of overloading ENOENT/EIO/ENOSUP (ENOSUP is IMO the
> >only one that really fits the problem..)
> >
> >>
> >>As an alternative now options/flags can be added to
> >>sdap_deref_search_send() as it was done for sdap_get_generic_ext_send()
> >>some time ago, but I would prefer the other.
> >
> >No, I also prefer to let the low-level function be dumb and just return
> >an error which the caller deals with.
>
> Hi,
>
> thanks for all comments.
>
> I found processing of LDAP_UNAVAILABLE_CRITICAL_EXTENSION at
> sdap_get_generic_op_finished() function. There is no processing of
> LDAP_PROTOCOL_ERROR. So I added it.
>
> Now, the error codes for LDAP_RES_SEARCH_RESULT are ENOTSUP for our case and
> EIO for the others.
>
> Next processing of those EIO and ENOTSUP is in sdap_deref_search_done(),
> it is added by 4709ff46db0dbe073aef061b796d2fd7adeaf18f commit. I am not
> sure, after Pavel Brezina comment, if the debug message for ENOTSUP case is
> right here.
>
> Attached patch is applicable for master and sssd-1-12 too.
I'm sorry, but this is not the right thing to do. If the request errors
out with ENOTSUP, we print a log message, too, just different, but on
top of that we also disable dereference..
I don't think the problem can be solved at the lowest sdap level, but at
the callers, similar to how Pavel solved the 'size exceeded' error in
commit 1f1b41931d299d6356ac205b75b402adb2cc9234. We should add a flag to
sdap_deref_search_state that would default to zero but
would allow the caller to suppress the warnings. In
sdap_deref_search_send() I think we can always leave the flag to zero, but
in sdap_deref_search_with_filter_send() we should expose the flag so
that the caller can set the suppress warnings flag. This would be
evaluated in sdap_deref_search_done() and if the flag is set, then the
sss_log() function calls would not be called.
I hope this is doable in a couple of hours. Feel free to ping me if you
need some help.
Petr won't be around for a couple of days and our downstream requested
this fix..so I took over (sorry Petr..) and attached is my version of
the patch.