From 38b5775ce695ff1002b3c2c6b02cdb71206b5100 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Tue, 8 Aug 2017 10:03:59 +1200 Subject: [PATCH] Log diagnostic messages if errors occur during LDAP auth. Diagnostic messages seem likely to help users diagnose root causes more easily, so let's report them as errdetail. In passing, remove some preexisting code that tries to reuse an LDAP object after calling ldap_unbind, which seems unsafe. Author: Thomas Munro Reviewed-By: Ashutosh Bapat, Christoph Berg, Alvaro Herrera Discussion: https://postgr.es/m/CAEepm=2_dA-SYpFdmNVwvKsEBXOUj=K4ooKovHmvj6jnMdt8dw@mail.gmail.com --- src/backend/libpq/auth.c | 73 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 63 insertions(+), 10 deletions(-) diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 62ff624dbd..de6d0964da 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -2306,6 +2306,27 @@ CheckBSDAuth(Port *port, char *user) #ifdef USE_LDAP /* + * Return a palloc'd copy of the current LDAP diagnostic message, or NULL if + * there is none. + */ +static char * +GetLDAPDiagnosticMessage(LDAP *ldap) +{ + char *result = NULL; + char *message; + int rc; + + rc = ldap_get_option(ldap, LDAP_OPT_DIAGNOSTIC_MESSAGE, &message); + if (rc == LDAP_SUCCESS && message != NULL) + { + result = pstrdup(message); + ldap_memfree(message); + } + + return result; +} + +/* * Initialize a connection to the LDAP server, including setting up * TLS if requested. */ @@ -2331,9 +2352,14 @@ InitializeLDAPConnection(Port *port, LDAP **ldap) if ((r = ldap_set_option(*ldap, LDAP_OPT_PROTOCOL_VERSION, &ldapversion)) != LDAP_SUCCESS) { + char *message = GetLDAPDiagnosticMessage(*ldap); + ldap_unbind(*ldap); ereport(LOG, - (errmsg("could not set LDAP protocol version: %s", ldap_err2string(r)))); + (errmsg("could not set LDAP protocol version: %s", ldap_err2string(r)), + message ? errdetail("LDAP diagnostics: %s", message) : 0)); + if (message) + pfree(message); return STATUS_ERROR; } @@ -2384,9 +2410,14 @@ InitializeLDAPConnection(Port *port, LDAP **ldap) if ((r = _ldap_start_tls_sA(*ldap, NULL, NULL, NULL, NULL)) != LDAP_SUCCESS) #endif { + char *message = GetLDAPDiagnosticMessage(*ldap); + ldap_unbind(*ldap); ereport(LOG, - (errmsg("could not start LDAP TLS session: %s", ldap_err2string(r)))); + (errmsg("could not start LDAP TLS session: %s", ldap_err2string(r)), + message ? errdetail("LDAP diagnostics: %s", message) : 0)); + if (message) + pfree(message); return STATUS_ERROR; } } @@ -2500,9 +2531,15 @@ CheckLDAPAuth(Port *port) port->hba->ldapbindpasswd ? port->hba->ldapbindpasswd : ""); if (r != LDAP_SUCCESS) { + char *message = GetLDAPDiagnosticMessage(ldap); + + ldap_unbind(ldap); ereport(LOG, (errmsg("could not perform initial LDAP bind for ldapbinddn \"%s\" on server \"%s\": %s", - port->hba->ldapbinddn, port->hba->ldapserver, ldap_err2string(r)))); + port->hba->ldapbinddn, port->hba->ldapserver, ldap_err2string(r)), + message ? errdetail("LDAP diagnostics: %s", message) : 0)); + if (message) + pfree(message); pfree(passwd); return STATUS_ERROR; } @@ -2525,9 +2562,15 @@ CheckLDAPAuth(Port *port) if (r != LDAP_SUCCESS) { + char *message = GetLDAPDiagnosticMessage(ldap); + + ldap_unbind(ldap); ereport(LOG, (errmsg("could not search LDAP for filter \"%s\" on server \"%s\": %s", - filter, port->hba->ldapserver, ldap_err2string(r)))); + filter, port->hba->ldapserver, ldap_err2string(r)), + message ? errdetail("LDAP diagnostics: %s", message) : 0)); + if (message) + pfree(message); pfree(passwd); pfree(filter); return STATUS_ERROR; @@ -2559,12 +2602,17 @@ CheckLDAPAuth(Port *port) dn = ldap_get_dn(ldap, entry); if (dn == NULL) { + char *message = GetLDAPDiagnosticMessage(ldap); int error; (void) ldap_get_option(ldap, LDAP_OPT_ERROR_NUMBER, &error); + ldap_unbind(ldap); ereport(LOG, (errmsg("could not get dn for the first entry matching \"%s\" on server \"%s\": %s", - filter, port->hba->ldapserver, ldap_err2string(error)))); + filter, port->hba->ldapserver, ldap_err2string(error)), + message ? errdetail("LDAP diagnostics: %s", message) : 0)); + if (message) + pfree(message); pfree(passwd); pfree(filter); ldap_msgfree(search_message); @@ -2582,10 +2630,9 @@ CheckLDAPAuth(Port *port) { int error; - (void) ldap_get_option(ldap, LDAP_OPT_ERROR_NUMBER, &error); ereport(LOG, - (errmsg("could not unbind after searching for user \"%s\" on server \"%s\": %s", - fulluser, port->hba->ldapserver, ldap_err2string(error)))); + (errmsg("could not unbind after searching for user \"%s\" on server \"%s\"", + fulluser, port->hba->ldapserver))); pfree(passwd); pfree(fulluser); return STATUS_ERROR; @@ -2611,18 +2658,24 @@ CheckLDAPAuth(Port *port) port->hba->ldapsuffix ? port->hba->ldapsuffix : ""); r = ldap_simple_bind_s(ldap, fulluser, passwd); - ldap_unbind(ldap); if (r != LDAP_SUCCESS) { + char *message = GetLDAPDiagnosticMessage(ldap); + + ldap_unbind(ldap); ereport(LOG, (errmsg("LDAP login failed for user \"%s\" on server \"%s\": %s", - fulluser, port->hba->ldapserver, ldap_err2string(r)))); + fulluser, port->hba->ldapserver, ldap_err2string(r)), + message ? errdetail("LDAP diagnostics: %s", message) : 0)); + if (message) + pfree(message); pfree(passwd); pfree(fulluser); return STATUS_ERROR; } + ldap_unbind(ldap); pfree(passwd); pfree(fulluser); -- 2.13.5