Fix server getting completely choked up on even a little of DoS

* If client count is unbearable, immediately delete denied clients
* Re-prioritize the checking order of things about incoming clients
* Remove a huge CPU-wasting exception in ReliablePacketBuffer

src/connection.cpp

@@ -207,12 +207,13 @@ RPBSearchResult ReliablePacketBuffer::notFound()
 {
 	return m_list.end();
 }
-u16 ReliablePacketBuffer::getFirstSeqnum()
+bool ReliablePacketBuffer::getFirstSeqnum(u16 *result)
 {
 	if(empty())
-		throw NotFoundException("Buffer is empty");
+		return false;
 	BufferedPacket p = *m_list.begin();
-	return readU16(&p.data[BASE_HEADER_SIZE+1]);
+	*result = readU16(&p.data[BASE_HEADER_SIZE+1]);
+	return true;
 }
 BufferedPacket ReliablePacketBuffer::popFirst()
 {
@@ -700,7 +701,7 @@ void Connection::receive()
 
 	bool single_wait_done = false;
 	
-	for(;;)
+	for(u32 loop_i=0; loop_i<1000; loop_i++) // Limit in case of DoS
 	{
 	try{
 		/* Check if some buffer has relevant data */
@@ -1245,17 +1246,16 @@ bool Connection::checkIncomingBuffers(Channel *channel, u16 &peer_id,
 {
 	u16 firstseqnum = 0;
 	// Clear old packets from start of buffer
-	try{
 	for(;;){
-		firstseqnum = channel->incoming_reliables.getFirstSeqnum();
+		bool found = channel->incoming_reliables.getFirstSeqnum(&firstseqnum);
+		if(!found)
+			break;
 		if(seqnum_higher(channel->next_incoming_seqnum, firstseqnum))
 			channel->incoming_reliables.popFirst();
 		else
 			break;
 	}
 	// This happens if all packets are old
-	}catch(con::NotFoundException)
-	{}
 	
 	if(channel->incoming_reliables.empty() == false)
 	{

src/connection.h

@@ -281,7 +281,7 @@ public:
 	u32 size();
 	RPBSearchResult findPacket(u16 seqnum);
 	RPBSearchResult notFound();
-	u16 getFirstSeqnum();
+	bool getFirstSeqnum(u16 *result);
 	BufferedPacket popFirst();
 	BufferedPacket popSeqnum(u16 seqnum);
 	void insert(BufferedPacket &p);

src/server.cpp

@@ -60,6 +60,14 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "util/serialize.h"
 #include "defaultsettings.h"
 
+class ClientNotFoundException : public BaseException
+{
+public:
+	ClientNotFoundException(const char *s):
+		BaseException(s)
+	{}
+};
+
 void * ServerThread::Thread()
 {
 	ThreadStarted();
@@ -90,6 +98,9 @@ void * ServerThread::Thread()
 		{
 			infostream<<"Server: PeerNotFoundException"<<std::endl;
 		}
+		catch(ClientNotFoundException &e)
+		{
+		}
 		catch(con::ConnectionBindFailed &e)
 		{
 			m_server->setAsyncFatalError(e.what());
@@ -1766,8 +1777,7 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 					<<addr_s<<"; banned name was "
 					<<m_banmanager.getBanName(addr_s)<<std::endl;
 			// This actually doesn't seem to transfer to the client
-			SendAccessDenied(m_con, peer_id,
+			DenyAccess(peer_id, L"Your ip is banned. Banned name was "
-					L"Your ip is banned. Banned name was "
 					+narrow_to_wide(m_banmanager.getBanName(addr_s)));
 			m_con.DeletePeer(peer_id);
 			return;
@@ -1803,6 +1813,15 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 		verbosestream<<"Server: Got TOSERVER_INIT from "
 				<<peer_id<<std::endl;
 
+		// Do not allow multiple players in simple singleplayer mode.
+		// This isn't a perfect way to do it, but will suffice for now.
+		if(m_simple_singleplayer_mode && m_clients.size() > 1){
+			infostream<<"Server: Not allowing another client to connect in"
+					<<" simple singleplayer mode"<<std::endl;
+			DenyAccess(peer_id, L"Running in simple singleplayer mode.");
+			return;
+		}
+
 		// First byte after command is maximum supported
 		// serialization version
 		u8 client_max = data[2];
@@ -1823,7 +1842,7 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 			infostream<<"Server: Cannot negotiate "
 					"serialization version with peer "
 					<<peer_id<<std::endl;
-			SendAccessDenied(m_con, peer_id, std::wstring(
+			DenyAccess(peer_id, std::wstring(
 					L"Your client's version is not supported.\n"
 					L"Server version is ")
 					+ narrow_to_wide(VERSION_STRING) + L"."
@@ -1871,7 +1890,7 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 		{
 			actionstream<<"Server: A mismatched client tried to connect from "<<addr_s
 					<<std::endl;
-			SendAccessDenied(m_con, peer_id, std::wstring(
+			DenyAccess(peer_id, std::wstring(
 					L"Your client's version is not supported.\n"
 					L"Server version is ")
 					+ narrow_to_wide(VERSION_STRING) + L",\n"
@@ -1893,7 +1912,7 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 			{
 				actionstream<<"Server: A mismatched (strict) client tried to "
 						<<"connect from "<<addr_s<<std::endl;
-				SendAccessDenied(m_con, peer_id, std::wstring(
+				DenyAccess(peer_id, std::wstring(
 						L"Your client's version is not supported.\n"
 						L"Server version is ")
 						+ narrow_to_wide(VERSION_STRING) + L",\n"
@@ -1924,8 +1943,7 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 		{
 			actionstream<<"Server: Player with an empty name "
 					<<"tried to connect from "<<addr_s<<std::endl;
-			SendAccessDenied(m_con, peer_id,
+			DenyAccess(peer_id, L"Empty name");
-					L"Empty name");
 			return;
 		}
 
@@ -1933,8 +1951,7 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 		{
 			actionstream<<"Server: Player with an invalid name "
 					<<"tried to connect from "<<addr_s<<std::endl;
-			SendAccessDenied(m_con, peer_id,
+			DenyAccess(peer_id, L"Name contains unallowed characters");
-					L"Name contains unallowed characters");
 			return;
 		}
 
@@ -1942,8 +1959,7 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 		{
 			actionstream<<"Server: Player with an invalid name "
 					<<"tried to connect from "<<addr_s<<std::endl;
-			SendAccessDenied(m_con, peer_id,
+			DenyAccess(peer_id, L"Name is not allowed");
-					L"Name is not allowed");
 			return;
 		}
 
@@ -1967,9 +1983,25 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 		}
 
 		if(!base64_is_valid(given_password)){
-			infostream<<"Server: "<<playername
+			actionstream<<"Server: "<<playername
 					<<" supplied invalid password hash"<<std::endl;
-			SendAccessDenied(m_con, peer_id, L"Invalid password hash");
+			DenyAccess(peer_id, L"Invalid password hash");
+			return;
+		}
+
+		// Enforce user limit.
+		// Don't enforce for users that have some admin right
+		if(m_clients.size() >= g_settings->getU16("max_users") &&
+				!checkPriv(playername, "server") &&
+				!checkPriv(playername, "ban") &&
+				!checkPriv(playername, "privs") &&
+				!checkPriv(playername, "password") &&
+				playername != g_settings->get("name"))
+		{
+			actionstream<<"Server: "<<playername<<" tried to join, but there"
+					<<" are already max_users="
+					<<g_settings->getU16("max_users")<<" players."<<std::endl;
+			DenyAccess(peer_id, L"Too many users.");
 			return;
 		}
 
@@ -1981,7 +2013,9 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 			if(!isSingleplayer() &&
 					g_settings->getBool("disallow_empty_password") &&
 					std::string(given_password) == ""){
-				SendAccessDenied(m_con, peer_id, L"Empty passwords are "
+				actionstream<<"Server: "<<playername
+						<<" supplied empty password"<<std::endl;
+				DenyAccess(peer_id, L"Empty passwords are "
 						L"disallowed. Set a password and try again.");
 				return;
 			}
@@ -2000,41 +2034,16 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 		has_auth = m_script->getAuth(playername, &checkpwd, NULL);
 
 		if(!has_auth){
-			SendAccessDenied(m_con, peer_id, L"Not allowed to login");
+			actionstream<<"Server: "<<playername<<" cannot be authenticated"
+					<<" (auth handler does not work?)"<<std::endl;
+			DenyAccess(peer_id, L"Not allowed to login");
 			return;
 		}
 
 		if(given_password != checkpwd){
-			infostream<<"Server: peer_id="<<peer_id
+			actionstream<<"Server: "<<playername<<" supplied invalid password"
-					<<": supplied invalid password for "
+					<<" (peer_id="<<peer_id<<")"<<std::endl;
-					<<playername<<std::endl;
+			DenyAccess(peer_id, L"Invalid password");
-			SendAccessDenied(m_con, peer_id, L"Invalid password");
-			return;
-		}
-
-		// Do not allow multiple players in simple singleplayer mode.
-		// This isn't a perfect way to do it, but will suffice for now.
-		if(m_simple_singleplayer_mode && m_clients.size() > 1){
-			infostream<<"Server: Not allowing another client to connect in"
-					<<" simple singleplayer mode"<<std::endl;
-			SendAccessDenied(m_con, peer_id,
-					L"Running in simple singleplayer mode.");
-			return;
-		}
-
-		// Enforce user limit.
-		// Don't enforce for users that have some admin right
-		if(m_clients.size() >= g_settings->getU16("max_users") &&
-				!checkPriv(playername, "server") &&
-				!checkPriv(playername, "ban") &&
-				!checkPriv(playername, "privs") &&
-				!checkPriv(playername, "password") &&
-				playername != g_settings->get("name"))
-		{
-			actionstream<<"Server: "<<playername<<" tried to join, but there"
-					<<" are already max_users="
-					<<g_settings->getU16("max_users")<<" players."<<std::endl;
-			SendAccessDenied(m_con, peer_id, L"Too many users.");
 			return;
 		}
 
@@ -2046,6 +2055,8 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 		{
 			errorstream<<"Server: peer_id="<<peer_id
 					<<": failed to emerge player"<<std::endl;
+			DenyAccess(peer_id, L"Could not allocate player. You"
+					" may need to wait for a timeout.");
 			return;
 		}
 
@@ -4232,7 +4243,11 @@ void Server::SendBlocks(float dtime)
 			continue;
 		}
 
-		RemoteClient *client = getClient(q.peer_id);
+		RemoteClient *client = getClientNoEx(q.peer_id);
+		if(!client)
+			continue;
+		if(client->denied)
+			continue;
 
 		SendBlockNoLock(q.peer_id, block, client->serialization_version, client->net_proto_version);
 
@@ -4618,6 +4633,139 @@ void Server::RespawnPlayer(u16 peer_id)
 	}
 }
 
+void Server::DenyAccess(u16 peer_id, const std::wstring &reason)
+{
+	DSTACK(__FUNCTION_NAME);
+
+	SendAccessDenied(m_con, peer_id, reason);
+
+	RemoteClient *client = getClientNoEx(peer_id);
+	if(client)
+		client->denied = true;
+
+	// If there are way too many clients, get rid of denied new ones immediately
+	if(m_clients.size() > 2 * g_settings->getU16("max_users")){
+		// Delete peer to stop sending it data
+		m_con.DeletePeer(peer_id);
+		// Delete client also to stop block sends and other stuff
+		DeleteClient(peer_id, CDR_DENY);
+	}
+}
+
+void Server::DeleteClient(u16 peer_id, ClientDeletionReason reason)
+{
+	DSTACK(__FUNCTION_NAME);
+
+	// Error check
+	std::map<u16, RemoteClient*>::iterator n;
+	n = m_clients.find(peer_id);
+	// The client may not exist; clients are immediately removed if their
+	// access is denied, and this event occurs later then.
+	if(n == m_clients.end())
+		return;
+
+	/*
+		Mark objects to be not known by the client
+	*/
+	RemoteClient *client = n->second;
+	// Handle objects
+	for(std::set<u16>::iterator
+			i = client->m_known_objects.begin();
+			i != client->m_known_objects.end(); ++i)
+	{
+		// Get object
+		u16 id = *i;
+		ServerActiveObject* obj = m_env->getActiveObject(id);
+
+		if(obj && obj->m_known_by_count > 0)
+			obj->m_known_by_count--;
+	}
+
+	/*
+		Clear references to playing sounds
+	*/
+	for(std::map<s32, ServerPlayingSound>::iterator
+			i = m_playing_sounds.begin();
+			i != m_playing_sounds.end();)
+	{
+		ServerPlayingSound &psound = i->second;
+		psound.clients.erase(peer_id);
+		if(psound.clients.size() == 0)
+			m_playing_sounds.erase(i++);
+		else
+			i++;
+	}
+
+	Player *player = m_env->getPlayer(peer_id);
+
+	// Collect information about leaving in chat
+	std::wstring message;
+	{
+		if(player != NULL && reason != CDR_DENY)
+		{
+			std::wstring name = narrow_to_wide(player->getName());
+			message += L"*** ";
+			message += name;
+			message += L" left the game.";
+			if(reason == CDR_TIMEOUT)
+				message += L" (timed out)";
+		}
+	}
+
+	/* Run scripts and remove from environment */
+	{
+		if(player != NULL)
+		{
+			PlayerSAO *playersao = player->getPlayerSAO();
+			assert(playersao);
+
+			m_script->on_leaveplayer(playersao);
+
+			playersao->disconnected();
+		}
+	}
+
+	/*
+		Print out action
+	*/
+	{
+		if(player != NULL && reason != CDR_DENY)
+		{
+			std::ostringstream os(std::ios_base::binary);
+			for(std::map<u16, RemoteClient*>::iterator
+				i = m_clients.begin();
+				i != m_clients.end(); ++i)
+			{
+				RemoteClient *client = i->second;
+				assert(client->peer_id == i->first);
+				if(client->serialization_version == SER_FMT_VER_INVALID)
+					continue;
+				// Get player
+				Player *player = m_env->getPlayer(client->peer_id);
+				if(!player)
+					continue;
+				// Get name of player
+				os<<player->getName()<<" ";
+			}
+
+			actionstream<<player->getName()<<" "
+					<<(reason==CDR_TIMEOUT?"times out.":"leaves game.")
+					<<" List of players: "<<os.str()<<std::endl;
+		}
+	}
+
+	// Delete client
+	delete m_clients[peer_id];
+	m_clients.erase(peer_id);
+
+	// Send player info to all remaining clients
+	//SendPlayerInfos();
+
+	// Send leave chat message to all remaining clients
+	if(message.length() != 0)
+		BroadcastChatMessage(message);
+}
+
 void Server::UpdateCrafting(u16 peer_id)
 {
 	DSTACK(__FUNCTION_NAME);
@@ -4638,12 +4786,19 @@ void Server::UpdateCrafting(u16 peer_id)
 
 RemoteClient* Server::getClient(u16 peer_id)
 {
-	DSTACK(__FUNCTION_NAME);
+	RemoteClient *client = getClientNoEx(peer_id);
-	//JMutexAutoLock lock(m_con_mutex);
+	if(!client)
+		throw ClientNotFoundException("Client not found");
+	return client;
+}
+RemoteClient* Server::getClientNoEx(u16 peer_id)
+{
 	std::map<u16, RemoteClient*>::iterator n;
 	n = m_clients.find(peer_id);
-	// A client should exist for all peers
+	// The client may not exist; clients are immediately removed if their
-	assert(n != m_clients.end());
+	// access is denied, and this event occurs later then.
+	if(n == m_clients.end())
+		return NULL;
 	return n->second;
 }
 
@@ -5249,113 +5404,7 @@ void Server::handlePeerChange(PeerChange &c)
 			Delete
 		*/
 
-		// Error check
+		DeleteClient(c.peer_id, c.timeout?CDR_TIMEOUT:CDR_LEAVE);
-		std::map<u16, RemoteClient*>::iterator n;
-		n = m_clients.find(c.peer_id);
-		// The client should exist
-		assert(n != m_clients.end());
-
-		/*
-			Mark objects to be not known by the client
-		*/
-		RemoteClient *client = n->second;
-		// Handle objects
-		for(std::set<u16>::iterator
-				i = client->m_known_objects.begin();
-				i != client->m_known_objects.end(); ++i)
-		{
-			// Get object
-			u16 id = *i;
-			ServerActiveObject* obj = m_env->getActiveObject(id);
-
-			if(obj && obj->m_known_by_count > 0)
-				obj->m_known_by_count--;
-		}
-
-		/*
-			Clear references to playing sounds
-		*/
-		for(std::map<s32, ServerPlayingSound>::iterator
-				i = m_playing_sounds.begin();
-				i != m_playing_sounds.end();)
-		{
-			ServerPlayingSound &psound = i->second;
-			psound.clients.erase(c.peer_id);
-			if(psound.clients.size() == 0)
-				m_playing_sounds.erase(i++);
-			else
-				i++;
-		}
-
-		Player *player = m_env->getPlayer(c.peer_id);
-
-		// Collect information about leaving in chat
-		std::wstring message;
-		{
-			if(player != NULL)
-			{
-				std::wstring name = narrow_to_wide(player->getName());
-				message += L"*** ";
-				message += name;
-				message += L" left the game.";
-				if(c.timeout)
-					message += L" (timed out)";
-			}
-		}
-
-		/* Run scripts and remove from environment */
-		{
-			if(player != NULL)
-			{
-				PlayerSAO *playersao = player->getPlayerSAO();
-				assert(playersao);
-
-				m_script->on_leaveplayer(playersao);
-
-				playersao->disconnected();
-			}
-		}
-
-		/*
-			Print out action
-		*/
-		{
-			if(player != NULL)
-			{
-				std::ostringstream os(std::ios_base::binary);
-				for(std::map<u16, RemoteClient*>::iterator
-					i = m_clients.begin();
-					i != m_clients.end(); ++i)
-				{
-					RemoteClient *client = i->second;
-					assert(client->peer_id == i->first);
-					if(client->serialization_version == SER_FMT_VER_INVALID)
-						continue;
-					// Get player
-					Player *player = m_env->getPlayer(client->peer_id);
-					if(!player)
-						continue;
-					// Get name of player
-					os<<player->getName()<<" ";
-				}
-
-				actionstream<<player->getName()<<" "
-						<<(c.timeout?"times out.":"leaves game.")
-						<<" List of players: "
-						<<os.str()<<std::endl;
-			}
-		}
-
-		// Delete client
-		delete m_clients[c.peer_id];
-		m_clients.erase(c.peer_id);
-
-		// Send player info to all remaining clients
-		//SendPlayerInfos();
-
-		// Send leave chat message to all remaining clients
-		if(message.length() != 0)
-			BroadcastChatMessage(message);
 
 	} // PEER_REMOVED
 	else

src/server.h

@@ -250,6 +250,8 @@ public:
 
 	bool definitions_sent;
 
+	bool denied;
+
 	RemoteClient():
 		m_time_from_building(9999),
 		m_excess_gotblocks(0)
@@ -259,6 +261,7 @@ public:
 		net_proto_version = 0;
 		pending_serialization_version = SER_FMT_VER_INVALID;
 		definitions_sent = false;
+		denied = false;
 		m_nearest_unsent_d = 0;
 		m_nearest_unsent_reset_timer = 0.0;
 		m_nothing_to_send_counter = 0;
@@ -589,7 +592,7 @@ private:
 	void SendHUDChange(u16 peer_id, u32 id, HudElementStat stat, void *value);
 	void SendHUDSetFlags(u16 peer_id, u32 flags, u32 mask);
 	void SendHUDSetParam(u16 peer_id, u16 param, const std::string &value);
-	
+
 	/*
 		Send a node removal/addition event to all clients except ignore_id.
 		Additionally, if far_players!=NULL, players further away than
@@ -658,11 +661,20 @@ private:
 
 	void DiePlayer(u16 peer_id);
 	void RespawnPlayer(u16 peer_id);
+	void DenyAccess(u16 peer_id, const std::wstring &reason);
+
+	enum ClientDeletionReason {
+		CDR_LEAVE,
+		CDR_TIMEOUT,
+		CDR_DENY
+	};
+	void DeleteClient(u16 peer_id, ClientDeletionReason reason);
 
 	void UpdateCrafting(u16 peer_id);
 
 	// When called, connection mutex should be locked
 	RemoteClient* getClient(u16 peer_id);
+	RemoteClient* getClientNoEx(u16 peer_id);
 
 	// When called, environment mutex should be locked
 	std::string getPlayerName(u16 peer_id)
@@ -737,7 +749,7 @@ private:
 	std::map<u16, RemoteClient*> m_clients;
 	u16 m_clients_number; //for announcing masterserver
 
-	// Bann checking
+	// Ban checking
 	BanManager m_banmanager;
 
 	// Rollback manager (behind m_env_mutex)