From: Timo Sirainen <timo.sirainen@open-xchange.com>
Date: Tue, 18 Jan 2022 14:10:00 +0000 (+0100)
Subject: anvil: Don't track proxy connections in userip_hash
X-Git-Tag: 2.4.0~4515
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=63529a087189522b7ae69421ca0ef46fe4a23cbb;p=thirdparty%2Fdovecot%2Fcore.git

anvil: Don't track proxy connections in userip_hash

Otherwise proxy connections would also be counted towards tracking
mail_max_userip_connections, which could cause the same session to be
counted multiple times.
---

diff --git a/src/anvil/connect-limit.c b/src/anvil/connect-limit.c
index c339e122f3..dbbaed794a 100644
--- a/src/anvil/connect-limit.c
+++ b/src/anvil/connect-limit.c
@@ -60,6 +60,12 @@ struct alt_username_field {
 	unsigned int refcount;
 };
 
+/* Track only non-proxying sessions for mail_max_userip_connections. Otherwise
+   if the same server is acting as both proxy and backend the connection could
+   be counted twice. */
+#define SESSION_TRACK_USERIP(session) \
+	((session)->dest_ip.family == 0)
+
 struct connect_limit {
 	struct str_table *strings;
 
@@ -67,7 +73,8 @@ struct connect_limit {
 
 	/* username => struct session linked list */
 	HASH_TABLE(char *, struct session *) user_hash;
-	/* userip => unsigned int refcount */
+	/* userip => unsigned int refcount. Only track for sessions where
+	   SESSION_TRACK_USERIP() returns TRUE. */
 	HASH_TABLE(struct userip *, void *) userip_hash;
 	/* conn_guid => struct session */
 	HASH_TABLE(const uint8_t *, struct session *) session_hash;
@@ -381,32 +388,35 @@ void connect_limit_connect(struct connect_limit *limit, pid_t pid,
 		first_user_session = NULL;
 	}
 
+	session = i_new(struct session, 1);
+	guid_128_copy(session->conn_guid, conn_guid);
+	if (dest_ip != NULL)
+		session->dest_ip = *dest_ip;
+	T_BEGIN {
+		session_set_alt_usernames(limit, session, alt_usernames);
+	} T_END;
+
 	struct userip userip_lookup = {
 		.username = username,
 		.service = key->service,
 		.ip = key->ip,
 	};
-	if (!hash_table_lookup_full(limit->userip_hash, &userip_lookup,
+
+	if (!SESSION_TRACK_USERIP(session) ||
+	    !hash_table_lookup_full(limit->userip_hash, &userip_lookup,
 				    &userip, &value)) {
 		userip = i_new(struct userip, 1);
 		userip->username = username;
 		userip->service = str_table_ref(limit->strings, key->service);
 		userip->ip = key->ip;
 		value = POINTER_CAST(1);
-		hash_table_insert(limit->userip_hash, userip, value);
+		if (SESSION_TRACK_USERIP(session))
+			hash_table_insert(limit->userip_hash, userip, value);
 	} else {
 		value = POINTER_CAST(POINTER_CAST_TO(value, unsigned int) + 1);
 		hash_table_update(limit->userip_hash, userip, value);
 	}
-
-	session = i_new(struct session, 1);
 	session->userip = userip;
-	guid_128_copy(session->conn_guid, conn_guid);
-	if (dest_ip != NULL)
-		session->dest_ip = *dest_ip;
-	T_BEGIN {
-		session_set_alt_usernames(limit, session, alt_usernames);
-	} T_END;
 
 	session_link_process(limit, session, pid, kick_type);
 	const uint8_t *conn_guid_p = session->conn_guid;
@@ -416,13 +426,16 @@ void connect_limit_connect(struct connect_limit *limit, pid_t pid,
 	hash_table_update(limit->user_hash, username, session);
 }
 
+static void userip_free(struct connect_limit *limit, struct userip *userip)
+{
+	str_table_unref(limit->strings, &userip->service);
+	i_free(userip);
+}
+
 static void
-session_free(struct connect_limit *limit, struct session *session)
+userip_hash_unref(struct connect_limit *limit, struct session *session)
 {
 	struct userip *userip = session->userip;
-	struct session *first_user_session;
-	const char *username = userip->username;
-	char *orig_username;
 	void *value;
 	unsigned int new_refcount;
 
@@ -436,9 +449,21 @@ session_free(struct connect_limit *limit, struct session *session)
 		hash_table_update(limit->userip_hash, userip, value);
 	} else {
 		hash_table_remove(limit->userip_hash, userip);
-		str_table_unref(limit->strings, &userip->service);
-		i_free(userip);
+		userip_free(limit, userip);
 	}
+}
+
+static void
+session_free(struct connect_limit *limit, struct session *session)
+{
+	struct session *first_user_session;
+	char *orig_username;
+	const char *username = session->userip->username;
+
+	if (SESSION_TRACK_USERIP(session))
+		userip_hash_unref(limit, session);
+	else
+		userip_free(limit, session->userip);
 
 	if (!hash_table_lookup_full(limit->user_hash, username,
 				    &orig_username, &first_user_session))
diff --git a/src/anvil/test-connect-limit.c b/src/anvil/test-connect-limit.c
index f999fc6bf4..10fe244c6b 100644
--- a/src/anvil/test-connect-limit.c
+++ b/src/anvil/test-connect-limit.c
@@ -74,10 +74,9 @@ static void test_connect_limit(void)
 	struct ip_addr dest_ip;
 	i_zero(&dest_ip);
 	test_assert(net_addr2ip("1.2.3.4", &key.ip) == 0);
-	test_assert(net_addr2ip("55.44.33.22", &dest_ip) == 0);
 	connect_limit_connect(limit, 501, &key, session1_guid, KICK_TYPE_NONE,
 			      &dest_ip, alt_usernames1);
-#define TEST_SESSION1_STR "501\tuser1\tservice1\t1.2.3.4\t"SESSION1_HEX"\t55.44.33.22\n"
+#define TEST_SESSION1_STR "501\tuser1\tservice1\t1.2.3.4\t"SESSION1_HEX"\t\n"
 	test_session_dump(limit, TEST_SESSION1_STR);
 	test_assert(connect_limit_lookup(limit, &key) == 1);
 
@@ -118,7 +117,7 @@ static void test_connect_limit(void)
 #define TEST_SESSION3_STR "600\tuser2\tservice2\t4.3.2.1\t"SESSION3_HEX"\t1.0.0.2\n"
 	test_session_dump(limit, TEST_SESSION1_STR TEST_SESSION2_STR TEST_SESSION3_STR);
 	test_assert(connect_limit_lookup(limit, &key) == 2);
-	test_assert(connect_limit_lookup(limit, &key3) == 1);
+	test_assert(connect_limit_lookup(limit, &key3) == 0);
 
 	/* duplicate conn-guid */
 	struct connect_limit_key key4 = {