From: W.C.A. Wijngaards Date: Mon, 15 Jun 2026 14:18:56 +0000 (+0200) Subject: - Fix for fast_reload that removes an auth zone while its X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2f8aa8a43a9a10570d473f482de3f29929e7d0c1;p=thirdparty%2Funbound.git - Fix for fast_reload that removes an auth zone while its lookups are in progress, for a primary name. Also after the change, it no longer picks up the old results. Thanks to Qifan Zhang, Palo Alto Networks, for the report. --- diff --git a/doc/Changelog b/doc/Changelog index e81c9aaf3..2311deedc 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -21,6 +21,10 @@ it in progress with subnet loaded, deregisters the callback. Thanks to Qifan Zhang, Palo Alto Networks, for the report. + - Fix for fast_reload that removes an auth zone while its + lookups are in progress, for a primary name. Also after the + change, it no longer picks up the old results. Thanks to + Qifan Zhang, Palo Alto Networks, for the report. 12 June 2026: Wouter - Fix that for auth-zone and rpz zones the allow-notify diff --git a/services/authzone.c b/services/authzone.c index f7224a3df..c342e08b5 100644 --- a/services/authzone.c +++ b/services/authzone.c @@ -5482,6 +5482,32 @@ xfr_process_chunk_list(struct auth_xfer* xfr, struct module_env* env, return 1; } +/** Stop lookup using callback */ +static void +xfr_stop_lookup(struct auth_master** lookup_target, void* lookup_unique_info, + int lookup_aaaa, uint16_t dclass, struct mesh_area* mesh, + mesh_cb_func_type cb, void* cb_arg) +{ + struct query_info qinfo; + uint8_t dname[LDNS_MAX_DOMAINLEN+1]; + if(!*lookup_target) return; + qinfo.qname_len = sizeof(dname); + if(sldns_str2wire_dname_buf((*lookup_target)->host, dname, + &qinfo.qname_len) != 0) { + *lookup_target = NULL; + return; + } + qinfo.qname = dname; + qinfo.qclass = dclass; + qinfo.qtype = lookup_aaaa ? LDNS_RR_TYPE_AAAA : LDNS_RR_TYPE_A; + qinfo.local_alias = NULL; + log_query_info(VERB_ALGO, "removing xfr callback", &qinfo); + + mesh_remove_callback(mesh, &qinfo, BIT_RD, cb, cb_arg, + lookup_unique_info); + *lookup_target = NULL; +} + /** disown task_transfer. caller must hold xfr.lock */ static void xfr_transfer_disown(struct auth_xfer* xfr) @@ -5492,6 +5518,12 @@ xfr_transfer_disown(struct auth_xfer* xfr) /* remove the commpoint */ comm_point_delete(xfr->task_transfer->cp); xfr->task_transfer->cp = NULL; + if(xfr->task_transfer->env) + xfr_stop_lookup(&xfr->task_transfer->lookup_target, + xfr->task_transfer->lookup_unique_info, + xfr->task_transfer->lookup_aaaa, xfr->dclass, + xfr->task_transfer->env->mesh, + &auth_xfer_transfer_lookup_callback, xfr); /* we don't own this item anymore */ xfr->task_transfer->worker = NULL; xfr->task_transfer->env = NULL; @@ -5787,6 +5819,26 @@ xfr_master_add_addrs(struct auth_master* m, struct ub_packed_rrset_key* rrset, } } +/** check if the lookup target name equals the found answer name. */ +static int +xfer_target_equals_answer_name(struct auth_master* lookup_target, + struct ub_packed_rrset_key* answer) +{ + uint8_t qname[LDNS_MAX_DOMAINLEN+1]; + size_t qname_len; + if(!lookup_target) return 0; + if(!answer) return 0; + qname_len = sizeof(qname); + if(sldns_str2wire_dname_buf(lookup_target->host, qname, &qname_len) + != 0) { + verbose(VERB_ALGO, "xfer_target_equals_answer_name: could not parse auth host name"); + return 0; + } + if(query_dname_compare(answer->rk.dname, qname) == 0) + return 1; + return 0; +} + /** callback for task_transfer lookup of host name, of A or AAAA */ void auth_xfer_transfer_lookup_callback(void* arg, int rcode, sldns_buffer* buf, enum sec_status ATTR_UNUSED(sec), char* ATTR_UNUSED(why_bogus), @@ -5817,21 +5869,28 @@ void auth_xfer_transfer_lookup_callback(void* arg, int rcode, sldns_buffer* buf, /* parsed successfully */ struct ub_packed_rrset_key* answer = reply_find_answer_rrset(&rq, rep); - if(answer) { + if(answer && xfer_target_equals_answer_name( + xfr->task_transfer->lookup_target, answer)) { xfr_master_add_addrs(xfr->task_transfer-> lookup_target, answer, wanted_qtype); + } else if(answer) { + if(verbosity >= VERB_ALGO) { + char zname[LDNS_MAX_DOMAINLEN]; + dname_str(xfr->name, zname); + verbose(VERB_ALGO, "auth zone %s host %s type %s transfer lookup has mismatch in answer name", zname, ((xfr->task_transfer->lookup_target && xfr->task_transfer->lookup_target->host) ? xfr->task_transfer->lookup_target->host : "null"), (xfr->task_transfer->lookup_aaaa?"AAAA":"A")); + } } else { if(verbosity >= VERB_ALGO) { char zname[LDNS_MAX_DOMAINLEN]; dname_str(xfr->name, zname); - verbose(VERB_ALGO, "auth zone %s host %s type %s transfer lookup has nodata", zname, xfr->task_transfer->lookup_target->host, (xfr->task_transfer->lookup_aaaa?"AAAA":"A")); + verbose(VERB_ALGO, "auth zone %s host %s type %s transfer lookup has nodata", zname, ((xfr->task_transfer->lookup_target && xfr->task_transfer->lookup_target->host) ? xfr->task_transfer->lookup_target->host : "null"), (xfr->task_transfer->lookup_aaaa?"AAAA":"A")); } } } else { if(verbosity >= VERB_ALGO) { char zname[LDNS_MAX_DOMAINLEN]; dname_str(xfr->name, zname); - verbose(VERB_ALGO, "auth zone %s host %s type %s transfer lookup has no answer", zname, xfr->task_transfer->lookup_target->host, (xfr->task_transfer->lookup_aaaa?"AAAA":"A")); + verbose(VERB_ALGO, "auth zone %s host %s type %s transfer lookup has no answer", zname, ((xfr->task_transfer->lookup_target && xfr->task_transfer->lookup_target->host) ? xfr->task_transfer->lookup_target->host : "null"), (xfr->task_transfer->lookup_aaaa?"AAAA":"A")); } } regional_free_all(temp); @@ -5839,10 +5898,11 @@ void auth_xfer_transfer_lookup_callback(void* arg, int rcode, sldns_buffer* buf, if(verbosity >= VERB_ALGO) { char zname[LDNS_MAX_DOMAINLEN]; dname_str(xfr->name, zname); - verbose(VERB_ALGO, "auth zone %s host %s type %s transfer lookup failed", zname, xfr->task_transfer->lookup_target->host, (xfr->task_transfer->lookup_aaaa?"AAAA":"A")); + verbose(VERB_ALGO, "auth zone %s host %s type %s transfer lookup failed", zname, ((xfr->task_transfer->lookup_target && xfr->task_transfer->lookup_target->host) ? xfr->task_transfer->lookup_target->host : "null"), (xfr->task_transfer->lookup_aaaa?"AAAA":"A")); } } - if(xfr->task_transfer->lookup_target->list && + if(xfr->task_transfer->lookup_target && + xfr->task_transfer->lookup_target->list && xfr->task_transfer->lookup_target == xfr_transfer_current_master(xfr)) xfr->task_transfer->scan_addr = xfr->task_transfer->lookup_target->list; @@ -6499,6 +6559,12 @@ xfr_probe_disown(struct auth_xfer* xfr) /* remove the commpoint */ comm_point_delete(xfr->task_probe->cp); xfr->task_probe->cp = NULL; + if(xfr->task_probe->env) + xfr_stop_lookup(&xfr->task_probe->lookup_target, + xfr->task_probe->lookup_unique_info, + xfr->task_probe->lookup_aaaa, xfr->dclass, + xfr->task_probe->env->mesh, + &auth_xfer_probe_lookup_callback, xfr); /* we don't own this item anymore */ xfr->task_probe->worker = NULL; xfr->task_probe->env = NULL; @@ -6953,21 +7019,28 @@ void auth_xfer_probe_lookup_callback(void* arg, int rcode, sldns_buffer* buf, /* parsed successfully */ struct ub_packed_rrset_key* answer = reply_find_answer_rrset(&rq, rep); - if(answer) { + if(answer && xfer_target_equals_answer_name( + xfr->task_probe->lookup_target, answer)) { xfr_master_add_addrs(xfr->task_probe-> lookup_target, answer, wanted_qtype); + } else if(answer) { + if(verbosity >= VERB_ALGO) { + char zname[LDNS_MAX_DOMAINLEN]; + dname_str(xfr->name, zname); + verbose(VERB_ALGO, "auth zone %s host %s type %s probe lookup has mismatch in answer name", zname, ((xfr->task_probe->lookup_target && xfr->task_probe->lookup_target->host) ? xfr->task_probe->lookup_target->host : "null"), (xfr->task_probe->lookup_aaaa?"AAAA":"A")); + } } else { if(verbosity >= VERB_ALGO) { char zname[LDNS_MAX_DOMAINLEN]; dname_str(xfr->name, zname); - verbose(VERB_ALGO, "auth zone %s host %s type %s probe lookup has nodata", zname, xfr->task_probe->lookup_target->host, (xfr->task_probe->lookup_aaaa?"AAAA":"A")); + verbose(VERB_ALGO, "auth zone %s host %s type %s probe lookup has nodata", zname, ((xfr->task_probe->lookup_target && xfr->task_probe->lookup_target->host) ? xfr->task_probe->lookup_target->host : "null"), (xfr->task_probe->lookup_aaaa?"AAAA":"A")); } } } else { if(verbosity >= VERB_ALGO) { char zname[LDNS_MAX_DOMAINLEN]; dname_str(xfr->name, zname); - verbose(VERB_ALGO, "auth zone %s host %s type %s probe lookup has no address", zname, xfr->task_probe->lookup_target->host, (xfr->task_probe->lookup_aaaa?"AAAA":"A")); + verbose(VERB_ALGO, "auth zone %s host %s type %s probe lookup has no address", zname, ((xfr->task_probe->lookup_target && xfr->task_probe->lookup_target->host) ? xfr->task_probe->lookup_target->host : "null"), (xfr->task_probe->lookup_aaaa?"AAAA":"A")); } } regional_free_all(temp); @@ -6975,10 +7048,11 @@ void auth_xfer_probe_lookup_callback(void* arg, int rcode, sldns_buffer* buf, if(verbosity >= VERB_ALGO) { char zname[LDNS_MAX_DOMAINLEN]; dname_str(xfr->name, zname); - verbose(VERB_ALGO, "auth zone %s host %s type %s probe lookup failed", zname, xfr->task_probe->lookup_target->host, (xfr->task_probe->lookup_aaaa?"AAAA":"A")); + verbose(VERB_ALGO, "auth zone %s host %s type %s probe lookup failed", zname, ((xfr->task_probe->lookup_target && xfr->task_probe->lookup_target->host) ? xfr->task_probe->lookup_target->host : "null"), (xfr->task_probe->lookup_aaaa?"AAAA":"A")); } } - if(xfr->task_probe->lookup_target->list && + if(xfr->task_probe->lookup_target && + xfr->task_probe->lookup_target->list && xfr->task_probe->lookup_target == xfr_probe_current_master(xfr)) xfr->task_probe->scan_addr = xfr->task_probe->lookup_target->list; diff --git a/testdata/fast_reload_authdel.tdir/example.com.zone b/testdata/fast_reload_authdel.tdir/example.com.zone new file mode 100644 index 000000000..695eb1c32 --- /dev/null +++ b/testdata/fast_reload_authdel.tdir/example.com.zone @@ -0,0 +1,3 @@ +example.com. IN SOA ns.example.com. hostmaster.example.com. 1 3600 900 86400 3600 +example.com. IN NS ns.example.net. +www.example.com. IN A 1.2.3.4 diff --git a/testdata/fast_reload_authdel.tdir/fast_reload_authdel.conf b/testdata/fast_reload_authdel.tdir/fast_reload_authdel.conf new file mode 100644 index 000000000..1820eaa99 --- /dev/null +++ b/testdata/fast_reload_authdel.tdir/fast_reload_authdel.conf @@ -0,0 +1,34 @@ +server: + verbosity: 7 + # num-threads: 1 + interface: 127.0.0.1 + port: @PORT@ + use-syslog: no + directory: "" + pidfile: "unbound.pid" + chroot: "" + username: "" + do-not-query-localhost: no + use-caps-for-id: no + +stub-zone: + name: "." + stub-addr: 127.0.0.1@@TOPORT@ + +remote-control: + control-enable: yes + control-interface: @CONTROL_PATH@/controlpipe.@CONTROL_PID@ + control-use-cert: no + +auth-zone: + name: "example.com" + for-upstream: yes + for-downstream: yes + allow-notify: notif.example.net + zonefile: "example.com.zone" + master: "127.0.0.1@@TOPORT@" +auth-zone: + name: "example2.com" + for-upstream: yes + for-downstream: yes + master: prim.example.net diff --git a/testdata/fast_reload_authdel.tdir/fast_reload_authdel.conf2 b/testdata/fast_reload_authdel.tdir/fast_reload_authdel.conf2 new file mode 100644 index 000000000..aefde13c2 --- /dev/null +++ b/testdata/fast_reload_authdel.tdir/fast_reload_authdel.conf2 @@ -0,0 +1,24 @@ +server: + verbosity: 7 + # num-threads: 1 + interface: 127.0.0.1 + port: @PORT@ + use-syslog: no + directory: "" + pidfile: "unbound.pid" + chroot: "" + username: "" + do-not-query-localhost: no + use-caps-for-id: no + +stub-zone: + name: "." + stub-addr: 127.0.0.1@@TOPORT@ + +remote-control: + control-enable: yes + control-interface: @CONTROL_PATH@/controlpipe.@CONTROL_PID@ + control-use-cert: no + +# auth zone removed for example.com +# auth zone removed for example2.com diff --git a/testdata/fast_reload_authdel.tdir/fast_reload_authdel.dsc b/testdata/fast_reload_authdel.tdir/fast_reload_authdel.dsc new file mode 100644 index 000000000..ed53225a8 --- /dev/null +++ b/testdata/fast_reload_authdel.tdir/fast_reload_authdel.dsc @@ -0,0 +1,16 @@ +BaseName: fast_reload_authdel +Version: 1.0 +Description: Test fast_reload with auth zone delete of callbacks. +CreationDate: Tue May 13 03:00:00 PM CEST 2026 +Maintainer: dr. W.C.A. Wijngaards +Category: +Component: +CmdDepends: +Depends: +Help: +Pre: fast_reload_authdel.pre +Post: fast_reload_authdel.post +Test: fast_reload_authdel.test +AuxFiles: +Passed: +Failure: diff --git a/testdata/fast_reload_authdel.tdir/fast_reload_authdel.post b/testdata/fast_reload_authdel.tdir/fast_reload_authdel.post new file mode 100644 index 000000000..46c34871f --- /dev/null +++ b/testdata/fast_reload_authdel.tdir/fast_reload_authdel.post @@ -0,0 +1,13 @@ +# #-- fast_reload_authdel.post --# +# source the master var file when it's there +[ -f ../.tpkg.var.master ] && source ../.tpkg.var.master +# source the test var file when it's there +[ -f .tpkg.var.test ] && source .tpkg.var.test +# +# do your teardown here +. ../common.sh +kill_pid $FWD_PID +kill_pid $UNBOUND_PID +echo "> cat logfiles" +cat fwd.log +cat unbound.log diff --git a/testdata/fast_reload_authdel.tdir/fast_reload_authdel.pre b/testdata/fast_reload_authdel.tdir/fast_reload_authdel.pre new file mode 100644 index 000000000..226f24d04 --- /dev/null +++ b/testdata/fast_reload_authdel.tdir/fast_reload_authdel.pre @@ -0,0 +1,46 @@ +# #-- fast_reload_authdel.pre--# +# source the master var file when it's there +[ -f ../.tpkg.var.master ] && source ../.tpkg.var.master +# use .tpkg.var.test for in test variable passing +[ -f .tpkg.var.test ] && source .tpkg.var.test + +PRE="../.." +. ../common.sh +# if no threads; exit +if grep -e "define HAVE_PTHREAD 1" -e "define HAVE_SOLARIS_THREADS 1" -e "define HAVE_WINDOWS_THREADS 1" $PRE/config.h; then + echo "have threads" +else + skip_test "no threads" +fi + +get_random_port 2 +UNBOUND_PORT=$RND_PORT +FWD_PORT=$(($RND_PORT + 1)) +echo "UNBOUND_PORT=$UNBOUND_PORT" >> .tpkg.var.test +echo "FWD_PORT=$FWD_PORT" >> .tpkg.var.test + +# start forwarder +get_ldns_testns +$LDNS_TESTNS -p $FWD_PORT fast_reload_authdel.testns >fwd.log 2>&1 & +FWD_PID=$! +echo "FWD_PID=$FWD_PID" >> .tpkg.var.test + +# make config files +CONTROL_PATH=/tmp +CONTROL_PID=$$ +sed -e 's/@PORT\@/'$UNBOUND_PORT'/' -e 's/@TOPORT\@/'$FWD_PORT'/' -e 's?@CONTROL_PATH\@?'$CONTROL_PATH'?' -e 's/@CONTROL_PID@/'$CONTROL_PID'/' < fast_reload_authdel.conf > ub.conf +sed -e 's/@PORT\@/'$UNBOUND_PORT'/' -e 's/@TOPORT\@/'$FWD_PORT'/' -e 's?@CONTROL_PATH\@?'$CONTROL_PATH'?' -e 's/@CONTROL_PID@/'$CONTROL_PID'/' < fast_reload_authdel.conf2 > ub.conf2 + +# make config file +# start unbound in the background +PRE="../.." +$PRE/unbound -d -c ub.conf >unbound.log 2>&1 & +UNBOUND_PID=$! +echo "UNBOUND_PID=$UNBOUND_PID" >> .tpkg.var.test +echo "CONTROL_PATH=$CONTROL_PATH" >> .tpkg.var.test +echo "CONTROL_PID=$CONTROL_PID" >> .tpkg.var.test + +cat .tpkg.var.test +wait_ldns_testns_up fwd.log +wait_unbound_up unbound.log + diff --git a/testdata/fast_reload_authdel.tdir/fast_reload_authdel.test b/testdata/fast_reload_authdel.tdir/fast_reload_authdel.test new file mode 100644 index 000000000..0a147c8ed --- /dev/null +++ b/testdata/fast_reload_authdel.tdir/fast_reload_authdel.test @@ -0,0 +1,47 @@ +# #-- fast_reload_authdel.test --# +# source the master var file when it's there +[ -f ../.tpkg.var.master ] && source ../.tpkg.var.master +# use .tpkg.var.test for in test variable passing +[ -f .tpkg.var.test ] && source .tpkg.var.test + +PRE="../.." +# do the test + +# the upstream delays. +# wait for the notify: probe of example to be in the middle of the callback, +# and for the primary: probe of example2 to be in the middle of the callback. +# And then fastreload to delete. + +echo "> wait for unbound to transfer" +sleep 1 + +mv ub.conf ub.conf.orig +mv ub.conf2 ub.conf +echo "" +echo "> unbound-control fast_reload" +$PRE/unbound-control -c ub.conf fast_reload +vv 2>&1 | tee output +if test $? -ne 0; then + echo "wrong exit value." + exit 1 +else + echo "exit value: OK" +fi + +sleep 1 + +echo "> check log" +if grep "removing xfr callback prim.example.net. A IN" unbound.log; then + echo "OK" +else + echo "Not OK" + exit 1 +fi + +if grep "removing xfr callback notif.example.net. A IN" unbound.log; then + echo "OK" +else + echo "Not OK" + exit 1 +fi + +exit 0 diff --git a/testdata/fast_reload_authdel.tdir/fast_reload_authdel.testns b/testdata/fast_reload_authdel.tdir/fast_reload_authdel.testns new file mode 100644 index 000000000..ba16798d1 --- /dev/null +++ b/testdata/fast_reload_authdel.tdir/fast_reload_authdel.testns @@ -0,0 +1,78 @@ +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +example.com. IN SOA +SECTION ANSWER +example.com. IN SOA ns.example.com. hostmaster.example.com. 1 3600 900 86400 3600 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +example.com. IN AXFR +SECTION ANSWER +example.com. IN SOA ns.example.com. hostmaster.example.com. 1 3600 900 86400 3600 +example.com. IN NS ns.example.net. +www.example.com. IN A 1.2.3.4 +example.com. IN SOA ns.example.com. hostmaster.example.com. 1 3600 900 86400 3600 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +example2.com. IN SOA +SECTION ANSWER +example2.com. IN SOA ns.example2.com. hostmaster.example2.com. 1 3600 900 86400 3600 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +example2.com. IN AXFR +SECTION ANSWER +example2.com. IN SOA ns.example2.com. hostmaster.example2.com. 1 3600 900 86400 3600 +example2.com. IN NS ns.example2.net. +EXTRA_PACKET +REPLY QR AA NOERROR +; too slow +ADJUST packet_sleep=5 +SECTION QUESTION +example2.com. IN AXFR +SECTION ANSWER +extra.example2.com. IN A 1.2.3.5 +EXTRA_PACKET +REPLY QR AA NOERROR +SECTION QUESTION +example2.com. IN AXFR +SECTION ANSWER +www.example2.com. IN A 1.2.3.4 +example2.com. IN SOA ns.example2.com. hostmaster.example2.com. 1 3600 900 86400 3600 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id sleep=5 +REPLY QR AA NOERROR +SECTION QUESTION +prim.example.net. IN A +SECTION ANSWER +prim.example.net. IN A 127.0.0.1 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id sleep=5 +REPLY QR AA NOERROR +SECTION QUESTION +notif.example.net. IN A +SECTION ANSWER +notif.example.net. IN A 127.0.0.1 +ENTRY_END