Fix spurious CAP NEW sasl on netsplit of a half-linked SASL server

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);
 

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);

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;
 

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;
 }

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)

ircd/m_server.c

@@ -49,6 +49,7 @@
 #include "s_serv.h"
 #include "send.h"
 #include "userload.h"
+#include "sasl.h"
 
 /* #include <assert.h> -- Now using assert in ircd_log.h */
 #include <stdlib.h>
@@ -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;
 }

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;
 }

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,

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

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,