[master] omit NS from authority section if it was in answer

4780.	[bug]		When answering ANY queries, don't include the NS
			RRset in the authority section if it was already
			in the answer section. [RT #44543]

CHANGES

+4780.	[bug]		When answering ANY queries, don't include the NS
+			RRset in the authority section if it was already
+			in the answer section. [RT #44543]
+
 4779.	[bug]		Expire NTA at the start of the second. Don't update
 			the expiry value if the record has already expired
 			after a successful check. [RT #46368]

bin/tests/system/additional/clean.sh

@@ -6,14 +6,12 @@
 # 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: clean.sh,v 1.6 2007/09/26 03:22:44 marka Exp $
-
 #
 # Clean up after tests.
 #
 
 rm -f dig.out.*
 rm -f */named.memstats
-rm -f */named.conf
+rm -f ns1/named.conf
 rm -f */named.run
 rm -f ns*/named.lock

bin/tests/system/additional/ns3/named.conf

+/*
+ * Copyright (C) 2017  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/.
+ */
+
+controls { /* empty */ };
+
+options {
+	query-source address 10.53.0.3;
+	notify-source 10.53.0.3;
+	transfer-source 10.53.0.3;
+	port 5300;
+	pid-file "named.pid";
+	listen-on { 10.53.0.3; };
+	listen-on-v6 { none; };
+	recursion yes;
+};
+
+zone "." {
+	type hint;
+	file "root.hint";
+};

bin/tests/system/additional/ns3/root.hint

+; Copyright (C) 2013, 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/.
+
+. NS ns1.
+ns1. A 10.53.0.1

bin/tests/system/additional/tests.sh

@@ -297,5 +297,30 @@ if [ $ret -eq 1 ] ; then
     echo "I: failed"; status=1
 fi
 
+echo "I:reconfiguring server: minimal-responses no"
+cp ns1/named2.conf ns1/named.conf
+$RNDC -c ../common/rndc.conf -s 10.53.0.1 -p 9953 reconfig 2>&1 | sed 's/^/I:ns1 /'
+sleep 2
+
+echo "I:testing NS handling in ANY responses (authoritative)"
+n=`expr $n + 1`
+ret=0
+$DIG -t ANY rt.example @10.53.0.1 -p 5300 > dig.out.$n || ret=1
+grep "AUTHORITY: 0" dig.out.$n  > /dev/null || ret=1
+grep "NS[ 	]*ns" dig.out.$n  > /dev/null || ret=1
+if [ $ret -eq 1 ] ; then
+    echo "I: failed"; status=1
+fi
+
+echo "I:testing NS handling in ANY responses (recursive)"
+n=`expr $n + 1`
+ret=0
+$DIG -t ANY rt.example @10.53.0.3 -p 5300 > dig.out.$n || ret=1
+grep "AUTHORITY: 0" dig.out.$n  > /dev/null || ret=1
+grep "NS[ 	]*ns" dig.out.$n  > /dev/null || ret=1
+if [ $ret -eq 1 ] ; then
+    echo "I: failed"; status=1
+fi
+
 echo "I:exit status: $status"
 [ $status -eq 0 ] || exit 1

lib/ns/include/ns/query.h

@@ -119,6 +119,7 @@ typedef struct query_ctx {
 	isc_boolean_t nxrewrite;		/* negative answer from RPZ */
 	isc_boolean_t findcoveringnsec;		/* lookup covering NSEC */
 	isc_boolean_t want_stale;		/* want stale records? */
+	isc_boolean_t answer_has_ns;		/* NS is in answer */
 	dns_fixedname_t wildcardname;		/* name needing wcard proof */
 	dns_fixedname_t dsname;			/* name needing DS */
 

lib/ns/query.c

@@ -4989,6 +4989,7 @@ qctx_init(ns_client_t *client, dns_fetchevent_t *event,
 	qctx->is_staticstub_zone = ISC_FALSE;
 	qctx->nxrewrite = ISC_FALSE;
 	qctx->want_stale = ISC_FALSE;
+	qctx->answer_has_ns = ISC_FALSE;
 	qctx->authoritative = ISC_FALSE;
 }
 
@@ -6572,6 +6573,21 @@ query_respond_any(query_ctx_t *qctx) {
 				have_a = ISC_TRUE;
 		}
 #endif
+		/*
+		 * We found an NS RRset; no need to add one later.
+		 */
+		if (qctx->qtype == dns_rdatatype_any &&
+		    qctx->rdataset->type == dns_rdatatype_ns)
+		{
+			qctx->answer_has_ns = ISC_TRUE;
+		}
+
+		/*
+		 * Note: if we're in this function, then qctx->type
+		 * is guaranteed to be ANY, but qctx->qtype (i.e. the
+		 * original type requested) might have been RRSIG or
+		 * SIG; we need to check for that.
+		 */
 		if (qctx->is_zone && qctx->qtype == dns_rdatatype_any &&
 		    !dns_db_issecure(qctx->db) &&
 		    dns_rdatatype_isdnssec(qctx->rdataset->type))
@@ -6669,6 +6685,7 @@ query_respond_any(query_ctx_t *qctx) {
 			 */
 			dns_rdataset_disassociate(qctx->rdataset);
 		}
+
 		result = dns_rdatasetiter_next(rdsiter);
 	}
 
@@ -6971,12 +6988,26 @@ query_respond(query_ctx_t *qctx) {
 	}
 
 	/*
-	 * BIND 8 priming queries need the additional section.
+	 * Special case NS handling
 	 */
-	if (qctx->is_zone && qctx->qtype == dns_rdatatype_ns &&
-	    dns_name_equal(qctx->client->query.qname, dns_rootname))
-	{
-		qctx->client->query.attributes &= ~NS_QUERYATTR_NOADDITIONAL;
+	if (qctx->is_zone && qctx->qtype == dns_rdatatype_ns) {
+		/*
+		 * We've already got an NS, no need to add one in
+		 * the authority section
+		 */
+		if (dns_name_equal(qctx->client->query.qname,
+				   dns_db_origin(qctx->db)))
+		{
+			qctx->answer_has_ns = ISC_TRUE;
+		}
+
+		/*
+		 * BIND 8 priming queries need the additional section.
+		 */
+		if (dns_name_equal(qctx->client->query.qname, dns_rootname)) {
+			qctx->client->query.attributes &=
+				~NS_QUERYATTR_NOADDITIONAL;
+		}
 	}
 
 	/*
@@ -10213,14 +10244,12 @@ query_addauth(query_ctx_t *qctx) {
 	 */
 	if (!qctx->want_restart && !NOAUTHORITY(qctx->client)) {
 		if (qctx->is_zone) {
-			if (!((qctx->qtype == dns_rdatatype_ns ||
-			       qctx->qtype == dns_rdatatype_any) &&
-			      dns_name_equal(qctx->client->query.qname,
-					     dns_db_origin(qctx->db))))
-			{
+			if (!qctx->answer_has_ns) {
 				(void)query_addns(qctx);
 			}
-		} else if (qctx->qtype != dns_rdatatype_ns) {
+		} else if (!qctx->answer_has_ns &&
+			   qctx->qtype != dns_rdatatype_ns)
+		{
 			if (qctx->fname != NULL) {
 				query_releasename(qctx->client, &qctx->fname);
 			}