]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
3148. [bug] Processing of normal queries could be stalled when
authorMark Andrews <marka@isc.org>
Wed, 31 Aug 2011 06:49:10 +0000 (06:49 +0000)
committerMark Andrews <marka@isc.org>
Wed, 31 Aug 2011 06:49:10 +0000 (06:49 +0000)
                        forwarding a UPDATE message. [RT #24711]

CHANGES
bin/named/update.c
bin/tests/system/upforwd/ans4/ans.pl [new file with mode: 0644]
bin/tests/system/upforwd/ns3/named.conf
bin/tests/system/upforwd/ns3/nomaster.db [new file with mode: 0644]
bin/tests/system/upforwd/tests.sh
lib/dns/zone.c

diff --git a/CHANGES b/CHANGES
index d90e7a15c7db50880d2c4929e43b259dd443e004..cffff9792c2b0394c5fbde28fb829b9e22fb41cf 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+3148.  [bug]           Processing of normal queries could be stalled when
+                       forwarding a UPDATE message. [RT #24711]
+
 3147.  [func]          Initial inline signing support.  [RT #23657]
 
        --- 9.9.0a1 released ---
index 46e3403ba56d32184057ce82a380d7f1127368c4..c51ff43f9eec72e234673fd4803068d0abdd63b0 100644 (file)
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: update.c,v 1.196 2011/08/30 05:16:11 marka Exp $ */
+/* $Id: update.c,v 1.197 2011/08/31 06:49:09 marka Exp $ */
 
 #include <config.h>
 
@@ -3346,6 +3346,12 @@ send_forward_event(ns_client_t *client, dns_zone_t *zone) {
        isc_task_t *zonetask = NULL;
        ns_client_t *evclient;
 
+       /*
+        * This may take some time so replace this client.
+        */
+       if (!client->mortal && (client->attributes & NS_CLIENTATTR_TCP) == 0)
+               CHECK(ns_client_replace(client));
+
        event = (update_event_t *)
                isc_event_allocate(client->mctx, client, DNS_EVENT_UPDATE,
                                   forward_action, NULL, sizeof(*event));
diff --git a/bin/tests/system/upforwd/ans4/ans.pl b/bin/tests/system/upforwd/ans4/ans.pl
new file mode 100644 (file)
index 0000000..c000a52
--- /dev/null
@@ -0,0 +1,351 @@
+#!/usr/bin/perl
+#
+# Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+#
+# Permission to use, copy, modify, and/or distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+# REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+# AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+# LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+# OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+# PERFORMANCE OF THIS SOFTWARE.
+
+# $Id: ans.pl,v 1.2 2011/08/31 06:49:10 marka Exp $
+
+#
+# This is the name server from hell.  It provides canned
+# responses based on pattern matching the queries, and
+# can be reprogrammed on-the-fly over a TCP connection.
+#
+# The server listens for control connections on port 5301.
+# A control connection is a TCP stream of lines like
+#
+#  /pattern/
+#  name ttl type rdata
+#  name ttl type rdata
+#  ...
+#  /pattern/
+#  name ttl type rdata
+#  name ttl type rdata
+#  ...
+#
+# There can be any number of patterns, each associated
+# with any number of response RRs.  Each pattern is a
+# Perl regular expression.
+#
+# Each incoming query is converted into a string of the form
+# "qname qtype" (the printable query domain name, space,
+# printable query type) and matched against each pattern.
+#
+# The first pattern matching the query is selected, and
+# the RR following the pattern line are sent in the
+# answer section of the response.
+#
+# Each new control connection causes the current set of
+# patterns and responses to be cleared before adding new
+# ones.
+#
+# The server handles UDP and TCP queries.  Zone transfer
+# responses work, but must fit in a single 64 k message.
+#
+# Now you can add TSIG, just specify key/key data with:
+#
+#  /pattern <key> <key_data>/
+#  name ttl type rdata
+#  name ttl type rdata
+#
+#  Note that this data will still be sent with any request for
+#  pattern, only this data will be signed. Currently, this is only
+#  done for TCP.
+
+
+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.4";
+
+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;
+
+#my @answers = ();
+my @rules;
+sub handleUDP {
+        my ($buf) = @_;
+
+        my ($packet, $err) = new Net::DNS::Packet(\$buf, 0);
+        $err and die $err;
+
+        $packet->header->qr(1);
+        $packet->header->aa(1);
+
+        my @questions = $packet->question;
+        my $qname = $questions[0]->qname;
+        my $qtype = $questions[0]->qtype;
+
+        # get the existing signature if any, and clear the additional section
+        my $prev_tsig;
+        while (my $rr = $packet->pop("additional")) {
+                if ($rr->type eq "TSIG") {
+                        $prev_tsig = $rr;
+                }
+        }
+
+        my $r;
+        foreach $r (@rules) {
+                my $pattern = $r->{pattern};
+               my($dbtype, $key_name, $key_data) = split(/ /,$pattern);
+               print "[handleUDP] $dbtype, $key_name, $key_data \n";
+                if ("$qname $qtype" =~ /$dbtype/) {
+                        my $a;
+                        foreach $a (@{$r->{answer}}) {
+                                $packet->push("answer", $a);
+                        }
+                       if(defined($key_name) && defined($key_data)) {
+                               # Sign the packet
+                               print "  Signing the response with " .
+                                      "$key_name/$key_data\n";
+                                my $tsig = Net::DNS::RR->
+                                        new("$key_name TSIG $key_data");
+
+                                # These kluges are necessary because Net::DNS
+                                # doesn't know how to sign responses.  We
+                                # clear compnames so that the TSIG key and
+                                # algorithm name won't be compressed, and
+                                # add one to arcount because the signing
+                                # function will attempt to decrement it,
+                                # which is incorrect in a response. Finally
+                                # we set request_mac to the previous digest.
+                                $packet->{"compnames"} = {};
+                                $packet->{"header"}{"arcount"} += 1;
+                                if (defined($prev_tsig)) {
+                                        my $rmac = pack('n H*',
+                                                $prev_tsig->mac_size,
+                                                $prev_tsig->mac);
+                                        $tsig->{"request_mac"} =
+                                                unpack("H*", $rmac);
+                                }
+                                
+                               $packet->sign_tsig($tsig);
+                       }
+                        last;
+                }
+        }
+        #$packet->print;
+
+        return $packet->data;
+}
+
+# namelen:
+# given a stream of data, reads a DNS-formatted name and returns its
+# total length, thus making it possible to skip past it.
+sub namelen {
+        my ($data) = @_;
+        my $len = 0;
+        my $label_len = 0;
+        do {
+                $label_len = unpack("c", $data);
+                $data = substr($data, $label_len + 1);
+                $len += $label_len + 1;
+        } while ($label_len != 0);
+        return ($len);
+}
+
+# packetlen:
+# given a stream of data, reads a DNS wire-format packet and returns
+# its total length, making it possible to skip past it.
+sub packetlen {
+        my ($data) = @_;
+        my $q;
+        my $rr;
+
+        my ($header, $offset) = Net::DNS::Header->parse(\$data);
+        for (1 .. $header->qdcount) {
+                ($q, $offset) = Net::DNS::Question->parse(\$data, $offset);
+        }
+        for (1 .. $header->ancount) {
+                ($rr, $offset) = Net::DNS::RR->parse(\$data, $offset);
+        }
+        for (1 .. $header->nscount) {
+                ($rr, $offset) = Net::DNS::RR->parse(\$data, $offset);
+        }
+        for (1 .. $header->arcount) {
+                ($rr, $offset) = Net::DNS::RR->parse(\$data, $offset);
+        }
+        return $offset;
+}
+
+# sign_tcp_continuation:
+# This is a hack to correct the problem that Net::DNS has no idea how
+# to sign multiple-message TCP responses.  Several data that are included
+# in the digest when signing a query or the first message of a response are
+# omitted when signing subsequent messages in a TCP stream.
+#
+# Net::DNS::Packet->sign_tsig() has the ability to use a custom signing
+# function (specified by calling Packet->sign_func()).  We use this
+# function as the signing function for TCP continuations, and it removes
+# the unwanted data from the digest before calling the default sign_hmac
+# function.
+sub sign_tcp_continuation {
+        my ($key, $data) = @_;
+
+        # copy out first two bytes: size of the previous MAC
+        my $rmacsize = unpack("n", $data);
+        $data = substr($data, 2);
+
+        # copy out previous MAC
+        my $rmac = substr($data, 0, $rmacsize);
+        $data = substr($data, $rmacsize);
+
+        # try parsing out the packet information
+        my $plen = packetlen($data);
+        my $pdata = substr($data, 0, $plen);
+        $data = substr($data, $plen);
+
+        # remove the keyname, ttl, class, and algorithm name
+        $data = substr($data, namelen($data));
+        $data = substr($data, 6);
+        $data = substr($data, namelen($data));
+
+        # preserve the TSIG data
+        my $tdata = substr($data, 0, 8);
+
+        # prepare a new digest and sign with it
+        $data = pack("n", $rmacsize) . $rmac . $pdata . $tdata;
+        return Net::DNS::RR::TSIG::sign_hmac($key, $data);
+}
+
+sub handleTCP {
+       my ($buf) = @_;
+
+       my ($packet, $err) = new Net::DNS::Packet(\$buf, 0);
+       $err and die $err;
+       
+       $packet->header->qr(1);
+       $packet->header->aa(1);
+       
+       my @questions = $packet->question;
+       my $qname = $questions[0]->qname;
+       my $qtype = $questions[0]->qtype;
+
+        # get the existing signature if any, and clear the additional section
+        my $prev_tsig;
+        my $signer;
+        while (my $rr = $packet->pop("additional")) {
+                if ($rr->type eq "TSIG") {
+                        $prev_tsig = $rr;
+                }
+        }
+
+       my @results = ();
+       my $count_these = 0;
+
+       my $r;
+       foreach $r (@rules) {
+               my $pattern = $r->{pattern};
+               my($dbtype, $key_name, $key_data) = split(/ /,$pattern);
+               print "[handleTCP] $dbtype, $key_name, $key_data \n";
+               if ("$qname $qtype" =~ /$dbtype/) {
+                       $count_these++;
+                       my $a;
+                       foreach $a (@{$r->{answer}}) {
+                               $packet->push("answer", $a);
+                       }
+                       if(defined($key_name) && defined($key_data)) {
+                               # sign the packet
+                               print "  Signing the data with " . 
+                                      "$key_name/$key_data\n";
+
+                                my $tsig = Net::DNS::RR->
+                                        new("$key_name TSIG $key_data");
+
+                                # These kluges are necessary because Net::DNS
+                                # doesn't know how to sign responses.  We
+                                # clear compnames so that the TSIG key and
+                                # algorithm name won't be compressed, and
+                                # add one to arcount because the signing
+                                # function will attempt to decrement it,
+                                # which is incorrect in a response. Finally
+                                # we set request_mac to the previous digest.
+                                $packet->{"compnames"} = {};
+                                $packet->{"header"}{"arcount"} += 1;
+                                if (defined($prev_tsig)) {
+                                        my $rmac = pack('n H*',
+                                                $prev_tsig->mac_size,
+                                                $prev_tsig->mac);
+                                        $tsig->{"request_mac"} =
+                                                unpack("H*", $rmac);
+                                }
+                                
+                                $tsig->sign_func($signer) if defined($signer);
+                               $packet->sign_tsig($tsig);
+                                $signer = \&sign_tcp_continuation;
+
+                                my $copy =
+                                        Net::DNS::Packet->new(\($packet->data));
+                                $prev_tsig = $copy->pop("additional");
+                       }
+                       #$packet->print;
+                       push(@results,$packet->data);
+                       $packet = new Net::DNS::Packet(\$buf, 0);
+                       $packet->header->qr(1);
+                       $packet->header->aa(1);
+               }
+       }
+       print " A total of $count_these patterns matched\n";
+       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);
+       } 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);
+               }
+               sleep(1);
+       }
+}
index b1fe854d0072bf040d7045bfd88cd940176a6f25..befdda017cb7ad5dfff4b73ad6aa0982ac598a98 100644 (file)
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: named.conf,v 1.10 2007/06/18 23:47:31 tbox Exp $ */
+/* $Id: named.conf,v 1.11 2011/08/31 06:49:10 marka Exp $ */
 
 controls { /* empty */ };
 
@@ -38,3 +38,10 @@ zone "example" {
        allow-update-forwarding { any; };
        masters { 10.53.0.1; };
 };
+
+zone "nomaster" {
+       type slave;
+       file "nomaster.db";
+       allow-update-forwarding { any; };
+       masters { 10.53.0.4; };
+};
diff --git a/bin/tests/system/upforwd/ns3/nomaster.db b/bin/tests/system/upforwd/ns3/nomaster.db
new file mode 100644 (file)
index 0000000..578b2c3
--- /dev/null
@@ -0,0 +1,3 @@
+@ 0 SOA        . . 141235 3600 1200 86400 1200
+@ 0 NS ns4
+ns4 0 A 10.53.0.4
index 81156c8cc33a6b8937a221b6ee1bf2371d09b214..242342214d00b40a26efb360ff67fe5b5cc3e601 100644 (file)
@@ -15,7 +15,7 @@
 # OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 # PERFORMANCE OF THIS SOFTWARE.
 
-# $Id: tests.sh,v 1.10 2007/06/19 23:47:06 tbox Exp $ 
+# $Id: tests.sh,v 1.11 2011/08/31 06:49:10 marka Exp $ 
 
 # ns1 = stealth master
 # ns2 = slave with update forwarding disabled; not currently used
@@ -99,5 +99,26 @@ $PERL ../digcomp.pl knowngood.after2 dig.out.ns1 || status=1
 $PERL ../digcomp.pl knowngood.after2 dig.out.ns2 || status=1
 $PERL ../digcomp.pl knowngood.after2 dig.out.ns3 || status=1
 
+echo "I:checking update forwarding to dead master"
+count=0
+ret=0
+while [ $count -lt 5 -a $ret -eq 0 ]
+do
+(
+$NSUPDATE -- - <<EOF 
+server 10.53.0.3 5300
+zone nomaster
+update add unsigned.nomaster. 600 A 10.10.10.1
+update add unsigned.nomaster. 600 TXT Foo
+send
+EOF
+) > /dev/null 2>&1 &
+       $DIG +notcp +noadd +noauth nomaster.\
+               @10.53.0.3 soa -p 5300 > dig.out.ns3 || ret=1
+       grep "status: NOERROR" dig.out.ns3 > /dev/null || ret=1
+       count=`expr $count + 1`
+done
+if [ $ret != 0 ] ; then echo "I:failed"; status=`expr $status + $ret`; fi
+
 echo "I:exit status: $status"
 exit $status
index 061cc8b98bf325cd141ce4290ca7649889fda7fb..7f125fb525ba42eb6bda49928951ebda2c28b5f6 100644 (file)
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: zone.c,v 1.627 2011/08/30 23:46:52 tbox Exp $ */
+/* $Id: zone.c,v 1.628 2011/08/31 06:49:10 marka Exp $ */
 
 /*! \file */
 
@@ -138,6 +138,7 @@ typedef struct dns_notify dns_notify_t;
 typedef struct dns_stub dns_stub_t;
 typedef struct dns_load dns_load_t;
 typedef struct dns_forward dns_forward_t;
+typedef ISC_LIST(dns_forward_t) dns_forwardlist_t;
 typedef struct dns_io dns_io_t;
 typedef ISC_LIST(dns_io_t) dns_iolist_t;
 typedef struct dns_signing dns_signing_t;
@@ -346,6 +347,11 @@ struct dns_zone {
         */
        dns_updatemethod_t      updatemethod;
 
+       /*%
+        * Outstanding forwarded UPDATE requests.
+        */
+       dns_forwardlist_t       forwards;
+
        dns_zone_t              *raw;
        dns_zone_t              *secure;
 };
@@ -513,6 +519,7 @@ struct dns_forward {
        isc_sockaddr_t          addr;
        dns_updatecallback_t    callback;
        void                    *callback_arg;
+       ISC_LINK(dns_forward_t) link;
 };
 
 /*%
@@ -866,6 +873,7 @@ dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx) {
        zone->privatetype = (dns_rdatatype_t)0xffffU;
        zone->added = ISC_FALSE;
        zone->rpz_zone = ISC_FALSE;
+       ISC_LIST_INIT(zone->forwards);
        zone->raw = NULL;
        zone->secure = NULL;
 
@@ -8662,6 +8670,24 @@ notify_cancel(dns_zone_t *zone) {
        }
 }
 
+static void
+forward_cancel(dns_zone_t *zone) {
+       dns_forward_t *forward;
+
+       /*
+        * 'zone' locked by caller.
+        */
+
+       REQUIRE(LOCKED_ZONE(zone));
+
+       for (forward = ISC_LIST_HEAD(zone->forwards);
+            forward != NULL;
+            forward = ISC_LIST_NEXT(forward, link)) {
+               if (forward->request != NULL)
+                       dns_request_cancel(forward->request);
+       }
+}
+
 static void
 zone_unload(dns_zone_t *zone) {
 
@@ -10605,6 +10631,7 @@ zone_shutdown(isc_task_t *task, isc_event_t *event) {
        REQUIRE(DNS_ZONE_VALID(zone));
        INSIST(event->ev_type == DNS_EVENT_ZONECONTROL);
        INSIST(isc_refcount_current(&zone->erefs) == 0);
+
        zone_debuglog(zone, "zone_shutdown", 3, "shutting down");
 
        /*
@@ -10663,6 +10690,8 @@ zone_shutdown(isc_task_t *task, isc_event_t *event) {
 
        notify_cancel(zone);
 
+       forward_cancel(zone);
+
        if (zone->timer != NULL) {
                isc_timer_detach(&zone->timer);
                INSIST(zone->irefs > 0);
@@ -12863,8 +12892,13 @@ forward_destroy(dns_forward_t *forward) {
                dns_request_destroy(&forward->request);
        if (forward->msgbuf != NULL)
                isc_buffer_free(&forward->msgbuf);
-       if (forward->zone != NULL)
+       if (forward->zone != NULL) {
+               LOCK(&forward->zone->lock);
+               if (ISC_LINK_LINKED(forward, link))
+                       ISC_LIST_UNLINK(forward->zone->forwards, forward, link);
+               UNLOCK(&forward->zone->lock);
                dns_zone_idetach(&forward->zone);
+       }
        isc_mem_putanddetach(&forward->mctx, forward, sizeof(*forward));
 }
 
@@ -12874,6 +12908,12 @@ sendtomaster(dns_forward_t *forward) {
        isc_sockaddr_t src;
 
        LOCK_ZONE(forward->zone);
+
+       if (DNS_ZONE_FLAG(forward->zone, DNS_ZONEFLG_EXITING)) {
+               UNLOCK_ZONE(forward->zone);
+               return (ISC_R_CANCELED);
+       }
+
        if (forward->which >= forward->zone->masterscnt) {
                UNLOCK_ZONE(forward->zone);
                return (ISC_R_NOMORE);
@@ -12904,6 +12944,11 @@ sendtomaster(dns_forward_t *forward) {
                                       forward->zone->task,
                                       forward_callback, forward,
                                       &forward->request);
+       if (result == ISC_R_SUCCESS) {
+               if (!ISC_LINK_LINKED(forward, link))
+                       ISC_LIST_APPEND(forward->zone->forwards, forward, link);
+       }
+
  unlock:
        UNLOCK_ZONE(forward->zone);
        return (result);
@@ -13030,6 +13075,7 @@ dns_zone_forwardupdate(dns_zone_t *zone, dns_message_t *msg,
        forward->mctx = 0;
        forward->callback = callback;
        forward->callback_arg = callback_arg;
+       ISC_LINK_INIT(forward, link);
        forward->magic = FORWARD_MAGIC;
 
        mr = dns_message_getrawmessage(msg);
@@ -13312,6 +13358,8 @@ dns_zonemgr_resumexfrs(dns_zonemgr_t *zmgr) {
 
 void
 dns_zonemgr_shutdown(dns_zonemgr_t *zmgr) {
+       dns_zone_t *zone;
+
        REQUIRE(DNS_ZONEMGR_VALID(zmgr));
 
        isc_ratelimiter_shutdown(zmgr->rl);
@@ -13320,6 +13368,18 @@ dns_zonemgr_shutdown(dns_zonemgr_t *zmgr) {
                isc_task_destroy(&zmgr->task);
        if (zmgr->zonetasks != NULL)
                isc_taskpool_destroy(&zmgr->zonetasks);
+
+       RWLOCK(&zmgr->rwlock, isc_rwlocktype_read);
+       for (zone = ISC_LIST_HEAD(zmgr->zones);
+            zone != NULL;
+            zone = ISC_LIST_NEXT(zone, link))
+       {
+               LOCK_ZONE(zone);
+               forward_cancel(zone);
+               UNLOCK_ZONE(zone);
+       }
+       RWUNLOCK(&zmgr->rwlock, isc_rwlocktype_read);
+
 }
 
 isc_result_t