From 6d2a9ab245c8c2a1f6c890290c819cab3514c75b Mon Sep 17 00:00:00 2001 From: MrIron Date: Thu, 27 Aug 2026 11:42:24 +0200 Subject: [PATCH 1/3] Fix spurious CAP NEW sasl on netsplit of a half-linked SASL server A SASL server (channels.*) behind a hub that linked half-way -- introduced its downlinks but never completed its burst -- and then pinged out caused cap-notify clients to see "CAP NEW sasl" immediately followed by "CAP DEL sasl" on the split. Three defects combined: * SASL availability was only re-evaluated on END_OF_BURST_ACK, so the capability flag went stale relative to find_match_server() while the SASL server was linked through a bursting hop. * exit_one_client() re-checked availability for every server torn down in the split. A sibling of the SASL server exited first, the check still found the SASL server in server_list[], and emitted CAP NEW one message before the real CAP DEL. * cap_new()/cap_del() iterated i < HighestFd instead of <=, so the local client holding the highest fd never received CAP NEW/DEL at all. Fixes: * sasl_available() walks the uplink chain from the SASL server to &me and reports unavailable if any hop IsBurst(). This gates both advertising and m_sasl's runtime routing check. * Re-check on END_OF_BURST (the moment IsBurst clears) instead of on END_OF_BURST_ACK, which a services package may never send. * Re-check once at the end of exit_client() after the whole subtree is gone, instead of per server inside exit_one_client(). * cap_new()/cap_del() loop to <= HighestFd. * CAP NEW now prefixes the trailing parameter with ':' like CAP DEL. Tests: tests/pr66_capsasl/test_sasl_cap_burst_split.py reproduces the incident topology with a fake P10 hub introducing the SASL server plus a sibling, dropping mid-burst (fails on the old code with exactly [NEW sasl=PLAIN, DEL sasl]), and covers NEW-on-EB, single DEL on split, and a J10-introduced SASL server staying hidden until its own EB. P10Server.handshake() is split into begin_handshake()/send_end_of_burst()/ complete_handshake(), and send_downstream_server() gains bursting= (P10 vs J10) plus send_end_of_burst_for(). --- ircd/m_cap.c | 8 +- ircd/m_endburst.c | 5 +- ircd/s_misc.c | 10 +- ircd/sasl.c | 17 +- tests/p10_server.py | 37 +++- .../pr66_capsasl/test_sasl_cap_burst_split.py | 175 ++++++++++++++++++ 6 files changed, 239 insertions(+), 13 deletions(-) create mode 100644 tests/pr66_capsasl/test_sasl_cap_burst_split.py 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/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..ecde5228 100644 --- a/ircd/sasl.c +++ b/ircd/sasl.c @@ -72,15 +72,28 @@ static struct SaslSessionEntry* sasl_session_table[SASL_HASH_SIZE]; static struct SaslStats sasl_statistics = { 0, 0 }; /** Check if SASL is available - * @return 1 if SASL server is configured, 0 otherwise + * + * SASL is available when a SASL server and mechanism list are configured, + * the SASL server is linked, and no server on the path between us and the + * SASL server is still bursting. A half-completed link may already have + * introduced the SASL server, but we must not advertise (or route to) it + * until END_OF_BURST has been received from every hop on the way. + * @return 1 if the SASL server is reachable, 0 otherwise */ int sasl_available(void) { + struct Client* acptr; + if (!*netconf_str(NETCONF_SASL_SERVER) || !*netconf_str(NETCONF_SASL_MECHANISMS) - || !find_match_server((char*)netconf_str(NETCONF_SASL_SERVER))) + || !(acptr = find_match_server((char*)netconf_str(NETCONF_SASL_SERVER)))) return 0; + for (; acptr && !IsMe(acptr); acptr = cli_serv(acptr)->up) { + if (IsBurst(acptr)) + return 0; + } + return 1; } diff --git a/tests/p10_server.py b/tests/p10_server.py index 7814a458..66d2db2d 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=max(remaining, 0.1)) + + 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..53fc9b63 --- /dev/null +++ b/tests/pr66_capsasl/test_sasl_cap_burst_split.py @@ -0,0 +1,175 @@ +"""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" + + +async def _capnotify_client(ircd_hub, nick: str) -> IRCClient: + """Registered client with cap-notify active (implicit via CAP LS 302).""" + 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 _half_link_with_sasl_server(ircd_hub) -> P10Server: + """Link a fake hub that introduces the SASL server but never finishes its burst.""" + 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. Point SASL at a server *behind* us, then introduce it. + await srv.send_config("sasl.server", SASL_SERVER) + await srv.send_config("sasl.mechanisms", MECHANISMS) + # Like a real uplink re-introducing downlinks that finished their own + # burst long ago: P10, not J10. + await srv.send_downstream_server(SASL_SERVER, 5, flags="s", bursting=False) + await srv.send_downstream_server("other.test.net", 6, bursting=False) + return srv + + +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() + 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 + + 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 = P10Server(name="services.test.net", numeric=4, password="testpass") + await srv.connect(ircd_hub["host"], ircd_hub["server_port"]) + await srv.begin_handshake() + await srv.send_config("sasl.server", SASL_SERVER) + await srv.send_config("sasl.mechanisms", MECHANISMS) + sasl_num = await srv.send_downstream_server( + SASL_SERVER, 5, flags="s", bursting=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(sasl_num) + 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 + + await srv.disconnect() + caps = await _collect_cap(client, 2.0) + assert caps == [("DEL", "sasl")], caps + finally: + await client.send("QUIT :done") + await client.disconnect() From 1a064ff737885602b2ca288082eceea0a3df3d03 Mon Sep 17 00:00:00 2001 From: MrIron Date: Thu, 27 Aug 2026 11:54:41 +0200 Subject: [PATCH 2/3] Test: directly linked SASL server becomes available at its END_OF_BURST Covers the direct-link case where sasl.server already points at the bursting link: no CAP NEW while bursting, NEW exactly at EB, nothing on EA, a single DEL on disconnect. --- .../pr66_capsasl/test_sasl_cap_burst_split.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/pr66_capsasl/test_sasl_cap_burst_split.py b/tests/pr66_capsasl/test_sasl_cap_burst_split.py index 53fc9b63..c7491c20 100644 --- a/tests/pr66_capsasl/test_sasl_cap_burst_split.py +++ b/tests/pr66_capsasl/test_sasl_cap_burst_split.py @@ -173,3 +173,39 @@ async def test_cap_new_waits_for_sasl_server_own_end_of_burst(ircd_hub): 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 = P10Server(name="services.test.net", numeric=4, password="testpass") + await srv.connect(ircd_hub["host"], ircd_hub["server_port"]) + await srv.begin_handshake() + await srv.send_config("sasl.server", "services.test.net") + await srv.send_config("sasl.mechanisms", MECHANISMS) + + # 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() + 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 + + 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() From 7e4b5a7c07a77af7cbdb0a01c5273352d03fe672 Mon Sep 17 00:00:00 2001 From: MrIron Date: Sat, 29 Aug 2026 23:06:56 +0200 Subject: [PATCH 3/3] Address review: validated SASL server accessor, wildcard masks, introduction hook Review findings on #100: 1. With a wildcard sasl.server mask, find_match_server() returns the lowest-numnick match, so a matching server that is re-linking and still bursting could hide an established one and cause DEL/NEW churn. Add find_match_server_next() to enumerate all matches; sasl_server() walks them and returns the first whose path to us is not bursting. 2. A server matching sasl.server introduced as already past its burst (P) over an established link never sends END_OF_BURST, so nothing re-checked availability. Call sasl_check_capability() at the end of ms_server(); it is a no-op for J introductions and for servers behind a bursting hop, so nothing is advertised during a burst. 3. m_sasl() did its own find_match_server() lookup, only coincidentally routing AUTHENTICATE to the server sasl_available() validated. Both now use sasl_server(), which is the single source of truth and returns the validated struct Client *. This also stops collapse()ing the netconf value in place: the mask is copied first. 4. Tests: _half_link_with_sasl_server() is parameterized on the sasl.server value and the downstream servers to introduce, replacing the inline copies. New tests cover the P-introduction-over-established- link case, and the wildcard case with a bursting lower-numnick sibling including that AUTHENTICATE is routed to the validated server (XQ target numeric). 5. P10Server.handshake() passes the remaining budget straight to complete_handshake() instead of clamping to 100 ms. --- include/numnicks.h | 1 + include/sasl.h | 1 + ircd/m_sasl.c | 3 +- ircd/m_server.c | 8 + ircd/numnicks.c | 32 +++- ircd/sasl.c | 68 +++++-- tests/p10_server.py | 2 +- .../pr66_capsasl/test_sasl_cap_burst_split.py | 179 ++++++++++++++---- 8 files changed, 234 insertions(+), 60 deletions(-) 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_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/sasl.c b/ircd/sasl.c index ecde5228..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,30 +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 +/** 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. + */ +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. * - * SASL is available when a SASL server and mechanism list are configured, - * the SASL server is linked, and no server on the path between us and the - * SASL server is still bursting. A half-completed link may already have - * introduced the SASL server, but we must not advertise (or route to) it - * until END_OF_BURST has been received from every hop on the way. - * @return 1 if the SASL server is reachable, 0 otherwise + * 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. */ -int sasl_available(void) +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) - || !(acptr = find_match_server((char*)netconf_str(NETCONF_SASL_SERVER)))) - return 0; - - for (; acptr && !IsMe(acptr); acptr = cli_serv(acptr)->up) { - if (IsBurst(acptr)) - 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 66d2db2d..d7384aa3 100644 --- a/tests/p10_server.py +++ b/tests/p10_server.py @@ -241,7 +241,7 @@ async def handshake(self, timeout: float = 15.0): await self.begin_handshake(timeout=timeout) await self.send_end_of_burst() remaining = deadline - asyncio.get_event_loop().time() - await self.complete_handshake(timeout=max(remaining, 0.1)) + 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. diff --git a/tests/pr66_capsasl/test_sasl_cap_burst_split.py b/tests/pr66_capsasl/test_sasl_cap_burst_split.py index c7491c20..f1881079 100644 --- a/tests/pr66_capsasl/test_sasl_cap_burst_split.py +++ b/tests/pr66_capsasl/test_sasl_cap_burst_split.py @@ -41,9 +41,20 @@ 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).""" + """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") @@ -69,20 +80,58 @@ async def _collect_cap(client: IRCClient, timeout: float) -> list[tuple[str, str seen.append((msg.params[1], msg.params[-1])) -async def _half_link_with_sasl_server(ircd_hub) -> P10Server: - """Link a fake hub that introduces the SASL server but never finishes its burst.""" +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. Point SASL at a server *behind* us, then introduce it. - await srv.send_config("sasl.server", SASL_SERVER) + # not sent ours. + await srv.send_config("sasl.server", sasl_server) await srv.send_config("sasl.mechanisms", MECHANISMS) - # Like a real uplink re-introducing downlinks that finished their own - # burst long ago: P10, not J10. - await srv.send_downstream_server(SASL_SERVER, 5, flags="s", bursting=False) - await srv.send_downstream_server("other.test.net", 6, bursting=False) - return srv + 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): @@ -93,7 +142,7 @@ async def test_no_cap_new_when_half_linked_sasl_server_splits(ircd_hub): """ client = await _capnotify_client(ircd_hub, "capsplit1") try: - srv = await _half_link_with_sasl_server(ircd_hub) + srv, _ = await _half_link_with_sasl_server(ircd_hub) # Still bursting: nothing may be advertised yet. assert await _collect_cap(client, 1.5) == [] @@ -114,16 +163,14 @@ 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) + 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() - 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 + await _expect_cap_new(client) await srv.complete_handshake() # The EA exchange must not re-announce anything. @@ -146,13 +193,8 @@ async def test_cap_new_waits_for_sasl_server_own_end_of_burst(ircd_hub): """ client = await _capnotify_client(ircd_hub, "capsplit3") try: - srv = P10Server(name="services.test.net", numeric=4, password="testpass") - await srv.connect(ircd_hub["host"], ircd_hub["server_port"]) - await srv.begin_handshake() - await srv.send_config("sasl.server", SASL_SERVER) - await srv.send_config("sasl.mechanisms", MECHANISMS) - sasl_num = await srv.send_downstream_server( - SASL_SERVER, 5, flags="s", bursting=True + 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. @@ -162,10 +204,8 @@ async def test_cap_new_waits_for_sasl_server_own_end_of_burst(ircd_hub): "sasl advertised while the SASL server itself is still bursting" ) - await srv.send_end_of_burst_for(sasl_num) - 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 + 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) @@ -184,11 +224,9 @@ async def test_directly_linked_sasl_server_available_at_end_of_burst(ircd_hub): """ client = await _capnotify_client(ircd_hub, "capsplit4") try: - srv = P10Server(name="services.test.net", numeric=4, password="testpass") - await srv.connect(ircd_hub["host"], ircd_hub["server_port"]) - await srv.begin_handshake() - await srv.send_config("sasl.server", "services.test.net") - await srv.send_config("sasl.mechanisms", MECHANISMS) + 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) == [], ( @@ -196,13 +234,86 @@ async def test_directly_linked_sasl_server_available_at_end_of_burst(ircd_hub): ) await srv.send_end_of_burst() - 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 + 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