diff --git a/ncat/ncat_core.c b/ncat/ncat_core.c index 04a279d5f..08e235817 100644 --- a/ncat/ncat_core.c +++ b/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; } } diff --git a/ncat/ncat_listen.c b/ncat/ncat_listen.c index b865f3110..dc1959fbf 100644 --- a/ncat/ncat_listen.c +++ b/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; } } diff --git a/ncat/util.c b/ncat/util.c index 7b6d2f63b..4cfd6e504 100644 --- a/ncat/util.c +++ b/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; } diff --git a/ncat/util.h b/ncat/util.h index 8a47175da..e969bcf41 100644 --- a/ncat/util.h +++ b/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 *);