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