Move the SSL setup and host/port list checking to daemon_serviceloop().

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.

3 files changed, 133 insertions, 100 deletions

diff --git a/rpcapd/daemon.c b/rpcapd/daemon.c
index dc92c0ac..88c72b96 100644
--- a/rpcapd/daemon.c
+++ b/rpcapd/daemon.c
@@ -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);
diff --git a/rpcapd/daemon.h b/rpcapd/daemon.h
index 83901e77..9e0b9c1a 100644
--- a/rpcapd/daemon.h
+++ b/rpcapd/daemon.h
@@ -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);
 
diff --git a/rpcapd/rpcapd.c b/rpcapd/rpcapd.c
index 1aa83758..b7fa21f1 100644
--- a/rpcapd/rpcapd.c
+++ b/rpcapd/rpcapd.c
@@ -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,34 +1258,30 @@ 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);