Drive the inbound TLS handshake by socket events; kqueue: deliver unread data before EOF

include/ircd_tls.h

@@ -93,7 +93,8 @@ static inline int ircd_tls_trust_verifies_ca(ircd_tls_trust_policy policy)
   return policy == TLS_TRUST_REQUIRE_CA;
 }
 
-/** Timeout for TLS handshake in seconds */
+/** Timeout for a TLS handshake in seconds, measured from when the handshake
+ * starts (enforced by the connection timer in s_bsd.c, not by the backends). */
 #define TLS_HANDSHAKE_TIMEOUT 5
 
 /** Size of the human-readable reason buffer filled by ircd_tls_negotiate(). */
@@ -226,19 +227,26 @@ void ircd_tls_listen_free(struct Listener *listener);
 /** ircd_tls_negotiate() attempts to continue an initial TLS handshake
  * for \a cptr.  If the handshake completes, this function calls
  * \a ClearNegotiatingTLS(cptr) and returns 1.  If the handshake failed,
- * this function returns -1.  Otherwise it updates event flags for the
- * client's socket and returns 0.
+ * this function returns -1.  Otherwise it returns 0 and reports through
+ * \a wants_write which socket direction the handshake is blocked on, so the
+ * caller can adjust the socket's event interest (the backend itself never
+ * touches socket events).
  *
  * @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).
+ * @param[out] wants_write If non-NULL, set to 1 when a 0 return means the
+ *   handshake is waiting to write (SSL_ERROR_WANT_WRITE and equivalents,
+ *   or a backend asking to be called again immediately), 0 when it is
+ *   waiting for peer data.  Always 0 on a non-zero return.
  * \returns 1 on completed handshake, 0 on continuing handshake, -1 on
  *   error.
  */
-int ircd_tls_negotiate(struct Client *cptr, char *reason, size_t reasonlen);
+int ircd_tls_negotiate(struct Client *cptr, char *reason, size_t reasonlen,
+                       int *wants_write);
 
 /** ircd_tls_recv() performs a non-blocking receive of TLS application
  * data from \a cptr into \a buf.

ircd/engine_kqueue.c

@@ -410,7 +410,15 @@ engine_loop(struct Generators* gen)
       case SS_CONNECTED:
 	if (evt->filter == EVFILT_READ) { /* data on socket */
 	  Debug((DEBUG_ENGINE, "kqueue: EOF or data to be read"));
-	  event_generate(evt->flags & EV_EOF ? ET_EOF : ET_READ, sock, 0);
+	  /* EV_EOF is set as soon as the peer's FIN arrives, even while
+	   * evt->data bytes are still unread (typically the peer's final
+	   * ERROR/SQUIT line).  Deliver those as ET_READ first; the filter is
+	   * level-triggered, so once the buffer is drained the next kevent()
+	   * returns EV_EOF with data == 0 and becomes the real ET_EOF. */
+	  if ((evt->flags & EV_EOF) && evt->data <= 0)
+	    event_generate(ET_EOF, sock, 0);
+	  else
+	    event_generate(ET_READ, sock, 0);
 	}
 	if (evt->filter == EVFILT_WRITE) { /* socket writable */
 	  Debug((DEBUG_ENGINE, "kqueue: Data can be written"));

ircd/s_bsd.c

@@ -107,6 +107,9 @@ const char* const TOS_ERROR_MSG	      = "error setting TOS for %s: %s";
 
 static void client_sock_callback(struct Event* ev);
 static void client_timer_callback(struct Event* ev);
+static void tls_negotiation_events(struct Client *cptr, int wants_write);
+static void tls_handshake_timer_arm(struct Client *cptr);
+static int tls_negotiate_client(struct Client *cptr, char **fmt, char **fallback);
 
 
 /*
@@ -371,30 +374,21 @@ static int completed_connection(struct Client* cptr)
       s_tls(&cli_socket(cptr)) = tls;
       SetNegotiatingTLS(cptr);
       SetTLS(cptr);
+      tls_handshake_timer_arm(cptr);
     }
 
-    /* Are we making progress?  Handle the result like tls_negotiate_client():
-     * a negative result (timeout, fatal handshake error, missing session) must
-     * fail the link now rather than wait for the ping timeout or fall through
-     * to sending PASS/SERVER on a socket without a TLS session. */
+    /* Are we making progress?  A failure (fatal handshake error, missing
+     * session) must fail the link now rather than fall through to sending
+     * PASS/SERVER on a socket without a TLS session; tls_negotiate_client()
+     * has notified opers, marked the socket dead and dropped the session, so
+     * the caller's exit cannot leak plaintext into the handshake stream. */
     if (IsNegotiatingTLS(cptr)) {
-      char reason[TLS_REASON_LEN];
-      int res = ircd_tls_negotiate(cptr, reason, sizeof(reason));
-
-      if (res < 0) {
-        sendto_opmask_butone(0, SNO_OLDSNO, "TLS negotiation failed to %s%s%s",
-                             cli_name(cptr), reason[0] ? ": " : "", reason);
-        /* Mark dead before returning so exit_client() does not flush an
-         * ERROR line as plaintext into the half-open handshake stream
-         * (can_send() rejects a dead socket).  Mirrors tls_negotiate_client(). */
-        SetFlag(cptr, FLAG_DEADSOCKET);
-        ClearNegotiatingTLS(cptr);
-        if (s_tls(&cli_socket(cptr))) {
-          ircd_tls_close(s_tls(&cli_socket(cptr)), NULL);
-          s_tls(&cli_socket(cptr)) = NULL;
-        }
+      char *fmt = "%s";
+      char *fallback = 0;
+      int res = tls_negotiate_client(cptr, &fmt, &fallback);
+
+      if (res < 0)
         return 0;
-      }
       if (res == 0)
         return 1; /* still negotiating */
     }
@@ -550,6 +544,7 @@ void add_connection(struct Listener* listener, int fd) {
   struct Client      *new_client;
   time_t             next_target = 0;
   void               *tls;
+  int                ipchecked;
 
   const char* const throttle_message =
          "ERROR :Your host is trying to (re)connect too fast -- throttled\r\n";
@@ -595,33 +590,37 @@ void add_connection(struct Listener* listener, int fd) {
     }
   }
 
-  if (listener_server(listener))
+  /*
+   * Throttle check before allocating the Client, so a rejected connection
+   * has nothing to leak but the TLS session freed here.  Cloudflare
+   * websocket ports defer IPcheck until CF-Connecting-IP is known at
+   * handshake; the socket peer is a Cloudflare edge node.
+   */
+  ipchecked = 0;
+  if (!listener_server(listener) && !listener_webirc(listener)
+      && !(listener_websocket(listener) && listener_cloudflare(listener)))
   {
-    new_client = make_client(0, STAT_UNKNOWN_SERVER);
+    if (!IPcheck_local_connect(&addr.addr, &next_target))
+    {
+      ++ServerStats->is_throttled;
+      write(fd, throttle_message, strlen(throttle_message));
+      close(fd);
+      if (tls)
+        ircd_tls_close(tls, NULL);
+      return;
+    }
+    ipchecked = 1;
   }
+
+  if (listener_server(listener))
+    new_client = make_client(0, STAT_UNKNOWN_SERVER);
   else if (listener_webirc(listener))
-  {
-      new_client = make_client(0, STAT_WEBIRC);
-  }
+    new_client = make_client(0, STAT_WEBIRC);
   else
-  {
     new_client = make_client(0, listener_websocket(listener) ? STAT_WEBSOCKET : STAT_UNKNOWN_USER);
 
-    /*
-     * Cloudflare websocket ports: defer IPcheck until CF-Connecting-IP is
-     * known at handshake; the socket peer is a Cloudflare edge node.
-     */
-    if (!(listener_websocket(listener) && listener_cloudflare(listener))) {
-      if (!IPcheck_local_connect(&addr.addr, &next_target))
-      {
-        ++ServerStats->is_throttled;
-        write(fd, throttle_message, strlen(throttle_message));
-        close(fd);
-        return;
-      }
-      SetIPChecked(new_client);
-    }
-  }
+  if (ipchecked)
+    SetIPChecked(new_client);
 
   /*
    * Copy ascii address to 'sockhost' just in case. Then we have something
@@ -641,6 +640,11 @@ void add_connection(struct Listener* listener, int fd) {
     write(fd, register_message, strlen(register_message));
     close(fd);
     cli_fd(new_client) = -1;
+    if (tls)
+      ircd_tls_close(tls, NULL);
+    if (IsIPChecked(new_client))
+      IPcheck_disconnect(new_client);
+    free_client(new_client);
     return;
   }
   cli_freeflag(new_client) |= FREEFLAG_SOCKET;
@@ -652,7 +656,16 @@ void add_connection(struct Listener* listener, int fd) {
   {
     SetTLS(new_client);
     SetNegotiatingTLS(new_client);
-    socket_events(&cli_socket(new_client), SOCK_EVENT_WRITABLE);
+    /* Wait for the ClientHello.  The handshake is driven by ET_READ /
+     * ET_WRITE in client_sock_callback(); tls_negotiation_events() adds
+     * WRITABLE only while the backend is blocked on a write.  Registering
+     * WRITABLE here would busy-loop on a level-triggered writable socket
+     * until the peer's first flight arrived. */
+    socket_events(&cli_socket(new_client), SOCK_EVENT_READABLE);
+    /* Until start_auth() runs after the handshake this client is not in
+     * LocalClientArray, so check_pings(), /CLOSE and kill_highest_sendq()
+     * cannot see it; the handshake timer is the only thing that reaps it. */
+    tls_handshake_timer_arm(new_client);
   }
 
   Count_newunknown(UserStats);
@@ -1099,34 +1112,103 @@ void init_server_identity(void)
   SetYXXServerName(&me, conf->numeric);
 }
 
-/** Notify operators of inbound TLS failures on server ports. */
+/** Notify operators of a failed TLS handshake on a server link: an
+ * outbound link we initiated, or an inbound connection on a server port.
+ * This is the only place that reports it, so a failure detected on a
+ * later socket event (ET_READ after the connect step) is reported exactly
+ * like one detected during the connect step itself. */
 static void tls_negotiation_failed(struct Client *cptr, const char *reason)
 {
-  if (IsServerPort(cptr))
+  if (IsConnecting(cptr))
+    sendto_opmask_butone(0, SNO_OLDSNO, "TLS negotiation failed to %s%s%s",
+                         cli_name(cptr),
+                         (reason && reason[0]) ? ": " : "",
+                         reason ? reason : "");
+  else if (IsServerPort(cptr))
     sendto_opmask_butone(0, SNO_OLDSNO,
                          "TLS negotiation failed from unknown server%s%s",
                          (reason && reason[0]) ? ": " : "",
                          reason ? reason : "");
 }
 
+/** Adjust socket event interest for a handshake still in progress.
+ * Wait on exactly the direction the backend is blocked on.  A writable
+ * socket is level-triggered and almost always ready, so holding WRITABLE
+ * while waiting for the peer spins; holding READABLE while blocked on a
+ * write lets a peer that leaves bytes unread re-run the handshake every
+ * loop pass.  Errors (RST) are reported regardless of interest, and a
+ * silent peer is bounded by the handshake timer either way.
+ * @param[in] cptr Client whose TLS handshake returned "in progress".
+ * @param[in] wants_write Non-zero if the backend is waiting to write.
+ */
+static void tls_negotiation_events(struct Client *cptr, int wants_write)
+{
+  socket_events(&cli_socket(cptr), SOCK_ACTION_SET
+                | (wants_write ? SOCK_EVENT_WRITABLE : SOCK_EVENT_READABLE));
+}
+
+/** Arm the TLS handshake deadline for \a cptr.
+ * The handshake is driven purely by socket events, so a peer that never
+ * speaks (or stops mid-handshake) would otherwise sit forever.  Measured
+ * from here, i.e. from when the handshake actually starts, not from
+ * make_client(): for outbound links that would include the TCP connect
+ * and SYN retransmits.  con_proc is unused until read_packet() runs,
+ * which cannot precede the handshake; tls_handshake_succeeded() cancels
+ * the timer and free_client() deletes it on any other exit.
+ * @param[in] cptr Client whose handshake is starting.
+ */
+static void tls_handshake_timer_arm(struct Client *cptr)
+{
+  cli_freeflag(cptr) |= FREEFLAG_TIMER;
+  timer_add(&cli_proc(cptr), client_timer_callback, cli_connect(cptr),
+            TT_RELATIVE, TLS_HANDSHAKE_TIMEOUT);
+}
+
+/** Drop the TLS session of a failed handshake.
+ * Marks the socket dead first so exit_client() cannot flush an ERROR line
+ * as plaintext into the peer's half-open TLS stream (can_send() rejects a
+ * dead socket).
+ * @param[in] cptr Client whose handshake failed.
+ */
+static void tls_handshake_drop(struct Client *cptr)
+{
+  SetFlag(cptr, FLAG_DEADSOCKET);
+  ClrFlag(cptr, FLAG_NEGOTIATING_TLS);
+  if (s_tls(&cli_socket(cptr)))
+  {
+    ircd_tls_close(s_tls(&cli_socket(cptr)), NULL);
+    s_tls(&cli_socket(cptr)) = NULL;
+  }
+}
+
+/** Abort a handshake with \a reason: notify opers, drop the session and
+ * exit the client.  Used where no backend call is needed (the deadline).
+ * @param[in] cptr Client whose handshake is aborted.
+ * @param[in] reason Human-readable reason for notices and the exit.
+ */
+static void tls_handshake_abort(struct Client *cptr, const char *reason)
+{
+  tls_negotiation_failed(cptr, reason);
+  tls_handshake_drop(cptr);
+  exit_client_msg(cptr, cptr, &me, "%s", reason);
+}
+
 /** Run ircd_tls_negotiate() and handle a fatal result. */
 static int tls_negotiate_client(struct Client *cptr, char **fmt, char **fallback)
 {
   /* static: *fallback is read by the caller after we return, still within the
    * same (synchronous) socket callback, so a stack buffer would dangle. */
   static char reason[TLS_REASON_LEN];
-  int res = ircd_tls_negotiate(cptr, reason, sizeof(reason));
+  int wants_write = 0;
+  int res = ircd_tls_negotiate(cptr, reason, sizeof(reason), &wants_write);
+
+  if (res == 0)
+    tls_negotiation_events(cptr, wants_write);
 
   if (res < 0)
   {
     tls_negotiation_failed(cptr, reason);
-    SetFlag(cptr, FLAG_DEADSOCKET);
-    ClrFlag(cptr, FLAG_NEGOTIATING_TLS);
-    if (s_tls(&cli_socket(cptr)))
-    {
-      ircd_tls_close(s_tls(&cli_socket(cptr)), "TLS negotiation failed");
-      s_tls(&cli_socket(cptr)) = NULL;
-    }
+    tls_handshake_drop(cptr);
     *fmt = "%s";
     *fallback = reason[0] ? reason : "TLS negotiation failed";
   }
@@ -1137,6 +1219,10 @@ static int tls_negotiate_client(struct Client *cptr, char **fmt, char **fallback
 /** Continue client setup after an inbound or outbound TLS handshake completes. */
 static void tls_handshake_succeeded(struct Client *cptr)
 {
+  /* Drop the handshake deadline armed by tls_handshake_timer_arm(). */
+  if (t_onqueue(&cli_proc(cptr)))
+    timer_del(&cli_proc(cptr));
+
   if (IsConnecting(cptr)) {
     /* completed_connection() returns 0 when the link can no longer be set up
      * (e.g. the Connect block vanished on a rehash mid-handshake).  Exit the
@@ -1241,7 +1327,14 @@ static void client_sock_callback(struct Event* ev)
     break;
 
   case ET_READ: /* socket is readable */
-    if (!IsDead(cptr)) {
+    if (IsDead(cptr)) {
+      /* dead_link() deferred the exit to check_pings(); the readable
+       * event is level-triggered and would re-fire every loop pass until
+       * then, so exit now (same context as the ET_EOF case). */
+      exit_client(cptr, cptr, &me, cli_info(cptr));
+      return;
+    }
+    {
       Debug((DEBUG_DEBUG, "Reading data from %C", cptr));
       if (IsNegotiatingTLS(cptr)) {
         int res = tls_negotiate_client(cptr, &fmt, &fallback);
@@ -1251,8 +1344,12 @@ static void client_sock_callback(struct Event* ev)
           /* Still negotiating */
           break;
         }
-        /* TLS negotiation succeeded */
+        /* TLS negotiation succeeded.  start_auth() / completed_connection()
+         * may have exited (and freed) cptr, so do not touch it again; any
+         * application data already queued re-fires the level-triggered
+         * readable event. */
         tls_handshake_succeeded(cptr);
+        return;
       }
       if (read_packet(cptr, 1) == 0) /* error while reading packet */
         fallback = "EOF from client";
@@ -1306,6 +1403,13 @@ static void client_timer_callback(struct Event* ev)
 
     if (!con_freeflag(con) && !cptr)
       free_connection(con); /* client is being destroyed */
+  } else if (IsNegotiatingTLS(cptr)) {
+    /* Handshake deadline from tls_handshake_timer_arm().  No peer write: a
+     * stalled handshake must close with a plain EOF, not a plaintext line
+     * that would corrupt a mid-handshake peer's TLS stream.  Exiting from
+     * inside the timer's own callback is fine: timer_del() is a no-op while
+     * it is GEN_MARKED and timer_run() destroys the one-shot afterwards. */
+    tls_handshake_abort(cptr, "TLS handshake timed out");
   } else {
     Debug((DEBUG_LIST, "Client process timer for %C expired; processing",
 	   cptr));

ircd/s_misc.c

@@ -419,8 +419,12 @@ int exit_client(struct Client *cptr,
                 NumNick(victim), /* two %s's */
                 cli_name(victim), cli_info(victim));
 
+    /* IsClient() does not cover STAT_CONNECTING, but the "Link with %s
+     * canceled" notices below are meant for connecting links too: without
+     * this, an outbound link that dies between connect() and registration
+     * (e.g. reset during the TLS handshake) is invisible to opers. */
     if (victim != cli_from(killer)  /* The source knows already */
-        && IsClient(victim))    /* Not a Ping struct or Log file */
+        && (IsClient(victim) || IsConnecting(victim))) /* Not a Ping struct or Log file */
     {
       if (IsServer(victim) || IsHandshake(victim))
 	sendcmdto_one(killer, CMD_SQUIT, victim, "%s 0 :%s", cli_name(&me), comment);