]> The Tcpdump Group git mirrors - libpcap/commitdiff
Move the SSL setup and host/port list checking to daemon_serviceloop().
authorGuy Harris <[email protected]>
Tue, 8 Jan 2019 05:21:20 +0000 (21:21 -0800)
committerGuy Harris <[email protected]>
Tue, 8 Jan 2019 05:21:20 +0000 (21:21 -0800)
That:

1) arranges that it's done only in one code path;

2) arranges that it not be done in the main connection-accepting
   thread/process if this isn't an inetd-style daemon;

3) means that we're doing the host/port list checking in
   inetd-style daemons - we weren't doing it before;

4) means that we're doing both of them after we've turned off
   non-blocking mode on Windows, not before - doing it before
   may cause the SSL setup and sending a host/port list check
   error not to work (as we won't block waiting for input or
   waiting for buffer space to be available for output).

Fix the file descriptor handling for inetd-style daemons while we're at
it; we should redirect the standard error to /dev/null - it's not
guaranteed to, for example, go to a daemon that reads your error
messages and logs them, and it could be going over the connection, which
would be a problem.

Close the control socket with sock_close() after daemon_serviceloop()
returns, in case shutting down the write side is necessary to have the
connection shut down cleanly.

rpcapd/daemon.c
rpcapd/daemon.h
rpcapd/rpcapd.c

index dc92c0ac99301d781ec26251a4317d2ec70c0fc0..88c72b96f5c0104e6cb1c62ca1f743817df3b1f4 100644 (file)
@@ -144,11 +144,14 @@ static int rpcapd_discard(SOCKET sock, SSL *, uint32 len);
 static void session_close(struct session *);
 
 int
-daemon_serviceloop(SOCKET sockctrl_in, SOCKET sockctrl_out, SSL *ssl, int isactive, int nullAuthAllowed, int uses_ssl)
+daemon_serviceloop(SOCKET sockctrl_in, SOCKET sockctrl_out,
+    int isactive, char *passiveClients, int nullAuthAllowed, int uses_ssl)
 {
        struct daemon_slpars pars;              // service loop parameters
        char errbuf[PCAP_ERRBUF_SIZE + 1];      // keeps the error string, prior to be printed
        char errmsgbuf[PCAP_ERRBUF_SIZE + 1];   // buffer for errors to send to the client
+       int host_port_ok;
+       SSL *ssl = NULL;
        int nrecv;
        struct rpcap_header header;             // RPCAP message general header
        uint32 plen;                            // payload length from header
@@ -172,14 +175,6 @@ daemon_serviceloop(SOCKET sockctrl_in, SOCKET sockctrl_out, SSL *ssl, int isacti
        struct timeval tv;                      // maximum time the select() can block waiting for data
        int retval;                             // select() return value
 
-       // Set parameters structure
-       pars.sockctrl_in = sockctrl_in;
-       pars.sockctrl_out = sockctrl_out;
-       pars.ssl = ssl;
-       pars.protocol_version = 0;              // not yet known
-       pars.isactive = isactive;               // active mode
-       pars.nullAuthAllowed = nullAuthAllowed;
-
        // We don't have a thread yet.
        threaddata.have_thread = 0;
        //
@@ -199,6 +194,86 @@ daemon_serviceloop(SOCKET sockctrl_in, SOCKET sockctrl_out, SSL *ssl, int isacti
 
        *errbuf = 0;    // Initialize errbuf
 
+#ifdef HAVE_OPENSSL
+       //
+       // We have to upgrade to TLS as soon as possible, so that the
+       // whole protocol goes through the encrypted tunnel, including
+       // early error messages.
+       //
+       // Even in active mode, the other end has to initiate the TLS
+       // handshake as we still are the server as far as TLS is concerned,
+       // so we don't check isactive.
+       //
+       if (uses_ssl)
+       {
+               ssl = ssl_promotion_rw(1, pars.sockctrl_in, pars.sockctrl_out, errbuf, PCAP_ERRBUF_SIZE);
+               if (! ssl)
+               {
+                       rpcapd_log(LOGPRIO_ERROR, "TLS handshake on control connection failed: %s",
+                           errbuf);
+                       goto end;
+               }
+       }
+#endif
+
+       // Set parameters structure
+       pars.sockctrl_in = sockctrl_in;
+       pars.sockctrl_out = sockctrl_out;
+       pars.ssl = ssl;
+       pars.protocol_version = 0;              // not yet known
+       pars.isactive = isactive;               // active mode
+       pars.nullAuthAllowed = nullAuthAllowed;
+
+       //
+       // We have a connection.
+       //
+       // If it's a passive mode connection, check whether the connecting
+       // host is among the ones allowed.
+       //
+       // In either case, we were handed a copy of the host list; free it
+       // as soon as we're done with it.
+       //
+       if (pars.isactive)
+       {
+               // Nothing to do.
+               free(passiveClients);
+               passiveClients = NULL;
+       }
+       else
+       {
+               struct sockaddr_storage from;
+               socklen_t fromlen;
+
+               //
+               // Get the address of the other end of the connection.
+               //
+               fromlen = sizeof(struct sockaddr_storage);
+               if (getpeername(pars.sockctrl_in, (struct sockaddr *)&from,
+                   &fromlen) == -1)
+               {
+                       sock_geterror("getpeername(): ", errmsgbuf, PCAP_ERRBUF_SIZE);
+                       if (rpcap_senderror(pars.sockctrl_out, pars.ssl, 0, PCAP_ERR_NETW, errmsgbuf, errbuf) == -1)
+                               rpcapd_log(LOGPRIO_ERROR, "Send to client failed: %s", errbuf);
+                       goto end;
+               }
+
+               //
+               // Are they in the list of host/port combinations we allow?
+               //
+               host_port_ok = (sock_check_hostlist(passiveClients, RPCAP_HOSTLIST_SEP, &from, errmsgbuf, PCAP_ERRBUF_SIZE) == 0);
+               free(passiveClients);
+               passiveClients = NULL;
+               if (!host_port_ok)
+               {
+                       //
+                       // Sorry, you're not on the guest list.
+                       //
+                       if (rpcap_senderror(pars.sockctrl_out, pars.ssl, 0, PCAP_ERR_HOSTNOAUTH, errmsgbuf, errbuf) == -1)
+                               rpcapd_log(LOGPRIO_ERROR, "Send to client failed: %s", errbuf);
+                       goto end;
+               }
+       }
+
        //
        // The client must first authenticate; loop until they send us a
        // message with a version we support and credentials we accept,
@@ -880,6 +955,13 @@ end:
                session = NULL;
        }
 
+#ifdef HAVE_OPENSSL
+       if (ssl)
+       {
+               SSL_free(ssl);
+       }
+#endif
+
        // Print message and return
        rpcapd_log(LOGPRIO_DEBUG, "I'm exiting from the child loop");
        rpcapd_log(LOGPRIO_DEBUG, "%s", errbuf);
index 83901e77737bdcdf3d6ac514074778d8176d8590..9e0b9c1a348f227b92a37cd372b411c95bf73cda 100644 (file)
@@ -43,8 +43,8 @@
 // Returns 1 if the client closed the control connection explicitly, 0
 // otherwise; used in active mode only.
 //
-int daemon_serviceloop(SOCKET sockctrl_in, SOCKET sockctrl_out, SSL *ssl,
-    int isactive, int nullAuthAllowed, int uses_ssl);
+int daemon_serviceloop(SOCKET sockctrl_in, SOCKET sockctrl_out,
+    int isactive, char *passiveClients, int nullAuthAllowed, int uses_ssl);
 
 void sleep_secs(int secs);
 
index 1aa83758ca59ea3fe909205bee39a5606898b31c..b7fa21f1da37b0108710d10e38f60b17c516ce33 100644 (file)
@@ -425,7 +425,8 @@ int main(int argc, char *argv[])
                }
 
                //
-               // Try to set the standard input and output to /dev/null.
+               // Try to set the standard input, output, and error
+               // to /dev/null.
                //
                devnull_fd = open("/dev/null", O_RDWR);
                if (devnull_fd != -1)
@@ -435,29 +436,25 @@ int main(int argc, char *argv[])
                        //
                        (void)dup2(devnull_fd, 0);
                        (void)dup2(devnull_fd, 1);
+                       (void)dup2(devnull_fd, 2);
                        close(devnull_fd);
                }
 
-               SSL *ssl = NULL;
-#ifdef HAVE_OPENSSL
-               if (uses_ssl)
-               {
-                       ssl = ssl_promotion_rw(1, sockctrl_in, sockctrl_out, errbuf, PCAP_ERRBUF_SIZE);
-                       if (! ssl)
-                       {
-                               rpcapd_log(LOGPRIO_ERROR, "TLS handshake on control connection failed: %s",
-                                   errbuf);
-                               exit(2);
-                       }
-               }
-#endif
                //
                // Handle this client.
                // This is passive mode, so we don't care whether we were
                // told by the client to close.
                //
-               (void)daemon_serviceloop(sockctrl_in, sockctrl_out, ssl, 0,
-                   nullAuthAllowed, uses_ssl);
+               char *hostlist_copy = strdup(hostlist);
+               if (hostlist_copy == NULL)
+               {
+                       rpcapd_log(LOGPRIO_ERROR, "Out of memory copying the host/port list");
+                       exit(0);
+               }
+               (void)daemon_serviceloop(sockctrl_in, sockctrl_out, 0,
+                   hostlist_copy, nullAuthAllowed, uses_ssl);
+
+               sock_close(sockctrl_out, NULL, 0);
 
                //
                // Nothing more to do.
@@ -1111,11 +1108,6 @@ accept_connections(void)
 //
 struct sock_copy {
        SOCKET sockctrl;
-#      ifdef HAVE_OPENSSL
-       SSL *ssl;
-#      else
-       void *ssl;
-#      endif
 };
 #endif
 
@@ -1131,8 +1123,6 @@ accept_connection(SOCKET listen_sock)
        struct sockaddr_storage from;           // generic sockaddr_storage variable
        socklen_t fromlen;                      // keeps the length of the sockaddr_storage variable
 
-       SSL *ssl = NULL;
-
 #ifdef _WIN32
        HANDLE threadId;                        // handle for the subthread
        u_long off = 0;
@@ -1175,31 +1165,6 @@ accept_connection(SOCKET listen_sock)
                return;
        }
 
-#ifdef HAVE_OPENSSL
-       /* We have to upgrade to TLS as soon as possible so that the whole protocol
-        * goes through the encrypted tunnel, including early error messages. */
-       if (uses_ssl)
-       {
-               ssl = ssl_promotion(1, sockctrl, errbuf, PCAP_ERRBUF_SIZE);
-               if (! ssl)
-               {
-                       rpcapd_log(LOGPRIO_ERROR, "TLS handshake on control connection failed: %s",
-                           errbuf);
-                       goto error;
-               }
-       }
-#endif
-
-       //
-       // We have a connection.
-       // Check whether the connecting host is among the ones allowed.
-       //
-       if (sock_check_hostlist(hostlist, RPCAP_HOSTLIST_SEP, &from, errbuf, PCAP_ERRBUF_SIZE) < 0)
-       {
-               rpcap_senderror(sockctrl, ssl, 0, PCAP_ERR_HOSTNOAUTH, errbuf, NULL);
-               goto error;
-       }
-
 #ifdef _WIN32
        //
        // Put the socket back into blocking mode; doing WSAEventSelect()
@@ -1247,23 +1212,21 @@ accept_connection(SOCKET listen_sock)
                goto error;
        }
        sock_copy->sockctrl = sockctrl;
-       sock_copy->ssl = NULL;
 
        threadId = (HANDLE)_beginthreadex(NULL, 0,
            main_passive_serviceloop_thread, (void *) sock_copy, 0, NULL);
        if (threadId == 0)
        {
-               pcap_snprintf(errbuf, PCAP_ERRBUF_SIZE, "Error creating the child thread");
-               rpcap_senderror(sockctrl, ssl, 0, PCAP_ERR_OPEN, errbuf, NULL);
+               rpcapd_log(LOG_ERROR, "Error creating the child thread");
                goto error;
        }
        CloseHandle(threadId);
-#else
+#else /* _WIN32 */
        pid = fork();
        if (pid == -1)
        {
-               pcap_snprintf(errbuf, PCAP_ERRBUF_SIZE, "Error creating the child process");
-               rpcap_senderror(sockctrl, ssl, 0, PCAP_ERR_OPEN, errbuf, NULL);
+               rpcapd_log(LOGPRIO_ERROR, "Error creating the child process: %s",
+                   strerror(errno));
                goto error;
        }
        if (pid == 0)
@@ -1295,33 +1258,29 @@ accept_connection(SOCKET listen_sock)
                // This is passive mode, so we don't care whether we were
                // told by the client to close.
                //
-               (void)daemon_serviceloop(sockctrl, sockctrl, ssl, 0,
-                   nullAuthAllowed, uses_ssl);
+               char *hostlist_copy = strdup(hostlist);
+               if (hostlist_copy == NULL)
+               {
+                       rpcapd_log(LOGPRIO_ERROR, "Out of memory copying the host/port list");
+                       exit(0);
+               }
+               (void)daemon_serviceloop(sockctrl, sockctrl, 0,
+                   hostlist_copy, nullAuthAllowed, uses_ssl);
 
-               close(sockctrl);
+               sock_close(sockctrl, NULL, 0);
 
                exit(0);
        }
 
        // I am the parent
        // Close the socket for this session (must be open only in the child)
-#ifdef HAVE_OPENSSL
-       if (ssl)
-       {
-               SSL_free(ssl);
-               ssl = NULL;
-       }
-#endif
        closesocket(sockctrl);
-#endif
+#endif /* _WIN32 */
        return;
 
 error:
 #ifdef _WIN32
        if (sock_copy) free(sock_copy);
-#endif
-#ifdef HAVE_OPENSSL
-       if (ssl) SSL_free(ssl);  // Have to be done before closing soskctrl
 #endif
        sock_close(sockctrl, NULL, 0);
 }
@@ -1347,7 +1306,6 @@ main_active(void *ptr)
        struct addrinfo hints;                  // temporary struct to keep settings needed to open the new socket
        struct addrinfo *addrinfo;              // keeps the addrinfo chain; required to open a new socket
        struct active_pars *activepars;
-       SSL *ssl = NULL;
 
        activepars = (struct active_pars *) ptr;
 
@@ -1391,28 +1349,21 @@ main_active(void *ptr)
                        continue;
                }
 
-#ifdef HAVE_OPENSSL
-               /* Even in active mode the other other end has to initiate the TLS handshake
-                * as we still are the server as far as TLS is concerned: */
-               if (uses_ssl)
+               char *hostlist_copy = strdup(hostlist);
+               if (hostlist_copy == NULL)
                {
-                       ssl = ssl_promotion(1, sockctrl, errbuf, PCAP_ERRBUF_SIZE);
-                       if (! ssl)
-                       {
-                               rpcapd_log(LOGPRIO_ERROR, "TLS handshake on control connection failed: %s",
-                                   errbuf);
-                               sock_close(sockctrl, NULL, 0);
-                               continue;
-                       }
+                       rpcapd_log(LOGPRIO_ERROR, "Out of memory copying the host/port list");
+                       activeclose = 0;
+               }
+               else
+               {
+                       //
+                       // daemon_serviceloop() will free the copy.
+                       //
+                       activeclose = daemon_serviceloop(sockctrl, sockctrl, 1,
+                           hostlist_copy, nullAuthAllowed, uses_ssl);
                }
-#endif
-
-               activeclose = daemon_serviceloop(sockctrl, sockctrl, ssl, 1,
-                   nullAuthAllowed, uses_ssl);
 
-#ifdef HAVE_OPENSSL
-               if (ssl) SSL_free(ssl);
-#endif
                sock_close(sockctrl, NULL, 0);
 
                // If the connection is closed by the user explicitely, don't try to connect to it again
@@ -1439,7 +1390,7 @@ unsigned __stdcall main_passive_serviceloop_thread(void *ptr)
        // This is passive mode, so we don't care whether we were
        // told by the client to close.
        //
-       (void)daemon_serviceloop(sock.sockctrl, sock.sockctrl, sock.ssl, 0,
+       (void)daemon_serviceloop(sock.sockctrl, sock.sockctrl, 0,
            nullAuthAllowed, uses_ssl);
 
        sock_close(sock.sockctrl, NULL, 0);