]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
[master] improve handling of qcount=0 replies
authorEvan Hunt <each@isc.org>
Tue, 12 Sep 2017 22:26:30 +0000 (15:26 -0700)
committerEvan Hunt <each@isc.org>
Tue, 12 Sep 2017 22:26:30 +0000 (15:26 -0700)
4717. [bug] Treat replies with QCOUNT=0 as truncated if TC=1,
FORMERR if TC=0, and log the error correctly.
[RT #45836]

CHANGES
bin/tests/system/resolver/ans8/ans.pl [new file with mode: 0644]
bin/tests/system/resolver/ns4/root.db
bin/tests/system/resolver/tests.sh
lib/dns/resolver.c

diff --git a/CHANGES b/CHANGES
index a21f15975610a808bdd6157d0dc1e16ac6630fb7..02b0a655622b7f2dd10e13646023d430d70e7407 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,7 @@
+4717.  [bug]           Treat replies with QCOUNT=0 as truncated if TC=1,
+                       FORMERR if TC=0, and log the error correctly.
+                       [RT #45836]
+
 4716.  [placeholder]
 
        --- 9.12.0a1 released ---
diff --git a/bin/tests/system/resolver/ans8/ans.pl b/bin/tests/system/resolver/ans8/ans.pl
new file mode 100644 (file)
index 0000000..d2f9aad
--- /dev/null
@@ -0,0 +1,142 @@
+#!/usr/bin/perl
+#
+# Copyright (C) 2011, 2012, 2014, 2016  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/.
+
+use IO::File;
+use IO::Socket;
+use Data::Dumper;
+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.8";
+my $udpsock = IO::Socket::INET->new(LocalAddr => "$server_addr",
+   LocalPort => 5300, Proto => "udp", Reuse => 1) or die "$!";
+my $tcpsock = IO::Socket::INET->new(LocalAddr => "$server_addr",
+   LocalPort => 5300, Proto => "tcp", Listen => 5, Reuse => 1) or die "$!";
+
+print "listening on $server_addr:5300.\n";
+
+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;
+
+sub handleUDP {
+       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();
+
+       $packet->header->qr(1);
+       $packet->header->aa(0);
+       $packet->header->id($id);
+
+       if ($qname eq "truncated.no-questions") {
+               $packet->header->tc(1);
+       } else {
+               $packet->header->tc(0);
+       }
+
+       return $packet->data;
+}
+
+sub handleTCP {
+       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 @results = ();
+       my $packet = new Net::DNS::Packet($qname, $qtype, $qclass);
+
+       $packet->header->qr(1);
+       $packet->header->aa(1);
+       $packet->header->id($id);
+
+       $packet->push("answer", new Net::DNS::RR("$qname 300 A 1.2.3.4"));
+       push(@results, $packet->data);
+
+       return \@results;
+}
+
+# Main
+my $rin;
+my $rout;
+for (;;) {
+       $rin = '';
+       vec($rin, fileno($tcpsock), 1) = 1;
+       vec($rin, fileno($udpsock), 1) = 1;
+
+       select($rout = $rin, undef, undef, undef);
+
+       if (vec($rout, fileno($udpsock), 1)) {
+               printf "UDP request\n";
+               my $buf;
+               $udpsock->recv($buf, 512);
+               my $result = handleUDP($buf);
+               my $num_chars = $udpsock->send($result);
+               print "  Sent $num_chars bytes via UDP\n";
+       } elsif (vec($rout, fileno($tcpsock), 1)) {
+               my $conn = $tcpsock->accept;
+               my $buf;
+               for (;;) {
+                       my $lenbuf;
+                       my $n = $conn->sysread($lenbuf, 2);
+                       last unless $n == 2;
+                       my $len = unpack("n", $lenbuf);
+                       $n = $conn->sysread($buf, $len);
+                       last unless $n == $len;
+                       print "TCP request\n";
+                       my $result = handleTCP($buf);
+                       foreach my $response (@$result) {
+                               $len = length($response);
+                               $n = $conn->syswrite(pack("n", $len), 2);
+                               $n = $conn->syswrite($response, $len);
+                               print "    Sent: $n chars via TCP\n";
+                       }
+               }
+               $conn->close;
+       }
+}
index da9fd5c901a3c31ca38c334117a6a70b8fe7fbce..c15af4e3dfdde217459f6900684e0d3f871173a4 100644 (file)
@@ -21,3 +21,5 @@ delegation-only.      NS      ns.delegation-only.
 ns.delegation-only.    A       10.53.0.6
 example.net.           NS      ns.example.net.
 ns.example.net.                A       10.53.0.6
+no-questions.          NS      ns.no-questions.
+ns.no-questions.       A       10.53.0.8
index b5fb679e4fa1a606c1b51ba4e536fd12bdc32cdf..a31ed0b042e31a463f4ace5b8e9e027d3baf25b6 100755 (executable)
@@ -750,5 +750,25 @@ grep "flags: qr aa tc; QUERY: 1, ANSWER: 0, AUTHORITY: 0" dig.out.$n > /dev/null
 if [ $ret != 0 ]; then echo "I:failed"; fi
 status=`expr $status + $ret`
 
+n=`expr $n + 1`
+echo "I: check that the resolver accepts a reply with empty question section with TC=1 and retries over TCP ($n)"
+ret=0
+$DIG @10.53.0.5 -p 5300 truncated.no-questions. a > dig.ns5.out.${n} || ret=1
+grep "status: NOERROR" dig.ns5.out.${n} > /dev/null || ret=1
+grep "ANSWER: 1," dig.ns5.out.${n} > /dev/null || ret=1
+grep "1.2.3.4" dig.ns5.out.${n} > /dev/null || ret=1
+if [ $ret != 0 ]; then echo "I:failed"; fi
+status=`expr $status + $ret`
+
+n=`expr $n + 1`
+echo "I: check that the resolver rejects a reply with empty question section with TC=0 ($n)"
+ret=0
+$DIG @10.53.0.5 -p 5300 not-truncated.no-questions. a > dig.ns5.out.${n} || ret=1
+grep "status: NOERROR" dig.ns5.out.${n} > /dev/null && ret=1
+grep "ANSWER: 1," dig.ns5.out.${n} > /dev/null && ret=1
+grep "1.2.3.4" dig.ns5.out.${n} > /dev/null && ret=1
+if [ $ret != 0 ]; then echo "I:failed"; fi
+status=`expr $status + $ret`
+
 echo "I:exit status: $status"
 [ $status -eq 0 ] || exit 1
index 7d00a332d3a6d8fc04195c818a7f11cdd0eb09d4..bb6b41ad41f14ff44a84fdba863275ee3efc9a36 100644 (file)
@@ -4856,7 +4856,7 @@ log_formerr(fetchctx_t *fctx, const char *format, ...) {
                      nsbuf, fctx->info, clmsg, clbuf, msgbuf);
 }
 
-static inline isc_result_t
+static isc_result_t
 same_question(fetchctx_t *fctx) {
        isc_result_t result;
        dns_message_t *message = fctx->rmessage;
@@ -4870,7 +4870,32 @@ same_question(fetchctx_t *fctx) {
        /*
         * XXXRTH  Currently we support only one question.
         */
-       if (message->counts[DNS_SECTION_QUESTION] != 1) {
+       if (ISC_UNLIKELY(message->counts[DNS_SECTION_QUESTION] == 0)) {
+               if ((message->flags & DNS_MESSAGEFLAG_TC) != 0) {
+                       /*
+                        * If TC=1 and the question section is empty, we
+                        * accept the reply message as a truncated
+                        * answer, to be retried over TCP.
+                        *
+                        * It is really a FORMERR condition, but this is
+                        * a workaround to accept replies from some
+                        * implementations.
+                        *
+                        * Because the question section matching is not
+                        * performed, the worst that could happen is
+                        * that an attacker who gets past the ID and
+                        * source port checks can force the use of
+                        * TCP. This is considered an acceptable risk.
+                        */
+                       log_formerr(fctx,
+                                   "empty question section, "
+                                   "accepting it anyway as TC=1");
+                       return (ISC_R_SUCCESS);
+               } else {
+                       log_formerr(fctx, "empty question section");
+                       return (DNS_R_FORMERR);
+               }
+       } else if (ISC_UNLIKELY(message->counts[DNS_SECTION_QUESTION] > 1)) {
                log_formerr(fctx, "too many questions");
                return (DNS_R_FORMERR);
        }