Detect send errors and shut down the connection when they happen

ncat/ncat_core.c

@@ -298,8 +298,8 @@ int fdinfo_close(struct fdinfo *fdn)
 /* Do a recv on an fdinfo, without other side effects. */
 int fdinfo_recv(struct fdinfo *fdn, char *buf, size_t size)
 {
-#ifdef HAVE_OPENSSL
     int n;
+#ifdef HAVE_OPENSSL
     int err = SSL_ERROR_NONE;
     if (o.ssl && fdn->ssl)
     {
@@ -311,12 +311,18 @@ int fdinfo_recv(struct fdinfo *fdn, char *buf, size_t size)
             err = (n < 0) ? SSL_get_error(fdn->ssl, n) : SSL_ERROR_NONE;
         } while (err == SSL_ERROR_WANT_READ || err == SSL_ERROR_WANT_WRITE);
         if (err != SSL_ERROR_NONE) {
+            fdn->lasterr = err;
             logdebug("SSL_read error on %d: %s\n", fdn->fd, ERR_error_string(err, NULL));
         }
         return n;
     }
 #endif
-    return recv(fdn->fd, buf, size, 0);
+    n = recv(fdn->fd, buf, size, 0);
+    if (n == 0)
+        fdn->lasterr = EOF;
+    else if (n < 0)
+        fdn->lasterr = socket_errno();
+    return n;
 }
 
 int fdinfo_pending(struct fdinfo *fdn)
@@ -366,8 +372,8 @@ int ncat_recv(struct fdinfo *fdn, char *buf, size_t size, int *pending)
 /* Do a send on an fdinfo, without any logging or other side effects. */
 int fdinfo_send(struct fdinfo *fdn, const char *buf, size_t size)
 {
-#ifdef HAVE_OPENSSL
     int n;
+#ifdef HAVE_OPENSSL
     int err = SSL_ERROR_NONE;
     if (o.ssl && fdn->ssl != NULL)
     {
@@ -379,12 +385,16 @@ int fdinfo_send(struct fdinfo *fdn, const char *buf, size_t size)
             err = (n < 0) ? SSL_get_error(fdn->ssl, n) : SSL_ERROR_NONE;
         } while (err == SSL_ERROR_WANT_READ || err == SSL_ERROR_WANT_WRITE);
         if (err != SSL_ERROR_NONE) {
+            fdn->lasterr = err;
             logdebug("SSL_write error on %d: %s\n", fdn->fd, ERR_error_string(err, NULL));
         }
         return n;
     }
 #endif
-    return send(fdn->fd, buf, size, 0);
+    n = send(fdn->fd, buf, size, 0);
+    if (n <= 0)
+        fdn->lasterr = socket_errno();
+    return n;
 }
 
 /* If we are sending a large amount of data, we might momentarily run out of send
@@ -430,15 +440,14 @@ int ncat_broadcast(fd_set *fds, const fd_list_t *fdlist, const char *msg, size_t
         return size;
 
     ret = 0;
-    for (i = 0; i <= fdlist->fdmax; i++) {
-        if (!checked_fd_isset(i, fds))
+    for (i = 0; i < fdlist->nfds; i++) {
+        fdn = &fdlist->fds[i];
+        if (!checked_fd_isset(fdn->fd, fds))
             continue;
 
-        fdn = get_fdinfo(fdlist, i);
-        ncat_assert(fdn != NULL);
         if (blocking_fdinfo_send(fdn, msg, size) <= 0) {
             if (o.debug > 1)
-                logdebug("Error sending to fd %d: %s.\n", i, socket_strerror(socket_errno()));
+                logdebug("Error sending to fd %d: %s.\n", fdn->fd, socket_strerror(fdn->lasterr));
             ret = -1;
         }
     }

ncat/ncat_listen.c

@@ -121,6 +121,7 @@ static void handle_connection(int socket_accept, int type, fd_set *listen_fds);
 static int read_stdin(void);
 static int read_socket(int recv_fd);
 static void post_handle_connection(struct fdinfo *sinfo);
+static void close_fd(struct fdinfo *fdn, int eof);
 static void read_and_broadcast(int recv_socket);
 static void shutdown_sockets(int how);
 static int chat_announce_connect(const struct fdinfo *fdi);
@@ -167,7 +168,7 @@ static void sigchld_handler(int signum)
 
 int new_listen_socket(int type, int proto, const union sockaddr_u *addr, fd_set *listen_fds)
 {
-  struct fdinfo fdi;
+  struct fdinfo fdi = {0};
   fdi.fd = do_listen(type, proto, addr);
   if (fdi.fd < 0) {
     return -1;
@@ -289,7 +290,7 @@ int ncat_listen()
     if (o.idletimeout > 0)
         tvp = &tv;
 
-    while (1) {
+    while (client_fdlist.nfds > 1 || get_conn_count() > 0) {
         /* We pass these temporary descriptor sets to fselect, since fselect
            modifies the sets it receives. */
         fd_set readfds = master_readfds, writefds = master_writefds;
@@ -316,9 +317,18 @@ int ncat_listen()
         if (fds_ready == 0)
             bye("Idle timeout expired (%d ms).", o.idletimeout);
 
+        /* If client_fdlist.state increases, the list has changed and we
+         * need to go over it again. */
+restart_fd_loop:
+        client_fdlist.state = 0;
         for (i = 0; i < client_fdlist.nfds && fds_ready > 0; i++) {
             struct fdinfo *fdi = &client_fdlist.fds[i];
             int cfd = fdi->fd;
+            /* If we saw an error, close this fd */
+            if (fdi->lasterr != 0) {
+                close_fd(fdi, 0);
+                goto restart_fd_loop;
+            }
             /* Loop through descriptors until there's something to read */
             if (!checked_fd_isset(cfd, &readfds) && !checked_fd_isset(cfd, &writefds))
                 continue;
@@ -350,10 +360,6 @@ int ncat_listen()
                     checked_fd_clr(cfd, &sslpending_fds);
                     checked_fd_clr(cfd, &master_readfds);
                     rm_fd(&client_fdlist, cfd);
-                    /* Since we removed this one, start loop over at the beginning.
-                     * Wastes a little time, but ensures correctness.
-                     */
-                    i = 0;
                     /* Are we in single listening mode(without -k)? If so
                        then we should quit also. */
                     if (!o.keepopen && !o.broker)
@@ -395,6 +401,19 @@ int ncat_listen()
             }
 
             fds_ready--;
+            if (client_fdlist.state > 0)
+                goto restart_fd_loop;
+
+            /* Check if any send errors were logged. */
+            for (int j = 0; j < broadcast_fdlist.nfds; j++) {
+                fdi = &broadcast_fdlist.fds[j];
+                if (fdi->lasterr != 0) {
+                    close_fd(fdi, 0);
+                    /* close_fd mucks with client_fdlist, so jump back and
+                     * start the loop over */
+                    goto restart_fd_loop;
+                }
+            }
         }
     }
 
@@ -567,6 +586,32 @@ static void post_handle_connection(struct fdinfo *sinfo)
     }
 }
 
+static void close_fd(struct fdinfo *fdn, int eof) {
+    /* rm_fd invalidates fdn, so save what we need here. */
+    int fd = fdn->fd;
+    if (o.debug)
+        logdebug("Closing connection.\n");
+#ifdef HAVE_OPENSSL
+    if (o.ssl && fdn->ssl) {
+        if (eof)
+            SSL_shutdown(fdn->ssl);
+        SSL_free(fdn->ssl);
+    }
+#endif
+    Close(fd);
+    checked_fd_clr(fd, &master_readfds);
+    rm_fd(&client_fdlist, fd);
+    checked_fd_clr(fd, &master_broadcastfds);
+    rm_fd(&broadcast_fdlist, fd);
+
+    conn_inc--;
+    if (get_conn_count() == 0)
+        checked_fd_clr(STDIN_FILENO, &master_readfds);
+
+    if (o.chat)
+        chat_announce_disconnect(fd);
+}
+
 /* Read from stdin and broadcast to all client sockets. Return the number of
    bytes read, or -1 on error. */
 int read_stdin(void)
@@ -626,25 +671,13 @@ int read_socket(int recv_fd)
 
         n = ncat_recv(fdn, buf, sizeof(buf), &pending);
         if (n <= 0) {
-            if (o.debug)
-                logdebug("Closing fd %d.\n", recv_fd);
-#ifdef HAVE_OPENSSL
-            if (o.ssl && fdn->ssl) {
-                if (nbytes == 0)
-                    SSL_shutdown(fdn->ssl);
-                SSL_free(fdn->ssl);
+            /* return value can be 0 without meaning EOF in some cases such as SSL
+             * renegotiations that require read/write socket operations but do not
+             * have any application data. */
+            if(n == 0 && fdn->lasterr != 0) {
+                continue; /* Check pending */
             }
-#endif
-            close(recv_fd);
-            checked_fd_clr(recv_fd, &master_readfds);
-            rm_fd(&client_fdlist, recv_fd);
-            checked_fd_clr(recv_fd, &master_broadcastfds);
-            rm_fd(&broadcast_fdlist, recv_fd);
-
-            conn_inc--;
-            if (get_conn_count() == 0)
-                checked_fd_clr(STDIN_FILENO, &master_readfds);
-
+            close_fd(fdn, n == 0);
             return n;
         }
         else {
@@ -706,28 +739,13 @@ static void read_and_broadcast(int recv_fd)
             n = ncat_recv(fdn, buf, sizeof(buf), &pending);
 
             if (n <= 0) {
-                if (o.debug)
-                    logdebug("Closing connection.\n");
-#ifdef HAVE_OPENSSL
-                if (o.ssl && fdn->ssl) {
-                    if (n == 0)
-                        SSL_shutdown(fdn->ssl);
-                    SSL_free(fdn->ssl);
+                /* return value can be 0 without meaning EOF in some cases such as SSL
+                 * renegotiations that require read/write socket operations but do not
+                 * have any application data. */
+                if(n == 0 && fdn->lasterr != 0) {
+                    continue; /* Check pending */
                 }
-#endif
-                close(recv_fd);
-                checked_fd_clr(recv_fd, &master_readfds);
-                rm_fd(&client_fdlist, recv_fd);
-                checked_fd_clr(recv_fd, &master_broadcastfds);
-                rm_fd(&broadcast_fdlist, recv_fd);
-
-                conn_inc--;
-                if (conn_inc == 0)
-                    checked_fd_clr(STDIN_FILENO, &master_readfds);
-
-                if (o.chat)
-                    chat_announce_disconnect(recv_fd);
-
+                close_fd(fdn, n == 0);
                 return;
             }
         }

ncat/util.c

@@ -635,6 +635,7 @@ int rm_fd(fd_list_t *fdl, int fd)
         logdebug("Swapping fd[%d] (%d) with fd[%d] (%d)\n",
                  found, fdl->fds[found].fd, last - 1, fdl->fds[last - 1].fd);
     fdl->fds[found] = fdl->fds[last - 1];
+    fdl->state++;
 
     fdl->nfds--;
 
@@ -672,6 +673,7 @@ void init_fdlist(fd_list_t *fdl, int maxfds)
     fdl->nfds = 0;
     fdl->fdmax = -1;
     fdl->maxfds = maxfds;
+    fdl->state = 0;
 
     if (o.debug > 1)
         logdebug("Initialized fdlist with %d maxfds\n", maxfds);
@@ -682,6 +684,7 @@ void free_fdlist(fd_list_t *fdl)
     free(fdl->fds);
     fdl->nfds = 0;
     fdl->fdmax = -1;
+    fdl->state = 0;
 }
 
 

ncat/util.h

@@ -152,6 +152,7 @@ void ms_to_timeval(struct timeval *tv, long ms)
 
 struct fdinfo {
     int fd;
+    int lasterr;
     union sockaddr_u remoteaddr;
     socklen_t ss_len;
 #ifdef HAVE_OPENSSL
@@ -162,6 +163,7 @@ struct fdinfo {
 typedef struct fd_list {
     struct fdinfo *fds;
     int nfds, maxfds, fdmax;
+    int state; /* incremented each time the list is modified */
 } fd_list_t;
 
 int add_fdinfo(fd_list_t *, struct fdinfo *);