From 6f030e9672d34e36653cc8f573662304a7b22bcd Mon Sep 17 00:00:00 2001 From: Yorgos Thessalonikefs Date: Fri, 24 May 2024 15:21:40 +0200 Subject: [PATCH] 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 | 71 ++++++++++++++++-------------- services/localzone.h | 19 ++++++++ testcode/unitmain.c | 102 +++++++++++++++++++++++++++++++++++++++++++ util/data/dname.h | 2 +- 4 files changed, 161 insertions(+), 33 deletions(-) diff --git a/services/localzone.c b/services/localzone.c index 51056c8ff..d21e0c48a 100644 --- a/services/localzone.c +++ b/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); diff --git a/services/localzone.h b/services/localzone.h index 4456893ee..6f0f28b12 100644 --- a/services/localzone.h +++ b/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 */ diff --git a/testcode/unitmain.c b/testcode/unitmain.c index 647cbca3b..b976ad97a 100644 --- a/testcode/unitmain.c +++ b/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; ilock); + 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 */ diff --git a/util/data/dname.h b/util/data/dname.h index cb0f6735d..15e28bd97 100644 --- a/util/data/dname.h +++ b/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.