- Introduce new NET_CompareBaseAdrMask for easy comparison of ip address ranges

- Overhaul of the new banning functions:
  * basic check for redundant bans/exceptions
  * introduction of sv_banFile to make it possible to configure the file where to read bans and exceptions from
  * bans can now be deleted by giving address ranges, too.



git-svn-id: svn://svn.icculus.org/quake3/trunk@1557 edf5b092-35ff-0310-97b2-ce42778d08ea

5 files changed, 230 insertions, 132 deletions

diff --git a/code/server/server.h b/code/server/server.h
index d3c49c9..a194e45 100644
--- a/code/server/server.h
+++ b/code/server/server.h
@@ -234,7 +234,6 @@ typedef struct {
 } serverStatic_t;
 
 #define SERVER_MAXBANS	1024
-#define SERVER_BANFILE	"serverbans.dat"
 // Structure for managing bans
 typedef struct
 {
@@ -280,6 +279,7 @@ extern	cvar_t	*sv_pure;
 extern	cvar_t	*sv_floodProtect;
 extern	cvar_t	*sv_lanForceRate;
 extern	cvar_t	*sv_strictAuth;
+extern	cvar_t	*sv_banFile;
 
 extern	serverBan_t serverBans[SERVER_MAXBANS];
 extern	int serverBansCount;
diff --git a/code/server/sv_ccmds.c b/code/server/sv_ccmds.c
index 7d87960..482e4a2 100644
--- a/code/server/sv_ccmds.c
+++ b/code/server/sv_ccmds.c
@@ -527,10 +527,13 @@ static void SV_RehashBans_f(void)
 	
 	serverBansCount = 0;
 	
+	if(!sv_banFile->string || !*sv_banFile->string)
+		return;
+
 	if(!(curpos = Cvar_VariableString("fs_game")) || !*curpos)
 		curpos = BASEGAME;
 	
-	Com_sprintf(filepath, sizeof(filepath), "%s/%s", curpos, SERVER_BANFILE);
+	Com_sprintf(filepath, sizeof(filepath), "%s/%s", curpos, sv_banFile->string);
 
 	if((filelen = FS_SV_FOpenFileRead(filepath, &readfrom)) >= 0)
 	{
@@ -573,12 +576,12 @@ static void SV_RehashBans_f(void)
 				serverBans[index].subnet = atoi(maskpos);
 				
 				if(serverBans[index].ip.type == NA_IP &&
-				   (serverBans[index].subnet < 0 || serverBans[index].subnet > 32))
+				   (serverBans[index].subnet < 1 || serverBans[index].subnet > 32))
 				{
 					serverBans[index].subnet = 32;
 				}
 				else if(serverBans[index].ip.type == NA_IP6 &&
-					(serverBans[index].subnet < 0 || serverBans[index].subnet > 128))
+					(serverBans[index].subnet < 1 || serverBans[index].subnet > 128))
 				{
 					serverBans[index].subnet = 128;
 				}
@@ -595,6 +598,113 @@ static void SV_RehashBans_f(void)
 
 /*
 ==================
+SV_WriteBans_f
+
+Save bans to file.
+==================
+*/
+static void SV_WriteBans(void)
+{
+	int index;
+	fileHandle_t writeto;
+	char *curpos, filepath[MAX_QPATH];
+	
+	if(!sv_banFile->string || !*sv_banFile->string)
+		return;
+	
+	if(!(curpos = Cvar_VariableString("fs_game")) || !*curpos)
+		curpos = BASEGAME;
+	
+	Com_sprintf(filepath, sizeof(filepath), "%s/%s", curpos, sv_banFile->string);
+
+	if((writeto = FS_SV_FOpenFileWrite(filepath)))
+	{
+		char writebuf[128];
+		serverBan_t *curban;
+		
+		for(index = 0; index < serverBansCount; index++)
+		{
+			curban = &serverBans[index];
+			
+			Com_sprintf(writebuf, sizeof(writebuf), "%d %s %d\n",
+				    curban->isexception, NET_AdrToString(curban->ip), curban->subnet);
+			FS_Write(writebuf, strlen(writebuf), writeto);
+		}
+
+		FS_FCloseFile(writeto);
+	}
+}
+
+/*
+==================
+SV_DelBanEntryFromList
+
+Remove a ban or an exception from the list.
+==================
+*/
+
+static qboolean SV_DelBanEntryFromList(int index)
+{
+	if(index == serverBansCount - 1)
+		serverBansCount--;
+	else if(index < sizeof(serverBans) / sizeof(*serverBans) - 1)
+	{
+		memmove(serverBans + index, serverBans + index + 1, (serverBansCount - index - 1) * sizeof(*serverBans));
+		serverBansCount--;
+	}
+	else
+		return qtrue;
+
+	return qfalse;
+}
+
+/*
+==================
+SV_ParseCIDRNotation
+
+Parse a CIDR notation type string and return a netadr_t and suffix by reference
+==================
+*/
+
+static qboolean SV_ParseCIDRNotation(netadr_t *dest, int *mask, char *adrstr)
+{
+	char *suffix;
+	
+	suffix = strchr(adrstr, '/');
+	if(suffix)
+	{
+		*suffix = '\0';
+		suffix++;
+	}
+
+	if(!NET_StringToAdr(adrstr, dest, NA_UNSPEC))
+		return qtrue;
+
+	if(suffix)
+	{
+		*mask = atoi(suffix);
+		
+		if(dest->type == NA_IP)
+		{
+			if(*mask < 1 || *mask > 32)
+				*mask = 32;
+		}
+		else
+		{
+			if(*mask < 1 || *mask > 128)
+				*mask = 128;
+		}
+	}
+	else if(dest->type == NA_IP)
+		*mask = 32;
+	else
+		*mask = 128;
+	
+	return qfalse;
+}
+
+/*
+==================
 SV_AddBanToList
 
 Ban a user from being able to play on this server based on his ip address.
@@ -603,10 +713,10 @@ Ban a user from being able to play on this server based on his ip address.
 
 static void SV_AddBanToList(qboolean isexception)
 {
-	char *banstring, *suffix;
+	char *banstring;
 	netadr_t ip;
-	int argc, mask;
-	fileHandle_t writeto;
+	int index, argc, mask;
+	serverBan_t *curban;
 	
 	argc = Cmd_Argc();
 	
@@ -628,15 +738,7 @@ static void SV_AddBanToList(qboolean isexception)
 	{
 		// This is an ip address, not a client num.
 		
-		// Look for a CIDR-Notation suffix
-		suffix = strchr(banstring, '/');
-		if(suffix)
-		{
-			*suffix = '\0';
-			suffix++;
-		}
-		
-		if(!NET_StringToAdr(banstring, &ip, NA_UNSPEC))
+		if(SV_ParseCIDRNotation(&ip, &mask, banstring))
 		{
 			Com_Printf("Error: Invalid address %s\n", banstring);
 			return;
@@ -664,9 +766,22 @@ static void SV_AddBanToList(qboolean isexception)
 		ip = cl->netchan.remoteAddress;
 		
 		if(argc == 3)
-			suffix = Cmd_Argv(2);
+		{
+			mask = atoi(Cmd_Argv(2));
+			
+			if(ip.type == NA_IP)
+			{
+				if(mask < 1 || mask > 32)
+					mask = 32;
+			}
+			else
+			{
+				if(mask < 1 || mask > 128)
+					mask = 128;
+			}
+		}
 		else
-			suffix = NULL;
+			mask = (ip.type == NA_IP6) ? 128 : 32;
 	}
 
 	if(ip.type != NA_IP && ip.type != NA_IP6)
@@ -675,44 +790,55 @@ static void SV_AddBanToList(qboolean isexception)
 		return;
 	}
 
-	if(suffix)
+	// first check whether a conflicting ban exists that would supersede the new one.
+	for(index = 0; index < serverBansCount; index++)
 	{
-		mask = atoi(suffix);
+		curban = &serverBans[index];
 		
-		if(ip.type == NA_IP)
+		if(curban->subnet <= mask)
 		{
-			if(mask < 0 || mask > 32)
-				mask = 32;
+			if((curban->isexception || !isexception) && NET_CompareBaseAdrMask(curban->ip, ip, curban->subnet))
+			{
+				Com_Printf("Error: %s %s/%d supersedes %s %s/%d\n", curban->isexception ? "Exception" : "Ban",
+					   NET_AdrToString(curban->ip), curban->subnet,
+					   isexception ? "exception" : "ban", NET_AdrToString(ip), mask);
+				return;
+			}
 		}
-		else
+		if(curban->subnet >= mask)
 		{
-			if(mask < 0 || mask > 128)
-				mask = 128;
+			if(!curban->isexception && isexception && NET_CompareBaseAdrMask(curban->ip, ip, mask))
+			{
+				Com_Printf("Error: %s %s/%d supersedes already existing %s %s/%d\n", isexception ? "Exception" : "Ban",
+					   NET_AdrToString(ip), mask,
+					   curban->isexception ? "exception" : "ban", NET_AdrToString(curban->ip), curban->subnet);
+				return;
+			}
 		}
 	}
-	else if(ip.type == NA_IP)
-		mask = 32;
-	else
-		mask = 128;
-	
-	serverBans[serverBansCount].ip = ip;
-	serverBans[serverBansCount].subnet = mask;
-	serverBans[serverBansCount].isexception = isexception;
-	
-	Com_Printf("Added %s: %s/%d\n", isexception ? "ban exception" : "ban",
-		   NET_AdrToString(ip), mask);
 
-	// Write out the ban information.
-	if((writeto = FS_FOpenFileAppend(SERVER_BANFILE)))
+	// now delete bans that are superseded by the new one
+	index = 0;
+	while(index < serverBansCount)
 	{
-		char writebuf[128];
+		curban = &serverBans[index];
 		
-		Com_sprintf(writebuf, sizeof(writebuf), "%d %s %d\n", isexception, NET_AdrToString(ip), mask);
-		FS_Write(writebuf, strlen(writebuf), writeto);
-		FS_FCloseFile(writeto);
+		if(curban->subnet > mask && (!curban->isexception || isexception) && NET_CompareBaseAdrMask(curban->ip, ip, mask))
+			SV_DelBanEntryFromList(index);
+		else
+			index++;
 	}
+
+	serverBans[serverBansCount].ip = ip;
+	serverBans[serverBansCount].subnet = mask;
+	serverBans[serverBansCount].isexception = isexception;
 	
 	serverBansCount++;
+	
+	SV_WriteBans();
+
+	Com_Printf("Added %s: %s/%d\n", isexception ? "ban exception" : "ban",
+		   NET_AdrToString(ip), mask);
 }
 
 /*
@@ -725,63 +851,82 @@ Remove a ban or an exception from the list.
 
 static void SV_DelBanFromList(qboolean isexception)
 {
-	int index, count, todel;
-	fileHandle_t writeto;
+	int index, count = 0, todel, mask;
+	netadr_t ip;
+	char *banstring;
 	
 	if(Cmd_Argc() != 2)
 	{
-		Com_Printf ("Usage: %s <num>\n", Cmd_Argv(0));
+		Com_Printf ("Usage: %s (ip[/subnet] | num)\n", Cmd_Argv(0));
 		return;
 	}
 
-	todel = atoi(Cmd_Argv(1));
-
-	if(todel < 0 || todel > serverBansCount)
-		return;
+	banstring = Cmd_Argv(1);
 	
-	for(index = count = 0; index < serverBansCount; index++)
+	if(strchr(banstring, '.') || strchr(banstring, ':'))
 	{
-		if(serverBans[index].isexception == isexception)
+		serverBan_t *curban;
+		
+		if(SV_ParseCIDRNotation(&ip, &mask, banstring))
 		{
-			count++;
+			Com_Printf("Error: Invalid address %s\n", banstring);
+			return;
+		}
+		
+		index = 0;
+		
+		while(index < serverBansCount)
+		{
+			curban = &serverBans[index];
 			
-			if(count == todel)
-				break;
+			if(curban->isexception == isexception		&&
+			   curban->subnet >= mask 			&&
+			   NET_CompareBaseAdrMask(curban->ip, ip, mask))
+			{
+				Com_Printf("Deleting %s %s/%d\n",
+					   isexception ? "exception" : "ban",
+					   NET_AdrToString(curban->ip), curban->subnet);
+					   
+				SV_DelBanEntryFromList(index);
+			}
+			else
+				index++;
 		}
 	}
-	
-	if(index == serverBansCount - 1)
-		serverBansCount--;
-	else if(index < sizeof(serverBans) / sizeof(*serverBans) - 1)
-	{
-		memmove(serverBans + index, serverBans + index + 1, (serverBansCount - index - 1) * sizeof(*serverBans));
-		serverBansCount--;
-	}
 	else
 	{
-		Com_Printf("Error: No such entry #%d\n", todel);
-		return;
-	}
+		todel = atoi(Cmd_Argv(1));
+
+		if(todel < 1 || todel > serverBansCount)
+		{
+			Com_Printf("Error: Invalid ban number given\n");
+			return;
+		}
 	
-	// Write out the ban information.
-	if((writeto = FS_FOpenFileWrite(SERVER_BANFILE)))
-	{
-		char writebuf[128];
-		serverBan_t *curban;
-		
 		for(index = 0; index < serverBansCount; index++)
 		{
-			curban = &serverBans[index];
+			if(serverBans[index].isexception == isexception)
+			{
+				count++;
 			
-			Com_sprintf(writebuf, sizeof(writebuf), "%d %s %d\n",
-				    curban->isexception, NET_AdrToString(curban->ip), curban->subnet);
-			FS_Write(writebuf, strlen(writebuf), writeto);
-		}
+				if(count == todel)
+				{
+					Com_Printf("Deleting %s %s/%d\n",
+					   isexception ? "exception" : "ban",
+					   NET_AdrToString(serverBans[index].ip), serverBans[index].subnet);
 
-		FS_FCloseFile(writeto);
+					SV_DelBanEntryFromList(index);
+
+					break;
+				}
+			}
+		}
 	}
+	
+	SV_WriteBans();
 }
 
+
 /*
 ==================
 SV_ListBans_f
@@ -831,15 +976,12 @@ Delete all bans and exceptions.
 
 static void SV_FlushBans_f(void)
 {
-	fileHandle_t blankf;
-	
 	serverBansCount = 0;
 	
 	// empty the ban file.
-	blankf = FS_FOpenFileWrite(SERVER_BANFILE);
+	SV_WriteBans();
 	
-	if(blankf)
-		FS_FCloseFile(blankf);
+	Com_Printf("All bans and exceptions have been deleted.\n");
 }
 
 static void SV_BanAddr_f(void)
diff --git a/code/server/sv_client.c b/code/server/sv_client.c
index 5ef1336..e208a5b 100644
--- a/code/server/sv_client.c
+++ b/code/server/sv_client.c
@@ -234,18 +234,9 @@ Check whether a certain address is banned
 
 static qboolean SV_IsBanned(netadr_t *from, qboolean isexception)
 {
-	int index, addrlen, curbyte, netmask, cmpmask;
+	int index;
 	serverBan_t *curban;
-	byte *addrfrom, *addrban;
-	qboolean differed;
 	
-	if(from->type == NA_IP)
-		addrlen = sizeof(from->ip);
-	else if(from->type == NA_IP6)
-		addrlen = sizeof(from->ip6);
-	else
-		return qfalse;
-
 	if(!isexception)
 	{
 		// If this is a query for a ban, first check whether the client is excepted
@@ -257,47 +248,10 @@ static qboolean SV_IsBanned(netadr_t *from, qboolean isexception)
 	{
 		curban = &serverBans[index];
 		
-		if(curban->isexception == isexception && from->type == curban->ip.type)
+		if(curban->isexception == isexception)
 		{
-			if(from->type == NA_IP)
-			{
-				addrfrom = from->ip;
-				addrban = curban->ip.ip;
-			}
-			else
-			{
-				addrfrom = from->ip6;
-				addrban = curban->ip.ip6;
-			}
-			
-			differed = qfalse;
-			curbyte = 0;
-			
-			for(netmask = curban->subnet; netmask > 7; netmask -= 8)
-			{
-				if(addrfrom[curbyte] != addrban[curbyte])
-				{
-					differed = qtrue;
-					break;
-				}
-				
-				curbyte++;
-			}
-			
-			if(differed)
-				continue;
-				
-			if(netmask)
-			{
-				cmpmask = (1 << netmask) - 1;
-				cmpmask <<= 8 - netmask;
-				
-				if((addrfrom[curbyte] & cmpmask) == (addrban[curbyte] & cmpmask))
-					return qtrue;
-			}
-			else
+			if(NET_CompareBaseAdrMask(curban->ip, *from, curban->subnet))
 				return qtrue;
-			
 		}
 	}
 	
diff --git a/code/server/sv_init.c b/code/server/sv_init.c
index 41ac351..874aeaf 100644
--- a/code/server/sv_init.c
+++ b/code/server/sv_init.c
@@ -683,6 +683,7 @@ void SV_Init (void) {
 	sv_mapChecksum = Cvar_Get ("sv_mapChecksum", "", CVAR_ROM);
 	sv_lanForceRate = Cvar_Get ("sv_lanForceRate", "1", CVAR_ARCHIVE );
 	sv_strictAuth = Cvar_Get ("sv_strictAuth", "1", CVAR_ARCHIVE );
+	sv_banFile = Cvar_Get("sv_banFile", "serverbans.dat", CVAR_ARCHIVE);
 
 	// initialize bot cvars so they are listed and can be set before loading the botlib
 	SV_BotInitCvars();
diff --git a/code/server/sv_main.c b/code/server/sv_main.c
index b509010..bce9c3d 100644
--- a/code/server/sv_main.c
+++ b/code/server/sv_main.c
@@ -57,6 +57,7 @@ cvar_t	*sv_pure;
 cvar_t	*sv_floodProtect;
 cvar_t	*sv_lanForceRate; // dedicated 1 (LAN) server forces local client rates to 99999 (bug #491)
 cvar_t	*sv_strictAuth;
+cvar_t	*sv_banFile;
 
 serverBan_t serverBans[SERVER_MAXBANS];
 int serverBansCount = 0;