Handle the client closing the connection better.

On the server side, don't treat an immediate EOF when trying to read a
message header as an error, just silently terminate the session.  Treat
EOFs after reading *some* of the message as an error.

4 files changed, 91 insertions, 26 deletions

diff --git a/pcap-rpcap.c b/pcap-rpcap.c
index efbca407..871280e4 100644
--- a/pcap-rpcap.c
+++ b/pcap-rpcap.c
@@ -414,12 +414,17 @@ static int pcap_read_nocb_remote(pcap_t *p, struct pcap_pkthdr **pkt_header, u_c
 	if (pr->rmt_flags & PCAP_OPENFLAG_DATATX_UDP)
 	{
 		/* Read the entire message from the network */
-		if (sock_recv(pr->rmt_sockdata, netbuf, RPCAP_NETBUF_SIZE, SOCK_RECEIVEALL_NO, p->errbuf, PCAP_ERRBUF_SIZE) == -1)
+		if (sock_recv(pr->rmt_sockdata, netbuf, RPCAP_NETBUF_SIZE,
+		    SOCK_RECEIVEALL_NO|SOCK_EOF_IS_ERROR, p->errbuf,
+		    PCAP_ERRBUF_SIZE) == -1)
 			return -1;
 	}
 	else
 	{
-		if (sock_recv(pr->rmt_sockdata, netbuf, sizeof(struct rpcap_header), SOCK_RECEIVEALL_YES, p->errbuf, PCAP_ERRBUF_SIZE) == -1)
+		if (sock_recv(pr->rmt_sockdata, netbuf,
+		    sizeof(struct rpcap_header),
+		    SOCK_RECEIVEALL_YES|SOCK_EOF_IS_ERROR, p->errbuf,
+		    PCAP_ERRBUF_SIZE) == -1)
 			return -1;
 	}
 	header->plen = ntohl(header->plen);
@@ -2812,7 +2817,12 @@ int pcap_remoteact_list(char *hostlist, char sep, int size, char *errbuf)
  */
 static int rpcap_recv_msg_header(SOCKET sock, struct rpcap_header *header, char *errbuf)
 {
-	if (sock_recv(sock, (char *) header, sizeof(struct rpcap_header), SOCK_RECEIVEALL_YES, errbuf, PCAP_ERRBUF_SIZE) == -1)
+	int nrecv;
+
+	nrecv = sock_recv(sock, (char *) header, sizeof(struct rpcap_header),
+	    SOCK_RECEIVEALL_YES|SOCK_EOF_IS_ERROR, errbuf,
+	    PCAP_ERRBUF_SIZE);
+	if (nrecv == -1)
 	{
 		/* Network error. */
 		return -1;
@@ -2969,7 +2979,7 @@ static int rpcap_recv(SOCKET sock, void *buffer, size_t toread, uint32 *plen, ch
 		return -1;
 	}
 	nread = sock_recv(sock, buffer, toread,
-	    SOCK_RECEIVEALL_YES, errbuf, PCAP_ERRBUF_SIZE);
+	    SOCK_RECEIVEALL_YES|SOCK_EOF_IS_ERROR, errbuf, PCAP_ERRBUF_SIZE);
 	if (nread == -1)
 	{
 		return -1;
@@ -2991,7 +3001,9 @@ static void rpcap_msg_err(SOCKET sockctrl, uint32 plen, char *remote_errbuf)
 		 * Message is too long; just read as much of it as we
 		 * can into the buffer provided, and discard the rest.
 		 */
-		if (sock_recv(sockctrl, remote_errbuf, PCAP_ERRBUF_SIZE - 1, SOCK_RECEIVEALL_YES, errbuf, PCAP_ERRBUF_SIZE) == -1)
+		if (sock_recv(sockctrl, remote_errbuf, PCAP_ERRBUF_SIZE - 1,
+		    SOCK_RECEIVEALL_YES|SOCK_EOF_IS_ERROR, errbuf,
+		    PCAP_ERRBUF_SIZE) == -1)
 		{
 			// Network error.
 			pcap_snprintf(remote_errbuf, PCAP_ERRBUF_SIZE, "Read of error message from client failed: %s", errbuf);
@@ -3015,7 +3027,9 @@ static void rpcap_msg_err(SOCKET sockctrl, uint32 plen, char *remote_errbuf)
 	}
 	else
 	{
-		if (sock_recv(sockctrl, remote_errbuf, plen, SOCK_RECEIVEALL_YES, errbuf, PCAP_ERRBUF_SIZE) == -1)
+		if (sock_recv(sockctrl, remote_errbuf, plen,
+		    SOCK_RECEIVEALL_YES|SOCK_EOF_IS_ERROR, errbuf,
+		    PCAP_ERRBUF_SIZE) == -1)
 		{
 			// Network error.
 			pcap_snprintf(remote_errbuf, PCAP_ERRBUF_SIZE, "Read of error message from client failed: %s", errbuf);
diff --git a/rpcapd/daemon.c b/rpcapd/daemon.c
index 4cee36fa..455cdb2e 100755
--- a/rpcapd/daemon.c
+++ b/rpcapd/daemon.c
@@ -128,6 +128,7 @@ void *daemon_serviceloop(void *ptr)
 {
 	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 nrecv;
 	struct rpcap_header header;		// RPCAP message general header
 	uint32 plen;				// payload length from header
 	int authenticated = 0;			// 1 if the client has successfully authenticated
@@ -216,11 +217,17 @@ void *daemon_serviceloop(void *ptr)
 		//
 		// Read the message header from the client.
 		//
-		if (rpcapd_recv_msg_header(pars->sockctrl, &header) == -1)
+		nrecv = rpcapd_recv_msg_header(pars->sockctrl, &header);
+		if (nrecv == -1)
 		{
 			// Fatal error.
 			goto end;
 		}
+		if (nrecv == -2)
+		{
+			// Client closed the connection.
+			goto end;
+		}
 
 		plen = header.plen;
 
@@ -489,11 +496,17 @@ void *daemon_serviceloop(void *ptr)
 		//
 		// Read the message header from the client.
 		//
-		if (rpcapd_recv_msg_header(pars->sockctrl, &header) == -1)
+		nrecv = rpcapd_recv_msg_header(pars->sockctrl, &header);
+		if (nrecv == -1)
 		{
 			// Fatal error.
 			goto end;
 		}
+		if (nrecv == -2)
+		{
+			// Client closed the connection.
+			goto end;
+		}
 
 		plen = header.plen;
 
@@ -841,7 +854,9 @@ int daemon_msg_err(SOCKET sockctrl, uint32 plen)
 		 * Message is too long; just read as much of it as we
 		 * can into the buffer provided, and discard the rest.
 		 */
-		if (sock_recv(sockctrl, remote_errbuf, PCAP_ERRBUF_SIZE - 1, SOCK_RECEIVEALL_YES, errbuf, PCAP_ERRBUF_SIZE) == -1)
+		if (sock_recv(sockctrl, remote_errbuf, PCAP_ERRBUF_SIZE - 1,
+		    SOCK_RECEIVEALL_YES|SOCK_EOF_IS_ERROR, errbuf,
+		    PCAP_ERRBUF_SIZE) == -1)
 		{
 			// Network error.
 			rpcapd_log(LOGPRIO_ERROR, "Read from client failed: %s", errbuf);
@@ -865,7 +880,9 @@ int daemon_msg_err(SOCKET sockctrl, uint32 plen)
 	}
 	else
 	{
-		if (sock_recv(sockctrl, remote_errbuf, plen, SOCK_RECEIVEALL_YES, errbuf, PCAP_ERRBUF_SIZE) == -1)
+		if (sock_recv(sockctrl, remote_errbuf, plen,
+		    SOCK_RECEIVEALL_YES|SOCK_EOF_IS_ERROR, errbuf,
+		    PCAP_ERRBUF_SIZE) == -1)
 		{
 			// Network error.
 			rpcapd_log(LOGPRIO_ERROR, "Read from client failed: %s", errbuf);
@@ -1402,7 +1419,8 @@ static int daemon_msg_open_req(struct daemon_slpars *pars, uint32 plen, char *so
 		goto error;
 	}
 
-	nread = sock_recv(pars->sockctrl, source, plen, SOCK_RECEIVEALL_YES, errbuf, PCAP_ERRBUF_SIZE);
+	nread = sock_recv(pars->sockctrl, source, plen,
+	    SOCK_RECEIVEALL_YES|SOCK_EOF_IS_ERROR, errbuf, PCAP_ERRBUF_SIZE);
 	if (nread == -1)
 	{
 		rpcapd_log(LOGPRIO_ERROR, "Read from client failed: %s", errbuf);
@@ -2292,14 +2310,22 @@ void sleep_secs(int secs)
  */
 static int rpcapd_recv_msg_header(SOCKET sock, struct rpcap_header *headerp)
 {
+	int nread;
 	char errbuf[PCAP_ERRBUF_SIZE];		// buffer for network errors
 
-	if (sock_recv(sock, (char *) headerp, sizeof(struct rpcap_header), SOCK_RECEIVEALL_YES, errbuf, PCAP_ERRBUF_SIZE) == -1)
+	nread = sock_recv(sock, (char *) headerp, sizeof(struct rpcap_header),
+	    SOCK_RECEIVEALL_YES|SOCK_EOF_ISNT_ERROR, errbuf, PCAP_ERRBUF_SIZE);
+	if (nread == -1)
 	{
 		// Network error.
 		rpcapd_log(LOGPRIO_ERROR, "Read from client failed: %s", errbuf);
 		return -1;
 	}
+	if (nread == 0)
+	{
+		// Immediate EOF; that's treated like a close message.
+		return -2;
+	}
 	headerp->plen = ntohl(headerp->plen);
 	return 0;
 }
@@ -2325,7 +2351,7 @@ static int rpcapd_recv(SOCKET sock, char *buffer, size_t toread, uint32 *plen, c
 		return -2;
 	}
 	nread = sock_recv(sock, buffer, toread,
-	    SOCK_RECEIVEALL_YES, errbuf, PCAP_ERRBUF_SIZE);
+	    SOCK_RECEIVEALL_YES|SOCK_EOF_IS_ERROR, errbuf, PCAP_ERRBUF_SIZE);
 	if (nread == -1)
 	{
 		rpcapd_log(LOGPRIO_ERROR, "Read from client failed: %s", errbuf);
diff --git a/sockutils.c b/sockutils.c
index 509a8b68..7d0f9eb7 100644
--- a/sockutils.c
+++ b/sockutils.c
@@ -690,9 +690,19 @@ int sock_bufferize(const char *buffer, int size, char *tempbuf, int *offset, int
  * \param size: size of the allocated buffer. WARNING: this indicates the number of bytes
  * that we are expecting to be read.
  *
- * \param receiveall: if '0' (or SOCK_RECEIVEALL_NO), it returns as soon as some data
- * is ready; otherwise, (or SOCK_RECEIVEALL_YES) it waits until 'size' data has been
- * received (in case the socket does not have enough data available).
+ * \param flags:
+ *
+ *   SOCK_RECEIVALL_XXX:
+ *
+ * 	if SOCK_RECEIVEALL_NO, return as soon as some data is ready
+ *	if SOCK_RECEIVALL_YES, wait until 'size' data has been
+ *	    received (in case the socket does not have enough data available).
+ *
+ *   SOCK_EOF_XXX:
+ *
+ *	if SOCK_EOF_ISNT_ERROR, if the first read returns 0, just return 0,
+ *	    and return an error on any subsequent read that returns 0;
+ *	if SOCK_EOF_IS_ERROR, if any read returns 0, return an error.
  *
  * \param errbuf: a pointer to an user-allocated buffer that will contain the complete
  * error message. This buffer has to be at least 'errbuflen' in length.
@@ -717,7 +727,7 @@ int sock_bufferize(const char *buffer, int size, char *tempbuf, int *offset, int
 typedef int ssize_t;
 #endif
 
-int sock_recv(SOCKET sock, void *buffer, size_t size, int receiveall,
+int sock_recv(SOCKET sock, void *buffer, size_t size, int flags,
     char *errbuf, int errbuflen)
 {
 	char *bufp = buffer;
@@ -762,19 +772,30 @@ int sock_recv(SOCKET sock, void *buffer, size_t size, int receiveall,
 
 		if (nread == 0)
 		{
-			if (errbuf)
+			if ((flags & SOCK_EOF_IS_ERROR) ||
+			    (remaining != (int) size))
 			{
-				pcap_snprintf(errbuf, errbuflen,
-				    "The other host terminated the connection.");
+				/*
+				 * Either we've already read some data,
+				 * or we're always supposed to return
+				 * an error on EOF.
+				 */
+				if (errbuf)
+				{
+					pcap_snprintf(errbuf, errbuflen,
+					    "The other host terminated the connection.");
+				}
+				return -1;
 			}
-			return -1;
+			else
+				return 0;
 		}
 
 		/*
 		 * Do we want to read the amount requested, or just return
 		 * what we got?
 		 */
-		if (!receiveall)
+		if (!(flags & SOCK_RECEIVEALL_YES))
 		{
 			/*
 			 * Just return what we got.
diff --git a/sockutils.h b/sockutils.h
index 87f9c65d..20127f03 100644
--- a/sockutils.h
+++ b/sockutils.h
@@ -168,10 +168,14 @@ int WSAAPI getnameinfo(const struct sockaddr*,socklen_t,char*,DWORD,
 /* 'server' flag; it opens a server socket */
 #define SOCKOPEN_SERVER 1
 
-/* Changes the behaviour of the sock_recv(); it does not wait to receive all data */
-#define SOCK_RECEIVEALL_NO 0
-/* Changes the behaviour of the sock_recv(); it waits to receive all data */
-#define SOCK_RECEIVEALL_YES 1
+/*
+ * Flags for sock_recv().
+ */
+#define SOCK_RECEIVEALL_NO	0x00000000	/* Don't wait to receive all data */
+#define SOCK_RECEIVEALL_YES	0x00000001	/* Wait to receive all data */
+
+#define SOCK_EOF_ISNT_ERROR	0x00000000	/* Return 0 on EOF */
+#define SOCK_EOF_IS_ERROR	0x00000002	/* Return an error on EOF */
 
 /*
  * \}