k-lines produce a QUIT snotice without a preceding CONNECT

[slingamn]

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:

ergo/irc/server.go

Line 372 in 2a3b8e6

// check KLINEs (#671: ignore KLINEs for loopback connections)

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

ergo/irc/server.go

Line 403 in 2a3b8e6

server.snomasks.Send(sno.LocalConnects, fmt.Sprintf("Client connected [%s] [u:%s] [h:%s] [ip:%s] [r:%s]", d.nick, d.username, session.rawHostname, session.IP().String(), d.realname))

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

ergo/irc/client.go

Line 1349 in 2a3b8e6

client.server.snomasks.Send(sno.LocalQuits, fmt.Sprintf(ircfmt.Unescape("%s$r exited the network"), details.nick))

causing confusion.

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

[slingamn]

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

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]

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]

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.

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