dovecot-2.0: auth: Log a warning if ldap attribute has unexpecte...

dovecot at dovecot.org dovecot at dovecot.org
Mon Feb 28 17:34:41 EET 2011


details:   http://hg.dovecot.org/dovecot-2.0/rev/946d1cd3300b
changeset: 12625:946d1cd3300b
user:      Timo Sirainen <tss at iki.fi>
date:      Mon Feb 28 17:34:24 2011 +0200
description:
auth: Log a warning if ldap attribute has unexpectedly multiple values.

diffstat:

 src/auth/auth-request.c |  23 +++++++++--
 src/auth/auth-request.h |   3 +
 src/auth/db-ldap.c      |  77 +++++++++++++++-----------------------
 src/auth/db-ldap.h      |   6 +--
 src/auth/passdb-ldap.c  |  11 ++++-
 src/auth/userdb-ldap.c  |   4 +-
 6 files changed, 64 insertions(+), 60 deletions(-)

diffs (280 lines):

diff -r 067f038ded8b -r 946d1cd3300b src/auth/auth-request.c
--- a/src/auth/auth-request.c	Mon Feb 28 16:03:40 2011 +0200
+++ b/src/auth/auth-request.c	Mon Feb 28 17:34:24 2011 +0200
@@ -1308,10 +1308,7 @@
 	if (*values == NULL)
 		return;
 
-	if (strcmp(name, "uid") == 0) {
-		/* there can be only one. use the first one. */
-		auth_request_set_userdb_field(request, name, *values);
-	} else if (strcmp(name, "gid") == 0) {
+	if (strcmp(name, "gid") == 0) {
 		/* convert gids to comma separated list */
 		string_t *value;
 		gid_t gid;
@@ -1332,6 +1329,11 @@
 				      str_c(value));
 	} else {
 		/* add only one */
+		if (values[1] != NULL) {
+			auth_request_log_warning(request, "userdb",
+				"Multiple values found for '%s', "
+				"using value '%s'", name, *values);
+		}
 		auth_request_set_userdb_field(request, name, *values);
 	}
 }
@@ -1672,6 +1674,19 @@
 	va_end(va);
 }
 
+void auth_request_log_warning(struct auth_request *auth_request,
+			      const char *subsystem,
+			      const char *format, ...)
+{
+	va_list va;
+
+	va_start(va, format);
+	T_BEGIN {
+		i_warning("%s", get_log_str(auth_request, subsystem, format, va));
+	} T_END;
+	va_end(va);
+}
+
 void auth_request_log_error(struct auth_request *auth_request,
 			    const char *subsystem,
 			    const char *format, ...)
diff -r 067f038ded8b -r 946d1cd3300b src/auth/auth-request.h
--- a/src/auth/auth-request.h	Mon Feb 28 16:03:40 2011 +0200
+++ b/src/auth/auth-request.h	Mon Feb 28 17:34:24 2011 +0200
@@ -192,6 +192,9 @@
 void auth_request_log_info(struct auth_request *auth_request,
 			   const char *subsystem,
 			   const char *format, ...) ATTR_FORMAT(3, 4);
+void auth_request_log_warning(struct auth_request *auth_request,
+			      const char *subsystem,
+			      const char *format, ...);
 void auth_request_log_error(struct auth_request *auth_request,
 			    const char *subsystem,
 			    const char *format, ...) ATTR_FORMAT(3, 4);
diff -r 067f038ded8b -r 946d1cd3300b src/auth/db-ldap.c
--- a/src/auth/db-ldap.c	Mon Feb 28 16:03:40 2011 +0200
+++ b/src/auth/db-ldap.c	Mon Feb 28 17:34:24 2011 +0200
@@ -58,12 +58,11 @@
 	struct var_expand_table *var_table;
 
 	char *attr, **vals;
-	const char *name, *value, *template, *val_1_arr[2];
+	const char *name, *template, *val_1_arr[2];
 	const char *const *static_attrs;
 	BerElement *ber;
 
 	string_t *var, *debug;
-	unsigned int value_idx;
 };
 
 struct db_ldap_sasl_bind_context {
@@ -1111,6 +1110,8 @@
 static void
 db_ldap_result_change_attr(struct db_ldap_result_iterate_context *ctx)
 {
+	i_assert(ctx->vals == NULL);
+
 	ctx->name = hash_table_lookup(ctx->attr_map, ctx->attr);
 	ctx->template = NULL;
 
@@ -1119,10 +1120,8 @@
 			    ctx->name != NULL ? ctx->name : "?unknown?");
 	}
 
-	if (ctx->name == NULL || *ctx->name == '\0') {
-		ctx->value = NULL;
+	if (ctx->name == NULL || *ctx->name == '\0')
 		return;
-	}
 
 	if (strchr(ctx->name, '%') != NULL &&
 	    (ctx->template = strchr(ctx->name, '=')) != NULL) {
@@ -1138,31 +1137,35 @@
 
 	ctx->vals = ldap_get_values(ctx->conn->ld, ctx->entry,
 				    ctx->attr);
-	ctx->value = ctx->vals[0];
-	ctx->value_idx = 0;
 }
 
 static void
 db_ldap_result_return_value(struct db_ldap_result_iterate_context *ctx)
 {
-	bool first = ctx->value_idx == 0;
+	unsigned int i;
 
 	if (ctx->template != NULL) {
-		ctx->var_table[0].value = ctx->value;
+		if (ctx->vals[1] != NULL) {
+			auth_request_log_warning(ctx->auth_request, "ldap",
+				"Multiple values found for '%s', "
+				"using value '%s'", ctx->name, ctx->vals[0]);
+		}
+		ctx->var_table[0].value = ctx->vals[0];
 		str_truncate(ctx->var, 0);
 		var_expand(ctx->var, ctx->template, ctx->var_table);
-		ctx->value = str_c(ctx->var);
+		ctx->val_1_arr[0] = str_c(ctx->var);
 	}
 
-	if (ctx->debug != NULL) {
-		if (!first)
-			str_append_c(ctx->debug, '/');
-		if (ctx->auth_request->set->debug_passwords ||
-		    (strcmp(ctx->name, "password") != 0 &&
-		     strcmp(ctx->name, "password_noscheme") != 0))
-			str_append(ctx->debug, ctx->value);
-		else
-			str_append(ctx->debug, PASSWORD_HIDDEN_STR);
+	if (ctx->debug == NULL) {
+		/* no debugging */
+	} else if (ctx->auth_request->set->debug_passwords ||
+		   (strcmp(ctx->name, "password") != 0 &&
+		    strcmp(ctx->name, "password_noscheme") != 0)) {
+		str_append(ctx->debug, ctx->vals[0]);
+		for (i = 1; ctx->vals[i] != NULL; i++)
+			str_printfa(ctx->debug, ", %s", ctx->vals[i]);
+	} else {
+		str_append(ctx->debug, PASSWORD_HIDDEN_STR);
 	}
 }
 
@@ -1171,16 +1174,10 @@
 	const char *p;
 
 	while (ctx->attr != NULL) {
-		if (ctx->vals == NULL) {
-			/* a new attribute */
-			db_ldap_result_change_attr(ctx);
-		} else {
-			/* continuing existing attribute */
-			if (ctx->value != NULL)
-				ctx->value = ctx->vals[++ctx->value_idx];
-		}
+		/* a new attribute */
+		db_ldap_result_change_attr(ctx);
 
-		if (ctx->value != NULL) {
+		if (ctx->vals != NULL) {
 			db_ldap_result_return_value(ctx);
 			return TRUE;
 		}
@@ -1195,14 +1192,13 @@
 		p = strchr(*ctx->static_attrs, '=');
 		if (p == NULL) {
 			ctx->name = *ctx->static_attrs;
-			ctx->value = "";
+			ctx->val_1_arr[0] = "";
 		} else {
 			ctx->name = t_strdup_until(*ctx->static_attrs, p);
-			ctx->value = p + 1;
+			ctx->val_1_arr[0] = p + 1;
 		}
-		/* make _next_all() return correct values */
+		/* make _next() return correct values */
 		ctx->template = "";
-		ctx->val_1_arr[0] = ctx->value;
 		ctx->static_attrs++;
 		return TRUE;
 	}
@@ -1212,31 +1208,18 @@
 }
 
 bool db_ldap_result_iterate_next(struct db_ldap_result_iterate_context *ctx,
-				 const char **name_r, const char **value_r)
-{
-	if (!db_ldap_result_int_next(ctx))
-		return FALSE;
-
-	*name_r = ctx->name;
-	*value_r = ctx->value;
-	return TRUE;
-}
-
-bool db_ldap_result_iterate_next_all(struct db_ldap_result_iterate_context *ctx,
-				     const char **name_r,
-				     const char *const **values_r)
+				 const char **name_r,
+				 const char *const **values_r)
 {
 	if (!db_ldap_result_int_next(ctx))
 		return FALSE;
 
 	if (ctx->template != NULL) {
 		/* we can use only one value with templates */
-		ctx->val_1_arr[0] = ctx->value;
 		*values_r = ctx->val_1_arr;
 	} else {
 		*values_r = (const char *const *)ctx->vals;
 	}
-	ctx->value = NULL;
 	*name_r = ctx->name;
 	return TRUE;
 }
diff -r 067f038ded8b -r 946d1cd3300b src/auth/db-ldap.h
--- a/src/auth/db-ldap.h	Mon Feb 28 16:03:40 2011 +0200
+++ b/src/auth/db-ldap.h	Mon Feb 28 17:34:24 2011 +0200
@@ -178,9 +178,7 @@
 			    struct auth_request *auth_request,
 			    struct hash_table *attr_map);
 bool db_ldap_result_iterate_next(struct db_ldap_result_iterate_context *ctx,
-				 const char **name_r, const char **value_r);
-bool db_ldap_result_iterate_next_all(struct db_ldap_result_iterate_context *ctx,
-				     const char **name_r,
-				     const char *const **values_r);
+				 const char **name_r,
+				 const char *const **values_r);
 
 #endif
diff -r 067f038ded8b -r 946d1cd3300b src/auth/passdb-ldap.c
--- a/src/auth/passdb-ldap.c	Mon Feb 28 16:03:40 2011 +0200
+++ b/src/auth/passdb-ldap.c	Mon Feb 28 17:34:24 2011 +0200
@@ -43,12 +43,17 @@
 		       LDAPMessage *entry, struct auth_request *auth_request)
 {
 	struct db_ldap_result_iterate_context *ldap_iter;
-	const char *name, *value;
+	const char *name, *const *values;
 
 	ldap_iter = db_ldap_result_iterate_init(conn, entry, auth_request,
 						conn->pass_attr_map);
-	while (db_ldap_result_iterate_next(ldap_iter, &name, &value)) {
-		auth_request_set_field(auth_request, name, value,
+	while (db_ldap_result_iterate_next(ldap_iter, &name, &values)) {
+		if (values[1] != NULL) {
+			auth_request_log_warning(auth_request, "ldap",
+				"Multiple values found for '%s', "
+				"using value '%s'", name, values[0]);
+		}
+		auth_request_set_field(auth_request, name, values[0],
 				       conn->set.default_pass_scheme);
 	}
 }
diff -r 067f038ded8b -r 946d1cd3300b src/auth/userdb-ldap.c
--- a/src/auth/userdb-ldap.c	Mon Feb 28 16:03:40 2011 +0200
+++ b/src/auth/userdb-ldap.c	Mon Feb 28 17:34:24 2011 +0200
@@ -52,7 +52,7 @@
 
 	ldap_iter = db_ldap_result_iterate_init(conn, entry, auth_request,
 						conn->user_attr_map);
-	while (db_ldap_result_iterate_next_all(ldap_iter, &name, &values)) {
+	while (db_ldap_result_iterate_next(ldap_iter, &name, &values)) {
 		auth_request_set_userdb_field_values(auth_request,
 						     name, values);
 	}
@@ -168,7 +168,7 @@
 	ldap_iter = db_ldap_result_iterate_init(conn, res,
 						request->auth_request,
 						conn->iterate_attr_map);
-	while (db_ldap_result_iterate_next_all(ldap_iter, &name, &values)) {
+	while (db_ldap_result_iterate_next(ldap_iter, &name, &values)) {
 		if (strcmp(name, "user") != 0) {
 			i_warning("ldap: iterate: "
 				  "Ignoring field not named 'user': %s", name);


More information about the dovecot-cvs mailing list