Commits
Failing to set outgoing_response_info to NULL after finishing parsing a
command means any future commands that do *not* have a label tag will
incorrectly inherit that labeled response context. This can cause, among
other things, crashes when sending an unlabeled remote command after a
labeled remote command.
Also add a regression test for this behaviour.
Should be plural, not singular. This cap is unused in solanum's codebase yet advertised anyway so the only correction needed to happen is in the cap list table.
- Solve crash in free_response_batch by not attempting to remove NULL
batch ids from the dictionary (indicating there was no response batch
to actually free)
- Specify the correct server mask for the solanum.chat/response tag when
using hunt_server based on whether the hunted param is a client vs a
server
The TAP protocol doesn't allow extra stdout lines unless they're
commented (which these were not), and I don't see the value in printing
out every line we're munching, so instead use the new drain_client_sendq
function introduced in the previous commit to empty out the client's
sendq rather than these printf while loops.
- Add helper method to drain a client's sendq without asserting on any
of its lines
- Add helper method to fetch a dictionary by name, as these are
typically static linkage but could be useful for inspection or
manipulation in tests
- Add helper method to run an event by name, as these are typically
static linkage but could be useful to run during tests
- Make the cleanup_remote functions work properly by exiting the correct
client with the correct parameters
- Add clients created by text utilities to relevant tracking variables
- New helper methods to create clients with IDs
- Fix return value of get_client_sendq to always be a char * (some cases
previously returned const char *)
Local dev env had /bin/sh aliased to /bin/bash but CI runs on Ubuntu
where /bin/sh is dash, and lacks array support. Debian is similar in
this regard, so ensure that the script works across any posix-compliant
shell rather than using bash-isms.
A new custom build target sets up the runtime dir the way the
testsuite expects it so that it can run properly. Also fix the help
script as it was trying to treat a file as a symlink when it wasn't
which broke the new build target. Tests now also properly use the
TAP protocol in the meson build file, and fix a segfault in the
send1 tests that passes NULL to a function that previously assumed
that NULL would not be passed.
The error path was not freeing memory allocated during earlier
loop invocations.
The new documentation is organized differently to (hopefully) introduce
the TS6 server-to-server protocol in an easy to understand manner while
also serving as a useful reference for ircd and services developers who
wish to make use of this protocol to link to solanum.
Other documentation files were updated as well to correct discrepancies
noticed while writing the updated TS6 documentation.
The previous API was too error-prone, as evidenced by the large amount
of crash bugs discovered with it. New functions were added to response.h
to more easily manage labeled-response state, and some state tracking
was moved into core (such as CLICAP_RECEIVE_LABEL, which must only ever
be on one client at a time). Because this tracking is now in core,
cap_labeled_response needs to be a core module as having that bit
unloaded will cause issues. m_batch was also moved into being a core
module as more and more core things have depended on batch support
existing (and more will depend on it in the future).
m_list no longer re-uses remote response batches for SAFELIST, instead
making use of new APIs to suspend a labeled-response batch and more
easily resume it.
Batch resumption was made more focused as well, to ensure that we don't
accidentally lose batches due to setting outgoing_response_info to NULL.
The free_response_batch takes a resumption pointer to avoid cases where
we could introduce use-after-free by combining
free+resume_response_batch.
struct ResponseInfo now takes strduped pointers instead of directly
holding the buffers, and some fields were merged into a flags field or
omitted entirely due to new suspension/resumption support. This will
have a minor positive impact on memory usage as we no longer need to
allocate buffers for maximum-sized strings for every labeled-response.
Finally, shorten the dictionary label for pending responses. It was
overflowing its spot in STATS B, making the data after it misaligned.
Introduced in 66a8049e700c5c55ddd32cf211c1ceef06992ff3
We now disconnect the client if they attempt to run CAPAB before running
PASS (introducing themselves as a TS6 server with an SID) or if they've
already sent USER/NICK info. USER/NICK already has the other half of the
requisite checks by disconnecting the user if they have an SID. As such,
the logic flow of requiring an SID before CAPAB means users can never
have server caps set on them.
This prevents various issues such as a client sending CAPAB STAG
receiving all message tags, including those designed to be s2s-only.
This commit adds a missing send_multiline_reset call
before a return in clicap_generate in m_cap.c.
Without this, if running CAP LIST, the send_multiline_init call
just above it would not be cleaned up.
It assumes that send_multiline_reset is always safe to call,
which is true at the time of this commit.
Avoids use-after-free if the client takes more than 10 seconds to fully
receive the LIST output.
free_response_batch will no-op if MyConnect(client_p) is false. This
would always be the case with the former timing, as it was called after
exit_local_client which clears the MyConnect flag. Do it before instead
so we can properly clean up the tracking vars (namely, the
pending_responses dict, which otherwise had a use after free when firing
the event to clean up a response for an exited client).
While we're at it, change the previous fix in cap_labeled_response from
IsIOError to IsAnyDead to catch more cases where a client cannot
receive data. This has not caused any crashes, but the expanded check
includes IOError so is safe to use, and all of the statuses checked by
IsAnyDead preclude the ability to send data to the client.
Compare pointer values and set to NULL before freeing the pointer. Also
set outgoing_response_info to NULL as well (this is pointless at the
moment since it will always be followed by a call that resets it to the
orig_response_info pointer, but future-proofs it in the event that
future code doesn't do that)
In the event that LIST output is small enough to complete without
invoking SAFELIST code (i.e. the buffer doesn't exceed half of the
client's SendQ), the stashed outgoing_response_info in struct ListClient
is the same pointer as the outgoing_response_info for the LIST command.
When we finish sending the LIST, we free the stashed struct ListClient's
outgoing_response_info and then restore the outgoing_response_info from
the LIST command, and attempt to do things with it. However, that memory
has already been freed. Detect the case when the pointers are the same
value and properly wipe them both so we no longer attempt to read from
it.
Trying to send things to disconnected clients turns out to be a bad
idea. This change effectively ignores any labels on QUIT.
- Unconditionally require stdbool.h in IRCd
We are a C99 project anyway (configure.ac and meson.build both
require a C99 compiler), so this header will always be present
This removes the awful hack defining bool, true, and false in
its absense, which was creating build errors when using meson,
because meson.build did not set the HAVE_STDBOOL_H macro
- Modernise definition of alloca(3) according to GNU guidance
This was in both librb and IRCd, while it only needs to be in
the former
- Fix missing endianness check in librb/meson.build
IRCd and librb both use the WORDS_BIGENDIAN macro, which was
only being set by their configure.ac (with the AC_C_BIGENDIAN
M4 macro)
Since IRCd's stdinc.h includes librb's rb_lib.h, which then
includes librb's librb-config.h, which would have the macro
defined there, we only need to check for it in librb, so we
can remove the endianness check from IRCd's configure.ac and
don't need to add it to IRCd's meson.build
- Use __has_attribute to detect whether compiler attributes are
available
This is supported by gcc (5+), clang (2.9+), and ICC (17+)
- Add a __noreturn macro for __attribute__((__noreturn__)) and
adjust the codebase to use that
- Remove inclusion of headers that are only used by librb
- Remove inclusion of headers that are included in rb_lib.h
because stdinc.h includes rb_lib.h anyway
- Remove the checks for headers and functions from the IRCd
configure.ac and meson.build that are only used by librb
- Alphabetise checks for headers and functions (by the name of
the macro they define) in the meson.build files
- Use check_header() instead of has_header() in meson to see if
it is actually possible to compile code using the header
- Remove dead code using srand48(3) which has never been
compiled because neither build system has ever set the
HAVE_SRAND48 macro and the surrounding code makes no use of
this libc RNG
- Remove the TLS defines from IRCd's meson.build because librb's
meson.build already defines them and IRCd pulls those in
- The hurt extension doesn't work well and is now supplanted by
m_quarantine
- ip_cloaking_old has been broken (causes segfaults) for years now,
remove it
- ip_cloaking_3.0 has hardcoded keys that require .c file edits to be
secure. Anyone using it will probably maintain their fork of this file
anyway, but it's not good for new people to use. Remove that too
- Remove LOCOPS and umode +l entirely; they serve no useful purpose on
unified networks. This also removes the no_locops extension since it
is now useless
Previously, batches would end prematurely if they traveled through an
intermediate server before reaching their destination because the
intermediate server would send ENCAP ACK multiple times, causing the
pending server count to go down way too fast. Solve this by introducing
two things:
- Check that the direction of the incoming command matches the direction
we'd be sending the ACK to (avoids sending additional ACKs for replies
from remote servers that we pass along)
- Add a mask parameter to the tracking metadata for remote responses and
only send an ACK if we match the mask. this way things like remote
WHOIS don't generate ACKs from intermediate servers
Additionally, fix our manipulation of outgoing_response_info in m_list
to properly set CLICAP_RECEIVE_LABEL during SAFELIST buffering. The
logic here largely matches that in m_batch.
While base cases already worked fine with just cap_labeled_response, a
handful of special cases need batched responses, such as multi-target
messages. The module also needed updating to work properly with
echo-message, as the spec says that the echo is the one that needs the
label if the client messages themselves, rather than the main message,
and the echo is always sent last. This logic is also applied to mass
messages that include the oper sending it.
The handle_special method was broken up and made less special, treating
the various cases as additional target types rather than processing them
right away. The opers@server hack was also removed entirely since it is
obviated by other means of messaging opers on a server such as LOCOPS in
the event such is even necessary. Or, you know, just use the staff
channel.
With SAFELIST, the labeled-response batch does not necessarily finish
right away. As such, save off the context for the client's LIST command
and restore it each time we send them more data.
This concerns client-initiated batches only. Save the labeled-response
context as part of the Batch so that we can restore that context later
once the batch is complete. The label is added to the batch start
message, but we only process batches once we receive the batch complete
message (which lacks a label tag), and we could potentially be finishing
a batch due to it timing out if the client never sends a batch complete
message. By restoring context, we ensure that hooks such as
outbound_msgbuf are operating with all of the state they would expect to
have in order to properly attach tags to outgoing messages.
Add calls to generate local or remote labeled-response batches to all
modules which generate multiple lines of response. In general, this is
only done once we've confirmed we will be sending multiple lines, to
avoid spurious batches which only contain single lines inside of the
batch, however some cases will always generate a batch regardless due to
ease of implementation.
This patch covers all of the "easy" cases; future patches will cover the
complicated ones: m_batch, m_message, and m_list.
This module provides the labeled-response capability as well as an
internal non-propagated ?receive_label capability that is dynamically
added/removed from the client which should receive a labeled-response.
By using a second dummy capability, I don't need to rewrite our msgbuf
caching layer which is keyed entirely off the capmasks of the client in
order to vary who receives the label/batch tags.
It also introduces a s2s-only message tag solanum.chat/response which is
used to inform remote servers that their replies should be labelled.
Remote responses are always batched, and the batch is generated by the
local server to ensure that 100% of incoming labelled commands receive a
labeled-response. The solanum.chat/response tag is passed back in
replies so the local server can add the appropriate batch tag to the
response passed to the client. Remote servers will also send an ENCAP
ACK command to indicate they are done responding in the event that they
received an incoming s2s command with the solanum.chat/response tag.
The module keeps track of outgoing responses and if, by the end of
command processing (parse_end hook), it hasn't attached a label to
anything it will send a labelled ACK to the client. If it has begun a
labeled-response batch, it will also automatically close that batch,
reducing the likelihood of programming errors forgetting to close
batches.
Two new lines were added at the end of STATS t output to list the number
of pending client-initiated batches (and pending lines in those batches)
as well as the number of pending remote labeled-response batches.
When allocate_batch_message is called on a batch containing valueless
tags (i.e. tag value is NULL), the ircd would core because we attempted
to call strlen on a NULL pointer. Properly detect NULL values and don't
attempt to copy them. Additionally, misbehaving hooks may set tag keys
to be NULL. While this is not documented or officially supported, other
parts of the codebase detect NULL keys as an indication that the tag
should be stripped; detect this as well and avoid copying those tags
entirely in the newly-allocated batch message for consistency and to
avoid spurious crashes.
This crash is currently not exploitable because allocate_batch_message()
is only called for client-initiated batches and batches sent s2s, of
which there are currently none.
CLICAP_SERVONLY is a client capability that no clients will ever have
(it is not associated with any CAP string), but is included in the
server's client cap mask. Message tags with their capmask set to
CLICAP_SERVONLY will only ever be propagated s2s and will never be sent
or received c2s. This allows for the easy introduction of s2s-only
message tags.
Similarly, a new flag for client capabilities defined by modules was
introduced name CLICAP_FLAGS_NOPROP which indicates that message tags
having a capmask including such a capability MUST NOT be sent s2s.
The CLIENT_CAP_MASK macro in send.c was updated to omit capabilities
marked as CLICAP_FLAGS_NOPROP from what it will send s2s.
- new struct ResponseInfo that keeps track of all relevant state for a
labeled-response (whether local or remote)
- event to clean up pending remote labeled-responses that we haven't
gotten responses for (perhaps due to a bad network link with the
remote server, or it doesn't have labeled-response loaded)
- struct Batch grew a new field to track its labeled-response status
- struct Client grew a new linked list to track pending remote
labeled-responses for it, so we can abort them easily if the client
disconnects
- struct ListClient grew a new field to track labeled-response status
for a SAFELIST iteration
- remove arbitrary size limit on batch id generation; it will now
fill the passed-in buffer according to the passed-in buffer size,
leaving room for the null byte at the end.
NULL fields indicate that no responses are being labeled. The actual
population of those fields will happen in future patches; this one
simply provides the core support needed.
The new struct is ABI-compatible with hook_data for the first 3 fields
so that modules compiled using the old hook definition will continue to
work. The new data enables much richer evaluation during outbound_msgbuf
hook functions by exposing the target or channel the message is being
sent to (if known) as well as whether the message source is a recipient
of the message being sent.
This hook runs after parsing and processing a command, so that any
cleanup that needs to happen after we're finished with parsing can
happen. Some reorganization happened so that we ensure the hook and any
tags processing happens for every event that isn't a parse error.
This will be used by labeled-response in order to automatically send
ACKs or close labeled-response BATCHes after processing client commands.
Commit 6ba61b33f7ec557c02c1 split this logic up from the top
level Makefile.am but then failed to adjust the path to the
directory in the check
The result is that all builds now use a datecode of 0 and a
revision of "unknown"
A new flag ACT_BYPASS was added as a generic flag to bypass +u if set.
Opers remain immune, both as senders and recipients, so that spamfilter
cannot be abused to lock people out of managing filters, talking to
services, etc.
Three new configuration items were added to the general section of
ircd.conf to control filter behaviour:
- filter_sees_user_info: This replaces the old #defines for FILTER_NICK,
FILTER_IDENT, and FILTER_HOST to determine whether the hostmask of the
user is passed to the spamfilter or whether we pass a dummy *!*@*.
- filter_bypass_all: If set, the new ACT_BYPASS can allow every filter
action (DROP, KILL, ALARM) to bypass +u if set in conjunction with
those flags. If unset, only the DROP action can bypass +u and
ACT_BYPASS has no effect on other actions.
- filter_exit_message: This is the quit reason displayed, to move it out
of a #define (we still keep a define around for a default reason in
case it is unspecified).
If either filter_sees_user_info or filter_bypass_all are set, the
VERSION command will display a new 'F' flag to indicate the ability for
filter to spy on users.
When loading a module that modifies config (add/remove conf items or top
conf sections), we now queue a rehash operation so that new config can
be immediately applied. A new rb_defer_once() function was added to
deduplicate these rehash requests so that at most one rehash will occur
regardless of how many modules change conf.
This extension module provides a new usermode (+q) and two commands that
allow opers to set/unset that mode. The mode can be set either via
ircd.conf (general::default_umodes or auth::umodes) or via the
QUARANTINE command. When using the command, a reason is required which
will be broadcast to all other opers via the snote.
The quarantine will be automatically removed if the target opers up or
logs into a services account, and opers and logged in users cannot be
quarantined. The UNQUARNTINE command may additionally be used to remove
the quarantine status from a user. Users cannot otherwise set or remove
this usermode from themselves.
The QUARANTINE and UNQUARANTINE commands require a new priv
oper:quarantine.
To support this, a new account_change hook was added to m_services and
m_signon. This hook is fired whenever a user's services account changes
(via logging in or logging out) from the SVSLOGIN, SIGNON, LOGIN, or SU
services commands. It is **not** fired for SVSLOGIN on unknown
(pre-registration) users.
Documentation was updated for the new usermode, hook, and commands.
Usermode help was alphabetized for consistency, a missing entry for
helpops was added, and cloaking was moved to +x (since all modern
cloaking modules use that mode instead) since I was touching the file
anyway.
We validate that the reply looks like a valid message id as generated by
tag_message_id and, for channel messages, that the reply is going to the
same channel as specified in the message id. Since tag values do not
currently go through spamfilter, this is an easy-ish way to ensure that
it cannot be used as a backchannel for spam.
The sender must be in the channel mentioned by the tag and must be able
to send messages to that channel. Since this is a hint for clients to
display the received PM in a channel buffer, having the ircd strip the
tag when the sender would otherwise not be able to talk in the channel
will help reduce confusion and make chanop actions more effective in
reducing perceived in-channel issues.
- Initialize hooks before the main log file, because if we send a snote
due to failing to open a logfile, that calls a hook. Attempting to
call a hook before hooks are initialized will cause the ircd to
segfault.
- Skip access() check for the logfile itself and simply check the return
value of fopen(). This fixes a TOCTTOU race where an access check
succeeds but something causes the file to become inaccessible between
access() and fopen(), preventing an appropriate error from being
logged. Also makes my life easier with my custom SELinux policy for
the ircd since access() checks for full-write access whereas my policy
only allows append access for logfiles.
The ban.db file as well as both MOTD files were in hardcoded locations. Add new config options to the main solanum binary to manipulate where these files are looked for. This should make it fully possible to run multiple solanum instances from the same directory side-by-side.
Commit 9a1ee1b64ed660164197 introduced support for building IRCd
with Meson. It defines the "HAVE_OPENSSL" preprocessor macro
for both IRCd and librb. While librb does use that macro, IRCd
unfortunately uses only the "HAVE_LIBCRYPTO" macro instead, set
implicitly by AC_CHECK_LIB from autotools.
Make the autotools build system define HAVE_OPENSSL (to unify
IRCd with librb) and change IRCd to use that.
The existing code had several defects:
- Core module file names were hardcoded, while autoloaded non-
core module filenames were not. Code that loaded core modules
had a different but in some places duplicated structure to the
code that loaded autoloaded modules.
- Module loading for autoloaded modules happened in dentry
order, making it unpredictable from one server to the next, or
even across instances of `make install` on the same server.
- Module loading for autoloaded modules did not verify that the
dentry was a file before trying to load it.
- An IRCd start (or a conftest) only printed lines about loading
modules mentioned in the configuration file.
- The configuration file was parsed before loading core and
autoloaded modules, meaning any modules specified in the
configuration file would be loaded first.
- Once a module was loaded, it was added to the head of the
module list, making MODLIST show them in reverse loaded order
(newest (re)loaded modules at the top of the list).
These are addressed as follows:
- We now scan the core and autoload module directories and use
this to determine which modules to load.
- We now sort the dentries according to their filename before
iterating them to load them, making module load order finally
deterministic.
- We now test if a dentry is actually a file before trying to
load it.
- We now print a message to the console (if running in
foreground or conftest mode) when loading any module,
including core and autoloaded modules.
- The message printed to the console now has the IRCd module
directory prefix stripped from it if it matches.
- The configuration file is now parsed after loading core and
autoload modules.
- The modules are added to the module list in the order that
they are loaded. This now makes MODLIST show the order that
modules were (re)loaded in.
Modifications to two modules are part of this effort:
- The autoload module `m_alias` relied upon the alias dict not
being NULL when loaded, but this is only the case after the
configuration file has been parsed.
The module already had a "rehash" hook to destroy all existing
aliases and create new ones, so we can just make the module do
nothing on load (remove its modinit function) and change it to
use the "conf_read_end" hook instead of the "rehash" hook, and
it will still create aliases after the configuration file is
subsequently parsed.
With this modification, there are now no in-tree consumers of
the "rehash" hook.
- The autoload module `m_services` iterated `service_list` upon
loading it, which will now be empty (a no-op) since the module
is loaded before configuration file is parsed.
This module too handles the configuration file being parsed
with a "conf_read_end" hook and takes the appropriate action,
so we can just remove this code from its modinit function.
All other modules' modinit functions have been audited for
behaviour like this; there was nothing of note.
The macro does not use this parameter but nevertheless this is
the only module in the tree to pass a string to it, and if a
future macro were to use it, it could easily stringify the name
by using the # preprocessor operator.
This has been recently ratified and suppresses NAMES output during
channel joins when enabled, providing for a decent chunk of bandwidth
savings.
Failing to set outgoing_response_info to NULL after finishing parsing a
command means any future commands that do *not* have a label tag will
incorrectly inherit that labeled response context. This can cause, among
other things, crashes when sending an unlabeled remote command after a
labeled remote command.
Also add a regression test for this behaviour.
- Solve crash in free_response_batch by not attempting to remove NULL
batch ids from the dictionary (indicating there was no response batch
to actually free)
- Specify the correct server mask for the solanum.chat/response tag when
using hunt_server based on whether the hunted param is a client vs a
server
The TAP protocol doesn't allow extra stdout lines unless they're
commented (which these were not), and I don't see the value in printing
out every line we're munching, so instead use the new drain_client_sendq
function introduced in the previous commit to empty out the client's
sendq rather than these printf while loops.
- Add helper method to drain a client's sendq without asserting on any
of its lines
- Add helper method to fetch a dictionary by name, as these are
typically static linkage but could be useful for inspection or
manipulation in tests
- Add helper method to run an event by name, as these are typically
static linkage but could be useful to run during tests
- Make the cleanup_remote functions work properly by exiting the correct
client with the correct parameters
- Add clients created by text utilities to relevant tracking variables
- New helper methods to create clients with IDs
- Fix return value of get_client_sendq to always be a char * (some cases
previously returned const char *)
A new custom build target sets up the runtime dir the way the
testsuite expects it so that it can run properly. Also fix the help
script as it was trying to treat a file as a symlink when it wasn't
which broke the new build target. Tests now also properly use the
TAP protocol in the meson build file, and fix a segfault in the
send1 tests that passes NULL to a function that previously assumed
that NULL would not be passed.
The new documentation is organized differently to (hopefully) introduce
the TS6 server-to-server protocol in an easy to understand manner while
also serving as a useful reference for ircd and services developers who
wish to make use of this protocol to link to solanum.
Other documentation files were updated as well to correct discrepancies
noticed while writing the updated TS6 documentation.
The previous API was too error-prone, as evidenced by the large amount
of crash bugs discovered with it. New functions were added to response.h
to more easily manage labeled-response state, and some state tracking
was moved into core (such as CLICAP_RECEIVE_LABEL, which must only ever
be on one client at a time). Because this tracking is now in core,
cap_labeled_response needs to be a core module as having that bit
unloaded will cause issues. m_batch was also moved into being a core
module as more and more core things have depended on batch support
existing (and more will depend on it in the future).
m_list no longer re-uses remote response batches for SAFELIST, instead
making use of new APIs to suspend a labeled-response batch and more
easily resume it.
Batch resumption was made more focused as well, to ensure that we don't
accidentally lose batches due to setting outgoing_response_info to NULL.
The free_response_batch takes a resumption pointer to avoid cases where
we could introduce use-after-free by combining
free+resume_response_batch.
struct ResponseInfo now takes strduped pointers instead of directly
holding the buffers, and some fields were merged into a flags field or
omitted entirely due to new suspension/resumption support. This will
have a minor positive impact on memory usage as we no longer need to
allocate buffers for maximum-sized strings for every labeled-response.
Finally, shorten the dictionary label for pending responses. It was
overflowing its spot in STATS B, making the data after it misaligned.
We now disconnect the client if they attempt to run CAPAB before running
PASS (introducing themselves as a TS6 server with an SID) or if they've
already sent USER/NICK info. USER/NICK already has the other half of the
requisite checks by disconnecting the user if they have an SID. As such,
the logic flow of requiring an SID before CAPAB means users can never
have server caps set on them.
This prevents various issues such as a client sending CAPAB STAG
receiving all message tags, including those designed to be s2s-only.
This commit adds a missing send_multiline_reset call
before a return in clicap_generate in m_cap.c.
Without this, if running CAP LIST, the send_multiline_init call
just above it would not be cleaned up.
It assumes that send_multiline_reset is always safe to call,
which is true at the time of this commit.
free_response_batch will no-op if MyConnect(client_p) is false. This
would always be the case with the former timing, as it was called after
exit_local_client which clears the MyConnect flag. Do it before instead
so we can properly clean up the tracking vars (namely, the
pending_responses dict, which otherwise had a use after free when firing
the event to clean up a response for an exited client).
While we're at it, change the previous fix in cap_labeled_response from
IsIOError to IsAnyDead to catch more cases where a client cannot
receive data. This has not caused any crashes, but the expanded check
includes IOError so is safe to use, and all of the statuses checked by
IsAnyDead preclude the ability to send data to the client.
Compare pointer values and set to NULL before freeing the pointer. Also
set outgoing_response_info to NULL as well (this is pointless at the
moment since it will always be followed by a call that resets it to the
orig_response_info pointer, but future-proofs it in the event that
future code doesn't do that)
In the event that LIST output is small enough to complete without
invoking SAFELIST code (i.e. the buffer doesn't exceed half of the
client's SendQ), the stashed outgoing_response_info in struct ListClient
is the same pointer as the outgoing_response_info for the LIST command.
When we finish sending the LIST, we free the stashed struct ListClient's
outgoing_response_info and then restore the outgoing_response_info from
the LIST command, and attempt to do things with it. However, that memory
has already been freed. Detect the case when the pointers are the same
value and properly wipe them both so we no longer attempt to read from
it.
- Unconditionally require stdbool.h in IRCd
We are a C99 project anyway (configure.ac and meson.build both
require a C99 compiler), so this header will always be present
This removes the awful hack defining bool, true, and false in
its absense, which was creating build errors when using meson,
because meson.build did not set the HAVE_STDBOOL_H macro
- Modernise definition of alloca(3) according to GNU guidance
This was in both librb and IRCd, while it only needs to be in
the former
- Fix missing endianness check in librb/meson.build
IRCd and librb both use the WORDS_BIGENDIAN macro, which was
only being set by their configure.ac (with the AC_C_BIGENDIAN
M4 macro)
Since IRCd's stdinc.h includes librb's rb_lib.h, which then
includes librb's librb-config.h, which would have the macro
defined there, we only need to check for it in librb, so we
can remove the endianness check from IRCd's configure.ac and
don't need to add it to IRCd's meson.build
- Use __has_attribute to detect whether compiler attributes are
available
This is supported by gcc (5+), clang (2.9+), and ICC (17+)
- Add a __noreturn macro for __attribute__((__noreturn__)) and
adjust the codebase to use that
- Remove inclusion of headers that are only used by librb
- Remove inclusion of headers that are included in rb_lib.h
because stdinc.h includes rb_lib.h anyway
- Remove the checks for headers and functions from the IRCd
configure.ac and meson.build that are only used by librb
- Alphabetise checks for headers and functions (by the name of
the macro they define) in the meson.build files
- Use check_header() instead of has_header() in meson to see if
it is actually possible to compile code using the header
- Remove dead code using srand48(3) which has never been
compiled because neither build system has ever set the
HAVE_SRAND48 macro and the surrounding code makes no use of
this libc RNG
- Remove the TLS defines from IRCd's meson.build because librb's
meson.build already defines them and IRCd pulls those in
- The hurt extension doesn't work well and is now supplanted by
m_quarantine
- ip_cloaking_old has been broken (causes segfaults) for years now,
remove it
- ip_cloaking_3.0 has hardcoded keys that require .c file edits to be
secure. Anyone using it will probably maintain their fork of this file
anyway, but it's not good for new people to use. Remove that too
- Remove LOCOPS and umode +l entirely; they serve no useful purpose on
unified networks. This also removes the no_locops extension since it
is now useless
Previously, batches would end prematurely if they traveled through an
intermediate server before reaching their destination because the
intermediate server would send ENCAP ACK multiple times, causing the
pending server count to go down way too fast. Solve this by introducing
two things:
- Check that the direction of the incoming command matches the direction
we'd be sending the ACK to (avoids sending additional ACKs for replies
from remote servers that we pass along)
- Add a mask parameter to the tracking metadata for remote responses and
only send an ACK if we match the mask. this way things like remote
WHOIS don't generate ACKs from intermediate servers
Additionally, fix our manipulation of outgoing_response_info in m_list
to properly set CLICAP_RECEIVE_LABEL during SAFELIST buffering. The
logic here largely matches that in m_batch.
While base cases already worked fine with just cap_labeled_response, a
handful of special cases need batched responses, such as multi-target
messages. The module also needed updating to work properly with
echo-message, as the spec says that the echo is the one that needs the
label if the client messages themselves, rather than the main message,
and the echo is always sent last. This logic is also applied to mass
messages that include the oper sending it.
The handle_special method was broken up and made less special, treating
the various cases as additional target types rather than processing them
right away. The opers@server hack was also removed entirely since it is
obviated by other means of messaging opers on a server such as LOCOPS in
the event such is even necessary. Or, you know, just use the staff
channel.
This concerns client-initiated batches only. Save the labeled-response
context as part of the Batch so that we can restore that context later
once the batch is complete. The label is added to the batch start
message, but we only process batches once we receive the batch complete
message (which lacks a label tag), and we could potentially be finishing
a batch due to it timing out if the client never sends a batch complete
message. By restoring context, we ensure that hooks such as
outbound_msgbuf are operating with all of the state they would expect to
have in order to properly attach tags to outgoing messages.
Add calls to generate local or remote labeled-response batches to all
modules which generate multiple lines of response. In general, this is
only done once we've confirmed we will be sending multiple lines, to
avoid spurious batches which only contain single lines inside of the
batch, however some cases will always generate a batch regardless due to
ease of implementation.
This patch covers all of the "easy" cases; future patches will cover the
complicated ones: m_batch, m_message, and m_list.
This module provides the labeled-response capability as well as an
internal non-propagated ?receive_label capability that is dynamically
added/removed from the client which should receive a labeled-response.
By using a second dummy capability, I don't need to rewrite our msgbuf
caching layer which is keyed entirely off the capmasks of the client in
order to vary who receives the label/batch tags.
It also introduces a s2s-only message tag solanum.chat/response which is
used to inform remote servers that their replies should be labelled.
Remote responses are always batched, and the batch is generated by the
local server to ensure that 100% of incoming labelled commands receive a
labeled-response. The solanum.chat/response tag is passed back in
replies so the local server can add the appropriate batch tag to the
response passed to the client. Remote servers will also send an ENCAP
ACK command to indicate they are done responding in the event that they
received an incoming s2s command with the solanum.chat/response tag.
The module keeps track of outgoing responses and if, by the end of
command processing (parse_end hook), it hasn't attached a label to
anything it will send a labelled ACK to the client. If it has begun a
labeled-response batch, it will also automatically close that batch,
reducing the likelihood of programming errors forgetting to close
batches.
When allocate_batch_message is called on a batch containing valueless
tags (i.e. tag value is NULL), the ircd would core because we attempted
to call strlen on a NULL pointer. Properly detect NULL values and don't
attempt to copy them. Additionally, misbehaving hooks may set tag keys
to be NULL. While this is not documented or officially supported, other
parts of the codebase detect NULL keys as an indication that the tag
should be stripped; detect this as well and avoid copying those tags
entirely in the newly-allocated batch message for consistency and to
avoid spurious crashes.
This crash is currently not exploitable because allocate_batch_message()
is only called for client-initiated batches and batches sent s2s, of
which there are currently none.
CLICAP_SERVONLY is a client capability that no clients will ever have
(it is not associated with any CAP string), but is included in the
server's client cap mask. Message tags with their capmask set to
CLICAP_SERVONLY will only ever be propagated s2s and will never be sent
or received c2s. This allows for the easy introduction of s2s-only
message tags.
Similarly, a new flag for client capabilities defined by modules was
introduced name CLICAP_FLAGS_NOPROP which indicates that message tags
having a capmask including such a capability MUST NOT be sent s2s.
The CLIENT_CAP_MASK macro in send.c was updated to omit capabilities
marked as CLICAP_FLAGS_NOPROP from what it will send s2s.
- new struct ResponseInfo that keeps track of all relevant state for a
labeled-response (whether local or remote)
- event to clean up pending remote labeled-responses that we haven't
gotten responses for (perhaps due to a bad network link with the
remote server, or it doesn't have labeled-response loaded)
- struct Batch grew a new field to track its labeled-response status
- struct Client grew a new linked list to track pending remote
labeled-responses for it, so we can abort them easily if the client
disconnects
- struct ListClient grew a new field to track labeled-response status
for a SAFELIST iteration
- remove arbitrary size limit on batch id generation; it will now
fill the passed-in buffer according to the passed-in buffer size,
leaving room for the null byte at the end.
NULL fields indicate that no responses are being labeled. The actual
population of those fields will happen in future patches; this one
simply provides the core support needed.
The new struct is ABI-compatible with hook_data for the first 3 fields
so that modules compiled using the old hook definition will continue to
work. The new data enables much richer evaluation during outbound_msgbuf
hook functions by exposing the target or channel the message is being
sent to (if known) as well as whether the message source is a recipient
of the message being sent.
This hook runs after parsing and processing a command, so that any
cleanup that needs to happen after we're finished with parsing can
happen. Some reorganization happened so that we ensure the hook and any
tags processing happens for every event that isn't a parse error.
This will be used by labeled-response in order to automatically send
ACKs or close labeled-response BATCHes after processing client commands.
A new flag ACT_BYPASS was added as a generic flag to bypass +u if set.
Opers remain immune, both as senders and recipients, so that spamfilter
cannot be abused to lock people out of managing filters, talking to
services, etc.
Three new configuration items were added to the general section of
ircd.conf to control filter behaviour:
- filter_sees_user_info: This replaces the old #defines for FILTER_NICK,
FILTER_IDENT, and FILTER_HOST to determine whether the hostmask of the
user is passed to the spamfilter or whether we pass a dummy *!*@*.
- filter_bypass_all: If set, the new ACT_BYPASS can allow every filter
action (DROP, KILL, ALARM) to bypass +u if set in conjunction with
those flags. If unset, only the DROP action can bypass +u and
ACT_BYPASS has no effect on other actions.
- filter_exit_message: This is the quit reason displayed, to move it out
of a #define (we still keep a define around for a default reason in
case it is unspecified).
If either filter_sees_user_info or filter_bypass_all are set, the
VERSION command will display a new 'F' flag to indicate the ability for
filter to spy on users.
When loading a module that modifies config (add/remove conf items or top
conf sections), we now queue a rehash operation so that new config can
be immediately applied. A new rb_defer_once() function was added to
deduplicate these rehash requests so that at most one rehash will occur
regardless of how many modules change conf.
This extension module provides a new usermode (+q) and two commands that
allow opers to set/unset that mode. The mode can be set either via
ircd.conf (general::default_umodes or auth::umodes) or via the
QUARANTINE command. When using the command, a reason is required which
will be broadcast to all other opers via the snote.
The quarantine will be automatically removed if the target opers up or
logs into a services account, and opers and logged in users cannot be
quarantined. The UNQUARNTINE command may additionally be used to remove
the quarantine status from a user. Users cannot otherwise set or remove
this usermode from themselves.
The QUARANTINE and UNQUARANTINE commands require a new priv
oper:quarantine.
To support this, a new account_change hook was added to m_services and
m_signon. This hook is fired whenever a user's services account changes
(via logging in or logging out) from the SVSLOGIN, SIGNON, LOGIN, or SU
services commands. It is **not** fired for SVSLOGIN on unknown
(pre-registration) users.
Documentation was updated for the new usermode, hook, and commands.
Usermode help was alphabetized for consistency, a missing entry for
helpops was added, and cloaking was moved to +x (since all modern
cloaking modules use that mode instead) since I was touching the file
anyway.
We validate that the reply looks like a valid message id as generated by
tag_message_id and, for channel messages, that the reply is going to the
same channel as specified in the message id. Since tag values do not
currently go through spamfilter, this is an easy-ish way to ensure that
it cannot be used as a backchannel for spam.
The sender must be in the channel mentioned by the tag and must be able
to send messages to that channel. Since this is a hint for clients to
display the received PM in a channel buffer, having the ircd strip the
tag when the sender would otherwise not be able to talk in the channel
will help reduce confusion and make chanop actions more effective in
reducing perceived in-channel issues.
- Initialize hooks before the main log file, because if we send a snote
due to failing to open a logfile, that calls a hook. Attempting to
call a hook before hooks are initialized will cause the ircd to
segfault.
- Skip access() check for the logfile itself and simply check the return
value of fopen(). This fixes a TOCTTOU race where an access check
succeeds but something causes the file to become inaccessible between
access() and fopen(), preventing an appropriate error from being
logged. Also makes my life easier with my custom SELinux policy for
the ircd since access() checks for full-write access whereas my policy
only allows append access for logfiles.
Commit 9a1ee1b64ed660164197 introduced support for building IRCd
with Meson. It defines the "HAVE_OPENSSL" preprocessor macro
for both IRCd and librb. While librb does use that macro, IRCd
unfortunately uses only the "HAVE_LIBCRYPTO" macro instead, set
implicitly by AC_CHECK_LIB from autotools.
Make the autotools build system define HAVE_OPENSSL (to unify
IRCd with librb) and change IRCd to use that.
The existing code had several defects:
- Core module file names were hardcoded, while autoloaded non-
core module filenames were not. Code that loaded core modules
had a different but in some places duplicated structure to the
code that loaded autoloaded modules.
- Module loading for autoloaded modules happened in dentry
order, making it unpredictable from one server to the next, or
even across instances of `make install` on the same server.
- Module loading for autoloaded modules did not verify that the
dentry was a file before trying to load it.
- An IRCd start (or a conftest) only printed lines about loading
modules mentioned in the configuration file.
- The configuration file was parsed before loading core and
autoloaded modules, meaning any modules specified in the
configuration file would be loaded first.
- Once a module was loaded, it was added to the head of the
module list, making MODLIST show them in reverse loaded order
(newest (re)loaded modules at the top of the list).
These are addressed as follows:
- We now scan the core and autoload module directories and use
this to determine which modules to load.
- We now sort the dentries according to their filename before
iterating them to load them, making module load order finally
deterministic.
- We now test if a dentry is actually a file before trying to
load it.
- We now print a message to the console (if running in
foreground or conftest mode) when loading any module,
including core and autoloaded modules.
- The message printed to the console now has the IRCd module
directory prefix stripped from it if it matches.
- The configuration file is now parsed after loading core and
autoload modules.
- The modules are added to the module list in the order that
they are loaded. This now makes MODLIST show the order that
modules were (re)loaded in.
Modifications to two modules are part of this effort:
- The autoload module `m_alias` relied upon the alias dict not
being NULL when loaded, but this is only the case after the
configuration file has been parsed.
The module already had a "rehash" hook to destroy all existing
aliases and create new ones, so we can just make the module do
nothing on load (remove its modinit function) and change it to
use the "conf_read_end" hook instead of the "rehash" hook, and
it will still create aliases after the configuration file is
subsequently parsed.
With this modification, there are now no in-tree consumers of
the "rehash" hook.
- The autoload module `m_services` iterated `service_list` upon
loading it, which will now be empty (a no-op) since the module
is loaded before configuration file is parsed.
This module too handles the configuration file being parsed
with a "conf_read_end" hook and takes the appropriate action,
so we can just remove this code from its modinit function.
All other modules' modinit functions have been audited for
behaviour like this; there was nothing of note.