[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]

diff --git a/CHANGES b/CHANGES
index 2949650e8242a35ca17c9fe2136eb1b8260cfa6a..99bd9446d8915243ca5a012e01117f1af567b351 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,7 @@
+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]

diff --git a/bin/tests/system/additional/clean.sh b/bin/tests/system/additional/clean.sh
index f2eba9a5f233699dcb395c7b50ec1773d101e0bd..37f1de9569ecb767697d319ff46353d6bc35b4d3 100644 (file)
--- a/bin/tests/system/additional/clean.sh
+++ b/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

diff --git a/bin/tests/system/additional/ns3/named.conf b/bin/tests/system/additional/ns3/named.conf
new file mode 100644 (file)
index 0000000..d367b48
--- /dev/null
+++ b/bin/tests/system/additional/ns3/named.conf
@@ -0,0 +1,25 @@
+/*
+ * 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";
+};

diff --git a/bin/tests/system/additional/ns3/root.hint b/bin/tests/system/additional/ns3/root.hint
new file mode 100644 (file)
index 0000000..c0eb8d1
--- /dev/null
+++ b/bin/tests/system/additional/ns3/root.hint
@@ -0,0 +1,8 @@
+; 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

diff --git a/bin/tests/system/additional/tests.sh b/bin/tests/system/additional/tests.sh
index e578388a31aed33f35c9608cdbef437a9d57413b..6afc9390f5166cb66ea4d80bd74dff880aaba706 100644 (file)
--- a/bin/tests/system/additional/tests.sh
+++ b/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

diff --git a/lib/ns/include/ns/query.h b/lib/ns/include/ns/query.h
index 63b15a756982e5e6bc6ad779c75a85a63fa6cc03..a81736497fb040697f0fafca1654d42b624a6f03 100644 (file)
--- a/lib/ns/include/ns/query.h
+++ b/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 */
 

diff --git a/lib/ns/query.c b/lib/ns/query.c
index fb9fb230571baf30fbe95811d684bc5fbf671a4a..d4926987415747506d76a8106ffc6579635d35bb 100644 (file)
--- a/lib/ns/query.c
+++ b/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);
                        }