From 37d24f6bf2d66d3bfc123416c27136bc4d2e22a4 Mon Sep 17 00:00:00 2001
From: Sean Talts <sean.talts@gmail.com>
Date: Mon, 20 Apr 2020 13:19:03 -0400
Subject: src/murmur: Add autobanSuccessfulConnections flag.

The idea here is that sometimes you really do have a lot of folks connecting from a single IP,
and if those connections are successful you don't want to ban any of them.
However, in cases where the server needs to guard against malicious users attempting a DDOS
by reconnecting their valid user account over and over, we need to be able to configure the
server to still ban those successful attempts.
---
 scripts/murmur.ini    |  4 ++++
 src/murmur/Meta.cpp   | 15 +++++++++++++++
 src/murmur/Meta.h     |  5 +++++
 src/murmur/Server.cpp |  2 ++
 4 files changed, 26 insertions(+)

diff --git a/scripts/murmur.ini b/scripts/murmur.ini
index b18881c13..651918c37 100644
--- a/scripts/murmur.ini
+++ b/scripts/murmur.ini
@@ -373,9 +373,13 @@ allowping=true
 ; To disable, set autobanAttempts or autobanTimeframe to 0. Commenting these
 ; settings out will cause Murmur to use the defaults:
 ;
+; To avoid autobanning successful connection attempts from the same IP address,
+; set autobanSuccessfulConnections=False.
+;
 ;autobanAttempts=10
 ;autobanTimeframe=120
 ;autobanTime=300
+;autobanSuccessfulConnections=True
 
 ; Enables logging of group changes. This means that every time a group in a
 ; channel changes, the server will log all groups and their members from before
diff --git a/src/murmur/Meta.cpp b/src/murmur/Meta.cpp
index 39c0e4ebb..31c7e2b9e 100644
--- a/src/murmur/Meta.cpp
+++ b/src/murmur/Meta.cpp
@@ -78,6 +78,7 @@ MetaParams::MetaParams() {
 	iBanTries = 10;
 	iBanTimeframe = 120;
 	iBanTime = 300;
+	bBanSuccessful = true;
 
 #ifdef Q_OS_UNIX
 	uiUid = uiGid = 0;
@@ -329,6 +330,7 @@ void MetaParams::read(QString fname) {
 	iBanTries = typeCheckedFromSettings("autobanAttempts", iBanTries);
 	iBanTimeframe = typeCheckedFromSettings("autobanTimeframe", iBanTimeframe);
 	iBanTime = typeCheckedFromSettings("autobanTime", iBanTime);
+	bBanSuccessful = typeCheckedFromSettings("autobanSuccessfulConnections", bBanSuccessful);
 
 	qvSuggestVersion = MumbleVersion::getRaw(qsSettings->value("suggestVersion").toString());
 	if (qvSuggestVersion.toUInt() == 0)
@@ -747,6 +749,19 @@ void Meta::killAll() {
 	qhServers.clear();
 }
 
+void Meta::successfulConnectionFrom(const QHostAddress &addr) {
+	if (!mp.bBanSuccessful) {
+		QList<Timer> &ql = qhAttempts[addr];
+		// Seems like this is the most efficient way to clear the list, given:
+		// 1. ql.clear() allocates a new array
+		// 2. ql has less than iBanAttempts members
+		// 3. seems like ql.removeFirst() might actually copy elements to shift to the front
+		while (!ql.empty()) {
+			ql.removeLast();
+		}
+	}
+}
+
 bool Meta::banCheck(const QHostAddress &addr) {
 	if ((mp.iBanTries == 0) || (mp.iBanTimeframe == 0))
 		return false;
diff --git a/src/murmur/Meta.h b/src/murmur/Meta.h
index 470ffbdc2..9720180fb 100644
--- a/src/murmur/Meta.h
+++ b/src/murmur/Meta.h
@@ -59,6 +59,7 @@ public:
 	int iBanTries;
 	int iBanTimeframe;
 	int iBanTime;
+	bool bBanSuccessful;
 
 	QString qsDatabase;
 	int iSQLiteWAL;
@@ -192,6 +193,10 @@ class Meta : public QObject {
 		void bootAll();
 		bool boot(int);
 		bool banCheck(const QHostAddress &);
+
+		/// Called whenever we get a successful connection from a client.
+		/// Used to reset autoban tracking for the address.
+		void successfulConnectionFrom(const QHostAddress &);
 		void kill(int);
 		void killAll();
 		void getOSInfo();
diff --git a/src/murmur/Server.cpp b/src/murmur/Server.cpp
index bce2a2172..9a1f76099 100644
--- a/src/murmur/Server.cpp
+++ b/src/murmur/Server.cpp
@@ -1362,6 +1362,8 @@ void Server::newClient() {
 		sock->setProtocol(QSsl::TlsV1_0);
 #endif
 		sock->startServerEncryption();
+
+		meta->successfulConnectionFrom(adr);
 	}
 }
 
-- 
cgit v1.2.3


From 6c6a5918264cc9d202f302c31c8f4959d6b4f563 Mon Sep 17 00:00:00 2001
From: Sean Talts <sean.talts@gmail.com>
Date: Mon, 20 Apr 2020 15:06:07 -0400
Subject: src/murmur/Meta.cpp: Negative values of these settings don't make
 sense.

Instead, we'll turn off the autoban feature if either are negative.
---
 src/murmur/Meta.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/murmur/Meta.cpp b/src/murmur/Meta.cpp
index 31c7e2b9e..8c6ab9502 100644
--- a/src/murmur/Meta.cpp
+++ b/src/murmur/Meta.cpp
@@ -763,7 +763,7 @@ void Meta::successfulConnectionFrom(const QHostAddress &addr) {
 }
 
 bool Meta::banCheck(const QHostAddress &addr) {
-	if ((mp.iBanTries == 0) || (mp.iBanTimeframe == 0))
+	if ((mp.iBanTries <= 0) || (mp.iBanTimeframe <= 0))
 		return false;
 
 	if (qhBans.contains(addr)) {
-- 
cgit v1.2.3