This was in src/hash.c, src/list.c and src/modules/stats.c.
Now all in src/modules/nick.c... or should this go into a new module?
Again, this needs some more testing, like previous commit.
Better to have this all in one place. Though, must admit, the
config checking is still in src/conf.c and a bit of a hassle to move.
Some testing may be wise to see if everything still works ;)
Several notes:
* This only checks on-JOIN (not on nick change, message, etc)
for performance reasons
* If the #channel in ~inherit:#channel also contains ~inherit
entries then those are not processed (no recursion and no looping)
* Only a limited number of ~inherit entries is permitted.
This will be moved to set:: items in a future commit so you
can set different amounts for +b/+e/+I ~inherit.
* This is work in progress, UnrealIRCd or the entire world could explode
* Documentation will follow later
Developers:
* Sadly, clean_ban_mask() needed to be changed to have two more
parameters, 'ban_type' and 'channel' were added at different positions.
This because the module needs the ban type (EXBTYPE_BAN, EXBTYPE_EXCEPT,
EXBTYPE_INVEX) and channel because it rejects based on number of
existing ~inherit entries in the channel... and while is_ok() is called
for local clients and has all this information, for services clients
is_ok() is not called so the only way to reject the +beI is through
xxx_conv_param() which comes from clean_ban_mask().
We already allow users in after 10 seconds if CBL is too slow, and
that part worked correctly. However 5 seconds later, when the URL
API 15 second timeout hits, it would try to allow the user in AGAIN.
This caused the user to be introduced twice, causing remote servers
to kill the user, and also screwing up user counts.
Reported by multiple people, including Jellis who actually filed
a report with logs, and alice providing logs as well, all when CBL
was temporarily down for a few hours in August 2024. In hindsight
this bug was already reported by k4be back in November 2023 but
was more rare at the time and mistaken for another very similar
bug that was fixed in 6.1.3.
With this patch, we check before we call cbl_allow(), but also
cbl_allow() itself checks the "user already allowed in?".
Oh yeah and this is hot patchable, within the hour I will make
this work: ./unrealircd hot-patch cbl-timeout-fix
This to replace the scattered IP setting. It is very important to always
use set_client_ip() from this point. Everywhere!
Also, in addition to client->ip, this adds client->rawip that contains
the IP in network byte order. In older UnrealIRCd versions we always had
the raw IP but not the IP as a string, so we moved to IP as a string,
but it can be useful to have both in terms of optimizations.
Of course, then the client->ip and client->rawip always need to 100% match,
hence the set_client_ip().
This also changes IsIPV6() to do A BUGFIX, it changes it from:
* if local user is the user connected over IPv6? Otherwise, does it have ':' in the IP?
To:
* check if the IPv6 flag is set (which is set if IP contains ':')
This may seem insignificant but it means that for spoofed IP addresses,
such as WEBIRC or transparant proxy, we use the correct transport.
Previously, if the proxy was IPv6 then even if the spoofed user was using
IPv4, the ident check would still be tried over IPv6. That sort of fun.
From now in, in such a situation client->local->socket_type will be
SOCKET_TYPE_IPV6 but since client->ip (and rawip) will contain IPv4
the IsIPV6() will actually return false, as it should be.
Also, in the HOOKTYPE_IP_CHANGE, enforce that if HOOK_DENY is returned,
the the user is killed by dead_link(). The user must be killed because
that is what we expect, and you cannot use exit_client() because from
some code paths that would be too much freed structures / hassle,
as a comment in src/modules/connect-flood.c correctly states:
/* There are two reasons why we can't use exit_client() here:
* 1) Because the HOOKTYPE_IP_CHANGE call may be too deep.
* Eg: read_packet -> webserver_packet_in ->
* webserver_handle_request_header -> webserver_handle_request ->
* RunHook().... and then returning without touching anything
* after an exit_client() would not be feasible.
* 2) Because in HOOKTYPE_ACCEPT we always need to use dead_socket
* if we want to print a friendly message to TLS users.
*/
Contrary to retrieving country code / country name, the AS organisation
name that was returned needs to be freed by *US*. Makes sense, though
a bit inconsistent, heh.
[skip ci]
Yeah not really important except for like SVSMODE -b nick, which
removes all bans that affect nick. That's the only type of code
that runs bans against external users.
This also automatically adds it as a security group and mask item:
ban user {
mask { asn 64496; }
reason "Testing ASN ban";
}
And yeah, it is a normal extban too (in +b and +I). Users usually
don't know the AS Number of other users, though, unless you change
the default configuration (at the cost of privacy).
Updated release notes a bit... more will follow.
* Including default download via unrealircd.org
* Shown in WHOIS - currently in RLP_WHOISCOUNTRY, not sure
if that is correct.
* Shown in connect notices [asn: XYZ] [asname: BLAH BLAH]
* Shown in json user expansion (JSON logging and JSON-RPC)
* Only via geoip_classic at the moment
* Structs and serializing in geoip_base done
* Extbans not added yet
The list is as follows with the number of bytes in the test leaked,
but this can vary depending on your configuration:
* charsys with multibyte ranges (112 bytes)
* set::whois-details (909 bytes)
* +F default profile (7 bytes)
The whois one is in the default configuration, so likely
affected everyone. It's nothing catastrophic, as you need a 1000
REHASHes in order to reach 1MB but.. we shouldn't leak, of course.
at selected places (there needs to be explicit code in place to handle this).
At the moment it is supported at two places only:
* For spamfilters (was already possible via crules via ::rule with
a destination('xyz') but now non-crule destination "#xyz"; works as well, eg:
spamfilter {
...
except {
destination "#main";
}
}
Note that if you want to exempt a destination in all spamfilters,
we already have set::spamfilter::except for that!
* In restrict commands for like channel-message and such:
set {
restrict-commands {
channel-message {
except {
connect-time 600;
destination "#test";
}
}
}
}
Allow passing a crule_context via user_allowed_by_security_group_context()
and make user_allowed_by_security_group() call that.
Actually document spamfilter::except online in the docs (yeah you
won't see it in this commit, just mentioning...)
And yeah, by now i wonder if we should really call it crule_context
since it is more like a security group matching context, but.. whatever.
The reason for the warning is that in some future UnrealIRCd version I want the
rpc-user::rpc-class to become a required item.
This commit also adds rpc-class.default.conf which is by default
included from rpc.modules.default.conf.
This also completes the TODO list from b9de933378
(the rpc.add_timer was never a loophole and i kept rpc.info as-is)
This wasn't caught by Address Sanitizer because we simply never removed
it from the linked list, and thus it was a reachable pointer.
Found this bug when adding the rpc-class { } stuff.
This so you can restrict the JSON Methods that can be called, eg:
rpc-class limited {
privileges {
server { list; get; }
channel { list; get; }
user { list; get; }
}
}
rpc-user xyz {
match { ip 127.0.0.1; }
password "test";
rpc-class limited;
}
NOTE: This is work in progress
1) Things are NOT yet fully contained, as i need to lock down
rpc.add_timer still :)
2) Some more work, eg rpc.info would be nice to show some
information about the restriction (??)
3) Need to fix a memory leak
4) Possibly more
The build tests were failing for tkldb save & restore. Cause was this:
if (tkl->ptr.serverban->match)
^ this is wrong.. because it doesn't check if tkl is a server ban
So it could be tkl->ptr.spamfilter->whatever_is_at_that_memory_offset
which is non-NULL.
Could have updated the code to an if (IsServerBan... && tkl->..etc..)
but decided to ditch this needless code altogether.
As this wasn't needed at all since it already skips config-based.
And all mask items bans are config-based.
One of those rare cases where trying to be extra careful actually
causes a fuckup.
Hmm... we should probably use json_expand_tkl() differently for match items
instead of returning "<match-item>" literally. Consider this a TODO item :D
This only happens for config-based bans that can't be removed anyway, so..
This so you can get the same spamreport data to your own custom system.
It works similar to set::central-blocklist::url but then on a
spamreport { } basis which is better, since then you can still
submit to UnrealIRCd central spamreport too.
So you can have two blocks:
spamreport unrealircd { type central-spamreport; }
spamreport custom { type central-spamreport; url 'https://www.example.org/xyz'; }
And then a /SPAMREPORT or 'report;' action will report it to BOTH.
Requested by Chris
* restrict-commands: add option 'channel-create' for managing who may create new channels.
This has been a commonly requested feature with different requested options, I think it makes sense to do it properly from here
Reported by Valware. E.g. if HOOKTYPE_CAN_JOIN rejects the join
when it is a new channel.
( And yeah... +P channels are not destroyed... handled in
sub1_from_channel() -> HOOKTYPE_CHANNEL_DESTROY already. )
There was an incorrect OperOverride message if you were had +h, +a or +q
and was kicking someone that you should normally be able to (without override).
This requires quite a bit of further testing, though, it's so easy to get
this wrong. The FIXME still stands to fix this for good some day.
Reported by Valware in https://bugs.unrealircd.org/view.php?id=6423
Reported by hnj in https://bugs.unrealircd.org/view.php?id=6418
Appears to have been introduced as part of the 6.x refactor of secret/private channel modes in 8066c13876
Also adjust message for ERR_OPERSPVERIFY to include channel name.
This is to correspond closer to other similar numerics around this area, as well as agreeing with the definition within modern.
crule is used outside security groups / spamfilter, like in
deny link { }.
Also update the match_realname() since via the extban code it would
use match_esc() which is rather confusing if you have double (or
perhaps even triple) escaping when using this in the conf.
Bram Matthys