- 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.

README

@@ -181,6 +181,8 @@ New cvars
                                       for more information
   r_sdlDriver                       - read only, indicates the SDL driver
                                       backend being used
+  sv_banFile                        - Name of the file that is used for storing
+                                      the server bans.
 
 New commands
   video [filename]        - start video capture (use with demo command)
@@ -192,8 +194,8 @@ New commands
                             server, valid <range> is either playernum or CIDR
                             notation address range.
   exceptaddr <range>      - exempt an ip address range from a ban.
-  bandel <num>            - delete ban <num>
+  bandel <range>          - delete ban (either range or ban number)
-  exceptdel <num>         - delete exception <num>
+  exceptdel <range>       - delete exception (either range or exception number)
   listbans                - list all currently active bans and exceptions
   rehashbans              - reload the banlist from serverbans.dat
   flushbans               - delete all bans

code/qcommon/net_ip.c

@@ -380,6 +380,80 @@ qboolean Sys_StringToAdr( const char *s, netadr_t *a, netadrtype_t family ) {
 	return qtrue;
 }
 
+/*
+===================
+NET_CompareBaseAdrMask
+
+Compare without port, and up to the bit number given in netmask.
+===================
+*/
+qboolean NET_CompareBaseAdrMask(netadr_t a, netadr_t b, int netmask)
+{
+	qboolean differed;
+	byte cmpmask, *addra, *addrb;
+	int curbyte;
+	
+	if (a.type != b.type)
+		return qfalse;
+
+	if (a.type == NA_LOOPBACK)
+		return qtrue;
+
+	if(a.type == NA_IP)
+	{
+		addra = (byte *) &a.ip;
+		addrb = (byte *) &b.ip;
+		
+		if(netmask < 0 || netmask > 32)
+			netmask = 32;
+	}
+	else if(a.type == NA_IP6)
+	{
+		addra = (byte *) &a.ip6;
+		addrb = (byte *) &b.ip6;
+		
+		if(netmask < 0 || netmask > 128)
+			netmask = 128;
+	}
+	else
+	{
+		Com_Printf ("NET_CompareBaseAdr: bad address type\n");
+		return qfalse;
+	}
+
+	differed = qfalse;
+	curbyte = 0;
+
+	while(netmask > 7)
+	{
+		if(addra[curbyte] != addrb[curbyte])
+		{
+			differed = qtrue;
+			break;
+		}
+
+		curbyte++;
+		netmask -= 8;
+	}
+
+	if(differed)
+		return qfalse;
+
+	if(netmask)
+	{
+		cmpmask = (1 << netmask) - 1;
+		cmpmask <<= 8 - netmask;
+
+		if((addra[curbyte] & cmpmask) == (addrb[curbyte] & cmpmask))
+			return qtrue;
+	}
+	else
+		return qtrue;
+	
+	return qfalse;
+}
+
+
 /*
 ===================
 NET_CompareBaseAdr
@@ -389,30 +463,7 @@ Compares without the port
 */
 qboolean NET_CompareBaseAdr (netadr_t a, netadr_t b)
 {
-	if (a.type != b.type)
+	return NET_CompareBaseAdrMask(a, b, -1);
-		return qfalse;
-
-	if (a.type == NA_LOOPBACK)
-		return qtrue;
-
-	if (a.type == NA_IP)
-	{
-		if(!memcmp(a.ip, b.ip, sizeof(a.ip)))
-			return qtrue;
-		
-		return qfalse;
-	}
-	
-	if (a.type == NA_IP6)
-	{
-		if(!memcmp(a.ip6, b.ip6, sizeof(a.ip6)) && a.scope_id == b.scope_id)
-				  return qtrue;
-		
-		return qfalse;
-	}
-
-	Com_Printf ("NET_CompareBaseAdr: bad address type\n");
-	return qfalse;
 }
 
 const char	*NET_AdrToString (netadr_t a)

code/qcommon/qcommon.h

@@ -168,6 +168,7 @@ void		QDECL NET_OutOfBandPrint( netsrc_t net_socket, netadr_t adr, const char *f
 void		QDECL NET_OutOfBandData( netsrc_t sock, netadr_t adr, byte *format, int len );
 
 qboolean	NET_CompareAdr (netadr_t a, netadr_t b);
+qboolean	NET_CompareBaseAdrMask(netadr_t a, netadr_t b, int netmask);
 qboolean	NET_CompareBaseAdr (netadr_t a, netadr_t b);
 qboolean	NET_IsLocalAddress (netadr_t adr);
 const char	*NET_AdrToString (netadr_t a);

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;

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;
 				}
@@ -593,6 +596,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
@@ -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;
+	int index, argc, mask;
-	fileHandle_t writeto;
+	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
+		if(SV_ParseCIDRNotation(&ip, &mask, banstring))
-		suffix = strchr(banstring, '/');
-		if(suffix)
-		{
-			*suffix = '\0';
-			suffix++;
-		}
-		
-		if(!NET_StringToAdr(banstring, &ip, NA_UNSPEC))
 		{
 			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
-			suffix = NULL;
+			{
+				if(mask < 1 || mask > 128)
+					mask = 128;
+			}
+		}
+		else
+			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)
+			if((curban->isexception || !isexception) && NET_CompareBaseAdrMask(curban->ip, ip, curban->subnet))
-				mask = 32;
-		}
-		else
 			{
-			if(mask < 0 || mask > 128)
+				Com_Printf("Error: %s %s/%d supersedes %s %s/%d\n", curban->isexception ? "Exception" : "Ban",
-				mask = 128;
+					   NET_AdrToString(curban->ip), curban->subnet,
+					   isexception ? "exception" : "ban", NET_AdrToString(ip), mask);
+				return;
 			}
 		}
-	else if(ip.type == NA_IP)
+		if(curban->subnet >= mask)
-		mask = 32;
+		{
+			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;
+			}
+		}
+	}
+
+	// now delete bans that are superseded by the new one
+	index = 0;
+	while(index < serverBansCount)
+	{
+		curban = &serverBans[index];
+		
+		if(curban->subnet > mask && (!curban->isexception || isexception) && NET_CompareBaseAdrMask(curban->ip, ip, mask))
+			SV_DelBanEntryFromList(index);
 		else
-		mask = 128;
+			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);
-
-	// Write out the ban information.
-	if((writeto = FS_FOpenFileAppend(SERVER_BANFILE)))
-	{
-		char writebuf[128];
-		
-		Com_sprintf(writebuf, sizeof(writebuf), "%d %s %d\n", isexception, NET_AdrToString(ip), mask);
-		FS_Write(writebuf, strlen(writebuf), writeto);
-		FS_FCloseFile(writeto);
-	}
-	
-	serverBansCount++;
 }
 
 /*
@@ -725,63 +851,82 @@ Remove a ban or an exception from the list.
 
 static void SV_DelBanFromList(qboolean isexception)
 {
-	int index, count, todel;
+	int index, count = 0, todel, mask;
-	fileHandle_t writeto;
+	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;
 	}
 
+	banstring = Cmd_Argv(1);
+	
+	if(strchr(banstring, '.') || strchr(banstring, ':'))
+	{
+		serverBan_t *curban;
+		
+		if(SV_ParseCIDRNotation(&ip, &mask, banstring))
+		{
+			Com_Printf("Error: Invalid address %s\n", banstring);
+			return;
+		}
+		
+		index = 0;
+		
+		while(index < serverBansCount)
+		{
+			curban = &serverBans[index];
+			
+			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++;
+		}
+	}
+	else
+	{
 		todel = atoi(Cmd_Argv(1));
 
-	if(todel < 0 || todel > serverBansCount)
+		if(todel < 1 || todel > serverBansCount)
+		{
+			Com_Printf("Error: Invalid ban number given\n");
 			return;
+		}
 	
-	for(index = count = 0; index < serverBansCount; index++)
+		for(index = 0; index < serverBansCount; index++)
 		{
 			if(serverBans[index].isexception == isexception)
 			{
 				count++;
 			
 				if(count == todel)
+				{
+					Com_Printf("Deleting %s %s/%d\n",
+					   isexception ? "exception" : "ban",
+					   NET_AdrToString(serverBans[index].ip), serverBans[index].subnet);
+
+					SV_DelBanEntryFromList(index);
+
 					break;
 				}
 			}
-	
-	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;
 	}
 	
-	// Write out the ban information.
+	SV_WriteBans();
-	if((writeto = FS_FOpenFileWrite(SERVER_BANFILE)))
-	{
-		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_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)
+	Com_Printf("All bans and exceptions have been deleted.\n");
-		FS_FCloseFile(blankf);
 }
 
 static void SV_BanAddr_f(void)

code/server/sv_client.c

@@ -234,17 +234,8 @@ 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)
 	{
@@ -257,48 +248,11 @@ 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)
+			if(NET_CompareBaseAdrMask(curban->ip, *from, curban->subnet))
-			{
-				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
-				return qtrue;
-			
-		}
 	}
 	
 	return qfalse;

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();

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;