From 77e67d86bc53780324f8259f0c799846b73adf69 Mon Sep 17 00:00:00 2001 From: toaster Date: Tue, 14 Jun 2022 16:45:21 +0100 Subject: [PATCH 01/14] Remove MAXBANS Needs proper stress testing but seems to work. # Conflicts: # src/i_tcp.c --- src/d_clisrv.c | 4 +-- src/i_tcp.c | 88 ++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 65 insertions(+), 27 deletions(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index 22bd659c..1e31de51 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -3125,7 +3125,7 @@ static void Command_Ban(void) { if (COM_Argc() < 2) { - CONS_Printf(M_GetText("Ban : ban and kick a player\n")); + CONS_Printf(M_GetText("ban : ban and kick a player\n")); return; } @@ -3149,7 +3149,7 @@ static void Command_Ban(void) if (server && I_Ban && !I_Ban(node)) // only the server is allowed to do this right now { - CONS_Alert(CONS_WARNING, M_GetText("Too many bans! Geez, that's a lot of people you're excluding...\n")); + CONS_Alert(CONS_WARNING, M_GetText("Ban failed. Invalid node?\n")); WRITEUINT8(p, KICK_MSG_GO_AWAY); SendNetXCmd(XD_KICK, &buf, 2); } diff --git a/src/i_tcp.c b/src/i_tcp.c index ba973494..b1a2d30f 100644 --- a/src/i_tcp.c +++ b/src/i_tcp.c @@ -174,8 +174,6 @@ static UINT8 UPNP_support = TRUE; #endif // !NONET -#define MAXBANS 100 - #include "i_system.h" #include "i_net.h" #include "d_net.h" @@ -183,6 +181,7 @@ static UINT8 UPNP_support = TRUE; #include "i_tcp.h" #include "m_argv.h" #include "stun.h" +#include "z_zone.h" #include "doomstat.h" @@ -239,11 +238,13 @@ static mysockaddr_t clientaddress[MAXNETNODES+1]; static mysockaddr_t broadcastaddress[MAXNETNODES+1]; static size_t broadcastaddresses = 0; static boolean nodeconnected[MAXNETNODES+1]; -static mysockaddr_t banned[MAXBANS]; -static UINT8 bannedmask[MAXBANS]; +static mysockaddr_t *banned; +static UINT8 *bannedmask; #endif static size_t numbans = 0; +static size_t banned_size = 0; + static boolean SOCK_bannednode[MAXNETNODES+1]; /// \note do we really need the +1? static boolean init_tcp_driver = false; @@ -1395,6 +1396,39 @@ static boolean SOCK_OpenSocket(void) #endif } +static void AddBannedIndex(void) +{ + if (numbans >= banned_size) + { + if (banned_size == 0) + { + banned_size = 8; + } + else + { + banned_size *= 2; + } + + banned = Z_ReallocAlign( + (void*) banned, + sizeof(mysockaddr_t) * banned_size, + PU_STATIC, + NULL, + sizeof(mysockaddr_t) * 8 + ); + + bannedmask = Z_ReallocAlign( + (void*) banned, + sizeof(UINT8) * banned_size, + PU_STATIC, + NULL, + sizeof(UINT8) * 8 + ); + } + + numbans++; +} + static boolean SOCK_Ban(INT32 node) { if (node > MAXNETNODES) @@ -1402,23 +1436,23 @@ static boolean SOCK_Ban(INT32 node) #ifdef NONET return false; #else - if (numbans == MAXBANS) - return false; - M_Memcpy(&banned[numbans], &clientaddress[node], sizeof (mysockaddr_t)); - if (banned[numbans].any.sa_family == AF_INET) + AddBannedIndex(); + + M_Memcpy(&banned[numbans-1], &clientaddress[node], sizeof (mysockaddr_t)); + if (banned[numbans-1].any.sa_family == AF_INET) { - banned[numbans].ip4.sin_port = 0; - bannedmask[numbans] = 32; + banned[numbans-1].ip4.sin_port = 0; + bannedmask[numbans-1] = 32; } #ifdef HAVE_IPV6 - else if (banned[numbans].any.sa_family == AF_INET6) + else if (banned[numbans-1].any.sa_family == AF_INET6) { - banned[numbans].ip6.sin6_port = 0; - bannedmask[numbans] = 128; + banned[numbans-1].ip6.sin6_port = 0; + bannedmask[numbans-1] = 128; } #endif - numbans++; + return true; #endif } @@ -1433,7 +1467,7 @@ static boolean SOCK_SetBanAddress(const char *address, const char *mask) struct my_addrinfo *ai, *runp, hints; int gaie; - if (numbans == MAXBANS || !address) + if (!address) return false; memset(&hints, 0x00, sizeof(hints)); @@ -1448,26 +1482,27 @@ static boolean SOCK_SetBanAddress(const char *address, const char *mask) runp = ai; - while(runp != NULL && numbans != MAXBANS) + while (runp != NULL) { - memcpy(&banned[numbans], runp->ai_addr, runp->ai_addrlen); + AddBannedIndex(); + + memcpy(&banned[numbans-1], runp->ai_addr, runp->ai_addrlen); if (mask) - bannedmask[numbans] = (UINT8)atoi(mask); + bannedmask[numbans-1] = (UINT8)atoi(mask); #ifdef HAVE_IPV6 else if (runp->ai_family == AF_INET6) - bannedmask[numbans] = 128; + bannedmask[numbans-1] = 128; #endif else - bannedmask[numbans] = 32; + bannedmask[numbans-1] = 32; - if (bannedmask[numbans] > 32 && runp->ai_family == AF_INET) - bannedmask[numbans] = 32; + if (bannedmask[numbans-1] > 32 && runp->ai_family == AF_INET) + bannedmask[numbans-1] = 32; #ifdef HAVE_IPV6 - else if (bannedmask[numbans] > 128 && runp->ai_family == AF_INET6) - bannedmask[numbans] = 128; + else if (bannedmask[numbans-1] > 128 && runp->ai_family == AF_INET6) + bannedmask[numbans-1] = 128; #endif - numbans++; runp = runp->ai_next; } @@ -1480,6 +1515,9 @@ static boolean SOCK_SetBanAddress(const char *address, const char *mask) static void SOCK_ClearBans(void) { numbans = 0; + banned_size = 0; + banned = NULL; + bannedmask = NULL; } boolean I_InitTcpNetwork(void) From 2f0e1521d03a9d5b32877b5ae54d1ac3a6068442 Mon Sep 17 00:00:00 2001 From: toaster Date: Tue, 14 Jun 2022 16:48:29 +0100 Subject: [PATCH 02/14] Combine banned & bannedmask into banned_t Better code cleanliness, also makes it easier to add more data to bans later (such as a timestamp for temporary bans) # Conflicts: # src/i_tcp.c --- src/i_tcp.c | 70 +++++++++++++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/src/i_tcp.c b/src/i_tcp.c index b1a2d30f..d4bf5b87 100644 --- a/src/i_tcp.c +++ b/src/i_tcp.c @@ -230,6 +230,14 @@ typedef int socklen_t; #endif #ifndef NONET + +typedef struct +{ + mysockaddr_t address; + UINT8 mask; + // TODO: timestamp, for tempbans! +} banned_t; + static SOCKET_TYPE mysockets[MAXNETNODES+1] = {ERRSOCKET}; static size_t mysocketses = 0; static int myfamily[MAXNETNODES+1] = {0}; @@ -238,8 +246,7 @@ static mysockaddr_t clientaddress[MAXNETNODES+1]; static mysockaddr_t broadcastaddress[MAXNETNODES+1]; static size_t broadcastaddresses = 0; static boolean nodeconnected[MAXNETNODES+1]; -static mysockaddr_t *banned; -static UINT8 *bannedmask; +static banned_t *banned; #endif static size_t numbans = 0; @@ -473,7 +480,7 @@ static const char *SOCK_GetBanAddress(size_t ban) #ifdef NONET return NULL; #else - return SOCK_AddrToStr(&banned[ban]); + return SOCK_AddrToStr(&banned[ban].address); #endif } @@ -485,7 +492,7 @@ static const char *SOCK_GetBanMask(size_t ban) static char s[16]; //255.255.255.255 netmask? no, just CDIR for only if (ban >= numbans) return NULL; - if (sprintf(s,"%d",bannedmask[ban]) > 0) + if (sprintf(s,"%d",banned[ban].mask) > 0) return s; #endif return NULL; @@ -648,7 +655,7 @@ static boolean SOCK_Get(void) // check if it's a banned dude so we can send a refusal later for (i = 0; i < numbans; i++) { - if (SOCK_cmpaddr(&fromaddress, &banned[i], bannedmask[i])) + if (SOCK_cmpaddr(&fromaddress, &banned[i].address, banned[i].mask)) { SOCK_bannednode[j] = true; DEBFILE("This dude has been banned\n"); @@ -1411,18 +1418,10 @@ static void AddBannedIndex(void) banned = Z_ReallocAlign( (void*) banned, - sizeof(mysockaddr_t) * banned_size, + sizeof(banned_t) * banned_size, PU_STATIC, NULL, - sizeof(mysockaddr_t) * 8 - ); - - bannedmask = Z_ReallocAlign( - (void*) banned, - sizeof(UINT8) * banned_size, - PU_STATIC, - NULL, - sizeof(UINT8) * 8 + sizeof(banned_t) * 8 ); } @@ -1431,25 +1430,31 @@ static void AddBannedIndex(void) static boolean SOCK_Ban(INT32 node) { + INT32 ban; + if (node > MAXNETNODES) return false; + #ifdef NONET + (void)ban; return false; #else + ban = numbans; AddBannedIndex(); - M_Memcpy(&banned[numbans-1], &clientaddress[node], sizeof (mysockaddr_t)); - if (banned[numbans-1].any.sa_family == AF_INET) + M_Memcpy(&banned[ban].address, &clientaddress[node], sizeof (mysockaddr_t)); + + if (banned[ban].address.any.sa_family == AF_INET) { - banned[numbans-1].ip4.sin_port = 0; - bannedmask[numbans-1] = 32; + banned[ban].address.ip4.sin_port = 0; + banned[ban].mask = 32; } #ifdef HAVE_IPV6 - else if (banned[numbans-1].any.sa_family == AF_INET6) + else if (banned[ban].address.any.sa_family == AF_INET6) { - banned[numbans-1].ip6.sin6_port = 0; - bannedmask[numbans-1] = 128; + banned[ban].address.ip6.sin6_port = 0; + banned[ban].mask = 128; } #endif @@ -1484,25 +1489,29 @@ static boolean SOCK_SetBanAddress(const char *address, const char *mask) while (runp != NULL) { + INT32 ban; + + ban = numbans; AddBannedIndex(); - memcpy(&banned[numbans-1], runp->ai_addr, runp->ai_addrlen); + memcpy(&banned[ban].address, runp->ai_addr, runp->ai_addrlen); if (mask) - bannedmask[numbans-1] = (UINT8)atoi(mask); + banned[ban].mask = (UINT8)atoi(mask); #ifdef HAVE_IPV6 else if (runp->ai_family == AF_INET6) - bannedmask[numbans-1] = 128; + banned[ban].mask = 128; #endif else - bannedmask[numbans-1] = 32; + banned[ban].mask = 32; - if (bannedmask[numbans-1] > 32 && runp->ai_family == AF_INET) - bannedmask[numbans-1] = 32; + if (banned[ban].mask > 32 && runp->ai_family == AF_INET) + banned[ban].mask = 32; #ifdef HAVE_IPV6 - else if (bannedmask[numbans-1] > 128 && runp->ai_family == AF_INET6) - bannedmask[numbans-1] = 128; + else if (banned[ban].mask > 128 && runp->ai_family == AF_INET6) + banned[ban].mask = 128; #endif + runp = runp->ai_next; } @@ -1517,7 +1526,6 @@ static void SOCK_ClearBans(void) numbans = 0; banned_size = 0; banned = NULL; - bannedmask = NULL; } boolean I_InitTcpNetwork(void) From 24181ae7380de8f64c885ba1ebce4ccbebdb5712 Mon Sep 17 00:00:00 2001 From: toaster Date: Tue, 14 Jun 2022 16:58:07 +0100 Subject: [PATCH 03/14] - Attach ban reasons to banned_t - Properly call D_SaveBan after remote bans. Bans are no longer saved in the ban command and instead wait for the actual kick to process, since before they were split between the two, which is what caused the discrepancy. # Conflicts: # src/d_clisrv.c # src/i_tcp.c --- src/d_clisrv.c | 149 ++++++++++++++----------------------------------- src/d_net.c | 2 + src/i_net.h | 2 + src/i_tcp.c | 30 ++++++++++ 4 files changed, 77 insertions(+), 106 deletions(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index 1e31de51..65f3e8b7 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -2637,40 +2637,27 @@ static void CL_ConnectToServer(void) } #ifndef NONET -typedef struct banreason_s -{ - char *reason; - struct banreason_s *prev; //-1 - struct banreason_s *next; //+1 -} banreason_t; - -static banreason_t *reasontail = NULL; //last entry, use prev -static banreason_t *reasonhead = NULL; //1st entry, use next - static void Command_ShowBan(void) //Print out ban list { size_t i; - const char *address, *mask; - banreason_t *reasonlist = reasonhead; + const char *address, *mask, *reason; if (I_GetBanAddress) CONS_Printf(M_GetText("Ban List:\n")); else return; - for (i = 0;(address = I_GetBanAddress(i)) != NULL;i++) + for (i = 0; (address = I_GetBanAddress(i)) != NULL; i++) { if (!I_GetBanMask || (mask = I_GetBanMask(i)) == NULL) CONS_Printf("%s: %s ", sizeu1(i+1), address); else CONS_Printf("%s: %s/%s ", sizeu1(i+1), address, mask); - if (reasonlist && reasonlist->reason) - CONS_Printf("(%s)\n", reasonlist->reason); + if (I_GetBanReason && (reason = I_GetBanReason(i)) != NULL) + CONS_Printf("(%s)\n", reason); else CONS_Printf("\n"); - - if (reasonlist) reasonlist = reasonlist->next; } if (i == 0 && !address) @@ -2681,13 +2668,10 @@ void D_SaveBan(void) { FILE *f; size_t i; - banreason_t *reasonlist = reasonhead; - const char *address, *mask; + const char *address, *mask, *reason; + const char *path = va("%s"PATHSEP"%s", srb2home, "ban.txt"); - if (!reasonhead) - return; - - f = fopen(va("%s"PATHSEP"%s", srb2home, "ban.txt"), "w"); + f = fopen(path, "w"); if (!f) { @@ -2695,66 +2679,36 @@ void D_SaveBan(void) return; } - for (i = 0;(address = I_GetBanAddress(i)) != NULL;i++) + for (i = 0; (address = I_GetBanAddress(i)) != NULL; i++) { if (!I_GetBanMask || (mask = I_GetBanMask(i)) == NULL) fprintf(f, "%s 0", address); else fprintf(f, "%s %s", address, mask); - if (reasonlist && reasonlist->reason) - fprintf(f, " %s\n", reasonlist->reason); + if (I_GetBanReason && (reason = I_GetBanReason(i)) != NULL) + fprintf(f, " %s\n", reason); else - fprintf(f, " %s\n", "NA"); - - if (reasonlist) reasonlist = reasonlist->next; + fprintf(f, " %s\n", "No reason given"); } fclose(f); } -static void Ban_Add(const char *reason) -{ - banreason_t *reasonlist = malloc(sizeof(*reasonlist)); - - if (!reasonlist) - return; - if (!reason) - reason = "NA"; - - reasonlist->next = NULL; - reasonlist->reason = Z_StrDup(reason); - if ((reasonlist->prev = reasontail) == NULL) - reasonhead = reasonlist; - else - reasontail->next = reasonlist; - reasontail = reasonlist; -} - static void Command_ClearBans(void) { - banreason_t *temp; - if (!I_ClearBans) return; I_ClearBans(); D_SaveBan(); - reasontail = NULL; - while (reasonhead) - { - temp = reasonhead->next; - Z_Free(reasonhead->reason); - free(reasonhead); - reasonhead = temp; - } } static void Ban_Load_File(boolean warning) { FILE *f; size_t i; - const char *address, *mask; + const char *address, *mask, *reason; char buffer[MAX_WADPATH]; f = fopen(va("%s"PATHSEP"%s", srb2home, "ban.txt"), "r"); @@ -2766,22 +2720,17 @@ static void Ban_Load_File(boolean warning) return; } - if (I_ClearBans) - Command_ClearBans(); - else - { - fclose(f); - return; - } + I_ClearBans(); for (i=0; fgets(buffer, (int)sizeof(buffer), f); i++) { address = strtok(buffer, " \t\r\n"); mask = strtok(NULL, " \t\r\n"); + reason = strtok(NULL, "\r\n"); I_SetBanAddress(address, mask); - - Ban_Add(strtok(NULL, "\r\n")); + if (I_SetBanReason) + I_SetBanReason(reason); } fclose(f); @@ -3140,54 +3089,37 @@ static void Command_Ban(void) XBOXSTATIC UINT8 buf[3 + MAX_REASONLENGTH]; UINT8 *p = buf; const SINT8 pn = nametonum(COM_Argv(1)); - const INT32 node = playernode[(INT32)pn]; if (pn == -1 || pn == 0) return; WRITEUINT8(p, pn); - if (server && I_Ban && !I_Ban(node)) // only the server is allowed to do this right now + if (COM_Argc() == 2) { - CONS_Alert(CONS_WARNING, M_GetText("Ban failed. Invalid node?\n")); - WRITEUINT8(p, KICK_MSG_GO_AWAY); + WRITEUINT8(p, KICK_MSG_BANNED); SendNetXCmd(XD_KICK, &buf, 2); } else { - if (server) // only the server is allowed to do this right now + size_t i, j = COM_Argc(); + char message[MAX_REASONLENGTH]; + + //Steal from the motd code so you don't have to put the reason in quotes. + strlcpy(message, COM_Argv(2), sizeof message); + for (i = 3; i < j; i++) { - Ban_Add(COM_Argv(2)); - D_SaveBan(); // save the ban list + strlcat(message, " ", sizeof message); + strlcat(message, COM_Argv(i), sizeof message); } - if (COM_Argc() == 2) - { - WRITEUINT8(p, KICK_MSG_BANNED); - SendNetXCmd(XD_KICK, &buf, 2); - } - else - { - size_t i, j = COM_Argc(); - char message[MAX_REASONLENGTH]; - - //Steal from the motd code so you don't have to put the reason in quotes. - strlcpy(message, COM_Argv(2), sizeof message); - for (i = 3; i < j; i++) - { - strlcat(message, " ", sizeof message); - strlcat(message, COM_Argv(i), sizeof message); - } - - WRITEUINT8(p, KICK_MSG_CUSTOM_BAN); - WRITESTRINGN(p, message, MAX_REASONLENGTH); - SendNetXCmd(XD_KICK, &buf, p - buf); - } + WRITEUINT8(p, KICK_MSG_CUSTOM_BAN); + WRITESTRINGN(p, message, MAX_REASONLENGTH); + SendNetXCmd(XD_KICK, &buf, p - buf); } } else CONS_Printf(M_GetText("Only the server or a remote admin can use this.\n")); - } static void Command_BanIP(void) @@ -3208,7 +3140,6 @@ static void Command_BanIP(void) else reason = COM_Argv(2); - if (I_SetBanAddress && I_SetBanAddress(address, NULL)) { if (reason) @@ -3216,7 +3147,8 @@ static void Command_BanIP(void) else CONS_Printf("Banned IP address %s\n", address); - Ban_Add(reason); + if (I_SetBanReason) + I_SetBanReason(reason); D_SaveBan(); } else @@ -3357,16 +3289,21 @@ static void Got_KickCmd(UINT8 **p, INT32 playernum) //CONS_Printf("\x82%s ", player_names[pnum]); - // If a verified admin banned someone, the server needs to know about it. - // If the playernum isn't zero (the server) then the server needs to record the ban. - if (server && playernum && (msg == KICK_MSG_BANNED || msg == KICK_MSG_CUSTOM_BAN)) + // Save bans here. Used to be split between here and the actual command, depending on + // whenever the server did it or a remote admin did it, but it's simply more convenient + // to keep it all in one place. + if (server && (msg == KICK_MSG_BANNED || msg == KICK_MSG_CUSTOM_BAN)) { if (I_Ban && !I_Ban(playernode[(INT32)pnum])) - CONS_Alert(CONS_WARNING, M_GetText("Too many bans! Geez, that's a lot of people you're excluding...\n")); -#ifndef NONET + { + CONS_Alert(CONS_WARNING, M_GetText("Ban failed. Invalid node?\n")); + } else - Ban_Add(reason); -#endif + { + if (I_SetBanReason) + I_SetBanReason(reason); + D_SaveBan(); + } } if (msg == KICK_MSG_PLAYER_QUIT) diff --git a/src/d_net.c b/src/d_net.c index d974d6cf..b28aab7a 100644 --- a/src/d_net.c +++ b/src/d_net.c @@ -79,7 +79,9 @@ void (*I_ClearBans)(void) = NULL; const char *(*I_GetNodeAddress) (INT32 node) = NULL; const char *(*I_GetBanAddress) (size_t ban) = NULL; const char *(*I_GetBanMask) (size_t ban) = NULL; +const char *(*I_GetBanReason) (size_t ban) = NULL; boolean (*I_SetBanAddress) (const char *address, const char *mask) = NULL; +boolean (*I_SetBanReason) (const char *reason) = NULL; boolean *bannednode = NULL; diff --git a/src/i_net.h b/src/i_net.h index 0e17077b..3608d81b 100644 --- a/src/i_net.h +++ b/src/i_net.h @@ -145,7 +145,9 @@ extern void (*I_ClearBans)(void); extern const char *(*I_GetNodeAddress) (INT32 node); extern const char *(*I_GetBanAddress) (size_t ban); extern const char *(*I_GetBanMask) (size_t ban); +extern const char *(*I_GetBanReason) (size_t ban); extern boolean (*I_SetBanAddress) (const char *address,const char *mask); +extern boolean (*I_SetBanReason) (const char *reason); extern boolean *bannednode; /// \brief Called by D_SRB2Main to be defined by extern network driver diff --git a/src/i_tcp.c b/src/i_tcp.c index d4bf5b87..75e3aaa6 100644 --- a/src/i_tcp.c +++ b/src/i_tcp.c @@ -235,6 +235,7 @@ typedef struct { mysockaddr_t address; UINT8 mask; + char *reason; // TODO: timestamp, for tempbans! } banned_t; @@ -498,6 +499,16 @@ static const char *SOCK_GetBanMask(size_t ban) return NULL; } +static const char *SOCK_GetBanReason(size_t ban) +{ +#ifdef NONET + (void)ban; + return NULL; +#else + return banned[ban].reason; +#endif +} + #ifndef NONET static boolean SOCK_cmpaddr(mysockaddr_t *a, mysockaddr_t *b, UINT8 mask) { @@ -1521,6 +1532,23 @@ static boolean SOCK_SetBanAddress(const char *address, const char *mask) #endif } +static boolean SOCK_SetBanReason(const char *reason) +{ +#ifdef NONET + (void)reason; + return false; +#else + + if (!reason) + { + reason = "No reason given"; + } + + banned[numbans - 1].reason = Z_StrDup(reason); + return true; +#endif +} + static void SOCK_ClearBans(void) { numbans = 0; @@ -1617,7 +1645,9 @@ boolean I_InitTcpNetwork(void) I_GetNodeAddress = SOCK_GetNodeAddress; I_GetBanAddress = SOCK_GetBanAddress; I_GetBanMask = SOCK_GetBanMask; + I_GetBanReason = SOCK_GetBanReason; I_SetBanAddress = SOCK_SetBanAddress; + I_SetBanReason = SOCK_SetBanReason; bannednode = SOCK_bannednode; return ret; From 8d05bf669b80e0caddf000ffb065bf4672ed1fd4 Mon Sep 17 00:00:00 2001 From: toaster Date: Tue, 14 Jun 2022 17:15:28 +0100 Subject: [PATCH 04/14] Kicks are now temp bans Length is determined by the "kicktime" cvar, in minutes. By default, this is set to 10, but I'm willing to adjust this. Only applies to manual kicks (in the future, maybe also name filter kicks). The timestamp for the unban time is even saved in ban.txt, so long-term temporary bans are completely possible. (I checked, you can attempt to ban someone for up to 1902 years if you really want to.) # Conflicts: # src/d_clisrv.c # src/d_clisrv.h # src/i_tcp.c --- src/d_clisrv.c | 121 +++++++++++++++++++++++++++++++++++++++++++------ src/d_clisrv.h | 2 + src/d_net.c | 3 ++ src/i_net.h | 5 ++ src/i_tcp.c | 64 ++++++++++++++++++++++++-- 5 files changed, 175 insertions(+), 20 deletions(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index 65f3e8b7..ec2ce776 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -180,6 +180,8 @@ consvar_t cv_playbackspeed = {"playbackspeed", "1", 0, playbackspeed_cons_t, NUL consvar_t cv_httpsource = {"http_source", "", CV_SAVE, NULL, NULL, 0, NULL, NULL, 0, 0, NULL}; +consvar_t cv_kicktime = CVAR_INIT ("kicktime", "10", CV_SAVE, CV_Unsigned, NULL); + static inline void *G_DcpyTiccmd(void* dest, const ticcmd_t* src, const size_t n) { const size_t d = n / sizeof(ticcmd_t); @@ -2654,6 +2656,13 @@ static void Command_ShowBan(void) //Print out ban list else CONS_Printf("%s: %s/%s ", sizeu1(i+1), address, mask); + if (I_GetUnbanTime && I_GetUnbanTime(i) != NO_BAN_TIME) + { + // todo: maybe try to actually print out the time remaining, + // and/or the datetime of the unbanning? + CONS_Printf("(temporary) "); + } + if (I_GetBanReason && (reason = I_GetBanReason(i)) != NULL) CONS_Printf("(%s)\n", reason); else @@ -2669,6 +2678,8 @@ void D_SaveBan(void) FILE *f; size_t i; const char *address, *mask, *reason; + const time_t curTime = time(NULL); + time_t unbanTime = NO_BAN_TIME; const char *path = va("%s"PATHSEP"%s", srb2home, "ban.txt"); f = fopen(path, "w"); @@ -2681,11 +2692,24 @@ void D_SaveBan(void) for (i = 0; (address = I_GetBanAddress(i)) != NULL; i++) { + if (I_GetUnbanTime) + { + unbanTime = I_GetUnbanTime(i); + } + + if (unbanTime != NO_BAN_TIME && curTime >= unbanTime) + { + // Don't need to save this one anymore. + continue; + } + if (!I_GetBanMask || (mask = I_GetBanMask(i)) == NULL) fprintf(f, "%s 0", address); else fprintf(f, "%s %s", address, mask); + fprintf(f, " %ld", (long)unbanTime); + if (I_GetBanReason && (reason = I_GetBanReason(i)) != NULL) fprintf(f, " %s\n", reason); else @@ -2709,6 +2733,7 @@ static void Ban_Load_File(boolean warning) FILE *f; size_t i; const char *address, *mask, *reason; + time_t unbanTime = NO_BAN_TIME; char buffer[MAX_WADPATH]; f = fopen(va("%s"PATHSEP"%s", srb2home, "ban.txt"), "r"); @@ -2726,9 +2751,12 @@ static void Ban_Load_File(boolean warning) { address = strtok(buffer, " \t\r\n"); mask = strtok(NULL, " \t\r\n"); + unbanTime = atoi(strtok(NULL, " \t\r\n")); reason = strtok(NULL, "\r\n"); I_SetBanAddress(address, mask); + if (I_SetUnbanTime) + I_SetUnbanTime(unbanTime); if (I_SetBanReason) I_SetBanReason(reason); } @@ -3229,6 +3257,7 @@ static void Got_KickCmd(UINT8 **p, INT32 playernum) XBOXSTATIC char buf[3 + MAX_REASONLENGTH]; char *reason = buf; kickreason_t kickreason = KR_KICK; + UINT32 banMinutes = 0; pnum = READUINT8(*p); msg = READUINT8(*p); @@ -3292,17 +3321,35 @@ static void Got_KickCmd(UINT8 **p, INT32 playernum) // Save bans here. Used to be split between here and the actual command, depending on // whenever the server did it or a remote admin did it, but it's simply more convenient // to keep it all in one place. - if (server && (msg == KICK_MSG_BANNED || msg == KICK_MSG_CUSTOM_BAN)) + if (server) { - if (I_Ban && !I_Ban(playernode[(INT32)pnum])) + if (msg == KICK_MSG_GO_AWAY || msg == KICK_MSG_CUSTOM_KICK) { - CONS_Alert(CONS_WARNING, M_GetText("Ban failed. Invalid node?\n")); + // Kick as a temporary ban. + banMinutes = cv_kicktime.value; } - else + + if (msg == KICK_MSG_BANNED || msg == KICK_MSG_CUSTOM_BAN || banMinutes) { - if (I_SetBanReason) - I_SetBanReason(reason); - D_SaveBan(); + if (I_Ban && !I_Ban(playernode[(INT32)pnum])) + { + CONS_Alert(CONS_WARNING, M_GetText("Ban failed. Invalid node?\n")); + } + else + { + if (I_SetBanReason) + I_SetBanReason(reason); + + if (I_SetUnbanTime) + { + if (banMinutes) + I_SetUnbanTime(time(NULL) + (banMinutes * 60)); + else + I_SetUnbanTime(NO_BAN_TIME); + } + + D_SaveBan(); + } } } @@ -3388,9 +3435,13 @@ static void Got_KickCmd(UINT8 **p, INT32 playernum) #ifdef DUMPCONSISTENCY if (msg == KICK_MSG_CON_FAIL) SV_SavedGame(); #endif + + LUAh_GameQuit(false); + D_QuitNetGame(); CL_Reset(); D_StartTitle(); + if (msg == KICK_MSG_CON_FAIL) M_StartMessage(M_GetText("Server closed connection\n(Synch failure)\nPress ESC\n"), NULL, MM_NOTHING); else if (msg == KICK_MSG_PING_HIGH) @@ -3566,6 +3617,7 @@ void D_ClientServerInit(void) #ifndef NONET COM_AddCommand("getplayernum", Command_GetPlayerNum); COM_AddCommand("kick", Command_Kick); + CV_RegisterVar(&cv_kicktime); COM_AddCommand("ban", Command_Ban); COM_AddCommand("banip", Command_BanIP); COM_AddCommand("clearbans", Command_ClearBans); @@ -4066,26 +4118,65 @@ static void HandleConnect(SINT8 node) UINT8 maxplayers = min((dedicated ? MAXPLAYERS-1 : MAXPLAYERS), cv_maxplayers.value); if (bannednode && bannednode[node]) - SV_SendRefuse(node, M_GetText("You have been banned\nfrom the server")); + { + if (bannednodetimeleft && bannednodetimeleft[node] != NO_BAN_TIME) + { + int minutes = bannednodetimeleft[node] / 60; + int hours = minutes / 60; + + if (hours) + { + SV_SendRefuse(node, va(M_GetText("You have been temporarily\nkicked from the server.\n(Time remaining: %d hour%s)"), hours, hours > 1 ? "s" : "")); + } + else if (minutes) + { + SV_SendRefuse(node, va(M_GetText("You have been temporarily\nkicked from the server.\n(Time remaining: %d minute%s)"), minutes, minutes > 1 ? "s" : "")); + } + else + { + SV_SendRefuse(node, M_GetText("You have been temporarily\nkicked from the server.\n(Time remaining: <1 minute)")); + } + } + else + { + SV_SendRefuse(node, M_GetText("You have been banned\nfrom the server.")); + } + } else if (netbuffer->u.clientcfg._255 != 255 || netbuffer->u.clientcfg.packetversion != PACKETVERSION) + { SV_SendRefuse(node, "Incompatible packet formats."); + } else if (strncmp(netbuffer->u.clientcfg.application, SRB2APPLICATION, sizeof netbuffer->u.clientcfg.application)) - SV_SendRefuse(node, "Different SRB2 modifications\nare not compatible."); + { + SV_SendRefuse(node, "Different SRB2Kart modifications\nare not compatible."); + } else if (netbuffer->u.clientcfg.version != VERSION || netbuffer->u.clientcfg.subversion != SUBVERSION) + { SV_SendRefuse(node, va(M_GetText("Different SRB2Kart versions cannot\nplay a netgame!\n(server version %d.%d)"), VERSION, SUBVERSION)); + } else if (!cv_allownewplayer.value && node) - SV_SendRefuse(node, M_GetText("The server is not accepting\njoins for the moment")); + { + SV_SendRefuse(node, M_GetText("The server is not accepting\njoins for the moment.")); + } else if (D_NumPlayers() >= maxplayers) + { SV_SendRefuse(node, va(M_GetText("Maximum players reached: %d"), maxplayers)); - else if (netgame && D_NumPlayers() + netbuffer->u.clientcfg.localplayers > maxplayers) - SV_SendRefuse(node, va(M_GetText("Number of local players\nwould exceed maximum: %d"), maxplayers)); + } else if (netgame && netbuffer->u.clientcfg.localplayers > 4) // Hacked client? + { SV_SendRefuse(node, M_GetText("Too many players from\nthis node.")); + } + else if (netgame && D_NumPlayers() + netbuffer->u.clientcfg.localplayers > maxplayers) + { + SV_SendRefuse(node, va(M_GetText("Number of local players\nwould exceed maximum: %d"), maxplayers)); + } else if (netgame && !netbuffer->u.clientcfg.localplayers) // Stealth join? + { SV_SendRefuse(node, M_GetText("No players from\nthis node.")); + } else { #ifndef NONET @@ -4319,13 +4410,13 @@ static void HandlePacketFromAwayNode(SINT8 node) break; } - M_StartMessage(va(M_GetText("Server refuses connection\n\nReason:\n%s"), - reason), NULL, MM_NOTHING); - D_QuitNetGame(); CL_Reset(); D_StartTitle(); + M_StartMessage(va(M_GetText("Server refuses connection\n\nReason:\n%s"), + reason), NULL, MM_NOTHING); + free(reason); // Will be reset by caller. Signals refusal. diff --git a/src/d_clisrv.h b/src/d_clisrv.h index 0ce20f1a..c51af62b 100644 --- a/src/d_clisrv.h +++ b/src/d_clisrv.h @@ -504,6 +504,8 @@ extern INT32 mapchangepending; extern doomdata_t *netbuffer; extern consvar_t cv_stunserver; extern consvar_t cv_httpsource; +extern consvar_t cv_kicktime; + extern consvar_t cv_showjoinaddress; extern consvar_t cv_playbackspeed; diff --git a/src/d_net.c b/src/d_net.c index b28aab7a..70086ef2 100644 --- a/src/d_net.c +++ b/src/d_net.c @@ -80,9 +80,12 @@ const char *(*I_GetNodeAddress) (INT32 node) = NULL; const char *(*I_GetBanAddress) (size_t ban) = NULL; const char *(*I_GetBanMask) (size_t ban) = NULL; const char *(*I_GetBanReason) (size_t ban) = NULL; +time_t (*I_GetUnbanTime) (size_t ban) = NULL; boolean (*I_SetBanAddress) (const char *address, const char *mask) = NULL; boolean (*I_SetBanReason) (const char *reason) = NULL; +boolean (*I_SetUnbanTime) (time_t timestamp) = NULL; boolean *bannednode = NULL; +time_t *bannednodetimeleft = NULL; // network stats diff --git a/src/i_net.h b/src/i_net.h index 3608d81b..9a2e11f4 100644 --- a/src/i_net.h +++ b/src/i_net.h @@ -31,6 +31,8 @@ /// For use on the internet #define INETPACKETLENGTH 1024 +#define NO_BAN_TIME (time_t)(-1) + extern INT16 hardware_MAXPACKETLENGTH; extern INT32 net_bandwidth; // in byte/s @@ -146,9 +148,12 @@ extern const char *(*I_GetNodeAddress) (INT32 node); extern const char *(*I_GetBanAddress) (size_t ban); extern const char *(*I_GetBanMask) (size_t ban); extern const char *(*I_GetBanReason) (size_t ban); +extern time_t (*I_GetUnbanTime) (size_t ban); extern boolean (*I_SetBanAddress) (const char *address,const char *mask); extern boolean (*I_SetBanReason) (const char *reason); +extern boolean (*I_SetUnbanTime) (time_t timestamp); extern boolean *bannednode; +extern time_t *bannednodetimeleft; /// \brief Called by D_SRB2Main to be defined by extern network driver boolean I_InitNetwork(void); diff --git a/src/i_tcp.c b/src/i_tcp.c index 75e3aaa6..f425bbce 100644 --- a/src/i_tcp.c +++ b/src/i_tcp.c @@ -236,7 +236,7 @@ typedef struct mysockaddr_t address; UINT8 mask; char *reason; - // TODO: timestamp, for tempbans! + time_t timestamp; } banned_t; static SOCKET_TYPE mysockets[MAXNETNODES+1] = {ERRSOCKET}; @@ -254,6 +254,7 @@ static size_t numbans = 0; static size_t banned_size = 0; static boolean SOCK_bannednode[MAXNETNODES+1]; /// \note do we really need the +1? +static time_t SOCK_bannednodetimeleft[MAXNETNODES+1]; static boolean init_tcp_driver = false; static const char *serverport_name = DEFAULTPORT; @@ -505,10 +506,24 @@ static const char *SOCK_GetBanReason(size_t ban) (void)ban; return NULL; #else + if (ban >= numbans) + return NULL; return banned[ban].reason; #endif } +static time_t SOCK_GetUnbanTime(size_t ban) +{ +#ifdef NONET + (void)ban; + return NO_BAN_TIME; +#else + if (ban >= numbans) + return NO_BAN_TIME; + return banned[ban].timestamp; +#endif +} + #ifndef NONET static boolean SOCK_cmpaddr(mysockaddr_t *a, mysockaddr_t *b, UINT8 mask) { @@ -656,6 +671,8 @@ static boolean SOCK_Get(void) j = getfreenode(); if (j > 0) { + const time_t curTime = time(NULL); + M_Memcpy(&clientaddress[j], &fromaddress, fromlen); nodesocket[j] = mysockets[n]; DEBFILE(va("New node detected: node:%d address:%s\n", j, @@ -668,13 +685,37 @@ static boolean SOCK_Get(void) { if (SOCK_cmpaddr(&fromaddress, &banned[i].address, banned[i].mask)) { - SOCK_bannednode[j] = true; - DEBFILE("This dude has been banned\n"); - break; + if (banned[i].timestamp != NO_BAN_TIME) + { + if (curTime >= banned[i].timestamp) + { + SOCK_bannednodetimeleft[j] = NO_BAN_TIME; + SOCK_bannednode[j] = false; + DEBFILE("This dude was banned, but enough time has passed\n"); + break; + } + + SOCK_bannednodetimeleft[j] = banned[i].timestamp - curTime; + SOCK_bannednode[j] = true; + DEBFILE("This dude has been temporarily banned\n"); + break; + } + else + { + SOCK_bannednodetimeleft[j] = NO_BAN_TIME; + SOCK_bannednode[j] = true; + DEBFILE("This dude has been banned\n"); + break; + } } } + if (i == numbans) + { + SOCK_bannednodetimeleft[j] = NO_BAN_TIME; SOCK_bannednode[j] = false; + } + return true; } else @@ -1538,7 +1579,6 @@ static boolean SOCK_SetBanReason(const char *reason) (void)reason; return false; #else - if (!reason) { reason = "No reason given"; @@ -1549,6 +1589,17 @@ static boolean SOCK_SetBanReason(const char *reason) #endif } +static boolean SOCK_SetUnbanTime(time_t timestamp) +{ +#ifdef NONET + (void)reason; + return false; +#else + banned[numbans - 1].timestamp = timestamp; + return true; +#endif +} + static void SOCK_ClearBans(void) { numbans = 0; @@ -1646,9 +1697,12 @@ boolean I_InitTcpNetwork(void) I_GetBanAddress = SOCK_GetBanAddress; I_GetBanMask = SOCK_GetBanMask; I_GetBanReason = SOCK_GetBanReason; + I_GetUnbanTime = SOCK_GetUnbanTime; I_SetBanAddress = SOCK_SetBanAddress; I_SetBanReason = SOCK_SetBanReason; + I_SetUnbanTime = SOCK_SetUnbanTime; bannednode = SOCK_bannednode; + bannednodetimeleft = SOCK_bannednodetimeleft; return ret; } From 55be74396fdfd1deb163e0c0e6b48c9c33f64e0f Mon Sep 17 00:00:00 2001 From: toaster Date: Tue, 14 Jun 2022 17:25:40 +0100 Subject: [PATCH 05/14] Ban improvements - Save a note of the username, not just the reason. - Allow setting a mask with the `banip` command. - Make ban.txt's formatting a lot more sane. Username and reason are stored in quotes. The mask uses the same formatting as actual CDIR. - Keep track of if we tried to load ban.txt. If it wasn't, then don't save over it with a blank file. - Disallow quotes in player names, as it makes player name detection in console more annoying, and saving username in files scary. # Conflicts: # src/d_clisrv.c # src/d_netcmd.c # src/i_tcp.c --- src/d_clisrv.c | 122 +++++++++++++++++++++++++++++++++++++++---------- src/d_net.c | 2 + src/d_net.h | 1 + src/d_netcmd.c | 23 +++++++++- src/i_net.h | 2 + src/i_tcp.c | 50 +++++++++++++++++++- 6 files changed, 172 insertions(+), 28 deletions(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index ec2ce776..90f8cfa1 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -2673,15 +2673,25 @@ static void Command_ShowBan(void) //Print out ban list CONS_Printf(M_GetText("(empty)\n")); } +static boolean bansLoaded = false; + void D_SaveBan(void) { FILE *f; size_t i; - const char *address, *mask, *reason; + const char *address, *mask; + const char *username, *reason; const time_t curTime = time(NULL); time_t unbanTime = NO_BAN_TIME; const char *path = va("%s"PATHSEP"%s", srb2home, "ban.txt"); + if (bansLoaded != true) + { + // You didn't even get to ATTEMPT to load bans.txt. + // Don't immediately save nothing over it. + return; + } + f = fopen(path, "w"); if (!f) @@ -2696,24 +2706,37 @@ void D_SaveBan(void) { unbanTime = I_GetUnbanTime(i); } + else + { + unbanTime = NO_BAN_TIME; + } if (unbanTime != NO_BAN_TIME && curTime >= unbanTime) { - // Don't need to save this one anymore. + // This one has served their sentence. + // We don't need to save them in the file anymore. continue; } + mask = NULL; if (!I_GetBanMask || (mask = I_GetBanMask(i)) == NULL) - fprintf(f, "%s 0", address); + fprintf(f, "%s/0", address); else - fprintf(f, "%s %s", address, mask); + fprintf(f, "%s/%s", address, mask); fprintf(f, " %ld", (long)unbanTime); - if (I_GetBanReason && (reason = I_GetBanReason(i)) != NULL) - fprintf(f, " %s\n", reason); + username = NULL; + if (I_GetBanUsername && (username = I_GetBanUsername(i)) != NULL) + fprintf(f, " \"%s\"", username); else - fprintf(f, " %s\n", "No reason given"); + fprintf(f, " \"%s\"", "Direct IP ban"); + + reason = NULL; + if (I_GetBanReason && (reason = I_GetBanReason(i)) != NULL) + fprintf(f, " \"%s\"\n", reason); + else + fprintf(f, " \"%s\"\n", "No reason given"); } fclose(f); @@ -2728,14 +2751,21 @@ static void Command_ClearBans(void) D_SaveBan(); } -static void Ban_Load_File(boolean warning) +void D_LoadBan(boolean warning) { FILE *f; size_t i; - const char *address, *mask, *reason; + const char *address, *mask; + const char *username, *reason; time_t unbanTime = NO_BAN_TIME; char buffer[MAX_WADPATH]; + if (!I_ClearBans) + return; + + // We at least attempted loading bans.txt + bansLoaded = true; + f = fopen(va("%s"PATHSEP"%s", srb2home, "ban.txt"), "r"); if (!f) @@ -2747,16 +2777,26 @@ static void Ban_Load_File(boolean warning) I_ClearBans(); - for (i=0; fgets(buffer, (int)sizeof(buffer), f); i++) + for (i = 0; fgets(buffer, (int)sizeof(buffer), f); i++) { - address = strtok(buffer, " \t\r\n"); + address = strtok(buffer, "/\t\r\n"); mask = strtok(NULL, " \t\r\n"); - unbanTime = atoi(strtok(NULL, " \t\r\n")); - reason = strtok(NULL, "\r\n"); + + unbanTime = atoi(strtok(NULL, " \"\t\r\n")); + + username = strtok(NULL, "\"\t\r\n"); // go until next " + + strtok(NULL, "\"\t\r\n"); // remove first " + reason = strtok(NULL, "\"\r\n"); // go until next " I_SetBanAddress(address, mask); + if (I_SetUnbanTime) I_SetUnbanTime(unbanTime); + + if (I_SetBanUsername) + I_SetBanUsername(username); + if (I_SetBanReason) I_SetBanReason(reason); } @@ -2766,7 +2806,7 @@ static void Ban_Load_File(boolean warning) static void Command_ReloadBan(void) //recheck ban.txt { - Ban_Load_File(true); + D_LoadBan(true); } static void Command_connect(void) @@ -3152,31 +3192,60 @@ static void Command_Ban(void) static void Command_BanIP(void) { - if (COM_Argc() < 2) + size_t ac = COM_Argc(); + + if (ac < 2) { - CONS_Printf(M_GetText("banip : ban an ip address\n")); + CONS_Printf(M_GetText("banip []: ban an ip address\n")); return; } if (server) // Only the server can use this, otherwise does nothing. { - const char *address = (COM_Argv(1)); - const char *reason; + char *addressInput = Z_StrDup(COM_Argv(1)); - if (COM_Argc() == 2) - reason = NULL; - else + const char *address = NULL; + const char *mask = NULL; + + const char *reason = NULL; + + address = strtok(addressInput, "/"); + mask = strtok(NULL, ""); + + if (ac > 2) + { reason = COM_Argv(2); + } - if (I_SetBanAddress && I_SetBanAddress(address, NULL)) + if (I_SetBanAddress && I_SetBanAddress(address, mask)) { if (reason) - CONS_Printf("Banned IP address %s for: %s\n", address, reason); + { + CONS_Printf( + "Banned IP address %s%s for: %s\n", + address, + (mask && (strlen(mask) > 0)) ? va("/%s", mask) : "", + reason + ); + } else - CONS_Printf("Banned IP address %s\n", address); + { + CONS_Printf( + "Banned IP address %s%s\n", + address, + (mask && (strlen(mask) > 0)) ? va("/%s", mask) : "" + ); + } + + if (I_SetUnbanTime) + I_SetUnbanTime(NO_BAN_TIME); + + if (I_SetBanUsername) + I_SetBanUsername(NULL); if (I_SetBanReason) I_SetBanReason(reason); + D_SaveBan(); } else @@ -3337,6 +3406,9 @@ static void Got_KickCmd(UINT8 **p, INT32 playernum) } else { + if (I_SetBanUsername) + I_SetBanUsername(player_names[pnum]); + if (I_SetBanReason) I_SetBanReason(reason); @@ -3645,7 +3717,7 @@ void D_ClientServerInit(void) #ifdef DUMPCONSISTENCY CV_RegisterVar(&cv_dumpconsistency); #endif - Ban_Load_File(false); + D_LoadBan(false); #endif gametic = 0; diff --git a/src/d_net.c b/src/d_net.c index 70086ef2..264e7c8b 100644 --- a/src/d_net.c +++ b/src/d_net.c @@ -79,9 +79,11 @@ void (*I_ClearBans)(void) = NULL; const char *(*I_GetNodeAddress) (INT32 node) = NULL; const char *(*I_GetBanAddress) (size_t ban) = NULL; const char *(*I_GetBanMask) (size_t ban) = NULL; +const char *(*I_GetBanUsername) (size_t ban) = NULL; const char *(*I_GetBanReason) (size_t ban) = NULL; time_t (*I_GetUnbanTime) (size_t ban) = NULL; boolean (*I_SetBanAddress) (const char *address, const char *mask) = NULL; +boolean (*I_SetBanUsername) (const char *username) = NULL; boolean (*I_SetBanReason) (const char *reason) = NULL; boolean (*I_SetUnbanTime) (time_t timestamp) = NULL; boolean *bannednode = NULL; diff --git a/src/d_net.h b/src/d_net.h index 607f3e90..e267f526 100644 --- a/src/d_net.h +++ b/src/d_net.h @@ -55,6 +55,7 @@ boolean HGetPacket(void); void D_SetDoomcom(void); #ifndef NONET void D_SaveBan(void); +void D_LoadBan(boolean warning); #endif boolean D_CheckNetGame(void); void D_CloseConnection(void); diff --git a/src/d_netcmd.c b/src/d_netcmd.c index a192feda..aadd48aa 100644 --- a/src/d_netcmd.c +++ b/src/d_netcmd.c @@ -1035,7 +1035,16 @@ void D_RegisterClientCommands(void) * \sa CleanupPlayerName, SetPlayerName, Got_NameAndColor * \author Graue */ -static boolean IsNameGood(char *name, INT32 playernum) + +static boolean AllowedPlayerNameChar(char ch) +{ + if (!isprint(ch) || ch == ';' || ch == '"' || (UINT8)(ch) >= 0x80) + return false; + + return true; +} + +boolean EnsurePlayerNameIsGood(char *name, INT32 playernum) { INT32 ix; @@ -1056,7 +1065,7 @@ static boolean IsNameGood(char *name, INT32 playernum) // Also, anything over 0x80 is disallowed too, since compilers love to // differ on whether they're printable characters or not. for (ix = 0; name[ix] != '\0'; ix++) - if (!isprint(name[ix]) || name[ix] == ';' || (UINT8)(name[ix]) >= 0x80) + if (!AllowedPlayerNameChar(name[ix])) return false; // Check if a player is currently using the name, case-insensitively. @@ -1142,6 +1151,16 @@ static void CleanupPlayerName(INT32 playernum, const char *newname) tmpname = p; + do + { + if (!AllowedPlayerNameChar(*p)) + break; + } + while (*++p) ; + + if (*p)/* bad char found */ + break; + // Remove trailing spaces. p = &tmpname[strlen(tmpname)-1]; // last character while (*p == ' ' && p >= tmpname) diff --git a/src/i_net.h b/src/i_net.h index 9a2e11f4..155935f5 100644 --- a/src/i_net.h +++ b/src/i_net.h @@ -147,9 +147,11 @@ extern void (*I_ClearBans)(void); extern const char *(*I_GetNodeAddress) (INT32 node); extern const char *(*I_GetBanAddress) (size_t ban); extern const char *(*I_GetBanMask) (size_t ban); +extern const char *(*I_GetBanUsername) (size_t ban); extern const char *(*I_GetBanReason) (size_t ban); extern time_t (*I_GetUnbanTime) (size_t ban); extern boolean (*I_SetBanAddress) (const char *address,const char *mask); +extern boolean (*I_SetBanUsername) (const char *username); extern boolean (*I_SetBanReason) (const char *reason); extern boolean (*I_SetUnbanTime) (time_t timestamp); extern boolean *bannednode; diff --git a/src/i_tcp.c b/src/i_tcp.c index f425bbce..25720b8e 100644 --- a/src/i_tcp.c +++ b/src/i_tcp.c @@ -235,6 +235,7 @@ typedef struct { mysockaddr_t address; UINT8 mask; + char *username; char *reason; time_t timestamp; } banned_t; @@ -500,6 +501,18 @@ static const char *SOCK_GetBanMask(size_t ban) return NULL; } +static const char *SOCK_GetBanUsername(size_t ban) +{ +#ifdef NONET + (void)ban; + return NULL; +#else + if (ban >= numbans) + return NULL; + return banned[ban].username; +#endif +} + static const char *SOCK_GetBanReason(size_t ban) { #ifdef NONET @@ -1564,6 +1577,11 @@ static boolean SOCK_SetBanAddress(const char *address, const char *mask) banned[ban].mask = 128; #endif + // Set defaults, in case anything funny happens. + SOCK_SetBanUsername(NULL); + SOCK_SetBanReason(NULL); + SOCK_SetUnbanTime(NO_BAN_TIME); + runp = runp->ai_next; } @@ -1573,17 +1591,45 @@ static boolean SOCK_SetBanAddress(const char *address, const char *mask) #endif } +static boolean SOCK_SetBanUsername(const char *username) +{ +#ifdef NONET + (void)username; + return false; +#else + if (username == NULL || strlen(username) == 0) + { + username = "Direct IP ban"; + } + + if (banned[numbans - 1].username) + { + Z_Free(banned[numbans - 1].username); + banned[numbans - 1].username = NULL; + } + + banned[numbans - 1].username = Z_StrDup(username); + return true; +#endif +} + static boolean SOCK_SetBanReason(const char *reason) { #ifdef NONET (void)reason; return false; #else - if (!reason) + if (reason == NULL || strlen(reason) == 0) { reason = "No reason given"; } + if (banned[numbans - 1].reason) + { + Z_Free(banned[numbans - 1].reason); + banned[numbans - 1].reason = NULL; + } + banned[numbans - 1].reason = Z_StrDup(reason); return true; #endif @@ -1696,9 +1742,11 @@ boolean I_InitTcpNetwork(void) I_GetNodeAddress = SOCK_GetNodeAddress; I_GetBanAddress = SOCK_GetBanAddress; I_GetBanMask = SOCK_GetBanMask; + I_GetBanUsername = SOCK_GetBanUsername; I_GetBanReason = SOCK_GetBanReason; I_GetUnbanTime = SOCK_GetUnbanTime; I_SetBanAddress = SOCK_SetBanAddress; + I_SetBanUsername = SOCK_SetBanUsername; I_SetBanReason = SOCK_SetBanReason; I_SetUnbanTime = SOCK_SetUnbanTime; bannednode = SOCK_bannednode; From 8addea0d45a0d5505b99878ee249b2405ef9da56 Mon Sep 17 00:00:00 2001 From: toaster Date: Tue, 14 Jun 2022 20:23:59 +0100 Subject: [PATCH 06/14] Core 1.4 specific changes. * Ensure it can compile. * Removing vanilla 2.2 properties that slipped in to previous commits. * Rearranging i_tcp.c to avoid implicit declaration. * Complete rename of `IsNameGood` to `EnsurePlayerNameIsGood`. * Add "BANFORMAT" header, for versioning support. * Add conversion from 1.3-and-earlier format to new system. * Don't ban the entire internet - convert zero-masks to the most specific ones. --- src/d_clisrv.c | 45 +++++++++++++---- src/d_netcmd.c | 8 +-- src/i_tcp.c | 134 ++++++++++++++++++++++++++----------------------- 3 files changed, 110 insertions(+), 77 deletions(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index 90f8cfa1..9cbb8752 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -180,7 +180,7 @@ consvar_t cv_playbackspeed = {"playbackspeed", "1", 0, playbackspeed_cons_t, NUL consvar_t cv_httpsource = {"http_source", "", CV_SAVE, NULL, NULL, 0, NULL, NULL, 0, 0, NULL}; -consvar_t cv_kicktime = CVAR_INIT ("kicktime", "10", CV_SAVE, CV_Unsigned, NULL); +consvar_t cv_kicktime = {"kicktime", "10", CV_SAVE, CV_Unsigned, NULL, 0, NULL, NULL, 0, 0, NULL}; static inline void *G_DcpyTiccmd(void* dest, const ticcmd_t* src, const size_t n) { @@ -2674,6 +2674,10 @@ static void Command_ShowBan(void) //Print out ban list } static boolean bansLoaded = false; +// If you're a community contributor looking to improve how bans are written, please +// offer your changes back to our Git repository. Kart Krew reserve the right to +// utilise format numbers in use by community builds for different layouts. +#define BANFORMAT 1 void D_SaveBan(void) { @@ -2700,6 +2704,9 @@ void D_SaveBan(void) return; } + // Add header. + fprintf(f, "BANFORMAT %d\n", BANFORMAT); + for (i = 0; (address = I_GetBanAddress(i)) != NULL; i++) { if (I_GetUnbanTime) @@ -2759,6 +2766,7 @@ void D_LoadBan(boolean warning) const char *username, *reason; time_t unbanTime = NO_BAN_TIME; char buffer[MAX_WADPATH]; + boolean banmode = 0; if (!I_ClearBans) return; @@ -2779,15 +2787,35 @@ void D_LoadBan(boolean warning) for (i = 0; fgets(buffer, (int)sizeof(buffer), f); i++) { - address = strtok(buffer, "/\t\r\n"); + address = strtok(buffer, " /\t\r\n"); mask = strtok(NULL, " \t\r\n"); - unbanTime = atoi(strtok(NULL, " \"\t\r\n")); + if (i == 0 && !strncmp(address, "BANFORMAT", 9)) + { + banmode = atoi(mask); + continue; + } - username = strtok(NULL, "\"\t\r\n"); // go until next " + // One-way legacy format conversion -- the game will crash otherwise + if (banmode == 0) + { + unbanTime = NO_BAN_TIME; + username = NULL; // not guaranteed to be accurate, but only sane substitute + reason = strtok(NULL, "\r\n"); + if (reason[0] == 'N' && reason[1] == 'A' && reason[2] == '\0') + { + reason = NULL; + } + } + else + { + unbanTime = atoi(strtok(NULL, " \"\t\r\n")); - strtok(NULL, "\"\t\r\n"); // remove first " - reason = strtok(NULL, "\"\r\n"); // go until next " + username = strtok(NULL, "\"\t\r\n"); // go until next " + + strtok(NULL, "\"\t\r\n"); // remove first " + reason = strtok(NULL, "\"\r\n"); // go until next " + } I_SetBanAddress(address, mask); @@ -2804,6 +2832,8 @@ void D_LoadBan(boolean warning) fclose(f); } +#undef BANFORMAT + static void Command_ReloadBan(void) //recheck ban.txt { D_LoadBan(true); @@ -3507,9 +3537,6 @@ static void Got_KickCmd(UINT8 **p, INT32 playernum) #ifdef DUMPCONSISTENCY if (msg == KICK_MSG_CON_FAIL) SV_SavedGame(); #endif - - LUAh_GameQuit(false); - D_QuitNetGame(); CL_Reset(); D_StartTitle(); diff --git a/src/d_netcmd.c b/src/d_netcmd.c index aadd48aa..7a76002f 100644 --- a/src/d_netcmd.c +++ b/src/d_netcmd.c @@ -1085,14 +1085,14 @@ boolean EnsurePlayerNameIsGood(char *name, INT32 playernum) if (len > 1) { name[len-1] = '\0'; - if (!IsNameGood (name, playernum)) + if (!EnsurePlayerNameIsGood(name, playernum)) return false; } else if (len == 1) // Agh! { // Last ditch effort... sprintf(name, "%d", M_RandomKey(10)); - if (!IsNameGood (name, playernum)) + if (!EnsurePlayerNameIsGood(name, playernum)) return false; } else @@ -1232,12 +1232,12 @@ static void CleanupPlayerName(INT32 playernum, const char *newname) * \param newname New name for that player. Should be good, but won't * necessarily be if the client is maliciously modified or * buggy. - * \sa CleanupPlayerName, IsNameGood + * \sa CleanupPlayerName, EnsurePlayerNameIsGood * \author Graue */ static void SetPlayerName(INT32 playernum, char *newname) { - if (IsNameGood(newname, playernum)) + if (EnsurePlayerNameIsGood(newname, playernum)) { if (strcasecmp(newname, player_names[playernum]) != 0) { diff --git a/src/i_tcp.c b/src/i_tcp.c index 25720b8e..c3d49ccc 100644 --- a/src/i_tcp.c +++ b/src/i_tcp.c @@ -1527,70 +1527,6 @@ static boolean SOCK_Ban(INT32 node) #endif } -static boolean SOCK_SetBanAddress(const char *address, const char *mask) -{ -#ifdef NONET - (void)address; - (void)mask; - return false; -#else - struct my_addrinfo *ai, *runp, hints; - int gaie; - - if (!address) - return false; - - memset(&hints, 0x00, sizeof(hints)); - hints.ai_flags = 0; - hints.ai_family = AF_UNSPEC; - hints.ai_socktype = SOCK_DGRAM; - hints.ai_protocol = IPPROTO_UDP; - - gaie = I_getaddrinfo(address, "0", &hints, &ai); - if (gaie != 0) - return false; - - runp = ai; - - while (runp != NULL) - { - INT32 ban; - - ban = numbans; - AddBannedIndex(); - - memcpy(&banned[ban].address, runp->ai_addr, runp->ai_addrlen); - - if (mask) - banned[ban].mask = (UINT8)atoi(mask); -#ifdef HAVE_IPV6 - else if (runp->ai_family == AF_INET6) - banned[ban].mask = 128; -#endif - else - banned[ban].mask = 32; - - if (banned[ban].mask > 32 && runp->ai_family == AF_INET) - banned[ban].mask = 32; -#ifdef HAVE_IPV6 - else if (banned[ban].mask > 128 && runp->ai_family == AF_INET6) - banned[ban].mask = 128; -#endif - - // Set defaults, in case anything funny happens. - SOCK_SetBanUsername(NULL); - SOCK_SetBanReason(NULL); - SOCK_SetUnbanTime(NO_BAN_TIME); - - runp = runp->ai_next; - } - - I_freeaddrinfo(ai); - - return true; -#endif -} - static boolean SOCK_SetBanUsername(const char *username) { #ifdef NONET @@ -1646,6 +1582,76 @@ static boolean SOCK_SetUnbanTime(time_t timestamp) #endif } +static boolean SOCK_SetBanAddress(const char *address, const char *mask) +{ +#ifdef NONET + (void)address; + (void)mask; + return false; +#else + struct my_addrinfo *ai, *runp, hints; + int gaie; + + if (!address) + return false; + + memset(&hints, 0x00, sizeof(hints)); + hints.ai_flags = 0; + hints.ai_family = AF_UNSPEC; + hints.ai_socktype = SOCK_DGRAM; + hints.ai_protocol = IPPROTO_UDP; + + gaie = I_getaddrinfo(address, "0", &hints, &ai); + if (gaie != 0) + return false; + + runp = ai; + + while (runp != NULL) + { + INT32 ban; + UINT8 numericalmask; + + ban = numbans; + AddBannedIndex(); + + memcpy(&banned[ban].address, runp->ai_addr, runp->ai_addrlen); + +#ifdef HAVE_IPV6 + if (runp->ai_family == AF_INET6) + banned[ban].mask = 128; + else +#endif + banned[ban].mask = 32; + + if (mask) + { + numericalmask = (UINT8)atoi(mask); + } + else + { + numericalmask = 0; + } + + if (numericalmask > 0 && numericalmask < banned[ban].mask) + { + banned[ban].mask = numericalmask; + } + + // Set defaults, in case anything funny happens. + SOCK_SetBanUsername(NULL); + SOCK_SetBanReason(NULL); + SOCK_SetUnbanTime(NO_BAN_TIME); + + runp = runp->ai_next; + } + + I_freeaddrinfo(ai); + + return true; +#endif +} + static void SOCK_ClearBans(void) { numbans = 0; From cd6b1b2cd955d0b5a6c70dbc80b5356696fbd4bc Mon Sep 17 00:00:00 2001 From: toaster Date: Tue, 14 Jun 2022 21:42:41 +0100 Subject: [PATCH 07/14] Enforce MAX_REASONLENGTH when reading ban.txt. --- src/d_clisrv.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index 9cbb8752..4c2579d2 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -2761,9 +2761,9 @@ static void Command_ClearBans(void) void D_LoadBan(boolean warning) { FILE *f; - size_t i; - const char *address, *mask; - const char *username, *reason; + size_t i, j; + char *address, *mask; + char *username, *reason; time_t unbanTime = NO_BAN_TIME; char buffer[MAX_WADPATH]; boolean banmode = 0; @@ -2817,6 +2817,19 @@ void D_LoadBan(boolean warning) reason = strtok(NULL, "\"\r\n"); // go until next " } + // Enforce MAX_REASONLENGTH. + if (reason) + { + j = 0; + while (reason[j] != '\0') + { + if ((j++) < MAX_REASONLENGTH) + continue; + reason[j] = '\0'; + break; + } + } + I_SetBanAddress(address, mask); if (I_SetUnbanTime) From 7bc59abc30f210e76ab8751e0a1dc22f82d9e3b8 Mon Sep 17 00:00:00 2001 From: toaster Date: Tue, 14 Jun 2022 22:09:27 +0100 Subject: [PATCH 08/14] Create a single struct for bannednode and bannednodetimelft, and use the matching ban ID inside that struct. While this commit does not increase the visibility of ban reasons, it makes this possible later. --- src/d_clisrv.c | 6 +++--- src/d_net.c | 3 +-- src/i_net.h | 9 +++++++-- src/i_tcp.c | 20 +++++++++----------- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index 4c2579d2..22de9981 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -4229,11 +4229,11 @@ static void HandleConnect(SINT8 node) // It's too much effort to legimately fix right now. Just prevent it from reaching that state. UINT8 maxplayers = min((dedicated ? MAXPLAYERS-1 : MAXPLAYERS), cv_maxplayers.value); - if (bannednode && bannednode[node]) + if (bannednode && bannednode[node].banid != SIZE_MAX) { - if (bannednodetimeleft && bannednodetimeleft[node] != NO_BAN_TIME) + if (bannednode[node].timeleft != NO_BAN_TIME) { - int minutes = bannednodetimeleft[node] / 60; + int minutes = bannednode[node].timeleft / 60; int hours = minutes / 60; if (hours) diff --git a/src/d_net.c b/src/d_net.c index 264e7c8b..cccd2088 100644 --- a/src/d_net.c +++ b/src/d_net.c @@ -86,8 +86,7 @@ boolean (*I_SetBanAddress) (const char *address, const char *mask) = NULL; boolean (*I_SetBanUsername) (const char *username) = NULL; boolean (*I_SetBanReason) (const char *reason) = NULL; boolean (*I_SetUnbanTime) (time_t timestamp) = NULL; -boolean *bannednode = NULL; -time_t *bannednodetimeleft = NULL; +bannednode_t *bannednode = NULL; // network stats diff --git a/src/i_net.h b/src/i_net.h index 155935f5..a5b1aba8 100644 --- a/src/i_net.h +++ b/src/i_net.h @@ -154,8 +154,13 @@ extern boolean (*I_SetBanAddress) (const char *address,const char *mask); extern boolean (*I_SetBanUsername) (const char *username); extern boolean (*I_SetBanReason) (const char *reason); extern boolean (*I_SetUnbanTime) (time_t timestamp); -extern boolean *bannednode; -extern time_t *bannednodetimeleft; + +typedef struct +{ + size_t banid; + time_t timeleft; +} bannednode_t; +extern bannednode_t *bannednode; /// \brief Called by D_SRB2Main to be defined by extern network driver boolean I_InitNetwork(void); diff --git a/src/i_tcp.c b/src/i_tcp.c index c3d49ccc..d0abb4f9 100644 --- a/src/i_tcp.c +++ b/src/i_tcp.c @@ -254,8 +254,7 @@ static banned_t *banned; static size_t numbans = 0; static size_t banned_size = 0; -static boolean SOCK_bannednode[MAXNETNODES+1]; /// \note do we really need the +1? -static time_t SOCK_bannednodetimeleft[MAXNETNODES+1]; +static bannednode_t SOCK_bannednode[MAXNETNODES+1]; /// \note do we really need the +1? static boolean init_tcp_driver = false; static const char *serverport_name = DEFAULTPORT; @@ -702,21 +701,21 @@ static boolean SOCK_Get(void) { if (curTime >= banned[i].timestamp) { - SOCK_bannednodetimeleft[j] = NO_BAN_TIME; - SOCK_bannednode[j] = false; + SOCK_bannednode[j].timeleft = NO_BAN_TIME; + SOCK_bannednode[j].banid = SIZE_MAX; DEBFILE("This dude was banned, but enough time has passed\n"); break; } - SOCK_bannednodetimeleft[j] = banned[i].timestamp - curTime; - SOCK_bannednode[j] = true; + SOCK_bannednode[j].timeleft = banned[i].timestamp - curTime; + SOCK_bannednode[j].banid = i; DEBFILE("This dude has been temporarily banned\n"); break; } else { - SOCK_bannednodetimeleft[j] = NO_BAN_TIME; - SOCK_bannednode[j] = true; + SOCK_bannednode[j].timeleft = NO_BAN_TIME; + SOCK_bannednode[j].banid = i; DEBFILE("This dude has been banned\n"); break; } @@ -725,8 +724,8 @@ static boolean SOCK_Get(void) if (i == numbans) { - SOCK_bannednodetimeleft[j] = NO_BAN_TIME; - SOCK_bannednode[j] = false; + SOCK_bannednode[j].timeleft = NO_BAN_TIME; + SOCK_bannednode[j].banid = SIZE_MAX; } return true; @@ -1756,7 +1755,6 @@ boolean I_InitTcpNetwork(void) I_SetBanReason = SOCK_SetBanReason; I_SetUnbanTime = SOCK_SetUnbanTime; bannednode = SOCK_bannednode; - bannednodetimeleft = SOCK_bannednodetimeleft; return ret; } From 40f7be76762416a2f898d817da827b078214ca70 Mon Sep 17 00:00:00 2001 From: toaster Date: Wed, 15 Jun 2022 13:36:32 +0100 Subject: [PATCH 09/14] Static for compilation warning. --- src/d_netcmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/d_netcmd.c b/src/d_netcmd.c index 7a76002f..9fa5db78 100644 --- a/src/d_netcmd.c +++ b/src/d_netcmd.c @@ -1044,7 +1044,7 @@ static boolean AllowedPlayerNameChar(char ch) return true; } -boolean EnsurePlayerNameIsGood(char *name, INT32 playernum) +static boolean EnsurePlayerNameIsGood(char *name, INT32 playernum) { INT32 ix; From 3498f1cff11c5e5bcabd7a4ca55f810ee3f96426 Mon Sep 17 00:00:00 2001 From: toaster Date: Wed, 15 Jun 2022 16:56:22 +0100 Subject: [PATCH 10/14] First commit with actual human testing involved. * Fix some bugs. * Reset bannode information properly, fixing being unable to join your own server. * Write to the buffer before saving the kick/ban reason, rather than after. * Improve the print output for the `showbanlist` command. * Includes username. * Includes remaining time as seen by a kicked joiner. * Hides expired bans. * Improve the messages for ban/kick related refused joins. * Replace the Reason with the actual admin-provided reason for refused connection. * Replace the "Server refuses connection" header with "You have been [banned/temporarily kicked] from the server", the previous given Reason. * Fudge the time reported for temporary kicks so that a user is encouraged to return slightly after their tempkick ends, rather than before. * Add an extra newline to the M_StartMessage for being kicked/banned with a reason provided. --- src/d_clisrv.c | 103 ++++++++++++++++++++++++++++++++++++------------- 1 file changed, 76 insertions(+), 27 deletions(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index 22de9981..8a983812 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -2642,7 +2642,9 @@ static void CL_ConnectToServer(void) static void Command_ShowBan(void) //Print out ban list { size_t i; - const char *address, *mask, *reason; + const char *address, *mask, *reason, *username; + time_t unbanTime = NO_BAN_TIME; + const time_t curTime = time(NULL); if (I_GetBanAddress) CONS_Printf(M_GetText("Ban List:\n")); @@ -2651,22 +2653,43 @@ static void Command_ShowBan(void) //Print out ban list for (i = 0; (address = I_GetBanAddress(i)) != NULL; i++) { - if (!I_GetBanMask || (mask = I_GetBanMask(i)) == NULL) - CONS_Printf("%s: %s ", sizeu1(i+1), address); - else - CONS_Printf("%s: %s/%s ", sizeu1(i+1), address, mask); + unbanTime = NO_BAN_TIME; + if (I_GetUnbanTime) + unbanTime = I_GetUnbanTime(i); - if (I_GetUnbanTime && I_GetUnbanTime(i) != NO_BAN_TIME) - { - // todo: maybe try to actually print out the time remaining, - // and/or the datetime of the unbanning? - CONS_Printf("(temporary) "); - } + if (unbanTime != NO_BAN_TIME && curTime >= unbanTime) + continue; + + CONS_Printf("%s: ", sizeu1(i+1)); + + if (I_GetBanUsername && (username = I_GetBanUsername(i)) != NULL) + CONS_Printf("%s - ", username); + + if (!I_GetBanMask || (mask = I_GetBanMask(i)) == NULL) + CONS_Printf("%s", address); + else + CONS_Printf("%s/%s", address, mask); if (I_GetBanReason && (reason = I_GetBanReason(i)) != NULL) - CONS_Printf("(%s)\n", reason); - else - CONS_Printf("\n"); + CONS_Printf(" - %s", reason); + + if (unbanTime != NO_BAN_TIME) + { + // these are fudged a little to match what a joiner sees + int minutes = ((unbanTime - curTime) + 30) / 60; + int hours = (minutes + 1) / 60; + int days = (hours + 1) / 24; + if (days) + CONS_Printf(" (%d day%s)", days, days > 1 ? "s" : ""); + else if (hours) + CONS_Printf(" (%d hour%s)", hours, hours > 1 ? "s" : ""); + else if (minutes) + CONS_Printf(" (%d minute%s)", minutes, minutes > 1 ? "s" : ""); + else + CONS_Printf(" (<1 minute)"); + } + + CONS_Printf("\n"); } if (i == 0 && !address) @@ -3428,6 +3451,11 @@ static void Got_KickCmd(UINT8 **p, INT32 playernum) msg = KICK_MSG_CON_FAIL; } + if (msg == KICK_MSG_CUSTOM_BAN || msg == KICK_MSG_CUSTOM_KICK) + { + READSTRINGN(*p, reason, MAX_REASONLENGTH+1); + } + //CONS_Printf("\x82%s ", player_names[pnum]); // Save bans here. Used to be split between here and the actual command, depending on @@ -3534,12 +3562,10 @@ static void Got_KickCmd(UINT8 **p, INT32 playernum) kickreason = KR_BAN; break; case KICK_MSG_CUSTOM_KICK: - READSTRINGN(*p, reason, MAX_REASONLENGTH+1); HU_AddChatText(va("\x82*%s has been kicked (%s)", player_names[pnum], reason), false); kickreason = KR_KICK; break; case KICK_MSG_CUSTOM_BAN: - READSTRINGN(*p, reason, MAX_REASONLENGTH+1); HU_AddChatText(va("\x82*%s has been banned (%s)", player_names[pnum], reason), false); kickreason = KR_BAN; break; @@ -3561,9 +3587,9 @@ static void Got_KickCmd(UINT8 **p, INT32 playernum) else if (msg == KICK_MSG_BANNED) M_StartMessage(M_GetText("You have been banned by the server\n\nPress ESC\n"), NULL, MM_NOTHING); else if (msg == KICK_MSG_CUSTOM_KICK) - M_StartMessage(va(M_GetText("You have been kicked\n(%s)\nPress ESC\n"), reason), NULL, MM_NOTHING); + M_StartMessage(va(M_GetText("You have been kicked\n(%s)\n\nPress ESC\n"), reason), NULL, MM_NOTHING); else if (msg == KICK_MSG_CUSTOM_BAN) - M_StartMessage(va(M_GetText("You have been banned\n(%s)\nPress ESC\n"), reason), NULL, MM_NOTHING); + M_StartMessage(va(M_GetText("You have been banned\n(%s)\n\nPress ESC\n"), reason), NULL, MM_NOTHING); else M_StartMessage(M_GetText("You have been kicked by the server\n\nPress ESC\n"), NULL, MM_NOTHING); } @@ -3782,6 +3808,8 @@ static void ResetNode(INT32 node) nodewaiting[node] = 0; playerpernode[node] = 0; sendingsavegame[node] = false; + bannednode[node].banid = SIZE_MAX; + bannednode[node].timeleft = NO_BAN_TIME; } void SV_ResetServer(void) @@ -4231,27 +4259,39 @@ static void HandleConnect(SINT8 node) if (bannednode && bannednode[node].banid != SIZE_MAX) { + const char *reason = NULL; + + // Get the reason... + if (!I_GetBanReason || (reason = I_GetBanReason(bannednode[node].banid)) == NULL) + reason = "No reason given"; + if (bannednode[node].timeleft != NO_BAN_TIME) { - int minutes = bannednode[node].timeleft / 60; - int hours = minutes / 60; + // these are fudged a little to allow it to sink in for impatient rejoiners + int minutes = (bannednode[node].timeleft + 30) / 60; + int hours = (minutes + 1) / 60; + int days = (hours + 1) / 24; - if (hours) + if (days) { - SV_SendRefuse(node, va(M_GetText("You have been temporarily\nkicked from the server.\n(Time remaining: %d hour%s)"), hours, hours > 1 ? "s" : "")); + SV_SendRefuse(node, va("K|%s\n(Time remaining: %d day%s)", reason, days, days > 1 ? "s" : "")); + } + else if (hours) + { + SV_SendRefuse(node, va("K|%s\n(Time remaining: %d hour%s)", reason, hours, hours > 1 ? "s" : "")); } else if (minutes) { - SV_SendRefuse(node, va(M_GetText("You have been temporarily\nkicked from the server.\n(Time remaining: %d minute%s)"), minutes, minutes > 1 ? "s" : "")); + SV_SendRefuse(node, va("K|%s\n(Time remaining: %d minute%s)", reason, minutes, minutes > 1 ? "s" : "")); } else { - SV_SendRefuse(node, M_GetText("You have been temporarily\nkicked from the server.\n(Time remaining: <1 minute)")); + SV_SendRefuse(node, va("K|%s\n(Time remaining: <1 minute)", reason)); } } else { - SV_SendRefuse(node, M_GetText("You have been banned\nfrom the server.")); + SV_SendRefuse(node, va("B|%s", reason)); } } else if (netbuffer->u.clientcfg._255 != 255 || @@ -4526,8 +4566,17 @@ static void HandlePacketFromAwayNode(SINT8 node) CL_Reset(); D_StartTitle(); - M_StartMessage(va(M_GetText("Server refuses connection\n\nReason:\n%s"), - reason), NULL, MM_NOTHING); + if (reason[1] == '|') + { + M_StartMessage(va("You have been %sfrom the server\n\nReason:\n%s", + (reason[0] == 'B') ? "banned\n" : "temporarily\nkicked ", + reason+2), NULL, MM_NOTHING); + } + else + { + M_StartMessage(va(M_GetText("Server refuses connection\n\nReason:\n%s"), + reason), NULL, MM_NOTHING); + } free(reason); From 3ff5e3f8cdba1a816a46cc167442d478189d93dc Mon Sep 17 00:00:00 2001 From: toaster Date: Wed, 15 Jun 2022 17:00:09 +0100 Subject: [PATCH 11/14] Fix an issue where if the last line of an M_StartMessage was the longest, the box width wouldn't account for it. --- src/m_menu.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/m_menu.c b/src/m_menu.c index 09d0f393..b0738da3 100644 --- a/src/m_menu.c +++ b/src/m_menu.c @@ -4567,7 +4567,11 @@ void M_StartMessage(const char *string, void *routine, } if (i == strlen(message+start)) + { start += i; + if (i > max) + max = i; + } } MessageDef.x = (INT16)((BASEVIDWIDTH - 8*max-16)/2); From 7c92a7efbb269de20ac522b50ce679a193147909 Mon Sep 17 00:00:00 2001 From: toaster Date: Wed, 15 Jun 2022 22:34:05 +0100 Subject: [PATCH 12/14] Complete BANFORMAT header implementation. * Warn the user when an incompatible ban.txt is being loaded, and stop early. * Don't inexplicably assign as a boolean, you bafooligan! --- src/d_clisrv.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index 8a983812..a0ec5386 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -2789,7 +2789,7 @@ void D_LoadBan(boolean warning) char *username, *reason; time_t unbanTime = NO_BAN_TIME; char buffer[MAX_WADPATH]; - boolean banmode = 0; + UINT8 banmode = 0; if (!I_ClearBans) return; @@ -2816,6 +2816,18 @@ void D_LoadBan(boolean warning) if (i == 0 && !strncmp(address, "BANFORMAT", 9)) { banmode = atoi(mask); + switch (banmode) + { + case BANFORMAT: // currently supported format + //case 0: -- permitted only when BANFORMAT string not present + break; + default: + { + fclose(f); + CONS_Alert(CONS_WARNING, "Could not load unknown ban.txt for ban list (BANFORMAT %d, expected %d)\n", banmode, BANFORMAT); + return; + } + } continue; } From 36e44f2f6fb042fc611a0c8cc15b0a4834350684 Mon Sep 17 00:00:00 2001 From: toaster Date: Sun, 3 Jul 2022 22:01:51 +0100 Subject: [PATCH 13/14] Free the banned struct on SOCK_ClearBans. --- src/i_tcp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/i_tcp.c b/src/i_tcp.c index d0abb4f9..36740eb8 100644 --- a/src/i_tcp.c +++ b/src/i_tcp.c @@ -1655,6 +1655,7 @@ static void SOCK_ClearBans(void) { numbans = 0; banned_size = 0; + Z_Free(banned); banned = NULL; } From fbb613b2d62390dae3e91d26c6fd5917585ed173 Mon Sep 17 00:00:00 2001 From: toaster Date: Mon, 4 Jul 2022 14:00:12 +0100 Subject: [PATCH 14/14] Catch several ways ban.txt could be malformed by a well-meaning server host, and report it back via the log. --- src/d_clisrv.c | 43 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index a0ec5386..e31ccb56 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -2790,6 +2790,7 @@ void D_LoadBan(boolean warning) time_t unbanTime = NO_BAN_TIME; char buffer[MAX_WADPATH]; UINT8 banmode = 0; + boolean malformed = false; if (!I_ClearBans) return; @@ -2815,7 +2816,10 @@ void D_LoadBan(boolean warning) if (i == 0 && !strncmp(address, "BANFORMAT", 9)) { - banmode = atoi(mask); + if (mask) + { + banmode = atoi(mask); + } switch (banmode) { case BANFORMAT: // currently supported format @@ -2824,32 +2828,56 @@ void D_LoadBan(boolean warning) default: { fclose(f); - CONS_Alert(CONS_WARNING, "Could not load unknown ban.txt for ban list (BANFORMAT %d, expected %d)\n", banmode, BANFORMAT); + CONS_Alert(CONS_WARNING, "Could not load unknown ban.txt for ban list (BANFORMAT %s, expected %d)\n", mask, BANFORMAT); return; } } continue; } + if (I_SetBanAddress(address, mask) == false) // invalid IP input? + { + CONS_Alert(CONS_WARNING, "\"%s/%s\" is not a valid IP address, discarding...\n", address, mask); + continue; + } + // One-way legacy format conversion -- the game will crash otherwise if (banmode == 0) { unbanTime = NO_BAN_TIME; username = NULL; // not guaranteed to be accurate, but only sane substitute reason = strtok(NULL, "\r\n"); - if (reason[0] == 'N' && reason[1] == 'A' && reason[2] == '\0') + if (reason && reason[0] == 'N' && reason[1] == 'A' && reason[2] == '\0') { reason = NULL; } } else { - unbanTime = atoi(strtok(NULL, " \"\t\r\n")); + reason = strtok(NULL, " \"\t\r\n"); + if (reason) + { + unbanTime = atoi(reason); + reason = NULL; + } + else + { + unbanTime = NO_BAN_TIME; + malformed = true; + } username = strtok(NULL, "\"\t\r\n"); // go until next " + if (!username) + { + malformed = true; + } strtok(NULL, "\"\t\r\n"); // remove first " reason = strtok(NULL, "\"\r\n"); // go until next " + if (!reason) + { + malformed = true; + } } // Enforce MAX_REASONLENGTH. @@ -2865,8 +2893,6 @@ void D_LoadBan(boolean warning) } } - I_SetBanAddress(address, mask); - if (I_SetUnbanTime) I_SetUnbanTime(unbanTime); @@ -2877,6 +2903,11 @@ void D_LoadBan(boolean warning) I_SetBanReason(reason); } + if (malformed) + { + CONS_Alert(CONS_WARNING, "One or more lines of ban.txt are malformed. The game can correct for this, but some data may be lost.\n"); + } + fclose(f); }