From 08122316a7aec70b6ddc739de717be0e8263789b Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 29 Aug 2024 16:24:48 +0900 Subject: [PATCH 1/3] emit more helpful log for exceeding max-records-per-type The new log message is emitted when adding or updating an RRset fails due to exceeding the max-records-per-type limit. The log includes the owner name and type, corresponding zone name, and the limit value. It will be emitted on loading a zone file, inbound zone transfer (both AXFR and IXFR), handling a DDNS update, or updating a cache DB. It's especially helpful in the case of zone transfer, since the secondary side doesn't have direct access to the offending zone data. It could also be used for max-types-per-name, but this change doesn't implement it yet as it's much less likely to happen in practice. (cherry picked from commit 4156995431290ed6ac1a96424fc3527a453ffb7c) --- lib/dns/db.c | 24 ++++++++++++++++++++++++ lib/dns/db_p.h | 9 +++++++++ lib/dns/qpcache.c | 5 +++++ lib/dns/qpzone.c | 16 ++++++++++++++++ lib/dns/rbt-zonedb.c | 5 +++++ lib/dns/rbtdb.c | 13 +++++++++++++ 6 files changed, 72 insertions(+) diff --git a/lib/dns/db.c b/lib/dns/db.c index 370feac3cc..5dafc5b576 100644 --- a/lib/dns/db.c +++ b/lib/dns/db.c @@ -38,6 +38,7 @@ #include #include #include +#include #include #include @@ -1190,3 +1191,26 @@ dns_db_setmaxtypepername(dns_db_t *db, uint32_t value) { (db->methods->setmaxtypepername)(db, value); } } + +void +dns__db_logtoomanyrecords(dns_db_t *db, const dns_name_t *name, + dns_rdatatype_t type, const char *op, + uint32_t limit) { + char namebuf[DNS_NAME_FORMATSIZE]; + char originbuf[DNS_NAME_FORMATSIZE]; + char typebuf[DNS_RDATATYPE_FORMATSIZE]; + char clsbuf[DNS_RDATACLASS_FORMATSIZE]; + + dns_name_format(name, namebuf, sizeof(namebuf)); + dns_name_format(&db->origin, originbuf, sizeof(originbuf)); + dns_rdatatype_format(type, typebuf, sizeof(typebuf)); + dns_rdataclass_format(db->rdclass, clsbuf, sizeof(clsbuf)); + + isc_log_write( + dns_lctx, DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_RBTDB, + ISC_LOG_ERROR, + "error %s '%s/%s' in '%s/%s' (%s): %s (must not exceed %u)", op, + namebuf, typebuf, originbuf, clsbuf, + (db->attributes & DNS_DBATTR_CACHE) != 0 ? "cache" : "zone", + isc_result_totext(DNS_R_TOOMANYRECORDS), limit); +} diff --git a/lib/dns/db_p.h b/lib/dns/db_p.h index 0228536c35..3a982252a6 100644 --- a/lib/dns/db_p.h +++ b/lib/dns/db_p.h @@ -187,4 +187,13 @@ prio_type(dns_typepair_t type) { return false; } +void +dns__db_logtoomanyrecords(dns_db_t *db, const dns_name_t *name, + dns_rdatatype_t type, const char *op, uint32_t limit); +/* + * Emit a log message when adding an rdataset of name/type would exceed the + * 'maxrrperset' limit. 'op' is 'adding' or 'updating' depending on whether + * the addition is to create a new rdataset or to merge to an existing one. + */ + ISC_LANG_ENDDECLS diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c index 1a4fb543be..7dd90fa276 100644 --- a/lib/dns/qpcache.c +++ b/lib/dns/qpcache.c @@ -3414,6 +3414,11 @@ addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, ®ion, sizeof(dns_slabheader_t), qpdb->maxrrperset); if (result != ISC_R_SUCCESS) { + if (result == DNS_R_TOOMANYRECORDS) { + dns__db_logtoomanyrecords((dns_db_t *)qpdb, + &qpnode->name, rdataset->type, + "adding", qpdb->maxrrperset); + } return result; } diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c index 69bafead48..ae416b17ec 100644 --- a/lib/dns/qpzone.c +++ b/lib/dns/qpzone.c @@ -1875,6 +1875,12 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename, header->resign_lsb; } } else { + if (result == DNS_R_TOOMANYRECORDS) { + dns__db_logtoomanyrecords( + (dns_db_t *)qpdb, nodename, + (dns_rdatatype_t)header->type, + "updating", qpdb->maxrrperset); + } dns_slabheader_destroy(&newheader); return result; } @@ -2109,6 +2115,11 @@ loading_addrdataset(void *arg, const dns_name_t *name, ®ion, sizeof(dns_slabheader_t), qpdb->maxrrperset); if (result != ISC_R_SUCCESS) { + if (result == DNS_R_TOOMANYRECORDS) { + dns__db_logtoomanyrecords((dns_db_t *)qpdb, name, + rdataset->type, "adding", + qpdb->maxrrperset); + } return result; } @@ -4607,6 +4618,11 @@ addrdataset(dns_db_t *db, dns_dbnode_t *dbnode, dns_dbversion_t *dbversion, ®ion, sizeof(dns_slabheader_t), qpdb->maxrrperset); if (result != ISC_R_SUCCESS) { + if (result == DNS_R_TOOMANYRECORDS) { + dns__db_logtoomanyrecords((dns_db_t *)qpdb, &node->name, + rdataset->type, "adding", + qpdb->maxrrperset); + } return result; } diff --git a/lib/dns/rbt-zonedb.c b/lib/dns/rbt-zonedb.c index ecd5eb2bde..bed5c29527 100644 --- a/lib/dns/rbt-zonedb.c +++ b/lib/dns/rbt-zonedb.c @@ -1751,6 +1751,11 @@ loading_addrdataset(void *arg, const dns_name_t *name, ®ion, sizeof(dns_slabheader_t), rbtdb->maxrrperset); if (result != ISC_R_SUCCESS) { + if (result == DNS_R_TOOMANYRECORDS) { + dns__db_logtoomanyrecords((dns_db_t *)rbtdb, name, + rdataset->type, "adding", + rbtdb->maxrrperset); + } return result; } newheader = (dns_slabheader_t *)region.base; diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index a822b35d6c..ffd7801c06 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -2807,6 +2807,12 @@ find_header: header->resign_lsb; } } else { + if (result == DNS_R_TOOMANYRECORDS) { + dns__db_logtoomanyrecords( + (dns_db_t *)rbtdb, nodename, + (dns_rdatatype_t)(header->type), + "updating", rbtdb->maxrrperset); + } dns_slabheader_destroy(&newheader); return result; } @@ -3305,6 +3311,13 @@ dns__rbtdb_addrdataset(dns_db_t *db, dns_dbnode_t *node, ®ion, sizeof(dns_slabheader_t), rbtdb->maxrrperset); if (result != ISC_R_SUCCESS) { + if (result == DNS_R_TOOMANYRECORDS) { + name = dns_fixedname_initname(&fixed); + dns__rbtdb_nodefullname(db, node, name); + dns__db_logtoomanyrecords((dns_db_t *)rbtdb, name, + rdataset->type, "adding", + rbtdb->maxrrperset); + } return result; } From b8996b6e83d67d384ec7d39c01fb97de9f617022 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Wed, 18 Sep 2024 15:32:21 +0900 Subject: [PATCH 2/3] use more generic log module name for 'logtoomanyrecords' DNS_LOGMODULE_RBTDB was simply inappropriate, and this log message is actually dependent on db implementation details, so DNS_LOGMODULE_DB would be the best choice. (cherry picked from commit b0309ee631dc68c6462500354acc749ff41ccb55) --- lib/dns/db.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dns/db.c b/lib/dns/db.c index 5dafc5b576..43edbb76a4 100644 --- a/lib/dns/db.c +++ b/lib/dns/db.c @@ -1207,7 +1207,7 @@ dns__db_logtoomanyrecords(dns_db_t *db, const dns_name_t *name, dns_rdataclass_format(db->rdclass, clsbuf, sizeof(clsbuf)); isc_log_write( - dns_lctx, DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_RBTDB, + dns_lctx, DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_DB, ISC_LOG_ERROR, "error %s '%s/%s' in '%s/%s' (%s): %s (must not exceed %u)", op, namebuf, typebuf, originbuf, clsbuf, From c862555b664222ce68db3a9c90d5ba01b5167050 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 26 Sep 2024 17:04:57 +0900 Subject: [PATCH 3/3] update system tests to confirm new log messages (cherry picked from commit 000720fe14266595ce37ad9ca692b28310f5a6f7) --- bin/tests/system/ixfr/tests.sh | 5 +++++ bin/tests/system/masterformat/tests.sh | 8 ++++++++ bin/tests/system/nsupdate/ns1/named.conf.in | 1 + bin/tests/system/nsupdate/tests.sh | 21 +++++++++++++++++++++ bin/tests/system/reclimit/tests.sh | 6 ++++++ 5 files changed, 41 insertions(+) diff --git a/bin/tests/system/ixfr/tests.sh b/bin/tests/system/ixfr/tests.sh index 97014200f2..ca5114723f 100644 --- a/bin/tests/system/ixfr/tests.sh +++ b/bin/tests/system/ixfr/tests.sh @@ -183,6 +183,11 @@ $DIG $DIGOPTS @10.53.0.1 nil. TXT | grep 'AXFR on too many records' >/dev/null | if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) +msg="error adding 'nil/TXT' in 'nil/IN' (zone): too many records (must not exceed 5)" +wait_for_log 10 "$msg" ns1/named.run || ret=1 +if [ $ret != 0 ]; then echo_i "failed"; fi +status=$((status + ret)) + n=$((n + 1)) echo_i "testing AXFR fallback after IXFR failure (bad SOA owner) ($n)" ret=0 diff --git a/bin/tests/system/masterformat/tests.sh b/bin/tests/system/masterformat/tests.sh index e103e328c8..cbf52157e7 100755 --- a/bin/tests/system/masterformat/tests.sh +++ b/bin/tests/system/masterformat/tests.sh @@ -315,6 +315,14 @@ n=$((n + 1)) [ $ret -eq 0 ] || echo_i "failed" status=$((status + ret)) +# Check that the corresponding log message about exceeding the limit is present. +msg="error adding '2100-txt.above-limit/TXT' in 'above-limit/IN' (zone): too many records (must not exceed 2050)" +wait_for_log 10 "$msg" ns1/named.run || ret=1 +if [ $ret != 0 ]; then echo_i "failed"; fi +status=$((status + ret)) +# Prepare for any further checking of the logs later on. +nextpart ns1/named.run >/dev/null + echo_i "checking that kasp-max-records-per-type rdatasets loaded ($n)" for _attempt in 0 1 2 3 4 5 6 7 8 9; do ret=0 diff --git a/bin/tests/system/nsupdate/ns1/named.conf.in b/bin/tests/system/nsupdate/ns1/named.conf.in index 2c173bd5f2..56d95f5c1f 100644 --- a/bin/tests/system/nsupdate/ns1/named.conf.in +++ b/bin/tests/system/nsupdate/ns1/named.conf.in @@ -80,6 +80,7 @@ zone "max-ttl.nil" { check-integrity no; allow-update { named-acl; }; allow-transfer { any; }; + max-records-per-type 3; }; zone "other.nil" { diff --git a/bin/tests/system/nsupdate/tests.sh b/bin/tests/system/nsupdate/tests.sh index f3f9723667..8b4efc5051 100755 --- a/bin/tests/system/nsupdate/tests.sh +++ b/bin/tests/system/nsupdate/tests.sh @@ -1359,6 +1359,27 @@ if [ $ret -ne 0 ]; then status=1 fi +n=$((n + 1)) +echo_i "check adding more records than max-records-per-type fails ($n)" +ret=0 +$NSUPDATE <nsupdate.out.test$n 2>&1 && ret=1 +server 10.53.0.1 ${PORT} +zone max-ttl.nil. +update add a.max-ttl.nil. 60 IN A 192.0.2.1 +update add a.max-ttl.nil. 60 IN A 192.0.2.2 +update add a.max-ttl.nil. 60 IN A 192.0.2.3 +update add a.max-ttl.nil. 60 IN A 192.0.2.4 +send +END +grep "update failed: SERVFAIL" nsupdate.out.test$n >/dev/null || ret=1 +msg="error updating 'a.max-ttl.nil/A' in 'max-ttl.nil/IN' (zone): too many records (must not exceed 3)" +wait_for_log 10 "$msg" ns1/named.run || ret=1 +[ $ret = 0 ] || { + echo_i "failed" + status=1 +} +nextpart ns1/named.run >/dev/null + n=$((n + 1)) ret=0 echo_i "add a record which is truncated when logged. ($n)" diff --git a/bin/tests/system/reclimit/tests.sh b/bin/tests/system/reclimit/tests.sh index 4d9105e62e..8cc8fe122a 100644 --- a/bin/tests/system/reclimit/tests.sh +++ b/bin/tests/system/reclimit/tests.sh @@ -233,6 +233,12 @@ echo_i "checking RRset that exceeds max-records-per-type ($n)" ret=0 dig_with_opts @10.53.0.3 biganswer.big >dig.out.1.test$n || ret=1 grep 'status: SERVFAIL' dig.out.1.test$n >/dev/null || ret=1 + +msg="error adding 'biganswer.big/A' in './IN' (cache): too many records (must not exceed 100)" +wait_for_log 10 "$msg" ns3/named.run || ret=1 +if [ $ret != 0 ]; then echo_i "failed"; fi +status=$((status + ret)) + ns3_reset ns3/named5.conf.in dig_with_opts @10.53.0.3 biganswer.big >dig.out.2.test$n || ret=1 grep 'status: NOERROR' dig.out.2.test$n >/dev/null || ret=1