The former didn't make much sense:
if (!rejected_deletes && *rejected_deletes > max_rejected_deletes)
The most simple fix would have been removing the '!' there.
However, i chose to rewrite part of the function so we only set
*rejected_deletes once, and use normal integers (not pointer to integers)
in all the preceding code. Less room for error.
Only match was working earlier, and for now both are accepted,
like everywhere else. Reported by BlackBishop.
Also, added a missing check for unknown rpc-user items, so a
proper "Unknown directive" error is thrown.
(this missing check made the first issue worse)
that was added late in 6.1.1 development to fix a crash with removing
websocket listeners. Now replaced with a generic HOOKTYPE_CONFIG_LISTENER
that is not only called for removed listeners, but for all listeners.
The whole point of (G)ZLINEs is that it rejects instantly upon
accept, that's what makes them different from KLINE/GLINE.
Commit 89075e532a made it
accidentally use the slow path for this as well.
Without CMD_BIGLINES: parameters to commands can be 510 bytes max
(but eg. strlen(parv[1])+strlen(parv[2]) can be >510, like 510*2,
when received from servers with BIGLINES support).
If someone does set CMD_BIGLINES in their CommandAdd() then the
parameter(s) size is not limited an can be up to 16k.
This is a bit more risky than previous but i think most command
handlers can handle parameters of max BUFSIZE/512 just fine
and care less about the grand total. Also, the risk is only
from server traffic and not from user traffic. Still, we will
keep going through the source to check for issues.
to avoid a crash in todays code which was like:
1) exit_client gets called
2) close_connection() sets client->direction to NULL
3) a bit further it calls remove_dependents()
4) a sendto is attempted and the new code accesses
client->direction which is unexpected to be NULL
Actually i should probably trace the cause of the sendto_one()
but that is another story ;)
previously it was set to 'nick'
Also allow the full topic length for the nick-user-host case, now that
we have BIGLINES support. For non-BIGLINES-servers this could mean a
potential cutoff of the last 20 characters of the topic, which is why we
restricted it to 340 instead of 360 for nick-user-host previously, but
that is really only in the corner case / worst case, like with max NICKLEN,
max USERLEN, max HOSTLEN, max CHANNELLEN, etc... i think we can live
with that small "problem" until all servers upgrade.
when it is not possible, mixed server scenario).
Now a big RRPC response like server.module_list for a remote server
(44KB) fits in only 3 lines, instead of almost 100 lines.
`PROTOCTL BIGLINES` is set. This will allow us to do things more
efficiently and possibly raise some other limits in the future.
This 16k is the size of the complete line, including sender,
message tags, content and \r\n. Also, in server-to-server traffic
we now allow 30 parameters (MAXPARA*2).
The original input size limits for non-servers remain the same: the
complete line can be 4k+512, with the non-mtag portion limit set
at 512 bytes (including \r\n), and MAXPARA is still 15 as well.
* I chose 16k because I don't want to first raise it to like 8k
and then realize later that 16k would be better and raise it again.
* To receive BIGLINES in a command, you need to `CommandAdd()` with
flags `CMD_BIGLINES`, without it you still get regular 512 max.
This is so, because a lot of the code does not expect longer than
512 bytes lines or in parameters, so we can gradually change that
(where needed).
https://ircv3.net/specs/extensions/chathistory#isupport-tokens
The spec says they should be 'in order of decreasing preference'.
As currently the only backend is in-memory, this doesn't matter so I
picked `msgid` first (as it's less ambiguous); but this can be revisited
later if/when adding a backend which is more efficient with timestamps.
Has to do with running HOOKTYPE_SERVER_CONNECT too soon, before
introducing ourselves to the other side. This bug was created in
commit ddf639836b so exists in
all UnrealIRCd 6 versions (-beta1 and up).
The hook call is now moved further down.
unreal_duplicate_masks()
duplicate_nvplist()
duplicate_name_list()
And use this for when proxy::type is web, to duplicate the
exact criteria to the ban exception as mentioned in previous
commit.
for blacklist, connect-flood, handshake-data-flood
(Well, unless mask::ip is used with a wildcard, due to current
technical limitations, that will be resolved later)
and also for safety when redoing DNS and ident due to IP change,
we now:
ClearIdentLookupSent(client);
ClearIdentLookup(client);
ClearDNSLookup(client);
depending on what we get from the proxy, so it can be used later
in the websocket module for setting the user secure or not
(the latter similar to what k4be already did in the old code).
This now requires a proxy { } block -- docs follow soon
This uses part of k4be's code still, to do the parsing,
so still only "Forwarded" and quick workaround for bug
when for=XXX is the final item.
a function called start_dns_and_ident_lookup(). This can then
be easily called from other places as well, like the code k4be
did in src/modules/websocket.c to handle proxies.
Side-effect is that ident lookups would now be done, if we are
configured to do so, for forwarded webirc stuff (not that I
think many people use that feature at the moment...).
It is now possible to override some set settings per-security group by
having a set block with a name, like `set unknown-users { }`
* You could use this to set more limitations for unknown-users:
```
set unknown-users {
max-channels-per-user 5;
static-quit "Quit";
static-part yes;
}
```
* Or to set higher values (higher than the normal set block)
for trusted users:
```
security-group trusted-bots {
account { BotOne; BotTwo; }
}
set trusted-bots {
max-channels-per-user 25;
}
```
* Currently the following settings can be used in a set xxx { } block:
set::auto-join, set::modes-on-connect, set::restrict-usermodes,
set::max-channels-per-user, set::static-quit, set::static-part.
That file has such a clear warning in it but still people
manage to load old ones. That being said, usually it is not
deliberate, like an cp ../unrealircd.old/conf/* conf/
the default set::max-channels-per-user (also called set::maxchannelsperuser).
This way you can give known-users a higher max-channels-per-user,
or even a special security group for trusted users (that you may
already have given a more lax flood setting and lower lag-penalty
etc. etc. so that fits in nicely)
And yeah this also:
* Makes it both in set and the anti-flood block accept both
maxchannelsperuser and max-channels-per-user.
* Removes old MAXCHANNELS= in 005, as we already have CHANLIMIT=
This does not:
* Re-announce the 005 CHANLIMIT= if someone transitions from a security
group with a different max-channels-per-user. We don't do that for
IRCOps either, and I think no IRCd does that actually...
To be honest i wonder if sending the limit in 005 is useful at all,
do client really track this and limit their GUI based on it?? Doubt it!
(or changing the port number). Reported by Nini.
Rather complex case: when the listen block is removed, obviously
the config hooks are not called for the (now non-existing) listen
block, and thus the websocket->request_handler and such are not
set to the new address of the websocket handler.
We now use a slightly silly workaround / new hook to fix this
corner case. Ideally there would be an extra layer in-between
like a handler lookup by name, or something like that.
(Or make the websocket module PERM but we don't want that!)
This so you can use throttling exceptions (eg in ELINE) on hostnames.
That is, the above is during normal circumstances. Similar to previous
commit we will turn this feature of during high connection rates.
That is a TODO item.
This is the start of "be more friendly to TLS users with disconnect
error messages" from https://bugs.unrealircd.org/view.php?id=5532
As that bug explains:
Consider doing the SSL/TLS handshake even for throttling errors and such
when the (reject) connection rate is below a certain amount per second. If
it is higher than a certain rate, then fall back to the original behavior to
reject the user instantly without handshake or looking at any data.
Rationale: the current/original behavior is there so the ircd can handle
floods, both in terms of traffic and in terms of CPU usage (the SSL/TLS
handshake is quite costly after all). The downside of the current behavior
is that TLS users don't see the error message, usually. This feature
request tries to find a middle ground.
Still a TODO item:
* We don't detect high rates yet, so we only do this new behavior atm
and not yet the old behavior during high connection rates.
* Verify that error messages/behavior hasn't changed (too) much,
like the throttling and the banning disconnect messages.
Bram Matthys