From 475e790e0365fcd04fc1209abf1a648a59333d69 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Tue, 3 May 2022 22:24:32 +0000 Subject: [PATCH 1/5] Check that catz member zone is not a configured forward zone When processing a catalog zone member zone make sure that there is no configured pre-existing forward zone with that name. Refactor the `dns_fwdtable_find()` function to not alter the `DNS_R_PARTIALMATCH` result (coming from `dns_rbt_findname()`) into `DNS_R_SUCCESS`, so that now the caller can differentiate partial and exact matches. Patch the calling sites to expect and process the new return value. (cherry picked from commit 2aff264fb121e86df4063cd151b83ad0fa1f6aa2) --- bin/named/server.c | 37 ++++++++++++++++++++++++++--------- lib/dns/forward.c | 5 ----- lib/dns/include/dns/forward.h | 6 ++++-- lib/dns/resolver.c | 8 ++++---- 4 files changed, 36 insertions(+), 20 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 5c48da77b0..95a38b13dd 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -2655,6 +2655,8 @@ static void catz_addmodzone_taskaction(isc_task_t *task, isc_event_t *event0) { catz_chgzone_event_t *ev = (catz_chgzone_event_t *)event0; isc_result_t result; + dns_forwarders_t *dnsforwarders = NULL; + dns_name_t *name = NULL; isc_buffer_t namebuf; isc_buffer_t *confbuf; char nameb[DNS_NAME_FORMATSIZE]; @@ -2673,12 +2675,26 @@ catz_addmodzone_taskaction(isc_task_t *task, isc_event_t *event0) { goto cleanup; } + name = dns_catz_entry_getname(ev->entry); + isc_buffer_init(&namebuf, nameb, DNS_NAME_FORMATSIZE); - dns_name_totext(dns_catz_entry_getname(ev->entry), true, &namebuf); + dns_name_totext(name, true, &namebuf); isc_buffer_putuint8(&namebuf, 0); - result = dns_zt_find(ev->view->zonetable, - dns_catz_entry_getname(ev->entry), 0, NULL, &zone); + result = dns_fwdtable_find(ev->view->fwdtable, name, NULL, + &dnsforwarders); + if (result == ISC_R_SUCCESS && + dnsforwarders->fwdpolicy == dns_fwdpolicy_only) { + isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, + NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, + "catz: catz_addmodzone_taskaction: " + "zone '%s' will not be processed because of the " + "explicitly configured forwarding for that zone", + nameb); + goto cleanup; + } + + result = dns_zt_find(ev->view->zonetable, name, 0, NULL, &zone); if (ev->mod) { dns_catz_zone_t *parentcatz; @@ -2815,8 +2831,7 @@ catz_addmodzone_taskaction(isc_task_t *task, isc_event_t *event0) { } /* Is it there yet? */ - CHECK(dns_zt_find(ev->view->zonetable, - dns_catz_entry_getname(ev->entry), 0, NULL, &zone)); + CHECK(dns_zt_find(ev->view->zonetable, name, 0, NULL, &zone)); /* * Load the zone from the master file. If this fails, we'll @@ -5801,8 +5816,10 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, */ result = dns_fwdtable_find(view->fwdtable, name, NULL, &dnsforwarders); - if (result == ISC_R_SUCCESS && - dnsforwarders->fwdpolicy == dns_fwdpolicy_only) { + if ((result == ISC_R_SUCCESS || + result == DNS_R_PARTIALMATCH) && + dnsforwarders->fwdpolicy == dns_fwdpolicy_only) + { continue; } @@ -5887,8 +5904,10 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, */ result = dns_fwdtable_find(view->fwdtable, name, NULL, &dnsforwarders); - if (result == ISC_R_SUCCESS && - dnsforwarders->fwdpolicy == dns_fwdpolicy_only) { + if ((result == ISC_R_SUCCESS || + result == DNS_R_PARTIALMATCH) && + dnsforwarders->fwdpolicy == dns_fwdpolicy_only) + { continue; } diff --git a/lib/dns/forward.c b/lib/dns/forward.c index 72cb20e3d2..cd4959a76d 100644 --- a/lib/dns/forward.c +++ b/lib/dns/forward.c @@ -176,13 +176,8 @@ dns_fwdtable_find(dns_fwdtable_t *fwdtable, const dns_name_t *name, REQUIRE(VALID_FWDTABLE(fwdtable)); RWLOCK(&fwdtable->rwlock, isc_rwlocktype_read); - result = dns_rbt_findname(fwdtable->table, name, 0, foundname, (void **)forwardersp); - if (result == DNS_R_PARTIALMATCH) { - result = ISC_R_SUCCESS; - } - RWUNLOCK(&fwdtable->rwlock, isc_rwlocktype_read); return (result); diff --git a/lib/dns/include/dns/forward.h b/lib/dns/include/dns/forward.h index a6000ae044..b78d400f18 100644 --- a/lib/dns/include/dns/forward.h +++ b/lib/dns/include/dns/forward.h @@ -102,8 +102,10 @@ dns_fwdtable_find(dns_fwdtable_t *fwdtable, const dns_name_t *name, * \li foundname to be NULL or a valid name with buffer. * * Returns: - * \li #ISC_R_SUCCESS - * \li #ISC_R_NOTFOUND + * \li #ISC_R_SUCCESS Success + * \li #DNS_R_PARTIALMATCH Superdomain found with data + * \li #ISC_R_NOTFOUND No match + * \li #ISC_R_NOSPACE Concatenating nodes to form foundname failed */ void diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index b5cdcc8879..52063d16b2 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -3591,7 +3591,7 @@ fctx_getaddresses(fetchctx_t *fctx, bool badcache) { domain = dns_fixedname_initname(&fixed); result = dns_fwdtable_find(res->view->fwdtable, name, domain, &forwarders); - if (result == ISC_R_SUCCESS) { + if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { fwd = ISC_LIST_HEAD(forwarders->fwdrs); fctx->fwdpolicy = forwarders->fwdpolicy; dns_name_copy(domain, fctx->fwdname); @@ -4804,7 +4804,7 @@ fctx_create(dns_resolver_t *res, isc_task_t *task, const dns_name_t *name, /* Find the forwarder for this name. */ result = dns_fwdtable_find(fctx->res->view->fwdtable, fwdname, fname, &forwarders); - if (result == ISC_R_SUCCESS) { + if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { fctx->fwdpolicy = forwarders->fwdpolicy; dns_name_copy(fname, fctx->fwdname); } @@ -6898,7 +6898,7 @@ name_external(const dns_name_t *name, dns_rdatatype_t type, fetchctx_t *fctx) { /* * See if the forwarder declaration is better. */ - if (result == ISC_R_SUCCESS) { + if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { return (!dns_name_equal(fname, fctx->fwdname)); } @@ -6907,7 +6907,7 @@ name_external(const dns_name_t *name, dns_rdatatype_t type, fetchctx_t *fctx) { * changed: play it safe and don't cache. */ return (true); - } else if (result == ISC_R_SUCCESS && + } else if ((result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) && forwarders->fwdpolicy == dns_fwdpolicy_only && !ISC_LIST_EMPTY(forwarders->fwdrs)) { From 1626f61815c65ed6f6a742f9b35fd7beb602fa23 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Tue, 3 May 2022 22:28:45 +0000 Subject: [PATCH 2/5] Convert some catz error messages from ISC_LOG_INFO to ISC_LOG_WARNING There is no reason for these two messages to be `ISC_LOG_INFO` while all the other similar messages in `catz_addmodzone_taskaction()` and `catz_delzone_taskaction()` functions are logged as `ISC_LOG_WARNING`. (cherry picked from commit 8156c46bd20324625e8c2455c71f073e23965f79) --- bin/named/server.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 95a38b13dd..842277d46a 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -2746,7 +2746,7 @@ catz_addmodzone_taskaction(isc_task_t *task, isc_event_t *event0) { if (dns_zone_get_parentcatz(zone) == NULL) { isc_log_write( named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, + NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, "catz: " "catz_addmodzone_taskaction: " "zone '%s' will not be added " @@ -2756,7 +2756,7 @@ catz_addmodzone_taskaction(isc_task_t *task, isc_event_t *event0) { } else { isc_log_write( named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, + NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, "catz: " "catz_addmodzone_taskaction: " "zone '%s' will not be added " From 56cc6545dc24ba726c8ddac7e7ea0a11857e6429 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Tue, 3 May 2022 22:34:48 +0000 Subject: [PATCH 3/5] Add forward zone checks in the catz system test Add a new test to check that a catalog zone member zone does not get processed when there is a pre-existing forward zone with that same name. (cherry picked from commit b27969ee0ba86adc87550e7a31b9ed84dbac6fa7) --- bin/tests/system/catz/ns2/named1.conf.in | 12 +++ bin/tests/system/catz/ns2/named2.conf.in | 12 +++ bin/tests/system/catz/tests.sh | 93 +++++++++++++++++++++++- 3 files changed, 116 insertions(+), 1 deletion(-) diff --git a/bin/tests/system/catz/ns2/named1.conf.in b/bin/tests/system/catz/ns2/named1.conf.in index c52205aab3..a587b383af 100644 --- a/bin/tests/system/catz/ns2/named1.conf.in +++ b/bin/tests/system/catz/ns2/named1.conf.in @@ -80,6 +80,18 @@ view "default" { file "dom-existing.example.db"; }; + zone "dom-existing-forward.example" { + type forward; + forward only; + forwarders { 10.53.0.1; }; + }; + + zone "dom-existing-forward-off.example" { + type forward; + forward only; + forwarders { }; + }; + zone "catalog1.example" { type secondary; file "catalog1.example.db"; diff --git a/bin/tests/system/catz/ns2/named2.conf.in b/bin/tests/system/catz/ns2/named2.conf.in index 89c15c71af..62b76a600e 100644 --- a/bin/tests/system/catz/ns2/named2.conf.in +++ b/bin/tests/system/catz/ns2/named2.conf.in @@ -40,6 +40,18 @@ view "default" { file "dom-existing.example.db"; }; + zone "dom-existing-forward.example" { + type forward; + forward only; + forwarders { 10.53.0.1; }; + }; + + zone "dom-existing-forward-off.example" { + type forward; + forward only; + forwarders { }; + }; + zone "catalog1.example" { type secondary; file "catalog1.example.db"; diff --git a/bin/tests/system/catz/tests.sh b/bin/tests/system/catz/tests.sh index f305acff01..c6b1eb7746 100644 --- a/bin/tests/system/catz/tests.sh +++ b/bin/tests/system/catz/tests.sh @@ -713,7 +713,7 @@ n=$((n+1)) echo_i "waiting for secondary to sync up ($n)" ret=0 wait_for_message ns2/named.run "catz: adding zone 'dom-existing.example' from catalog 'catalog1.example'" && -wait_for_message ns2/named.run "catz_addmodzone_taskaction: zone 'dom-existing.example' will not be added because it is an explicitly configured zone" || ret=1 +wait_for_message ns2/named.run "catz_addmodzone_taskaction: zone 'dom-existing.example' will not be added because it is an explicitly configured zone" || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) @@ -725,6 +725,95 @@ grep "192.0.2.1" dig.out.test$n > /dev/null && ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) +nextpart ns2/named.run >/dev/null + +n=$((n+1)) +echo_i "adding a domain dom-existing-forward.example. to primary via RNDC ($n)" +ret=0 +echo "@ 3600 IN SOA . . 1 3600 3600 3600 3600" > ns1/dom-existing-forward.example.db +echo "@ IN NS invalid." >> ns1/dom-existing-forward.example.db +echo "@ IN A 192.0.2.1" >> ns1/dom-existing-forward.example.db +rndccmd 10.53.0.1 addzone dom-existing-forward.example. in default '{type primary; file "dom-existing-forward.example.db"; also-notify { 10.53.0.2; }; notify explicit; };' || ret=1 +if [ $ret -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +n=$((n+1)) +echo_i "checking that dom-existing-forward.example. is served by primary ($n)" +ret=0 +wait_for_a @10.53.0.1 dom-existing-forward.example. dig.out.test$n || ret=1 +if [ $ret -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +n=$((n+1)) +echo_i "adding domain dom-existing-forward.example. to catalog1 zone to test that existing forward zones don't get overwritten ($n)" +ret=0 +$NSUPDATE -d <> nsupdate.out.test$n 2>&1 || ret=1 + server 10.53.0.1 ${PORT} + update add dom-existing-forward.zones.catalog1.example. 3600 IN PTR dom-existing-forward.example. + send +END +if [ $ret -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +n=$((n+1)) +echo_i "waiting for secondary to sync up ($n)" +ret=0 +wait_for_message ns2/named.run "catz: adding zone 'dom-existing-forward.example' from catalog 'catalog1.example'" && +wait_for_message ns2/named.run "catz_addmodzone_taskaction: zone 'dom-existing-forward.example' will not be processed because of the explicitly configured forwarding for that zone" || ret=1 +if [ $ret -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +n=$((n+1)) +echo_i "checking that dom-existing-forward.example. is not served by secondary ($n)" +ret=0 +wait_for_no_soa @10.53.0.2 dom-existing-forward.example. dig.out.test$n || ret=1 +if [ $ret -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +nextpart ns2/named.run >/dev/null + +n=$((n+1)) +echo_i "adding a domain dom-existing-forward-off.example. to primary via RNDC ($n)" +ret=0 +echo "@ 3600 IN SOA . . 1 3600 3600 3600 3600" > ns1/dom-existing-forward-off.example.db +echo "@ IN NS invalid." >> ns1/dom-existing-forward-off.example.db +echo "@ IN A 192.0.2.1" >> ns1/dom-existing-forward-off.example.db +rndccmd 10.53.0.1 addzone dom-existing-forward-off.example. in default '{type primary; file "dom-existing-forward-off.example.db"; also-notify { 10.53.0.2; }; notify explicit; };' || ret=1 +if [ $ret -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +n=$((n+1)) +echo_i "checking that dom-existing-forward-off.example. is served by primary ($n)" +ret=0 +wait_for_a @10.53.0.1 dom-existing-forward-off.example. dig.out.test$n || ret=1 +if [ $ret -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +n=$((n+1)) +echo_i "adding domain dom-existing-forward-off.example. to catalog1 zone to test that a zone with turned off forwarding can be used in a catalog zone ($n)" +ret=0 +$NSUPDATE -d <> nsupdate.out.test$n 2>&1 || ret=1 + server 10.53.0.1 ${PORT} + update add dom-existing-forward-off.zones.catalog1.example. 3600 IN PTR dom-existing-forward-off.example. + send +END +if [ $ret -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +n=$((n+1)) +echo_i "waiting for secondary to sync up ($n)" +ret=0 +wait_for_message ns2/named.run "catz: adding zone 'dom-existing-forward-off.example' from catalog 'catalog1.example'" && +if [ $ret -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +n=$((n+1)) +echo_i "checking that dom-existing-forward-off.example. is served by secondary ($n)" +ret=0 +wait_for_soa @10.53.0.2 dom-existing-forward-off.example. dig.out.test$n || ret=1 +if [ $ret -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + n=$((n+1)) echo_i "removing all records from catalog1 zone ($n)" ret=0 @@ -743,6 +832,8 @@ $NSUPDATE -d <> nsupdate.out.test$n 2>&1 || ret=1 update delete blahblah.636722929740e507aaf27c502812fc395d30fb17.zones.catalog1.example. 3600 IN TXT "blah blah" update delete version.catalog1.example. 3600 IN A 1.2.3.4 update delete dom-existing.zones.catalog1.example. 3600 IN PTR dom-existing.example. + update delete dom-existing-forward.zones.catalog1.example. 3600 IN PTR dom-existing-forward.example. + update delete dom-existing-forward-off.zones.catalog1.example. 3600 IN PTR dom-existing-forward.example. send END if [ $ret -ne 0 ]; then echo_i "failed"; fi From b870d5297066b0ba416e8dd5e2725db2e46c8910 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Tue, 3 May 2022 22:38:44 +0000 Subject: [PATCH 4/5] Add CHANGES and release note for [GL #2506] (cherry picked from commit 3191eabbc7452b9a77b7d8648054fc4d94b654e2) --- CHANGES | 4 ++++ doc/notes/notes-current.rst | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index 8e279f5d93..961f3c35ff 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5901. [bug] When processing a catalog zone member zone make sure + that there is no configured pre-existing forward-only + forward zone with that name. [GL #2506] + 5899. [func] Don't try to process DNSSEC-related and ZONEMD records in catz. [GL #3380] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index c8b5dcb8e4..a85f1b3ab0 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -40,4 +40,6 @@ Feature Changes Bug Fixes ~~~~~~~~~ -- None. +- It was possible for a catalog zone consumer to process a catalog zone member + zone when there was a configured pre-existing forward-only forward zone with + the same name. This has been fixed. :gl:`#2506`. From 8e8b44649f8f2d9e990e593230e40706b4076979 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Wed, 4 May 2022 09:43:49 +0000 Subject: [PATCH 5/5] Cleanup dns_fwdtable_delete() The conversion of `DNS_R_PARTIALMATCH` into `DNS_R_NOTFOUND` is done in the `dns_rbt_deletename()` function so there is no need to do that in `dns_fwdtable_delete()`. Add a possible return value of `ISC_R_NOSPACE` into the header file's function description comment. (cherry picked from commit 887aa7a2904d74021dfd9b7615f22625dee5d729) --- lib/dns/forward.c | 4 ---- lib/dns/include/dns/forward.h | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/dns/forward.c b/lib/dns/forward.c index cd4959a76d..9fe4d262ae 100644 --- a/lib/dns/forward.c +++ b/lib/dns/forward.c @@ -161,10 +161,6 @@ dns_fwdtable_delete(dns_fwdtable_t *fwdtable, const dns_name_t *name) { result = dns_rbt_deletename(fwdtable->table, name, false); RWUNLOCK(&fwdtable->rwlock, isc_rwlocktype_write); - if (result == DNS_R_PARTIALMATCH) { - result = ISC_R_NOTFOUND; - } - return (result); } diff --git a/lib/dns/include/dns/forward.h b/lib/dns/include/dns/forward.h index b78d400f18..a821826704 100644 --- a/lib/dns/include/dns/forward.h +++ b/lib/dns/include/dns/forward.h @@ -86,6 +86,7 @@ dns_fwdtable_delete(dns_fwdtable_t *fwdtable, const dns_name_t *name); * Returns: * \li #ISC_R_SUCCESS * \li #ISC_R_NOTFOUND + * \li #ISC_R_NOSPACE */ isc_result_t