[dovecot-cvs] dovecot/src/auth auth-digest-md5.c,1.9,1.10 auth-interface.h,1.3,1.4 auth-plain.c,1.6,1.7 auth.c,1.4,1.5 auth.h,1.2,1.3 cookie.c,1.4,1.5 cookie.h,1.2,1.3 login-connection.c,1.9,1.10 main.c,1.9,1.10 master.c,1.7,1.8

[cras at procontrol.fi]

Update of /home/cvs/dovecot/src/auth
In directory danu:/tmp/cvs-serv14140/auth

Modified Files:
	auth-digest-md5.c auth-interface.h auth-plain.c auth.c auth.h 
	cookie.c cookie.h login-connection.c main.c master.c 
Log Message:
Instead of just trusting randomness of authentication cookies between
auth<->master<->login process IPC, master now doesn't accept any cookies
from login process which weren't created by it (identified by PID). When
login process dies, all it's pending cookies are also removed, so I can't
see even a theoretical possiblity anymore for exploited login process to    
authenticate as someone else.

Also fixed some int -> unsigned int.



Index: auth-digest-md5.c
===================================================================
RCS file: /home/cvs/dovecot/src/auth/auth-digest-md5.c,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -d -r1.9 -r1.10
--- auth-digest-md5.c	18 Dec 2002 15:15:41 -0000	1.9
+++ auth-digest-md5.c	19 Dec 2002 23:56:23 -0000	1.10
@@ -575,7 +575,8 @@
 	pool_unref(((AuthData *) cookie->context)->pool);
 }
 
-static void auth_digest_md5_init(AuthInitRequestData *request,
+static void auth_digest_md5_init(unsigned int login_pid,
+				 AuthInitRequestData *request,
 				 AuthCallback callback, void *context)
 {
 	CookieData *cookie;
@@ -591,6 +592,7 @@
 	auth->qop = QOP_AUTH;
 
 	cookie = p_new(pool, CookieData, 1);
+	cookie->login_pid = login_pid;
 	cookie->auth_fill_reply = auth_digest_md5_fill_reply;
 	cookie->auth_continue = auth_digest_md5_continue;
 	cookie->free = auth_digest_md5_free;

Index: auth-interface.h
===================================================================
RCS file: /home/cvs/dovecot/src/auth/auth-interface.h,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -d -r1.3 -r1.4
--- auth-interface.h	17 Dec 2002 03:00:44 -0000	1.3
+++ auth-interface.h	19 Dec 2002 23:56:23 -0000	1.4
@@ -34,16 +34,21 @@
 
 /* Initialization reply, sent after client is connected */
 typedef struct {
-	int auth_process; /* unique auth process identifier */
+	unsigned int auth_process; /* unique auth process identifier */
 	AuthMethod auth_methods; /* valid authentication methods */
 } AuthInitData;
 
+/* Initialization handshake from client. */
+typedef struct {
+	unsigned int pid; /* unique identifier for client process */
+} ClientAuthInitData;
+
 /* New authentication request */
 typedef struct {
 	AuthRequestType type; /* AUTH_REQUEST_INIT */
 
 	AuthMethod method;
-	int id; /* AuthReplyData.id will contain this value */
+	unsigned int id; /* AuthReplyData.id will contain this value */
 } AuthInitRequestData;
 
 /* Continued authentication request */
@@ -51,7 +56,7 @@
 	AuthRequestType type; /* AUTH_REQUEST_CONTINUE */
 
 	unsigned char cookie[AUTH_COOKIE_SIZE];
-	int id; /* AuthReplyData.id will contain this value */
+	unsigned int id; /* AuthReplyData.id will contain this value */
 
 	size_t data_size;
 	/* unsigned char data[]; */
@@ -59,7 +64,7 @@
 
 /* Reply to authentication */
 typedef struct {
-	int id;
+	unsigned int id;
 	unsigned char cookie[AUTH_COOKIE_SIZE];
 	AuthResult result;
 
@@ -69,13 +74,14 @@
 
 /* Request data associated to cookie */
 typedef struct {
-	int id;
+	unsigned int id;
+	unsigned int login_pid;
 	unsigned char cookie[AUTH_COOKIE_SIZE];
 } AuthCookieRequestData;
 
 /* Reply to cookie request */
 typedef struct {
-	int id;
+	unsigned int id;
 	int success; /* FALSE if cookie wasn't found */
 
 	char system_user[AUTH_MAX_USER_LEN]; /* system user, if available */

Index: auth-plain.c
===================================================================
RCS file: /home/cvs/dovecot/src/auth/auth-plain.c,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -d -r1.6 -r1.7
--- auth-plain.c	18 Dec 2002 10:40:43 -0000	1.6
+++ auth-plain.c	19 Dec 2002 23:56:23 -0000	1.7
@@ -80,13 +80,15 @@
 	i_free(cookie);
 }
 
-static void auth_plain_init(AuthInitRequestData *request,
+static void auth_plain_init(unsigned int login_pid,
+			    AuthInitRequestData *request,
 			    AuthCallback callback, void *context)
 {
 	CookieData *cookie;
 	AuthReplyData reply;
 
 	cookie = i_new(CookieData, 1);
+	cookie->login_pid = login_pid;
 	cookie->auth_fill_reply = auth_plain_fill_reply;
 	cookie->auth_continue = auth_plain_continue;
 	cookie->free = auth_plain_free;

Index: auth.c
===================================================================
RCS file: /home/cvs/dovecot/src/auth/auth.c,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -d -r1.4 -r1.5
--- auth.c	28 Oct 2002 09:46:02 -0000	1.4
+++ auth.c	19 Dec 2002 23:56:23 -0000	1.5
@@ -54,7 +54,8 @@
 	}
 }
 
-void auth_init_request(AuthInitRequestData *request,
+void auth_init_request(unsigned int login_pid,
+		       AuthInitRequestData *request,
 		       AuthCallback callback, void *context)
 {
 	AuthModuleList *list;
@@ -70,7 +71,8 @@
 
 	for (list = auth_modules; list != NULL; list = list->next) {
 		if (list->module.method == request->method) {
-			list->module.init(request, callback, context);
+			list->module.init(login_pid, request,
+					  callback, context);
 			return;
 		}
 	}
@@ -78,7 +80,8 @@
 	i_unreached();
 }
 
-void auth_continue_request(AuthContinuedRequestData *request,
+void auth_continue_request(unsigned int login_pid,
+			   AuthContinuedRequestData *request,
 			   const unsigned char *data,
 			   AuthCallback callback, void *context)
 {
@@ -89,9 +92,11 @@
 		/* timeouted cookie */
 		failure_reply.id = request->id;
 		callback(&failure_reply, NULL, context);
+	} else if (cookie_data->login_pid != login_pid) {
+		i_error("BUG: imap-login requested cookie it didn't own");
 	} else {
-		cookie_data->auth_continue(cookie_data, request, data,
-					   callback, context);
+		cookie_data->auth_continue(cookie_data, request,
+					   data, callback, context);
 	}
 }
 

Index: auth.h
===================================================================
RCS file: /home/cvs/dovecot/src/auth/auth.h,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -d -r1.2 -r1.3
--- auth.h	22 Aug 2002 12:48:38 -0000	1.2
+++ auth.h	19 Dec 2002 23:56:23 -0000	1.3
@@ -9,7 +9,8 @@
 typedef struct {
 	AuthMethod method;
 
-	void (*init)(AuthInitRequestData *request,
+	void (*init)(unsigned int login_pid,
+		     AuthInitRequestData *request,
 		     AuthCallback callback, void *context);
 } AuthModule;
 
@@ -19,9 +20,11 @@
 void auth_register_module(AuthModule *module);
 void auth_unregister_module(AuthModule *module);
 
-void auth_init_request(AuthInitRequestData *request,
+void auth_init_request(unsigned int login_pid,
+		       AuthInitRequestData *request,
 		       AuthCallback callback, void *context);
-void auth_continue_request(AuthContinuedRequestData *request,
+void auth_continue_request(unsigned int login_pid,
+			   AuthContinuedRequestData *request,
 			   const unsigned char *data,
 			   AuthCallback callback, void *context);
 

Index: cookie.c
===================================================================
RCS file: /home/cvs/dovecot/src/auth/cookie.c,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -d -r1.4 -r1.5
--- cookie.c	15 Oct 2002 20:53:16 -0000	1.4
+++ cookie.c	19 Dec 2002 23:56:23 -0000	1.5
@@ -84,6 +84,7 @@
 
 	hash_remove(cookies, cookie);
 
+	/* FIXME: slow */
 	list = NULL;
 	for (pos = &oldest_cookie; *pos != NULL; pos = &(*pos)->next) {
 		if (cookie_cmp((*pos)->data->cookie, cookie) == 0) {
@@ -112,14 +113,32 @@
 	cookie_destroy(cookie, TRUE);
 }
 
-CookieData *cookie_lookup_and_remove(unsigned char cookie[AUTH_COOKIE_SIZE])
+CookieData *cookie_lookup_and_remove(unsigned int login_pid,
+				     unsigned char cookie[AUTH_COOKIE_SIZE])
 {
 	CookieData *data;
 
 	data = hash_lookup(cookies, cookie);
-	if (data != NULL)
-		cookie_destroy(cookie, FALSE);
+	if (data != NULL) {
+		if (data->login_pid != login_pid)
+			data = NULL;
+		else
+			cookie_destroy(cookie, FALSE);
+	}
 	return data;
+}
+
+void cookies_remove_login_pid(unsigned int login_pid)
+{
+	CookieList *list, *next;
+
+	/* FIXME: slow */
+	for (list = oldest_cookie; list != NULL; list = next) {
+		next = list->next;
+
+		if (list->data->login_pid == login_pid)
+			cookie_destroy(list->data->cookie, TRUE);
+	}
 }
 
 static void cookie_timeout(void *context __attr_unused__,

Index: cookie.h
===================================================================
RCS file: /home/cvs/dovecot/src/auth/cookie.h,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -d -r1.2 -r1.3
--- cookie.h	22 Aug 2002 12:48:38 -0000	1.2
+++ cookie.h	19 Dec 2002 23:56:23 -0000	1.3
@@ -6,6 +6,7 @@
 typedef struct _CookieData CookieData;
 
 struct _CookieData {
+	unsigned int login_pid;
 	unsigned char cookie[AUTH_COOKIE_SIZE];
 
 	/* continue authentication */
@@ -32,7 +33,11 @@
 /* Removes and frees the cookie */
 void cookie_remove(unsigned char cookie[AUTH_COOKIE_SIZE]);
 /* Looks up the cookie and removes it, you have to free the returned data. */
-CookieData *cookie_lookup_and_remove(unsigned char cookie[AUTH_COOKIE_SIZE]);
+CookieData *cookie_lookup_and_remove(unsigned int login_pid,
+				     unsigned char cookie[AUTH_COOKIE_SIZE]);
+
+/* Remove all cookies created by a login process. */
+void cookies_remove_login_pid(unsigned int login_pid);
 
 void cookies_init(void);
 void cookies_deinit(void);

Index: login-connection.c
===================================================================
RCS file: /home/cvs/dovecot/src/auth/login-connection.c,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -d -r1.9 -r1.10
--- login-connection.c	18 Dec 2002 10:40:43 -0000	1.9
+++ login-connection.c	19 Dec 2002 23:56:23 -0000	1.10
@@ -6,6 +6,7 @@
 #include "ostream.h"
 #include "network.h"
 #include "safe-memset.h"
+#include "cookie.h"
 #include "login-connection.h"
 
 #include <stdlib.h>
@@ -23,7 +24,9 @@
 	IO io;
 	IStream *input;
 	OStream *output;
-        AuthRequestType type;
+
+	unsigned int pid;
+	AuthRequestType type;
 };
 
 static AuthInitData auth_init_data;
@@ -44,27 +47,49 @@
 	}
 }
 
-static void login_input(void *context, int fd __attr_unused__,
-			IO io __attr_unused__)
+static LoginConnection *login_find_pid(unsigned int pid)
 {
-	LoginConnection *conn  = context;
+	LoginConnection *conn;
+
+	for (conn = connections; conn != NULL; conn = conn->next) {
+		if (conn->pid == pid)
+			return conn;
+	}
+
+	return NULL;
+}
+
+static void login_input_handshake(LoginConnection *conn)
+{
+        ClientAuthInitData rec;
         unsigned char *data;
 	size_t size;
 
-	switch (i_stream_read(conn->input)) {
-	case 0:
+	data = i_stream_get_modifyable_data(conn->input, &size);
+	if (size < sizeof(ClientAuthInitData))
 		return;
-	case -1:
-		/* disconnected */
+
+	/* Don't just cast because of alignment issues. */
+	memcpy(&rec, data, sizeof(rec));
+	i_stream_skip(conn->input, sizeof(rec));
+
+	if (rec.pid == 0) {
+		i_error("BUG: imap-login said it's PID 0");
 		login_connection_destroy(conn);
-		return;
-	case -2:
-		/* buffer full */
-		i_error("BUG: imap-login sent us more than %d bytes of data",
-			(int)MAX_INBUF_SIZE);
+	} else if (login_find_pid(rec.pid) != NULL) {
+		/* well, it might have just reconnected very fast .. although
+		   there's not much reason for it. */
+		i_error("BUG: imap-login gave a PID of existing connection");
 		login_connection_destroy(conn);
-		return;
+	} else {
+		conn->pid = rec.pid;
 	}
+}
+
+static void login_input_request(LoginConnection *conn)
+{
+        unsigned char *data;
+	size_t size;
 
 	data = i_stream_get_modifyable_data(conn->input, &size);
 	if (size < sizeof(AuthRequestType))
@@ -87,7 +112,7 @@
 		i_stream_skip(conn->input, sizeof(request));
 
 		/* we have a full init request */
-		auth_init_request(&request, request_callback, conn);
+		auth_init_request(conn->pid, &request, request_callback, conn);
 		conn->type = AUTH_REQUEST_NONE;
 	} else if (conn->type == AUTH_REQUEST_CONTINUE) {
                 AuthContinuedRequestData request;
@@ -102,7 +127,8 @@
 		i_stream_skip(conn->input, sizeof(request) + request.data_size);
 
 		/* we have a full continued request */
-		auth_continue_request(&request, data + sizeof(request),
+		auth_continue_request(conn->pid, &request,
+				      data + sizeof(request),
 				      request_callback, conn);
 		conn->type = AUTH_REQUEST_NONE;
 
@@ -116,6 +142,32 @@
 	}
 }
 
+static void login_input(void *context, int fd __attr_unused__,
+			IO io __attr_unused__)
+{
+	LoginConnection *conn  = context;
+
+	switch (i_stream_read(conn->input)) {
+	case 0:
+		return;
+	case -1:
+		/* disconnected */
+		login_connection_destroy(conn);
+		return;
+	case -2:
+		/* buffer full */
+		i_error("BUG: imap-login sent us more than %d bytes of data",
+			(int)MAX_INBUF_SIZE);
+		login_connection_destroy(conn);
+		return;
+	}
+
+	if (conn->pid == 0)
+		login_input_handshake(conn);
+	else
+		login_input_request(conn);
+}
+
 LoginConnection *login_connection_create(int fd)
 {
 	LoginConnection *conn;
@@ -152,6 +204,8 @@
 			break;
 		}
 	}
+
+	cookies_remove_login_pid(conn->pid);
 
 	i_stream_unref(conn->input);
 	o_stream_unref(conn->output);

Index: main.c
===================================================================
RCS file: /home/cvs/dovecot/src/auth/main.c,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -d -r1.9 -r1.10
--- main.c	18 Dec 2002 04:00:01 -0000	1.9
+++ main.c	19 Dec 2002 23:56:23 -0000	1.10
@@ -71,7 +71,8 @@
 	master_init();
 	userinfo_init();
 
-	io_listen = io_add(LOGIN_LISTEN_FD, IO_READ, auth_accept, NULL);
+	io_listen = io_add_priority(LOGIN_LISTEN_FD, IO_PRIORITY_LOW,
+				    IO_READ, auth_accept, NULL);
 }
 
 static void main_deinit(void)

Index: master.c
===================================================================
RCS file: /home/cvs/dovecot/src/auth/master.c,v
retrieving revision 1.7
retrieving revision 1.8
diff -u -d -r1.7 -r1.8
--- master.c	6 Dec 2002 01:09:22 -0000	1.7
+++ master.c	19 Dec 2002 23:56:23 -0000	1.8
@@ -23,7 +23,7 @@
 	CookieData *cookie;
         AuthCookieReplyData *reply, temp_reply;
 
-	cookie = cookie_lookup_and_remove(request->cookie);
+	cookie = cookie_lookup_and_remove(request->login_pid, request->cookie);
 	if (cookie == NULL)
 		reply = &failure_reply;
 	else {