ergochat / ergo

A modern IRC server (daemon/ircd) written in Go.
https://ergo.chat/
MIT License
2.28k stars 181 forks source link

k-lines produce a QUIT snotice without a preceding CONNECT #1941

Closed slingamn closed 2 years ago

slingamn commented 2 years ago

reported by @tacerus and @xnaas

See #1303 for a similar issue. K-lines are processed only after the user completes connection registration and obtains a nickname:

https://github.com/ergochat/ergo/blob/2a3b8e648cbaedc5a2d6d347ad44aadc3e240bf5/irc/server.go#L372

If the user is k-lined, registration is stopped before playRegistrationBurst, which also sends the CONNECT snotice:

https://github.com/ergochat/ergo/blob/2a3b8e648cbaedc5a2d6d347ad44aadc3e240bf5/irc/server.go#L403

however, (*Client).destroy() sees that the client is in fact registered and sends a QUIT snotice:

https://github.com/ergochat/ergo/blob/2a3b8e648cbaedc5a2d6d347ad44aadc3e240bf5/irc/client.go#L1349

causing confusion.

We should probably just special-case k-lines and prevent them from sending the QUIT snotice?

slingamn commented 2 years ago

Discussion from IRC: we can't move the evaluation of klines before the decisive moment of connection registration (claiming a nickname) because that code can also override the user's choice of nickname, which potentially changes the outcome of the kline evaluation.

Evaluation of a kline depends on untrusted user-manipulable data that is submitted during connection registration (the NICK and USER lines): therefore klines are simultaneously harder for the server to evaluate and easier for clients to evade. Consequently they are deprecated in favor of the other two UBAN types (IP bans and account suspensions).

ghost commented 2 years ago

Consequently they are deprecated in favor of the other two UBAN types (IP bans and account suspensions).

The only trouble with that is: a kline can cover an "infinite" (very large, since IRC has nick length limits) number of bans that account suspensions cannot easily. e.g. wanting to ban a racist or derogatory term from being used in a nickname. It's easier to kline say badword!*@* and badword*!*@* and other variations than to register and suspend an impossibly large number of accounts.

Unless account suspensions support wildcards in some way and I'm just stupid. :D

prdes commented 2 years ago

Watching NICK <badword> abuse has usually been looked upon as a client implementation, if required.

  1. On its own it provides a very specific soln to a specific problem unless we're writing OperServ this doesn't solve any meta goal.
  2. Whereas client-bot implementations seem to already be doing a decent job among other things.
slingamn commented 2 years ago

We'll have to agree to disagree here, but I don't see the "bad words in nicknames" use case as very compelling:

  1. It's relatively easy for a determined troll to work around the restrictions
  2. Is a bad word in a nickname more of a moderation problem than a client joining with an innocuous nickname, then posting offensive or shocking content?

In both cases the recommended solution is to put REQUIRE-SASL bans on the affected IP blocks.

ghost commented 2 years ago

As mentioned in IRC, I'm aware of the limitations of a KLINE and how it isn't an all-powerful tool to stop all abuse, just wanted to put in my 2¢ after seeing the word "deprecated." 😛

slingamn already re-assured me with:

<slingamn> oh don't worry, klines will always be supported
<slingamn> they are just a "dark corner" of ergo that has cobwebs
<slingamn> and you might get bitten by a spider