From 352c9f0750ecda9b9714baabc903e107ba71cecb Mon Sep 17 00:00:00 2001
From: Mark Andrews <marka@isc.org>
Date: Fri, 31 Aug 2012 11:20:38 +1000
Subject: [PATCH] 3371.   [bug]           AD=1 should behave like DO=1 when
 deciding whether to                         add NS RRsets to the additional
 section or not.                         [RT #30479]

---
 CHANGES                          |  4 ++++
 bin/named/include/named/client.h | 17 +++++++++--------
 bin/named/query.c                | 21 +++++++++++++++------
 3 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/CHANGES b/CHANGES
index 8a98f3647b..b0a5458eed 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,7 @@
+3371.	[bug]		AD=1 should behave like DO=1 when deciding whether to
+			add NS RRsets to the additional section or not.
+			[RT #30479]
+
 	--- 9.9.2 released ---
 
 3364.	[security]	Named could die on specially crafted record.
diff --git a/bin/named/include/named/client.h b/bin/named/include/named/client.h
index 8254e15746..98e79df706 100644
--- a/bin/named/include/named/client.h
+++ b/bin/named/include/named/client.h
@@ -167,16 +167,17 @@ typedef ISC_LIST(ns_client_t) client_list_t;
 #define NS_CLIENT_MAGIC			ISC_MAGIC('N','S','C','c')
 #define NS_CLIENT_VALID(c)		ISC_MAGIC_VALID(c, NS_CLIENT_MAGIC)
 
-#define NS_CLIENTATTR_TCP		0x01
-#define NS_CLIENTATTR_RA		0x02 /*%< Client gets recursive service */
-#define NS_CLIENTATTR_PKTINFO		0x04 /*%< pktinfo is valid */
-#define NS_CLIENTATTR_MULTICAST		0x08 /*%< recv'd from multicast */
-#define NS_CLIENTATTR_WANTDNSSEC	0x10 /*%< include dnssec records */
-#define NS_CLIENTATTR_WANTNSID          0x20 /*%< include nameserver ID */
+#define NS_CLIENTATTR_TCP		0x001
+#define NS_CLIENTATTR_RA		0x002 /*%< Client gets recursive service */
+#define NS_CLIENTATTR_PKTINFO		0x004 /*%< pktinfo is valid */
+#define NS_CLIENTATTR_MULTICAST		0x008 /*%< recv'd from multicast */
+#define NS_CLIENTATTR_WANTDNSSEC	0x010 /*%< include dnssec records */
+#define NS_CLIENTATTR_WANTNSID          0x020 /*%< include nameserver ID */
 #ifdef ALLOW_FILTER_AAAA_ON_V4
-#define NS_CLIENTATTR_FILTER_AAAA	0x40 /*%< suppress AAAAs */
-#define NS_CLIENTATTR_FILTER_AAAA_RC	0x80 /*%< recursing for A against AAAA */
+#define NS_CLIENTATTR_FILTER_AAAA	0x040 /*%< suppress AAAAs */
+#define NS_CLIENTATTR_FILTER_AAAA_RC	0x080 /*%< recursing for A against AAAA */
 #endif
+#define NS_CLIENTATTR_WANTAD		0x100 /*%< want AD in response if possible */
 
 extern unsigned int ns_client_requests;
 
diff --git a/bin/named/query.c b/bin/named/query.c
index 3c8b91b9fd..13ef81e3e0 100644
--- a/bin/named/query.c
+++ b/bin/named/query.c
@@ -94,6 +94,10 @@
 /*% Want DNSSEC? */
 #define WANTDNSSEC(c)		(((c)->attributes & \
 				  NS_CLIENTATTR_WANTDNSSEC) != 0)
+/*% Want WANTAD? */
+#define WANTAD(c)		(((c)->attributes & \
+				  NS_CLIENTATTR_WANTAD) != 0)
+
 /*% No authority? */
 #define NOAUTHORITY(c)		(((c)->query.attributes & \
 				  NS_QUERYATTR_NOAUTHORITY) != 0)
@@ -3111,6 +3115,14 @@ query_addbestns(ns_client_t *client) {
 	    SECURE(client) && WANTDNSSEC(client))
 		goto cleanup;
 
+	/*
+	 * If the answer is secure only add NS records if they are secure		 * when the client may be looking for AD in the response.
+	 */
+	if (SECURE(client) && (WANTDNSSEC(client) || WANTAD(client)) &&
+	    ((rdataset->trust != dns_trust_secure) ||
+	    (sigrdataset != NULL && sigrdataset->trust != dns_trust_secure)))
+		goto cleanup;
+
 	/*
 	 * If the client doesn't want DNSSEC we can discard the sigrdataset
 	 * now.
@@ -7309,7 +7321,6 @@ ns_query_start(ns_client_t *client) {
 	dns_rdatatype_t qtype;
 	unsigned int saved_extflags = client->extflags;
 	unsigned int saved_flags = client->message->flags;
-	isc_boolean_t want_ad;
 
 	CTRACE("ns_query_start");
 
@@ -7471,13 +7482,11 @@ ns_query_start(ns_client_t *client) {
 		client->query.attributes &= ~NS_QUERYATTR_SECURE;
 
 	/*
-	 * Set 'want_ad' if the client has set AD in the query.
+	 * Set NS_CLIENTATTR_WANTDNSSEC if the client has set AD in the query.
 	 * This allows AD to be returned on queries without DO set.
 	 */
 	if ((message->flags & DNS_MESSAGEFLAG_AD) != 0)
-		want_ad = ISC_TRUE;
-	else
-		want_ad = ISC_FALSE;
+		client->attributes |= NS_CLIENTATTR_WANTAD;
 
 	/*
 	 * This is an ordinary query.
@@ -7502,7 +7511,7 @@ ns_query_start(ns_client_t *client) {
 	 * Set AD.  We must clear it if we add non-validated data to a
 	 * response.
 	 */
-	if (WANTDNSSEC(client) || want_ad)
+	if (WANTDNSSEC(client) || WANTAD(client))
 		message->flags |= DNS_MESSAGEFLAG_AD;
 
 	qclient = NULL;