From ba07d876d30c6259159c9e541ff3beda663fc90c 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 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 cb30fc7b71..3d0b03419d 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("Diagnostic message: %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("Diagnostic message: %s", message) : 0)); + if (message) + pfree(message); return STATUS_ERROR; } } @@ -2472,9 +2503,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("Diagnostic message: %s", message) : 0)); + if (message) + pfree(message); pfree(passwd); return STATUS_ERROR; } @@ -2497,9 +2534,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("Diagnostic message: %s", message) : 0)); + if (message) + pfree(message); pfree(passwd); pfree(filter); return STATUS_ERROR; @@ -2531,12 +2574,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("Diagnostic message: %s", message) : 0)); + if (message) + pfree(message); pfree(passwd); pfree(filter); ldap_msgfree(search_message); @@ -2554,10 +2602,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; @@ -2583,18 +2630,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("Diagnostic message: %s", message) : 0)); + if (message) + pfree(message); pfree(passwd); pfree(fulluser); return STATUS_ERROR; } + ldap_unbind(ldap); pfree(passwd); pfree(fulluser); -- 2.13.2