- Fix #923: processQueryResponse() THROWAWAY should be mindful of

  fail_reply.

diff --git a/doc/Changelog b/doc/Changelog
index eedd5847043da7021f930930779f26e60f7febef..c5dd1d402403f64fde76d406260743b64ce8032f 100644 (file)
--- a/doc/Changelog
+++ b/doc/Changelog
@@ -3,6 +3,8 @@
        - Fix windows ci workflow to install bison and flex.
        - Fix for #925: unbound.service: Main process exited, code=killed,
          status=11/SEGV. Fixes cachedb configuration handling.
+       - Fix #923: processQueryResponse() THROWAWAY should be mindful of
+         fail_reply.
 
 18 August 2023: Wouter
        - Fix for iter_dec_attempts that could cause a hang, part of

diff --git a/iterator/iterator.c b/iterator/iterator.c
index a31e95c05a5f5a4a20092c192f4e5b6c9fd4de9a..1548dfcaee627e4f5c4d18bc9abcc011b61e9fdb 100644 (file)
--- a/iterator/iterator.c
+++ b/iterator/iterator.c
@@ -574,6 +574,54 @@ handle_cname_response(struct module_qstate* qstate, struct iter_qstate* iq,
        return 1;
 }
 
+/** fill fail address for later recovery */
+static void
+fill_fail_addr(struct iter_qstate* iq, struct sockaddr_storage* addr,
+       socklen_t addrlen)
+{
+       if(addrlen == 0) {
+               iq->fail_addr_type = 0;
+               return;
+       }
+       if(((struct sockaddr_in*)addr)->sin_family == AF_INET) {
+               iq->fail_addr_type = 4;
+               memcpy(&iq->fail_addr.in,
+                       &((struct sockaddr_in*)addr)->sin_addr,
+                       sizeof(iq->fail_addr.in));
+       }
+#ifdef AF_INET6
+       else if(((struct sockaddr_in*)addr)->sin_family == AF_INET6) {
+               iq->fail_addr_type = 6;
+               memcpy(&iq->fail_addr.in6,
+                       &((struct sockaddr_in6*)addr)->sin6_addr,
+                       sizeof(iq->fail_addr.in6));
+       }
+#endif
+       else {
+               iq->fail_addr_type = 0;
+       }
+}
+
+/** print fail addr to string */
+static void
+print_fail_addr(struct iter_qstate* iq, char* buf, size_t len)
+{
+       if(iq->fail_addr_type == 4) {
+               if(inet_ntop(AF_INET, &iq->fail_addr.in, buf,
+                       (socklen_t)len) == 0)
+                       (void)strlcpy(buf, "(inet_ntop error)", len);
+       }
+#ifdef AF_INET6
+       else if(iq->fail_addr_type == 6) {
+               if(inet_ntop(AF_INET6, &iq->fail_addr.in6, buf,
+                       (socklen_t)len) == 0)
+                       (void)strlcpy(buf, "(inet_ntop error)", len);
+       }
+#endif
+       else
+               (void)strlcpy(buf, "", len);
+}
+
 /** add response specific error information for log servfail */
 static void
 errinf_reply(struct module_qstate* qstate, struct iter_qstate* iq)
@@ -581,16 +629,14 @@ errinf_reply(struct module_qstate* qstate, struct iter_qstate* iq)
        if(qstate->env->cfg->val_log_level < 2 && !qstate->env->cfg->log_servfail)
                return;
        if((qstate->reply && qstate->reply->remote_addrlen != 0) ||
-               (iq->fail_reply && iq->fail_reply->remote_addrlen != 0)) {
+               (iq->fail_addr_type != 0)) {
                char from[256], frm[512];
                if(qstate->reply && qstate->reply->remote_addrlen != 0)
                        addr_to_str(&qstate->reply->remote_addr,
                                qstate->reply->remote_addrlen, from,
                                sizeof(from));
                else
-                       addr_to_str(&iq->fail_reply->remote_addr,
-                               iq->fail_reply->remote_addrlen, from,
-                               sizeof(from));
+                       print_fail_addr(iq, from, sizeof(from));
                snprintf(frm, sizeof(frm), "from %s", from);
                errinf(qstate, frm);
        }
@@ -3199,7 +3245,7 @@ processQueryResponse(struct module_qstate* qstate, struct iter_qstate* iq,
                (*qstate->env->detach_subs)(qstate);
                iq->num_target_queries = 0;
                iq->response = NULL;
-               iq->fail_reply = NULL;
+               iq->fail_addr_type = 0;
                verbose(VERB_ALGO, "cleared outbound list for next round");
                return next_state(iq, QUERYTARGETS_STATE);
        } else if(type == RESPONSE_TYPE_CNAME) {
@@ -4007,7 +4053,8 @@ process_response(struct module_qstate* qstate, struct iter_qstate* iq,
        }
 
        /* parse message */
-       iq->fail_reply = qstate->reply;
+       fill_fail_addr(iq, &qstate->reply->remote_addr,
+               qstate->reply->remote_addrlen);
        prs = (struct msg_parse*)regional_alloc(qstate->env->scratch, 
                sizeof(struct msg_parse));
        if(!prs) {

diff --git a/iterator/iterator.h b/iterator/iterator.h
index 74299e05a23710e95e2f43a9b7c0271d9b05e45f..fad7f03e63deda5ab1c544f12c958a5b642bc7af 100644 (file)
--- a/iterator/iterator.h
+++ b/iterator/iterator.h
@@ -451,7 +451,14 @@ struct iter_qstate {
        /** true if there have been parse failures of reply packets */
        int parse_failures;
        /** a failure printout address for last received answer */
-       struct comm_reply* fail_reply;
+       union {
+               struct in_addr in;
+#ifdef AF_INET6
+               struct in6_addr in6;
+#endif
+       } fail_addr;
+       /** which fail_addr, 0 is nothing, 4 or 6 */
+       int fail_addr_type;
 };
 
 /**

diff --git a/testdata/iter_failreply.rpl b/testdata/iter_failreply.rpl
new file mode 100644 (file)
index 0000000..3937141
--- /dev/null
+++ b/testdata/iter_failreply.rpl
@@ -0,0 +1,132 @@
+; config options
+server:
+       target-fetch-policy: "0 0 0 0 0"
+       qname-minimisation: "no"
+       minimal-responses: no
+       log-servfail: yes
+
+stub-zone:
+       name: "."
+       stub-addr: 193.0.14.129         # K.ROOT-SERVERS.NET.
+CONFIG_END
+
+SCENARIO_BEGIN Test iterator fail_reply report
+
+; K.ROOT-SERVERS.NET.
+RANGE_BEGIN 0 100
+       ADDRESS 193.0.14.129 
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR NOERROR
+SECTION QUESTION
+. IN NS
+SECTION ANSWER
+. IN NS        K.ROOT-SERVERS.NET.
+SECTION ADDITIONAL
+K.ROOT-SERVERS.NET.    IN      A       193.0.14.129
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode subdomain
+ADJUST copy_id copy_query
+REPLY QR NOERROR
+SECTION QUESTION
+example.com. IN NS
+SECTION AUTHORITY
+example.com.   IN NS   ns.example.com.
+example.com.   IN NS   ns2.example.net.
+SECTION ADDITIONAL
+ns.example.com. IN A 1.2.3.4
+ns.example.com. IN AAAA ::1
+ns2.example.net. IN AAAA ::1
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR AA NOERROR
+SECTION QUESTION
+ns2.example.net. IN A
+SECTION ANSWER
+ns2.example.net. IN A 1.2.3.5
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR AA NOERROR
+SECTION QUESTION
+ns2.example.net. IN AAAA
+SECTION ANSWER
+ns2.example.net. IN AAAA ::1
+ENTRY_END
+
+RANGE_END
+
+RANGE_END
+
+; ns.example.com.
+RANGE_BEGIN 0 100
+       ADDRESS 1.2.3.4
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR SERVFAIL
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR SERVFAIL
+SECTION QUESTION
+ns.example.com. IN A
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR SERVFAIL
+SECTION QUESTION
+ns.example.com. IN AAAA
+ENTRY_END
+RANGE_END
+
+STEP 1 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+STEP 20 CHECK_OUT_QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+STEP 21 TIMEOUT
+STEP 22 TIMEOUT
+STEP 23 TIMEOUT
+STEP 24 TIMEOUT
+STEP 25 TIMEOUT
+
+STEP 31 TIMEOUT
+STEP 32 TIMEOUT
+STEP 33 TIMEOUT
+STEP 34 TIMEOUT
+
+; recursion happens here.
+STEP 50 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD RA SERVFAIL
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+ENTRY_END
+
+SCENARIO_END