diff --git a/include/numnicks.h b/include/numnicks.h index c6e42699..a6f44c4e 100644 --- a/include/numnicks.h +++ b/include/numnicks.h @@ -79,6 +79,7 @@ extern void SetYXXServerName(struct Client* myself, unsigned int numeric); extern int markMatchexServer(const char* cmask, int minlen); extern struct Client* find_match_server(char* mask); +extern struct Client* find_match_server_next(const char* mask, unsigned int* iter); extern struct Client* findNUser(const char* yxx); extern struct Client* FindNServer(const char* numeric); diff --git a/include/sasl.h b/include/sasl.h index 3938b4ad..e93e41c9 100644 --- a/include/sasl.h +++ b/include/sasl.h @@ -30,6 +30,7 @@ struct StatDesc; /* Public SASL functions */ extern void sasl_init(void); extern int sasl_available(void); +extern struct Client* sasl_server(void); extern int sasl_mechanism_supported(const char* mechanism); extern void sasl_check_capability(void); extern void sasl_send_xreply(struct Client* sptr, const char* routing, const char* reply); diff --git a/ircd/m_cap.c b/ircd/m_cap.c index 91cd7e09..5b6d2cf7 100644 --- a/ircd/m_cap.c +++ b/ircd/m_cap.c @@ -396,7 +396,7 @@ void cap_new(enum Capab cap) return; /* Iterate through all local clients */ - for (i = 0; i < HighestFd; i++) { + for (i = 0; i <= HighestFd; i++) { if (!(acptr = LocalClientArray[i])) continue; @@ -406,9 +406,9 @@ void cap_new(enum Capab cap) /* Send CAP NEW message */ if (cap_value && *cap_value && HasFlag(acptr, FLAG_CAP302)) { - sendcmdto_one(&me, CMD_CAP, acptr, "%C NEW %s=%s", acptr, cap_name, cap_value); + sendcmdto_one(&me, CMD_CAP, acptr, "%C NEW :%s=%s", acptr, cap_name, cap_value); } else { - sendcmdto_one(&me, CMD_CAP, acptr, "%C NEW %s", acptr, cap_name); + sendcmdto_one(&me, CMD_CAP, acptr, "%C NEW :%s", acptr, cap_name); } } } @@ -438,7 +438,7 @@ void cap_del(enum Capab cap) } /* Iterate through all local clients */ - for (i = 0; i < HighestFd; i++) { + for (i = 0; i <= HighestFd; i++) { if (!(acptr = LocalClientArray[i])) continue; diff --git a/ircd/m_endburst.c b/ircd/m_endburst.c index 241b6790..770a9855 100644 --- a/ircd/m_endburst.c +++ b/ircd/m_endburst.c @@ -125,6 +125,10 @@ int ms_end_of_burst(struct Client* cptr, struct Client* sptr, int parc, char* pa if (MyConnect(sptr)) sendcmdto_one(&me, CMD_END_OF_BURST_ACK, sptr, ""); + /* The SASL server may have become reachable now that this link (or a + * link on the path to it) has completed its burst. */ + sasl_check_capability(); + /* Count through channels... */ for (chan = GlobalChannelList; chan; chan = next_chan) { next_chan = chan->next; @@ -156,7 +160,6 @@ int ms_end_of_burst_ack(struct Client *cptr, struct Client *sptr, int parc, char sptr); sendcmdto_serv_butone(sptr, CMD_END_OF_BURST_ACK, cptr, ""); ClearBurstAck(sptr); - sasl_check_capability(); return 0; } diff --git a/ircd/m_sasl.c b/ircd/m_sasl.c index 4bf054da..b3354729 100644 --- a/ircd/m_sasl.c +++ b/ircd/m_sasl.c @@ -166,8 +166,7 @@ int m_sasl(struct Client* cptr, struct Client* sptr, int parc, char* parv[]) if (HasFlag(sptr, FLAG_SASL) || HasFlag(sptr, FLAG_ACCOUNT)) return send_reply(cptr, ERR_SASLALREADY); - acptr = find_match_server((char*)netconf_str(NETCONF_SASL_SERVER)); - if (!sasl_available() || !acptr) + if (!(acptr = sasl_server())) return send_reply(cptr, ERR_SASLFAIL, "The login server is currently disconnected. Please excuse the inconvenience."); if (strlen(parv[1]) > 400) diff --git a/ircd/m_server.c b/ircd/m_server.c index c2c469c3..bd5bcd61 100644 --- a/ircd/m_server.c +++ b/ircd/m_server.c @@ -49,6 +49,7 @@ #include "s_serv.h" #include "send.h" #include "userload.h" +#include "sasl.h" /* #include -- Now using assert in ircd_log.h */ #include @@ -808,5 +809,12 @@ int ms_server(struct Client* cptr, struct Client* sptr, int parc, char* parv[]) compute_secure_path_groups(); + /* A server matching sasl.server introduced as already past its burst + * (P) over an established link never sends END_OF_BURST, so this is + * the only chance to notice it. For a server introduced as bursting + * (J), or behind a bursting hop, sasl_server() still reports it + * unusable and the END_OF_BURST handler picks it up later. */ + sasl_check_capability(); + return 0; } diff --git a/ircd/numnicks.c b/ircd/numnicks.c index 01dc2ee7..da2611c3 100644 --- a/ircd/numnicks.c +++ b/ircd/numnicks.c @@ -414,16 +414,34 @@ int markMatchexServer(const char *cmask, int minlen) * @return Matching server with lowest numnick value (or NULL). */ struct Client* find_match_server(char *mask) +{ + unsigned int iter = 0; + + if (BadPtr(mask)) + return 0; + collapse(mask); + return find_match_server_next(mask, &iter); +} + +/** Find the next server whose name matches the given mask. + * Scans server_list[] in numnick order starting at *iter, which is + * advanced past the returned server so repeated calls enumerate every + * match. Unlike find_match_server(), the mask is not collapse()d and + * not modified. + * @param[in] mask %Server name mask (already collapse()d). + * @param[in,out] iter Scan position; start at 0. + * @return Next matching server, or NULL when exhausted. + */ +struct Client* find_match_server_next(const char *mask, unsigned int *iter) { struct Client *acptr; - int i; - if (!(BadPtr(mask))) { - collapse(mask); - for (i = 0; i < lastNNServer; i++) { - if ((acptr = server_list[i]) && (!match(mask, cli_name(acptr)))) - return acptr; - } + if (BadPtr(mask)) + return 0; + while (*iter < lastNNServer) { + acptr = server_list[(*iter)++]; + if (acptr && !match(mask, cli_name(acptr))) + return acptr; } return 0; } diff --git a/ircd/s_misc.c b/ircd/s_misc.c index d71fb5d7..a82da73e 100644 --- a/ircd/s_misc.c +++ b/ircd/s_misc.c @@ -266,8 +266,6 @@ static void exit_one_client(struct Client* bcptr, const char* comment) Count_serverdisconnects(UserStats); else Count_remoteserverquits(UserStats); - - sasl_check_capability(); } else if (IsMe(bcptr)) { @@ -517,8 +515,14 @@ int exit_client(struct Client *cptr, exit_downlinks(victim, killer, comment1); exit_one_client(victim, comment); - if (was_server) + if (was_server) { compute_secure_path_groups(); + /* Re-evaluate SASL availability once, now that the whole subtree is + * gone. Doing it per server inside exit_one_client() could see the + * SASL server still linked while a sibling was being removed and emit + * a spurious CAP NEW right before the CAP DEL. */ + sasl_check_capability(); + } /* * cptr can only have been killed if it was cptr itself that got killed here, diff --git a/ircd/sasl.c b/ircd/sasl.c index b50689e4..de5a2c3c 100644 --- a/ircd/sasl.c +++ b/ircd/sasl.c @@ -29,6 +29,7 @@ #include "ircd_events.h" #include "ircd_log.h" #include "ircd_string.h" +#include "match.h" #include "ircd_reply.h" #include "ircd_netconf.h" #include "send.h" @@ -71,17 +72,65 @@ static struct SaslSessionEntry* sasl_session_table[SASL_HASH_SIZE]; /** Global SASL statistics */ static struct SaslStats sasl_statistics = { 0, 0 }; -/** Check if SASL is available - * @return 1 if SASL server is configured, 0 otherwise +/** Check whether any server on the path from us to \a acptr is bursting. + * @param[in] acptr Server to test. + * @return 1 if \a acptr or one of its uplinks is still bursting. */ -int sasl_available(void) +static int sasl_path_bursting(struct Client* acptr) +{ + for (; acptr && !IsMe(acptr); acptr = cli_serv(acptr)->up) { + if (IsBurst(acptr)) + return 1; + } + return 0; +} + +/** Find the SASL server to use. + * + * A usable SASL server exists when a SASL server mask and a mechanism + * list are configured and some linked server matches the mask with no + * bursting server on the path between us and it. A half-completed link + * may already have introduced a matching server, but it is neither + * advertised nor routed to until END_OF_BURST has been received from + * every hop on the way. With a wildcard mask, every match is considered + * and the first (lowest numnick) fully linked one wins, so a matching + * server that is re-linking does not mask an established one. + * + * This is the single source of truth: sasl_available() and the + * AUTHENTICATE routing in m_sasl() both use it, so the server validated + * here is the one requests are sent to. + * @return The SASL server, or NULL if none is usable. + */ +struct Client* sasl_server(void) { + char mask[HOSTLEN + 1]; + struct Client* acptr; + unsigned int iter = 0; + if (!*netconf_str(NETCONF_SASL_SERVER) - || !*netconf_str(NETCONF_SASL_MECHANISMS) - || !find_match_server((char*)netconf_str(NETCONF_SASL_SERVER))) - return 0; + || !*netconf_str(NETCONF_SASL_MECHANISMS)) + return NULL; + + /* Work on a copy: find_match_server() would collapse() the netconf + * value in place. */ + ircd_strncpy(mask, netconf_str(NETCONF_SASL_SERVER), HOSTLEN); + mask[HOSTLEN] = '\0'; + collapse(mask); + + while ((acptr = find_match_server_next(mask, &iter))) { + if (!sasl_path_bursting(acptr)) + return acptr; + } + return NULL; +} - return 1; +/** Check if SASL is available + * @return 1 if a usable SASL server is linked, 0 otherwise + * @see sasl_server() + */ +int sasl_available(void) +{ + return sasl_server() != NULL; } /** Check if a mechanism exists in a mechanism list diff --git a/tests/p10_server.py b/tests/p10_server.py index 7814a458..d7384aa3 100644 --- a/tests/p10_server.py +++ b/tests/p10_server.py @@ -237,6 +237,21 @@ async def handshake(self, timeout: float = 15.0): Sends PASS + SERVER, reads the hub's PASS + SERVER + burst, sends our EB, waits for EA, sends EA. """ + deadline = asyncio.get_event_loop().time() + timeout + await self.begin_handshake(timeout=timeout) + await self.send_end_of_burst() + remaining = deadline - asyncio.get_event_loop().time() + await self.complete_handshake(timeout=remaining) + + async def begin_handshake(self, timeout: float = 15.0): + """Send PASS + SERVER and read the hub's burst up to its EB. + + Leaves the link in the "still bursting" state from the hub's point + of view: we have not sent our own EB yet. Tests that need a + half-linked server (e.g. to simulate a link that dies mid-burst) + stop here; otherwise follow with send_end_of_burst() and + complete_handshake(). + """ now = int(time.time()) # Send our credentials @@ -259,10 +274,13 @@ async def handshake(self, timeout: float = 15.0): if tok == "EB" or line == "EB": break - # Send our (empty) burst + end of burst + async def send_end_of_burst(self): + """Send our EB, marking the end of our (possibly empty) burst.""" await self._send(f"{self._num} EB") - # Wait for EA (end of burst ack) + async def complete_handshake(self, timeout: float = 15.0): + """Wait for the hub's EA and answer with our own EA.""" + deadline = asyncio.get_event_loop().time() + timeout while True: remaining = deadline - asyncio.get_event_loop().time() if remaining <= 0: @@ -329,21 +347,34 @@ async def send_downstream_server( flags: str = "", description: str = "Downstream test server", timestamp: int | None = None, + bursting: bool = True, ) -> str: """Introduce a remote server behind this link. + ``bursting`` selects the protocol field: ``J10`` (default) tells the + hub the server is still bursting -- it stays flagged as such until + an EB arrives from *that* server's numeric (see send_end_of_burst_for). + ``P10`` introduces a server whose burst already completed, which is + how an uplink re-introduces its existing downlinks during its own + burst. + Returns the 2-character P10 server numeric for the new server. """ ts = timestamp or _next_timestamp() down_num = server_numeric(numeric) down_mask = down_num + int_to_b64(self.max_clients, 3) flag_field = f"+{flags}" if flags else "+" + proto = "J10" if bursting else "P10" await self._send( - f"{self._num} SERVER {name} {hop} 0 {ts} J10 {down_mask} " + f"{self._num} SERVER {name} {hop} 0 {ts} {proto} {down_mask} " f"{flag_field} :{description}" ) return down_num + async def send_end_of_burst_for(self, server_numeric_prefix: str): + """Send EB on behalf of a downstream server introduced with J10.""" + await self._send(f"{server_numeric_prefix} EB") + async def send_downstream_nick( self, server_numeric_prefix: str, diff --git a/tests/pr66_capsasl/test_sasl_cap_burst_split.py b/tests/pr66_capsasl/test_sasl_cap_burst_split.py new file mode 100644 index 00000000..f1881079 --- /dev/null +++ b/tests/pr66_capsasl/test_sasl_cap_burst_split.py @@ -0,0 +1,322 @@ +"""SASL capability advertisement across a bursting link and a netsplit. + +Reproduces a production incident: the SASL server (``channels.*``) lived +behind a hub (``shub``) that linked half-way -- it introduced its downlinks +but never completed its burst -- and then pinged out. Clients with +cap-notify saw:: + + CAP hubby NEW sasl=plain,scram-sha-256,external + CAP hubby DEL :sasl + +on the *split*. Two defects combined to produce that: + +* SASL availability was only re-evaluated on END_OF_BURST_ACK, so a + half-linked SASL server was (correctly) never advertised, but the + capability flag stayed stale relative to ``find_match_server()``. +* ``exit_one_client()`` re-evaluated availability for every server torn + down in the split. A sibling of the SASL server exited first, the check + still found the SASL server linked, and emitted a spurious CAP NEW one + message before the real CAP DEL. + +Topology in these tests (fake ``services.test.net`` plays ``shub``):: + + hub.test.net --- services.test.net --+-- channels.test.net (sasl.server) + (fake, P10Server) +-- other.test.net + +``other`` is introduced *after* ``channels`` so it sits at the head of the +uplink's downlink list and is torn down first on a split -- the ordering +that triggered the spurious NEW. +""" + +import asyncio + +import pytest + +from irc_client import IRCClient +from p10_server import P10Server + + +pytestmark = pytest.mark.single_server + +SASL_SERVER = "channels.test.net" +MECHANISMS = "PLAIN" + +# (name, numeric, flags, bursting) -- the production shape: an uplink +# re-introducing downlinks whose own bursts completed long ago (P10). +DEFAULT_DOWNSTREAMS = ( + (SASL_SERVER, 5, "s", False), + ("other.test.net", 6, "", False), +) + + +async def _capnotify_client(ircd_hub, nick: str) -> IRCClient: + """Registered client with cap-notify active (implicit via CAP LS 302). + + Not cap_helpers.make_cap_client(): cap-notify is CAPFL_HIDDEN_302, so + that helper would skip these tests. + """ + client = IRCClient() + await client.connect(ircd_hub["host"], ircd_hub["port"]) + await client.send("CAP LS 302") + await client.wait_for("CAP", timeout=5.0) + await client.send("CAP END") + await client.register(nick, "testuser", "Test User") + return client + + +async def _collect_cap(client: IRCClient, timeout: float) -> list[tuple[str, str]]: + """Return every (subcommand, argument) CAP message seen within timeout.""" + seen = [] + loop = asyncio.get_running_loop() + deadline = loop.time() + timeout + while True: + remaining = deadline - loop.time() + if remaining <= 0: + return seen + try: + msg = await client.wait_for("CAP", timeout=remaining) + except asyncio.TimeoutError: + return seen + seen.append((msg.params[1], msg.params[-1])) + + +async def _expect_cap_new(client: IRCClient) -> None: + msg = await client.wait_for("CAP", timeout=5.0) + assert msg.params[1] == "NEW", f"expected CAP NEW, got {msg.params}" + assert msg.params[-1] == f"sasl={MECHANISMS}", msg.params + + +async def _half_link_with_sasl_server( + ircd_hub, + *, + sasl_server: str = SASL_SERVER, + downstreams=DEFAULT_DOWNSTREAMS, +) -> tuple[P10Server, dict[str, str]]: + """Link a fake hub that has not finished its burst, pointing SASL at ``sasl_server``. + + Sets sasl.server/sasl.mechanisms via CF while still bursting, then + introduces ``downstreams`` behind us. Returns the server and a map of + downstream name -> P10 numeric. + """ + srv = P10Server(name="services.test.net", numeric=4, password="testpass") + await srv.connect(ircd_hub["host"], ircd_hub["server_port"]) + await srv.begin_handshake() + # Mid-burst from the hub's point of view: it has sent us EB, we have + # not sent ours. + await srv.send_config("sasl.server", sasl_server) + await srv.send_config("sasl.mechanisms", MECHANISMS) + numerics = {} + for name, numeric, flags, bursting in downstreams: + numerics[name] = await srv.send_downstream_server( + name, numeric, flags=flags, bursting=bursting + ) + return srv, numerics + + +async def _authenticate_target(ircd_hub, srv: P10Server, nick: str) -> str: + """Start SASL from a fresh client; return the numeric AUTHENTICATE was routed to.""" + client = IRCClient() + await client.connect(ircd_hub["host"], ircd_hub["port"]) + try: + await client.send("CAP LS 302") + msg = await client.wait_for("CAP", timeout=5.0) + assert "sasl" in msg.params[-1], "hub does not advertise sasl" + await client.send("CAP REQ :sasl") + msg = await client.wait_for("CAP", timeout=5.0) + assert msg.params[1] == "ACK", f"expected CAP ACK, got {msg.params}" + await client.send(f"AUTHENTICATE {MECHANISMS}") + # " XQ sasl: :SASL ..." + line = await srv.wait_for_token("XQ", timeout=5.0) + return line.split()[2] + finally: + await client.send("AUTHENTICATE *") + await client.send("QUIT :done") + await client.disconnect() + + +async def test_no_cap_new_when_half_linked_sasl_server_splits(ircd_hub): + """A link that dies mid-burst must not produce CAP NEW (nor DEL) for sasl. + + The SASL server was never advertised, so there is nothing to withdraw + and certainly nothing to announce. + """ + client = await _capnotify_client(ircd_hub, "capsplit1") + try: + srv, _ = await _half_link_with_sasl_server(ircd_hub) + + # Still bursting: nothing may be advertised yet. + assert await _collect_cap(client, 1.5) == [] + + # Link collapses without ever sending EB (ping timeout in production). + await srv.disconnect() + + assert await _collect_cap(client, 2.0) == [], ( + "sasl must not be announced/withdrawn for a server that never " + "finished bursting" + ) + finally: + await client.send("QUIT :done") + await client.disconnect() + + +async def test_cap_new_on_end_of_burst_and_single_del_on_split(ircd_hub): + """sasl is announced when the link finishes bursting, withdrawn once on split.""" + client = await _capnotify_client(ircd_hub, "capsplit2") + try: + srv, _ = await _half_link_with_sasl_server(ircd_hub) + assert await _collect_cap(client, 1.5) == [] + + # Our EB completes the path to the SASL server. Advertise now -- + # without waiting for the EA exchange, which a services package + # may never send. + await srv.send_end_of_burst() + await _expect_cap_new(client) + + await srv.complete_handshake() + # The EA exchange must not re-announce anything. + assert await _collect_cap(client, 1.0) == [] + + # Now the whole subtree goes away. Exactly one DEL, no NEW. + await srv.disconnect() + caps = await _collect_cap(client, 2.0) + assert caps == [("DEL", "sasl")], caps + finally: + await client.send("QUIT :done") + await client.disconnect() + + +async def test_cap_new_waits_for_sasl_server_own_end_of_burst(ircd_hub): + """A SASL server introduced with J10 is not advertised until *its* EB. + + Every hop on the path to the SASL server must be out of burst, not just + the direct link. + """ + client = await _capnotify_client(ircd_hub, "capsplit3") + try: + srv, nums = await _half_link_with_sasl_server( + ircd_hub, downstreams=((SASL_SERVER, 5, "s", True),) + ) + + # Our link finishes bursting, but channels.* is still in burst. + await srv.send_end_of_burst() + await srv.complete_handshake() + assert await _collect_cap(client, 1.5) == [], ( + "sasl advertised while the SASL server itself is still bursting" + ) + + await srv.send_end_of_burst_for(nums[SASL_SERVER]) + await _expect_cap_new(client) + + await srv.disconnect() + caps = await _collect_cap(client, 2.0) + assert caps == [("DEL", "sasl")], caps + finally: + await client.send("QUIT :done") + await client.disconnect() + + +async def test_directly_linked_sasl_server_available_at_end_of_burst(ircd_hub): + """SASL server that is our direct link: hidden while bursting, NEW at its EB. + + sasl.server is set (via CF during the burst) before the link completes, + so the transition must come from END_OF_BURST itself, not from the + netconf change callback. + """ + client = await _capnotify_client(ircd_hub, "capsplit4") + try: + srv, _ = await _half_link_with_sasl_server( + ircd_hub, sasl_server="services.test.net", downstreams=() + ) + + # Direct link still bursting: config points at us, but no NEW yet. + assert await _collect_cap(client, 1.5) == [], ( + "sasl advertised while the directly linked SASL server is bursting" + ) + + await srv.send_end_of_burst() + await _expect_cap_new(client) + + await srv.complete_handshake() + assert await _collect_cap(client, 1.0) == [] + + await srv.disconnect() + caps = await _collect_cap(client, 2.0) + assert caps == [("DEL", "sasl")], caps + finally: + await client.send("QUIT :done") + await client.disconnect() + + +async def test_cap_new_when_sasl_server_introduced_past_burst_over_established_link(ircd_hub): + """A P10 (already past burst) SASL server behind an established link is advertised at once. + + Nothing else fires for it: no EB will ever arrive for a server + introduced as past its burst, so introduction itself must trigger the + re-check. Services packages introducing virtual servers do exactly this. + """ + client = await _capnotify_client(ircd_hub, "capsplit5") + try: + srv, _ = await _half_link_with_sasl_server(ircd_hub, downstreams=()) + await srv.send_end_of_burst() + await srv.complete_handshake() + # Config points at a server that does not exist yet. + assert await _collect_cap(client, 1.0) == [] + + await srv.send_downstream_server(SASL_SERVER, 5, flags="s", bursting=False) + await _expect_cap_new(client) + + await srv.disconnect() + caps = await _collect_cap(client, 2.0) + assert caps == [("DEL", "sasl")], caps + finally: + await client.send("QUIT :done") + await client.disconnect() + + +async def test_wildcard_mask_prefers_fully_linked_match_and_routes_to_it(ircd_hub): + """With a wildcard sasl.server, a bursting lower-numnick match must not hide an established one. + + ``chan*.test.net`` matches both ``chanb`` (numeric 5, re-linking and still + bursting) and ``channels`` (numeric 6, established). Availability must + stay on, no DEL/NEW churn may be emitted, and AUTHENTICATE must be + routed to the server that was actually validated -- ``channels``. + """ + client = await _capnotify_client(ircd_hub, "capsplit6") + try: + srv, _ = await _half_link_with_sasl_server( + ircd_hub, sasl_server="chan*.test.net", downstreams=() + ) + await srv.send_end_of_burst() + await srv.complete_handshake() + assert await _collect_cap(client, 1.0) == [] + + channels_num = await srv.send_downstream_server( + SASL_SERVER, 6, flags="s", bursting=False + ) + await _expect_cap_new(client) + + # A lower-numnick match appears, still bursting. find_match_server() + # would return it first; sasl must keep using channels. + chanb_num = await srv.send_downstream_server( + "chanb.test.net", 5, flags="s", bursting=True + ) + assert await _collect_cap(client, 1.5) == [], ( + "bursting lower-numnick match flipped sasl availability" + ) + + target = await _authenticate_target(ircd_hub, srv, "capauth6") + assert target == channels_num, ( + f"AUTHENTICATE routed to {target}, expected {channels_num} " + f"(channels); chanb is {chanb_num}" + ) + + # chanb finishing its burst changes nothing that clients can see. + await srv.send_end_of_burst_for(chanb_num) + assert await _collect_cap(client, 1.0) == [] + + await srv.disconnect() + caps = await _collect_cap(client, 2.0) + assert caps == [("DEL", "sasl")], caps + finally: + await client.send("QUIT :done") + await client.disconnect()