Merge branch '1683-check-the-question-section-when-transferring-zones' into 'master'

Resolve "Check the question section when transferring zones."

Closes #1683

See merge request isc-projects/bind9!3244

CHANGES

@@ -1,3 +1,6 @@
+5432.	[bug]		Check the question section when processing AXFR, IXFR
+			and SOA replies when transfer a zone in. [GL #1683]
+
 5431.	[func]		Reject DS records at the zone apex when loading
 			master files. Log but otherwise ignore attempts to
 			add DS records at the zone apex via UPDATE. [GL #1798]

bin/tests/system/ans.pl

@@ -154,13 +154,13 @@ sub handleUDP {
 	foreach $r (@rules) {
 		my $pattern = $r->{pattern};
 		my($dbtype, $key_name, $key_data) = split(/ /,$pattern);
-		print "[handleUDP] $dbtype, $key_name, $key_data \n";
+		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)) {
+			if (defined($key_name) && defined($key_data)) {
 				my $tsig;
 				# Sign the packet
 				print "  Signing the response with " .
@@ -366,18 +366,18 @@ sub handleTCP {
 	my $r;
 	foreach $r (@rules) {
 		my $pattern = $r->{pattern};
-		my($dbtype, $key_name, $key_data, $extra) = split(/ /,$pattern);
+		my($dbtype, $key_name, $key_data, $tname) = split(/ /,$pattern);
-		print "[handleTCP] $dbtype, $key_name, $key_data \n";
+		print "[handleTCP] $dbtype, $key_name, $key_data, $tname \n";
 		if ("$qname $qtype" =~ /$dbtype/) {
 			$count_these++;
 			my $a;
 			foreach $a (@{$r->{answer}}) {
 				$packet->push("answer", $a);
 			}
-			if(defined($key_name) && $key_name eq "bad-id") {
+			if (defined($key_name) && $key_name eq "bad-id") {
 				$packet->header->id(($id+50)%0xffff);
 				$key_name = $key_data;
-				$key_data = $extra;
+				($key_data, $tname) = split(/ /,$tname)
 			}
 			if (defined($key_name) && defined($key_data)) {
 				my $tsig;
@@ -446,7 +446,10 @@ sub handleTCP {
 			}
 			#$packet->print;
 			push(@results,$packet->data);
-			$packet = new Net::DNS::Packet($qname, $qtype, $qclass);
+			if ($tname eq "") {
+				$tname = $qname;
+			}
+			$packet = new Net::DNS::Packet($tname, $qtype, $qclass);
 			$packet->header->qr(1);
 			$packet->header->aa(1);
 			$packet->header->id($id);

bin/tests/system/xfer/ans5/wrongname

@@ -0,0 +1,10 @@
+/SOA tsig_key LSAnCU+Z/
+nil.      	300	SOA	ns.nil. root.nil. 1 300 300 604800 300
+/AXFR tsig_key LSAnCU+Z/
+nil.      	300	SOA	ns.nil. root.nil. 1 300 300 604800 300
+/AXFR tsig_key LSAnCU+Z ns.wrong./
+nil.      	300	NS	ns.nil.
+nil.		300	TXT	"wrong question AXFR"
+a.nil.		60	A	10.0.0.61
+/AXFR tsig_key LSAnCU+Z/
+nil.      	300	SOA	ns.nil. root.nil. 1 300 300 604800 300

bin/tests/system/xfer/tests.sh

@@ -387,6 +387,25 @@ $DIGCMD nil. TXT | grep 'incorrect key AXFR' >/dev/null && {
     status=$((status+1))
 }
 
+n=$((n+1))
+echo_i "bad question section ($n)"
+
+sendcmd < ans5/wrongname
+
+$RNDCCMD 10.53.0.4 retransfer nil | sed 's/^/ns4 /' | cat_i
+
+sleep 2
+
+nextpart ns4/named.run | grep "question name mismatch" > /dev/null || {
+    echo_i "failed: expected status was not logged"
+    status=$((status+1))
+}
+
+$DIGCMD nil. TXT | grep 'wrong question AXFR' >/dev/null && {
+    echo_i "failed"
+    status=$((status+1))
+}
+
 n=$((n+1))
 echo_i "bad message id ($n)"
 

doc/notes/notes-current.rst

@@ -132,6 +132,9 @@ Feature Changes
    :rfc:`3493` and :rfc:`3542`, this change was introduced in 9.16.0
    but accudently ommited from documentation.
 
+-  The question section is now checked when processing AXFR, IXFR
+   and SOA replies while transferring a zone in. [GL #1683]
+
 Bug Fixes
 ~~~~~~~~~
 

lib/dns/xfrin.c

@@ -1312,6 +1312,61 @@ xfrin_recv_done(isc_task_t *task, isc_event_t *ev) {
 		return;
 	}
 
+	/*
+	 * The question section should exist for SOA and in the first
+	 * message of a AXFR or IXFR response.  The question section
+	 * may exist in the 2nd and subsequent messages in a AXFR or
+	 * IXFR response.  If the question section exists it should
+	 * match the question that was sent.
+	 */
+	if (msg->counts[DNS_SECTION_QUESTION] > 1) {
+		xfrin_log(xfr, ISC_LOG_DEBUG(3), "too many questions (%u)",
+			  msg->counts[DNS_SECTION_QUESTION]);
+		result = DNS_R_FORMERR;
+		goto failure;
+	}
+
+	if ((xfr->state == XFRST_SOAQUERY || xfr->state == XFRST_INITIALSOA) &&
+	    msg->counts[DNS_SECTION_QUESTION] != 1)
+	{
+		xfrin_log(xfr, ISC_LOG_DEBUG(3), "missing question section");
+		result = DNS_R_FORMERR;
+		goto failure;
+	}
+
+	for (result = dns_message_firstname(msg, DNS_SECTION_QUESTION);
+	     result == ISC_R_SUCCESS;
+	     result = dns_message_nextname(msg, DNS_SECTION_QUESTION))
+	{
+		dns_rdataset_t *rds;
+
+		name = NULL;
+		dns_message_currentname(msg, DNS_SECTION_QUESTION, &name);
+		if (!dns_name_equal(name, &xfr->name)) {
+			result = DNS_R_FORMERR;
+			xfrin_log(xfr, ISC_LOG_DEBUG(3),
+				  "question name mismatch");
+			goto failure;
+		}
+		rds = ISC_LIST_HEAD(name->list);
+		INSIST(rds != NULL);
+		if (rds->type != xfr->reqtype) {
+			result = DNS_R_FORMERR;
+			xfrin_log(xfr, ISC_LOG_DEBUG(3),
+				  "question type mismatch");
+			goto failure;
+		}
+		if (rds->rdclass != xfr->rdclass) {
+			result = DNS_R_FORMERR;
+			xfrin_log(xfr, ISC_LOG_DEBUG(3),
+				  "question class mismatch");
+			goto failure;
+		}
+	}
+	if (result != ISC_R_NOMORE) {
+		goto failure;
+	}
+
 	/*
 	 * Does the server know about IXFR?  If it doesn't we will get
 	 * a message with a empty answer section or a potentially a CNAME /

util/copyrights

@@ -934,6 +934,7 @@
 ./bin/tests/system/xfer/ans5/unknownkey		X	2011,2018,2019,2020
 ./bin/tests/system/xfer/ans5/unsigned		X	2011,2018,2019,2020
 ./bin/tests/system/xfer/ans5/wrongkey		X	2011,2018,2019,2020
+./bin/tests/system/xfer/ans5/wrongname		X	2020
 ./bin/tests/system/xfer/axfr-stats.good		X	2019,2020
 ./bin/tests/system/xfer/clean.sh		SH	2000,2001,2004,2007,2011,2012,2013,2014,2015,2016,2018,2019,2020
 ./bin/tests/system/xfer/dig1.good		X	2000,2001,2003,2004,2009,2011,2012,2013,2014,2015,2016,2017,2018,2019,2020