search

ClassiCube / MCGalaxy

A Minecraft Classic / ClassiCube server software
GNU General Public License v3.0
162 stars 76 forks source link

There is not enough default functionality to prevent cheating in games/parkour maps #722

Open Goodlyay opened 1 year ago

Goodlyay commented 1 year ago

The following have been addressed in NA2 through a plugin, but new servers should not have to write code or find plugins to fix them.

forkiesassds commented 1 year ago

shouldn't be able to use reload in -hax maps (realmowner exempt)

Was partially resolved in #612

forkiesassds commented 1 year ago

Also shouldn't the player's model be reset if it's scaled from the default humanoid model?

Goodlyay commented 1 year ago

Also shouldn't the player's model be reset if it's scaled from the default humanoid model?

Good point. LockedModel plugin should also be default functionality, but I forgot to include it here as these issues are only what was addressed by a single na2 plugin

ddinan commented 1 year ago

Fully agree with most of these as I've spent ages working on plugins to prevent some of these issues, though I must admit, I'm not sure how I feel about the shouldn't be able to use write or MB while muted "feature". I can understand the sentiment but these are building tools so it would very much weigh under whether or not muted players should be allowed to use build tools. I guess it could be seen as evasion if used in a public map but I don't think it's really needed when used in the player's own maps.

rdebath commented 1 year ago

Nearly all of these changes strike me as "shouldn't have extra powers while playing a subgame". (Not the first four.)

Perhaps what's really needed here is a 'drop privileges' mode, where a user only has "guest" permissions when playing on a specific map. If they must switch back to "operator" (or whatever) they have to do /main to leave the game map first (or use /rank ?).

This could be faked by adding ranks that have guest permissions plus the ability to switch back to a normal rank. (probably some auto-switching and property saving would be good too)

Oh, and /reload should be fixed so it just does a reload not a setspawn. (Because block mis-matches between server and client are a thing)

Goodlyay commented 1 year ago

Nearly all of these changes strike me as "shouldn't have extra powers while playing a subgame". (Not the first four.)

Perhaps what's really needed here is a 'drop privileges' mode, where a user only has "guest" permissions when playing on a specific map. If they must switch back to "operator" (or whatever) they have to do /main to leave the game map first (or use /rank ?).

This sounds far too over-engineered and over-encompassing than it needs to be. What about all the non-guest commands that are fine to use in an adventure map? Not to mention how confusing it would be if your rank in /who or /info is suddenly different.

Oh, and /reload should be fixed so it just does a reload not a setspawn. (Because block mis-matches between server and client are a thing)

Any operation which causes server to tp the client to an absolute location must be prohibited in -hax maps, to prevent clipping through walls. Rejoining a map does this.

rdebath commented 1 year ago

Your confusing aspect is is solved by the "rank" description of "operator playing as guest".

The complexity issues you mention also apply to what you're suggesting in that you are adding -hax which is officially a client side controller into the rank/permission calculations. You're creating a new pseudo rank (permission set) that has a few, hard coded, permissions turned off in exactly the same way that the rank switching does. This scatters code across the system into these individual items. Much of the code added is actually a duplication of the existing permission checks (and special case code in the /rank changes).

It certainly sounds like something close to the faked suggestion would fit what you want. A rank with just the safe commands enabled that everyone uses on the game map. They use the same permissions so that no one has an unfair advantage. Also it's a rank (permission set) so it can be adjusted without code change if necessary.

I'll agree that there is some additional changes like these that I'd love to see done, for example moving the /os and the /mb permissions into the standard permission framework. But this doesn't do that, it would just integrate your changes into the existing framework rather than duck taping them onto the side like the /mb restrictions.

WRT: /reload I guess you mean either round trip delays or the 32nds being a rounded position. Okay I see that. I guess that means we want a /reload without position change and/or a partial reload.

Goodlyay commented 1 year ago

Your confusing aspect is is solved by the "rank" description of "operator playing as guest".

The complexity issues you mention also apply to what you're suggesting in that you are adding -hax which is officially a client side controller into the rank/permission calculations. You're creating a new pseudo rank (permission set) that has a few, hard coded, permissions turned off in exactly the same way that the rank switching does. This scatters code across the system into these individual items. Much of the code added is actually a duplication of the existing permission checks (and special case code in the /rank changes).

Unless you're suggesting that the pseudo rank can be overlaid like a mask on top of your existing rank to only change a select few permissions, then I stand by my original comment. Your method breaks intended permissions for: • Ops being able to use opchat • Admins being able to use adminchat • Being able to use staff commands in general • Staff rank being immune to kicks/bans from lower staff ranks • Commands like /nick, /title, /teams, and a massive host of others that have nothing to do with -hax maps

However if you are suggesting a mask-type rank, then your criticisms about my method adding complexity to the code seems to be unfounded since this would require a bunch of changes to the existing framework as well.

Also it's a rank (permission set) so it can be adjusted without code change if necessary.

I agree that it would be nice if it wasn't completely hardcoded, though. There should probably be server property which describes the highest rank to be affected by the -hax restrictions (and all higher are not affected).

WRT: /reload I guess you mean either round trip delays or the 32nds being a rounded position. Okay I see that. I guess that means we want a /reload without position change and/or a partial reload.

I haven't done any testing to see if the round trip delay causes issues, but you're correct that the 32nds rounded position is what causes the clipping.

Additionally, reloads must be prevented because some adventure maps have clientside changes that will be reset if the map reloads, which can completely break the adventure map.

rdebath commented 1 year ago

Okay, I'm not looking at permission set merging.

What I'm looking at here is something related to the su, sg, and sudo unix commands. The ranks are much like unix groups, so this would be related to the sg or substitute group command. You have highlighted that some commands would have to use the real group rather than the effective group but otherwise the idea is for the user to run completely as their effective group with the exception that they can switch back to their real group when they choose.

So given that you want these users to still have two hats (player and admin) this implies that a sudo like command is also needed so the result could be:

/ban dumbuser
Only Operator+ can use /Ban
/sudo /ban dumbuser
dumbuser was banned by rdebath.
- dumbuser kicked Banned by rdebath: You're banned!

Here the /sudo command runs the command using the real group rather than the effective group. Obviously this /sudo command would have to be runnable by anyone, but effective only for people who have a suitable real group (eg the group placed in "extra permission").

There is already some substitute rank functionality in the /sendcmd implementation, so that's not new. The new stuff would be switching groups and forcing the use of the real rank for selected functions.

One last thing I would usually consider /invincible an admin tool which should not be turned off by this. However, you feel it's been used for abuse ... this is likely a problem, in that it seems difficult to define what an admin is and what commands are "nothing to do with -hax maps". So I can't tell for sure if this is what you actually need.

Goodlyay commented 1 year ago

One last thing I would usually consider /invincible an admin tool which should not be turned off by this. However, you feel it's been used for abuse ... this is likely a problem, in that it seems difficult to define what an admin is and what commands are "nothing to do with -hax maps". So I can't tell for sure if this is what you actually need.

Users need /invincible in order to build/test in their own maps that use kill barriers and such.

You're correct that there's a problem with some of these features being turned off if you're an admin. Two possible solutions: 1) Every item here with (realmowner exempt) specified needs to also be (staff exempt). 2) Implement a command that allows you to ignore -hax restrictions. NA2 already does this with /maphack, which can be used by realm owners, or staff on any map.

NA2 implements both solutions, but the second one alone is better than the first one alone IMO, since the hax off switcher is very useful for testing/editing adventure maps as well.

/maphack is similar to your proposed sudo idea, except it's a mode instead of having to prefix any blocked command you want to use.