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.

diff --git a/bin/tests/system/rpzrecurse/ans5/ans.py b/bin/tests/system/rpzrecurse/ans5/ans.py
index 576f9d0f06e6751771dfc2b058fe7fefe2d1e4bb..4f6fe5ccad4fe0994bd4c7f4b5d749355c4e4f96 100644 (file)
--- 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 7f47623f88206bbc8a553ec0fd65d4f2c2a14a28..19892fab3a33f70bad090d21ae766f535bd57b4c 100644 (file)
--- 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 3366599ec1d13c129a9ec95d293987ca9be12e4f..b83dd47817ce398b559535ceb0e749253be5a922 100644 (file)
--- 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 665953d25eba61abb9b193d889c07b5137f4b179..327be8040405d78afe366e3676931e63904d126e 100644 (file)
--- 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 47a90fb6f7c171726f45d168a7301625dafbf04a..a5b720664982393777f439dd7ae6794b3c2df7e3 100644 (file)
--- 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 251e353ff8849c057f68e02735ccac6f6650756c..d87a39eddaa3cc847bdf1c5e28abac55c8e8820f 100644 (file)
--- 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 3774d4dea7916f6d7b0558a8afa555d69be44bd6..441a12fe5b039c4ea15f63bd4e7dea9d93c4622f 100644 (file)
--- 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;