changeset 8583:2ff2cac3578b HEAD

imap/pop3-login: Cleaned up proxying code. Don't disconnect client on proxy failures. Log proxy failures as errors.
author Timo Sirainen <tss@iki.fi>
date Sun, 21 Dec 2008 09:43:20 +0200
parents 467606dbabb7
children 0eac46c235e7
files src/imap-login/client-authenticate.c src/imap-login/client.c src/imap-login/client.h src/imap-login/imap-proxy.c src/imap-login/imap-proxy.h src/login-common/client-common.c src/login-common/client-common.h src/login-common/login-proxy.c src/login-common/login-proxy.h src/pop3-login/client-authenticate.c src/pop3-login/client.c src/pop3-login/client.h src/pop3-login/pop3-proxy.c
diffstat 13 files changed, 179 insertions(+), 135 deletions(-) [+]
line wrap: on
line diff
--- a/src/imap-login/client-authenticate.c	Fri Dec 19 18:08:09 2008 +0200
+++ b/src/imap-login/client-authenticate.c	Sun Dec 21 09:43:20 2008 +0200
@@ -91,7 +91,7 @@
 	client_input(client);
 }
 
-static void client_auth_failed(struct imap_client *client, bool nodelay)
+void client_auth_failed(struct imap_client *client, bool nodelay)
 {
 	unsigned int delay_msecs;
 
@@ -129,7 +129,7 @@
 	const char *master_user = NULL;
 	string_t *reply;
 	unsigned int port = 143;
-	bool proxy = FALSE, temp = FALSE, nologin = !success, proxy_self;
+	bool proxy = FALSE, temp = FALSE, nologin = !success;
 	bool authz_failure = FALSE;
 
 	*nodelay_r = FALSE;
@@ -167,9 +167,7 @@
 	if (destuser == NULL)
 		destuser = client->common.virtual_user;
 
-	proxy_self = proxy &&
-		login_proxy_is_ourself(&client->common, host, port, destuser);
-	if (proxy && !proxy_self) {
+	if (proxy) {
 		/* we want to proxy the connection to another server.
 		   don't do this unless authentication succeeded. with
 		   master user proxying we can get FAIL with proxy still set.
@@ -179,11 +177,11 @@
 			return FALSE;
 		if (imap_proxy_new(client, host, port, destuser, master_user,
 				   pass) < 0)
-			client_destroy_internal_failure(client);
+			client_auth_failed(client, TRUE);
 		return TRUE;
 	}
 
-	if (!proxy && host != NULL) {
+	if (host != NULL) {
 		/* IMAP referral
 
 		   [nologin] referral host=.. [port=..] [destuser=..]
@@ -213,18 +211,13 @@
 			client_destroy_success(client, "Login with referral");
 			return TRUE;
 		}
-	} else if (nologin || proxy_self) {
+	} else if (nologin) {
 		/* Authentication went ok, but for some reason user isn't
 		   allowed to log in. Shouldn't probably happen. */
-		if (proxy_self) {
-			client_syslog(&client->common,
-				      "Proxying loops to itself");
-		}
-
 		reply = t_str_new(128);
 		if (reason != NULL)
 			str_printfa(reply, "NO %s", reason);
-		else if (temp || proxy_self) {
+		else if (temp) {
 			str_append(reply, "NO ["IMAP_RESP_CODE_UNAVAILABLE"] "
 				   AUTH_TEMP_FAILED_MSG);
 		} else if (authz_failure) {
@@ -238,7 +231,7 @@
 		return FALSE;
 	}
 
-	i_assert(nologin || proxy_self);
+	i_assert(nologin);
 
 	if (!client->destroyed)
 		client_auth_failed(client, *nodelay_r);
--- a/src/imap-login/client.c	Fri Dec 19 18:08:09 2008 +0200
+++ b/src/imap-login/client.c	Sun Dec 21 09:43:20 2008 +0200
@@ -589,10 +589,8 @@
 	i_free_and_null(client->proxy_user);
 	i_free_and_null(client->proxy_master_user);
 
-	if (client->proxy != NULL) {
-		login_proxy_free(client->proxy);
-		client->proxy = NULL;
-	}
+	if (client->proxy != NULL)
+		login_proxy_free(&client->proxy);
 
 	if (client->common.proxy != NULL) {
 		ssl_proxy_free(client->common.proxy);
--- a/src/imap-login/client.h	Fri Dec 19 18:08:09 2008 +0200
+++ b/src/imap-login/client.h	Sun Dec 21 09:43:20 2008 +0200
@@ -53,5 +53,6 @@
 bool client_unref(struct imap_client *client);
 
 void client_set_auth_waiting(struct imap_client *client);
+void client_auth_failed(struct imap_client *client, bool nodelay);
 
 #endif
--- a/src/imap-login/imap-proxy.c	Fri Dec 19 18:08:09 2008 +0200
+++ b/src/imap-login/imap-proxy.c	Sun Dec 21 09:43:20 2008 +0200
@@ -9,9 +9,13 @@
 #include "str-sanitize.h"
 #include "safe-memset.h"
 #include "client.h"
+#include "imap-resp-code.h"
 #include "imap-quote.h"
 #include "imap-proxy.h"
 
+#define PROXY_FAILURE_MSG \
+	"NO ["IMAP_RESP_CODE_UNAVAILABLE"] "AUTH_TEMP_FAILED_MSG
+
 static bool imap_banner_has_capability(const char *line, const char *capability)
 {
 	unsigned int capability_len = strlen(capability);
@@ -49,10 +53,27 @@
 
 static void proxy_free_password(struct imap_client *client)
 {
+	if (client->proxy_password == NULL)
+		return;
+
 	safe_memset(client->proxy_password, 0, strlen(client->proxy_password));
 	i_free_and_null(client->proxy_password);
 }
 
+static void proxy_failed(struct imap_client *client, bool send_tagline)
+{
+	if (send_tagline)
+		client_send_tagline(client, PROXY_FAILURE_MSG);
+
+	login_proxy_free(&client->proxy);
+	proxy_free_password(client);
+	i_free_and_null(client->proxy_user);
+	i_free_and_null(client->proxy_master_user);
+
+	/* call this last - it may destroy the client */
+	client_auth_failed(client, TRUE);
+}
+
 static void get_plain_auth(struct imap_client *client, string_t *dest)
 {
 	string_t *str;
@@ -72,10 +93,9 @@
 	string_t *str;
 
 	if (strncmp(line, "* OK ", 5) != 0) {
-		client_syslog(&client->common, t_strdup_printf(
+		client_syslog_err(&client->common, t_strdup_printf(
 			"proxy: Remote returned invalid banner: %s",
 			str_sanitize(line, 160)));
-		client_destroy_internal_failure(client);
 		return -1;
 	}
 
@@ -116,7 +136,11 @@
 
 	if (!client->proxy_login_sent) {
 		/* this is a banner */
-		return proxy_input_banner(client, output, line);
+		if (proxy_input_banner(client, output, line) < 0) {
+			proxy_failed(client, TRUE);
+			return -1;
+		}
+		return 0;
 	} else if (*line == '+') {
 		/* AUTHENTICATE started. finish it. */
 		str = t_str_new(128);
@@ -200,17 +224,7 @@
 				str_append(str, line + 2);
 			i_info("%s", str_c(str));
 		}
-
-		/* allow client input again */
-		i_assert(client->io == NULL);
-		client->io = io_add(client->common.fd, IO_READ,
-				    client_input, client);
-
-		login_proxy_free(client->proxy);
-		client->proxy = NULL;
-
-		i_free_and_null(client->proxy_user);
-		i_free_and_null(client->proxy_master_user);
+		proxy_failed(client, FALSE);
 		return -1;
 	} else {
 		/* probably some untagged reply */
@@ -224,9 +238,8 @@
 	const char *line;
 
 	if (input == NULL) {
-		if (client->io != NULL) {
-			/* remote authentication failed, we're just
-			   freeing the proxy */
+		if (client->proxy == NULL) {
+			/* we're just freeing the proxy */
 			return;
 		}
 
@@ -236,8 +249,7 @@
 		}
 
 		/* failed for some reason, probably server disconnected */
-		client_send_line(client, "* BYE Temporary login failure.");
-		client_destroy_success(client, NULL);
+		proxy_failed(client, TRUE);
 		return;
 	}
 
@@ -245,14 +257,14 @@
 
 	switch (i_stream_read(input)) {
 	case -2:
-		/* buffer full */
-		client_syslog(&client->common,
-			      "proxy: Remote input buffer full");
-		client_destroy_internal_failure(client);
+		client_syslog_err(&client->common,
+				  "proxy: Remote input buffer full");
+		proxy_failed(client, TRUE);
 		return;
 	case -1:
-		/* disconnected */
-		client_destroy_success(client, "Proxy: Remote disconnected");
+		client_syslog_err(&client->common,
+				  "proxy: Remote disconnected");
+		proxy_failed(client, TRUE);
 		return;
 	}
 
@@ -270,7 +282,8 @@
 	i_assert(!client->destroyed);
 
 	if (password == NULL) {
-		client_syslog(&client->common, "proxy: password not given");
+		client_syslog_err(&client->common, "proxy: password not given");
+		client_send_tagline(client, PROXY_FAILURE_MSG);
 		return -1;
 	}
 
@@ -282,11 +295,18 @@
 		   connection and killed us. */
 		return -1;
 	}
+	if (login_proxy_is_ourself(&client->common, host, port, user)) {
+		client_syslog_err(&client->common, "Proxying loops to itself");
+		client_send_tagline(client, PROXY_FAILURE_MSG);
+		return -1;
+	}
 
 	client->proxy = login_proxy_new(&client->common, host, port,
 					proxy_input, client);
-	if (client->proxy == NULL)
+	if (client->proxy == NULL) {
+		client_send_tagline(client, PROXY_FAILURE_MSG);
 		return -1;
+	}
 
 	client->proxy_login_sent = FALSE;
 	client->proxy_user = i_strdup(user);
@@ -296,6 +316,5 @@
 	/* disable input until authentication is finished */
 	if (client->io != NULL)
 		io_remove(&client->io);
-
 	return 0;
 }
--- a/src/imap-login/imap-proxy.h	Fri Dec 19 18:08:09 2008 +0200
+++ b/src/imap-login/imap-proxy.h	Sun Dec 21 09:43:20 2008 +0200
@@ -3,7 +3,7 @@
 
 #include "login-proxy.h"
 
-int imap_proxy_new(struct imap_client *client, const char *host,
+int imap_proxy_new(struct imap_client *client, const char *hosts,
 		   unsigned int port, const char *user, const char *master_user,
 		   const char *password);
 
--- a/src/login-common/client-common.c	Fri Dec 19 18:08:09 2008 +0200
+++ b/src/login-common/client-common.c	Sun Dec 21 09:43:20 2008 +0200
@@ -109,7 +109,8 @@
 	return FALSE;
 }
 
-static void client_syslog_real(struct client *client, const char *msg)
+static const char *
+client_get_log_str(struct client *client, const char *msg)
 {
 	static struct var_expand_table static_tab[3] = {
 		{ 's', NULL, NULL },
@@ -147,13 +148,20 @@
 	str_truncate(str, 0);
 
 	var_expand(str, log_format, tab);
-	i_info("%s", str_c(str));
+	return str_c(str);
 }
 
 void client_syslog(struct client *client, const char *msg)
 {
 	T_BEGIN {
-		client_syslog_real(client, msg);
+		i_info("%s", client_get_log_str(client, msg));
+	} T_END;
+}
+
+void client_syslog_err(struct client *client, const char *msg)
+{
+	T_BEGIN {
+		i_error("%s", client_get_log_str(client, msg));
 	} T_END;
 }
 
--- a/src/login-common/client-common.h	Fri Dec 19 18:08:09 2008 +0200
+++ b/src/login-common/client-common.h	Sun Dec 21 09:43:20 2008 +0200
@@ -54,6 +54,7 @@
 unsigned int clients_get_count(void) ATTR_PURE;
 
 void client_syslog(struct client *client, const char *msg);
+void client_syslog_err(struct client *client, const char *msg);
 const char *client_get_extra_disconnect_reason(struct client *client);
 bool client_is_trusted(struct client *client);
 
--- a/src/login-common/login-proxy.c	Fri Dec 19 18:08:09 2008 +0200
+++ b/src/login-common/login-proxy.c	Sun Dec 21 09:43:20 2008 +0200
@@ -48,7 +48,7 @@
 
 	ret = net_receive(proxy->server_fd, buf, sizeof(buf));
 	if (ret < 0 || o_stream_send(proxy->client_output, buf, ret) != ret)
-                login_proxy_free(proxy);
+                login_proxy_free(&proxy);
 }
 
 static void proxy_client_input(struct login_proxy *proxy)
@@ -66,13 +66,13 @@
 
 	ret = net_receive(proxy->client_fd, buf, sizeof(buf));
 	if (ret < 0 || o_stream_send(proxy->server_output, buf, ret) != ret)
-                login_proxy_free(proxy);
+                login_proxy_free(&proxy);
 }
 
 static int server_output(struct login_proxy *proxy)
 {
 	if (o_stream_flush(proxy->server_output) < 0) {
-                login_proxy_free(proxy);
+                login_proxy_free(&proxy);
 		return 1;
 	}
 
@@ -90,7 +90,7 @@
 static int proxy_client_output(struct login_proxy *proxy)
 {
 	if (o_stream_flush(proxy->client_output) < 0) {
-                login_proxy_free(proxy);
+                login_proxy_free(&proxy);
 		return 1;
 	}
 
@@ -119,7 +119,7 @@
 	if (err != 0) {
 		i_error("proxy: connect(%s, %u) failed: %s",
 			proxy->host, proxy->port, strerror(err));
-                login_proxy_free(proxy);
+                login_proxy_free(&proxy);
 		return;
 	}
 
@@ -178,10 +178,13 @@
 	return proxy;
 }
 
-void login_proxy_free(struct login_proxy *proxy)
+void login_proxy_free(struct login_proxy **_proxy)
 {
+	struct login_proxy *proxy = *_proxy;
 	const char *ipstr;
 
+	*_proxy = NULL;
+
 	if (proxy->destroying)
 		return;
 	proxy->destroying = TRUE;
@@ -295,6 +298,10 @@
 
 void login_proxy_deinit(void)
 {
-	while (login_proxies != NULL)
-		login_proxy_free(login_proxies);
+	struct login_proxy *proxy;
+
+	while (login_proxies != NULL) {
+		proxy = login_proxies;
+		login_proxy_free(&proxy);
+	}
 }
--- a/src/login-common/login-proxy.h	Fri Dec 19 18:08:09 2008 +0200
+++ b/src/login-common/login-proxy.h	Sun Dec 21 09:43:20 2008 +0200
@@ -24,7 +24,7 @@
 		(proxy_callback_t *)callback, context)
 #endif
 /* Free the proxy. This should be called if authentication fails. */
-void login_proxy_free(struct login_proxy *proxy);
+void login_proxy_free(struct login_proxy **proxy);
 
 /* Return TRUE if host/port/destuser combination points to same as current
    connection. */
--- a/src/pop3-login/client-authenticate.c	Fri Dec 19 18:08:09 2008 +0200
+++ b/src/pop3-login/client-authenticate.c	Sun Dec 21 09:43:20 2008 +0200
@@ -93,7 +93,7 @@
 	client_input(client);
 }
 
-static void client_auth_failed(struct pop3_client *client, bool nodelay)
+void client_auth_failed(struct pop3_client *client, bool nodelay)
 {
 	unsigned int delay_msecs;
 
@@ -164,8 +164,7 @@
 	if (destuser == NULL)
 		destuser = client->common.virtual_user;
 
-	if (proxy &&
-	    !login_proxy_is_ourself(&client->common, host, port, destuser)) {
+	if (proxy) {
 		/* we want to proxy the connection to another server.
 		   don't do this unless authentication succeeded. with
 		   master user proxying we can get FAIL with proxy still set.
@@ -175,7 +174,7 @@
 			return FALSE;
 		if (pop3_proxy_new(client, host, port, destuser, master_user,
 				   pass) < 0)
-			client_destroy_internal_failure(client);
+			client_auth_failed(client, TRUE);
 		return TRUE;
 	}
 
@@ -187,7 +186,7 @@
 	if (reason != NULL)
 		str_append(reply, reason);
 	else if (temp)
-		str_append(reply, AUTH_TEMP_FAILED_MSG);
+		str_append(reply, "[IN-USE] "AUTH_TEMP_FAILED_MSG);
 	else
 		str_append(reply, AUTH_FAILED_MSG);
 
--- a/src/pop3-login/client.c	Fri Dec 19 18:08:09 2008 +0200
+++ b/src/pop3-login/client.c	Sun Dec 21 09:43:20 2008 +0200
@@ -396,10 +396,8 @@
 	i_free(client->proxy_user);
 	client->proxy_user = NULL;
 
-	if (client->proxy != NULL) {
-		login_proxy_free(client->proxy);
-		client->proxy = NULL;
-	}
+	if (client->proxy != NULL)
+		login_proxy_free(&client->proxy);
 
 	if (client->common.proxy != NULL) {
 		ssl_proxy_free(client->common.proxy);
--- a/src/pop3-login/client.h	Fri Dec 19 18:08:09 2008 +0200
+++ b/src/pop3-login/client.h	Sun Dec 21 09:43:20 2008 +0200
@@ -48,4 +48,6 @@
 void client_ref(struct pop3_client *client);
 bool client_unref(struct pop3_client *client);
 
+void client_auth_failed(struct pop3_client *client, bool nodelay);
+
 #endif
--- a/src/pop3-login/pop3-proxy.c	Fri Dec 19 18:08:09 2008 +0200
+++ b/src/pop3-login/pop3-proxy.c	Sun Dec 21 09:43:20 2008 +0200
@@ -11,6 +11,31 @@
 #include "client.h"
 #include "pop3-proxy.h"
 
+#define PROXY_FAILURE_MSG "-ERR [IN-USE] "AUTH_TEMP_FAILED_MSG
+
+static void proxy_free_password(struct pop3_client *client)
+{
+	if (client->proxy_password == NULL)
+		return;
+
+	safe_memset(client->proxy_password, 0, strlen(client->proxy_password));
+	i_free_and_null(client->proxy_password);
+}
+
+static void proxy_failed(struct pop3_client *client, bool send_line)
+{
+	if (send_line)
+		client_send_line(client, PROXY_FAILURE_MSG);
+
+	login_proxy_free(&client->proxy);
+	proxy_free_password(client);
+	i_free_and_null(client->proxy_user);
+	i_free_and_null(client->proxy_master_user);
+
+	/* call this last - it may destroy the client */
+	client_auth_failed(client, TRUE);
+}
+
 static void get_plain_auth(struct pop3_client *client, string_t *dest)
 {
 	string_t *str;
@@ -24,59 +49,22 @@
 	base64_encode(str_data(str), str_len(str), dest);
 }
 
-static void proxy_input(struct istream *input, struct ostream *output,
-			struct pop3_client *client)
+static int proxy_input_line(struct pop3_client *client,
+			    struct ostream *output, const char *line)
 {
 	string_t *str;
-	const char *line;
-
-	if (input == NULL) {
-		if (client->io != NULL) {
-			/* remote authentication failed, we're just
-			   freeing the proxy */
-			return;
-		}
-
-		if (client->destroyed) {
-			/* we came here from client_destroy() */
-			return;
-		}
-
-		/* failed for some reason, probably server disconnected */
-		client_send_line(client,
-				 "-ERR [IN-USE] Temporary login failure.");
-		client_destroy_success(client, NULL);
-		return;
-	}
 
 	i_assert(!client->destroyed);
 
-	switch (i_stream_read(input)) {
-	case -2:
-		/* buffer full */
-		client_syslog(&client->common,
-			      "proxy: Remote input buffer full");
-		client_destroy_internal_failure(client);
-		return;
-	case -1:
-		/* disconnected */
-		client_destroy_success(client, "Proxy: Remote disconnected");
-		return;
-	}
-
-	line = i_stream_next_line(input);
-	if (line == NULL)
-		return;
-
 	switch (client->proxy_state) {
 	case 0:
 		/* this is a banner */
 		if (strncmp(line, "+OK", 3) != 0) {
-			client_syslog(&client->common, t_strdup_printf(
+			client_syslog_err(&client->common, t_strdup_printf(
 				"proxy: Remote returned invalid banner: %s",
 				str_sanitize(line, 160)));
-			client_destroy_internal_failure(client);
-			return;
+			proxy_failed(client, TRUE);
+			return -1;
 		}
 
 		str = t_str_new(128);
@@ -92,7 +80,7 @@
 		(void)o_stream_send(output, str_data(str), str_len(str));
 
 		client->proxy_state++;
-		return;
+		return 0;
 	case 1:
 		str = t_str_new(128);
 		if (client->proxy_master_user == NULL) {
@@ -110,15 +98,10 @@
 			get_plain_auth(client, str);
 			str_append(str, "\r\n");
 		}
-		(void)o_stream_send(output, str_data(str),
-				    str_len(str));
-
-		safe_memset(client->proxy_password, 0,
-			    strlen(client->proxy_password));
-		i_free_and_null(client->proxy_password);
-
+		(void)o_stream_send(output, str_data(str), str_len(str));
+		proxy_free_password(client);
 		client->proxy_state++;
-		return;
+		return 0;
 	case 2:
 		if (strncmp(line, "+OK", 3) != 0)
 			break;
@@ -151,7 +134,7 @@
 		client->output = NULL;
 		client->common.fd = -1;
 		client_destroy_success(client, str_c(str));
-		return;
+		return 0;
 	}
 
 	/* Login failed. Pass through the error message to client
@@ -184,23 +167,50 @@
 			str_append(str, line);
 		i_info("%s", str_c(str));
 	}
+	proxy_failed(client, FALSE);
+	return -1;
+}
 
-	/* allow client input again */
-	i_assert(client->io == NULL);
-	client->io = io_add(client->common.fd, IO_READ,
-			    client_input, client);
+static void proxy_input(struct istream *input, struct ostream *output,
+			struct pop3_client *client)
+{
+	const char *line;
 
-	login_proxy_free(client->proxy);
-	client->proxy = NULL;
+	if (input == NULL) {
+		if (client->proxy == NULL) {
+			/* we're just freeing the proxy */
+			return;
+		}
 
-	if (client->proxy_password != NULL) {
-		safe_memset(client->proxy_password, 0,
-			    strlen(client->proxy_password));
-		i_free_and_null(client->proxy_password);
+		if (client->destroyed) {
+			/* we came here from client_destroy() */
+			return;
+		}
+
+		/* failed for some reason, probably server disconnected */
+		proxy_failed(client, TRUE);
+		return;
 	}
 
-	i_free_and_null(client->proxy_user);
-	i_free_and_null(client->proxy_master_user);
+	i_assert(!client->destroyed);
+
+	switch (i_stream_read(input)) {
+	case -2:
+		client_syslog_err(&client->common,
+				  "proxy: Remote input buffer full");
+		proxy_failed(client, TRUE);
+		return;
+	case -1:
+		client_syslog_err(&client->common,
+				  "proxy: Remote disconnected");
+		proxy_failed(client, TRUE);
+		return;
+	}
+
+	while ((line = i_stream_next_line(input)) != NULL) {
+		if (proxy_input_line(client, output, line) < 0)
+			break;
+	}
 }
 
 int pop3_proxy_new(struct pop3_client *client, const char *host,
@@ -211,7 +221,8 @@
 	i_assert(!client->destroyed);
 
 	if (password == NULL) {
-		client_syslog(&client->common, "proxy: password not given");
+		client_syslog_err(&client->common, "proxy: password not given");
+		client_send_line(client, PROXY_FAILURE_MSG);
 		return -1;
 	}
 
@@ -223,11 +234,18 @@
 		   connection and killed us. */
 		return -1;
 	}
+	if (login_proxy_is_ourself(&client->common, host, port, user)) {
+		client_syslog_err(&client->common, "Proxying loops to itself");
+		client_send_line(client, PROXY_FAILURE_MSG);
+		return -1;
+	}
 
 	client->proxy = login_proxy_new(&client->common, host, port,
 					proxy_input, client);
-	if (client->proxy == NULL)
+	if (client->proxy == NULL) {
+		client_send_line(client, PROXY_FAILURE_MSG);
 		return -1;
+	}
 
 	client->proxy_state = 0;
 	client->proxy_user = i_strdup(user);