From: Timo Sirainen <timo.sirainen@open-xchange.com>
Date: Fri, 21 May 2021 13:11:45 +0000 (+0300)
Subject: imap: Send tagged login reply before finalizing user initialization
X-Git-Tag: 2.3.16~97
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d9f2b426ea5ced3d0bb840106615396a77c88f96;p=thirdparty%2Fdovecot%2Fcore.git

imap: Send tagged login reply before finalizing user initialization

Broken by 5fc66f182ff6941639d30372b414c1b39ae1e67e
---

diff --git a/src/imap/imap-client.c b/src/imap/imap-client.c
index 36db6fce45..da56eb21eb 100644
--- a/src/imap/imap-client.c
+++ b/src/imap/imap-client.c
@@ -219,19 +219,21 @@ struct client *client_create(int fd_in, int fd_out,
 	return client;
 }
 
-int client_create_finish(struct client *client, const char **error_r)
+void client_create_finish_io(struct client *client)
 {
-	if (mail_namespaces_init(client->user, error_r) < 0)
-		return -1;
-	mail_namespaces_set_storage_callbacks(client->user->namespaces,
-					      &mail_storage_callbacks, client);
-
 	if (client->set->rawlog_dir[0] != '\0') {
 		(void)iostream_rawlog_create(client->set->rawlog_dir,
 					     &client->input, &client->output);
 	}
 	client->io = io_add_istream(client->input, client_input, client);
+}
 
+int client_create_finish(struct client *client, const char **error_r)
+{
+	if (mail_namespaces_init(client->user, error_r) < 0)
+		return -1;
+	mail_namespaces_set_storage_callbacks(client->user->namespaces,
+					      &mail_storage_callbacks, client);
 	client->v.init(client);
 	return 0;
 }
diff --git a/src/imap/imap-client.h b/src/imap/imap-client.h
index 153320919f..24c22b23b3 100644
--- a/src/imap/imap-client.h
+++ b/src/imap/imap-client.h
@@ -266,6 +266,7 @@ struct client *client_create(int fd_in, int fd_out,
 			     struct mail_storage_service_user *service_user,
 			     const struct imap_settings *set,
 			     const struct smtp_submit_settings *smtp_set);
+void client_create_finish_io(struct client *client);
 /* Finish creating the client. Returns 0 if ok, -1 if there's an error. */
 int client_create_finish(struct client *client, const char **error_r);
 void client_add_istream_prefix(struct client *client,
diff --git a/src/imap/imap-master-client.c b/src/imap/imap-master-client.c
index ffc7fb9535..181d6bb4f9 100644
--- a/src/imap/imap-master-client.c
+++ b/src/imap/imap-master-client.c
@@ -287,6 +287,7 @@ imap_master_client_input_args(struct connection *conn, const char *const *args,
 					  master_input.client_input->used);
 	}
 
+	client_create_finish_io(imap_client);
 	if (client_create_finish(imap_client, &error) < 0) {
 		event_add_str(event, "error", error);
 		e_error(event, "imap-master: %s", error);
diff --git a/src/imap/main.c b/src/imap/main.c
index e16bac02cd..5e8526d98f 100644
--- a/src/imap/main.c
+++ b/src/imap/main.c
@@ -352,11 +352,12 @@ static void main_stdio_run(const char *username)
 				 &request);
 	}
 
-	if (client_create_finish(client, &error) < 0)
-		i_fatal("%s", error);
+	client_create_finish_io(client);
 	client_send_login_reply(client->output,
 				str_c(client->capability_string),
 				client->user->username, &request);
+	if (client_create_finish(client, &error) < 0)
+		i_fatal("%s", error);
 	client_add_input_finalize(client);
 	/* client may be destroyed now */
 }
@@ -365,6 +366,7 @@ static void
 login_client_connected(const struct master_login_client *login_client,
 		       const char *username, const char *const *extra_fields)
 {
+#define MSG_BYE_INTERNAL_ERROR "* BYE "MAIL_ERRSTR_CRITICAL_MSG"\r\n"
 	struct mail_storage_service_input input;
 	struct client *client;
 	struct imap_login_request request;
@@ -390,6 +392,7 @@ login_client_connected(const struct master_login_client *login_client,
 		int fd = login_client->fd;
 		struct ostream *output =
 			o_stream_create_fd_autoclose(&fd, IO_BLOCK_SIZE);
+		i_zero(&request);
 		client_send_login_reply(output, NULL, NULL, &request);
 		o_stream_destroy(&output);
 
@@ -402,18 +405,27 @@ login_client_connected(const struct master_login_client *login_client,
 	client_add_input(client, login_client->data,
 			 login_client->auth_req.data_size, &request);
 
-	/* finish initializing the user (see comment in main()) */
+	/* The order here is important:
+	   1. Finish setting up rawlog, so all input/output is written there.
+	   2. Send tagged reply to login before any potentially long-running
+	      work (during which client could disconnect due to timeout).
+	   3. Finish initializing user, which can potentially take a long time.
+	*/
+	client_create_finish_io(client);
+	client_send_login_reply(client->output,
+				str_c(client->capability_string),
+				NULL, &request);
 	if (client_create_finish(client, &error) < 0) {
-		/* Even though client initialization failed, send the login
-		   OK reply so client doesn't think that the login failed. */
-		client_send_login_reply(client->output, NULL, NULL, &request);
+		if (write_full(login_client->fd, MSG_BYE_INTERNAL_ERROR,
+			       strlen(MSG_BYE_INTERNAL_ERROR)) < 0)
+			if (errno != EAGAIN && errno != EPIPE)
+				e_error(client->event,
+					"write_full(client) failed: %m");
+
 		e_error(client->event, "%s", error);
 		client_destroy(client, error);
 		return;
 	}
-	client_send_login_reply(client->output,
-				str_c(client->capability_string),
-				NULL, &request);
 
 	client_add_input_finalize(client);
 	/* client may be destroyed now */