Merge branch '3020-dighost-servfail-bug-v9_18' into 'v9_18'

[v9_18] When resending a UDP request, insert the query to the lookup's list

See merge request isc-projects/bind9!5998

CHANGES

@@ -1,3 +1,9 @@
+5831.	[bug]		When resending a UDP request in the result of a timeout,
+			the recv_done() function in dighost.c was prepending
+			the new query into the loookup's queries list instead
+			of inserting, which could cause an assertion failure
+			when the resent query's result was SERVFAIL. [GL #3020]
+
 5828.	[bug]		Replace single TCP write timer with per-TCP write
 			timers. [GL #3200]
 

bin/dig/dighost.c

@@ -3061,7 +3061,8 @@ force_next(dig_query_t *query) {
 
 	if (l->retries > 1) {
 		l->retries--;
-		debug("making new TCP request, %d tries left", l->retries);
+		debug("making new %s request, %d tries left",
+		      l->tcp_mode ? "TCP" : "UDP", l->retries);
 		requeue_lookup(l, true);
 		lookup_detach(&l);
 		isc_refcount_decrement0(&recvcount);
@@ -3625,13 +3626,14 @@ recv_done(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region,
 		dig_query_t *newq = NULL;
 
 		l->retries--;
-		debug("resending UDP request to first server, %d tries left",
-		      l->retries);
+		debug("making new UDP request, %d tries left", l->retries);
 		newq = new_query(l, query->servname, query->userarg);
 
-		ISC_LIST_PREPEND(l->q, newq, link);
-
-		start_udp(ISC_LIST_HEAD(l->q));
+		ISC_LIST_INSERTAFTER(l->q, query, newq, link);
+		if (l->current_query == query) {
+			query_detach(&l->current_query);
+		}
+		start_udp(newq);
 		goto detach_query;
 	}
 
@@ -3923,15 +3925,6 @@ recv_done(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region,
 			} else {
 				start_udp(next);
 			}
-		}
-
-		/*
-		 * If our query is at the head of the list and there
-		 * is no next, we're the only one left, so fall
-		 * through to print the message.
-		 */
-		if ((ISC_LIST_HEAD(l->q) != query) ||
-		    (ISC_LIST_NEXT(query, link) != NULL)) {
 			dighost_comments(l,
 					 "Got %s from %s, trying next "
 					 "server",
@@ -3939,7 +3932,7 @@ recv_done(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region,
 						 ? "SERVFAIL reply"
 						 : "recursion not available",
 					 query->servname);
-			goto next_lookup;
+			goto detach_query;
 		}
 	}
 

bin/tests/system/digdelv/ans8/ans.py

@@ -0,0 +1,138 @@
+# Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+#
+# SPDX-License-Identifier: MPL-2.0
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0.  If a copy of the MPL was not distributed with this
+# file, you can obtain one at https://mozilla.org/MPL/2.0/.
+#
+# See the COPYRIGHT file distributed with this work for additional
+# information regarding copyright ownership.
+
+from __future__ import print_function
+import os
+import sys
+import signal
+import socket
+import select
+import struct
+
+import dns, dns.message
+from dns.rcode import *
+
+def create_response(msg):
+    m = dns.message.from_wire(msg)
+    qname = m.question[0].name.to_text()
+    rrtype = m.question[0].rdtype
+    typename = dns.rdatatype.to_text(rrtype)
+
+    with open("query.log", "a") as f:
+        f.write("%s %s\n" % (typename, qname))
+        print("%s %s" % (typename, qname), end=" ")
+
+    r = dns.message.make_response(m)
+    r.set_rcode(SERVFAIL)
+    return r
+
+def sigterm(signum, frame):
+    print("Shutting down now...")
+    os.remove("ans.pid")
+    running = False
+    sys.exit(0)
+
+ip4 = "10.53.0.8"
+
+try: port=int(os.environ["PORT"])
+except: port=5300
+
+query4_udp = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
+query4_udp.bind((ip4, port))
+
+query4_tcp = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+query4_tcp.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+query4_tcp.bind((ip4, port))
+query4_tcp.listen(100)
+
+signal.signal(signal.SIGTERM, sigterm)
+
+f = open("ans.pid", "w")
+pid = os.getpid()
+print (pid, file=f)
+f.close()
+
+running = True
+
+print ("Listening on %s port %d" % (ip4, port))
+print ("Ctrl-c to quit")
+
+input = [query4_udp, query4_tcp]
+
+n_udp = 0
+n_tcp = 0
+hung_conns = []
+
+while running:
+    try:
+        inputready, outputready, exceptready = select.select(input, [], [])
+    except select.error as e:
+        break
+    except socket.error as e:
+        break
+    except KeyboardInterrupt:
+        break
+
+    for s in inputready:
+        if s == query4_udp:
+            print("UDP query received on %s" % ip4, end=" ")
+            n_udp = n_udp + 1
+            msg = s.recvfrom(65535)
+            # Do not response to every other query.
+            if n_udp % 2 == 1:
+                print("NO RESPONSE")
+                continue
+            rsp = create_response(msg[0])
+            if rsp:
+                print(dns.rcode.to_text(rsp.rcode()))
+                s.sendto(rsp.to_wire(), msg[1])
+            else:
+                print("NO RESPONSE")
+        elif s == query4_tcp:
+            print("TCP query received on %s" % ip4, end=" ")
+            n_tcp = n_tcp + 1
+            conn = None
+            try:
+                conn, addr = s.accept()
+                # Do not response to every other query, hang the connection.
+                if n_tcp % 2 == 1:
+                    print("NO RESPONSE")
+                    hung_conns.append(conn)
+                    conn = None
+                    continue
+                else:
+                    # get TCP message length
+                    msg = conn.recv(2)
+                    if len(msg) != 2:
+                        print("NO RESPONSE")
+                        conn.close()
+                        continue
+                    length = struct.unpack('>H', msg[:2])[0]
+                    msg = conn.recv(length)
+                    if len(msg) != length:
+                        print("NO RESPONSE")
+                        conn.close()
+                        continue
+                    rsp = create_response(msg)
+                    if rsp:
+                        print(dns.rcode.to_text(rsp.rcode()))
+                        wire = rsp.to_wire()
+                        conn.send(struct.pack('>H', len(wire)))
+                        conn.send(wire)
+                    else:
+                        print("NO RESPONSE")
+            except:
+                print("NO RESPONSE")
+            if conn:
+                conn.close()
+
+    if not running:
+        break

bin/tests/system/digdelv/clean.sh

@@ -17,6 +17,8 @@ rm -f ./*/anchor.*
 rm -f ./*/named.conf
 rm -f ./*/named.memstats
 rm -f ./*/named.run
+rm -f ./ans*/ans.run
+rm -f ./ans*/query.log
 rm -f ./delv.out.test*
 rm -f ./dig.out.*test*
 rm -f ./dig.out.mm.*

bin/tests/system/digdelv/tests.sh

@@ -998,6 +998,22 @@ if [ -x "$DIG" ] ; then
   if [ $ret -ne 0 ]; then echo_i "failed"; fi
   status=$((status+ret))
 
+  # See [GL #3020] for more information
+  n=$((n+1))
+  echo_i "check that dig handles UDP timeout followed by a SERVFAIL correctly ($n)"
+  ret=0
+  dig_with_opts +timeout=1 +nofail @10.53.0.8 a.example > dig.out.test$n 2>&1 || ret=1
+  grep "status: SERVFAIL" dig.out.test$n > /dev/null || ret=1
+  if [ $ret -ne 0 ]; then echo_i "failed"; fi
+  status=$((status+ret))
+
+  n=$((n+1))
+  echo_i "check that dig handles TCP timeout followed by a SERVFAIL correctly ($n)"
+  ret=0
+  dig_with_opts +timeout=1 +nofail +tcp @10.53.0.8 a.example > dig.out.test$n 2>&1 || ret=1
+  grep "status: SERVFAIL" dig.out.test$n > /dev/null || ret=1
+  if [ $ret -ne 0 ]; then echo_i "failed"; fi
+  status=$((status+ret))
 else
   echo_i "$DIG is needed, so skipping these dig tests"
 fi