diff --git a/CHANGES b/CHANGES index a21f159756..02b0a65562 100644 --- 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 index 0000000000..d2f9aad27c --- /dev/null +++ b/bin/tests/system/resolver/ans8/ans.pl @@ -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; + } +} diff --git a/bin/tests/system/resolver/ns4/root.db b/bin/tests/system/resolver/ns4/root.db index da9fd5c901..c15af4e3df 100644 --- a/bin/tests/system/resolver/ns4/root.db +++ b/bin/tests/system/resolver/ns4/root.db @@ -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 diff --git a/bin/tests/system/resolver/tests.sh b/bin/tests/system/resolver/tests.sh index b5fb679e4f..a31ed0b042 100755 --- a/bin/tests/system/resolver/tests.sh +++ b/bin/tests/system/resolver/tests.sh @@ -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 diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 7d00a332d3..bb6b41ad41 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -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); }