From d1a32c003c6dc54fd336288bcc63cff7a8202e9d Mon Sep 17 00:00:00 2001
From: Evan Hunt <each@isc.org>
Date: Mon, 4 May 2026 17:05:11 -0700
Subject: [PATCH] Clear REDIRECT flag when it isn't needed

The NS_QUERYATTR_REDIRECT flag is set when processing a recursive
NXDOMAIN redirection lookup, so that if that lookup also returns
NXDOMAIN we don't end up looping.

Previously, the flag was left active after use, but if the
same client triggered a subsequent recursive lookup (for example,
in the filter-aaaa plugin), then the wrong branch could be reached
in query_resume(), potentially leading to an assertion failure.  This
has been fixed.

(cherry picked from commit 3ff00183086f304c71076a069376e37692a7c9ff)
---
 lib/ns/query.c | 56 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/lib/ns/query.c b/lib/ns/query.c
index 231ad9f847..8909ed2608 100644
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -5314,9 +5314,12 @@ redirect2(ns_client_t *client, dns_name_t *name, dns_rdataset_t *rdataset,
 	dns_zone_t *zone = NULL;
 	bool is_zone;
 	unsigned int labels;
+	bool redirected = REDIRECT(client);
 
 	CTRACE(ISC_LOG_DEBUG(3), "redirect2");
 
+	client->query.attributes &= ~NS_QUERYATTR_REDIRECT;
+
 	if (client->view->redirectzone == NULL) {
 		return ISC_R_NOTFOUND;
 	}
@@ -5418,17 +5421,17 @@ redirect2(ns_client_t *client, dns_name_t *name, dns_rdataset_t *rdataset,
 			dns_db_detachnode(db, &node);
 		}
 		dns_db_detach(&db);
+
 		/*
 		 * Don't loop forever if the lookup failed last time.
 		 */
-		if (!REDIRECT(client)) {
+		if (!redirected) {
 			result = ns_query_recurse(client, qtype, redirectname,
 						  NULL, NULL, true);
 			if (result == ISC_R_SUCCESS) {
 				client->query.attributes |=
-					NS_QUERYATTR_RECURSING;
-				client->query.attributes |=
-					NS_QUERYATTR_REDIRECT;
+					(NS_QUERYATTR_RECURSING |
+					 NS_QUERYATTR_REDIRECT);
 				return DNS_R_CONTINUE;
 			}
 		}
@@ -6649,6 +6652,7 @@ query_resume(query_ctx_t *qctx) {
 	char qbuf[DNS_NAME_FORMATSIZE];
 	char tbuf[DNS_RDATATYPE_FORMATSIZE];
 #endif /* ifdef WANT_QUERYTRACE */
+	bool redirect = REDIRECT(qctx->client);
 
 	CCTRACE(ISC_LOG_DEBUG(3), "query_resume");
 
@@ -6657,9 +6661,10 @@ query_resume(query_ctx_t *qctx) {
 	qctx->want_restart = false;
 
 	qctx->rpz_st = qctx->client->query.rpz_st;
-	if (qctx->rpz_st != NULL &&
-	    (qctx->rpz_st->state & DNS_RPZ_RECURSING) != 0)
-	{
+	bool rpz = (qctx->rpz_st != NULL &&
+		    (qctx->rpz_st->state & DNS_RPZ_RECURSING) != 0);
+
+	if (rpz) {
 		CCTRACE(ISC_LOG_DEBUG(3), "resume from RPZ recursion");
 #ifdef WANT_QUERYTRACE
 		{
@@ -6703,7 +6708,7 @@ query_resume(query_ctx_t *qctx) {
 		qctx->rpz_st->r.r_type = qctx->fresp->qtype;
 		SAVE(qctx->rpz_st->r.r_rdataset, qctx->fresp->rdataset);
 		ns_client_putrdataset(qctx->client, &qctx->fresp->sigrdataset);
-	} else if (REDIRECT(qctx->client)) {
+	} else if (redirect) {
 		/*
 		 * Restore saved state.
 		 */
@@ -6772,9 +6777,7 @@ query_resume(query_ctx_t *qctx) {
 		qctx->dns64_exclude = true;
 	}
 
-	if (qctx->rpz_st != NULL &&
-	    (qctx->rpz_st->state & DNS_RPZ_RECURSING) != 0)
-	{
+	if (rpz) {
 		/*
 		 * Has response policy changed out from under us?
 		 */
@@ -6796,11 +6799,9 @@ query_resume(query_ctx_t *qctx) {
 	qctx->dbuf = ns_client_getnamebuf(qctx->client);
 	qctx->fname = ns_client_newname(qctx->client, qctx->dbuf, &b);
 
-	if (qctx->rpz_st != NULL &&
-	    (qctx->rpz_st->state & DNS_RPZ_RECURSING) != 0)
-	{
+	if (rpz) {
 		tname = qctx->rpz_st->fname;
-	} else if (REDIRECT(qctx->client)) {
+	} else if (redirect) {
 		tname = qctx->client->query.redirect.fname;
 	} else {
 		tname = qctx->fresp->foundname;
@@ -6808,14 +6809,25 @@ query_resume(query_ctx_t *qctx) {
 
 	dns_name_copy(tname, qctx->fname);
 
-	if (qctx->rpz_st != NULL &&
-	    (qctx->rpz_st->state & DNS_RPZ_RECURSING) != 0)
-	{
+	if (rpz) {
 		qctx->rpz_st->r.r_result = qctx->fresp->result;
 		result = qctx->rpz_st->q.result;
 		free_fresp(qctx->client, &qctx->fresp);
-	} else if (REDIRECT(qctx->client)) {
+	} else if (redirect) {
 		result = qctx->client->query.redirect.result;
+
+		/*
+		 * If we got an answer from a redirect query that could
+		 * trigger another redirect, keep the REDIRECT flag set
+		 * so we can avoid looping; we'll clear it later.
+		 * Otherwise, we're done with it now.
+		 */
+		if (result != DNS_R_COVERINGNSEC && result != DNS_R_NXDOMAIN &&
+		    result != DNS_R_NCACHENXDOMAIN)
+		{
+			qctx->client->query.attributes &=
+				~NS_QUERYATTR_REDIRECT;
+		}
 	} else {
 		result = qctx->fresp->result;
 	}
@@ -9573,8 +9585,6 @@ query_nxdomain(query_ctx_t *qctx, isc_result_t result) {
 
 	CALL_HOOK(NS_QUERY_NXDOMAIN_BEGIN, qctx);
 
-	INSIST(qctx->is_zone || REDIRECT(qctx->client));
-
 	if (!empty_wild) {
 		result = query_redirect(qctx, result);
 		if (result != ISC_R_COMPLETE) {
@@ -9659,6 +9669,10 @@ cleanup:
  *
  * Any result code other than ISC_R_COMPLETE means redirection was
  * successful and the result code should be returned up the call stack.
+ * DNS_R_CONTINUE means we've initiated a recursive query to the
+ * redirect zone, and we'll resume processing with the answer to that
+ * in query_resume(); other results mean we have the redirected answer
+ * now.
  *
  * ISC_R_COMPLETE means we reached the end of this function without
  * redirecting, so query processing should continue past it.