Blob Blame History Raw
From c3314d0fa0756d39cab1e9d9e3cf2e36dd6273da Mon Sep 17 00:00:00 2001
From: Petr Mensik <pemensik@redhat.com>
Date: Mon, 18 Nov 2019 16:44:58 +0100
Subject: [PATCH] Limit number of queries per TCP connection

5306.	[security]	Set a limit on the number of concurrently served
			pipelined TCP queries. (CVE-2019-6477) [GL #1264]
---
 bin/named/client.c               | 73 ++++++++++++++++++++------------
 bin/named/include/named/client.h |  5 ++-
 2 files changed, 50 insertions(+), 28 deletions(-)

diff --git a/bin/named/client.c b/bin/named/client.c
index f21a77ba52..23f70edaff 100644
--- a/bin/named/client.c
+++ b/bin/named/client.c
@@ -98,7 +98,15 @@
 #define SEND_BUFFER_SIZE		4096
 #define RECV_BUFFER_SIZE		4096
 
+#define TCP_CLIENTS_PER_CONN		23
+/*%<
+ * Number of simultaneous ns_clients_t (queries in flight) for one
+ * TCP connection.  The number was arbitrarily picked and might be
+ * changed in the future.
+ */
+
 #ifdef ISC_PLATFORM_USETHREADS
+
 #define NMCTXS				100
 /*%<
  * Number of 'mctx pools' for clients. (Should this be configurable?)
@@ -333,7 +341,7 @@ tcpconn_init(ns_client_t *client, isc_boolean_t force) {
 	 */
 	tconn = isc_mem_allocate(ns_g_mctx, sizeof(*tconn));
 
-	isc_refcount_init(&tconn->refs, 1);
+	isc_refcount_init(&tconn->clients, 1);	/* Current client */
 	tconn->tcpquota = quota;
 	quota = NULL;
 	tconn->pipelined = ISC_FALSE;
@@ -350,14 +358,14 @@ tcpconn_init(ns_client_t *client, isc_boolean_t force) {
  */
 static void
 tcpconn_attach(ns_client_t *source, ns_client_t *target) {
-	int refs;
+	int old_clients;
 
 	REQUIRE(source->tcpconn != NULL);
 	REQUIRE(target->tcpconn == NULL);
 	REQUIRE(source->tcpconn->pipelined);
 
-	isc_refcount_increment(&source->tcpconn->refs, &refs);
-	INSIST(refs > 1);
+	isc_refcount_increment(&source->tcpconn->clients, &old_clients);
+	INSIST(old_clients > 1);
 	target->tcpconn = source->tcpconn;
 }
 
@@ -370,15 +378,15 @@ tcpconn_attach(ns_client_t *source, ns_client_t *target) {
 static void
 tcpconn_detach(ns_client_t *client) {
 	ns_tcpconn_t *tconn = NULL;
-	int refs;
+	int old_clients;
 
 	REQUIRE(client->tcpconn != NULL);
 
 	tconn = client->tcpconn;
 	client->tcpconn = NULL;
 
-	isc_refcount_decrement(&tconn->refs, &refs);
-	if (refs == 0) {
+	isc_refcount_decrement(&tconn->clients, &old_clients);
+	if (old_clients == 0) {
 		isc_quota_detach(&tconn->tcpquota);
 		isc_mem_free(ns_g_mctx, tconn);
 	}
@@ -2629,28 +2637,39 @@ client_request(isc_task_t *task, isc_event_t *event) {
 	/*
 	 * Pipeline TCP query processing.
 	 */
-	if (TCP_CLIENT(client) &&
-	    client->message->opcode != dns_opcode_query)
-	{
-		client->tcpconn->pipelined = ISC_FALSE;
-	}
-	if (TCP_CLIENT(client) && client->tcpconn->pipelined) {
-		/*
-		 * We're pipelining. Replace the client; the
-		 * replacement can read the TCP socket looking
-		 * for new messages and this one can process the
-		 * current message asynchronously.
-		 *
-		 * There will now be at least three clients using this
-		 * TCP socket - one accepting new connections,
-		 * one reading an existing connection to get new
-		 * messages, and one answering the message already
-		 * received.
-		 */
-		result = ns_client_replace(client);
-		if (result != ISC_R_SUCCESS) {
+	if (TCP_CLIENT(client)) {
+		if (client->message->opcode != dns_opcode_query) {
 			client->tcpconn->pipelined = ISC_FALSE;
 		}
+
+ 		/*
+		 * Limit the maximum number of simultaneous pipelined
+		 * queries on TCP connection to TCP_CLIENTS_PER_CONN.
+ 		 */
+		if ((isc_refcount_current(&client->tcpconn->clients)
+			    > TCP_CLIENTS_PER_CONN))
+		{
+ 			client->tcpconn->pipelined = ISC_FALSE;
+ 		}
+
+		if (client->tcpconn->pipelined) {
+			/*
+			 * We're pipelining. Replace the client; the
+			 * replacement can read the TCP socket looking
+			 * for new messages and this one can process the
+			 * current message asynchronously.
+			 *
+			 * There will now be at least three clients using this
+			 * TCP socket - one accepting new connections,
+			 * one reading an existing connection to get new
+			 * messages, and one answering the message already
+			 * received.
+			 */
+			result = ns_client_replace(client);
+			if (result != ISC_R_SUCCESS) {
+				client->tcpconn->pipelined = ISC_FALSE;
+			}
+		}
 	}
 
 	dns_opcodestats_increment(ns_g_server->opcodestats,
diff --git a/bin/named/include/named/client.h b/bin/named/include/named/client.h
index 0f54d2267b..86437ade22 100644
--- a/bin/named/include/named/client.h
+++ b/bin/named/include/named/client.h
@@ -77,7 +77,10 @@
 
 /*% reference-counted TCP connection object */
 typedef struct ns_tcpconn {
-	isc_refcount_t		refs;
+	isc_refcount_t		clients;	/* Number of clients using
+						 * this connection. Conn can
+						 * be freed if goes to 0
+						 */
 	isc_quota_t		*tcpquota;
 	isc_boolean_t		pipelined;
 } ns_tcpconn_t;
-- 
2.20.1