Log LDAP "diagnostic messages"?

Started by Thomas Munroover 8 years ago23 messages

Thomas Munro

thomas.munro@enterprisedb.com

over 8 years ago

1 attachment(s)

Hi hackers,

Some LDAP error codes are a bit vague. For example:

LDAP_CONNECT_ERROR Indicates a connection problem.
LDAP_PROTOCOL_ERROR A protocol violation was detected.

To learn more, you have to call
ldap_get_option(LDAP_OPT_DIAGNOSTIC_MESSAGE). Should we do that? For
example, instead of:

LOG: could not start LDAP TLS session: Protocol error

... you could see:

LOG: could not start LDAP TLS session: Protocol error
DETAIL: LDAP diagnostic message: unsupported extended operation

Well, that may not be the most illuminating example, but that's a
message sent back by the LDAP server that we're currently throwing
away, and can be used to distinguish between unsupported TLS versions,
missing StartTLS extension and various other cases. Perhaps that
particular message would also be available via your LDAP server's
logs, if you can access them, but in some cases we're throwing away
client-side messages that are not available anywhere else like "TLS:
unable to get CN from peer certificate", "TLS: hostname does not match
CN in peer certificate" and more.

Something like the attached.

--
Thomas Munro
http://www.enterprisedb.com

Attachments:

ldap-diagnostic-message.patchapplication/octet-stream; name=ldap-diagnostic-message.patchDownload

diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index dd7de7c3a4..1f4f2d4f29 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -2292,6 +2292,33 @@ CheckBSDAuth(Port *port, char *user)
 #ifdef USE_LDAP
 
 /*
+ * Return an error detail message built from the current LDAP diagnostic
+ * message, allocated with palloc.
+ */
+static const char *
+GetLDAPErrorDetail(LDAP *ldap)
+{
+	char	   *detail;
+	char	   *message;
+	int			rc;
+
+	rc = ldap_get_option(ldap, LDAP_OPT_DIAGNOSTIC_MESSAGE, &message);
+	if (rc != LDAP_SUCCESS)
+		detail =
+			psprintf(_("LDAP diagnostic message unavailable: %s"),
+					 ldap_err2string(rc));
+	else if (message == NULL)
+		detail = pstrdup(_("No LDAP diagnostic message available."));
+	else
+	{
+		detail = psprintf(_("LDAP diagnostic message: %s"), message);
+		ldap_memfree(message);
+	}
+
+	return detail;
+}
+
+/*
  * Initialize a connection to the LDAP server, including setting up
  * TLS if requested.
  */
@@ -2370,9 +2397,13 @@ InitializeLDAPConnection(Port *port, LDAP **ldap)
 		if ((r = _ldap_start_tls_sA(*ldap, NULL, NULL, NULL, NULL)) != LDAP_SUCCESS)
 #endif
 		{
+			char	   *detail = GetLDAPErrorDetail(*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)),
+					 errdetail("%s", detail)));
+			pfree(detail);
 			return STATUS_ERROR;
 		}
 	}
@@ -2461,9 +2492,13 @@ CheckLDAPAuth(Port *port)
 							   port->hba->ldapbindpasswd ? port->hba->ldapbindpasswd : "");
 		if (r != LDAP_SUCCESS)
 		{
+			char	   *detail = GetLDAPErrorDetail(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)),
+					 errdetail("%s", detail)));
+			pfree(detail);
 			return STATUS_ERROR;
 		}
 
@@ -2485,9 +2520,13 @@ CheckLDAPAuth(Port *port)
 
 		if (r != LDAP_SUCCESS)
 		{
+			char	   *detail = GetLDAPErrorDetail(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)),
+					 errdetail("%s", detail)));
+			pfree(detail);
 			pfree(filter);
 			return STATUS_ERROR;
 		}
@@ -2517,12 +2556,15 @@ CheckLDAPAuth(Port *port)
 		dn = ldap_get_dn(ldap, entry);
 		if (dn == NULL)
 		{
+			char	   *detail = GetLDAPErrorDetail(ldap);
 			int			error;
 
 			(void) ldap_get_option(ldap, LDAP_OPT_ERROR_NUMBER, &error);
 			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)),
+					 errdetail("%s", detail)));
+			pfree(detail);
 			pfree(filter);
 			ldap_msgfree(search_message);
 			return STATUS_ERROR;
@@ -2537,12 +2579,14 @@ CheckLDAPAuth(Port *port)
 		r = ldap_unbind_s(ldap);
 		if (r != LDAP_SUCCESS)
 		{
+			char	   *detail = GetLDAPErrorDetail(ldap);
 			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))));
+							fulluser, port->hba->ldapserver, ldap_err2string(error)),
+					 errdetail("%s", detail)));
 			pfree(fulluser);
 			return STATUS_ERROR;
 		}