From 5fb982a94827819a1c8e1d1f704c0306e1907108 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@enterprisedb.com>
Date: Fri, 15 Sep 2017 20:32:16 +1200
Subject: [PATCH 1/3] 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, Peter Eisentraut
Discussion: https://postgr.es/m/20170914141205.eup4kxzlkagtmfac%40alvherre.pgsql
---
 src/backend/libpq/auth.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 39a57d4835..1c3938c397 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -2331,9 +2331,9 @@ InitializeLDAPConnection(Port *port, LDAP **ldap)
 
 	if ((r = ldap_set_option(*ldap, LDAP_OPT_PROTOCOL_VERSION, &ldapversion)) != LDAP_SUCCESS)
 	{
-		ldap_unbind(*ldap);
 		ereport(LOG,
 				(errmsg("could not set LDAP protocol version: %s", ldap_err2string(r))));
+		ldap_unbind(*ldap);
 		return STATUS_ERROR;
 	}
 
@@ -2360,18 +2360,18 @@ InitializeLDAPConnection(Port *port, LDAP **ldap)
 				 * should never happen since we import other files from
 				 * wldap32, but check anyway
 				 */
-				ldap_unbind(*ldap);
 				ereport(LOG,
 						(errmsg("could not load wldap32.dll")));
+				ldap_unbind(*ldap);
 				return STATUS_ERROR;
 			}
 			_ldap_start_tls_sA = (__ldap_start_tls_sA) GetProcAddress(ldaphandle, "ldap_start_tls_sA");
 			if (_ldap_start_tls_sA == NULL)
 			{
-				ldap_unbind(*ldap);
 				ereport(LOG,
 						(errmsg("could not load function _ldap_start_tls_sA in wldap32.dll"),
 						 errdetail("LDAP over SSL is not supported on this platform.")));
+				ldap_unbind(*ldap);
 				return STATUS_ERROR;
 			}
 
@@ -2384,9 +2384,9 @@ InitializeLDAPConnection(Port *port, LDAP **ldap)
 		if ((r = _ldap_start_tls_sA(*ldap, NULL, NULL, NULL, NULL)) != LDAP_SUCCESS)
 #endif
 		{
-			ldap_unbind(*ldap);
 			ereport(LOG,
 					(errmsg("could not start LDAP TLS session: %s", ldap_err2string(r))));
+			ldap_unbind(*ldap);
 			return STATUS_ERROR;
 		}
 	}
@@ -2491,6 +2491,7 @@ CheckLDAPAuth(Port *port)
 			{
 				ereport(LOG,
 						(errmsg("invalid character in user name for LDAP authentication")));
+				ldap_unbind(ldap);
 				pfree(passwd);
 				return STATUS_ERROR;
 			}
@@ -2508,6 +2509,7 @@ CheckLDAPAuth(Port *port)
 			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))));
+			ldap_unbind(ldap);
 			pfree(passwd);
 			return STATUS_ERROR;
 		}
@@ -2533,6 +2535,7 @@ CheckLDAPAuth(Port *port)
 			ereport(LOG,
 					(errmsg("could not search LDAP for filter \"%s\" on server \"%s\": %s",
 							filter, port->hba->ldapserver, ldap_err2string(r))));
+			ldap_unbind(ldap);
 			pfree(passwd);
 			pfree(filter);
 			return STATUS_ERROR;
@@ -2554,6 +2557,7 @@ CheckLDAPAuth(Port *port)
 										  count,
 										  filter, port->hba->ldapserver, count)));
 
+			ldap_unbind(ldap);
 			pfree(passwd);
 			pfree(filter);
 			ldap_msgfree(search_message);
@@ -2570,6 +2574,7 @@ CheckLDAPAuth(Port *port)
 			ereport(LOG,
 					(errmsg("could not get dn for the first entry matching \"%s\" on server \"%s\": %s",
 							filter, port->hba->ldapserver, ldap_err2string(error))));
+			ldap_unbind(ldap);
 			pfree(passwd);
 			pfree(filter);
 			ldap_msgfree(search_message);
@@ -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

