From 3dadeec1ce7a5bf72fbd850658df1db3cedd4416 Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Mon, 28 Feb 2011 17:34:24 +0200 Subject: [PATCH] auth: Log a warning if ldap attribute has unexpectedly multiple values. --- 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(-) diff --git a/src/auth/auth-request.c b/src/auth/auth-request.c index 67907f169d..aa8bc580f7 100644 --- a/src/auth/auth-request.c +++ b/src/auth/auth-request.c @@ -1308,10 +1308,7 @@ void auth_request_set_userdb_field_values(struct auth_request *request, 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 @@ void auth_request_set_userdb_field_values(struct auth_request *request, 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 @@ void auth_request_log_info(struct auth_request *auth_request, 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 --git a/src/auth/auth-request.h b/src/auth/auth-request.h index 42cffd12aa..081112ec41 100644 --- a/src/auth/auth-request.h +++ b/src/auth/auth-request.h @@ -192,6 +192,9 @@ void auth_request_log_debug(struct auth_request *auth_request, 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 --git a/src/auth/db-ldap.c b/src/auth/db-ldap.c index b9c2506256..f628fe2a92 100644 --- a/src/auth/db-ldap.c +++ b/src/auth/db-ldap.c @@ -58,12 +58,11 @@ struct db_ldap_result_iterate_context { 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 @@ db_ldap_result_iterate_finish(struct db_ldap_result_iterate_context *ctx) 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 @@ db_ldap_result_change_attr(struct db_ldap_result_iterate_context *ctx) 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 @@ db_ldap_result_change_attr(struct db_ldap_result_iterate_context *ctx) 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 @@ static bool db_ldap_result_int_next(struct db_ldap_result_iterate_context *ctx) 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 @@ static bool db_ldap_result_int_next(struct db_ldap_result_iterate_context *ctx) 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 @@ static bool db_ldap_result_int_next(struct db_ldap_result_iterate_context *ctx) } 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 --git a/src/auth/db-ldap.h b/src/auth/db-ldap.h index bd18e82600..8e8d68c7d7 100644 --- a/src/auth/db-ldap.h +++ b/src/auth/db-ldap.h @@ -178,9 +178,7 @@ db_ldap_result_iterate_init(struct ldap_connection *conn, LDAPMessage *entry, 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 --git a/src/auth/passdb-ldap.c b/src/auth/passdb-ldap.c index b7d9233915..4601015643 100644 --- a/src/auth/passdb-ldap.c +++ b/src/auth/passdb-ldap.c @@ -43,12 +43,17 @@ ldap_query_save_result(struct ldap_connection *conn, 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 --git a/src/auth/userdb-ldap.c b/src/auth/userdb-ldap.c index ec7c2659df..b629f740e0 100644 --- a/src/auth/userdb-ldap.c +++ b/src/auth/userdb-ldap.c @@ -52,7 +52,7 @@ ldap_query_get_result(struct ldap_connection *conn, LDAPMessage *entry, 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 @@ static void userdb_ldap_iterate_callback(struct ldap_connection *conn, 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); -- 2.47.3