900526
From e92ac3b83209ddc46ca9a3facd7edf1f14052edf Mon Sep 17 00:00:00 2001
900526
From: rpm-build <rpm-build>
900526
Date: Wed, 8 Feb 2017 13:49:47 +0100
900526
Subject: [PATCH] 4558.   [bug]       Synthesised CNAME before matching DNAME
900526
 was still                     being cached when it should not have been.  [RT
900526
 #44318]
900526
900526
Fixes and tests last case fixed by CVE-2016-9147
900526
---
900526
 bin/tests/system/dname/ans3/ans.pl |  95 +++++++++++++++++++++++
900526
 bin/tests/system/dname/ns1/root.db |   5 +-
900526
 bin/tests/system/dname/tests.sh    |  25 ++++++-
900526
 lib/dns/resolver.c                 | 150 +++++++++++++++++++++++++------------
900526
 4 files changed, 225 insertions(+), 50 deletions(-)
900526
 create mode 100644 bin/tests/system/dname/ans3/ans.pl
900526
900526
diff --git a/bin/tests/system/dname/ans3/ans.pl b/bin/tests/system/dname/ans3/ans.pl
900526
new file mode 100644
900526
index 0000000..271fc7d
900526
--- /dev/null
900526
+++ b/bin/tests/system/dname/ans3/ans.pl
900526
@@ -0,0 +1,95 @@
900526
+#!/usr/bin/env perl
900526
+#
900526
+# Copyright (C) 2014-2016  Internet Systems Consortium, Inc. ("ISC")
900526
+#
900526
+# This Source Code Form is subject to the terms of the Mozilla Public
900526
+# License, v. 2.0. If a copy of the MPL was not distributed with this
900526
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
900526
+
900526
+use strict;
900526
+use warnings;
900526
+
900526
+use IO::File;
900526
+use Getopt::Long;
900526
+use Net::DNS::Nameserver;
900526
+
900526
+my $pidf = new IO::File "ans.pid", "w" or die "cannot open pid file: $!";
900526
+print $pidf "$$\n" or die "cannot write pid file: $!";
900526
+$pidf->close or die "cannot close pid file: $!";
900526
+sub rmpid { unlink "ans.pid"; exit 1; };
900526
+
900526
+$SIG{INT} = \&rmpid;
900526
+$SIG{TERM} = \&rmpid;
900526
+
900526
+my $localaddr = "10.53.0.3";
900526
+my $localport = 5300;
900526
+my $verbose = 0;
900526
+my $ttl = 60;
900526
+my $zone = "example.broken";
900526
+my $nsname = "ns3.$zone";
900526
+my $synth = "synth-then-dname.$zone";
900526
+my $synth2 = "synth2-then-dname.$zone";
900526
+
900526
+sub reply_handler {
900526
+    my ($qname, $qclass, $qtype, $peerhost, $query, $conn) = @_;
900526
+    my ($rcode, @ans, @auth, @add);
900526
+
900526
+    print ("request: $qname/$qtype\n");
900526
+    STDOUT->flush();
900526
+
900526
+    if ($qname eq "example.broken") {
900526
+        if ($qtype eq "SOA") {
900526
+	    my $rr = new Net::DNS::RR("$qname $ttl $qclass SOA . . 0 0 0 0 0");
900526
+	    push @ans, $rr;
900526
+        } elsif ($qtype eq "NS") {
900526
+	    my $rr = new Net::DNS::RR("$qname $ttl $qclass NS $nsname");
900526
+	    push @ans, $rr;
900526
+	    $rr = new Net::DNS::RR("$nsname $ttl $qclass A $localaddr");
900526
+	    push @add, $rr;
900526
+        }
900526
+        $rcode = "NOERROR";
900526
+    } elsif ($qname eq "cname-to-$synth2") {
900526
+        my $rr = new Net::DNS::RR("$qname $ttl $qclass CNAME name.$synth2");
900526
+	push @ans, $rr;
900526
+        $rr = new Net::DNS::RR("name.$synth2 $ttl $qclass CNAME name");
900526
+	push @ans, $rr;
900526
+        $rr = new Net::DNS::RR("$synth2 $ttl $qclass DNAME .");
900526
+	push @ans, $rr;
900526
+	$rcode = "NOERROR";
900526
+    } elsif ($qname eq "$synth" || $qname eq "$synth2") {
900526
+	if ($qtype eq "DNAME") {
900526
+	    my $rr = new Net::DNS::RR("$qname $ttl $qclass DNAME .");
900526
+	    push @ans, $rr;
900526
+	}
900526
+	$rcode = "NOERROR";
900526
+    } elsif ($qname eq "name.$synth") {
900526
+	my $rr = new Net::DNS::RR("$qname $ttl $qclass CNAME name.");
900526
+	push @ans, $rr;
900526
+	$rr = new Net::DNS::RR("$synth $ttl $qclass DNAME .");
900526
+	push @ans, $rr;
900526
+	$rcode = "NOERROR";
900526
+    } elsif ($qname eq "name.$synth2") {
900526
+	my $rr = new Net::DNS::RR("$qname $ttl $qclass CNAME name.");
900526
+	push @ans, $rr;
900526
+	$rr = new Net::DNS::RR("$synth2 $ttl $qclass DNAME .");
900526
+	push @ans, $rr;
900526
+	$rcode = "NOERROR";
900526
+    } else {
900526
+	$rcode = "REFUSED";
900526
+    }
900526
+    return ($rcode, \@ans, \@auth, \@add, { aa => 1 });
900526
+}
900526
+
900526
+GetOptions(
900526
+    'port=i' => \$localport,
900526
+    'verbose!' => \$verbose,
900526
+);
900526
+
900526
+my $ns = Net::DNS::Nameserver->new(
900526
+    LocalAddr => $localaddr,
900526
+    LocalPort => $localport,
900526
+    ReplyHandler => \&reply_handler,
900526
+    Verbose => $verbose,
900526
+);
900526
+
900526
+$ns->main_loop;
900526
diff --git a/bin/tests/system/dname/ns1/root.db b/bin/tests/system/dname/ns1/root.db
900526
index 7049e77..2e84ae0 100644
900526
--- a/bin/tests/system/dname/ns1/root.db
900526
+++ b/bin/tests/system/dname/ns1/root.db
900526
@@ -12,8 +12,6 @@
900526
 ; OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
900526
 ; PERFORMANCE OF THIS SOFTWARE.
900526
 
900526
-; $Id: root.db,v 1.2 2011/03/18 21:14:19 fdupont Exp $
900526
-
900526
 $TTL 300
900526
 . 			IN SOA	gson.nominum.com. a.root.servers.nil. (
900526
 				2000042100   	; serial
900526
@@ -27,3 +25,6 @@ a.root-servers.nil.	A	10.53.0.1
900526
 
900526
 example.		NS	ns2.example.
900526
 ns2.example.		A	10.53.0.2
900526
+
900526
+example.broken.		NS	ns3.example.broken.
900526
+ns3.example.broken.	A	10.53.0.3
900526
diff --git a/bin/tests/system/dname/tests.sh b/bin/tests/system/dname/tests.sh
900526
index 04bfcb2..6dc9e88 100644
900526
--- a/bin/tests/system/dname/tests.sh
900526
+++ b/bin/tests/system/dname/tests.sh
900526
@@ -20,6 +20,7 @@ SYSTEMTESTTOP=..
900526
 . $SYSTEMTESTTOP/conf.sh
900526
 
900526
 status=0
900526
+n=0
900526
 
900526
 echo "I:checking short dname from authoritative"
900526
 ret=0
900526
@@ -81,6 +82,26 @@ grep '^a.target.example.' dig.out.ns4.cname > /dev/null || ret=1
900526
 if [ $ret != 0 ]; then echo "I:failed"; fi
900526
 status=`expr $status + $ret`
900526
 
900526
-echo "I:exit status: $status"
900526
+n=`expr $n + 1`
900526
+echo "I:checking dname is returned with synthesized cname before dname ($n)"
900526
+ret=0
900526
+$DIG @10.53.0.4 -p 5300 name.synth-then-dname.example.broken A > dig.out.test$n
900526
+grep "status: NXDOMAIN" dig.out.test$n > /dev/null || ret=1
900526
+grep '^name.synth-then-dname\.example\.broken\..*CNAME.*name.$' dig.out.test$n > /dev/null || ret=1
900526
+grep '^synth-then-dname\.example\.broken\..*DNAME.*\.$' dig.out.test$n > /dev/null || ret=1
900526
+if [ $ret != 0 ]; then echo "I:failed"; fi
900526
+status=`expr $status + $ret`
900526
 
900526
-exit $status
900526
+n=`expr $n + 1`
900526
+echo "I:checking dname is returned with cname to synthesized cname before dname ($n)"
900526
+ret=0
900526
+$DIG @10.53.0.4 -p 5300 cname-to-synth2-then-dname.example.broken A > dig.out.test$n
900526
+grep "status: NXDOMAIN" dig.out.test$n > /dev/null || ret=1
900526
+grep '^cname-to-synth2-then-dname\.example\.broken\..*CNAME.*name\.synth2-then-dname\.example\.broken.$' dig.out.test$n > /dev/null || ret=1
900526
+grep '^name\.synth2-then-dname\.example\.broken\..*CNAME.*name.$' dig.out.test$n > /dev/null || ret=1
900526
+grep '^synth2-then-dname\.example\.broken\..*DNAME.*\.$' dig.out.test$n > /dev/null || ret=1
900526
+if [ $ret != 0 ]; then echo "I:failed"; fi
900526
+status=`expr $status + $ret`
900526
+
900526
+echo "I:exit status: $status"
900526
+[ $status -eq 0 ] || exit 1
900526
diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
900526
index bfd4dcb..c3607fa 100644
900526
--- a/lib/dns/resolver.c
900526
+++ b/lib/dns/resolver.c
900526
@@ -5406,9 +5406,13 @@ cname_target(dns_rdataset_t *rdataset, dns_name_t *tname) {
900526
 	return (ISC_R_SUCCESS);
900526
 }
900526
 
900526
+/*%
900526
+ * Construct the synthesised CNAME from the existing QNAME and
900526
+ * the DNAME RR and store it in 'target'.
900526
+ */
900526
 static inline isc_result_t
900526
 dname_target(dns_rdataset_t *rdataset, dns_name_t *qname,
900526
-	     unsigned int nlabels, dns_fixedname_t *fixeddname)
900526
+	     unsigned int nlabels, dns_name_t *target)
900526
 {
900526
 	isc_result_t result;
900526
 	dns_rdata_t rdata = DNS_RDATA_INIT;
900526
@@ -5428,14 +5432,33 @@ dname_target(dns_rdataset_t *rdataset, dns_name_t *qname,
900526
 
900526
 	dns_fixedname_init(&prefix);
900526
 	dns_name_split(qname, nlabels, dns_fixedname_name(&prefix), NULL);
900526
-	dns_fixedname_init(fixeddname);
900526
 	result = dns_name_concatenate(dns_fixedname_name(&prefix),
900526
-				      &dname.dname,
900526
-				      dns_fixedname_name(fixeddname), NULL);
900526
+				      &dname.dname, target, NULL);
900526
 	dns_rdata_freestruct(&dname);
900526
 	return (result);
900526
 }
900526
 
900526
+/*%
900526
+ * Check if it was possible to construct 'qname' from 'lastcname'
900526
+ * and 'rdataset'.
900526
+ */
900526
+static inline isc_result_t
900526
+fromdname(dns_rdataset_t *rdataset, dns_name_t *lastcname,
900526
+	  unsigned int nlabels, const dns_name_t *qname)
900526
+{
900526
+	dns_fixedname_t fixed;
900526
+	isc_result_t result;
900526
+	dns_name_t *target;
900526
+
900526
+	dns_fixedname_init(&fixed);
900526
+	target = dns_fixedname_name(&fixed);
900526
+	result = dname_target(rdataset, lastcname, nlabels, target);
900526
+	if (result != ISC_R_SUCCESS || !dns_name_equal(qname, target))
900526
+		return (ISC_R_NOTFOUND);
900526
+
900526
+	return (ISC_R_SUCCESS);
900526
+}
900526
+
900526
 static isc_boolean_t
900526
 is_answeraddress_allowed(dns_view_t *view, dns_name_t *name,
900526
 			 dns_rdataset_t *rdataset)
900526
@@ -6039,12 +6062,12 @@ answer_response(fetchctx_t *fctx) {
900526
 	isc_result_t result;
900526
 	dns_message_t *message;
900526
 	dns_name_t *name, *dname = NULL, *qname, tname, *ns_name;
900526
-	dns_name_t *cname = NULL;
900526
+	dns_name_t *cname = NULL, *lastcname = NULL;
900526
 	dns_rdataset_t *rdataset, *ns_rdataset;
900526
-	isc_boolean_t done, external, chaining, aa, found, want_chaining;
900526
+	isc_boolean_t done, external, aa, found, want_chaining;
900526
 	isc_boolean_t have_answer, found_cname, found_dname, found_type;
900526
 	isc_boolean_t wanted_chaining;
900526
-	unsigned int aflag;
900526
+	unsigned int aflag, chaining;
900526
 	dns_rdatatype_t type;
900526
 	dns_fixedname_t fdname, fqname;
900526
 	dns_view_t *view;
900526
@@ -6062,9 +6085,9 @@ answer_response(fetchctx_t *fctx) {
900526
 	found_cname = ISC_FALSE;
900526
 	found_dname = ISC_FALSE;
900526
 	found_type = ISC_FALSE;
900526
-	chaining = ISC_FALSE;
900526
 	have_answer = ISC_FALSE;
900526
 	want_chaining = ISC_FALSE;
900526
+	chaining = 0;
900526
 	POST(want_chaining);
900526
 	if ((message->flags & DNS_MESSAGEFLAG_AA) != 0)
900526
 		aa = ISC_TRUE;
900526
@@ -6075,14 +6098,15 @@ answer_response(fetchctx_t *fctx) {
900526
 	view = fctx->res->view;
900526
 	result = dns_message_firstname(message, DNS_SECTION_ANSWER);
900526
 	while (!done && result == ISC_R_SUCCESS) {
900526
-		dns_namereln_t namereln;
900526
-		int order;
900526
-		unsigned int nlabels;
900526
+		dns_namereln_t namereln, lastreln;
900526
+		int order, lastorder;
900526
+		unsigned int nlabels, lastnlabels;
900526
 
900526
 		name = NULL;
900526
 		dns_message_currentname(message, DNS_SECTION_ANSWER, &name);
900526
 		external = ISC_TF(!dns_name_issubdomain(name, &fctx->domain));
900526
 		namereln = dns_name_fullcompare(qname, name, &order, &nlabels);
900526
+
900526
 		if (namereln == dns_namereln_equal) {
900526
 			wanted_chaining = ISC_FALSE;
900526
 			for (rdataset = ISC_LIST_HEAD(name->list);
900526
@@ -6188,6 +6212,7 @@ answer_response(fetchctx_t *fctx) {
900526
 							&fctx->domain)) {
900526
 						return (DNS_R_SERVFAIL);
900526
 					}
900526
+					lastcname = name;
900526
 				} else if (rdataset->type == dns_rdatatype_rrsig
900526
 					   && rdataset->covers ==
900526
 					   dns_rdatatype_cname
900526
@@ -6211,7 +6236,7 @@ answer_response(fetchctx_t *fctx) {
900526
 					rdataset->attributes |=
900526
 						DNS_RDATASETATTR_CACHE;
900526
 					rdataset->trust = dns_trust_answer;
900526
-					if (!chaining) {
900526
+					if (chaining == 0) {
900526
 						/*
900526
 						 * This data is "the" answer
900526
 						 * to our question only if
900526
@@ -6288,10 +6313,21 @@ answer_response(fetchctx_t *fctx) {
900526
 			 * cause us to ignore the signatures of
900526
 			 * CNAMEs.
900526
 			 */
900526
-			if (wanted_chaining)
900526
-				chaining = ISC_TRUE;
900526
+			if (wanted_chaining && chaining < 2U)
900526
+				chaining++;
900526
 		} else {
900526
 			dns_rdataset_t *dnameset = NULL;
900526
+			isc_boolean_t synthcname = ISC_FALSE;
900526
+
900526
+			if (lastcname != NULL) {
900526
+				lastreln = dns_name_fullcompare(lastcname,
900526
+								name,
900526
+								&lastorder,
900526
+								&lastnlabels);
900526
+				if (lastreln == dns_namereln_subdomain &&
900526
+				    lastnlabels == dns_name_countlabels(name))
900526
+					synthcname = ISC_TRUE;
900526
+			}
900526
 
900526
 			/*
900526
 			 * Look for a DNAME (or its SIG).  Anything else is
900526
@@ -6320,7 +6356,7 @@ answer_response(fetchctx_t *fctx) {
900526
 				 * If we're not chaining, then the DNAME and
900526
 				 * its signature should not be external.
900526
 				 */
900526
-				if (!chaining && external) {
900526
+				if (chaining == 0 && external) {
900526
 					char qbuf[DNS_NAME_FORMATSIZE];
900526
 					char obuf[DNS_NAME_FORMATSIZE];
900526
 
900526
@@ -6338,16 +6374,9 @@ answer_response(fetchctx_t *fctx) {
900526
 				/*
900526
 				 * If DNAME + synthetic CNAME then the
900526
 				 * namereln is dns_namereln_subdomain.
900526
-				 *
900526
-				 * If synthetic CNAME + DNAME then the
900526
-				 * namereln is dns_namereln_commonancestor
900526
-				 * and the number of label must match the
900526
-				 * DNAME.  This order is not RFC compliant.
900526
 				 */
900526
-
900526
 				if (namereln != dns_namereln_subdomain &&
900526
-				    (namereln != dns_namereln_commonancestor ||
900526
-				     nlabels != dns_name_countlabels(name)))
900526
+				    !synthcname)
900526
 				{
900526
 					char qbuf[DNS_NAME_FORMATSIZE];
900526
 					char obuf[DNS_NAME_FORMATSIZE];
900526
@@ -6367,8 +6396,19 @@ answer_response(fetchctx_t *fctx) {
900526
 					want_chaining = ISC_TRUE;
900526
 					POST(want_chaining);
900526
 					aflag = DNS_RDATASETATTR_ANSWER;
900526
-					result = dname_target(rdataset, qname,
900526
-							      nlabels, &fdname);
900526
+					dns_fixedname_init(&fdname);
900526
+					dname = dns_fixedname_name(&fdname);
900526
+					if (synthcname) {
900526
+						result = fromdname(rdataset,
900526
+								   lastcname,
900526
+								   lastnlabels,
900526
+								   qname);
900526
+					} else {
900526
+						result = dname_target(rdataset,
900526
+								      qname,
900526
+								      nlabels,
900526
+								      dname);
900526
+					}
900526
 					if (result == ISC_R_NOSPACE) {
900526
 						/*
900526
 						 * We can't construct the
900526
@@ -6382,8 +6422,8 @@ answer_response(fetchctx_t *fctx) {
900526
 					else
900526
 						dnameset = rdataset;
900526
 
900526
-					dname = dns_fixedname_name(&fdname);
900526
-					if (!is_answertarget_allowed(view,
900526
+					if (!synthcname &&
900526
+					    !is_answertarget_allowed(view,
900526
 						     qname, rdataset->type,
900526
 						     dname, &fctx->domain))
900526
 					{
900526
@@ -6404,7 +6444,13 @@ answer_response(fetchctx_t *fctx) {
900526
 				name->attributes |= DNS_NAMEATTR_CACHE;
900526
 				rdataset->attributes |= DNS_RDATASETATTR_CACHE;
900526
 				rdataset->trust = dns_trust_answer;
900526
-				if (!chaining) {
900526
+				/*
900526
+				 * If we are not chaining or the first CNAME
900526
+				 * is a synthesised CNAME before the DNAME.
900526
+				 */
900526
+				if ((chaining == 0) ||
900526
+				    (chaining == 1U && synthcname))
900526
+				{
900526
 					/*
900526
 					 * This data is "the" answer to
900526
 					 * our question only if we're
900526
@@ -6414,9 +6460,12 @@ answer_response(fetchctx_t *fctx) {
900526
 					if (aflag == DNS_RDATASETATTR_ANSWER) {
900526
 						have_answer = ISC_TRUE;
900526
 						found_dname = ISC_TRUE;
900526
-						if (cname != NULL)
900526
+						if (cname != NULL &&
900526
+						    synthcname)
900526
+						{
900526
 							cname->attributes &=
900526
 							   ~DNS_NAMEATTR_ANSWER;
900526
+						}
900526
 						name->attributes |=
900526
 							DNS_NAMEATTR_ANSWER;
900526
 					}
900526
@@ -6434,26 +6483,35 @@ answer_response(fetchctx_t *fctx) {
900526
 			 * DNAME chaining.
900526
 			 */
900526
 			if (dnameset != NULL) {
900526
-				/*
900526
-				 * Copy the dname into the qname fixed name.
900526
-				 *
900526
-				 * Although we check for failure of the copy
900526
-				 * operation, in practice it should never fail
900526
-				 * since we already know that the  result fits
900526
-				 * in a fixedname.
900526
-				 */
900526
-				dns_fixedname_init(&fqname);
900526
-				qname = dns_fixedname_name(&fqname);
900526
-				result = dns_name_copy(dname, qname, NULL);
900526
-				if (result != ISC_R_SUCCESS)
900526
-					return (result);
900526
+				if (!synthcname) {
900526
+					/*
900526
+					 * Copy the dname into the qname fixed
900526
+					 * name.
900526
+					 *
900526
+					 * Although we check for failure of the
900526
+					 * copy operation, in practice it
900526
+					 * should never fail since we already
900526
+					 * know that the result fits in a
900526
+					 * fixedname.
900526
+					 */
900526
+					dns_fixedname_init(&fqname);
900526
+					qname = dns_fixedname_name(&fqname);
900526
+					result = dns_name_copy(dname, qname,
900526
+							       NULL);
900526
+					if (result != ISC_R_SUCCESS)
900526
+						return (result);
900526
+				}
900526
 				wanted_chaining = ISC_TRUE;
900526
 				name->attributes |= DNS_NAMEATTR_CHAINING;
900526
 				dnameset->attributes |=
900526
 					    DNS_RDATASETATTR_CHAINING;
900526
 			}
900526
-			if (wanted_chaining)
900526
-				chaining = ISC_TRUE;
900526
+			/*
900526
+			 * Ensure that we can't ever get chaining == 1
900526
+			 * above if we have processed a DNAME.
900526
+			 */
900526
+			if (wanted_chaining && chaining < 2U)
900526
+				chaining += 2;
900526
 		}
900526
 		result = dns_message_nextname(message, DNS_SECTION_ANSWER);
900526
 	}
900526
@@ -6478,7 +6536,7 @@ answer_response(fetchctx_t *fctx) {
900526
 	/*
900526
 	 * Did chaining end before we got the final answer?
900526
 	 */
900526
-	if (chaining) {
900526
+	if (chaining != 0) {
900526
 		/*
900526
 		 * Yes.  This may be a negative reply, so hand off
900526
 		 * authority section processing to the noanswer code.
900526
@@ -6527,7 +6585,7 @@ answer_response(fetchctx_t *fctx) {
900526
 						DNS_NAMEATTR_CACHE;
900526
 					rdataset->attributes |=
900526
 						DNS_RDATASETATTR_CACHE;
900526
-					if (aa && !chaining)
900526
+					if (aa && chaining == 0)
900526
 						rdataset->trust =
900526
 						    dns_trust_authauthority;
900526
 					else
900526
-- 
900526
2.9.3
900526