                    Known defects in ISC BIND 9.5.0

Just before the 9.5.0 release of BIND it was determined that some of
the changes in this release have caused an overuse of memory on systems
serving very large numbers of zones.

Zone ACLs, including allow-transfer, allow-query, allow-notify,
allow-update, and allow-update-forwarding, that are defined in the
"view" or "options" block of named.conf, should be parsed and loaded
once, and then referenced by the zones that use them; however, they
are currently parsed and loaded into memory separately by each zone.  On
systems with hundreds or thousands of zones, this can consume a huge
amount of memory--especially when the ACLs being copied are also large.

There is a fix for this problem, but it was developed too late in the
the test/release cycle for inclusion in BIND 9.5.0 as part of the mainline
source code.  After it has been sufficiently tested, it will be included in
BIND 9.5.1.

In the meantime, the patch is included below for those who wish to
experiment with it.  To apply, run: "patch -p0 < KNOWN-DEFECTS;
make clean; configure; make".

Index: bin/named/server.c
===================================================================
RCS file: /proj/cvs/prod/bind9/bin/named/server.c,v
retrieving revision 1.495.10.10
diff -u -r1.495.10.10 server.c
--- bin/named/server.c	3 Apr 2008 06:20:33 -0000	1.495.10.10
+++ bin/named/server.c	21 May 2008 23:46:14 -0000
@@ -1684,6 +1684,28 @@
 	CHECK(configure_view_sortlist(vconfig, config, actx, ns_g_mctx,
 				      &view->sortlist));
 
+        /*
+         * Configure default allow-transfer, allow-notify, allow-update
+         * and allow-update-forwarding ACLs, if set, so they can be
+         * inherited by zones.
+         */
+	if (view->notifyacl == NULL)
+		CHECK(configure_view_acl(NULL, ns_g_config,
+					 "allow-notify", actx,
+					 ns_g_mctx, &view->notifyacl));
+	if (view->transferacl == NULL)
+		CHECK(configure_view_acl(NULL, ns_g_config,
+					 "allow-transfer", actx,
+					 ns_g_mctx, &view->transferacl));
+	if (view->updateacl == NULL)
+		CHECK(configure_view_acl(NULL, ns_g_config,
+					 "allow-update", actx,
+					 ns_g_mctx, &view->updateacl));
+	if (view->upfwdacl == NULL)
+		CHECK(configure_view_acl(NULL, ns_g_config,
+					 "allow-update-forwarding", actx,
+					 ns_g_mctx, &view->upfwdacl));
+
 	obj = NULL;
 	result = ns_config_get(maps, "request-ixfr", &obj);
 	INSIST(result == ISC_R_SUCCESS);
Index: bin/named/zoneconf.c
===================================================================
RCS file: /proj/cvs/prod/bind9/bin/named/zoneconf.c,v
retrieving revision 1.139.56.3
diff -u -r1.139.56.3 zoneconf.c
--- bin/named/zoneconf.c	21 May 2008 23:26:11 -0000	1.139.56.3
+++ bin/named/zoneconf.c	21 May 2008 23:46:15 -0000
@@ -45,6 +45,15 @@
 #include <named/server.h>
 #include <named/zoneconf.h>
 
+/* ACLs associated with zone */
+typedef enum {
+	allow_notify,
+	allow_query,
+	allow_transfer,
+	allow_update,
+	allow_update_forwarding
+} acl_type_t;
+
 /*%
  * These are BIND9 server defaults, not necessarily identical to the
  * library defaults defined in zone.c.
@@ -60,19 +69,69 @@
  */
 static isc_result_t
 configure_zone_acl(const cfg_obj_t *zconfig, const cfg_obj_t *vconfig,
-		   const cfg_obj_t *config, const char *aclname,
+		   const cfg_obj_t *config, acl_type_t acltype,
 		   cfg_aclconfctx_t *actx, dns_zone_t *zone,
 		   void (*setzacl)(dns_zone_t *, dns_acl_t *),
 		   void (*clearzacl)(dns_zone_t *))
 {
 	isc_result_t result;
-	const cfg_obj_t *maps[5];
+	const cfg_obj_t *maps[5] = {NULL, NULL, NULL, NULL, NULL};
 	const cfg_obj_t *aclobj = NULL;
 	int i = 0;
-	dns_acl_t *dacl = NULL;
+	dns_acl_t **aclp = NULL, *acl = NULL;
+	const char *aclname;
+        dns_view_t *view;
+
+        view = dns_zone_getview(zone);
+
+	switch (acltype) {
+	    case allow_notify:
+                if (view != NULL)
+                        aclp = &view->notifyacl;
+		aclname = "allow-notify";
+		break;
+	    case allow_query:
+                if (view != NULL)
+                        aclp = &view->queryacl;
+		aclname = "allow-query";
+		break;
+	    case allow_transfer:
+                if (view != NULL)
+                        aclp = &view->transferacl;
+		aclname = "allow-transfer";
+		break;
+	    case allow_update:
+                if (view != NULL)
+                        aclp = &view->updateacl;
+		aclname = "allow-update";
+		break;
+	    case allow_update_forwarding:
+                if (view != NULL)
+                        aclp = &view->upfwdacl;
+		aclname = "allow-update-forwarding";
+		break;
+            default:
+                INSIST(0);
+                return (ISC_R_FAILURE);
+	}
 
-	if (zconfig != NULL)
-		maps[i++] = cfg_tuple_get(zconfig, "options");
+	/* First check to see if ACL is defined within the zone */
+	if (zconfig != NULL) {
+		maps[0] = cfg_tuple_get(zconfig, "options");
+		ns_config_get(maps, aclname, &aclobj);
+		if (aclobj != NULL) {
+			aclp = NULL;
+			goto parse_acl;
+		}
+	}
+
+	/* Failing that, see if there's a default ACL already in the view */
+	if (aclp != NULL && *aclp != NULL) {
+		(*setzacl)(zone, *aclp);
+		return (ISC_R_SUCCESS);
+	}
+
+	/* Check for default ACLs that haven't been parsed yet */
 	if (vconfig != NULL)
 		maps[i++] = cfg_tuple_get(vconfig, "options");
 	if (config != NULL) {
@@ -90,12 +149,18 @@
 		return (ISC_R_SUCCESS);
 	}
 
+parse_acl:
 	result = cfg_acl_fromconfig(aclobj, config, ns_g_lctx, actx,
-				    dns_zone_getmctx(zone), 0, &dacl);
+				    dns_zone_getmctx(zone), 0, &acl);
 	if (result != ISC_R_SUCCESS)
 		return (result);
-	(*setzacl)(zone, dacl);
-	dns_acl_detach(&dacl);
+	(*setzacl)(zone, acl);
+
+        /* Set the view default now */
+	if (aclp != NULL)
+		dns_acl_attach(acl, aclp);
+
+	dns_acl_detach(&acl);
 	return (ISC_R_SUCCESS);
 }
 
@@ -454,14 +519,14 @@
 
 	if (ztype == dns_zone_slave)
 		RETERR(configure_zone_acl(zconfig, vconfig, config,
-					  "allow-notify", ac, zone,
+					  allow_notify, ac, zone,
 					  dns_zone_setnotifyacl,
 					  dns_zone_clearnotifyacl));
 	/*
 	 * XXXAG This probably does not make sense for stubs.
 	 */
 	RETERR(configure_zone_acl(zconfig, vconfig, config,
-				  "allow-query", ac, zone,
+				  allow_query, ac, zone,
 				  dns_zone_setqueryacl,
 				  dns_zone_clearqueryacl));
 
@@ -564,7 +629,7 @@
 		dns_zone_setisself(zone, ns_client_isself, NULL);
 
 		RETERR(configure_zone_acl(zconfig, vconfig, config,
-					  "allow-transfer", ac, zone,
+					  allow_transfer, ac, zone,
 					  dns_zone_setxfracl,
 					  dns_zone_clearxfracl));
 
@@ -655,7 +720,7 @@
 	if (ztype == dns_zone_master) {
 		dns_acl_t *updateacl;
 		RETERR(configure_zone_acl(zconfig, vconfig, config,
-					  "allow-update", ac, zone,
+					  allow_update, ac, zone,
 					  dns_zone_setupdateacl,
 					  dns_zone_clearupdateacl));
 
@@ -754,7 +819,7 @@
 				   cfg_obj_asboolean(obj));
 	} else if (ztype == dns_zone_slave) {
 		RETERR(configure_zone_acl(zconfig, vconfig, config,
-					  "allow-update-forwarding", ac, zone,
+					  allow_update_forwarding, ac, zone,
 					  dns_zone_setforwardacl,
 					  dns_zone_clearforwardacl));
 	}
Index: lib/dns/view.c
===================================================================
RCS file: /proj/cvs/prod/bind9/lib/dns/view.c,v
retrieving revision 1.143.128.5
diff -u -r1.143.128.5 view.c
--- lib/dns/view.c	13 May 2008 23:46:31 -0000	1.143.128.5
+++ lib/dns/view.c	21 May 2008 23:46:19 -0000
@@ -172,6 +172,10 @@
 	view->recursionacl = NULL;
 	view->recursiononacl = NULL;
 	view->sortlist = NULL;
+	view->transferacl = NULL;
+	view->notifyacl = NULL;
+	view->updateacl = NULL;
+	view->upfwdacl = NULL;
 	view->requestixfr = ISC_TRUE;
 	view->provideixfr = ISC_TRUE;
 	view->maxcachettl = 7 * 24 * 3600;
@@ -299,6 +303,14 @@
 		dns_acl_detach(&view->recursiononacl);
 	if (view->sortlist != NULL)
 		dns_acl_detach(&view->sortlist);
+	if (view->transferacl != NULL)
+		dns_acl_detach(&view->transferacl);
+	if (view->notifyacl != NULL)
+		dns_acl_detach(&view->notifyacl);
+	if (view->updateacl != NULL)
+		dns_acl_detach(&view->updateacl);
+	if (view->upfwdacl != NULL)
+		dns_acl_detach(&view->upfwdacl);
 	if (view->delonly != NULL) {
 		dns_name_t *name;
 		int i;
Index: lib/dns/include/dns/view.h
===================================================================
RCS file: /proj/cvs/prod/bind9/lib/dns/include/dns/view.h,v
retrieving revision 1.107.128.4
diff -u -r1.107.128.4 view.h
--- lib/dns/include/dns/view.h	3 Apr 2008 06:20:34 -0000	1.107.128.4
+++ lib/dns/include/dns/view.h	21 May 2008 23:46:21 -0000
@@ -123,6 +123,10 @@
 	dns_acl_t *			recursionacl;
 	dns_acl_t *			recursiononacl;
 	dns_acl_t *			sortlist;
+	dns_acl_t *			notifyacl;
+	dns_acl_t *			transferacl;
+	dns_acl_t *			updateacl;
+	dns_acl_t *			upfwdacl;
 	isc_boolean_t			requestixfr;
 	isc_boolean_t			provideixfr;
 	isc_boolean_t			requestnsid;
Index: lib/isccfg/aclconf.c
===================================================================
RCS file: /proj/cvs/prod/bind9/lib/isccfg/aclconf.c,v
retrieving revision 1.17
diff -u -r1.17 aclconf.c
--- lib/isccfg/aclconf.c	21 Dec 2007 06:46:47 -0000	1.17
+++ lib/isccfg/aclconf.c	21 May 2008 23:46:21 -0000
@@ -175,6 +175,7 @@
 	const cfg_listelt_t *elt;
 	dns_iptable_t *iptab;
 	int new_nest_level = 0;
+	int nelem;
 
 	if (nest_level != 0)
 		new_nest_level = nest_level - 1;
@@ -206,6 +207,8 @@
 			return (result);
 	}
 
+	nelem = cfg_list_length(caml, ISC_FALSE);
+
 	de = dacl->elements;
 	for (elt = cfg_list_first(caml);
 	     elt != NULL;
@@ -350,6 +353,16 @@
 				if (result != ISC_R_SUCCESS)
 					goto cleanup;
 
+				/*
+				 * There was only one element and it was
+				 * a nested named ACL; attach it to the
+				 * target and let's go home.
+				 */
+				if (nelem == 1) {
+					dns_acl_attach(inneracl, target);
+					goto cleanup;
+				}
+
 				goto nested_acl;
 			}
 		} else {
