From 865a82c1e9bba11609835a36674964649025bf77 Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Mon, 13 Mar 2017 14:23:11 +0200 Subject: [PATCH] auth: Auth workers shouldn't return username if it wasn't changed This continues the previous fix where username was always added to passdb/userdb cache, even if the username wasn't changed. That could have resulted in wrongly changing usernames if the cache key didn't uniquely identify the user. --- src/auth/auth-request.c | 5 +++++ src/auth/auth-request.h | 3 +++ src/auth/auth-worker-client.c | 9 ++++++--- src/auth/passdb-blocking.c | 6 ++++-- src/auth/userdb-blocking.c | 2 +- 5 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/auth/auth-request.c b/src/auth/auth-request.c index af211e0f73..8c7dba9c3e 100644 --- a/src/auth/auth-request.c +++ b/src/auth/auth-request.c @@ -967,6 +967,7 @@ void auth_request_verify_plain(struct auth_request *request, request->mech_password = p_strdup(request->pool, password); else i_assert(request->mech_password == password); + request->user_changed_by_lookup = FALSE; if (request->policy_processed) { auth_request_verify_plain_continue(request, callback); @@ -1158,6 +1159,7 @@ void auth_request_lookup_credentials(struct auth_request *request, if (request->credentials_scheme == NULL) request->credentials_scheme = p_strdup(request->pool, scheme); + request->user_changed_by_lookup = FALSE; if (request->policy_processed) auth_request_lookup_credentials_policy_continue(request, callback); @@ -1400,6 +1402,7 @@ void auth_request_userdb_callback(enum userdb_result result, it set */ auth_fields_rollback(request->userdb_reply); } + request->user_changed_by_lookup = FALSE; request->userdb = next_userdb; auth_request_lookup_user(request, @@ -1464,6 +1467,7 @@ void auth_request_lookup_user(struct auth_request *request, const char *cache_key, *error; request->private_callback.userdb = callback; + request->user_changed_by_lookup = FALSE; request->userdb_lookup = TRUE; request->userdb_result_from_cache = FALSE; if (request->userdb_reply == NULL) @@ -1601,6 +1605,7 @@ bool auth_request_set_username(struct auth_request *request, /* similar to original_username, but after translations */ request->translated_username = request->user; } + request->user_changed_by_lookup = TRUE; if (login_username != NULL) { if (!auth_request_set_login_username(request, diff --git a/src/auth/auth-request.h b/src/auth/auth-request.h index 56e95b9434..7b35a3c2b0 100644 --- a/src/auth/auth-request.h +++ b/src/auth/auth-request.h @@ -130,6 +130,9 @@ struct auth_request { bool in_delayed_failure_queue:1; bool removed_from_handler:1; bool snapshot_have_userdb_prefetch_set:1; + /* username was changed by this passdb/userdb lookup. Used by + auth-workers to determine whether to send back a changed username. */ + bool user_changed_by_lookup:1; /* each passdb lookup can update the current success-status using the result_* rules. the authentication succeeds only if this is TRUE at the end. mechanisms that don't require passdb, but do a passdb diff --git a/src/auth/auth-worker-client.c b/src/auth/auth-worker-client.c index 017e221da8..7b182ad329 100644 --- a/src/auth/auth-worker-client.c +++ b/src/auth/auth-worker-client.c @@ -170,7 +170,8 @@ static void verify_plain_callback(enum passdb_result result, str_printfa(str, "FAIL\t%d", result); if (result != PASSDB_RESULT_INTERNAL_FAILURE) { str_append_c(str, '\t'); - str_append_tabescaped(str, request->user); + if (request->user_changed_by_lookup) + str_append_tabescaped(str, request->user); str_append_c(str, '\t'); if (request->passdb_password != NULL) str_append_tabescaped(str, request->passdb_password); @@ -255,7 +256,8 @@ lookup_credentials_callback(enum passdb_result result, str_append(str, "NEXT\t"); else str_append(str, "OK\t"); - str_append_tabescaped(str, request->user); + if (request->user_changed_by_lookup) + str_append_tabescaped(str, request->user); str_append_c(str, '\t'); if (request->credentials_scheme[0] != '\0') { str_printfa(str, "{%s.b64}", request->credentials_scheme); @@ -389,7 +391,8 @@ lookup_user_callback(enum userdb_result result, break; case USERDB_RESULT_OK: str_append(str, "OK\t"); - str_append_tabescaped(str, auth_request->user); + if (auth_request->user_changed_by_lookup) + str_append_tabescaped(str, auth_request->user); str_append_c(str, '\t'); /* export only the fields changed by this lookup */ auth_fields_append(auth_request->userdb_reply, str, diff --git a/src/auth/passdb-blocking.c b/src/auth/passdb-blocking.c index cb36dab478..e6724c46c3 100644 --- a/src/auth/passdb-blocking.c +++ b/src/auth/passdb-blocking.c @@ -31,14 +31,16 @@ auth_worker_reply_parse(struct auth_request *request, const char *reply) if (strcmp(*args, "OK") == 0 && args[1] != NULL && args[2] != NULL) { /* OK \t user \t password [\t extra] */ - auth_request_set_field(request, "user", args[1], NULL); + if (args[1][0] != '\0') + auth_request_set_field(request, "user", args[1], NULL); auth_worker_reply_parse_args(request, args + 2); return PASSDB_RESULT_OK; } if (strcmp(*args, "NEXT") == 0 && args[1] != NULL) { /* NEXT \t user [\t extra] */ - auth_request_set_field(request, "user", args[1], NULL); + if (args[1][0] != '\0') + auth_request_set_field(request, "user", args[1], NULL); auth_worker_reply_parse_args(request, args + 1); return PASSDB_RESULT_NEXT; } diff --git a/src/auth/userdb-blocking.c b/src/auth/userdb-blocking.c index c4fd510945..43cb77d3db 100644 --- a/src/auth/userdb-blocking.c +++ b/src/auth/userdb-blocking.c @@ -34,7 +34,7 @@ static bool user_callback(const char *reply, void *context) args = ""; else username = t_strdup_until(username, args++); - if (strcmp(request->user, username) != 0) + if (username[0] != '\0' && strcmp(request->user, username) != 0) request->user = p_strdup(request->pool, username); } else { result = USERDB_RESULT_INTERNAL_FAILURE; -- 2.47.3