From: Timo Sirainen <tss@iki.fi>
Date: Tue, 7 Apr 2009 20:41:23 +0000 (-0400)
Subject: imap-login: Use [resp-codes] to figure out when to replace remote's auth failed messa... 
X-Git-Tag: 2.0.alpha1~1014
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e3a838c80f54f024115fade93c6c87a0998f1fab;p=thirdparty%2Fdovecot%2Fcore.git

imap-login: Use [resp-codes] to figure out when to replace remote's auth failed message with ours.

--HG--
branch : HEAD
---

diff --git a/src/imap-login/client-authenticate.c b/src/imap-login/client-authenticate.c
index 995b3abe71..47b95069c8 100644
--- a/src/imap-login/client-authenticate.c
+++ b/src/imap-login/client-authenticate.c
@@ -21,9 +21,6 @@
 #define AUTH_FAILURE_DELAY_INCREASE_MSECS 5000
 
 #define IMAP_SERVICE_NAME "imap"
-#define IMAP_AUTH_FAILED_MSG "["IMAP_RESP_CODE_AUTHFAILED"] "AUTH_FAILED_MSG
-#define IMAP_AUTHZ_FAILED_MSG \
-	"["IMAP_RESP_CODE_AUTHZFAILED"] Authorization failed"
 
 const char *client_authenticate_get_capabilities(bool secured)
 {
diff --git a/src/imap-login/client-authenticate.h b/src/imap-login/client-authenticate.h
index c18359a51a..9932aefa92 100644
--- a/src/imap-login/client-authenticate.h
+++ b/src/imap-login/client-authenticate.h
@@ -1,6 +1,13 @@
 #ifndef CLIENT_AUTHENTICATE_H
 #define CLIENT_AUTHENTICATE_H
 
+struct imap_arg;
+
+#define IMAP_AUTH_FAILED_MSG \
+	"["IMAP_RESP_CODE_AUTHFAILED"] "AUTH_FAILED_MSG
+#define IMAP_AUTHZ_FAILED_MSG \
+	"["IMAP_RESP_CODE_AUTHZFAILED"] Authorization failed"
+
 const char *client_authenticate_get_capabilities(bool secured);
 
 int cmd_login(struct imap_client *client, const struct imap_arg *args);
diff --git a/src/imap-login/imap-proxy.c b/src/imap-login/imap-proxy.c
index bfc793f5d8..2b7667d443 100644
--- a/src/imap-login/imap-proxy.c
+++ b/src/imap-login/imap-proxy.c
@@ -9,6 +9,7 @@
 #include "str-sanitize.h"
 #include "safe-memset.h"
 #include "client.h"
+#include "client-authenticate.h"
 #include "imap-resp-code.h"
 #include "imap-quote.h"
 #include "imap-proxy.h"
@@ -186,21 +187,7 @@ static int proxy_input_line(struct imap_client *client,
 		client_destroy_success(client, str_c(str));
 		return 1;
 	} else if (strncmp(line, "L ", 2) == 0) {
-		/* If the backend server isn't Dovecot, the error message may
-		   be different from Dovecot's "user doesn't exist" error. This
-		   would allow an attacker to find out what users exist in the
-		   system.
-
-		   The optimal way to handle this would be to replace the
-		   backend's "password failed" error message with Dovecot's
-		   AUTH_FAILED_MSG, but this would require a new setting and
-		   the sysadmin to actually bother setting it properly.
-
-		   So for now we'll just forward the error message. This
-		   shouldn't be a real problem since of course everyone will
-		   be using only Dovecot as their backend :) */
-		client_send_tagline(client, line + 2);
-
+		line += 2;
 		if (login_settings->verbose_auth) {
 			str = t_str_new(128);
 			str_printfa(str, "proxy(%s): Login failed to %s:%u",
@@ -218,12 +205,35 @@ static int proxy_input_line(struct imap_client *client,
 					    client->proxy_master_user);
 			}
 			str_append(str, ": ");
-			if (strncasecmp(line + 2, "NO ", 3) == 0)
-				str_append(str, line + 2 + 3);
+			if (strncasecmp(line, "NO ", 3) == 0)
+				str_append(str, line + 3);
 			else
-				str_append(str, line + 2);
+				str_append(str, line);
 			i_info("%s", str_c(str));
 		}
+#define STR_NO_IMAP_RESP_CODE_AUTHFAILED "NO ["IMAP_RESP_CODE_AUTHFAILED"]"
+		if (strncmp(line, STR_NO_IMAP_RESP_CODE_AUTHFAILED,
+			    strlen(STR_NO_IMAP_RESP_CODE_AUTHFAILED)) == 0) {
+			/* the remote sent a generic "authentication failed"
+			   error. replace it with our one, so that in case
+			   the remote is sending a different error message
+			   an attacker can't find out what users exist in
+			   the system. */
+			line = "NO "IMAP_AUTH_FAILED_MSG;
+		} else if (strncmp(line, "NO [", 4) == 0) {
+			/* remote sent some other resp-code. forward it. */
+		} else {
+			/* there was no [resp-code], so remote isn't Dovecot
+			   v1.2+. we could either forward the line as-is and
+			   leak information about what users exist in this
+			   system, or we could hide other errors than password
+			   failures. since other errors are pretty rare,
+			   it's safer to just hide them. they're still
+			   available in logs though. */
+			line = "NO "IMAP_AUTH_FAILED_MSG;
+		}
+		client_send_tagline(client, line);
+
 		proxy_failed(client, FALSE);
 		return -1;
 	} else {
diff --git a/src/pop3-login/pop3-proxy.c b/src/pop3-login/pop3-proxy.c
index 8c2741890f..9cb77e2026 100644
--- a/src/pop3-login/pop3-proxy.c
+++ b/src/pop3-login/pop3-proxy.c
@@ -137,8 +137,21 @@ static int proxy_input_line(struct pop3_client *client,
 		return 1;
 	}
 
-	/* Login failed. Pass through the error message to client
-	   (see imap-proxy code for potential problems with this) */
+	/* Login failed. Pass through the error message to client.
+
+	   If the backend server isn't Dovecot, the error message may
+	   be different from Dovecot's "user doesn't exist" error. This
+	   would allow an attacker to find out what users exist in the
+	   system.
+
+	   The optimal way to handle this would be to replace the
+	   backend's "password failed" error message with Dovecot's
+	   AUTH_FAILED_MSG, but this would require a new setting and
+	   the sysadmin to actually bother setting it properly.
+
+	   So for now we'll just forward the error message. This
+	   shouldn't be a real problem since of course everyone will
+	   be using only Dovecot as their backend :) */
 	if (strncmp(line, "-ERR ", 5) != 0)
 		client_send_line(client, "-ERR "AUTH_FAILED_MSG);
 	else