]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
dig: return a non-zero exit code for failed TCP EOF retries
authorMichał Kępień <michal@isc.org>
Fri, 18 Jan 2019 10:15:19 +0000 (11:15 +0100)
committerEvan Hunt <each@isc.org>
Fri, 25 Jan 2019 07:05:43 +0000 (23:05 -0800)
dig retries a TCP query when a server closes the connection prematurely.
However, dig's exit code remains unaffected even if the second attempt
to get a response also fails with the same error for the same lookup,
which should not be the case.  Ensure the exit code is updated
appropriately when a retry triggered by a TCP EOF condition fails.

bin/dig/dighost.c
bin/tests/system/digdelv/ans5/ans.pl [new file with mode: 0644]
bin/tests/system/digdelv/tests.sh
util/copyrights

index d4fddf0cac90f342b4070acad078231b67c72a40..f3904d72632637245dec8281dd0ee66649d95e9e 100644 (file)
@@ -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);
diff --git a/bin/tests/system/digdelv/ans5/ans.pl b/bin/tests/system/digdelv/ans5/ans.pl
new file mode 100644 (file)
index 0000000..2adfd13
--- /dev/null
@@ -0,0 +1,174 @@
+#!/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;
+       }
+}
index 11408b4d7b55285103661fe0b4a09cbfcef15130..c031b20a6211b87f4d247639975ab0f7732732e5 100644 (file)
@@ -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
index 6ef35b4ca1c35f0c0fdf4a7af7540b773ca50ae5..dd6db610e2a3e58487bc1583c9eabd30d48b5d3b 100644 (file)
 ./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