From c05ded9128a919f2d4f7759ca638d0852481abe1 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Fri, 15 Sep 2017 20:32:16 +1200 Subject: [PATCH 1/2] Improve LDAP cleanup code in error paths. After calling ldap_unbind_s() we probably shouldn't try to use the LDAP connection again to call ldap_get_option(), even if it failed. The OpenLDAP man page for ldap_unbind[_s] says "Once it is called, the connection to the LDAP server is closed, and the ld structure is invalid." Otherwise, as a general rule we should probably call ldap_unbind() before returning in all paths to avoid leaking resources. It is unlikely there is any practical leak problem since failure to authenticate currently results in the backend exiting soon afterwards. Author: Thomas Munro Reviewed-By: Alvaro Herrera Discussion: https://postgr.es/m/20170914141205.eup4kxzlkagtmfac%40alvherre.pgsql --- src/backend/libpq/auth.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 39a57d4835..847ded30c0 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -2489,6 +2489,7 @@ CheckLDAPAuth(Port *port) *c == '\\' || *c == '/') { + ldap_unbind(ldap); ereport(LOG, (errmsg("invalid character in user name for LDAP authentication"))); pfree(passwd); @@ -2505,6 +2506,7 @@ CheckLDAPAuth(Port *port) port->hba->ldapbindpasswd ? port->hba->ldapbindpasswd : ""); if (r != LDAP_SUCCESS) { + 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)))); @@ -2530,6 +2532,7 @@ CheckLDAPAuth(Port *port) if (r != LDAP_SUCCESS) { + ldap_unbind(ldap); ereport(LOG, (errmsg("could not search LDAP for filter \"%s\" on server \"%s\": %s", filter, port->hba->ldapserver, ldap_err2string(r)))); @@ -2541,6 +2544,7 @@ CheckLDAPAuth(Port *port) count = ldap_count_entries(ldap, search_message); if (count != 1) { + ldap_unbind(ldap); if (count == 0) ereport(LOG, (errmsg("LDAP user \"%s\" does not exist", port->user_name), @@ -2567,6 +2571,7 @@ CheckLDAPAuth(Port *port) 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)))); @@ -2585,12 +2590,9 @@ CheckLDAPAuth(Port *port) r = ldap_unbind_s(ldap); if (r != LDAP_SUCCESS) { - 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; -- 2.13.5