From e861224cf4ec2b7845348f92845825e8867c328f Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Tue, 8 Feb 2022 11:24:18 +0000 Subject: [PATCH 1/6] Fix invalid function name in the error log The current function's name in one of the error logs in catz_addmodzone_taskaction() function is invalid. Fix the name. --- bin/named/server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/named/server.c b/bin/named/server.c index b533cc07dc..f27d4c3eda 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -2704,7 +2704,7 @@ catz_addmodzone_taskaction(isc_task_t *task, isc_event_t *event0) { isc_log_write( named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, - "catz: catz_delzone_taskaction: " + "catz: catz_addmodzone_taskaction: " "zone '%s' exists in multiple " "catalog zones", nameb); From d29e5f197b630c202d49efa1a6f9bcab7a02ee48 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Tue, 8 Feb 2022 11:47:48 +0000 Subject: [PATCH 2/6] Log a warning when catz is told to modify a zone not added by catz Catz logs a warning message when it is told to modify a zone which was not added by the current catalog zone. When logging a warning, distinguish the two cases when the zone was not added by a catalog zone at all, and when the zone was added by a different catalog zone. --- bin/named/server.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/bin/named/server.c b/bin/named/server.c index f27d4c3eda..677efed0d3 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -2689,6 +2689,8 @@ catz_addmodzone_taskaction(isc_task_t *task, isc_event_t *event0) { isc_result_totext(result), nameb); goto cleanup; } else { + dns_catz_zone_t *parentcatz; + if (!dns_zone_getadded(zone)) { isc_log_write( named_g_lctx, NAMED_LOGCATEGORY_GENERAL, @@ -2700,7 +2702,20 @@ catz_addmodzone_taskaction(isc_task_t *task, isc_event_t *event0) { nameb); goto cleanup; } - if (dns_zone_get_parentcatz(zone) != ev->origin) { + + parentcatz = dns_zone_get_parentcatz(zone); + + if (parentcatz == NULL) { + isc_log_write( + named_g_lctx, NAMED_LOGCATEGORY_GENERAL, + NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, + "catz: catz_addmodzone_taskaction: " + "zone '%s' exists and is not added by " + "a catalog zone, so won't be modified", + nameb); + goto cleanup; + } + if (parentcatz != ev->origin) { isc_log_write( named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, @@ -2710,6 +2725,7 @@ catz_addmodzone_taskaction(isc_task_t *task, isc_event_t *event0) { nameb); goto cleanup; } + dns_zone_detach(&zone); } } else { From 9b84bfb5f403945402194deca5a61016d6d02a4a Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Tue, 8 Feb 2022 12:01:51 +0000 Subject: [PATCH 3/6] Cleanup the code to remove unnecessary indentation Because of the "goto" in the "if" body the "else" part is unnecessary and adds another level of indentation. Cleanup the code to not have the "else" part. --- bin/named/server.c | 74 ++++++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 39 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 677efed0d3..d6f74d3789 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -2681,6 +2681,8 @@ catz_addmodzone_taskaction(isc_task_t *task, isc_event_t *event0) { dns_catz_entry_getname(ev->entry), 0, NULL, &zone); if (ev->mod) { + dns_catz_zone_t *parentcatz; + if (result != ISC_R_SUCCESS) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, @@ -2688,46 +2690,40 @@ catz_addmodzone_taskaction(isc_task_t *task, isc_event_t *event0) { "modify zone \"%s\"", isc_result_totext(result), nameb); goto cleanup; - } else { - dns_catz_zone_t *parentcatz; - - if (!dns_zone_getadded(zone)) { - isc_log_write( - named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, - "catz: " - "catz_addmodzone_taskaction: " - "zone '%s' is not a dynamically " - "added zone", - nameb); - goto cleanup; - } - - parentcatz = dns_zone_get_parentcatz(zone); - - if (parentcatz == NULL) { - isc_log_write( - named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, - "catz: catz_addmodzone_taskaction: " - "zone '%s' exists and is not added by " - "a catalog zone, so won't be modified", - nameb); - goto cleanup; - } - if (parentcatz != ev->origin) { - isc_log_write( - named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, - "catz: catz_addmodzone_taskaction: " - "zone '%s' exists in multiple " - "catalog zones", - nameb); - goto cleanup; - } - - dns_zone_detach(&zone); } + + if (!dns_zone_getadded(zone)) { + isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, + NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, + "catz: catz_addmodzone_taskaction: " + "zone '%s' is not a dynamically " + "added zone", + nameb); + goto cleanup; + } + + parentcatz = dns_zone_get_parentcatz(zone); + + if (parentcatz == NULL) { + isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, + NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, + "catz: catz_addmodzone_taskaction: " + "zone '%s' exists and is not added by " + "a catalog zone, so won't be modified", + nameb); + goto cleanup; + } + if (parentcatz != ev->origin) { + isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, + NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, + "catz: catz_addmodzone_taskaction: " + "zone '%s' exists in multiple " + "catalog zones", + nameb); + goto cleanup; + } + + dns_zone_detach(&zone); } else { if (result == ISC_R_SUCCESS) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, From f57c51fe0572ade29133232d5d56a00146ba579c Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Thu, 17 Mar 2022 14:09:21 +0000 Subject: [PATCH 4/6] Put some missing dns_rdata_freestruct() calls in catz.c A successful call to `dns_rdata_tostruct()` expects an accompanying call to `dns_rdata_freestruct()` to free up any memory that could have been allocated during the first call. In catz.c there are several places where `dns_rdata_freestruct()` call is skipped. Add the missing cleanup routines. --- lib/dns/catz.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/dns/catz.c b/lib/dns/catz.c index 6560f5c8df..ca71a5d36a 100644 --- a/lib/dns/catz.c +++ b/lib/dns/catz.c @@ -1059,12 +1059,14 @@ catz_process_primaries(dns_catz_zone_t *zone, dns_ipkeylist_t *ipkl, result = dns_rdata_tostruct(&rdata, &rdata_a, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); isc_sockaddr_fromin(&sockaddr, &rdata_a.in_addr, 0); + dns_rdata_freestruct(&rdata_a); break; case dns_rdatatype_aaaa: result = dns_rdata_tostruct(&rdata, &rdata_aaaa, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); isc_sockaddr_fromin6(&sockaddr, &rdata_aaaa.in6_addr, 0); + dns_rdata_freestruct(&rdata_aaaa); break; case dns_rdatatype_txt: result = dns_rdata_tostruct(&rdata, &rdata_txt, NULL); @@ -1072,16 +1074,19 @@ catz_process_primaries(dns_catz_zone_t *zone, dns_ipkeylist_t *ipkl, result = dns_rdata_txt_first(&rdata_txt); if (result != ISC_R_SUCCESS) { + dns_rdata_freestruct(&rdata_txt); return (result); } result = dns_rdata_txt_current(&rdata_txt, &rdatastr); if (result != ISC_R_SUCCESS) { + dns_rdata_freestruct(&rdata_txt); return (result); } result = dns_rdata_txt_next(&rdata_txt); if (result != ISC_R_NOMORE) { + dns_rdata_freestruct(&rdata_txt); return (ISC_R_FAILURE); } @@ -1090,6 +1095,7 @@ catz_process_primaries(dns_catz_zone_t *zone, dns_ipkeylist_t *ipkl, dns_name_init(keyname, 0); memmove(keycbuf, rdatastr.data, rdatastr.length); keycbuf[rdatastr.length] = 0; + dns_rdata_freestruct(&rdata_txt); result = dns_name_fromstring(keyname, keycbuf, 0, mctx); if (result != ISC_R_SUCCESS) { dns_name_free(keyname, mctx); @@ -1167,16 +1173,17 @@ catz_process_primaries(dns_catz_zone_t *zone, dns_ipkeylist_t *ipkl, RUNTIME_CHECK(result == ISC_R_SUCCESS); isc_sockaddr_fromin(&ipkl->addrs[ipkl->count], &rdata_a.in_addr, 0); + dns_rdata_freestruct(&rdata_a); } else { result = dns_rdata_tostruct(&rdata, &rdata_aaaa, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); isc_sockaddr_fromin6(&ipkl->addrs[ipkl->count], &rdata_aaaa.in6_addr, 0); + dns_rdata_freestruct(&rdata_aaaa); } ipkl->keys[ipkl->count] = NULL; ipkl->labels[ipkl->count] = NULL; ipkl->count++; - dns_rdata_freestruct(&rdata_a); } return (ISC_R_SUCCESS); } @@ -1418,6 +1425,7 @@ dns_catz_update_process(dns_catz_zones_t *catzs, dns_catz_zone_t *zone, /* * xxxwpk TODO do we want to save something from SOA? */ + dns_rdata_freestruct(&soa); return (result); } else if (rdataset->type == dns_rdatatype_ns) { return (ISC_R_SUCCESS); From a5a6362e922c024b196758e9cc5b580b6d56b86a Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Thu, 17 Mar 2022 14:47:15 +0000 Subject: [PATCH 5/6] Use 'bname' in dns_catz_update_from_db() only when it is ready There is a possible code path of using the uninitialized `bname` character array while logging an error message. Initialize the `bname` buffer earlier in the function. Also, change the initialization routine to use a helper function. --- lib/dns/catz.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/dns/catz.c b/lib/dns/catz.c index ca71a5d36a..b02157618a 100644 --- a/lib/dns/catz.c +++ b/lib/dns/catz.c @@ -1786,12 +1786,13 @@ dns_catz_update_from_db(dns_db_t *db, dns_catz_zones_t *catzs) { dns_rdatasetiter_t *rdsiter = NULL; dns_rdataset_t rdataset; char bname[DNS_NAME_FORMATSIZE]; - isc_buffer_t ibname; uint32_t vers; REQUIRE(DNS_DB_VALID(db)); REQUIRE(DNS_CATZ_ZONES_VALID(catzs)); + dns_name_format(&db->origin, bname, DNS_NAME_FORMATSIZE); + /* * Create a new catz in the same context as current catz. */ @@ -1805,10 +1806,6 @@ dns_catz_update_from_db(dns_db_t *db, dns_catz_zones_t *catzs) { return; } - isc_buffer_init(&ibname, bname, DNS_NAME_FORMATSIZE); - result = dns_name_totext(&db->origin, true, &ibname); - INSIST(result == ISC_R_SUCCESS); - result = dns_db_getsoaserial(db, oldzone->dbversion, &vers); if (result != ISC_R_SUCCESS) { /* A zone without SOA record?!? */ From 7fd24ded90f2d01ea0b1a4e33a8796410cce4aaf Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Wed, 23 Mar 2022 10:55:18 +0000 Subject: [PATCH 6/6] Add CHANGES note for [GL #3221] --- CHANGES | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGES b/CHANGES index 4bed2fc345..dc6429a66d 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,10 @@ +5838. [cleanup] When modifying a member zone in a catalog zone, and it + is detected that the zone exists and was not created by + the current catalog zone, distinguish the two cases when + the zone was not added by a catalog zone at all, and + when the zone was added by a different catalog zone, + and log a warning message accordingly. [GL #3221] + 5837. [func] Key timing options for `dnssec-keygen` and `dnssec-settime` now accept times as printed by `dnssec-settime -p`. [GL !2947]