mostly with regards to memory leaks if duplicate config directives are used.
Eg using allow::password twice in the same allow block, or using
link::outgoing::tls-options twice in the same link block. Unusual stuff.
Previously if a new history item was added (because someone sent a message)
we would always append at the end of chat history buffer of the channel.
Now we put the message at the position decided by the "time" message tag,
which could be at the end but also slightly before that.
* Upside: should result in a consistent chat history on all servers
* Downside: if your server time is off for several seconds then it
could look a little weird. Then again, it would already have looked weird
in real live chat with timestamps and when replaying chat history probably.
Also add some simple optimizations: in the log line object we now have direct
pointers to the msgid and time strings, so the code doesn't need to do a
find_mtag() all the time. This should lower CPU usage during log playback
and also makes things more simple in the source code.
I did some testing with various history injection variants but this needs
more extensive testing.
I was dumb: with an RSA cert you need ECDHE-RSA-* and i had
only included ECDHE-ECDSA-*. Long story short: TLSv1.2 didn't work
if you had an RSA certificate. Reported by BlackBishop, and in
hindsight also by Mi_92. Thanks for the quick reports, this should
be a quick fix :-)
For reference, the established TLS connections at irc*.unrealircd.org
over the past 6 months were:
14379 TLSv1.3-TLS_CHACHA20_POLY1305_SHA256
368 TLSv1.2-ECDHE-ECDSA-AES256-GCM-SHA384
160 TLSv1.2-ECDHE-ECDSA-CHACHA20-POLY1305
3 TLSv1.3-TLS_AES_256_GCM_SHA384
There is nobody connecting with AES CBC in those statistics
(ECDHE-ECDSA-AES256-SHA256 and ECDHE-ECDSA-AES128-SHA384)
In config.h we had a:
#if OPENSSL_VERSION_NUMBER >= 0x10100000L
#define UNREALIRCD_DEFAULT_ECDH_CURVES "X25519:secp521r1:secp384r1:prime256v1"
#else
#define UNREALIRCD_DEFAULT_ECDH_CURVES "secp521r1:secp384r1:prime256v1"
#endif
...which is fine in theory, but openssl headers are not included at that point,
so OPENSSL_VERSION_NUMBER was not defined.
From now on, we have:
#define UNREALIRCD_DEFAULT_ECDH_CURVES_PRIMARY "X25519:secp521r1:secp384r1:prime256v1"
#define UNREALIRCD_DEFAULT_ECDH_CURVES_SECONDARY "secp521r1:secp384r1:prime256v1"
...and we try them in that order. If both fail, we exit with an error (like before).
This because X25519 is not available in OpenSSL before 1.1.0 (so really old)
and may also not be available when running in FIPS mode.
+#define HAS_ASN1_TIME_diff
+#define HAS_SSL_CTX_SET_MIN_PROTO_VERSION
+#define HAS_SSL_CTX_SET_SECURITY_LEVEL
+#define HAS_X509_check_host
+#define HAS_X509_get0_notAfter
In practice, this only adds that we now do certificate expiry checks
and give warnings, like on *NIX.
The HAS_X509_check_host is good because then OpenSSL/LibreSSL code is
used instead of the one we have from cURL and the ssl conservatory.
To be honest I wanted to rip out this fallback completely at first,
but let's do that in next major version of UnrealIRCd and not during
an existing series.
The HAS_SSL_CTX_SET_* would have given an admin the option to downgrade
to TLSv1.0 or TLSv1.1 but LibreSSL no longer builds with these since
LibreSSL 3.8.1, which is sensible, so... no actual change there.
I'll document the behavior in the docs (wiki), though.
Also the previous claim in b653c68df0 with
regards to what curves were actually enabled in our LibreSSL UnrealIRCd 6
builds was incorrect, an hour ago I claimed X448 would show up as an extra,
but that is not the case (that was with OpenSSL). The correct statement is:
"This also meant the default curves that were offered were up to LibreSSL,
which meant the following list in practice:
Elliptic curves offered: prime256v1 secp384r1 X25519
Instead of:
Elliptic curves offered: prime256v1 secp384r1 secp521r1 X25519"
So it was only missing secp521r1. Nothing major.
"[error] ecdh-curves specified but your OpenSSL/LibreSSL library does not
support setting curves manually by name. Either upgrade to a newer library
version or remove the 'ecdh-curves' directive from your configuration file"
This also meant the default curves that were offered were up to LibreSSL,
which meant the following list in practice:
Elliptic curves offered: prime256v1 secp384r1 secp521r1 X25519 X448
Instead of:
Elliptic curves offered: prime256v1 secp384r1 secp521r1 X25519
Not that X448 is considered bad, it just didn't match what we claimed in
the docs at https://www.unrealircd.org/docs/TLS_Ciphers_and_protocols
Fixed by: #define HAS_SSL_CTX_SET1_CURVES_LIST
* [Security group blocks](https://www.unrealircd.org/docs/Security-group_block)
are now hidden in lists by default. If you want the security group to be shown
in things like `MODE #channel +b ~security-group:x` (which shows a list)
then you need to use `public yes;`. The default security groups
like known-users, webirc-users, etc. are public by default.
Eg: vhost "$operlogin@$operclass.example.net";
Also add potentially_valid_vhost() function which can be used in
config code to ignore invalid $vars. Then at runtime you use the
real valid_vhost() function after variable expansion by
unreal_expand_string().
and use it not only from vhost { } block code but also for like
blacklist::reason.
This so the same variables with the same names are available at
those places.
Supported are:
$nick, $username, $realname, $ip, $hostname, $server, $account,
$operlogin, $operclass, $country_code (xx for unknown),
$asn (0 for unknown).
* Convert to use module-based config handling
* Split part of VHOST command into do_vhost() for later
* Use AppendListItem instead of AddListItem so they are in config-order.
This is not really important atm but will matter later if we go auto.
* No other code changes at this point
This so if there is ever an issue, we can hot-patch it. This affects
exit_client(), exit_client_fmt(), exit_client_ex(), banned_client(),
and various (internal) help functions.
This also means you cannot call these functions during TEST/INIT (eg
during REHASH) since the 'quit' module which provides these modules
may not be loaded yet. I don't think that's a situation/problem but
this needs some more testing.
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().
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.
*/
Was previously shown as like :0 when removing/adding a listen block
with a UNIX socket. Now shows the file, as you would expect.
And in the boot screen it was like:
[info] IPv4: 127.0.0.1:6697(TLS), 127.0.0.1:6667
[info] IPv6: *:6667, *:6697(TLS)
And now also:
[info] Unix Sockets: /home/unrealircd/unrealircd/data/rpc.socket
* 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
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.
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
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
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.
match_user_extended_server_ban except that it works by name/value.
This can then be used by crules or in other mods, like:
user_matches_extended_server_ban(client, "country", "NL");
If the performance impact isn't too bad (of the extra work) then
this prevents duplicate code in the handler for things like
that: account, country, certfp, and whatever we add in the future..
Bram Matthys