From 3c22fb00593e78ff27210b2cb708128940077a49 Mon Sep 17 00:00:00 2001

From: Mark Andrews <marka@isc.org>

Date: Tue, 1 Mar 2022 09:48:05 +1100

Subject: [PATCH] Add additional name checks when using a forwarder

When using a forwarder, check that the owner name of response

records are within the bailiwick of the forwarded name space.

(cherry picked from commit e8df2802ac62016ea68585893eb4310fc3329028)

Check that the forward declaration is unchanged and not overridden

If we are using a fowarder, in addition to checking that names to

be cached are subdomains of the forwarded namespace, we must also

check that there are no subsidiary forwarded namespaces which would

take precedence. To be safe, we don't cache any responses if the

forwarding configuration has changed since the query was sent.

(cherry picked from commit 590f8698fc876d6d72f75cf35359e7546c3af972)

Check cached names for possible "forward only" clause

When caching additional and glue data *not* from a forwarder, we must

check that there is no "forward only" clause covering the owner name

that would take precedence.  Such names would normally be allowed by

baliwick rules, but a "forward only" zone introduces a new baliwick

scope.

(cherry picked from commit 4a144fae16e70517be894a971cef1d085ee68ebe)

Look for zones deeper than the current domain or forward name

When caching glue, we need to ensure that there is no closer

source of truth for the name. If the owner name for the glue

record would be answered by a locally configured zone, do not

cache.

(cherry picked from commit 42f8c538d3fb9d075b98d82688aeb71621798754)

Avoid use of compound literals

Compound literals are not used in BIND 9.11, in order to ensure backward

compatibility with ancient compilers.  Rework the relevant parts of the

BIND 9.11 backport of the CVE-2021-25220 fix so that compound literals

are not used.

(cherry picked from commit d4b1efbcbd4dfb8c6ef303968992440c5bdeed15)

---

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

 1 file changed, 125 insertions(+), 5 deletions(-)

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

index 7231e25..a55b5fd 100644

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

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

@@ -66,6 +66,7 @@

 #include <dns/stats.h>

 #include <dns/tsig.h>

 #include <dns/validator.h>

+#include <dns/zone.h>

 #ifdef WANT_QUERYTRACE

 #define RTRACE(m)       isc_log_write(dns_lctx, \

@@ -318,6 +319,8 @@ struct fetchctx {

 	isc_boolean_t			ns_ttl_ok;

 	isc_uint32_t			ns_ttl;

 	isc_counter_t *			qc;

+	dns_fixedname_t			fwdfname;

+	dns_name_t			*fwdname;

 	/*%

 	 * The number of events we're waiting for.

@@ -3331,6 +3334,7 @@ fctx_getaddresses(fetchctx_t *fctx, isc_boolean_t badcache) {

 		if (result == ISC_R_SUCCESS) {

 			fwd = ISC_LIST_HEAD(forwarders->fwdrs);

 			fctx->fwdpolicy = forwarders->fwdpolicy;

+			dns_name_copy(domain, fctx->fwdname, NULL);

 			if (fctx->fwdpolicy == dns_fwdpolicy_only &&

 			    isstrictsubdomain(domain, &fctx->domain)) {

 				fcount_decr(fctx);

@@ -4363,6 +4367,9 @@ fctx_create(dns_resolver_t *res, dns_name_t *name, dns_rdatatype_t type,

 	fctx->restarts = 0;

 	fctx->querysent = 0;

 	fctx->referrals = 0;

+

+	fctx->fwdname = dns_fixedname_initname(&fctx->fwdfname);

+

 	TIME_NOW(&fctx->start);

 	fctx->timeouts = 0;

 	fctx->lamecount = 0;

@@ -4415,8 +4422,10 @@ fctx_create(dns_resolver_t *res, dns_name_t *name, dns_rdatatype_t type,

 		domain = dns_fixedname_initname(&fixed);

 		result = dns_fwdtable_find2(fctx->res->view->fwdtable, fwdname,

 					    domain, &forwarders);

-		if (result == ISC_R_SUCCESS)

+		if (result == ISC_R_SUCCESS) {

 			fctx->fwdpolicy = forwarders->fwdpolicy;

+			dns_name_copy(domain, fctx->fwdname, NULL);

+		}

 		if (fctx->fwdpolicy != dns_fwdpolicy_only) {

 			/*

@@ -6165,6 +6174,112 @@ mark_related(dns_name_t *name, dns_rdataset_t *rdataset,

 		rdataset->attributes |= DNS_RDATASETATTR_EXTERNAL;

 }

+/*

+ * Returns true if 'name' is external to the namespace for which

+ * the server being queried can answer, either because it's not a

+ * subdomain or because it's below a forward declaration or a

+ * locally served zone.

+ */

+static inline isc_boolean_t

+name_external(dns_name_t *name, dns_rdatatype_t type, fetchctx_t *fctx) {

+	isc_result_t result;

+	dns_forwarders_t *forwarders = NULL;

+	dns_fixedname_t fixed, zfixed;

+	dns_name_t *fname = dns_fixedname_initname(&fixed);

+	dns_name_t *zfname = dns_fixedname_initname(&zfixed);

+	dns_name_t *apex = NULL;

+	dns_name_t suffix;

+	dns_zone_t *zone = NULL;

+	unsigned int labels;

+	dns_namereln_t rel;

+	/*

+	 * The following two variables do not influence code flow; they are

+	 * only necessary for calling dns_name_fullcompare().

+	 */

+	int _orderp = 0;

+	unsigned int _nlabelsp = 0;

+

+	apex = ISFORWARDER(fctx->addrinfo) ? fctx->fwdname : &fctx->domain;

+

+	/*

+	 * The name is outside the queried namespace.

+	 */

+	rel = dns_name_fullcompare(name, apex, &_orderp, &_nlabelsp);

+	if (rel != dns_namereln_subdomain && rel != dns_namereln_equal) {

+		return (ISC_TRUE);

+	}

+

+	/*

+	 * If the record lives in the parent zone, adjust the name so we

+	 * look for the correct zone or forward clause.

+	 */

+	labels = dns_name_countlabels(name);

+	if (dns_rdatatype_atparent(type) && labels > 1U) {

+		dns_name_init(&suffix, NULL);

+		dns_name_getlabelsequence(name, 1, labels - 1, &suffix);

+		name = &suffix;

+	} else if (rel == dns_namereln_equal) {

+		/* If 'name' is 'apex', no further checking is needed. */

+		return (ISC_FALSE);

+	}

+

+	/*

+	 * If there is a locally served zone between 'apex' and 'name'

+	 * then don't cache.

+	 */

+	LOCK(&fctx->res->view->lock);

+	if (fctx->res->view->zonetable != NULL) {

+		unsigned int options = DNS_ZTFIND_NOEXACT;

+		result = dns_zt_find(fctx->res->view->zonetable, name, options,

+				     zfname, &zone);

+		if (zone != NULL) {

+			dns_zone_detach(&zone);

+		}

+		if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) {

+			if (dns_name_fullcompare(zfname, apex, &_orderp,

+						 &_nlabelsp) ==

+			    dns_namereln_subdomain)

+			{

+				UNLOCK(&fctx->res->view->lock);

+				return (ISC_TRUE);

+			}

+		}

+	}

+	UNLOCK(&fctx->res->view->lock);

+

+	/*

+	 * Look for a forward declaration below 'name'.

+	 */

+	result = dns_fwdtable_find2(fctx->res->view->fwdtable, name, fname,

+				    &forwarders);

+

+	if (ISFORWARDER(fctx->addrinfo)) {

+		/*

+		 * See if the forwarder declaration is better.

+		 */

+		if (result == ISC_R_SUCCESS) {

+			return (!dns_name_equal(fname, fctx->fwdname));

+		}

+

+		/*

+		 * If the lookup failed, the configuration must have

+		 * changed: play it safe and don't cache.

+		 */

+		return (ISC_TRUE);

+	} else if (result == ISC_R_SUCCESS &&

+		   forwarders->fwdpolicy == dns_fwdpolicy_only &&

+		   !ISC_LIST_EMPTY(forwarders->fwdrs))

+	{

+		/*

+		 * If 'name' is covered by a 'forward only' clause then we

+		 * can't cache this repsonse.

+		 */

+		return (ISC_TRUE);

+	}

+

+	return (ISC_FALSE);

+}

+

 static isc_result_t

 check_section(void *arg, dns_name_t *addname, dns_rdatatype_t type,

 	      dns_section_t section)

@@ -6191,7 +6306,7 @@ check_section(void *arg, dns_name_t *addname, dns_rdatatype_t type,

 	result = dns_message_findname(fctx->rmessage, section, addname,

 				      dns_rdatatype_any, 0, &name, NULL);

 	if (result == ISC_R_SUCCESS) {

-		external = ISC_TF(!dns_name_issubdomain(name, &fctx->domain));

+		external = ISC_TF(name_external(name, type, fctx));

 		if (type == dns_rdatatype_a) {

 			for (rdataset = ISC_LIST_HEAD(name->list);

 			     rdataset != NULL;

@@ -7041,6 +7156,13 @@ answer_response(fetchctx_t *fctx) {

 			break;

 		case dns_namereln_subdomain:

+			/*

+			 * Don't accept DNAME from parent namespace.

+			 */

+			if (name_external(name, dns_rdatatype_dname, fctx)) {

+				continue;

+			}

+

 			/*

 			 * In-scope DNAME records must have at least

 			 * as many labels as the domain being queried.

@@ -7269,11 +7391,9 @@ answer_response(fetchctx_t *fctx) {

 	 */

 	result = dns_message_firstname(message, DNS_SECTION_AUTHORITY);

 	while (!done && result == ISC_R_SUCCESS) {

-		isc_boolean_t external;

 		name = NULL;

 		dns_message_currentname(message, DNS_SECTION_AUTHORITY, &name);

-		external = ISC_TF(!dns_name_issubdomain(name, &fctx->domain));

-		if (!external) {

+		if (!name_external(name, dns_rdatatype_ns, fctx)) {

 			/*

 			 * We expect to find NS or SIG NS rdatasets, and

 			 * nothing else.

-- 

2.38.1