From ae67c1851dcd4ce3e03012f7a6ac6cfa0eb6b37f Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Sat, 21 Mar 2026 20:28:24 -0700 Subject: [PATCH] rpz_rrset_find() now recurses on ISC_R_NOTFOUND previously, rpz_rrset_find() behaved differently depending on whether a cache lookup returned DNS_R_DELEGATION or ISC_R_NOTFOUND. the former indicates the presence of a cached NS rrset, and the latter indicates that the cache is cold or that all NS rrsets above the query name have expired. both results indicate that the caller should recurse, but rpz_rrset_find() only recursed in the case of DNS_R_DELEGATION. the nsip-wait-recurse and nsdname-wait-recurse test cases in the rpzrecurse system test were dependent on this misbehavior. the test server was configured with a lame delegation, so that recursion always failed, but once the lame delegation was expired due to a zero TTL, the cache returned ISC_R_NOTFOUND, which caused the recursion not to be attempted. the test seemed to be observing a delay before recursion succeeded, but it was actually observing a delay before recursion was skipped. fixing this bug caused the test to fail. the test server has now been reconfigured so that recursion succeeds after a delay, instead of failing. now we're able to test that we're waiting for the successful completion of recursion. --- bin/tests/system/rpzrecurse/ans5/ans.py | 17 +++++-- .../system/rpzrecurse/ns3/named2.conf.j2 | 4 +- .../system/rpzrecurse/ns3/named3.conf.j2 | 4 +- bin/tests/system/rpzrecurse/ns3/root.db | 2 +- .../system/rpzrecurse/ns4/child.example.db | 4 +- bin/tests/system/rpzrecurse/tests.sh | 8 ++-- lib/ns/query.c | 46 +++++++++---------- 7 files changed, 47 insertions(+), 38 deletions(-) diff --git a/bin/tests/system/rpzrecurse/ans5/ans.py b/bin/tests/system/rpzrecurse/ans5/ans.py index 576f9d0f06..4f6fe5ccad 100644 --- a/bin/tests/system/rpzrecurse/ans5/ans.py +++ b/bin/tests/system/rpzrecurse/ans5/ans.py @@ -21,7 +21,6 @@ from isctest.asyncserver import ( AsyncDnsServer, DnsResponseSend, QueryContext, - ResponseDrop, ResponseHandler, ) @@ -40,19 +39,27 @@ class ReplyA(ResponseHandler): yield DnsResponseSend(qctx.response) -class IgnoreNs(ResponseHandler): +class DelayNs(ResponseHandler): def match(self, qctx: QueryContext) -> bool: return qctx.qtype == dns.rdatatype.NS async def get_responses( self, qctx: QueryContext - ) -> AsyncGenerator[ResponseDrop, None]: - yield ResponseDrop() + ) -> AsyncGenerator[DnsResponseSend, None]: + ns_rrset = dns.rrset.from_text( + "foo.child.example.tld.", + 300, + qctx.qclass, + dns.rdatatype.NS, + "ns5.foo.", + ) + qctx.response.answer.append(ns_rrset) + yield DnsResponseSend(qctx.response, delay=1) def main() -> None: server = AsyncDnsServer(default_aa=True, default_rcode=dns.rcode.NOERROR) - server.install_response_handlers(ReplyA(), IgnoreNs()) + server.install_response_handlers(ReplyA(), DelayNs()) server.run() diff --git a/bin/tests/system/rpzrecurse/ns3/named2.conf.j2 b/bin/tests/system/rpzrecurse/ns3/named2.conf.j2 index 7f47623f88..19892fab3a 100644 --- a/bin/tests/system/rpzrecurse/ns3/named2.conf.j2 +++ b/bin/tests/system/rpzrecurse/ns3/named2.conf.j2 @@ -27,7 +27,9 @@ options { listen-on-v6 { none; }; recursion yes; dnssec-validation no; - response-policy { zone "policy"; } nsip-wait-recurse no + response-policy { zone "policy"; } + nsip-wait-recurse no + nsdname-wait-recurse no qname-wait-recurse yes nsip-enable yes nsdname-enable yes; diff --git a/bin/tests/system/rpzrecurse/ns3/named3.conf.j2 b/bin/tests/system/rpzrecurse/ns3/named3.conf.j2 index 3366599ec1..b83dd47817 100644 --- a/bin/tests/system/rpzrecurse/ns3/named3.conf.j2 +++ b/bin/tests/system/rpzrecurse/ns3/named3.conf.j2 @@ -27,7 +27,9 @@ options { listen-on-v6 { none; }; recursion yes; dnssec-validation no; - response-policy { zone "policy"; } nsdname-wait-recurse no + response-policy { zone "policy"; } + nsip-wait-recurse no + nsdname-wait-recurse no nsdname-enable yes; }; diff --git a/bin/tests/system/rpzrecurse/ns3/root.db b/bin/tests/system/rpzrecurse/ns3/root.db index 665953d25e..327be80404 100644 --- a/bin/tests/system/rpzrecurse/ns3/root.db +++ b/bin/tests/system/rpzrecurse/ns3/root.db @@ -13,5 +13,5 @@ $TTL 0 @ SOA . . 0 0 0 0 0 @ NS ns ns A 10.53.0.3 -foo NS foo.ns5 +foo NS ns5.foo ns5.foo A 10.53.0.5 diff --git a/bin/tests/system/rpzrecurse/ns4/child.example.db b/bin/tests/system/rpzrecurse/ns4/child.example.db index 47a90fb6f7..a5b7206649 100644 --- a/bin/tests/system/rpzrecurse/ns4/child.example.db +++ b/bin/tests/system/rpzrecurse/ns4/child.example.db @@ -13,6 +13,4 @@ $TTL 0 @ SOA . . 0 0 0 0 0 @ NS ns ns A 10.53.0.4 -foo NS ns.foo -foo NS ns.foo. -ns.foo A 10.53.0.5 +foo NS ns5.foo. diff --git a/bin/tests/system/rpzrecurse/tests.sh b/bin/tests/system/rpzrecurse/tests.sh index 251e353ff8..d87a39edda 100644 --- a/bin/tests/system/rpzrecurse/tests.sh +++ b/bin/tests/system/rpzrecurse/tests.sh @@ -442,7 +442,7 @@ add_test_marker 10.53.0.2 10.53.0.3 echo_i "timing 'nsip-wait-recurse yes' (default)" ret=0 t1=$($PERL -e 'print time()."\n";') -$DIG -p ${PORT} @10.53.0.3 foo.child.example.tld a >dig.out.yes.$t +$DIG -p ${PORT} @10.53.0.3 foo.child.example.tld a >dig.out.yes.$t || ret=1 t2=$($PERL -e 'print time()."\n";') p1=$((t2 - t1)) echo_i "elapsed time $p1 seconds" @@ -455,7 +455,7 @@ wait_for_log 20 "rpz: policy: reload done" ns3/named.run || ret=1 echo_i "timing 'nsip-wait-recurse no'" t3=$($PERL -e 'print time()."\n";') -$DIG -p ${PORT} @10.53.0.3 foo.child.example.tld a >dig.out.no.$t +$DIG -p ${PORT} @10.53.0.3 foo.child.example.tld a >dig.out.no.$t || ret=1 t4=$($PERL -e 'print time()."\n";') p2=$((t4 - t3)) echo_i "elapsed time $p2 seconds" @@ -477,7 +477,7 @@ add_test_marker 10.53.0.2 10.53.0.3 echo_i "timing 'nsdname-wait-recurse yes' (default)" ret=0 t1=$($PERL -e 'print time()."\n";') -$DIG -p ${PORT} @10.53.0.3 foo.child.example.tld a >dig.out.yes.$t +$DIG -p ${PORT} @10.53.0.3 foo.child.example.tld a >dig.out.yes.$t || ret=1 t2=$($PERL -e 'print time()."\n";') p1=$((t2 - t1)) echo_i "elapsed time $p1 seconds" @@ -490,7 +490,7 @@ wait_for_log 20 "rpz: policy: reload done" ns3/named.run || ret=1 echo_i "timing 'nsdname-wait-recurse no'" t3=$($PERL -e 'print time()."\n";') -$DIG -p ${PORT} @10.53.0.3 foo.child.example.tld a >dig.out.no.$t +$DIG -p ${PORT} @10.53.0.3 foo.child.example.tld a >dig.out.no.$t || ret=1 t4=$($PERL -e 'print time()."\n";') p2=$((t4 - t3)) echo_i "elapsed time $p2 seconds" diff --git a/lib/ns/query.c b/lib/ns/query.c index 3774d4dea7..441a12fe5b 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -3067,14 +3067,14 @@ rpz_rrset_find(ns_client_t *client, dns_name_t *name, dns_rdatatype_t type, unsigned int options, dns_rpz_type_t rpz_type, dns_db_t **dbp, dns_dbversion_t *version, dns_rdataset_t **rdatasetp, bool resuming) { - dns_rpz_st_t *st; - bool is_zone; - dns_dbnode_t *node; - dns_fixedname_t fixed; - dns_name_t *found; isc_result_t result; + dns_rpz_st_t *st = NULL; + dns_dbnode_t *node = NULL; + dns_fixedname_t fixed; + dns_name_t *found = dns_fixedname_initname(&fixed); dns_clientinfomethods_t cm; dns_clientinfo_t ci; + bool is_zone; CTRACE(ISC_LOG_DEBUG(3), "rpz_rrset_find"); @@ -3109,29 +3109,23 @@ rpz_rrset_find(ns_client_t *client, dns_name_t *name, dns_rdatatype_t type, if (*dbp != NULL) { is_zone = false; } else { - dns_zone_t *zone; + dns_zone_t *zone = NULL; version = NULL; - zone = NULL; result = query_getdb(client, name, type, (dns_getdb_options_t){ 0 }, &zone, dbp, &version, &is_zone); + if (zone != NULL) { + dns_zone_detach(&zone); + } if (result != ISC_R_SUCCESS) { rpz_log_fail(client, DNS_RPZ_ERROR_LEVEL, name, rpz_type, "rpz_rrset_find(2)", result); st->m.policy = DNS_RPZ_POLICY_ERROR; - if (zone != NULL) { - dns_zone_detach(&zone); - } return result; } - if (zone != NULL) { - dns_zone_detach(&zone); - } } - node = NULL; - found = dns_fixedname_initname(&fixed); dns_clientinfomethods_init(&cm, ns_client_sourceip); dns_clientinfo_init(&ci, client, NULL); result = dns_db_findext(*dbp, name, version, type, options, @@ -3140,14 +3134,19 @@ rpz_rrset_find(ns_client_t *client, dns_name_t *name, dns_rdatatype_t type, if (result == DNS_R_DELEGATION && is_zone && USECACHE(client)) { /* * Try the cache if we're authoritative for an - * ancestor but not the domain itself. + * ancestor but not the domain itself. DELEGATION or + * NOTFOUND indicates that we should recurse, if + * nsip-wait-recurse or nsdname-wait-recurse are + * enabled. */ rpz_clean(NULL, dbp, &node, rdatasetp); - version = NULL; dns_db_attach(client->inner.view->cachedb, dbp); - result = dns_db_findext(*dbp, name, version, type, 0, + result = dns_db_findext(*dbp, name, NULL, type, 0, client->inner.now, &node, found, &cm, &ci, *rdatasetp, NULL); + if (result == ISC_R_NOTFOUND) { + result = DNS_R_DELEGATION; + } } rpz_clean(NULL, dbp, &node, NULL); if (result == DNS_R_DELEGATION) { @@ -3158,13 +3157,11 @@ rpz_rrset_find(ns_client_t *client, dns_name_t *name, dns_rdatatype_t type, */ if (rpz_type == DNS_RPZ_TYPE_IP) { result = DNS_R_NXRRSET; - } else if (!client->inner.view->rpzs->p.nsip_wait_recurse || - (!client->inner.view->rpzs->p.nsdname_wait_recurse && + } else if ((client->inner.view->rpzs->p.nsip_wait_recurse && + rpz_type == DNS_RPZ_TYPE_NSIP) || + (client->inner.view->rpzs->p.nsdname_wait_recurse && rpz_type == DNS_RPZ_TYPE_NSDNAME)) { - query_rpzfetch(client, name, type); - result = DNS_R_NXRRSET; - } else { dns_name_copy(name, st->r_name); result = ns_query_recurse(client, type, st->r_name, NULL, NULL, resuming); @@ -3172,6 +3169,9 @@ rpz_rrset_find(ns_client_t *client, dns_name_t *name, dns_rdatatype_t type, st->state |= DNS_RPZ_RECURSING; result = DNS_R_DELEGATION; } + } else { + query_rpzfetch(client, name, type); + result = DNS_R_NXRRSET; } } return result;