From: Timo Sirainen Date: Sun, 21 Dec 2008 07:43:20 +0000 (+0200) Subject: imap/pop3-login: Cleaned up proxying code. Don't disconnect client on proxy failures. X-Git-Tag: 1.2.beta1~160 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=8372fc7efb6d64dff2e5f55fb4a3822c56869cfe;p=thirdparty%2Fdovecot%2Fcore.git imap/pop3-login: Cleaned up proxying code. Don't disconnect client on proxy failures. Log proxy failures as errors. --HG-- branch : HEAD --- diff --git a/src/imap-login/client-authenticate.c b/src/imap-login/client-authenticate.c index dbbd40d83e..fc22a8e7f4 100644 --- a/src/imap-login/client-authenticate.c +++ b/src/imap-login/client-authenticate.c @@ -91,7 +91,7 @@ static void client_authfail_delay_timeout(struct imap_client *client) 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 @@ static bool client_handle_args(struct imap_client *client, 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 @@ static bool client_handle_args(struct imap_client *client, 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 @@ static bool client_handle_args(struct imap_client *client, 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 @@ static bool client_handle_args(struct imap_client *client, 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 @@ static bool client_handle_args(struct imap_client *client, return FALSE; } - i_assert(nologin || proxy_self); + i_assert(nologin); if (!client->destroyed) client_auth_failed(client, *nodelay_r); diff --git a/src/imap-login/client.c b/src/imap-login/client.c index eb6950586f..46d6f622eb 100644 --- a/src/imap-login/client.c +++ b/src/imap-login/client.c @@ -589,10 +589,8 @@ void client_destroy(struct imap_client *client, const char *reason) 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); diff --git a/src/imap-login/client.h b/src/imap-login/client.h index e2f680b2c7..770cd48810 100644 --- a/src/imap-login/client.h +++ b/src/imap-login/client.h @@ -53,5 +53,6 @@ void client_ref(struct imap_client *client); 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 diff --git a/src/imap-login/imap-proxy.c b/src/imap-login/imap-proxy.c index e8330d68fd..8b7395697c 100644 --- a/src/imap-login/imap-proxy.c +++ b/src/imap-login/imap-proxy.c @@ -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_write_id(struct imap_client *client, string_t *str) 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 @@ static int proxy_input_banner(struct imap_client *client, 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 @@ static int proxy_input_line(struct imap_client *client, 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 @@ static int proxy_input_line(struct imap_client *client, 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 @@ static void proxy_input(struct istream *input, struct ostream *output, 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 @@ static void proxy_input(struct istream *input, struct ostream *output, } /* 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 @@ static void proxy_input(struct istream *input, struct ostream *output, 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 @@ int imap_proxy_new(struct imap_client *client, const char *host, 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 @@ int imap_proxy_new(struct imap_client *client, const char *host, 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 @@ int imap_proxy_new(struct imap_client *client, const char *host, /* disable input until authentication is finished */ if (client->io != NULL) io_remove(&client->io); - return 0; } diff --git a/src/imap-login/imap-proxy.h b/src/imap-login/imap-proxy.h index f9617dd5a5..b27b7b867c 100644 --- a/src/imap-login/imap-proxy.h +++ b/src/imap-login/imap-proxy.h @@ -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); diff --git a/src/login-common/client-common.c b/src/login-common/client-common.c index 432cb6e7da..752073f4f3 100644 --- a/src/login-common/client-common.c +++ b/src/login-common/client-common.c @@ -109,7 +109,8 @@ static bool have_key(const struct var_expand_table *table, const char *str) 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 @@ static void client_syslog_real(struct client *client, const char *msg) 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; } diff --git a/src/login-common/client-common.h b/src/login-common/client-common.h index fc1cf558a1..90e08c60a4 100644 --- a/src/login-common/client-common.h +++ b/src/login-common/client-common.h @@ -54,6 +54,7 @@ void client_unlink(struct client *client); 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); diff --git a/src/login-common/login-proxy.c b/src/login-common/login-proxy.c index 1130a8b953..7a2aabccbc 100644 --- a/src/login-common/login-proxy.c +++ b/src/login-common/login-proxy.c @@ -48,7 +48,7 @@ static void server_input(struct login_proxy *proxy) 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 @@ static void proxy_client_input(struct login_proxy *proxy) 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 server_output(struct login_proxy *proxy) 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 @@ static void proxy_wait_connect(struct login_proxy *proxy) 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 @@ login_proxy_new(struct client *client, const char *host, unsigned int port, 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_detach(struct login_proxy *proxy, struct istream *client_input, 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); + } } diff --git a/src/login-common/login-proxy.h b/src/login-common/login-proxy.h index 3517b0bcc4..74d5d5e993 100644 --- a/src/login-common/login-proxy.h +++ b/src/login-common/login-proxy.h @@ -24,7 +24,7 @@ login_proxy_new(struct client *client, const char *host, unsigned int port, (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. */ diff --git a/src/pop3-login/client-authenticate.c b/src/pop3-login/client-authenticate.c index 019182e2c0..a09d3042c8 100644 --- a/src/pop3-login/client-authenticate.c +++ b/src/pop3-login/client-authenticate.c @@ -93,7 +93,7 @@ static void client_authfail_delay_timeout(struct pop3_client *client) 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 @@ static bool client_handle_args(struct pop3_client *client, 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 @@ static bool client_handle_args(struct pop3_client *client, 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 @@ static bool client_handle_args(struct pop3_client *client, 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); diff --git a/src/pop3-login/client.c b/src/pop3-login/client.c index efe23ae3fb..7215b731e0 100644 --- a/src/pop3-login/client.c +++ b/src/pop3-login/client.c @@ -396,10 +396,8 @@ void client_destroy(struct pop3_client *client, const char *reason) 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); diff --git a/src/pop3-login/client.h b/src/pop3-login/client.h index 2826b84b85..750eb03c3b 100644 --- a/src/pop3-login/client.h +++ b/src/pop3-login/client.h @@ -48,4 +48,6 @@ void client_input(struct pop3_client *client); 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 diff --git a/src/pop3-login/pop3-proxy.c b/src/pop3-login/pop3-proxy.c index 40a55f9606..551cec69ef 100644 --- a/src/pop3-login/pop3-proxy.c +++ b/src/pop3-login/pop3-proxy.c @@ -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 @@ static void get_plain_auth(struct pop3_client *client, string_t *dest) 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 @@ static void proxy_input(struct istream *input, struct ostream *output, (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 @@ static void proxy_input(struct istream *input, struct ostream *output, 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 @@ static void proxy_input(struct istream *input, struct ostream *output, 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 @@ static void proxy_input(struct istream *input, struct ostream *output, str_append(str, line); i_info("%s", str_c(str)); } + proxy_failed(client, FALSE); + return -1; +} + +static void proxy_input(struct istream *input, struct ostream *output, + struct pop3_client *client) +{ + const char *line; + + if (input == NULL) { + if (client->proxy == NULL) { + /* we're just freeing the proxy */ + return; + } + + if (client->destroyed) { + /* we came here from client_destroy() */ + return; + } - /* allow client input again */ - i_assert(client->io == NULL); - client->io = io_add(client->common.fd, IO_READ, - client_input, client); + /* failed for some reason, probably server disconnected */ + proxy_failed(client, TRUE); + return; + } - login_proxy_free(client->proxy); - client->proxy = NULL; + i_assert(!client->destroyed); - if (client->proxy_password != NULL) { - safe_memset(client->proxy_password, 0, - strlen(client->proxy_password)); - i_free_and_null(client->proxy_password); + 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; } - i_free_and_null(client->proxy_user); - i_free_and_null(client->proxy_master_user); + 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 @@ int pop3_proxy_new(struct pop3_client *client, const char *host, 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 @@ int pop3_proxy_new(struct pop3_client *client, const char *host, 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);