Proper parent identification for dynamically entered local zones (#1076)

- Fix #1059: Intermittent DNS blocking failure with local-zone and
  always_nxdomain. Addition of local_zones dynamically via
  unbound-control was not finding the zone's parent correctly.

services/localzone.c

@@ -242,7 +242,7 @@ lz_enter_zone_dname(struct local_zones* zones, uint8_t* nm, size_t len,
 }
 
 /** enter a new zone */
-static struct local_zone*
+struct local_zone*
 lz_enter_zone(struct local_zones* zones, const char* name, const char* type,
 	uint16_t dclass)
 {
@@ -983,40 +983,43 @@ lz_enter_overrides(struct local_zones* zones, struct config_file* cfg)
 	return 1;
 }
 
+/* return closest parent in the tree, NULL if none */
+static struct local_zone* find_closest_parent(struct local_zone* curr,
+	struct local_zone* prev)
+{
+	struct local_zone* p;
+	int m;
+	if(!prev || prev->dclass != curr->dclass) return NULL;
+	(void)dname_lab_cmp(prev->name, prev->namelabs, curr->name,
+		curr->namelabs, &m); /* we know prev is smaller */
+	/* sort order like: . com. bla.com. zwb.com. net. */
+	/* find the previous, or parent-parent-parent */
+	for(p = prev; p; p = p->parent) {
+		/* looking for name with few labels, a parent */
+		if(p->namelabs <= m) {
+			/* ==: since prev matched m, this is closest*/
+			/* <: prev matches more, but is not a parent,
+			    * this one is a (grand)parent */
+			return p;
+		}
+	}
+	return NULL;
+}
+
 /** setup parent pointers, so that a lookup can be done for closest match */
-static void
-init_parents(struct local_zones* zones)
+void
+lz_init_parents(struct local_zones* zones)
 {
-        struct local_zone* node, *prev = NULL, *p;
-        int m;
+	struct local_zone* node, *prev = NULL;
 	lock_rw_wrlock(&zones->lock);
-        RBTREE_FOR(node, struct local_zone*, &zones->ztree) {
+	RBTREE_FOR(node, struct local_zone*, &zones->ztree) {
 		lock_rw_wrlock(&node->lock);
-                node->parent = NULL;
-                if(!prev || prev->dclass != node->dclass) {
-                        prev = node;
-			lock_rw_unlock(&node->lock);
-                        continue;
-                }
-                (void)dname_lab_cmp(prev->name, prev->namelabs, node->name,
-                        node->namelabs, &m); /* we know prev is smaller */
-                /* sort order like: . com. bla.com. zwb.com. net. */
-                /* find the previous, or parent-parent-parent */
-                for(p = prev; p; p = p->parent)
-                        /* looking for name with few labels, a parent */
-                        if(p->namelabs <= m) {
-                                /* ==: since prev matched m, this is closest*/
-                                /* <: prev matches more, but is not a parent,
-                                 * this one is a (grand)parent */
-                                node->parent = p;
-                                break;
-                        }
-                prev = node;
-
+		node->parent = find_closest_parent(node, prev);
+		prev = node;
 		if(node->override_tree)
 			addr_tree_init_parents(node->override_tree);
 		lock_rw_unlock(&node->lock);
-        }
+	}
 	lock_rw_unlock(&zones->lock);
 }
 
@@ -1036,7 +1039,7 @@ lz_setup_implicit(struct local_zones* zones, struct config_file* cfg)
 	int nmlabs = 0;
 	int match = 0; /* number of labels match count */
 
-	init_parents(zones); /* to enable local_zones_lookup() */
+	lz_init_parents(zones); /* to enable local_zones_lookup() */
 	for(p = cfg->local_data; p; p = p->next) {
 		uint8_t* rr_name;
 		uint16_t rr_class, rr_type;
@@ -1202,7 +1205,7 @@ local_zones_apply_cfg(struct local_zones* zones, struct config_file* cfg)
 	}
 
 	/* setup parent ptrs for lookup during data entry */
-	init_parents(zones);
+	lz_init_parents(zones);
 	/* insert local zone tags */
 	if(!lz_enter_zone_tags(zones, cfg)) {
 		return 0;
@@ -2028,7 +2031,9 @@ struct local_zone* local_zones_add_zone(struct local_zones* zones,
 	uint8_t* name, size_t len, int labs, uint16_t dclass,
 	enum localzone_type tp)
 {
+	int exact;
 	/* create */
+	struct local_zone *prev;
 	struct local_zone* z = local_zone_create(name, len, labs, tp, dclass);
 	if(!z) {
 		free(name);
@@ -2037,10 +2042,12 @@ struct local_zone* local_zones_add_zone(struct local_zones* zones,
 	lock_rw_wrlock(&z->lock);
 
 	/* find the closest parent */
-	z->parent = local_zones_find(zones, name, len, labs, dclass);
+	prev = local_zones_find_le(zones, name, len, labs, dclass, &exact);
+	if(!exact)
+		z->parent = find_closest_parent(z, prev);
 
 	/* insert into the tree */
-	if(!rbtree_insert(&zones->ztree, &z->node)) {
+	if(exact||!rbtree_insert(&zones->ztree, &z->node)) {
 		/* duplicate entry! */
 		lock_rw_unlock(&z->lock);
 		local_zone_delete(z);

services/localzone.h

@@ -641,4 +641,23 @@ local_zone_enter_rr(struct local_zone* z, uint8_t* nm, size_t nmlen,
  */
 struct local_data* 
 local_zone_find_data(struct local_zone* z, uint8_t* nm, size_t nmlen, int nmlabs);
+
+/** Enter a new zone; returns with WRlock
+ *  Made public for unit testing
+ *  @param zones: the local zones tree
+ *  @param name: name of the zone
+ *  @param type: type of the zone
+ *  @param dclass: class of the zone
+ *  @return local_zone (or duplicate), NULL on parse and malloc failures
+ */
+struct local_zone*
+lz_enter_zone(struct local_zones* zones, const char* name, const char* type,
+	uint16_t dclass);
+
+/** Setup parent pointers, so that a lookup can be done for closest match
+ *  Made public for unit testing
+ *  @param zones: the local zones tree
+ */
+void
+lz_init_parents(struct local_zones* zones);
 #endif /* SERVICES_LOCALZONE_H */

testcode/unitmain.c

@@ -1252,6 +1252,107 @@ static void edns_ede_answer_encode_test(void)
 	regional_destroy(region);
 }
 
+#include "services/localzone.h"
+/* Utility function that compares two localzone trees */
+static void compare_localzone_trees(struct local_zones* z1,
+	struct local_zones* z2)
+{
+	struct local_zone *node1, *node2;
+	lock_rw_rdlock(&z1->lock);
+	lock_rw_rdlock(&z2->lock);
+	/* size should be the same */
+	unit_assert(z1->ztree.count == z2->ztree.count);
+	for(node1=(struct local_zone*)rbtree_first(&z1->ztree),
+		node2=(struct local_zone*)rbtree_first(&z2->ztree);
+		(rbnode_type*)node1 != RBTREE_NULL &&
+		(rbnode_type*)node2 != RBTREE_NULL;
+		node1=(struct local_zone*)rbtree_next((rbnode_type*)node1),
+		node2=(struct local_zone*)rbtree_next((rbnode_type*)node2)) {
+		int labs;
+		/* the same zone should be at the same nodes */
+		unit_assert(!dname_lab_cmp(
+			node1->name, node1->namelabs,
+			node2->name, node2->namelabs,
+			&labs));
+		/* the zone's parent should be the same on both nodes */
+		unit_assert(
+			(node1->parent == NULL && node2->parent == NULL) ||
+			(node1->parent != NULL && node2->parent != NULL));
+		if(node1->parent) {
+			unit_assert(!dname_lab_cmp(
+				node1->parent->name, node1->parent->namelabs,
+				node2->parent->name, node2->parent->namelabs,
+				&labs));
+		}
+	}
+	lock_rw_unlock(&z1->lock);
+	lock_rw_unlock(&z2->lock);
+}
+
+/* test that zone addition results in the same tree from both the configuration
+ * file and the unbound-control commands */
+static void localzone_parents_test(void)
+{
+	struct local_zones *z1, *z2;
+	size_t i;
+	char* zone_data[] = {
+		"one",
+		"a.b.c.one",
+		"b.c.one",
+		"c.one",
+		"two",
+		"c.two",
+		"b.c.two",
+		"a.b.c.two",
+		"a.b.c.three",
+		"b.c.three",
+		"c.three",
+		"three",
+		"c.four",
+		"b.c.four",
+		"a.b.c.four",
+		"four",
+		"."
+	};
+	unit_show_feature("localzones parent calculation");
+	z1 = local_zones_create();
+	z2 = local_zones_create();
+	/* parse test data */
+	for(i=0; i<sizeof(zone_data)/sizeof(zone_data[0]); i++) {
+		uint8_t* nm;
+		int nmlabs;
+		size_t nmlen;
+		struct local_zone* z;
+
+		/* This is the config way */
+		z = lz_enter_zone(z1, zone_data[i], "always_nxdomain",
+			LDNS_RR_CLASS_IN);
+		lock_rw_unlock(&z->lock);
+		lz_init_parents(z1);
+
+		/* This is the unbound-control way */
+		nm = sldns_str2wire_dname(zone_data[i], &nmlen);
+		if(!nm) unit_assert(0);
+		nmlabs = dname_count_size_labels(nm, &nmlen);
+		lock_rw_wrlock(&z2->lock);
+		local_zones_add_zone(z2, nm, nmlen, nmlabs, LDNS_RR_CLASS_IN,
+			local_zone_always_nxdomain);
+		lock_rw_unlock(&z2->lock);
+	}
+	/* The trees should be the same, iterate and check the nodes */
+	compare_localzone_trees(z1, z2);
+
+	/* cleanup */
+	local_zones_delete(z1);
+	local_zones_delete(z2);
+}
+
+/** localzone unit tests */
+static void localzone_test(void)
+{
+	localzone_parents_test();
+}
+
 void unit_show_func(const char* file, const char* func)
 {
 	printf("test %s:%s\n", file, func);
@@ -1325,6 +1426,7 @@ main(int argc, char* argv[])
 	tcpreuse_test();
 	msgparse_test();
 	edns_ede_answer_encode_test();
+	localzone_test();
 #ifdef CLIENT_SUBNET
 	ecs_test();
 #endif /* CLIENT_SUBNET */

util/data/dname.h

@@ -225,7 +225,7 @@ int dname_strict_subdomain(uint8_t* d1, int labs1, uint8_t* d2, int labs2);
 int dname_strict_subdomain_c(uint8_t* d1, uint8_t* d2);
 
 /**
- * Counts labels. Tests is d1 is a subdomain of d2.
+ * Counts labels. Tests if d1 is a subdomain of d2.
  * @param d1: domain name, uncompressed wireformat
  * @param d2: domain name, uncompressed wireformat
  * @return true if d1 is a subdomain of d2.