]> git.ipfire.org Git - thirdparty/dovecot/core.git/commitdiff
auth: Auth workers shouldn't return username if it wasn't changed
authorTimo Sirainen <timo.sirainen@dovecot.fi>
Mon, 13 Mar 2017 12:23:11 +0000 (14:23 +0200)
committerGitLab <gitlab@git.dovecot.net>
Thu, 16 Mar 2017 06:50:05 +0000 (08:50 +0200)
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
src/auth/auth-request.h
src/auth/auth-worker-client.c
src/auth/passdb-blocking.c
src/auth/userdb-blocking.c

index af211e0f73efb7237e545d8c24f0972f4b61f9e9..8c7dba9c3e490f56c8cfe0a42f908b3cafd6a81d 100644 (file)
@@ -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,
index 56e95b9434b01b60eb2c7093d6dae315021da332..7b35a3c2b0c6546917ba5e4f0982167a39af4cc1 100644 (file)
@@ -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
index 017e221da86a7b9e06966822ebd985c3785262ce..7b182ad32939fac8d5e3d5137e8f61c926063cdd 100644 (file)
@@ -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,
index cb36dab47839773069eb14cb45105f205719ad58..e6724c46c38d0eee2337093dc99535c8a1f3999d 100644 (file)
@@ -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;
        }
index c4fd5109458a46622505a3a04d2dddd40a112d88..43cb77d3dba6d392f11708278210062ad7f70f99 100644 (file)
@@ -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;