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.