dovecot-2.1: auth: Don't allow auth clients to set internal auth...

[dovecot at dovecot.org]

details:   http://hg.dovecot.org/dovecot-2.1/rev/9a6aa717bc46
changeset: 13728:9a6aa717bc46
user:      Timo Sirainen <tss at iki.fi>
date:      Fri Nov 18 22:07:16 2011 +0200
description:
auth: Don't allow auth clients to set internal auth request fields.
This could have allowed attacker to bypass authentication if login process
was first successfully attacked to allow arbitrary code execution.

diffstat:

 src/auth/auth-master-connection.c |   2 +-
 src/auth/auth-request-handler.c   |   2 +-
 src/auth/auth-request.c           |  72 ++++++++++++++++++++++++++------------
 src/auth/auth-request.h           |   4 ++
 4 files changed, 55 insertions(+), 25 deletions(-)

diffs (131 lines):

diff -r ba1f99456742 -r 9a6aa717bc46 src/auth/auth-master-connection.c
--- a/src/auth/auth-master-connection.c	Fri Nov 18 21:37:34 2011 +0200
+++ b/src/auth/auth-master-connection.c	Fri Nov 18 22:07:16 2011 +0200
@@ -174,7 +174,7 @@
 			arg++;
 		}
 
-		(void)auth_request_import(auth_request, name, arg);
+		(void)auth_request_import_info(auth_request, name, arg);
 	}
 
 	if (auth_request->service == NULL) {
diff -r ba1f99456742 -r 9a6aa717bc46 src/auth/auth-request-handler.c
--- a/src/auth/auth-request-handler.c	Fri Nov 18 21:37:34 2011 +0200
+++ b/src/auth/auth-request-handler.c	Fri Nov 18 22:07:16 2011 +0200
@@ -433,7 +433,7 @@
 			arg++;
 		}
 
-		if (auth_request_import(request, name, arg))
+		if (auth_request_import_auth(request, name, arg))
 			;
 		else if (strcmp(name, "resp") == 0) {
 			initial_resp = arg;
diff -r ba1f99456742 -r 9a6aa717bc46 src/auth/auth-request.c
--- a/src/auth/auth-request.c	Fri Nov 18 21:37:34 2011 +0200
+++ b/src/auth/auth-request.c	Fri Nov 18 22:07:16 2011 +0200
@@ -207,25 +207,11 @@
 		auth_stream_reply_add(reply, "mech", request->mech_name);
 }
 
-bool auth_request_import(struct auth_request *request,
-			 const char *key, const char *value)
+bool auth_request_import_info(struct auth_request *request,
+			      const char *key, const char *value)
 {
-	if (strcmp(key, "user") == 0)
-		request->user = p_strdup(request->pool, value);
-	else if (strcmp(key, "master_user") == 0)
-		request->master_user = p_strdup(request->pool, value);
-	else if (strcmp(key, "original_username") == 0)
-		request->original_username = p_strdup(request->pool, value);
-	else if (strcmp(key, "requested_login_user") == 0)
-		request->requested_login_user = p_strdup(request->pool, value);
-	else if (strcmp(key, "cert_username") == 0) {
-		if (request->set->ssl_username_from_cert) {
-			/* get username from SSL certificate. it overrides
-			   the username given by the auth mechanism. */
-			request->user = p_strdup(request->pool, value);
-			request->cert_username = TRUE;
-		}
-	} else if (strcmp(key, "service") == 0)
+	/* authentication and user lookups may set these */
+	if (strcmp(key, "service") == 0)
 		request->service = p_strdup(request->pool, value);
 	else if (strcmp(key, "lip") == 0)
 		net_addr2ip(value, &request->local_ip);
@@ -235,14 +221,54 @@
 		request->local_port = atoi(value);
 	else if (strcmp(key, "rport") == 0)
 		request->remote_port = atoi(value);
-	else if (strcmp(key, "secured") == 0)
+	else
+		return FALSE;
+	return TRUE;
+}
+
+bool auth_request_import_auth(struct auth_request *request,
+			      const char *key, const char *value)
+{
+	if (auth_request_import_info(request, key, value))
+		return TRUE;
+
+	/* auth client may set these */
+	if (strcmp(key, "secured") == 0)
 		request->secured = TRUE;
+	else if (strcmp(key, "no-penalty") == 0)
+		request->no_penalty = TRUE;
+	else if (strcmp(key, "valid-client-cert") == 0)
+		request->valid_client_cert = TRUE;
+	else if (strcmp(key, "cert_username") == 0) {
+		if (request->set->ssl_username_from_cert) {
+			/* get username from SSL certificate. it overrides
+			   the username given by the auth mechanism. */
+			request->user = p_strdup(request->pool, value);
+			request->cert_username = TRUE;
+		}
+	} else {
+		return FALSE;
+	}
+	return TRUE;
+}
+
+bool auth_request_import(struct auth_request *request,
+			 const char *key, const char *value)
+{
+	if (auth_request_import_auth(request, key, value))
+		return TRUE;
+
+	/* for communication between auth master and worker processes */
+	if (strcmp(key, "user") == 0)
+		request->user = p_strdup(request->pool, value);
+	else if (strcmp(key, "master_user") == 0)
+		request->master_user = p_strdup(request->pool, value);
+	else if (strcmp(key, "original_username") == 0)
+		request->original_username = p_strdup(request->pool, value);
+	else if (strcmp(key, "requested_login_user") == 0)
+		request->requested_login_user = p_strdup(request->pool, value);
 	else if (strcmp(key, "nologin") == 0)
 		request->no_login = TRUE;
-	else if (strcmp(key, "valid-client-cert") == 0)
-		request->valid_client_cert = TRUE;
-	else if (strcmp(key, "no-penalty") == 0)
-		request->no_penalty = TRUE;
 	else if (strcmp(key, "successful") == 0)
 		request->successful = TRUE;
 	else if (strcmp(key, "skip_password_check") == 0) {
diff -r ba1f99456742 -r 9a6aa717bc46 src/auth/auth-request.h
--- a/src/auth/auth-request.h	Fri Nov 18 21:37:34 2011 +0200
+++ b/src/auth/auth-request.h	Fri Nov 18 22:07:16 2011 +0200
@@ -139,6 +139,10 @@
 			 struct auth_stream_reply *reply);
 bool auth_request_import(struct auth_request *request,
 			 const char *key, const char *value);
+bool auth_request_import_info(struct auth_request *request,
+			      const char *key, const char *value);
+bool auth_request_import_auth(struct auth_request *request,
+			      const char *key, const char *value);
 
 void auth_request_initial(struct auth_request *request);
 void auth_request_continue(struct auth_request *request,