From 165181b794e185af8621300e2a68777a04af8358 Mon Sep 17 00:00:00 2001

From: Petr Mensik <pemensik@redhat.com>

Date: Fri, 15 May 2020 14:55:26 +0200

Subject: [PATCH] CVE-2020-8616

5395.	[security]	Further limit the number of queries that can be

			triggered from a request.  Root and TLD servers

			are no longer exempt from max-recursion-queries.

			Fetches for missing name server address records

			are limited to 4 for any domain. (CVE-2020-8616)

			[GL #1388]

---

 lib/dns/adb.c             | 33 +++++++++++++----------

 lib/dns/include/dns/adb.h |  4 +++

 lib/dns/resolver.c        | 55 ++++++++++++++++++++++++++-------------

 3 files changed, 60 insertions(+), 32 deletions(-)

diff --git a/lib/dns/adb.c b/lib/dns/adb.c

index 3d12221..ec183d0 100644

--- a/lib/dns/adb.c

+++ b/lib/dns/adb.c

@@ -404,14 +404,13 @@ static void log_quota(dns_adbentry_t *entry, const char *fmt, ...)

  */

 #define FIND_WANTEVENT(fn)      (((fn)->options & DNS_ADBFIND_WANTEVENT) != 0)

 #define FIND_WANTEMPTYEVENT(fn) (((fn)->options & DNS_ADBFIND_EMPTYEVENT) != 0)

-#define FIND_AVOIDFETCHES(fn)   (((fn)->options & DNS_ADBFIND_AVOIDFETCHES) \

-				 != 0)

-#define FIND_STARTATZONE(fn)    (((fn)->options & DNS_ADBFIND_STARTATZONE) \

-				 != 0)

-#define FIND_HINTOK(fn)         (((fn)->options & DNS_ADBFIND_HINTOK) != 0)

-#define FIND_GLUEOK(fn)         (((fn)->options & DNS_ADBFIND_GLUEOK) != 0)

-#define FIND_HAS_ADDRS(fn)      (!ISC_LIST_EMPTY((fn)->list))

-#define FIND_RETURNLAME(fn)     (((fn)->options & DNS_ADBFIND_RETURNLAME) != 0)

+#define FIND_AVOIDFETCHES(fn)	(((fn)->options & DNS_ADBFIND_AVOIDFETCHES) != 0)

+#define FIND_STARTATZONE(fn)	(((fn)->options & DNS_ADBFIND_STARTATZONE) != 0)

+#define FIND_HINTOK(fn)		(((fn)->options & DNS_ADBFIND_HINTOK) != 0)

+#define FIND_GLUEOK(fn)		(((fn)->options & DNS_ADBFIND_GLUEOK) != 0)

+#define FIND_HAS_ADDRS(fn)	(!ISC_LIST_EMPTY((fn)->list))

+#define FIND_RETURNLAME(fn)	(((fn)->options & DNS_ADBFIND_RETURNLAME) != 0)

+#define FIND_NOFETCH(fn)	(((fn)->options & DNS_ADBFIND_NOFETCH) != 0)

 /*

  * These are currently used on simple unsigned ints, so they are

@@ -3155,21 +3154,26 @@ dns_adb_createfind2(dns_adb_t *adb, isc_task_t *task, isc_taskaction_t action,

 		 * Listen to negative cache hints, and don't start

 		 * another query.

 		 */

-		if (NCACHE_RESULT(result) || AUTH_NX(result))

+		if (NCACHE_RESULT(result) || AUTH_NX(result)) {

 			goto fetch;

+		}

-		if (!NAME_FETCH_V6(adbname))

+		if (!NAME_FETCH_V6(adbname)) {

 			wanted_fetches |= DNS_ADBFIND_INET6;

+		}

 	}

  fetch:

 	if ((WANT_INET(wanted_addresses) && NAME_HAS_V4(adbname)) ||

 	    (WANT_INET6(wanted_addresses) && NAME_HAS_V6(adbname)))

+	{

 		have_address = true;

-	else

+	} else {

 		have_address = false;

-	if (wanted_fetches != 0 &&

-	    ! (FIND_AVOIDFETCHES(find) && have_address)) {

+	}

+	if (wanted_fetches != 0 && !(FIND_AVOIDFETCHES(find) && have_address) &&

+	    !FIND_NOFETCH(find))

+	{

 		/*

 		 * We're missing at least one address family.  Either the

 		 * caller hasn't instructed us to avoid fetches, or we don't

@@ -3177,8 +3181,9 @@ dns_adb_createfind2(dns_adb_t *adb, isc_task_t *task, isc_taskaction_t action,

 		 * be acceptable so we have to launch fetches.

 		 */

-		if (FIND_STARTATZONE(find))

+		if (FIND_STARTATZONE(find)) {

 			start_at_zone = true;

+		}

 		/*

 		 * Start V4.

diff --git a/lib/dns/include/dns/adb.h b/lib/dns/include/dns/adb.h

index ca35bac..3e27c9e 100644

--- a/lib/dns/include/dns/adb.h

+++ b/lib/dns/include/dns/adb.h

@@ -207,6 +207,10 @@ struct dns_adbfind {

  *      lame for this query.

  */

 #define DNS_ADBFIND_OVERQUOTA		0x00000400

+/*%

+ *	Don't perform a fetch even if there are no address records available.

+ */

+#define DNS_ADBFIND_NOFETCH		0x00000800

 /*%

  * The answers to queries come back as a list of these.

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c

index 164fc01..79ad212 100644

--- a/lib/dns/resolver.c

+++ b/lib/dns/resolver.c

@@ -173,6 +173,14 @@

 #define DEFAULT_MAX_QUERIES 75

 #endif

+/*

+ * After NS_FAIL_LIMIT attempts to fetch a name server address,

+ * if the number of addresses in the NS RRset exceeds NS_RR_LIMIT,

+ * stop trying to fetch, in order to avoid wasting resources.

+ */

+#define NS_FAIL_LIMIT 4

+#define NS_RR_LIMIT   5

+

 /* Number of hash buckets for zone counters */

 #ifndef RES_DOMAIN_BUCKETS

 #define RES_DOMAIN_BUCKETS	523

@@ -3121,8 +3129,7 @@ sort_finds(dns_adbfindlist_t *findlist, unsigned int bias) {

 static void

 findname(fetchctx_t *fctx, dns_name_t *name, in_port_t port,

 	 unsigned int options, unsigned int flags, isc_stdtime_t now,

-	 bool *overquota, bool *need_alternate)

-{

+	 bool *overquota, bool *need_alternate, unsigned int *no_addresses) {

 	dns_adbaddrinfo_t *ai;

 	dns_adbfind_t *find;

 	dns_resolver_t *res;

@@ -3210,7 +3217,12 @@ findname(fetchctx_t *fctx, dns_name_t *name, in_port_t port,

 			      find->result_v6 != DNS_R_NXDOMAIN) ||

 			     (res->dispatches6 == NULL &&

 			      find->result_v4 != DNS_R_NXDOMAIN)))

+			{

 				*need_alternate = true;

+			}

+			if (no_addresses != NULL) {

+				(*no_addresses)++;

+			}

 		} else {

 			if ((find->options & DNS_ADBFIND_OVERQUOTA) != 0) {

 				if (overquota != NULL)

@@ -3261,6 +3273,7 @@ fctx_getaddresses(fetchctx_t *fctx, bool badcache) {

 	dns_rdata_ns_t ns;

 	bool need_alternate = false;

 	bool all_spilled = true;

+	unsigned int no_addresses = 0;

 	FCTXTRACE5("getaddresses", "fctx->depth=", fctx->depth);

@@ -3428,20 +3441,28 @@ fctx_getaddresses(fetchctx_t *fctx, bool badcache) {

 		 * Extract the name from the NS record.

 		 */

 		result = dns_rdata_tostruct(&rdata, &ns, NULL);

-		if (result != ISC_R_SUCCESS)

+		if (result != ISC_R_SUCCESS) {

 			continue;

+		}

-		findname(fctx, &ns.name, 0, stdoptions, 0, now,

-			 &overquota, &need_alternate);

+		if (no_addresses > NS_FAIL_LIMIT &&

+		    dns_rdataset_count(&fctx->nameservers) > NS_RR_LIMIT)

+		{

+			stdoptions |= DNS_ADBFIND_NOFETCH;

+		}

+		findname(fctx, &ns.name, 0, stdoptions, 0, now, &overquota,

+			 &need_alternate, &no_addresses);

-		if (!overquota)

+		if (!overquota) {

 			all_spilled = false;

+		}

 		dns_rdata_reset(&rdata);

 		dns_rdata_freestruct(&ns);

 	}

-	if (result != ISC_R_NOMORE)

+	if (result != ISC_R_NOMORE) {

 		return (result);

+	}

 	/*

 	 * Do we need to use 6 to 4?

@@ -3456,7 +3477,7 @@ fctx_getaddresses(fetchctx_t *fctx, bool badcache) {

 			if (!a->isaddress) {

 				findname(fctx, &a->_u._n.name, a->_u._n.port,

 					 stdoptions, FCTX_ADDRINFO_FORWARDER,

-					 now, NULL, NULL);

+					 now, NULL, NULL, NULL);

 				continue;

 			}

 			if (isc_sockaddr_pf(&a->_u.addr) != family)

@@ -3818,16 +3839,14 @@ fctx_try(fetchctx_t *fctx, bool retrying, bool badcache) {

 		}

 	}

-	if (dns_name_countlabels(&fctx->domain) > 2) {

-		result = isc_counter_increment(fctx->qc);

-		if (result != ISC_R_SUCCESS) {

-			isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER,

-				      DNS_LOGMODULE_RESOLVER, ISC_LOG_DEBUG(3),

-				      "exceeded max queries resolving '%s'",

-				      fctx->info);

-			fctx_done(fctx, DNS_R_SERVFAIL, __LINE__);

-			return;

-		}

+	result = isc_counter_increment(fctx->qc);

+	if (result != ISC_R_SUCCESS) {

+		isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER,

+			      DNS_LOGMODULE_RESOLVER, ISC_LOG_DEBUG(3),

+			      "exceeded max queries resolving '%s'",

+			      fctx->info);

+		fctx_done(fctx, DNS_R_SERVFAIL, __LINE__);

+		return;

 	}

 	bucketnum = fctx->bucketnum;

-- 

2.21.1