[rt46603] add release note

- don't overwrite cacheacl if previously set
- ensure recursionacl inherits from queryacl
- also noticed a test for view _bind that didn't check its rdclass; fixed

bin/named/server.c

@@ -511,32 +511,44 @@ nzf_append(dns_view_t *view, const cfg_obj_t *zconfig);
  */
 static isc_result_t
 configure_view_acl(const cfg_obj_t *vconfig, const cfg_obj_t *config,
-		   const char *aclname, const char *acltuplename,
-		   cfg_aclconfctx_t *actx, isc_mem_t *mctx, dns_acl_t **aclp)
+		   const cfg_obj_t *gconfig, const char *aclname,
+		   const char *acltuplename, cfg_aclconfctx_t *actx,
+		   isc_mem_t *mctx, dns_acl_t **aclp)
 {
 	isc_result_t result;
-	const cfg_obj_t *maps[3];
+	const cfg_obj_t *maps[4];
 	const cfg_obj_t *aclobj = NULL;
 	int i = 0;
 
-	if (*aclp != NULL)
+	if (*aclp != NULL) {
 		dns_acl_detach(aclp);
-	if (vconfig != NULL)
+	}
+	if (vconfig != NULL) {
 		maps[i++] = cfg_tuple_get(vconfig, "options");
+	}
 	if (config != NULL) {
 		const cfg_obj_t *options = NULL;
 		(void)cfg_map_get(config, "options", &options);
-		if (options != NULL)
+		if (options != NULL) {
 			maps[i++] = options;
+		}
+	}
+	if (gconfig != NULL) {
+		const cfg_obj_t *options = NULL;
+		(void)cfg_map_get(gconfig, "options", &options);
+		if (options != NULL) {
+			maps[i++] = options;
+		}
 	}
 	maps[i] = NULL;
 
 	(void)named_config_get(maps, aclname, &aclobj);
-	if (aclobj == NULL)
+	if (aclobj == NULL) {
 		/*
 		 * No value available.	*aclp == NULL.
 		 */
 		return (ISC_R_SUCCESS);
+	}
 
 	if (acltuplename != NULL) {
 		/*
@@ -3706,13 +3718,9 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist,
 	CHECKM(named_config_getport(config, &port), "port");
 	dns_view_setdstport(view, port);
 
-	CHECK(configure_view_acl(vconfig, config, "allow-query", NULL, actx,
+	CHECK(configure_view_acl(vconfig, config, named_g_config,
+				 "allow-query", NULL, actx,
 				 named_g_mctx, &view->queryacl));
-	if (view->queryacl == NULL) {
-		CHECK(configure_view_acl(NULL, named_g_config, "allow-query",
-					 NULL, actx, named_g_mctx,
-					 &view->queryacl));
-	}
 
 	/*
 	 * Make the list of response policy zone names for a view that
@@ -4595,11 +4603,16 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist,
 
 	/*
 	 * Configure the "match-clients" and "match-destinations" ACL.
+	 * (These are only meaningful at the view level, but 'config'
+	 * must be passed so that named ACLs defined at the global level
+	 * can be retrieved.)
 	 */
-	CHECK(configure_view_acl(vconfig, config, "match-clients", NULL, actx,
-				 named_g_mctx, &view->matchclients));
-	CHECK(configure_view_acl(vconfig, config, "match-destinations", NULL,
-				 actx, named_g_mctx, &view->matchdestinations));
+	CHECK(configure_view_acl(vconfig, config, NULL, "match-clients",
+				 NULL, actx, named_g_mctx,
+				 &view->matchclients));
+	CHECK(configure_view_acl(vconfig, config, NULL, "match-destinations",
+				 NULL, actx, named_g_mctx,
+				 &view->matchdestinations));
 
 	/*
 	 * Configure the "match-recursive-only" option.
@@ -4668,70 +4681,85 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist,
 	INSIST(result == ISC_R_SUCCESS);
 	view->trust_anchor_telemetry = cfg_obj_asboolean(obj);
 
+	CHECK(configure_view_acl(vconfig, config, named_g_config,
+				 "allow-query-cache-on", NULL, actx,
+				 named_g_mctx, &view->cacheonacl));
 	/*
-	 * Set "allow-query-cache", "allow-query-cache-on",
-	 * "allow-recursion", and "allow-recursion-on" acls if
-	 * configured in named.conf.
+	 * Set "allow-query-cache", "allow-recursion", and
+	 * "allow-recursion-on" acls if configured in named.conf.
+	 * (Ignore the global defaults for now, because these ACLs
+	 * can inherit from each other when only some of them set at
+	 * the options/view level.)
 	 */
-	CHECK(configure_view_acl(vconfig, config, "allow-query-cache", NULL,
-				 actx, named_g_mctx, &view->cacheacl));
-	CHECK(configure_view_acl(vconfig, config, "allow-query-cache-on", NULL,
-				 actx, named_g_mctx, &view->cacheonacl));
-	if (view->cacheonacl == NULL)
-		CHECK(configure_view_acl(NULL, named_g_config,
-					 "allow-query-cache-on", NULL, actx,
-					 named_g_mctx, &view->cacheonacl));
-	if (strcmp(view->name, "_bind") != 0) {
-		CHECK(configure_view_acl(vconfig, config, "allow-recursion",
-					 NULL, actx, named_g_mctx,
-					 &view->recursionacl));
-		CHECK(configure_view_acl(vconfig, config, "allow-recursion-on",
-					 NULL, actx, named_g_mctx,
-					 &view->recursiononacl));
-	}
+	CHECK(configure_view_acl(vconfig, config, NULL, "allow-query-cache",
+				 NULL, actx, named_g_mctx, &view->cacheacl));
 
-	/*
-	 * "allow-query-cache" inherits from "allow-recursion" if set,
-	 * otherwise from "allow-query" if set.
-	 * "allow-recursion" inherits from "allow-query-cache" if set,
-	 * otherwise from "allow-query" if set.
-	 */
-	if (view->cacheacl == NULL && view->recursionacl != NULL) {
-		dns_acl_attach(view->recursionacl, &view->cacheacl);
-	}
-
-	if (view->cacheacl == NULL && view->recursion) {
-		dns_acl_attach(view->queryacl, &view->cacheacl);
-	}
-
-	if (view->recursion &&
-	    view->recursionacl == NULL && view->cacheacl != NULL)
+	if (strcmp(view->name, "_bind") != 0 &&
+	    view->rdclass != dns_rdataclass_chaos)
 	{
-		dns_acl_attach(view->cacheacl, &view->recursionacl);
+		CHECK(configure_view_acl(vconfig, config, NULL,
+					 "allow-recursion", NULL, actx,
+					 named_g_mctx, &view->recursionacl));
+		CHECK(configure_view_acl(vconfig, config, NULL,
+					 "allow-recursion-on", NULL, actx,
+					 named_g_mctx, &view->recursiononacl));
 	}
 
-	/*
-	 * Set default "allow-recursion", "allow-recursion-on" and
-	 * "allow-query-cache" acls.
-	 */
-	if (view->recursionacl == NULL && view->recursion)
-		CHECK(configure_view_acl(NULL, named_g_config,
-					 "allow-recursion", NULL,
-					 actx, named_g_mctx,
-					 &view->recursionacl));
-	if (view->recursiononacl == NULL && view->recursion)
-		CHECK(configure_view_acl(NULL, named_g_config,
-					 "allow-recursion-on", NULL,
-					 actx, named_g_mctx,
-					 &view->recursiononacl));
-	if (view->cacheacl == NULL) {
-		if (view->recursion)
-			CHECK(configure_view_acl(NULL, named_g_config,
+	if (view->recursion) {
+		/*
+		 * "allow-query-cache" inherits from "allow-recursion" if set,
+		 * otherwise from "allow-query" if set.
+		 * "allow-recursion" inherits from "allow-query-cache" if set,
+		 * otherwise from "allow-query" if set.
+		 */
+		if (view->cacheacl == NULL) {
+			if (view->recursionacl != NULL) {
+				dns_acl_attach(view->recursionacl,
+					       &view->cacheacl);
+			} else if (view->queryacl != NULL) {
+				dns_acl_attach(view->queryacl,
+					       &view->cacheacl);
+			}
+		}
+		if (view->recursionacl == NULL) {
+			if (view->cacheacl != NULL) {
+				dns_acl_attach(view->cacheacl,
+					       &view->recursionacl);
+			} else if (view->queryacl != NULL) {
+				dns_acl_attach(view->queryacl,
+					       &view->recursionacl);
+			}
+		}
+
+		/*
+		 * If any are still unset, we now get default "allow-recursion",
+		 * "allow-recursion-on" and "allow-query-cache" ACLs from
+		 * the global config.
+		 */
+		if (view->recursionacl == NULL) {
+			CHECK(configure_view_acl(NULL, NULL, named_g_config,
+						 "allow-recursion", NULL,
+						 actx, named_g_mctx,
+						 &view->recursionacl));
+		}
+		if (view->recursiononacl == NULL) {
+			CHECK(configure_view_acl(NULL, NULL, named_g_config,
+						 "allow-recursion-on", NULL,
+						 actx, named_g_mctx,
+						 &view->recursiononacl));
+		}
+		if (view->cacheacl == NULL) {
+			CHECK(configure_view_acl(NULL, NULL, named_g_config,
 						 "allow-query-cache", NULL,
 						 actx, named_g_mctx,
 						 &view->cacheacl));
-		else
-			CHECK(dns_acl_none(mctx, &view->cacheacl));
+		}
+	} else if (view->cacheacl == NULL) {
+		/*
+		 * We're not recursive; if "allow-query-cache" hasn't been
+		 * set at the options/view level, set it to none.
+		 */
+		CHECK(dns_acl_none(mctx, &view->cacheacl));
 	}
 
 	/*
@@ -4739,14 +4767,14 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist,
 	 * clients. This causes case not always to be preserved,
 	 * and is needed by some broken clients.
 	 */
-	CHECK(configure_view_acl(vconfig, config, "no-case-compress", NULL,
-				 actx, named_g_mctx, &view->nocasecompress));
+	CHECK(configure_view_acl(vconfig, config, named_g_config,
+				 "no-case-compress", NULL, actx,
+				 named_g_mctx, &view->nocasecompress));
 
 	/*
 	 * Disable name compression completely, this is a tradeoff
 	 * between CPU and network usage.
 	 */
-
 	obj = NULL;
 	result = named_config_get(maps, "message-compression", &obj);
 	INSIST(result == ISC_R_SUCCESS);
@@ -4755,8 +4783,9 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist,
 	/*
 	 * Filter setting on addresses in the answer section.
 	 */
-	CHECK(configure_view_acl(vconfig, config, "deny-answer-addresses",
-				 "acl", actx, named_g_mctx,
+	CHECK(configure_view_acl(vconfig, config, named_g_config,
+				 "deny-answer-addresses", "acl",
+				 actx, named_g_mctx,
 				 &view->denyansweracl));
 	CHECK(configure_view_nametable(vconfig, config, "deny-answer-addresses",
 				       "except-from", named_g_mctx,
@@ -4779,26 +4808,36 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist,
 				      &view->sortlist));
 
 	/*
-	 * Configure default allow-transfer, allow-notify, allow-update
-	 * and allow-update-forwarding ACLs, if set, so they can be
-	 * inherited by zones.
+	 * Configure default allow-notify, allow-update
+	 * and allow-update-forwarding ACLs, so they can be
+	 * inherited by zones. (Note these cannot be set at
+	 * options/view level.)
 	 */
-	if (view->notifyacl == NULL)
-		CHECK(configure_view_acl(NULL, named_g_config,
+	if (view->notifyacl == NULL) {
+		CHECK(configure_view_acl(vconfig, config, named_g_config,
 					 "allow-notify", NULL, actx,
 					 named_g_mctx, &view->notifyacl));
-	if (view->transferacl == NULL)
-		CHECK(configure_view_acl(NULL, named_g_config,
-					 "allow-transfer", NULL, actx,
-					 named_g_mctx, &view->transferacl));
-	if (view->updateacl == NULL)
-		CHECK(configure_view_acl(NULL, named_g_config,
+	}
+	if (view->updateacl == NULL) {
+		CHECK(configure_view_acl(NULL, NULL, named_g_config,
 					 "allow-update", NULL, actx,
 					 named_g_mctx, &view->updateacl));
-	if (view->upfwdacl == NULL)
-		CHECK(configure_view_acl(NULL, named_g_config,
+	}
+	if (view->upfwdacl == NULL) {
+		CHECK(configure_view_acl(NULL, NULL, named_g_config,
 					 "allow-update-forwarding", NULL, actx,
 					 named_g_mctx, &view->upfwdacl));
+	}
+
+	/*
+	 * Configure default allow-transer ACL so it can be inherited
+	 * by zones. (Note this *can* be set at options or view level.)
+	 */
+	if (view->transferacl == NULL) {
+		CHECK(configure_view_acl(vconfig, config, named_g_config,
+					 "allow-transfer", NULL, actx,
+					 named_g_mctx, &view->transferacl));
+	}
 
 	obj = NULL;
 	result = named_config_get(maps, "provide-ixfr", &obj);
@@ -4921,8 +4960,9 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist,
 			INSIST(0);
 	}
 
-	CHECK(configure_view_acl(vconfig, config, "filter-aaaa", NULL,
-				 actx, named_g_mctx, &view->aaaa_acl));
+	CHECK(configure_view_acl(vconfig, config, named_g_config,
+				 "filter-aaaa", NULL, actx,
+				 named_g_mctx, &view->aaaa_acl));
 
 	obj = NULL;
 	result = named_config_get(maps, "prefetch", &obj);
@@ -7847,7 +7887,11 @@ load_configuration(const char *filename, named_server_t *server,
 
 	isc_quota_soft(&server->sctx->recursionquota, softquota);
 
-	CHECK(configure_view_acl(NULL, config, "blackhole", NULL,
+	/*
+	 * Set "blackhole". Only legal at options level; there is
+	 * no default.
+	 */
+	CHECK(configure_view_acl(NULL, config, NULL, "blackhole", NULL,
 				 named_g_aclconfctx, named_g_mctx,
 				 &server->sctx->blackholeacl));
 	if (server->sctx->blackholeacl != NULL) {
@@ -7855,7 +7899,11 @@ load_configuration(const char *filename, named_server_t *server,
 					     server->sctx->blackholeacl);
 	}
 
-	CHECK(configure_view_acl(NULL, config, "keep-response-order", NULL,
+	/*
+	 * Set "blackhole". Only legal at options or global defaults level.
+	 */
+	CHECK(configure_view_acl(NULL, config, named_g_config,
+				 "keep-response-order", NULL,
 				 named_g_aclconfctx, named_g_mctx,
 				 &server->sctx->keepresporder));
 

bin/tests/system/acl/clean.sh

@@ -15,3 +15,5 @@ rm -f ns2/example.db ns2/tsigzone.db ns2/example.db.jnl ns2/named.conf
 rm -f */named.memstats
 rm -f */named.run
 rm -f ns*/named.lock
+rm -f ns*/_default.nzf
+rm -f ns*/_default.nzd*

bin/tests/system/acl/ns3/example.db

@@ -0,0 +1,10 @@
+$TTL 300	; 5 minutes
+@			IN SOA	ns root (
+				2000082401 ; serial
+				1800       ; refresh (30 minutes)
+				1800       ; retry (30 minutes)
+				1814400    ; expire (3 weeks)
+				3600       ; minimum (1 hour)
+				)
+			NS	ns
+ns			A	10.53.0.3

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

@@ -0,0 +1,22 @@
+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 no;
+        notify no;
+        allow-new-zones yes;
+        allow-transfer { none; };
+};
+
+controls {
+        inet 10.53.0.3 port 9953 allow { any; } keys { rndc_key; };
+};
+
+key rndc_key {
+        secret "1234abcd8765";
+        algorithm hmac-sha256;
+};

bin/tests/system/acl/ns4/example.db

@@ -0,0 +1,10 @@
+$TTL 300	; 5 minutes
+@			IN SOA	ns root (
+				2000082401 ; serial
+				1800       ; refresh (30 minutes)
+				1800       ; retry (30 minutes)
+				1814400    ; expire (3 weeks)
+				3600       ; minimum (1 hour)
+				)
+			NS	ns
+ns			A	10.53.0.4

bin/tests/system/acl/ns4/existing.db

@@ -0,0 +1,10 @@
+$TTL 300	; 5 minutes
+@			IN SOA	ns root (
+				2000082401 ; serial
+				1800       ; refresh (30 minutes)
+				1800       ; retry (30 minutes)
+				1814400    ; expire (3 weeks)
+				3600       ; minimum (1 hour)
+				)
+			NS	ns
+ns			A	10.53.0.4

bin/tests/system/acl/ns4/named.conf

@@ -0,0 +1,27 @@
+options {
+        query-source address 10.53.0.4;
+        notify-source 10.53.0.4;
+        transfer-source 10.53.0.4;
+        port 5300;
+        pid-file "named.pid";
+        listen-on { 10.53.0.4; };
+        listen-on-v6 { none; };
+        recursion no;
+        notify no;
+        allow-new-zones yes;
+        allow-transfer { none; };
+};
+
+controls {
+        inet 10.53.0.4 port 9953 allow { any; } keys { rndc_key; };
+};
+
+key rndc_key {
+        secret "1234abcd8765";
+        algorithm hmac-sha256;
+};
+
+zone "existing" {
+        type master;
+        file "existing.db";
+};

bin/tests/system/acl/tests.sh

@@ -172,5 +172,65 @@ t=`expr $t + 1`
 $DIG example. soa @10.53.0.2 +subnet="192.0.2.128/32" -p 5300 > dig.out.${t}
 grep "CLIENT-SUBNET.*192.0.2.128/32/24" dig.out.${t} > /dev/null || { echo "I:test $t failed" ; status=1; }
 
+# AXFR tests against ns3
+
+echo "I:testing allow-transfer ACLs against ns3 (no existing zones)"
+
+echo "I:calling addzone example.com on ns3"
+$RNDC -c ../common/rndc.conf -s 10.53.0.3 -p 9953 addzone 'example.com {type master; file "example.db"; }; '
+
+sleep 1
+
+t=`expr $t + 1`
+ret=0
+echo "I:checking AXFR of example.com from ns3 with ACL allow-transfer { none; }; (${t})"
+$DIG @10.53.0.3 -p 5300 example.com axfr > dig.out.${t} 2>&1
+grep "Transfer failed." dig.out.${t} >/dev/null 2>&1 || ret=1
+[ $ret -eq 0 ] || echo "I:failed"
+status=`expr $status + $ret`
+
+echo "I:calling rndc reconfig"
+$RNDC -c ../common/rndc.conf -s 10.53.0.3 -p 9953 reconfig
+
+sleep 1
+
+t=`expr $t + 1`
+ret=0
+echo "I:re-checking AXFR of example.com from ns3 with ACL allow-transfer { none; }; (${t})"
+$DIG @10.53.0.3 -p 5300 example.com axfr > dig.out.${t} 2>&1
+grep "Transfer failed." dig.out.${t} >/dev/null 2>&1 || ret=1
+[ $ret -eq 0 ] || echo "I:failed"
+status=`expr $status + $ret`
+
+# AXFR tests against ns4
+
+echo "I:testing allow-transfer ACLs against ns4 (1 pre-existing zone)"
+
+echo "I:calling addzone example.com on ns4"
+$RNDC -c ../common/rndc.conf -s 10.53.0.4 -p 9953 addzone 'example.com {type master; file "example.db"; }; '
+
+sleep 1
+
+t=`expr $t + 1`
+ret=0
+echo "I:checking AXFR of example.com from ns4 with ACL allow-transfer { none; }; (${t})"
+$DIG @10.53.0.4 -p 5300 example.com axfr > dig.out.${t} 2>&1
+grep "Transfer failed." dig.out.${t} >/dev/null 2>&1 || ret=1
+[ $ret -eq 0 ] || echo "I:failed"
+status=`expr $status + $ret`
+
+echo "I:calling rndc reconfig"
+$RNDC -c ../common/rndc.conf -s 10.53.0.4 -p 9953 reconfig
+
+sleep 1
+
+t=`expr $t + 1`
+ret=0
+echo "I:re-checking AXFR of example.com from ns4 with ACL allow-transfer { none; }; (${t})"
+$DIG @10.53.0.4 -p 5300 example.com axfr > dig.out.${t} 2>&1
+grep "Transfer failed." dig.out.${t} >/dev/null 2>&1 || ret=1
+[ $ret -eq 0 ] || echo "I:failed"
+status=`expr $status + $ret`
+
 echo "I:exit status: $status"
 [ $status -eq 0 ] || exit 1

doc/arm/notes.xml

@@ -734,6 +734,14 @@
 
   <section xml:id="relnotes_bugs"><info><title>Bug Fixes</title></info>
     <itemizedlist>
+      <listitem>
+	<para>
+	  Zones created with <command>rndc addzone</command> could
+	  temporarily fail to inherit the <command>allow-transfer</command>
+	  ACL set in the <command>options</command> section of
+	  <filename>named.conf</filename>. [RT #46603]
+	</para>
+      </listitem>
       <listitem>
 	<para>
 	  The introduction of <command>libns</command> caused a bug