diff --git a/bin/tests/system/resend_loop/ans3/ans.py b/bin/tests/system/resend_loop/ans3/ans.py index 17ff396f11..57c3ffafeb 100644 --- a/bin/tests/system/resend_loop/ans3/ans.py +++ b/bin/tests/system/resend_loop/ans3/ans.py @@ -22,6 +22,7 @@ from isctest.asyncserver import ( AsyncDnsServer, DnsResponseSend, DomainHandler, + QnameHandler, QnameQtypeHandler, QueryContext, StaticResponseHandler, @@ -78,12 +79,19 @@ class ExampleCookieHandler(DomainHandler): yield DnsResponseSend(qctx.response) +class TestDotComServFailHandler(QnameHandler, StaticResponseHandler): + qnames = ["test.com."] + authoritative = False + rcode = dns.rcode.SERVFAIL + + def main() -> None: server = AsyncDnsServer(default_aa=True, default_rcode=dns.rcode.NOERROR) server.install_response_handlers( RootNsHandler(), ExampleNsHandler(), ExampleCookieHandler(), + TestDotComServFailHandler(), ) server.run() diff --git a/bin/tests/system/resend_loop/ns1/named.conf.j2 b/bin/tests/system/resend_loop/ns1/named.conf.j2 new file mode 100644 index 0000000000..4ef0c35531 --- /dev/null +++ b/bin/tests/system/resend_loop/ns1/named.conf.j2 @@ -0,0 +1,13 @@ +options { + query-source address 10.53.0.1; + notify-source 10.53.0.1; + transfer-source 10.53.0.1; + port @PORT@; + pid-file "named.pid"; + listen-on { 10.53.0.1; }; + listen-on-v6 { none; }; + recursion yes; + allow-query { any; }; + forwarders { 10.53.0.3 port @PORT@; }; + forward only; +}; diff --git a/bin/tests/system/resend_loop/tests_resend_loop.py b/bin/tests/system/resend_loop/tests_resend_loop.py index a9a236fa58..fbc791555b 100644 --- a/bin/tests/system/resend_loop/tests_resend_loop.py +++ b/bin/tests/system/resend_loop/tests_resend_loop.py @@ -83,3 +83,39 @@ def test_resend_loop_badcookie(ns4): prohibited_log = "query failed (timed out) for test.example/IN/A" assert prohibited_log not in ns4.log + + +def test_resend_loop_forward(ns1): + sending_packet = Re("sending packet from 10.53.0.1#[0-9]+ to 10.53.0.3#[0-9]+") + received_packet = Re("received packet from 10.53.0.3#[0-9]+ to 10.53.0.1#[0-9]+") + + # Make sure the server is done with priming and ./DNSKEY refresh (RFC5011) + msg = dns.message.make_query(".", "NS") + isctest.query.udp(msg, ns1.ip) + query_count_start = len(ns1.log.grep(sending_packet)) + + log_sequence = [ + sending_packet, + "flags: rd; QUESTION: 1", + received_packet, + "opcode: QUERY, status: SERVFAIL", + "flags: qr rd; QUESTION: 1", + sending_packet, + "flags: rd cd; QUESTION: 1", + received_packet, + "opcode: QUERY, status: SERVFAIL", + "flags: qr rd; QUESTION: 1", + ] + + msg = dns.message.make_query("test.com.", "A") + with ns1.watch_log_from_here() as watcher: + res = isctest.query.udp(msg, ns1.ip) + watcher.wait_for_sequence(log_sequence) + + isctest.check.servfail(res) + + query_count_end = len(ns1.log.grep(sending_packet)) + assert query_count_end - query_count_start == 2 + + prohibited_log = "query failed (timed out) for test.com/IN/A" + assert prohibited_log not in ns1.log diff --git a/lib/dns/include/dns/resolver.h b/lib/dns/include/dns/resolver.h index 8def29ed6b..b5e964ec06 100644 --- a/lib/dns/include/dns/resolver.h +++ b/lib/dns/include/dns/resolver.h @@ -133,16 +133,19 @@ enum { * possible. */ DNS_FETCHOPT_QMINFETCH = 1 << 16, /*%< Qmin fetch */ DNS_FETCHOPT_WANTZONEVERSION = 1 << 17, /*%< Request ZONEVERSION */ - DNS_FETCHOPT_TRYCD = 1 << 18, /*%< Send the first query - * to a forwader with - * CD=0, but retry with CD=1 - * if it returns SERVFAIL. - */ - DNS_FETCHOPT_PRIMING = 1 << 19, /*%< Root priming fetch. - * Copies the '.' NS answer - * and root-server glue from - * the response into - * view->rootdb. */ + + /* + * Send the first query to a forwarder with CD=0 (TRYNOCD), but retry + * with CD=1 if it returns SERVFAIL (TRYCD). + */ + DNS_FETCHOPT_TRYNOCD = 1 << 18, + DNS_FETCHOPT_TRYCD = 1 << 19, + + DNS_FETCHOPT_PRIMING = 1 << 20, /*%< Root priming fetch. + * Copies the '.' NS answer + * and root-server glue from + * the response into + * view->rootdb. */ /*% EDNS version bits: */ DNS_FETCHOPT_EDNSVERSIONSET = 1 << 23, diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 699f833eb8..4ad232a183 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -2533,11 +2533,31 @@ resquery_send(resquery_t *query) { } else if ((query->options & DNS_FETCHOPT_NOVALIDATE) != 0 || (query->options & DNS_FETCHOPT_TRYCD) != 0) { + /* + * `DNS_FETCHOPT_TRYCD` has been set by the code processing a + * SERVFAIL response from a forwarder with CD=0. This is a + * second (and last) attempt to resend the query with CD=1 to + * the same server. + */ fctx->qmessage->flags |= DNS_MESSAGEFLAG_CD; } else if (res->view->enablevalidation && ((fctx->qmessage->flags & DNS_MESSAGEFLAG_RD) != 0)) { - query->options |= DNS_FETCHOPT_TRYCD; + /* + * RD flag is set, meaning either that the client requests + * recursion or (non exclusive) we are asking a forwarder. The + * logic about CD doesn't matter in the non-forwarder case, but + * doesn't harm either (as an authoritative server will ignore + * this). However, for the forwarder case, it means the + * forwarder will attempt the validation. + * + * The query is sent with CD=0, and `DNS_FETCHOPT_TRYNOCD` + * enables the code processing the response to know (if this is + * a forwarder, which is re-checked in the code processing the + * response) that we can re-send the query again, one more time, + * with CD=1 (leaving the resolver doing the validation itself). + */ + query->options |= DNS_FETCHOPT_TRYNOCD; } /* @@ -9993,7 +10013,8 @@ rctx_badserver(respctx_t *rctx, isc_result_t result) { rctx->resend = true; } else if (ISFORWARDER(query->addrinfo) && query->rmessage->rcode == dns_rcode_servfail && - (query->options & DNS_FETCHOPT_TRYCD) != 0) + (query->options & DNS_FETCHOPT_TRYNOCD) != 0 && + (query->options & DNS_FETCHOPT_TRYCD) == 0) { /* * We got a SERVFAIL from a forwarder with