- Fix #946: Forwarder returns servfail on upstream response noerror no

  data.

diff --git a/Makefile.in b/Makefile.in
index 627a650f6f938bfc66a2a8c4e434b0d1113888fc..22fb75c123bdacc72c3138a897d38eaa19b698ec 100644 (file)
--- a/Makefile.in
+++ b/Makefile.in
@@ -793,7 +793,7 @@ iter_priv.lo iter_priv.o: $(srcdir)/iterator/iter_priv.c config.h $(srcdir)/iter
  $(srcdir)/util/data/msgparse.h $(srcdir)/sldns/pkthdr.h $(srcdir)/sldns/rrdef.h $(srcdir)/util/net_help.h \
  $(srcdir)/util/storage/dnstree.h $(srcdir)/sldns/str2wire.h $(srcdir)/sldns/sbuffer.h
 iter_resptype.lo iter_resptype.o: $(srcdir)/iterator/iter_resptype.c config.h \
- $(srcdir)/iterator/iter_resptype.h $(srcdir)/iterator/iter_delegpt.h $(srcdir)/util/log.h \
+ $(srcdir)/iterator/iter_resptype.h $(srcdir)/iterator/iter_delegpt.h $(srcdir)/iterator/iterator.h $(srcdir)/util/log.h \
  $(srcdir)/services/cache/dns.h $(srcdir)/util/storage/lruhash.h $(srcdir)/util/locks.h \
  $(srcdir)/util/data/msgreply.h $(srcdir)/util/data/packed_rrset.h $(srcdir)/util/net_help.h \
  $(srcdir)/util/data/dname.h $(srcdir)/sldns/rrdef.h $(srcdir)/sldns/pkthdr.h

diff --git a/doc/Changelog b/doc/Changelog
index 22029a89b6f786d4280240c646bfd07066553398..f18f97374a9c436b7b80335459793ecc95210b35 100644 (file)
--- a/doc/Changelog
+++ b/doc/Changelog
@@ -1,3 +1,7 @@
+4 October 2023: Wouter
+       - Fix #946: Forwarder returns servfail on upstream response noerror no
+         data.
+
 3 October 2023: George
        - Merge #881: Generalise the proxy protocol code.
 

diff --git a/iterator/iter_resptype.c b/iterator/iter_resptype.c
index e85595b843d31dc437240aad7d28a265dc4143d5..38e186e79048eaf7a9214ad8fe0c9b6f168477d0 100644 (file)
--- a/iterator/iter_resptype.c
+++ b/iterator/iter_resptype.c
@@ -42,6 +42,7 @@
 #include "config.h"
 #include "iterator/iter_resptype.h"
 #include "iterator/iter_delegpt.h"
+#include "iterator/iterator.h"
 #include "services/cache/dns.h"
 #include "util/net_help.h"
 #include "util/data/dname.h"
@@ -105,7 +106,8 @@ response_type_from_cache(struct dns_msg* msg,
 
 enum response_type 
 response_type_from_server(int rdset,
-       struct dns_msg* msg, struct query_info* request, struct delegpt* dp)
+       struct dns_msg* msg, struct query_info* request, struct delegpt* dp,
+       int* empty_nodata_found)
 {
        uint8_t* origzone = (uint8_t*)"\000"; /* the default */
        struct ub_packed_rrset_key* s;
@@ -284,13 +286,22 @@ response_type_from_server(int rdset,
 
        /* If we've gotten this far, this is NOERROR/NODATA (which could 
         * be an entirely empty message) */
-       /* but ignore entirely empty messages, noerror/nodata has a soa
-        * negative ttl value in the authority section, this makes it try
-        * again at another authority. And turns it from a 5 second empty
-        * message into a 5 second servfail response. */
+       /* For entirely empty messages, try again, at first, then accept
+        * it it happens more. A regular noerror/nodata response has a soa
+        * negative ttl value in the authority section. This makes it try
+        * again at another authority. And decides between storing a 5 second
+        * empty message or a 5 second servfail response. */
        if(msg->rep->an_numrrsets == 0 && msg->rep->ns_numrrsets == 0 &&
-               msg->rep->ar_numrrsets == 0)
-               return RESPONSE_TYPE_THROWAWAY;
+               msg->rep->ar_numrrsets == 0) {
+               if(empty_nodata_found) {
+                       /* detect as throwaway at first, but accept later. */
+                       (*empty_nodata_found)++;
+                       if(*empty_nodata_found < EMPTY_NODATA_RETRY_COUNT)
+                               return RESPONSE_TYPE_THROWAWAY;
+                       return RESPONSE_TYPE_ANSWER;
+               }
+               return RESPONSE_TYPE_ANSWER;
+       }
        /* check if recursive answer; saying it has empty cache */
        if( (msg->rep->flags&BIT_RA) && !(msg->rep->flags&BIT_AA) && !rdset)
                return RESPONSE_TYPE_REC_LAME;

diff --git a/iterator/iter_resptype.h b/iterator/iter_resptype.h
index fee9ef35f83f5b7f98023289da76747c785adc02..bfd4b664f62104dcdfd5f651ffbafd686982c7b1 100644 (file)
--- a/iterator/iter_resptype.h
+++ b/iterator/iter_resptype.h
@@ -119,9 +119,11 @@ enum response_type response_type_from_cache(struct dns_msg* msg,
  * @param request: the request that generated the response.
  * @param dp: The delegation point that was being queried
  *          when the response was returned.
+ * @param empty_nodata_found: flag to keep track of empty nodata detection.
  * @return the response type (CNAME or ANSWER).
  */
 enum response_type response_type_from_server(int rdset, 
-       struct dns_msg* msg, struct query_info* request, struct delegpt* dp);
+       struct dns_msg* msg, struct query_info* request, struct delegpt* dp,
+       int* empty_nodata_found);
 
 #endif /* ITERATOR_ITER_RESPTYPE_H */

diff --git a/iterator/iterator.c b/iterator/iterator.c
index 9f78aa17d84be234881ecb837c8b5aef484f13cc..106e2877e0855940af5ac7ed443b55a01ef8ea74 100644 (file)
--- a/iterator/iterator.c
+++ b/iterator/iterator.c
@@ -2940,7 +2940,7 @@ static int
 processQueryResponse(struct module_qstate* qstate, struct iter_qstate* iq,
        struct iter_env* ie, int id)
 {
-       int dnsseclame = 0, origtypecname = 0;
+       int dnsseclame = 0, origtypecname = 0, orig_empty_nodata_found;
        enum response_type type;
 
        iq->num_current_queries--;
@@ -2960,12 +2960,25 @@ processQueryResponse(struct module_qstate* qstate, struct iter_qstate* iq,
                return next_state(iq, QUERYTARGETS_STATE);
        }
        iq->timeout_count = 0;
+       orig_empty_nodata_found = iq->empty_nodata_found;
        type = response_type_from_server(
                (int)((iq->chase_flags&BIT_RD) || iq->chase_to_rd),
-               iq->response, &iq->qinfo_out, iq->dp);
+               iq->response, &iq->qinfo_out, iq->dp, &iq->empty_nodata_found);
        iq->chase_to_rd = 0;
        /* remove TC flag, if this is erroneously set by TCP upstream */
        iq->response->rep->flags &= ~BIT_TC;
+       if(orig_empty_nodata_found != iq->empty_nodata_found &&
+               iq->empty_nodata_found < EMPTY_NODATA_RETRY_COUNT) {
+               /* try to search at another server */
+               if(qstate->reply) {
+                       struct delegpt_addr* a = delegpt_find_addr(
+                               iq->dp, &qstate->reply->remote_addr,
+                               qstate->reply->remote_addrlen);
+                       /* make selection disprefer it */
+                       if(a) a->lame = 1;
+               }
+               return next_state(iq, QUERYTARGETS_STATE);
+       }
        if(type == RESPONSE_TYPE_REFERRAL && (iq->chase_flags&BIT_RD) &&
                !iq->auth_zone_response) {
                /* When forwarding (RD bit is set), we handle referrals 
@@ -3501,7 +3514,7 @@ processPrimeResponse(struct module_qstate* qstate, int id)
        iq->response->rep->flags &= ~(BIT_RD|BIT_RA); /* ignore rec-lame */
        type = response_type_from_server(
                (int)((iq->chase_flags&BIT_RD) || iq->chase_to_rd), 
-               iq->response, &iq->qchase, iq->dp);
+               iq->response, &iq->qchase, iq->dp, NULL);
        if(type == RESPONSE_TYPE_ANSWER) {
                qstate->return_rcode = LDNS_RCODE_NOERROR;
                qstate->return_msg = iq->response;

diff --git a/iterator/iterator.h b/iterator/iterator.h
index fad7f03e63deda5ab1c544f12c958a5b642bc7af..e253f3f7e2bd36ddea96e9de58abb11f32c63919 100644 (file)
--- a/iterator/iterator.h
+++ b/iterator/iterator.h
@@ -101,6 +101,8 @@ extern int BLACKLIST_PENALTY;
  * Chosen so that the UNKNOWN_SERVER_NICENESS falls within the band of a 
  * fast server, this causes server exploration as a side benefit. msec. */
 #define RTT_BAND 400
+/** Number of retries for empty nodata packets before it is accepted. */
+#define EMPTY_NODATA_RETRY_COUNT 2
 
 /**
  * Global state for the iterator.
@@ -415,6 +417,11 @@ struct iter_qstate {
         */
        int refetch_glue;
 
+       /**
+        * This flag detects that a completely empty nodata was received,
+        * already so that it is accepted later. */
+       int empty_nodata_found;
+
        /** list of pending queries to authoritative servers. */
        struct outbound_list outlist;
 

diff --git a/testdata/iter_ignore_empty.rpl b/testdata/iter_ignore_empty.rpl
index c70dd7e8df7b1d1b3916f6a753de939e4216d8a9..4b2f695b850130bf7b802bdc796a6457d1e42efa 100644 (file)
--- a/testdata/iter_ignore_empty.rpl
+++ b/testdata/iter_ignore_empty.rpl
@@ -78,6 +78,18 @@ example2.com.        IN NS   ns2.example2.com.
 SECTION ADDITIONAL
 ns2.example2.com.              IN      A       1.2.3.5
 ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode subdomain
+ADJUST copy_id copy_query
+REPLY QR NOERROR
+SECTION QUESTION
+foo.com. IN NS
+SECTION AUTHORITY
+foo.com.       IN NS   ns.foo.com.
+SECTION ADDITIONAL
+ns.foo.com.            IN      A       1.2.3.5
+ENTRY_END
 RANGE_END
 
 ; ns.example.com.
@@ -172,6 +184,27 @@ www.example.com. IN A
 SECTION ANSWER
 www.example.com. IN A  10.20.30.40
 ENTRY_END
+
+; foo.com
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR AA NOERROR
+SECTION QUESTION
+www.foo.com. IN A
+SECTION ANSWER
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR AA NOERROR
+SECTION QUESTION
+ns.foo.com. IN AAAA
+SECTION ANSWER
+SECTION AUTHORITY
+;foo.com.      IN SOA ns2.foo.com root.foo.com 4 14400 3600 604800 3600
+ENTRY_END
 RANGE_END
 
 STEP 1 QUERY
@@ -195,4 +228,21 @@ ENTRY_END
 ; wait for pending nameserver lookups.
 STEP 20 TRAFFIC
 
+; Test that a nodata stays a nodata.
+STEP 30 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.foo.com. IN A
+ENTRY_END
+
+STEP 40 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www.foo.com. IN A
+SECTION ANSWER
+ENTRY_END
+
 SCENARIO_END