Fix TLS sendq/partial-write handling, auth timer lifecycle, and backend close semantics

docker-compose.yml

@@ -68,7 +68,22 @@ services:
       args:
         IRCD_CONF: tests/docker/ircd-tls-hub.conf
         TLS_BACKEND: ${TLS_BACKEND:-openssl}
+        SANITIZE: ${IRCD_SANITIZE:-}
     container_name: ircu-tls-hub
+    environment:
+      IRCD_DEBUG: ${IRCD_DEBUG:-}
+      ASAN_OPTIONS: ${ASAN_OPTIONS:-abort_on_error=1:halt_on_error=1:detect_leaks=0:log_path=/opt/ircu/debug/asan}
+    volumes:
+      - ${IRCD_DEBUG_DIR:-./tests/debug-output}:/opt/ircu/debug
+    security_opt:
+      - seccomp:unconfined
+    ulimits:
+      core:
+        soft: -1
+        hard: -1
+      nofile:
+        soft: 4096
+        hard: 4096
     ports:
       - "16677:6677"
       - "16697:6697"
@@ -88,7 +103,22 @@ services:
       args:
         IRCD_CONF: tests/docker/ircd-tls-leaf.conf
         TLS_BACKEND: ${TLS_BACKEND:-openssl}
+        SANITIZE: ${IRCD_SANITIZE:-}
     container_name: ircu-tls-leaf
+    environment:
+      IRCD_DEBUG: ${IRCD_DEBUG:-}
+      ASAN_OPTIONS: ${ASAN_OPTIONS:-abort_on_error=1:halt_on_error=1:detect_leaks=0:log_path=/opt/ircu/debug/asan}
+    volumes:
+      - ${IRCD_DEBUG_DIR:-./tests/debug-output}:/opt/ircu/debug
+    security_opt:
+      - seccomp:unconfined
+    ulimits:
+      core:
+        soft: -1
+        hard: -1
+      nofile:
+        soft: 4096
+        hard: 4096
     ports:
       - "16678:6678"
       - "16680:6680"

include/ircd_tls.h

@@ -96,6 +96,9 @@ static inline int ircd_tls_trust_verifies_ca(ircd_tls_trust_policy policy)
 /** Timeout for TLS handshake in seconds */
 #define TLS_HANDSHAKE_TIMEOUT 5
 
+/** Size of the human-readable reason buffer filled by ircd_tls_negotiate(). */
+#define TLS_REASON_LEN 128
+
 /* The following variables and functions are provided by ircu2's core
  * code, not by the TLS interface.
  */
@@ -227,10 +230,15 @@ void ircd_tls_listen_free(struct Listener *listener);
  * client's socket and returns 0.
  *
  * @param[in] cptr Locally connected client to perform handshake for.
+ * @param[out] reason If non-NULL, receives a human-readable failure reason
+ *   on a -1 return (empty otherwise).  Intended for operator notices and
+ *   the disconnect log, not for the peer (a categorical ERROR line is sent
+ *   to the peer instead).
+ * @param[in] reasonlen Size of the \a reason buffer (see TLS_REASON_LEN).
  * \returns 1 on completed handshake, 0 on continuing handshake, -1 on
  *   error.
  */
-int ircd_tls_negotiate(struct Client *cptr);
+int ircd_tls_negotiate(struct Client *cptr, char *reason, size_t reasonlen);
 
 /** ircd_tls_recv() performs a non-blocking receive of TLS application
  * data from \a cptr into \a buf.

include/msgq.h

@@ -92,7 +92,7 @@ extern void msgq_append(struct Client *dest, struct MsgBuf *mb,
 			const char *format, ...);
 extern void msgq_clean(struct MsgBuf *mb);
 extern void msgq_add(struct MsgQ *mq, struct MsgBuf *mb, int prio);
-extern void msgq_excise(struct MsgQ *mq, const char *buf, unsigned int len);
+extern void msgq_excise(struct MsgQ *mq, const char *buf);
 extern void msgq_count_memory(struct Client *cptr,
                               size_t *msg_alloc, size_t *msg_used);
 extern void msgq_histogram(struct Client *cptr, const struct StatDesc *sd,

ircd/list.c

@@ -168,6 +168,9 @@ static void dealloc_connection(struct Connection* con)
   if (-1 < con_fd(con))
     close(con_fd(con));
   MsgQClear(&(con_sendQ(con)));
+  /* MsgQClear frees MsgBufs; drop TLS mid-message rexmit into them. */
+  con->con_rexmit = NULL;
+  con->con_rexmit_len = 0;
   client_drop_sendq(con);
   DBufClear(&(con_recvQ(con)));
   if (con_listener(con))

ircd/listener.c

@@ -156,7 +156,7 @@ void show_ports(struct Client* sptr, const struct StatDesc* sd,
                 char* param)
 {
   struct Listener *listener = 0;
-  char flags[8];
+  char flags[16];  /* type + H + E + "4-" + "6-" + F + NUL = 9 bytes max */
   int show_hidden = IsOper(sptr);
   int count = (IsOper(sptr) || MyUser(sptr)) ? 100 : 8;
   int port = 0;

ircd/m_server.c

@@ -612,8 +612,12 @@ int mr_server(struct Client* cptr, struct Client* sptr, int parc, char* parv[])
   if (!EmptyString(aconf->tls_fingerprint)
     && ircd_strcmp(cli_tls_fingerprint(cptr), aconf->tls_fingerprint)) {
     ++ServerStats->is_wrong_server;
-    sendto_opmask_butone(0, SNO_OLDSNO, "Access denied (fingerprint mismatch) %s",
-                         cli_name(cptr));
+    sendto_opmask_butone(0, SNO_OLDSNO,
+                         "TLS fingerprint mismatch for server %s: presented %s, "
+                         "configured %s", cli_name(cptr),
+                         EmptyString(cli_tls_fingerprint(cptr))
+                           ? "(none)" : cli_tls_fingerprint(cptr),
+                         aconf->tls_fingerprint);
     return exit_client_msg(cptr, cptr, &me,
                            "Access denied. Bad TLS fingerprint for server %s", cli_name(cptr));
   }

ircd/msgq.c

@@ -608,38 +608,49 @@ msgq_add(struct MsgQ *mq, struct MsgBuf *mb, int prio)
   mq->count++; /* and the queue count */
 }
 
+/** Excise the head message of \a qlist if \a buf points into its buffer.
+ * @param[in,out] mq Message queue owning \a qlist.
+ * @param[in] qlist Queue list (normal or priority) to test.
+ * @param[in] buf Pointer that may fall within the head message's buffer.
+ * @return Non-zero if the head message was found and removed.
+ */
 static int msgqlist_excise(struct MsgQ *mq, struct MsgQList *qlist,
-                           const char *buf, unsigned int len)
+                           const char *buf)
 {
-  struct Msg *msg;
+  struct Msg *msg = qlist->head;
+  unsigned int len;
 
-  msg = qlist->head;
   if (!msg)
     return 0;
 
-  if (buf != msg->msg->msg)
+  /* Does buf point somewhere within this head message's buffer? */
+  if (buf < msg->msg->msg || buf >= msg->msg->msg + msg->msg->length)
     return 0;
 
-  assert(len == msg->msg->length);
+  len = msg->msg->length - msg->sent; /* delete the whole remaining message */
   msgq_delmsg(mq, qlist, &len);
   return 1;
 }
 
-/** Excise a message from the front of a message queue.
+/** Remove, by identity, the queued message that \a buf points into.
  *
- * This is used for TLS, where TLS libraries may return an EAGAIN-like
- * condition for a send but also require the application to provide
- * exactly the same contents for the next send.
+ * Used by the TLS send path.  A partial-write remainder (con_rexmit) is a raw
+ * pointer into a queued message, decoupled from the queue's own byte
+ * accounting.  When that message finishes draining it must be removed by
+ * identity rather than by feeding its byte count to msgq_delete(): the latter
+ * deletes in (partial-normal, prio, normal) order and would misattribute the
+ * bytes to a priority message that jumped ahead of it while the socket was
+ * blocked.  \a buf always points into the head message of one of the two
+ * queues (new priority messages append at the tail, so they never displace an
+ * in-flight head), which both lists are checked for.
  *
- * @warning \a buf must be at the front of one of \a mq's queues.
- * @param[in] mq Message queue to operate on.
- * @param[in] buf Buffered message to excise.
- * @param[in] len Length of buffered message.
+ * @param[in,out] mq Message queue to operate on.
+ * @param[in] buf Pointer anywhere within the head message to remove.
  */
-void msgq_excise(struct MsgQ *mq, const char *buf, unsigned int len)
+void msgq_excise(struct MsgQ *mq, const char *buf)
 {
-  if (!msgqlist_excise(mq, &mq->queue, buf, len)
-      && !msgqlist_excise(mq, &mq->prio, buf, len))
+  if (!msgqlist_excise(mq, &mq->queue, buf)
+      && !msgqlist_excise(mq, &mq->prio, buf))
     assert(0 && "msgq_excise() could not find message to excise");
 }
 

ircd/s_auth.c

@@ -91,6 +91,7 @@ enum AuthRequestFlag {
     AR_IAUTH_FUSERNAME, /**< iauth sent a forced username */
     AR_IAUTH_SOFT_DONE, /**< iauth has no objection to client */
     AR_GLINE_CHECKED,   /**< checked for a G-line banning the client */
+    AR_FREE_PENDING,    /**< destroy during timer MARKED; freelist on ET_DESTROY */
     AR_NUM_FLAGS
 };
 
@@ -962,10 +963,28 @@ void destroy_auth_request(struct AuthRequest* auth)
     s_fd(&auth->socket) = -1;
   }
 
-  if (t_active(&auth->timeout))
+  /*
+   * Detach from the client before touching the freelist.  If this is
+   * called from auth_timeout_callback while the timeout timer is
+   * GEN_MARKED, timer_del() is a no-op and timer_run() still owns the
+   * Timer.  Freelisting now lets start_auth() memset/reuse the same
+   * AuthRequest, which zeros timeout links still referenced by the
+   * timer queue and creates a self-loop — timer_enqueue() then spins
+   * forever (100% CPU).
+   */
+  if (auth->client)
+    cli_auth(auth->client) = NULL;
+  auth->client = NULL;
+
+  if (t_onqueue(&auth->timeout) || t_active(&auth->timeout))
     timer_del(&auth->timeout);
 
-  cli_auth(auth->client) = NULL;
+  if (auth->timeout.t_header.gh_flags & GEN_MARKED) {
+    /* timer_run() will ET_DESTROY after the expire callback returns. */
+    FlagSet(&auth->flags, AR_FREE_PENDING);
+    return;
+  }
+
   auth->next = auth_freelist;
   auth_freelist = auth;
 }
@@ -1029,9 +1048,23 @@ static void auth_timeout_callback(struct Event* ev)
 
   auth = (struct AuthRequest*) t_data(ev_timer(ev));
 
+  if (ev_type(ev) == ET_DESTROY) {
+    /* Completes destroy_auth_request() deferred while GEN_MARKED. */
+    if (FlagHas(&auth->flags, AR_FREE_PENDING)) {
+      FlagClr(&auth->flags, AR_FREE_PENDING);
+      auth->next = auth_freelist;
+      auth_freelist = auth;
+    }
+    return;
+  }
+
   if (ev_type(ev) == ET_EXPIRE) {
     int flag = 0;
 
+    /* Already destroyed while marked (client gone). */
+    if (!auth->client)
+      return;
+
     /* Report the timeout in the log. */
     log_write(LS_RESOLVER, L_INFO, 0, "Registration timeout %s",
               get_client_name(auth->client, HIDE_IP));
@@ -1233,6 +1266,8 @@ static void start_iauth_query(struct AuthRequest *auth)
     FlagClr(&auth->flags, AR_IAUTH_PENDING);
 }
 
+static void start_dns_ident_queries(struct Client *client);
+
 /** Starts auth (identd) and dns queries for a client.
  * @param[in] client The client for which to start queries.
  */
@@ -1256,9 +1291,19 @@ void start_auth(struct Client* client)
 
   /* Allocate the AuthRequest. */
   auth = auth_freelist;
-  if (auth)
+  if (auth) {
       auth_freelist = auth->next;
-  else
+      /*
+       * A freelisted AuthRequest must have had its timeout timer fully
+       * destroyed (off-queue, inactive) before it was freed — destroy_auth_request()
+       * defers freelisting until the timer's ET_DESTROY via AR_FREE_PENDING for
+       * exactly this reason.  Assert the invariant rather than "repairing" it:
+       * a timer_del() on a still-GEN_MARKED timer is a no-op, after which the
+       * memset() below would zero links timer_run() still owns and recreate the
+       * timer-enqueue self-loop (100% CPU).
+       */
+      assert(!t_onqueue(&auth->timeout) && !t_active(&auth->timeout));
+  } else
       auth = MyMalloc(sizeof(*auth));
   assert(0 != auth);
   memset(auth, 0, sizeof(*auth));
@@ -1297,9 +1342,15 @@ void start_auth(struct Client* client)
     }
   }
 
-  /* Start DNS and ident queries, except websocket connections not having a handshake. */
+  /*
+   * Start DNS and ident queries, except websocket connections still waiting
+   * for the HTTP upgrade.  Use the query helper — not start_dns_ident() —
+   * so we do not call check_auth_finished() twice on the same AuthRequest.
+   * (start_dns_ident() is the deferred resume path from s_bsd after the
+   * WebSocket handshake and finishes auth itself.)
+   */
   if (!IsWebsocketPort(client) || IsWebsocket(client))
-    start_dns_ident(client);
+    start_dns_ident_queries(client);
 
   /* Add client to GlobalClientList. */
   add_client_to_list(client);
@@ -1308,12 +1359,14 @@ void start_auth(struct Client* client)
   check_auth_finished(auth, 0);
 }
 
-/** Start DNS and ident queries for a client, if appropriate.
- * @param[in] client The client for which to start queries.
+/** Start DNS and ident queries for \a client without finishing auth.
+ * Used from start_auth(); the caller owns the subsequent
+ * check_auth_finished().
  */
-void start_dns_ident(struct Client *client)
+static void start_dns_ident_queries(struct Client *client)
 {
   struct AuthRequest *auth;
+
   assert(client != NULL);
   auth = cli_auth(client);
   assert(auth != NULL);
@@ -1329,8 +1382,20 @@ void start_dns_ident(struct Client *client)
 
   if (IsCloudflarePort(client) && !FlagHas(&auth->flags, AR_IAUTH_PENDING))
     start_iauth_query(auth);
+}
 
-  /* Check which auth events remain pending. */
+/** Resume DNS/ident after a deferred setup phase (WebSocket handshake).
+ * Starts the queries and then checks whether auth can finish.  Must not be
+ * used from start_auth(), which already finishes auth itself.
+ * @param[in] client The client for which to start queries.
+ */
+void start_dns_ident(struct Client *client)
+{
+  struct AuthRequest *auth;
+
+  start_dns_ident_queries(client);
+  auth = cli_auth(client);
+  assert(auth != NULL);
   check_auth_finished(auth, 0);
 }