Pryaxis / TShock

☕️⚡️TShock provides Terraria servers with server-side characters, anti-cheat, and community management tools.
GNU General Public License v3.0
2.43k stars 382 forks source link

TShock Bans Unable to do Offline #2382

Open Marcus101RR opened 3 years ago

Marcus101RR commented 3 years ago

It appears there is no clear instructions, or proper documentation regarding offline ban before the changes were made to the new version. Previous version would ban offline players if the target was not online or matched exactly. In this case, it appears there is no source code that deliberately looks for an offline player, infact most code uses "player" and bans the account of that player, but requires all of this to be online.

This generally defeats the purpose of said player if we can't ban the account in offline mode, and hunting down the IP address of an account with user info seems futile.

Either, this was not implemented and ignored, or there is a bug.

hakusaro commented 3 years ago

We’ve heard multiple complaints from Off and it turns out that he was using a hard fork of TShock with a completely different ban system. Are you using vanilla TShock or a fork?

csmir commented 3 years ago

The current ban system has a very hard to grasp offline system using the -e flag and including the type of input as: "ip:playerip/uuid:playeruuid/.." to provide the exact input correctly. Without this '-e' flag, the command will look for an online player name and try to parse it. With the -e flag, you can put in any valid input, even if it (the player) doesnt exist. This works for offline bans, but it feels incredibly uncomfortable and is quite challenging to understand for new or unexperienced server staff/owners.

An example of a working command on banning an offline account:

I should note that I haven't been able to test how spaced names work, but I assume as following:

Marcus101RR commented 3 years ago

We’ve heard multiple complaints from Off and it turns out that he was using a hard fork of TShock with a completely different ban system. Are you using vanilla TShock or a fork?

The issue was posted part of vanilla tshock, so I expect that this is related to tshock. I don't use any forks, or other versions, due to stability.

@Rozen4334 So it is possible just wasn't in the help section apparently, Is ACC: required?

csmir commented 3 years ago

From what I've understood while reading up on the source code, the value type specifier is required so it can be pushed into database in the correct column. (You are basically formatting the exact('-e') input as it is sent into the ban table yourself, it only sorts by the specifier you grant) This is all the reason it has to have it included.

My worry is that as acc: is in fact required to ban a user by name, it makes it impossible to ban unregistered users if they're offline. I am positive this was an issue in previous versions as well however, as the username was and is never made known to the database while unregistered.

bartico6 commented 3 years ago

lmao what

It appears there is no clear instructions, or proper documentation regarding offline ban before the changes were made to the new version. Previous version would ban offline players if the target was not online or matched exactly. In this case, it appears there is no source code that deliberately looks for an offline player, infact most code uses "player" and bans the account of that player, but requires all of this to be online.

because the new system isnt centered around a "player" being banned. the new system operates on ban factors which it fills from an online player for convenience. if the player is offline you must specify the ban factors (account name, ip, whatever) yourself. the new system is essentially a robust playername, account, ip & uuid blacklist that can quickly dish out all those entries for the blacklist from an online player (or manually if offline)

This generally defeats the purpose of said player if we can't ban the account in offline mode, and hunting down the IP address of an account with user info seems futile.

always been the case, no? i dont think you can ban an offline player with old versions, itd only try to ban the account or name or whatever. if you really need to be able to action offline players as if they were online, make a plugin that tracks recent disconnections & their data and use that to issue bans lol

csmir commented 3 years ago

There have been methods to get around this in versions before 4.5, a piece of reworked ban code that provides the ability to ban offline users in the same format as online users. This code however has become quite a challenge to update with a decent chunk of the methods in ban being rewritten.

A fair chunk of the popular public servers have shared it between each other in plugin or simply as text. It does not surprise me that there is in fact some expectation that banning offline players is "normalized". Not having been an issue in the past and only appearing now would be caused by this popular workaround.

csmir commented 3 years ago

I should note that I haven't been able to test how spaced names work, but I assume as following:

  • acc:"name name"

Tested this to be sure. The above returns acc:.. "acc:name name" is the correct format.

QuiCM commented 3 years ago

There are, quite simply, far too many edge cases that prevent simple, accurate, and concise banning of offline players based on the limited and arbitrary data made available by Terraria to identify a specific user - particularly while that user isn't even connected

In the past we supported it and had a ban system that performed suboptimally and a ban database that was unmaintainable

The new system has the acc:, ip:, and uuid: specifiers. As mentioned you can use any and all of these with the -e (exact) flag to provide exact input to the system

The new system intends to be extensible (you should be able to very easily add new identifiers that can be banned with) - if it's difficult to do so please open a separate issue so it can be made more workable

Marcus101RR commented 3 years ago

There are, quite simply, far too many edge cases that prevent simple, accurate, and concise banning of offline players based on the limited and arbitrary data made available by Terraria to identify a specific user - particularly while that user isn't even connected

In the past we supported it and had a ban system that performed suboptimally and a ban database that was unmaintainable

The new system has the acc:, ip:, and uuid: specifiers. As mentioned you can use any and all of these with the -e (exact) flag to provide exact input to the system

The new system intends to be extensible (you should be able to very easily add new identifiers that can be banned with) - if it's difficult to do so please open a separate issue so it can be made more workable

It is confirmed, ip: bans are ineffective, I banned my IP address to test it, and was able to join the server, It is completely ignoring ip parameter, only noticed this today as we just had a massive hacker attack who bypassed every ban, only thing that we found is he used the same ip over and over again. I will test UUID next to make sure. Account works so far as confirmation.

TShock 4.5.4

I executed this command: /ban add ip:myip "Test" 0d -e I was able to join instantly.

Edit: My assumption is we are doign something wrong with infinite bans, as 0d seems to no longer work as permanent.

bartico6 commented 3 years ago

There are, quite simply, far too many edge cases that prevent simple, accurate, and concise banning of offline players based on the limited and arbitrary data made available by Terraria to identify a specific user - particularly while that user isn't even connected In the past we supported it and had a ban system that performed suboptimally and a ban database that was unmaintainable The new system has the acc:, ip:, and uuid: specifiers. As mentioned you can use any and all of these with the -e (exact) flag to provide exact input to the system The new system intends to be extensible (you should be able to very easily add new identifiers that can be banned with) - if it's difficult to do so please open a separate issue so it can be made more workable

It is confirmed, ip: bans are ineffective, I banned my IP address to test it, and was able to join the server, It is completely ignoring ip parameter, only noticed this today as we just had a massive hacker attack who bypassed every ban, only thing that we found is he used the same ip over and over again. I will test UUID next to make sure. Account works so far as confirmation.

TShock 4.5.4

I executed this command: /ban add ip:myip "Test" 0d -e I was able to join instantly.

Edit: My assumption is we are doign something wrong with infinite bans, as 0d seems to no longer work as permanent.

The fault is in 0d. 0d is a ban that expires the instant its issued. Try perm or some invalid date string.

QuiCM commented 3 years ago

No date string = permanent

moisterrific commented 3 years ago

offline bans are working fine, unfortunately there was not adequate explanation for how you could ban offline. even a long time TShock user like myself found the new bans to be very confusing at first. hopefully this (will be coming in a future TShock update) helps to addresses that problem and make it a bit easier to understand https://github.com/Pryaxis/TShock/pull/2412

if you're trying to ban an unregistered user offline then yea it doesn't work, that didn't work with the old ban system either.

it's probably a good idea to enforce player registration and login on your server if you think it's at risk of getting griefed.

Cordium1990 commented 2 years ago

The current ban system is a piece of shit.

QuiCM commented 2 years ago

The current ban system is a piece of shit.

Your contribution is neither helpful nor constructive. Please review the guidelines for participating in discussion in this community