autofs-5.0.1 - refactor ldap sasl bind

From: Ian Kent <raven@themaw.net>

During the sasl authentication (and possible authentication method
selection) we establish a connection and then dispose of it and then
bind again. This is a little inefficient but some servers don't like
to bind again using the same LDAP handle and authentication fails
when it should succeed. We should use the authentication connection
once we get it and not perform another re-connect later. Also,

Also, if a server returns a set of mechanisms that all require
authentication, then the conn pointer is returned to the caller
uninitialized (reported and fix provided by Jeff Moyer).
---

 modules/cyrus-sasl.c  |   55 ++++++++++++++++++---------------------------
 modules/lookup_ldap.c |   60 --------------------------------------------------
 2 files changed, 22 insertions(+), 93 deletions(-)


--- autofs-5.0.1.orig/modules/cyrus-sasl.c
+++ autofs-5.0.1/modules/cyrus-sasl.c
@@ -88,8 +88,8 @@ static sasl_callback_t callbacks[] = {
 	{ SASL_CB_LIST_END, NULL, NULL },
 };
 
-static char *sasl_auth_id, *sasl_auth_secret;
-sasl_secret_t *sasl_secret;
+static char *sasl_auth_id = NULL;
+static char *sasl_auth_secret = NULL;
 
 static int
 sasl_log_func(void *context, int level, const char *message)
@@ -790,7 +790,7 @@ sasl_bind_mech(unsigned logopt, LDAP *ld
 sasl_conn_t *
 sasl_choose_mech(unsigned logopt, LDAP *ldap, struct lookup_context *ctxt)
 {
-	sasl_conn_t *conn;
+	sasl_conn_t *conn = NULL;
 	int authenticated;
 	int i;
 	char **mechanisms;
@@ -837,22 +837,6 @@ sasl_choose_mech(unsigned logopt, LDAP *
 	return conn;
 }
 
-int
-autofs_sasl_bind(unsigned logopt, LDAP *ldap, struct lookup_context *ctxt)
-{
-	sasl_conn_t *conn;
-
-	if (!ctxt->sasl_mech)
-		return -1;
-
-	conn = sasl_bind_mech(logopt, ldap, ctxt, ctxt->sasl_mech);
-	if (!conn)
-		return -1;
-
-	ctxt->sasl_conn = conn;
-	return 0;
-}
-
 /*
  *  Routine called when unbinding an ldap connection.
  */
@@ -875,35 +859,40 @@ autofs_sasl_unbind(struct lookup_context
  * -1  -  Failure
  */
 int
-autofs_sasl_init(unsigned logopt, LDAP *ldap, struct lookup_context *ctxt)
+autofs_sasl_bind(unsigned logopt, LDAP *ldap, struct lookup_context *ctxt)
 {
-	sasl_conn_t *conn;
+	sasl_conn_t *conn = NULL;
+
+	/* If we already have a connection use it */
+	if (ctxt->sasl_conn)
+		return 0;
 
 	sasl_auth_id = ctxt->user;
 	sasl_auth_secret = ctxt->secret;
 
+	if (ctxt->auth_required & LDAP_AUTH_AUTODETECT) {
+		if (ctxt->sasl_mech) {
+			free(ctxt->sasl_mech);
+			ctxt->sasl_mech = NULL;
+		}
+	}
+
 	/*
 	 *  If LDAP_AUTH_AUTODETECT is set, it means that there was no
 	 *  mechanism specified in the configuration file or auto
 	 *  selection has been requested, so try to auto-select an
 	 *  auth mechanism.
 	 */
-	if (!(ctxt->auth_required & LDAP_AUTH_AUTODETECT))
+	if (ctxt->sasl_mech)
 		conn = sasl_bind_mech(logopt, ldap, ctxt, ctxt->sasl_mech);
-	else {
-		if (ctxt->sasl_mech) {
-			free(ctxt->sasl_mech);
-			ctxt->sasl_mech = NULL;
-		}
+	else
 		conn = sasl_choose_mech(logopt, ldap, ctxt);
-	}
 
-	if (conn) {
-		sasl_dispose(&conn);
-		return 0;
-	}
+	if (!conn)
+		return -1;
 
-	return -1;
+	ctxt->sasl_conn = conn;
+	return 0;
 }
 
 /*
--- autofs-5.0.1.orig/modules/lookup_ldap.c
+++ autofs-5.0.1/modules/lookup_ldap.c
@@ -49,7 +49,6 @@ static struct ldap_schema common_schema[
 };
 static unsigned int common_schema_count = sizeof(common_schema)/sizeof(struct ldap_schema);
 
-static LDAP *auth_init(unsigned logopt, const char *, struct lookup_context *);
 static int decode_percent_hack(const char *, char **);
 
 static void uris_mutex_lock(struct lookup_context *ctxt)
@@ -525,33 +524,6 @@ static LDAP *connect_to_server(unsigned 
 {
 	LDAP *ldap;
 
-#ifdef WITH_SASL
-	/*
-	 * Determine which authentication mechanism to use if we require
-	 * authentication.
-	 */
-	if (ctxt->auth_required & (LDAP_AUTH_REQUIRED|LDAP_AUTH_AUTODETECT)) {
-		ldap = auth_init(logopt, uri, ctxt);
-		if (!ldap && ctxt->auth_required & LDAP_AUTH_AUTODETECT)
-			info(logopt,
-			     "no authentication mechanisms auto detected.");
-		if (!ldap) {
-			error(logopt, MODPREFIX
-			      "cannot initialize authentication setup");
-			return NULL;
-		}
-
-		if (!do_bind(logopt, ldap, uri, ctxt)) {
-			unbind_ldap_connection(logopt, ldap, ctxt);
-			autofs_sasl_dispose(ctxt);
-			error(logopt, MODPREFIX "cannot bind to server");
-			return NULL;
-		}
-
-		return ldap;
-	}
-#endif
-
 	ldap = do_connect(logopt, uri, ctxt);
 	if (!ldap) {
 		warn(logopt,
@@ -999,38 +971,6 @@ out:
 
 	return ret;
 }
-
-/*
- *  Reads in the xml configuration file and parses out the relevant
- *  information.  If there is no configuration file, then we fall back to
- *  trying all supported authentication mechanisms until one works.
- *
- *  Returns ldap connection on success, with authtype, user and secret
- *  filled in as appropriate.  Returns NULL on failre.
- */
-static LDAP *auth_init(unsigned logopt, const char *uri, struct lookup_context *ctxt)
-{
-	int ret;
-	LDAP *ldap;
-
-	ldap = init_ldap_connection(logopt, uri, ctxt);
-	if (!ldap)
-		return NULL;
-
-	/*
-	 *  Initialize the sasl library.  It is okay if user and secret
-	 *  are NULL, here.
-	 *
-	 *  The autofs_sasl_init routine will figure out which mechamism
-	 *  to use. If kerberos is used, it will also take care to initialize
-	 *  the credential cache and the client and service principals.
-	 */
-	ret = autofs_sasl_init(logopt, ldap, ctxt);
-	if (ret)
-		return NULL;
-
-	return ldap;
-}
 #endif
 
 /*