From 05d3d94613168187cbf7d54ac6de345bb75910dd Mon Sep 17 00:00:00 2001
From: Luke Bratch <luke@bratch.co.uk>
Date: Tue, 9 Jul 2019 22:36:35 +0100
Subject: Avoid SSL_accept() blocking if the client fails to do TLS
 negotiation.

---
 blabouncer.c | 51 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 21 deletions(-)

(limited to 'blabouncer.c')

diff --git a/blabouncer.c b/blabouncer.c
index 73bef8c..3a5924f 100644
--- a/blabouncer.c
+++ b/blabouncer.c
@@ -387,6 +387,7 @@ void dochat(int *serversockfd, int *clientsockfd, struct settings *settings) {
     clients[i].fd = 0;
     clients[i].authed = 0;
     clients[i].registered = 0;
+    clients[i].pendingsslaccept = 0;
     clients[i].pendingchannelmode = 0;
     clients[i].pendingban = 0;
     clients[i].pendingwho = 0;
@@ -739,30 +740,32 @@ void dochat(int *serversockfd, int *clientsockfd, struct settings *settings) {
             if (newfd > fdmax) {    // keep track of the max
               fdmax = newfd;
             }
-            // Find a free element in the clients array and set to new fd value
+            // Find a free element in the clients array and set to new fd value (plus start SSL_accept() if using client TLS)
             for (int j = 0; j < MAXCLIENTS; j++) {
-                if (clients[j].fd == 0) {
-                  clients[j].fd = newfd;
-                  // Ensure its authentication status is set to 0
-                  clients[j].authed = 0;
-                  // If using TLS then...
-                  if (settings->clienttls) {
-                    // ...set as OpenSSL FD and SSL_accept it
-                    clients[j].ssl = SSL_new(ctx);
-                    SSL_set_fd(clients[j].ssl, newfd);
-                    if (SSL_accept(clients[j].ssl) <= 0) {
-                      char* errstr = openssl_error_string();
-                      debugprint(DEBUG_CRIT, "SSL_accept failed for fd %d - %s", clients[j].fd, errstr);
-                      if (errstr != NULL) free(errstr);
-                    } else {
-                      debugprint(DEBUG_FULL, "SSL_accept succeeded for fd %d.\n", clients[j].fd);
-                    }
-                  } else {
-                    // If not using TLS then cast newfd to SSL* even though it will just be the original newfd int really
-                    clients[j].ssl = (SSL*)(long int)newfd;
+              if (clients[j].fd == 0) {
+                clients[j].fd = newfd;
+                // Ensure its authentication status is set to 0
+                clients[j].authed = 0;
+                // If using TLS then...
+                if (settings->clienttls) {
+                  // ...set as OpenSSL FD and SSL_accept it
+                  clients[j].ssl = SSL_new(ctx);
+                  SSL_set_fd(clients[j].ssl, newfd);
+                  // Set that we are pending SSL_accept()
+                  clients[j].pendingsslaccept = 1;
+                  // Change the socket to non-blocking so SSL_accept() can't hang (e.g. if it disconnects mid-conversation or if it's some bad client)
+                  if (!fd_toggle_blocking(clients[j].fd, 0)) {
+                    debugprint(DEBUG_CRIT, "fd_toggle_blocking on failed for fd %d: %s.\n", clients[j].fd, strerror(errno));
+                    disconnectclient(clients[j].fd, clients, &ircdstate, settings, clientcodes);
                   }
-                  break;
+                  // Try to SSL_accept(), not interested in return code here since openssl_accept() does the right thing.
+                  openssl_accept(clients[j].fd, clients, &ircdstate, settings, clientcodes);
+                } else {
+                  // If not using TLS then cast newfd to SSL* even though it will just be the original newfd int really
+                  clients[j].ssl = (SSL*)(long int)newfd;
                 }
+                break;
+              }
             }
             // TODO - Handle the "find a free element" loop not finding a free element
             debugprint(DEBUG_FULL, "bouncer-client: new connection from %s on socket %d\n",
@@ -780,8 +783,14 @@ void dochat(int *serversockfd, int *clientsockfd, struct settings *settings) {
             sendtoallclients(clients, alertmsg, 0, settings);
             debugprint(DEBUG_FULL, "bouncer-client: total client connections: %d\n", numclients(clients));
           }
+        // If using client TLS and still pending SSL_accept() then re-try SSL_accept() (it can't be real client data yet)
+        } else if (clients[arrindex(clients, i)].pendingsslaccept) {
+          debugprint(DEBUG_FULL, "...previous connection pending SSL_accept!\n");
+          // Try to SSL_accept(), not interested in return code here since openssl_accept() does the right thing.
+          openssl_accept(clients[arrindex(clients, i)].fd, clients, &ircdstate, settings, clientcodes);
         } else {
           debugprint(DEBUG_FULL, "...previous connection!\n");
+
           // handle data from a client
           if ((clientnumbytes = sockread(clients[arrindex(clients, i)].ssl, clientbuf, MAXRCVSIZE - 1, settings->clienttls)) <= 0) {
             // got error or connection closed by client
-- 
cgit v1.2.3