From: Timo Sirainen Date: Thu, 21 Nov 2024 10:23:46 +0000 (+0200) Subject: auth: Fix userdb auth cache with username changes X-Git-Tag: 2.4.0~1419 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=3d19774fa3fe44a4778995c2cc9f0dcbc35e56a2;p=thirdparty%2Fdovecot%2Fcore.git auth: Fix userdb auth cache with username changes The problem was for example when userdb lookup uses only the username part of the username@domain lookup. Then: * "username" lookup caches the results for "username". Since the username didn't change, it doesn't store in the cache the "user" field. * "username@domain" lookup looks up "username" from cache. Since there is no "user" field, the code didn't think the username had changed. Fix this by saving the "user" field to auth cache, regardless of whether it's the same as the current username. --- diff --git a/src/auth/auth-request-fields.c b/src/auth/auth-request-fields.c index 3a8dbfb56c..dca7a92ceb 100644 --- a/src/auth/auth-request-fields.c +++ b/src/auth/auth-request-fields.c @@ -395,7 +395,7 @@ bool auth_request_set_username(struct auth_request *request, event_add_str(request->event, "translated_user", request->fields.translated_username); } - request->user_changed_by_lookup = TRUE; + request->user_returned_by_lookup = TRUE; if (login_username != NULL) { if (!auth_request_set_login_username(request, diff --git a/src/auth/auth-request.c b/src/auth/auth-request.c index 6e61844485..effa1b46af 100644 --- a/src/auth/auth-request.c +++ b/src/auth/auth-request.c @@ -1128,7 +1128,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; + request->user_returned_by_lookup = FALSE; if (request->policy_processed || !request->set->policy_check_before_auth) { @@ -1319,7 +1319,7 @@ void auth_request_lookup_credentials(struct auth_request *request, if (request->wanted_credentials_scheme == NULL) request->wanted_credentials_scheme = p_strdup(request->pool, scheme); - request->user_changed_by_lookup = FALSE; + request->user_returned_by_lookup = FALSE; if (request->policy_processed || !request->set->policy_check_before_auth) { @@ -1445,7 +1445,7 @@ auth_request_userdb_save_cache(struct auth_request *request, auth_fields_append(request->fields.userdb_reply, str, AUTH_FIELD_FLAG_CHANGED, AUTH_FIELD_FLAG_CHANGED, FALSE); - if (request->user_changed_by_lookup) { + if (request->user_returned_by_lookup) { /* username was changed by passdb or userdb */ if (str_len(str) > 0) str_append_c(str, '\t'); @@ -1592,7 +1592,7 @@ void auth_request_userdb_callback(enum userdb_result result, if (result == USERDB_RESULT_INTERNAL_FAILURE) request->userdbs_seen_internal_failure = TRUE; - request->user_changed_by_lookup = FALSE; + request->user_returned_by_lookup = FALSE; request->userdb = next_userdb; auth_request_lookup_user(request, @@ -1666,7 +1666,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->user_returned_by_lookup = FALSE; request->userdb_lookup = TRUE; request->userdb_cache_result = AUTH_REQUEST_CACHE_NONE; if (request->fields.userdb_reply == NULL) @@ -1833,8 +1833,8 @@ auth_request_try_update_username(struct auth_request *request, "username changed %s -> %s", request->fields.user, new_value); auth_request_set_username_forced(request, new_value); - request->user_changed_by_lookup = TRUE; } + request->user_returned_by_lookup = TRUE; return TRUE; } diff --git a/src/auth/auth-request.h b/src/auth/auth-request.h index 9cb7d8440b..6c06a4c962 100644 --- a/src/auth/auth-request.h +++ b/src/auth/auth-request.h @@ -207,9 +207,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; + /* The last passdb/userdb lookup returned a username field. This may + or may not have changed the current username. */ + bool user_returned_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-server.c b/src/auth/auth-worker-server.c index ff0c6fff81..9687c48656 100644 --- a/src/auth/auth-worker-server.c +++ b/src/auth/auth-worker-server.c @@ -231,7 +231,7 @@ static void verify_plain_callback(enum passdb_result result, } if (result != PASSDB_RESULT_INTERNAL_FAILURE) { str_append_c(str, '\t'); - if (request->user_changed_by_lookup) + if (request->user_returned_by_lookup) str_append_tabescaped(str, request->fields.user); str_append_c(str, '\t'); if (request->passdb_password != NULL) @@ -391,7 +391,7 @@ lookup_credentials_callback(enum passdb_result result, str_append(str, "NEXT\t"); else str_append(str, "OK\t"); - if (request->user_changed_by_lookup) + if (request->user_returned_by_lookup) str_append_tabescaped(str, request->fields.user); str_append_c(str, '\t'); if (request->wanted_credentials_scheme[0] != '\0') { @@ -528,7 +528,7 @@ lookup_user_callback(enum userdb_result result, break; case USERDB_RESULT_OK: str_append(str, "OK\t"); - if (auth_request->user_changed_by_lookup) + if (auth_request->user_returned_by_lookup) str_append_tabescaped(str, auth_request->fields.user); /* export only the fields changed by this lookup */ auth_fields_append(auth_request->fields.userdb_reply, str, diff --git a/src/auth/userdb-blocking.c b/src/auth/userdb-blocking.c index c2871f0415..5ce9523e1b 100644 --- a/src/auth/userdb-blocking.c +++ b/src/auth/userdb-blocking.c @@ -37,10 +37,10 @@ static bool user_callback(struct auth_worker_connection *conn ATTR_UNUSED, args += 2; } - if (username[0] != '\0' && - strcmp(request->fields.user, username) != 0) { - auth_request_set_username_forced(request, username); - request->user_changed_by_lookup = TRUE; + if (username[0] != '\0') { + if (strcmp(request->fields.user, username) != 0) + auth_request_set_username_forced(request, username); + request->user_returned_by_lookup = TRUE; } } else { result = USERDB_RESULT_INTERNAL_FAILURE;