From 5b2798e01346cd77741873091babf6c4a3128449 Mon Sep 17 00:00:00 2001

From: Mark Andrews <marka@isc.org>

Date: Wed, 19 Jan 2022 17:38:18 +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 24155213be59faad17f0215ecf73ea49ab781e5b)

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 3fc7accd88cd0890f8f57bb13765876774298ba3)

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 ea06552a3d1fed56f7d3a13710e084ec79797b78)

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 71b24210542730355149130770deea3e58d8527a)

---

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

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

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

index a7bc661bb7..7603a07b7b 100644

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

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

@@ -63,6 +63,8 @@

 #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, DNS_LOGCATEGORY_RESOLVER,                     \

@@ -337,6 +339,8 @@ struct fetchctx {

 	dns_fetch_t *qminfetch;

 	dns_rdataset_t qminrrset;

 	dns_name_t qmindcname;

+	dns_fixedname_t fwdfname;

+	dns_name_t *fwdname;

 	/*%

 	 * The number of events we're waiting for.

@@ -3764,6 +3768,7 @@ fctx_getaddresses(fetchctx_t *fctx, bool badcache) {

 		if (result == ISC_R_SUCCESS) {

 			fwd = ISC_LIST_HEAD(forwarders->fwdrs);

 			fctx->fwdpolicy = forwarders->fwdpolicy;

+			dns_name_copynf(domain, fctx->fwdname);

 			if (fctx->fwdpolicy == dns_fwdpolicy_only &&

 			    isstrictsubdomain(domain, &fctx->domain))

 			{

@@ -5153,6 +5158,9 @@ fctx_create(dns_resolver_t *res, const 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;

@@ -5215,6 +5223,7 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type,

 					   fname, &forwarders);

 		if (result == ISC_R_SUCCESS) {

 			fctx->fwdpolicy = forwarders->fwdpolicy;

+			dns_name_copynf(fname, fctx->fwdname);

 		}

 		if (fctx->fwdpolicy != dns_fwdpolicy_only) {

@@ -7118,6 +7127,107 @@ mark_related(dns_name_t *name, dns_rdataset_t *rdataset, bool 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 bool

+name_external(const 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;

+

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

+

+	/*

+	 * The name is outside the queried namespace.

+	 */

+	rel = dns_name_fullcompare(name, apex, &(int){ 0 },

+				   &(unsigned int){ 0U });

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

+		return (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 (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 | DNS_ZTFIND_MIRROR;

+		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, &(int){ 0 },

+						 &(unsigned int){ 0U }) ==

+			    dns_namereln_subdomain)

+			{

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

+				return (true);

+			}

+		}

+	}

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

+

+	/*

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

+	 */

+	result = dns_fwdtable_find(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 (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 (true);

+	}

+

+	return (false);

+}

+

 static isc_result_t

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

 	      dns_section_t section) {

@@ -7144,7 +7254,7 @@ check_section(void *arg, const dns_name_t *addname, dns_rdatatype_t type,

 	result = dns_message_findname(rctx->query->rmessage, section, addname,

 				      dns_rdatatype_any, 0, &name, NULL);

 	if (result == ISC_R_SUCCESS) {

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

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

 		if (type == dns_rdatatype_a) {

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

 			     rdataset != NULL;

@@ -8768,6 +8878,13 @@ rctx_answer_scan(respctx_t *rctx) {

 			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.

@@ -9081,13 +9198,11 @@ rctx_authority_positive(respctx_t *rctx) {

 				       DNS_SECTION_AUTHORITY);

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

 		dns_name_t *name = NULL;

-		bool external;

 		dns_message_currentname(rctx->query->rmessage,

 					DNS_SECTION_AUTHORITY, &name);

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

-		if (!external) {

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

 			dns_rdataset_t *rdataset = NULL;

 			/*

@@ -9474,7 +9589,10 @@ rctx_authority_dnssec(respctx_t *rctx) {

 		}

 		if (!dns_name_issubdomain(name, &fctx->domain)) {

-			/* Invalid name found; preserve it for logging later */

+			/*

+			 * Invalid name found; preserve it for logging

+			 * later.

+			 */

 			rctx->found_name = name;

 			rctx->found_type = ISC_LIST_HEAD(name->list)->type;

 			continue;

-- 

2.34.1