Merge branch '820-dig-return-a-non-zero-exit-code-for-failed-tcp-eof-retries' into 'master'

dig: return a non-zero exit code for failed TCP EOF retries

Closes #820

See merge request !1358

CHANGES

+5144.	[bug]		dig now returns a non-zero exit code when a TCP
+			connection is prematurely closed by a peer more than
+			once for the same lookup.  [GL #820]
+
 5143.	[bug]		dnssec-keymgr and dnssec-coverage failed to find
 			key files for zone names ending in ".". [GL #560]
 

bin/dig/dighost.c

@@ -2991,6 +2991,27 @@ connect_timeout(isc_task_t *task, isc_event_t *event) {
 	UNLOCK_LOOKUP;
 }
 
+/*%
+ * Called when a peer closes a TCP socket prematurely.
+ */
+static void
+requeue_or_update_exitcode(dig_lookup_t *lookup) {
+	if (lookup->eoferr == 0U) {
+		/*
+		 * Peer closed the connection prematurely for the first time
+		 * for this lookup.  Try again, keeping track of this failure.
+		 */
+		dig_lookup_t *requeued_lookup = requeue_lookup(lookup, true);
+		requeued_lookup->eoferr++;
+	} else {
+		/*
+		 * Peer closed the connection prematurely and it happened
+		 * previously for this lookup.  Indicate an error.
+		 */
+		exitcode = 9;
+	}
+}
+
 /*%
  * Event handler for the TCP recv which gets the length header of TCP
  * packets.  Start the next recv of length bytes.
@@ -3002,7 +3023,7 @@ tcp_length_done(isc_task_t *task, isc_event_t *event) {
 	isc_region_t r;
 	isc_result_t result;
 	dig_query_t *query = NULL;
-	dig_lookup_t *l, *n;
+	dig_lookup_t *l;
 	uint16_t length;
 
 	REQUIRE(event->ev_type == ISC_SOCKEVENT_RECVDONE);
@@ -3041,9 +3062,8 @@ tcp_length_done(isc_task_t *task, isc_event_t *event) {
 		sockcount--;
 		debug("sockcount=%d", sockcount);
 		INSIST(sockcount >= 0);
-		if (sevent->result == ISC_R_EOF && l->eoferr == 0U) {
-			n = requeue_lookup(l, true);
-			n->eoferr++;
+		if (sevent->result == ISC_R_EOF) {
+			requeue_or_update_exitcode(l);
 		}
 		isc_event_free(&event);
 		clear_query(query);
@@ -3544,9 +3564,8 @@ recv_done(isc_task_t *task, isc_event_t *event) {
 			debug("sockcount=%d", sockcount);
 			INSIST(sockcount >= 0);
 		}
-		if (sevent->result == ISC_R_EOF && l->eoferr == 0U) {
-			n = requeue_lookup(l, true);
-			n->eoferr++;
+		if (sevent->result == ISC_R_EOF) {
+			requeue_or_update_exitcode(l);
 		}
 		isc_event_free(&event);
 		clear_query(query);

bin/tests/system/digdelv/ans5/ans.pl

+#!/usr/bin/perl
+#
+# Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+#
+# 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 http://mozilla.org/MPL/2.0/.
+#
+# See the COPYRIGHT file distributed with this work for additional
+# information regarding copyright ownership.
+
+# This is a TCP-only DNS server whose aim is to facilitate testing how dig
+# copes with prematurely closed TCP connections.
+#
+# This server can be configured (through a separate control socket) with a
+# series of responses to send for subsequent incoming TCP DNS queries.  Only
+# one query is handled before closing each connection.  In order to keep things
+# simple, the server is not equipped with any mechanism for handling malformed
+# queries.
+#
+# Available response types are defined in the %response_types hash in the
+# getAnswerSection() function below.  Each RR returned is generated dynamically
+# based on the QNAME found in the incoming query.
+
+use IO::File;
+use Net::DNS;
+use Net::DNS::Packet;
+
+use strict;
+
+# Ignore SIGPIPE so we won't fail if peer closes a TCP socket early
+local $SIG{PIPE} = 'IGNORE';
+
+# Flush logged output after every line
+local $| = 1;
+
+my $server_addr = "10.53.0.5";
+if (@ARGV > 0) {
+	$server_addr = @ARGV[0];
+}
+
+my $mainport = int($ENV{'PORT'});
+if (!$mainport) { $mainport = 5300; }
+my $ctrlport = int($ENV{'EXTRAPORT1'});
+if (!$ctrlport) { $ctrlport = 5301; }
+
+my $ctlsock = IO::Socket::INET->new(LocalAddr => "$server_addr",
+   LocalPort => $ctrlport, Proto => "tcp", Listen => 5, Reuse => 1) or die "$!";
+
+my $tcpsock = IO::Socket::INET->new(LocalAddr => "$server_addr",
+   LocalPort => $mainport, Proto => "tcp", Listen => 5, Reuse => 1) or die "$!";
+
+my $pidf = new IO::File "ans.pid", "w" or die "cannot open pid file: $!";
+print $pidf "$$\n" or die "cannot write pid file: $!";
+$pidf->close or die "cannot close pid file: $!";;
+sub rmpid { unlink "ans.pid"; exit 1; };
+
+$SIG{INT} = \&rmpid;
+$SIG{TERM} = \&rmpid;
+
+my @response_sequence = ("complete_axfr");
+my $connection_counter = 0;
+
+# Return the next answer type to send, incrementing the connection counter and
+# making sure the latter does not exceed the size of the array holding the
+# configured response sequence.
+sub getNextResponseType {
+	my $response_type = $response_sequence[$connection_counter];
+
+	$connection_counter++;
+	$connection_counter %= scalar(@response_sequence);
+
+	return $response_type;
+}
+
+# Return an array of resource records comprising the answer section of a given
+# response type.
+sub getAnswerSection {
+	my ($response_type, $qname) = @_;
+
+	my %response_types = (
+		no_response => [],
+
+		partial_axfr => [
+			Net::DNS::RR->new("$qname 300 IN SOA . . 0 0 0 0 300"),
+			Net::DNS::RR->new("$qname NS ."),
+		],
+
+		complete_axfr => [
+			Net::DNS::RR->new("$qname 300 IN SOA . . 0 0 0 0 300"),
+			Net::DNS::RR->new("$qname NS ."),
+			Net::DNS::RR->new("$qname 300 IN SOA . . 0 0 0 0 300"),
+		],
+	);
+
+	return $response_types{$response_type};
+}
+
+
+# Generate a Net::DNS::Packet containing the response to send on the current
+# TCP connection.  If the answer section of the response is determined to be
+# empty, no data will be sent on the connection at all (immediate EOF).
+sub generateResponse {
+	my ($buf) = @_;
+	my $request;
+
+	if ($Net::DNS::VERSION > 0.68) {
+		$request = new Net::DNS::Packet(\$buf, 0);
+		$@ and die $@;
+	} else {
+		my $err;
+		($request, $err) = new Net::DNS::Packet(\$buf, 0);
+		$err and die $err;
+	}
+
+	my @questions = $request->question;
+	my $qname = $questions[0]->qname;
+	my $qtype = $questions[0]->qtype;
+	my $qclass = $questions[0]->qclass;
+	my $id = $request->header->id;
+
+	my $packet = new Net::DNS::Packet($qname, $qtype, $qclass);
+	$packet->header->qr(1);
+	$packet->header->aa(1);
+	$packet->header->id($id);
+
+	my $response_type = getNextResponseType();
+	my $answers = getAnswerSection($response_type, $qname);
+	for my $rr (@$answers) {
+		$packet->push("answer", $rr);
+	}
+
+	print "    Sending \"$response_type\" response\n";
+
+	return $packet->data if @$answers;
+}
+
+my $rin;
+my $rout;
+for (;;) {
+	$rin = '';
+	vec($rin, fileno($ctlsock), 1) = 1;
+	vec($rin, fileno($tcpsock), 1) = 1;
+
+	select($rout = $rin, undef, undef, undef);
+
+	if (vec($rout, fileno($ctlsock), 1)) {
+		my $conn = $ctlsock->accept;
+		@response_sequence = split(' ', $conn->getline);
+		$connection_counter = 0;
+		print "Response sequence set to: @response_sequence\n";
+		$conn->close;
+	} elsif (vec($rout, fileno($tcpsock), 1)) {
+		my $buf;
+		my $lenbuf;
+		my $conn = $tcpsock->accept;
+		my $n = $conn->sysread($lenbuf, 2);
+		die unless $n == 2;
+		my $len = unpack("n", $lenbuf);
+		$n = $conn->sysread($buf, $len);
+		die unless $n == $len;
+		print "TCP request\n";
+		my $response = generateResponse($buf);
+		if ($response) {
+			$len = length($response);
+			$n = $conn->syswrite(pack("n", $len), 2);
+			$n = $conn->syswrite($response, $len);
+			print "    Sent: $n chars via TCP\n";
+		} else {
+			print "    No response sent\n";
+		}
+		$conn->close;
+	}
+}

bin/tests/system/digdelv/tests.sh

@@ -18,7 +18,7 @@ status=0
 n=0
 
 sendcmd() {
-    "$PERL" "$SYSTEMTESTTOP/send.pl" 10.53.0.4 "$EXTRAPORT1"
+    "$PERL" "$SYSTEMTESTTOP/send.pl" "${1}" "$EXTRAPORT1"
 }
 
 dig_with_opts() {
@@ -247,7 +247,7 @@ if [ -x "$DIG" ] ; then
   echo_i "checking dig preserves origin on TCP retries ($n)"
   ret=0
   # Ask ans4 to still accept TCP connections, but not respond to queries
-  echo "//" | sendcmd
+  echo "//" | sendcmd 10.53.0.4
   dig_with_opts -d +tcp @10.53.0.4 +retry=1 +time=1 +domain=bar foo > dig.out.test$n 2>&1 && ret=1
   test "$(grep -c "trying origin bar" dig.out.test$n)" -eq 2 || ret=1
   grep "using root origin" < dig.out.test$n > /dev/null && ret=1
@@ -524,6 +524,66 @@ if [ -x "$DIG" ] ; then
   grep "Dump of all outstanding memory allocations" 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 "checking exit code for a retry upon TCP EOF (immediate -> immediate) ($n)"
+  ret=0
+  echo "no_response no_response" | sendcmd 10.53.0.5
+  dig_with_opts @10.53.0.5 example AXFR +tries=1 > dig.out.test$n 2>&1 && ret=1
+  # Sanity check: ensure ans5 behaves as expected.
+  [ `grep "communications error.*end of file" dig.out.test$n | wc -l` -eq 2 ] || ret=1
+  if [ $ret -ne 0 ]; then echo_i "failed"; fi
+  status=$((status+ret))
+
+  n=$((n+1))
+  echo_i "checking exit code for a retry upon TCP EOF (partial AXFR -> partial AXFR) ($n)"
+  ret=0
+  echo "partial_axfr partial_axfr" | sendcmd 10.53.0.5
+  dig_with_opts @10.53.0.5 example AXFR +tries=1 > dig.out.test$n 2>&1 && ret=1
+  # Sanity check: ensure ans5 behaves as expected.
+  [ `grep "communications error.*end of file" dig.out.test$n | wc -l` -eq 2 ] || ret=1
+  if [ $ret -ne 0 ]; then echo_i "failed"; fi
+  status=$((status+ret))
+
+  n=$((n+1))
+  echo_i "checking exit code for a retry upon TCP EOF (immediate -> partial AXFR) ($n)"
+  ret=0
+  echo "no_response partial_axfr" | sendcmd 10.53.0.5
+  dig_with_opts @10.53.0.5 example AXFR +tries=1 > dig.out.test$n 2>&1 && ret=1
+  # Sanity check: ensure ans5 behaves as expected.
+  [ `grep "communications error.*end of file" dig.out.test$n | wc -l` -eq 2 ] || ret=1
+  if [ $ret -ne 0 ]; then echo_i "failed"; fi
+  status=$((status+ret))
+
+  n=$((n+1))
+  echo_i "checking exit code for a retry upon TCP EOF (partial AXFR -> immediate) ($n)"
+  ret=0
+  echo "partial_axfr no_response" | sendcmd 10.53.0.5
+  dig_with_opts @10.53.0.5 example AXFR +tries=1 > dig.out.test$n 2>&1 && ret=1
+  # Sanity check: ensure ans5 behaves as expected.
+  [ `grep "communications error.*end of file" dig.out.test$n | wc -l` -eq 2 ] || ret=1
+  if [ $ret -ne 0 ]; then echo_i "failed"; fi
+  status=$((status+ret))
+
+  n=$((n+1))
+  echo_i "checking exit code for a retry upon TCP EOF (immediate -> complete AXFR) ($n)"
+  ret=0
+  echo "no_response complete_axfr" | sendcmd 10.53.0.5
+  dig_with_opts @10.53.0.5 example AXFR +tries=1 > dig.out.test$n 2>&1 || ret=1
+  # Sanity check: ensure ans5 behaves as expected.
+  [ `grep "communications error.*end of file" dig.out.test$n | wc -l` -eq 1 ] || ret=1
+  if [ $ret -ne 0 ]; then echo_i "failed"; fi
+  status=$((status+ret))
+
+  n=$((n+1))
+  echo_i "checking exit code for a retry upon TCP EOF (partial AXFR -> complete AXFR) ($n)"
+  ret=0
+  echo "partial_axfr complete_axfr" | sendcmd 10.53.0.5
+  dig_with_opts @10.53.0.5 example AXFR +tries=1 > dig.out.test$n 2>&1 || ret=1
+  # Sanity check: ensure ans5 behaves as expected.
+  [ `grep "communications error.*end of file" dig.out.test$n | wc -l` -eq 1 ] || 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

util/copyrights

@@ -493,6 +493,7 @@
 ./bin/tests/system/dialup/tests.sh		SH	2000,2001,2004,2007,2012,2016,2018,2019
 ./bin/tests/system/digcomp.pl			PERL	2000,2001,2004,2007,2012,2013,2016,2018,2019
 ./bin/tests/system/digdelv/ans4/startme		X	2017,2018,2019
+./bin/tests/system/digdelv/ans5/ans.pl		PERL	2019
 ./bin/tests/system/digdelv/clean.sh		SH	2015,2016,2018,2019
 ./bin/tests/system/digdelv/ns2/sign.sh		SH	2018,2019
 ./bin/tests/system/digdelv/prereq.sh		SH	2018,2019