Stephen Gallagher wrote:
On 07/02/2010 09:51 AM, Dmitri Pal wrote:
> Stephen Gallagher wrote:
>> Also adds support for detecting LDAPS errors by adding a check for
>> SDAP_DIAGNOSTIC_MESSAGE after ldap_search_ext()
>>
>> This should make debugging an initial setup easier, as the most common
>> configuration errors will appear in /var/log/messages, instead of
>> requiring the user to turn on the debug logs.
>>
> Visual ack with nitpicks:
> + syslog(LOG_ERR, "Could not start TLS. "
> + "Check for certificate issues\n");
> + syslog(LOG_ERR, "Could not install TLS. "
> + "Check for certificate issues\n");
> 1) Trailing dots are missing
Yeah, I should be using a period instead of \n here.
> 2) Don't you want to print out the ldap_err2string(ret) anyways?
ldap_err2string(ret) doesn't actually give any information useful for
resolving the issue. It's a very generic error (e.g. "Could not contact
LDAP server").
The purpose of the SDAP_DIAGNOSTIC_MESSAGE is to provide actual useful
information like "Self-signed certificate in certificate chain".
> 3) What does "install" TLS means? This message sounds confusing. What is
> it that you are "installing"?
This is standard terminology. When a START_TLS command is issued, TLS is
"installed" onto the existing connection.
Also note that this is the error handling for the ldap_install_tls()
function of the OpenLDAP library.
New patch attached to address 1)
The terminology is more developer specific IMO. And the admin who is
troubleshooting the issue might not be familiar with the terminology
that OpenLDAP internally uses.
I can give functional visual ack but I am concerned a bit about the
terminology. Can it be made less OpenLDAP focused?
-------------------------
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
--
Thank you,
Dmitri Pal
Engineering Manager IPA project,
Red Hat Inc.
-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/