* Send back the final response in an authentication dialog properly

 * Fix double free problems
 * Delete make file

1 files changed, 46 insertions, 30 deletions

diff --git a/plugin/delegateldap.c b/plugin/delegateldap.c
index 7a5c08e..e3460ab 100644
--- a/plugin/delegateldap.c
+++ b/plugin/delegateldap.c
@@ -267,45 +267,54 @@ delegateldap_step (void *context, sasl_server_params_t *sparams,
 	ccred.bv_len = clientinlen;
 	ccred.bv_val = (char*)clientin;
 
-	log_message (sparams->utils, SASL_LOG_NOTE,
+	log_message (sparams->utils, SASL_LOG_DEBUG,
 	             "%d bytes of client request", ccred.bv_len);
 
 	res = ldap_sasl_bind_s (ctx->ldap, NULL, ctx->mechanism, &ccred, NULL, NULL, &scred);
 
-	/* Authentication was successful! */
-	if (res == LDAP_SUCCESS) {
-		log_message (sparams->utils, SASL_LOG_NOTE, "successful authentication");
-		ret = calculate_user (ctx, sparams, oparams);
+	if (res == LDAP_SUCCESS || res == LDAP_SASL_BIND_IN_PROGRESS) {
 
-	/* We need to keep shuttling information */
-	} else if (res == LDAP_SASL_BIND_IN_PROGRESS) {
+		/* Authentication was successful! */
+		if (res == LDAP_SUCCESS) {
+			log_message (sparams->utils, SASL_LOG_NOTE, "successful authentication");
+			ret = calculate_user (ctx, sparams, oparams);
 
-		ret = SASL_CONTINUE;
+		/* We need to keep shuttling information */
+		} else {
+			ret = SASL_CONTINUE;
+		}
 
 		/* Copy the response from the server into SASL allocated memory */
-		if (scred && scred->bv_len) {
-			buffer = sparams->utils->malloc (scred->bv_len);
-			if (buffer) {
-				memcpy (buffer, scred->bv_val, scred->bv_len);
-				*serveroutlen = scred->bv_len;
-				*serverout = buffer;
-
-				log_message (sparams->utils, SASL_LOG_NOTE,
+		if (ret == SASL_OK || ret == SASL_CONTINUE) {
+
+			if (scred)
+				log_message (sparams->utils, SASL_LOG_DEBUG,
 				             "%d bytes of server response", scred->bv_len);
 
+			/* Send back the response to the client */
+			if (scred && scred->bv_len) {
+				buffer = (sparams->utils->malloc) (scred->bv_len);
+				if (buffer) {
+					memcpy (buffer, scred->bv_val, scred->bv_len);
+					*serveroutlen = scred->bv_len;
+					*serverout = buffer;
+				} else {
+					log_message (sparams->utils, SASL_LOG_ERR,
+					             "couldn't allocate memory for server response");
+					ret = SASL_NOMEM;
+				}
+
+			/* No response from the server */
 			} else {
-				log_message (sparams->utils, SASL_LOG_ERR,
-				             "couldn't allocate memory for server response");
-				ret = SASL_NOMEM;
+				*serverout = NULL;
+				*serveroutlen = 0;
+
+				/* We should always have a response when the dialog is going on */
+				if (ret == SASL_CONTINUE) {
+					log_message (sparams->utils, SASL_LOG_WARN,
+					             "no response from server during sasl step");
+				}
 			}
-
-		/* No response from the server, strange */
-		} else {
-			*serverout = NULL;
-			*serveroutlen = 0;
-
-			log_message (sparams->utils, SASL_LOG_WARN,
-			             "no response from server during sasl step");
 		}
 
 	/* Bad login */
@@ -454,11 +463,18 @@ cleanup:
 	} else {
 		log_message (sparams->utils, SASL_LOG_FAIL,
 		             "server doesn't support mechanism: %s", mechanism);
-		*context = NULL;
-		if (ctx)
-			delegateldap_dispose (ctx, sparams->utils);
 	}
 
+	/*
+	 * In theory you're supposed to be able to return the context
+ 	 * which then can be reused for efficiency, but in reality
+	 * cyrus-sasl2 is too fragile and dumb for this, and does strange
+	 * double frees and stuff.
+	 */
+	*context = NULL;
+	if (ctx)
+		delegateldap_dispose (ctx, sparams->utils);
+
 	return ret;
 }