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.
In all my tests on real servers this was never a reported leak,
because the dbuf_delete() already happens at other places where the
client is marked dead.
However, with my (private) fuzzing patches I need this freeing because
of a slightly different code path.
I'm putting the patch in mainline just in case I'm wrong and it does
trigger in some kind of niche situation.
The IRCd is still responsive (as the bad I/O is not prioritzed) but this
isn't good either. Only happens with some rare triggers.
This was previously reported over e-mail in an older UnrealIRCd version
but after 6-8 hours of debugging I was never able to trigger it.
Later it finally happened on one of my servers and I could debug it.
Reported by bss on IRC.
Changed:
r->ipv6 = IsIPV6(client);
To:
r->ipv6 = IsIPV6(client) ? 1 : 0;
The problem is that:
#define IsIPV6(x) ((x)->flags & CLIENT_FLAG_IPV6)
(..so without ?1:0..)
made this effectively:
r->ipv6 = CLIENT_FLAG_IPV6;
..which is..
#define CLIENT_FLAG_IPV6 0x800000000 /**< client is using IPv6 */
.. and 0x800000000 doesn't fit in r->ipv6, which is of size 'char' (so max is 0xff)
+#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
Now this works like:
if the time param exists, even without a reason, it will be checked if it's a time param. if it's not a time param, it'll be considered to be the reason (or the first part of it anyway)
Reported by PeGaSuS in https://bugs.unrealircd.org/view.php?id=6105
... and make set::max-inherit-extended-bans::ban-exception default to 0
because that functionality is not implemented
The +e's are already checked when using +b ~inherit though..
lookup fails the old result stays there which is confusing.
Reported on IRC where 10.x.x.x was shown as "Poland" which was a
leftover from the "real IP" before WEBIRC spoofing was used to set
the IP to 10.x.x.x. Reported by Jellis.
* [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.
ban ip {
mask { 1.1.1.1; 2.2.2.2; 3.3.3.3; }
reason "Go away";
}
Or the alternate form:
ban ip {
mask 1.1.1.1;
mask 2.2.2.2;
mask 3.3.3.3;
reason "Go away";
}
Suggested by magic000 in https://bugs.unrealircd.org/view.php?id=4599
Note that this is not a Mask item, these are special, hence the
special code.
There was a typo where it was inheriting exclude-ip entries as
ip entries. This could have been very dangerous but fortunately
exclude-ip was broken so it was impossible to add exclude-ip
entries and that list was always empty / NULL.
This only affected proxy { } blocks with type forwarded/x-forwarded/
cloudflare. The proxy block worked fine, but we also tried to exempt
these IPs from blacklist checking and connect-flood and this was
NOT effective due to this bug... even though the entries were shown
in "STATS except" with these IPs (because 'printable_list' was
correctly duplicated).
Other than that very particular use-case, this function is not used
at the moment.
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).
$nick, $username, $realname, $ip, $account, $operlogin, $operclass,
$country_code (xx for unknown), $asn (0 for unknown).
Note that if a $variable fails to expand, eg $operlogin but the
user is not oper, then the vhost will not be applied. A warning
is sent to the vhost snomask (+s +v) in such a case.
Examples:
/* Set authenticated users to $account.example.org */
vhost { auto-login yes; vhost $account.example.org; mask { identified yes; } }
/* Obviously not really a good idea, but.. to illustrate: */
vhost { auto-login yes; vhost $country_code.example.org; mask *; }
Also, when vhost { } blocks are read and need to be matched, they
are read top-down now, which is the most logical way. First match wins.
All this needs testing :)
and if so, it sets the vhost on the user. Except when the user already
has a vhost (eg from anope during SASL).
If vhost::auto-login is 'yes' then you don't need ::login and ::password.
Suggested by PeGaSuS.
Support for variables like $account in vhost::vhost, more examples and
a release notes entry will follow in later commit(s).
* 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
Bram Matthys