From: W.C.A. Wijngaards Date: Thu, 23 Jul 2026 14:28:45 +0000 (+0200) Subject: - Fix mesh cycle detection for configuration with respip CNAME X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;ds=sidebyside;p=thirdparty%2Funbound.git - Fix mesh cycle detection for configuration with respip CNAME loop and tagged clients. Thanks to Qifan Zhang, Palo Alto Networks, for the report. --- diff --git a/daemon/remote.c b/daemon/remote.c index 4d9fe50ac..8e6ba19b1 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -5026,6 +5026,74 @@ fr_check_changed_cfg_str2list(struct config_str2list* cmp1, } } +/** fast reload thread, check if config str3list has changed. */ +#define FR_CHECK_CHANGED_CFG_STR3LIST(desc, var, buff) do { \ + fr_check_changed_cfg_str3list(cfg->var, newcfg->var, desc, buff,\ + sizeof(buff)); \ + } while(0); +static void +fr_check_changed_cfg_str3list(struct config_str3list* cmp1, + struct config_str3list* cmp2, const char* desc, char* str, size_t len) +{ + struct config_str3list* p1 = cmp1, *p2 = cmp2; + while(p1 && p2) { + if((!p1->str && p2->str) || + (p1->str && !p2->str) || + (p1->str && p2->str && strcmp(p1->str, p2->str) != 0)) { + /* The str3list is different. */ + fr_add_incompatible_option(desc, str, len); + return; + } + if((!p1->str2 && p2->str2) || + (p1->str2 && !p2->str2) || + (p1->str2 && p2->str2 && + strcmp(p1->str2, p2->str2) != 0)) { + /* The str3list is different. */ + fr_add_incompatible_option(desc, str, len); + return; + } + if((!p1->str3 && p2->str3) || + (p1->str3 && !p2->str3) || + (p1->str3 && p2->str3 && + strcmp(p1->str3, p2->str3) != 0)) { + /* The str3list is different. */ + fr_add_incompatible_option(desc, str, len); + return; + } + p1 = p1->next; + p2 = p2->next; + } + if((!p1 && p2) || (p1 && !p2)) { + fr_add_incompatible_option(desc, str, len); + } +} + +/** fast reload thread, check tag datas. */ +static int +fr_check_tag_datas(struct fast_reload_thread* fr, struct config_file* newcfg) +{ + char changed_str[1024]; + struct config_file* cfg = fr->worker->env.cfg; + changed_str[0]=0; + + /* Check for tag_datas in acl_addr. */ + FR_CHECK_CHANGED_CFG_STR3LIST("interface-tag-data", interface_tag_datas, changed_str); + FR_CHECK_CHANGED_CFG_STR3LIST("access-control-tag-data", acl_tag_datas, changed_str); + + if(changed_str[0] != 0) { + if(fr->fr_drop_mesh) + return 1; /* already dropping queries */ + fr->fr_drop_mesh = 1; + fr->worker->daemon->fast_reload_drop_mesh = fr->fr_drop_mesh; + if(!fr_output_printf(fr, "recursion referenced data has changed, with: '%s" + "', and the queries have to be dropped" + ", setting '+d'\n", changed_str)) + return 0; + fr_send_notification(fr, fast_reload_notification_printout); + } + return 1; +} + /** fast reload thread, check compatible config items */ static int fr_check_compat_cfg(struct fast_reload_thread* fr, struct config_file* newcfg) @@ -6911,6 +6979,10 @@ fr_load_config(struct fast_reload_thread* fr, struct timeval* time_read, config_delete(newcfg); return 0; } + if(!fr_check_tag_datas(fr, newcfg)) { + config_delete(newcfg); + return 0; + } if(!fr_check_compat_cfg(fr, newcfg)) { config_delete(newcfg); return 0; diff --git a/doc/Changelog b/doc/Changelog index fee4ed411..a40ea4aa3 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -12,6 +12,9 @@ not considered valid when an NSEC next owner name is not under the signer zone name. Thanks to Qifan Zhang, Palo Alto Networks, for the report. + - Fix mesh cycle detection for configuration with respip CNAME + loop and tagged clients. Thanks to Qifan Zhang, Palo Alto + Networks, for the report. 22 July 2026: Wouter - Release tag for 1.25.2, with the security commits: diff --git a/respip/respip.c b/respip/respip.c index 6ade552db..fc9de4c01 100644 --- a/respip/respip.c +++ b/respip/respip.c @@ -1164,8 +1164,10 @@ respip_operate(struct module_qstate* qstate, enum module_ev event, int id, * clients. */ qstate->is_drop = 1; } else if(alias_rrset) { - if(!generate_cname_request(qstate, alias_rrset)) + if(!generate_cname_request(qstate, alias_rrset)) { + errinf(qstate, "Could not generate CNAME request"); goto servfail; + } next_state = module_wait_subquery; } qstate->return_msg->rep = new_rep; @@ -1179,6 +1181,7 @@ respip_operate(struct module_qstate* qstate, enum module_ev event, int id, servfail: qstate->return_rcode = LDNS_RCODE_SERVFAIL; qstate->return_msg = NULL; + qstate->ext_state[id] = module_finished; } int @@ -1275,6 +1278,7 @@ respip_inform_super(struct module_qstate* qstate, int id, return; fail: + errinf(super, "CNAME lookup failed"); super->return_rcode = LDNS_RCODE_SERVFAIL; super->return_msg = NULL; return; diff --git a/services/mesh.c b/services/mesh.c index 6159dd1cd..add773b88 100644 --- a/services/mesh.c +++ b/services/mesh.c @@ -965,32 +965,9 @@ void mesh_report_reply(struct mesh_area* mesh, struct outbound_entry* e, mesh_run(mesh, e->qstate->mesh_info, event, e); } -/** copy strlist to region */ -static struct config_strlist* -cfg_region_strlist_copy(struct regional* region, struct config_strlist* list) -{ - struct config_strlist* result = NULL, *last = NULL, *s = list; - while(s) { - struct config_strlist* n = regional_alloc_zero(region, - sizeof(*n)); - if(!n) - return NULL; - n->str = regional_strdup(region, s->str); - if(!n->str) - return NULL; - if(last) - last->next = n; - else result = n; - last = n; - s = s->next; - } - return result; -} - struct respip_client_info* mesh_copy_client_info(struct regional* region, struct respip_client_info* cinfo) { - size_t i; struct respip_client_info* client_info; client_info = regional_alloc_init(region, cinfo, sizeof(*cinfo)); if(!client_info) @@ -1009,20 +986,13 @@ mesh_copy_client_info(struct regional* region, struct respip_client_info* cinfo) if(!client_info->tag_actions) return NULL; } - if(cinfo->tag_datas) { - client_info->tag_datas = regional_alloc_zero(region, - sizeof(struct config_strlist*)*cinfo->tag_datas_size); - if(!client_info->tag_datas) - return NULL; - for(i=0; itag_datas_size; i++) { - if(cinfo->tag_datas[i]) { - client_info->tag_datas[i] = cfg_region_strlist_copy( - region, cinfo->tag_datas[i]); - if(!client_info->tag_datas[i]) - return NULL; - } - } - } + /* tag_datas is owned by the matched acl_addr in config_file; its + * lifetime is until config reload, which tears down all mesh states + * first. Keep the original pointer so client_info_compare() + * can recognise two states from the same ACL entry. */ + /* fast reload insists on dropping the queries when interface-tag-data + * or access-control-tag-data are changed. */ + /* client_info->tag_datas already copied by regional_alloc_init above */ if(cinfo->view) { /* Do not copy the view pointer but store a name instead. * The name is looked up later when done, this means that @@ -2306,8 +2276,29 @@ void mesh_run(struct mesh_area* mesh, struct mesh_state* mstate, enum module_ev ev, struct outbound_entry* e) { enum module_ext_state s; + int numrun = 0; verbose(VERB_ALGO, "mesh_run: start"); while(mstate) { + if(numrun++ > MESH_MAX_RUN_ITER) { + /* These modules are too much to activate, stop them.*/ + log_err("Too many module run iterations, deleting"); + while(mstate) { + /* notify supers */ + if(mstate->super_set.count > 0) { + verbose(VERB_ALGO, "notify supers of failure"); + mstate->s.return_msg = NULL; + mstate->s.return_rcode = LDNS_RCODE_SERVFAIL; + mesh_walk_supers(mesh, mstate); + } + mesh_state_delete(&mstate->s); + if(mesh->run.count > 0) { + /* pop random element off the runnable tree */ + mstate = (struct mesh_state*)mesh->run.root->key; + (void)rbtree_delete(&mesh->run, mstate); + } else mstate = NULL; + } + break; + } /* run the module */ fptr_ok(fptr_whitelist_mod_operate( mesh->mods.mod[mstate->s.curmod]->operate)); diff --git a/services/mesh.h b/services/mesh.h index 0f1d91c00..9e3da53a7 100644 --- a/services/mesh.h +++ b/services/mesh.h @@ -69,6 +69,13 @@ struct respip_client_info; */ #define MESH_MAX_ACTIVATION 10000 +/** + * Maximum number of mesh state run items. These are different modules + * activated during a mesh run. Any more is likely an infinite loop + * in the module. It is then terminated, and states are deleted. + */ +#define MESH_MAX_RUN_ITER 10000 + /** * Max number of references-to-references-to-references.. search size. * Any more is treated like 'too large', and the creation of a new diff --git a/testdata/respip_cname_loop_tagged.rpl b/testdata/respip_cname_loop_tagged.rpl new file mode 100644 index 000000000..a97ae77ad --- /dev/null +++ b/testdata/respip_cname_loop_tagged.rpl @@ -0,0 +1,254 @@ +; config options +; The island of trust is at test. +server: + target-fetch-policy: "0 0 0 0 0" + qname-minimisation: "no" + fake-sha1: yes + trust-anchor-signaling: no + minimal-responses: no + iter-scrub-promiscuous: no + aggressive-nsec: no + local-zone: test. nodefault + log-servfail: yes + discard-timeout: 0 + module-config: "respip iterator" + define-tag: "turqoise" + access-control-tag: 127.0.0.0/8 "turqoise" + access-control-tag-data: 127.0.0.0/8 "turqoise" "A 127.0.0.1" + + ; These two CNAMEs form a loop. + response-ip: 192.0.2.1/32 redirect + response-ip-data: 192.0.2.1/32 "CNAME loop2.far.test." + response-ip: 192.0.2.2/32 redirect + response-ip-data: 192.0.2.2/32 "CNAME loop1.far.test." + +stub-zone: + name: "." + stub-addr: 193.0.14.129 # K.ROOT-SERVERS.NET. +CONFIG_END + +SCENARIO_BEGIN Test respip CNAME loop that is tagged. + +; K.ROOT-SERVERS.NET. +RANGE_BEGIN 0 100 + ADDRESS 193.0.14.129 +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +. IN NS +SECTION ANSWER +. IN NS K.ROOT-SERVERS.NET. +SECTION ADDITIONAL +K.ROOT-SERVERS.NET. IN A 193.0.14.129 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode subdomain +ADJUST copy_id copy_query +REPLY QR NOERROR +SECTION QUESTION +test. IN NS +SECTION AUTHORITY +test. IN NS ns.test. +SECTION ADDITIONAL +ns.test. IN A 1.2.3.5 +ENTRY_END +RANGE_END + +; ns.test +RANGE_BEGIN 0 100 + ADDRESS 1.2.3.5 +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +test. IN NS +SECTION ANSWER +test. IN NS ns.test +SECTION ADDITIONAL +ns.test. IN A 1.2.3.5 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +ns.test. IN A +SECTION ANSWER +ns.test. IN A 1.2.3.5 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +ns.test. IN AAAA +SECTION AUTHORITY +test. 3600 IN SOA ns.test. host.test. 20201 3600 1800 604800 3600 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode subdomain +ADJUST copy_id copy_query +REPLY QR NOERROR +SECTION QUESTION +example.test. IN NS +SECTION AUTHORITY +example.test. IN NS ns.example.test. +SECTION ADDITIONAL +ns.example.test. IN A 1.2.3.4 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode subdomain +ADJUST copy_id copy_query +REPLY QR NOERROR +SECTION QUESTION +far.test. IN NS +SECTION AUTHORITY +far.test. IN NS ns.far.test. +SECTION ADDITIONAL +ns.far.test. IN A 1.2.3.6 +ENTRY_END +RANGE_END + +; ns.example.test. +RANGE_BEGIN 0 20 + ADDRESS 1.2.3.4 +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +example.test. IN NS +SECTION ANSWER +example.test. IN NS ns.example.test. +SECTION ADDITIONAL +ns.example.test. IN A 1.2.3.4 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +ns.example.test. IN A +SECTION ANSWER +ns.example.test. IN A 1.2.3.4 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +ns.example.test. IN AAAA +SECTION AUTHORITY +example.test. 3600 IN SOA ns.example.test. host.example.test. 20301 3600 1800 604800 3600 +ENTRY_END + +; response to query of interest +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +www.example.test. IN A +SECTION ANSWER +www.example.test. 1 IN A 192.0.2.1 +ENTRY_END +RANGE_END + +; ns.example.test. +RANGE_BEGIN 45 100 + ADDRESS 1.2.3.4 +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +www.example.test. IN A +SECTION ANSWER +www.example.test. 1 IN A 192.0.2.1 +ENTRY_END +RANGE_END + +; ns.far.test. +RANGE_BEGIN 0 100 + ADDRESS 1.2.3.6 +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +far.test. IN NS +SECTION ANSWER +far.test. IN NS ns.far.test. +SECTION ADDITIONAL +ns.far.test. IN A 1.2.3.6 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +ns.far.test. IN A +SECTION ANSWER +ns.far.test. IN A 1.2.3.6 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +ns.far.test. IN AAAA +SECTION AUTHORITY +far.test. 3600 IN SOA ns.far.test. host.far.test. 20301 3600 1800 604800 3600 +ENTRY_END + +; response to query of interest +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +loop1.far.test. IN A +SECTION ANSWER +loop1.far.test. IN A 192.0.2.1 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +loop2.far.test. IN A +SECTION ANSWER +loop2.far.test. IN A 192.0.2.2 +ENTRY_END +RANGE_END + +STEP 1 QUERY +ENTRY_BEGIN +REPLY RD DO +SECTION QUESTION +loop1.far.test. IN A +ENTRY_END + +STEP 2 CHECK_ANSWER +ENTRY_BEGIN +MATCH all +REPLY QR RD RA DO SERVFAIL +SECTION QUESTION +loop1.far.test. IN A +SECTION ANSWER +ENTRY_END + +SCENARIO_END