Mercurial > dovecot > core-2.2

changeset 21744:38e0c1b1b1c1

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
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.
author Timo Sirainen <timo.sirainen@dovecot.fi>
date Mon, 13 Mar 2017 14:23:11 +0200
parents 6f75b9de84e1
children b5311039c179
files 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
diffstat 5 files changed, 19 insertions(+), 6 deletions(-) [+]
line wrap: on
line diff
--- a/src/auth/auth-request.c	Mon Mar 13 13:49:04 2017 +0200
+++ b/src/auth/auth-request.c	Mon Mar 13 14:23:11 2017 +0200
@@ -952,6 +952,7 @@
 		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);
@@ -1131,6 +1132,7 @@
 
 	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);
@@ -1359,6 +1361,7 @@
 			   it set */
 			auth_fields_rollback(request->userdb_reply);
 		}
+		request->user_changed_by_lookup = FALSE;
 
 		request->userdb = next_userdb;
 		auth_request_lookup_user(request,
@@ -1417,6 +1420,7 @@
 	const char *cache_key;
 
 	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)
@@ -1541,6 +1545,7 @@
 		/* 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,
--- a/src/auth/auth-request.h	Mon Mar 13 13:49:04 2017 +0200
+++ b/src/auth/auth-request.h	Mon Mar 13 14:23:11 2017 +0200
@@ -130,6 +130,9 @@
 	unsigned int in_delayed_failure_queue:1;
 	unsigned int removed_from_handler:1;
 	unsigned int 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. */
+	unsigned int 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
--- a/src/auth/auth-worker-client.c	Mon Mar 13 13:49:04 2017 +0200
+++ b/src/auth/auth-worker-client.c	Mon Mar 13 14:23:11 2017 +0200
@@ -170,7 +170,8 @@
 		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 @@
 			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 @@
 		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,
--- a/src/auth/passdb-blocking.c	Mon Mar 13 13:49:04 2017 +0200
+++ b/src/auth/passdb-blocking.c	Mon Mar 13 14:23:11 2017 +0200
@@ -31,14 +31,16 @@
 
 	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;
 	}
--- a/src/auth/userdb-blocking.c	Mon Mar 13 13:49:04 2017 +0200
+++ b/src/auth/userdb-blocking.c	Mon Mar 13 14:23:11 2017 +0200
@@ -34,7 +34,7 @@
 			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;