]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
[v9_11_0_patch] handle synth CNAME before DNAME
authorEvan Hunt <each@isc.org>
Mon, 30 Jan 2017 22:20:49 +0000 (14:20 -0800)
committerEvan Hunt <each@isc.org>
Mon, 30 Jan 2017 22:20:49 +0000 (14:20 -0800)
4558.   [bug]           Synthesised CNAME before matching DNAME was still
                        being cached when it should have been.  [RT #44318]

(cherry picked from commit 9f4bf43b79e64463103395a29d41a903bf2b8846)

CHANGES
bin/tests/system/dname/ans3/ans.pl [new file with mode: 0644]
bin/tests/system/dname/ns1/root.db
bin/tests/system/dname/tests.sh
doc/arm/notes.xml
lib/dns/resolver.c

diff --git a/CHANGES b/CHANGES
index 9cd38fd0bad07c73e50672498464ad3856a8a637..55a6abada6406aa93f3fa1a246ccdff773bb898d 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+4558.  [bug]           Synthesised CNAME before matching DNAME was still
+                       being cached when it should have been.  [RT #44318]
+
 4557.  [security]      Combining dns64 and rpz can result in dereferencing
                        a NULL pointer (read).  (CVE-2017-3135) [RT#44434]
 
diff --git a/bin/tests/system/dname/ans3/ans.pl b/bin/tests/system/dname/ans3/ans.pl
new file mode 100644 (file)
index 0000000..271fc7d
--- /dev/null
@@ -0,0 +1,95 @@
+#!/usr/bin/env perl
+#
+# Copyright (C) 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 strict;
+use warnings;
+
+use IO::File;
+use Getopt::Long;
+use Net::DNS::Nameserver;
+
+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 $localaddr = "10.53.0.3";
+my $localport = 5300;
+my $verbose = 0;
+my $ttl = 60;
+my $zone = "example.broken";
+my $nsname = "ns3.$zone";
+my $synth = "synth-then-dname.$zone";
+my $synth2 = "synth2-then-dname.$zone";
+
+sub reply_handler {
+    my ($qname, $qclass, $qtype, $peerhost, $query, $conn) = @_;
+    my ($rcode, @ans, @auth, @add);
+
+    print ("request: $qname/$qtype\n");
+    STDOUT->flush();
+
+    if ($qname eq "example.broken") {
+        if ($qtype eq "SOA") {
+           my $rr = new Net::DNS::RR("$qname $ttl $qclass SOA . . 0 0 0 0 0");
+           push @ans, $rr;
+        } elsif ($qtype eq "NS") {
+           my $rr = new Net::DNS::RR("$qname $ttl $qclass NS $nsname");
+           push @ans, $rr;
+           $rr = new Net::DNS::RR("$nsname $ttl $qclass A $localaddr");
+           push @add, $rr;
+        }
+        $rcode = "NOERROR";
+    } elsif ($qname eq "cname-to-$synth2") {
+        my $rr = new Net::DNS::RR("$qname $ttl $qclass CNAME name.$synth2");
+       push @ans, $rr;
+        $rr = new Net::DNS::RR("name.$synth2 $ttl $qclass CNAME name");
+       push @ans, $rr;
+        $rr = new Net::DNS::RR("$synth2 $ttl $qclass DNAME .");
+       push @ans, $rr;
+       $rcode = "NOERROR";
+    } elsif ($qname eq "$synth" || $qname eq "$synth2") {
+       if ($qtype eq "DNAME") {
+           my $rr = new Net::DNS::RR("$qname $ttl $qclass DNAME .");
+           push @ans, $rr;
+       }
+       $rcode = "NOERROR";
+    } elsif ($qname eq "name.$synth") {
+       my $rr = new Net::DNS::RR("$qname $ttl $qclass CNAME name.");
+       push @ans, $rr;
+       $rr = new Net::DNS::RR("$synth $ttl $qclass DNAME .");
+       push @ans, $rr;
+       $rcode = "NOERROR";
+    } elsif ($qname eq "name.$synth2") {
+       my $rr = new Net::DNS::RR("$qname $ttl $qclass CNAME name.");
+       push @ans, $rr;
+       $rr = new Net::DNS::RR("$synth2 $ttl $qclass DNAME .");
+       push @ans, $rr;
+       $rcode = "NOERROR";
+    } else {
+       $rcode = "REFUSED";
+    }
+    return ($rcode, \@ans, \@auth, \@add, { aa => 1 });
+}
+
+GetOptions(
+    'port=i' => \$localport,
+    'verbose!' => \$verbose,
+);
+
+my $ns = Net::DNS::Nameserver->new(
+    LocalAddr => $localaddr,
+    LocalPort => $localport,
+    ReplyHandler => \&reply_handler,
+    Verbose => $verbose,
+);
+
+$ns->main_loop;
index ec35c4bfeba795d5343dde576205cf45dd8cceae..f589599c36c396838ee5ecb66dff82a25f2c56e0 100644 (file)
@@ -4,8 +4,6 @@
 ; 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/.
 
-; $Id: root.db,v 1.2 2011/03/18 21:14:19 fdupont Exp $
-
 $TTL 300
 .                      IN SOA  gson.nominum.com. a.root.servers.nil. (
                                2000042100      ; serial
@@ -19,3 +17,6 @@ a.root-servers.nil.   A       10.53.0.1
 
 example.               NS      ns2.example.
 ns2.example.           A       10.53.0.2
+
+example.broken.                NS      ns3.example.broken.
+ns3.example.broken.    A       10.53.0.3
index d97da66b81690a85767bfc049b0fc36e083ba385..fa04e8ec6e525cd79eefe63a9a9be105155f493d 100644 (file)
@@ -12,6 +12,7 @@ SYSTEMTESTTOP=..
 . $SYSTEMTESTTOP/conf.sh
 
 status=0
+n=0
 
 echo "I:checking short dname from authoritative"
 ret=0
@@ -73,6 +74,26 @@ grep '^a.target.example.' dig.out.ns4.cname > /dev/null || ret=1
 if [ $ret != 0 ]; then echo "I:failed"; fi
 status=`expr $status + $ret`
 
-echo "I:exit status: $status"
+n=`expr $n + 1`
+echo "I:checking dname is returned with synthesized cname before dname ($n)"
+ret=0
+$DIG @10.53.0.4 -p 5300 name.synth-then-dname.example.broken A > dig.out.test$n
+grep "status: NXDOMAIN" dig.out.test$n > /dev/null || ret=1
+grep '^name.synth-then-dname\.example\.broken\..*CNAME.*name.$' dig.out.test$n > /dev/null || ret=1
+grep '^synth-then-dname\.example\.broken\..*DNAME.*\.$' dig.out.test$n > /dev/null || ret=1
+if [ $ret != 0 ]; then echo "I:failed"; fi
+status=`expr $status + $ret`
 
+n=`expr $n + 1`
+echo "I:checking dname is returned with cname to synthesized cname before dname ($n)"
+ret=0
+$DIG @10.53.0.4 -p 5300 cname-to-synth2-then-dname.example.broken A > dig.out.test$n
+grep "status: NXDOMAIN" dig.out.test$n > /dev/null || ret=1
+grep '^cname-to-synth2-then-dname\.example\.broken\..*CNAME.*name\.synth2-then-dname\.example\.broken.$' dig.out.test$n > /dev/null || ret=1
+grep '^name\.synth2-then-dname\.example\.broken\..*CNAME.*name.$' dig.out.test$n > /dev/null || ret=1
+grep '^synth2-then-dname\.example\.broken\..*DNAME.*\.$' dig.out.test$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 deb5c7c75f245bb586496d3715bfd3fcd439f7f2..d0043e0b0f542a4a6c5f2247ee41f6c6b5109a81 100644 (file)
 
   <section xml:id="relnotes_bugs"><info><title>Bug Fixes</title></info>
     <itemizedlist>
-      <listitem>
-       <para>
-         None.
+       <listitem>
+       <para>
+         A synthesized CNAME record appearing in a response before the
+         associated DNAME could be cached, when it should not have been.
+         This was a regression introduced while addressing CVE-2016-8864.
+         [RT #44318]
        </para>
       </listitem>
     </itemizedlist>
index 699c805d0f4cfb6dfe6b368edc02b31b68dd416d..961bd3bc41210fd6f1b839f0f986eed1cd172ab6 100644 (file)
@@ -6165,9 +6165,13 @@ cname_target(dns_rdataset_t *rdataset, dns_name_t *tname) {
        return (ISC_R_SUCCESS);
 }
 
+/*%
+ * Construct the synthesised CNAME from the existing QNAME and
+ * the DNAME RR and store it in 'target'.
+ */
 static inline isc_result_t
-dname_target(dns_rdataset_t *rdataset, dns_name_t *qname,
-            unsigned int nlabels, dns_fixedname_t *fixeddname)
+dname_target(dns_rdataset_t *rdataset, const dns_name_t *qname,
+            unsigned int nlabels, dns_name_t *target)
 {
        isc_result_t result;
        dns_rdata_t rdata = DNS_RDATA_INIT;
@@ -6187,14 +6191,33 @@ dname_target(dns_rdataset_t *rdataset, dns_name_t *qname,
 
        dns_fixedname_init(&prefix);
        dns_name_split(qname, nlabels, dns_fixedname_name(&prefix), NULL);
-       dns_fixedname_init(fixeddname);
        result = dns_name_concatenate(dns_fixedname_name(&prefix),
-                                     &dname.dname,
-                                     dns_fixedname_name(fixeddname), NULL);
+                                     &dname.dname, target, NULL);
        dns_rdata_freestruct(&dname);
        return (result);
 }
 
+/*%
+ * Check if it was possible to construct 'qname' from 'lastcname'
+ * and 'rdataset'.
+ */
+static inline isc_result_t
+fromdname(dns_rdataset_t *rdataset, const dns_name_t *lastcname,
+         unsigned int nlabels, const dns_name_t *qname)
+{
+       dns_fixedname_t fixed;
+       isc_result_t result;
+       dns_name_t *target;
+
+       dns_fixedname_init(&fixed);
+       target = dns_fixedname_name(&fixed);
+       result = dname_target(rdataset, lastcname, nlabels, target);
+       if (result != ISC_R_SUCCESS || !dns_name_equal(qname, target))
+               return (ISC_R_NOTFOUND);
+
+       return (ISC_R_SUCCESS);
+}
+
 static isc_boolean_t
 is_answeraddress_allowed(dns_view_t *view, dns_name_t *name,
                         dns_rdataset_t *rdataset)
@@ -6803,12 +6826,12 @@ answer_response(fetchctx_t *fctx) {
        isc_result_t result;
        dns_message_t *message;
        dns_name_t *name, *dname = NULL, *qname, tname, *ns_name;
-       dns_name_t *cname = NULL;
+       dns_name_t *cname = NULL, *lastcname = NULL;
        dns_rdataset_t *rdataset, *ns_rdataset;
-       isc_boolean_t done, external, chaining, aa, found, want_chaining;
+       isc_boolean_t done, external, aa, found, want_chaining;
        isc_boolean_t have_answer, found_cname, found_dname, found_type;
        isc_boolean_t wanted_chaining;
-       unsigned int aflag;
+       unsigned int aflag, chaining;
        dns_rdatatype_t type;
        dns_fixedname_t fdname, fqname;
        dns_view_t *view;
@@ -6826,9 +6849,9 @@ answer_response(fetchctx_t *fctx) {
        found_cname = ISC_FALSE;
        found_dname = ISC_FALSE;
        found_type = ISC_FALSE;
-       chaining = ISC_FALSE;
        have_answer = ISC_FALSE;
        want_chaining = ISC_FALSE;
+       chaining = 0;
        POST(want_chaining);
        if ((message->flags & DNS_MESSAGEFLAG_AA) != 0)
                aa = ISC_TRUE;
@@ -6839,14 +6862,15 @@ answer_response(fetchctx_t *fctx) {
        view = fctx->res->view;
        result = dns_message_firstname(message, DNS_SECTION_ANSWER);
        while (!done && result == ISC_R_SUCCESS) {
-               dns_namereln_t namereln;
-               int order;
-               unsigned int nlabels;
+               dns_namereln_t namereln, lastreln;
+               int order, lastorder;
+               unsigned int nlabels, lastnlabels;
 
                name = NULL;
                dns_message_currentname(message, DNS_SECTION_ANSWER, &name);
                external = ISC_TF(!dns_name_issubdomain(name, &fctx->domain));
                namereln = dns_name_fullcompare(qname, name, &order, &nlabels);
+
                if (namereln == dns_namereln_equal) {
                        wanted_chaining = ISC_FALSE;
                        for (rdataset = ISC_LIST_HEAD(name->list);
@@ -6952,6 +6976,7 @@ answer_response(fetchctx_t *fctx) {
                                                        &fctx->domain)) {
                                                return (DNS_R_SERVFAIL);
                                        }
+                                       lastcname = name;
                                } else if (rdataset->type == dns_rdatatype_rrsig
                                           && rdataset->covers ==
                                              dns_rdatatype_cname
@@ -6975,7 +7000,7 @@ answer_response(fetchctx_t *fctx) {
                                        rdataset->attributes |=
                                                DNS_RDATASETATTR_CACHE;
                                        rdataset->trust = dns_trust_answer;
-                                       if (!chaining) {
+                                       if (chaining == 0) {
                                                /*
                                                 * This data is "the" answer
                                                 * to our question only if
@@ -7052,10 +7077,21 @@ answer_response(fetchctx_t *fctx) {
                         * cause us to ignore the signatures of
                         * CNAMEs.
                         */
-                       if (wanted_chaining)
-                               chaining = ISC_TRUE;
+                       if (wanted_chaining && chaining < 2U)
+                               chaining++;
                } else {
                        dns_rdataset_t *dnameset = NULL;
+                       isc_boolean_t synthcname = ISC_FALSE;
+
+                       if (lastcname != NULL) {
+                               lastreln = dns_name_fullcompare(lastcname,
+                                                               name,
+                                                               &lastorder,
+                                                               &lastnlabels);
+                               if (lastreln == dns_namereln_subdomain &&
+                                   lastnlabels == dns_name_countlabels(name))
+                                       synthcname = ISC_TRUE;
+                       }
 
                        /*
                         * Look for a DNAME (or its SIG).  Anything else is
@@ -7084,7 +7120,7 @@ answer_response(fetchctx_t *fctx) {
                                 * If we're not chaining, then the DNAME and
                                 * its signature should not be external.
                                 */
-                               if (!chaining && external) {
+                               if (chaining == 0 && external) {
                                        char qbuf[DNS_NAME_FORMATSIZE];
                                        char obuf[DNS_NAME_FORMATSIZE];
 
@@ -7102,16 +7138,9 @@ answer_response(fetchctx_t *fctx) {
                                /*
                                 * If DNAME + synthetic CNAME then the
                                 * namereln is dns_namereln_subdomain.
-                                *
-                                * If synthetic CNAME + DNAME then the
-                                * namereln is dns_namereln_commonancestor
-                                * and the number of label must match the
-                                * DNAME.  This order is not RFC compliant.
                                 */
-
                                if (namereln != dns_namereln_subdomain &&
-                                   (namereln != dns_namereln_commonancestor ||
-                                    nlabels != dns_name_countlabels(name)))
+                                   !synthcname)
                                {
                                        char qbuf[DNS_NAME_FORMATSIZE];
                                        char obuf[DNS_NAME_FORMATSIZE];
@@ -7131,8 +7160,19 @@ answer_response(fetchctx_t *fctx) {
                                        want_chaining = ISC_TRUE;
                                        POST(want_chaining);
                                        aflag = DNS_RDATASETATTR_ANSWER;
-                                       result = dname_target(rdataset, qname,
-                                                             nlabels, &fdname);
+                                       dns_fixedname_init(&fdname);
+                                       dname = dns_fixedname_name(&fdname);
+                                       if (synthcname) {
+                                               result = fromdname(rdataset,
+                                                                  lastcname,
+                                                                  lastnlabels,
+                                                                  qname);
+                                       } else {
+                                               result = dname_target(rdataset,
+                                                                     qname,
+                                                                     nlabels,
+                                                                     dname);
+                                       }
                                        if (result == ISC_R_NOSPACE) {
                                                /*
                                                 * We can't construct the
@@ -7146,8 +7186,8 @@ answer_response(fetchctx_t *fctx) {
                                        else
                                                dnameset = rdataset;
 
-                                       dname = dns_fixedname_name(&fdname);
-                                       if (!is_answertarget_allowed(view,
+                                       if (!synthcname &&
+                                           !is_answertarget_allowed(view,
                                                     qname, rdataset->type,
                                                     dname, &fctx->domain))
                                        {
@@ -7168,7 +7208,13 @@ answer_response(fetchctx_t *fctx) {
                                name->attributes |= DNS_NAMEATTR_CACHE;
                                rdataset->attributes |= DNS_RDATASETATTR_CACHE;
                                rdataset->trust = dns_trust_answer;
-                               if (!chaining) {
+                               /*
+                                * If we are not chaining or the first CNAME
+                                * is a synthesised CNAME before the DNAME.
+                                */
+                               if ((chaining == 0) ||
+                                   (chaining == 1U && synthcname))
+                               {
                                        /*
                                         * This data is "the" answer to
                                         * our question only if we're
@@ -7178,9 +7224,12 @@ answer_response(fetchctx_t *fctx) {
                                        if (aflag == DNS_RDATASETATTR_ANSWER) {
                                                have_answer = ISC_TRUE;
                                                found_dname = ISC_TRUE;
-                                               if (cname != NULL)
+                                               if (cname != NULL &&
+                                                   synthcname)
+                                               {
                                                        cname->attributes &=
                                                           ~DNS_NAMEATTR_ANSWER;
+                                               }
                                                name->attributes |=
                                                        DNS_NAMEATTR_ANSWER;
                                        }
@@ -7198,26 +7247,35 @@ answer_response(fetchctx_t *fctx) {
                         * DNAME chaining.
                         */
                        if (dnameset != NULL) {
-                               /*
-                                * Copy the dname into the qname fixed name.
-                                *
-                                * Although we check for failure of the copy
-                                * operation, in practice it should never fail
-                                * since we already know that the  result fits
-                                * in a fixedname.
-                                */
-                               dns_fixedname_init(&fqname);
-                               qname = dns_fixedname_name(&fqname);
-                               result = dns_name_copy(dname, qname, NULL);
-                               if (result != ISC_R_SUCCESS)
-                                       return (result);
+                               if (!synthcname) {
+                                       /*
+                                        * Copy the dname into the qname fixed
+                                        * name.
+                                        *
+                                        * Although we check for failure of the
+                                        * copy operation, in practice it
+                                        * should never fail since we already
+                                        * know that the result fits in a
+                                        * fixedname.
+                                        */
+                                       dns_fixedname_init(&fqname);
+                                       qname = dns_fixedname_name(&fqname);
+                                       result = dns_name_copy(dname, qname,
+                                                              NULL);
+                                       if (result != ISC_R_SUCCESS)
+                                               return (result);
+                               }
                                wanted_chaining = ISC_TRUE;
                                name->attributes |= DNS_NAMEATTR_CHAINING;
                                dnameset->attributes |=
                                            DNS_RDATASETATTR_CHAINING;
                        }
-                       if (wanted_chaining)
-                               chaining = ISC_TRUE;
+                       /*
+                        * Ensure that we can't ever get chaining == 1
+                        * above if we have processed a DNAME.
+                        */
+                       if (wanted_chaining && chaining < 2U)
+                               chaining += 2;
                }
                result = dns_message_nextname(message, DNS_SECTION_ANSWER);
        }
@@ -7242,7 +7300,7 @@ answer_response(fetchctx_t *fctx) {
        /*
         * Did chaining end before we got the final answer?
         */
-       if (chaining) {
+       if (chaining != 0) {
                /*
                 * Yes.  This may be a negative reply, so hand off
                 * authority section processing to the noanswer code.
@@ -7291,7 +7349,7 @@ answer_response(fetchctx_t *fctx) {
                                                DNS_NAMEATTR_CACHE;
                                        rdataset->attributes |=
                                                DNS_RDATASETATTR_CACHE;
-                                       if (aa && !chaining)
+                                       if (aa && chaining == 0)
                                                rdataset->trust =
                                                    dns_trust_authauthority;
                                        else