From: Andre Noll <maan@systemlinux.org>
Date: Thu, 4 Oct 2007 14:11:59 +0000 (+0200)
Subject: afs.c: Introduce client list for afs (fixes dosable bug).
X-Git-Tag: v0.3.0~312
X-Git-Url: http://git.tuebingen.mpg.de/?p=paraslash.git;a=commitdiff_plain;h=769888ee3175ed60df3c23caf51cdad5db33d141;ds=sidebyside

afs.c: Introduce client list for afs (fixes dosable bug).

With the old code, a malicious local user could easily cause a DOS
by connecting to the local afs socket but never sending any data
through that socket. This caused afs to block on the read() from the
new client fd.

This patch fixes this flaw by introducing a doubly liked list of
connected clients.  Each entry in this list contains an open file
descriptor and the time the client connected.

In command_pre_select() we add each fd in the list to the set of
fds to be checked for readability. In command_post_select() we loop
through that list and check each fd again. Whenever the fd for a
client is readable we read from that fd and execute an afs command
if the client sent the valid magic cookie.

However, if the fd is not ready, we close the connection if more than
AFS_CLIENT_TIMEOUT seconds (default: 3) have passed since the client
connected. This hopefully avoids another possible DOS: A malicious
user could try to flood us with connections until the limit of open
fds is reached.
---

diff --git a/afs.c b/afs.c
index b7d27927..f6d6b8d7 100644
--- a/afs.c
+++ b/afs.c
@@ -584,11 +584,23 @@ static void register_signal_task(void)
 	register_task(&st->task);
 }
 
+static struct list_head afs_client_list;
+
+struct afs_client {
+	struct list_head node;
+	int fd;
+	struct timeval connect_time;
+};
+
 static void command_pre_select(struct sched *s, struct task *t)
 {
 	struct command_task *ct = t->private_data;
-	t->ret = 1;
+	struct afs_client *client;
+
 	para_fd_set(ct->fd, &s->rfds, &s->max_fileno);
+	list_for_each_entry(client, &afs_client_list, node)
+		para_fd_set(client->fd, &s->rfds, &s->max_fileno);
+	t->ret = 1;
 }
 
 /*
@@ -646,54 +658,75 @@ out:
 	return ret;
 }
 
-static void command_post_select(struct sched *s, struct task *t)
+static void execute_afs_command(int fd, uint32_t expected_cookie)
 {
-	struct command_task *ct = t->private_data;
-	struct sockaddr_un unix_addr;
-	char buf[sizeof(uint32_t) + sizeof(int)];
 	uint32_t cookie;
-	int query_shmid, fd;
+	int query_shmid;
+	char buf[sizeof(cookie) + sizeof(query_shmid)];
+	int ret = recv_bin_buffer(fd, buf, sizeof(buf));
 
-	t->ret = 1;
-	if (!FD_ISSET(ct->fd, &s->rfds))
-		return;
-	t->ret = para_accept(ct->fd, &unix_addr, sizeof(unix_addr));
-	if (t->ret < 0)
+	if (ret < 0) {
+		PARA_NOTICE_LOG("%s\n", PARA_STRERROR(-ret));
 		return;
-	/*
-	 * The following errors may be caused by a malicious local user. So do
-	 * not return an error in this case as this would terminate  para_afs
-	 * and para_server.
-	 */
-	fd = t->ret;
-	/* FIXME: This is easily dosable (peer doesn't send data) */
-	t->ret = recv_bin_buffer(fd, buf, sizeof(buf));
-	if (t->ret < 0) {
-		PARA_NOTICE_LOG("%s (%d)\n", PARA_STRERROR(-t->ret), t->ret);
-		goto out;
 	}
-	if (t->ret != sizeof(buf)) {
+	if (ret != sizeof(buf)) {
 		PARA_NOTICE_LOG("short read (%d bytes, expected %lu)\n",
-			t->ret, (long unsigned) sizeof(buf));
-		goto out;
+			ret, (long unsigned) sizeof(buf));
+		return;
 	}
 	cookie = *(uint32_t *)buf;
-	if (cookie != ct->cookie) {
+	if (cookie != expected_cookie) {
 		PARA_NOTICE_LOG("received invalid cookie(got %u, expected %u)\n",
-			(unsigned)cookie, (unsigned)ct->cookie);
-		goto out;
+			(unsigned)cookie, (unsigned)expected_cookie);
+		return;
 	}
 	query_shmid = *(int *)(buf + sizeof(cookie));
 	if (query_shmid < 0) {
 		PARA_WARNING_LOG("received invalid query shmid %d)\n",
 			query_shmid);
-		goto out;
+		return;
 	}
 	/* Ignore return value: Errors might be ok here. */
 	call_callback(fd, query_shmid);
+}
+
+#define AFS_CLIENT_TIMEOUT 3
+
+static void command_post_select(struct sched *s, struct task *t)
+{
+	struct command_task *ct = t->private_data;
+	struct sockaddr_un unix_addr;
+	struct afs_client *client, *tmp;
+
+	/* First, check the list of connected clients. */
+	list_for_each_entry_safe(client, tmp, &afs_client_list, node) {
+		if (FD_ISSET(client->fd, &s->rfds))
+			execute_afs_command(client->fd, ct->cookie);
+		else { /* prevent bogus connection flodding */
+			struct timeval diff;
+			tv_diff(now, &client->connect_time, &diff);
+			if (diff.tv_sec < AFS_CLIENT_TIMEOUT)
+				continue;
+			PARA_WARNING_LOG("connection timeout\n");
+		}
+		close(client->fd);
+		list_del(&client->node);
+		free(client);
+	}
+	/* Next, accept connections on the local socket. */
+	if (!FD_ISSET(ct->fd, &s->rfds))
+		goto out;
+	t->ret = para_accept(ct->fd, &unix_addr, sizeof(unix_addr));
+	if (t->ret < 0) {
+		PARA_NOTICE_LOG("%s\n", PARA_STRERROR(-t->ret));
+		goto out;
+	}
+	client = para_malloc(sizeof(*client));
+	client->fd = t->ret;
+	client->connect_time = *now;
+	para_list_add(&client->node, &afs_client_list);
 out:
 	t->ret = 1;
-	close(fd);
 }
 
 static void register_command_task(uint32_t cookie)
@@ -788,7 +821,10 @@ __noreturn int afs_init(uint32_t cookie, int socket_fd)
 {
 	enum play_mode current_play_mode;
 	struct sched s;
-	int ret = open_afs_tables();
+	int ret;
+
+	INIT_LIST_HEAD(&afs_client_list);
+	ret = open_afs_tables();
 
 	if (ret < 0) {
 		PARA_EMERG_LOG("%s\n", PARA_STRERROR(-ret));