From: Timo Sirainen <tss@iki.fi>
Date: Fri, 18 Nov 2011 20:07:16 +0000 (+0200)
Subject: auth: Don't allow auth clients to set internal auth request fields.
X-Git-Tag: 2.1.rc1~53
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ab122a3bbae3b5fd2aad66e2f2840149d98cee52;p=thirdparty%2Fdovecot%2Fcore.git

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.
---

diff --git a/src/auth/auth-master-connection.c b/src/auth/auth-master-connection.c
index bb57ce39c5..f639b9edf1 100644
--- a/src/auth/auth-master-connection.c
+++ b/src/auth/auth-master-connection.c
@@ -174,7 +174,7 @@ master_input_auth_request(struct auth_master_connection *conn, const char *args,
 			arg++;
 		}
 
-		(void)auth_request_import(auth_request, name, arg);
+		(void)auth_request_import_info(auth_request, name, arg);
 	}
 
 	if (auth_request->service == NULL) {
diff --git a/src/auth/auth-request-handler.c b/src/auth/auth-request-handler.c
index f13bd1dbb6..b4946c4794 100644
--- a/src/auth/auth-request-handler.c
+++ b/src/auth/auth-request-handler.c
@@ -433,7 +433,7 @@ bool auth_request_handler_auth_begin(struct auth_request_handler *handler,
 			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 --git a/src/auth/auth-request.c b/src/auth/auth-request.c
index 7b653605d7..989d49572f 100644
--- a/src/auth/auth-request.c
+++ b/src/auth/auth-request.c
@@ -207,25 +207,11 @@ void auth_request_export(struct auth_request *request,
 		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 @@ bool auth_request_import(struct auth_request *request,
 		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, "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, "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, "successful") == 0)
 		request->successful = TRUE;
 	else if (strcmp(key, "skip_password_check") == 0) {
diff --git a/src/auth/auth-request.h b/src/auth/auth-request.h
index 048c60a4ef..c2969d030d 100644
--- a/src/auth/auth-request.h
+++ b/src/auth/auth-request.h
@@ -139,6 +139,10 @@ void auth_request_export(struct auth_request *request,
 			 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,