From a1c95d5fa479ac722f0cf758c494a37ffe1508c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Sat, 25 May 2024 11:46:56 +0200 Subject: [PATCH] Add a limit to the number of RR types for single name Previously, the number of RR types for a single owner name was limited only by the maximum number of the types (64k). As the data structure that holds the RR types for the database node is just a linked list, and there are places where we just walk through the whole list (again and again), adding a large number of RR types for a single owner named with would slow down processing of such name (database node). Add a configurable limit to cap the number of the RR types for a single owner. This is enforced at the database (rbtdb, qpzone, qpcache) level and configured with new max-types-per-name configuration option that can be configured globally, per-view and per-zone. (cherry picked from commit 00d16211d6368b99f070c1182d8c76b3798ca1db) (cherry picked from commit 89f1779bc28b27adbd00325b974ede7a683f8632) fix a memory leak that could occur when signing when signatures were not added because of too many types already existing at a node, the diff was not being cleaned up; this led to a memory leak being reported at shutdown. (cherry picked from commit 2825bdb1ae5be801e7ed603ba2455ed9a308f1f7) (cherry picked from commit a080317de0efb7f6ffa12415a863729d416007d5) Be smarter about refusing to add many RR types to the database Instead of outright refusing to add new RR types to the cache, be a bit smarter: 1. If the new header type is in our priority list, we always add either positive or negative entry at the beginning of the list. 2. If the new header type is negative entry, and we are over the limit, we mark it as ancient immediately, so it gets evicted from the cache as soon as possible. 3. Otherwise add the new header after the priority headers (or at the head of the list). 4. If we are over the limit, evict the last entry on the normal header list. (cherry picked from commit 57cd34441a1b4ecc9874a4a106c2c95b8d7a3120) (cherry picked from commit 92a680a3ef708281267e4fd7b1e62b57c929447b) Log error when update fails The new "too many records" error can make an update fail without the error being logged. This commit fixes that. (cherry picked from commit 558923e5405894cf976d102f0d246a28bdbb400c) (cherry picked from commit d72adf4b927d83a2a0ff8e431b911ec1df7aeb88) --- bin/named/config.c | 1 + bin/named/server.c | 9 +++++++++ bin/named/zoneconf.c | 8 ++++++++ bin/tests/system/dyndb/driver/db.c | 3 ++- doc/arm/reference.rst | 12 ++++++++++++ lib/dns/cache.c | 12 ++++++++++++ lib/dns/db.c | 9 +++++++++ lib/dns/dnsrps.c | 3 ++- lib/dns/ecdb.c | 3 ++- lib/dns/include/dns/cache.h | 6 ++++++ lib/dns/include/dns/db.h | 11 +++++++++++ lib/dns/include/dns/view.h | 7 +++++++ lib/dns/include/dns/zone.h | 13 +++++++++++++ lib/dns/rbtdb.c | 28 +++++++++++++++++----------- lib/dns/sdb.c | 3 ++- lib/dns/sdlz.c | 3 ++- lib/dns/view.c | 10 ++++++++++ lib/dns/zone.c | 16 ++++++++++++++++ lib/isccfg/namedconf.c | 3 +++ lib/ns/update.c | 15 ++++++++++++--- 20 files changed, 156 insertions(+), 19 deletions(-) diff --git a/bin/named/config.c b/bin/named/config.c index 9cba6f588b..c9888ada65 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -218,6 +218,7 @@ options {\n\ max-records-per-type 100;\n\ max-refresh-time 2419200; /* 4 weeks */\n\ max-retry-time 1209600; /* 2 weeks */\n\ + max-types-per-name 100;\n\ max-transfer-idle-in 60;\n\ max-transfer-idle-out 60;\n\ max-transfer-time-in 120;\n\ diff --git a/bin/named/server.c b/bin/named/server.c index 7bf5f2664d..4cc69b54a1 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -5427,6 +5427,15 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, INSIST(result == ISC_R_SUCCESS); dns_view_setmaxrrperset(view, cfg_obj_asuint32(obj)); + /* + * This is used for the cache and also as a default value + * for zone databases. + */ + obj = NULL; + result = named_config_get(maps, "max-types-per-name", &obj); + INSIST(result == ISC_R_SUCCESS); + dns_view_setmaxtypepername(view, cfg_obj_asuint32(obj)); + obj = NULL; result = named_config_get(maps, "max-recursion-depth", &obj); INSIST(result == ISC_R_SUCCESS); diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c index ae5cc656ee..f6e8c64866 100644 --- a/bin/named/zoneconf.c +++ b/bin/named/zoneconf.c @@ -1100,6 +1100,14 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig, dns_zone_setmaxrrperset(zone, 0); } + obj = NULL; + result = named_config_get(maps, "max-types-per-name", &obj); + INSIST(result == ISC_R_SUCCESS && obj != NULL); + dns_zone_setmaxtypepername(mayberaw, cfg_obj_asuint32(obj)); + if (zone != mayberaw) { + dns_zone_setmaxtypepername(zone, 0); + } + if (raw != NULL && filename != NULL) { #define SIGNED ".signed" size_t signedlen = strlen(filename) + sizeof(SIGNED); diff --git a/bin/tests/system/dyndb/driver/db.c b/bin/tests/system/dyndb/driver/db.c index 6725a3bacd..c95fc8212b 100644 --- a/bin/tests/system/dyndb/driver/db.c +++ b/bin/tests/system/dyndb/driver/db.c @@ -593,7 +593,8 @@ static dns_dbmethods_t sampledb_methods = { NULL, /* getservestalerefresh */ NULL, /* setgluecachestats */ NULL, /* adjusthashsize */ - NULL /* setmaxrrperset */ + NULL, /* setmaxrrperset */ + NULL /* setmaxtypepername */ }; /* Auxiliary driver functions. */ diff --git a/doc/arm/reference.rst b/doc/arm/reference.rst index b1983ef30d..a8a3c7911d 100644 --- a/doc/arm/reference.rst +++ b/doc/arm/reference.rst @@ -2902,6 +2902,18 @@ system. a failure. If set to 0, there is no cap on RRset size. The default is 100. +``max-types-per-name`` + This sets the maximum number of resource record types that can be stored + for a single owner name in a database. When configured in ``options`` + or ``view``, it controls the cache database, and also sets + the default value for zone databases, which can be overridden by setting + it at the ``zone`` level + + If set to a positive value, any attempt to cache or to add to a zone an owner + name with more than the specified number of resource record types will result + in a failure. If set to 0, there is no cap on RR types number. The default is + 100. + ``recursive-clients`` This sets the maximum number (a "hard quota") of simultaneous recursive lookups the server performs on behalf of clients. The default is diff --git a/lib/dns/cache.c b/lib/dns/cache.c index 9f0412dbe7..0b474fc313 100644 --- a/lib/dns/cache.c +++ b/lib/dns/cache.c @@ -150,6 +150,7 @@ struct dns_cache { /* Access to the on-disk cache file is also locked by 'filelock'. */ uint32_t maxrrperset; + uint32_t maxtypepername; }; /*** @@ -178,6 +179,7 @@ cache_create_db(dns_cache_t *cache, dns_db_t **db) { if (result == ISC_R_SUCCESS) { dns_db_setservestalettl(*db, cache->serve_stale_ttl); dns_db_setmaxrrperset(*db, cache->maxrrperset); + dns_db_setmaxtypepername(*db, cache->maxtypepername); } return (result); } @@ -1290,6 +1292,16 @@ dns_cache_setmaxrrperset(dns_cache_t *cache, uint32_t value) { } } +void +dns_cache_setmaxtypepername(dns_cache_t *cache, uint32_t value) { + REQUIRE(VALID_CACHE(cache)); + + cache->maxtypepername = value; + if (cache->db != NULL) { + dns_db_setmaxtypepername(cache->db, value); + } +} + /* * XXX: Much of the following code has been copied in from statschannel.c. * We should refactor this into a generic function in stats.c that can be diff --git a/lib/dns/db.c b/lib/dns/db.c index 8439265a7f..18583d41c2 100644 --- a/lib/dns/db.c +++ b/lib/dns/db.c @@ -1131,3 +1131,12 @@ dns_db_setmaxrrperset(dns_db_t *db, uint32_t value) { (db->methods->setmaxrrperset)(db, value); } } + +void +dns_db_setmaxtypepername(dns_db_t *db, uint32_t value) { + REQUIRE(DNS_DB_VALID(db)); + + if (db->methods->setmaxtypepername != NULL) { + (db->methods->setmaxtypepername)(db, value); + } +} diff --git a/lib/dns/dnsrps.c b/lib/dns/dnsrps.c index 539090d1bd..e1a1b21a8b 100644 --- a/lib/dns/dnsrps.c +++ b/lib/dns/dnsrps.c @@ -971,7 +971,8 @@ static dns_dbmethods_t rpsdb_db_methods = { NULL, /* getservestalerefresh */ NULL, /* setgluecachestats */ NULL, /* adjusthashsize */ - NULL /* setmaxrrperset */ + NULL, /* setmaxrrperset */ + NULL /* setmaxtypepername */ }; static dns_rdatasetmethods_t rpsdb_rdataset_methods = { diff --git a/lib/dns/ecdb.c b/lib/dns/ecdb.c index bab5da5503..27d03b4e3a 100644 --- a/lib/dns/ecdb.c +++ b/lib/dns/ecdb.c @@ -560,7 +560,8 @@ static dns_dbmethods_t ecdb_methods = { NULL, /* getservestalerefresh */ NULL, /* setgluecachestats */ NULL, /* adjusthashsize */ - NULL /* setmaxrrperset */ + NULL, /* setmaxrrperset */ + NULL /* setmaxtypepername */ }; static isc_result_t diff --git a/lib/dns/include/dns/cache.h b/lib/dns/include/dns/cache.h index 3fa2a891e0..72de21600a 100644 --- a/lib/dns/include/dns/cache.h +++ b/lib/dns/include/dns/cache.h @@ -343,6 +343,12 @@ dns_cache_setmaxrrperset(dns_cache_t *cache, uint32_t value); * Set the maximum resource records per RRSet that can be cached. */ +void +dns_cache_setmaxtypepername(dns_cache_t *cache, uint32_t value); +/*%< + * Set the maximum resource record types per owner name that can be cached. + */ + #ifdef HAVE_LIBXML2 int dns_cache_renderxml(dns_cache_t *cache, void *writer0); diff --git a/lib/dns/include/dns/db.h b/lib/dns/include/dns/db.h index 732bfe473d..411881d48a 100644 --- a/lib/dns/include/dns/db.h +++ b/lib/dns/include/dns/db.h @@ -183,6 +183,7 @@ typedef struct dns_dbmethods { isc_result_t (*setgluecachestats)(dns_db_t *db, isc_stats_t *stats); isc_result_t (*adjusthashsize)(dns_db_t *db, size_t size); void (*setmaxrrperset)(dns_db_t *db, uint32_t value); + void (*setmaxtypepername)(dns_db_t *db, uint32_t value); } dns_dbmethods_t; typedef isc_result_t (*dns_dbcreatefunc_t)(isc_mem_t *mctx, @@ -1791,6 +1792,16 @@ dns_db_setmaxrrperset(dns_db_t *db, uint32_t value); * is nonzero, then any subsequent attempt to add an rdataset with * more than 'value' RRs will return ISC_R_NOSPACE. */ + +void +dns_db_setmaxtypepername(dns_db_t *db, uint32_t value); +/*%< + * Set the maximum permissible number of RR types per owner name. + * + * If 'value' is nonzero, then any subsequent attempt to add an rdataset with a + * RR type that would exceed the number of already stored RR types will return + * ISC_R_NOSPACE. + */ ISC_LANG_ENDDECLS #endif /* DNS_DB_H */ diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index 0d502f4dd2..0a72f58e98 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -187,6 +187,7 @@ struct dns_view { uint32_t fail_ttl; dns_badcache_t *failcache; uint32_t maxrrperset; + uint32_t maxtypepername; /* * Configurable data for server use only, @@ -1346,6 +1347,12 @@ dns_view_setmaxrrperset(dns_view_t *view, uint32_t value); * Set the maximum resource records per RRSet that can be cached. */ +void +dns_view_setmaxtypepername(dns_view_t *view, uint32_t value); +/*%< + * Set the maximum resource record types per owner name that can be cached. + */ + ISC_LANG_ENDDECLS #endif /* DNS_VIEW_H */ diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h index e902043357..6fca11f3fd 100644 --- a/lib/dns/include/dns/zone.h +++ b/lib/dns/include/dns/zone.h @@ -356,6 +356,19 @@ dns_zone_setmaxrrperset(dns_zone_t *zone, uint32_t maxrrperset); *\li void */ +void +dns_zone_setmaxtypepername(dns_zone_t *zone, uint32_t maxtypepername); +/*%< + * Sets the maximum number of resource record types per owner name + * permitted in a zone. 0 implies unlimited. + * + * Requires: + *\li 'zone' to be valid initialised zone. + * + * Returns: + *\li void + */ + void dns_zone_setmaxttl(dns_zone_t *zone, uint32_t maxttl); /*%< diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index ca71bb9c03..ed5015c2d4 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -483,6 +483,7 @@ struct dns_rbtdb { rbtdb_serial_t least_serial; rbtdb_serial_t next_serial; uint32_t maxrrperset; + uint32_t maxtypepername; rbtdb_version_t *current_version; rbtdb_version_t *future_version; rbtdb_versionlist_t open_versions; @@ -6222,19 +6223,13 @@ update_recordsandxfrsize(bool add, rbtdb_version_t *rbtversion, RWUNLOCK(&rbtversion->rwlock, isc_rwlocktype_write); } -#ifndef DNS_RBTDB_MAX_RTYPES -#define DNS_RBTDB_MAX_RTYPES 100 -#endif /* DNS_RBTDB_MAX_RTYPES */ - static bool overmaxtype(dns_rbtdb_t *rbtdb, uint32_t ntypes) { - UNUSED(rbtdb); - - if (DNS_RBTDB_MAX_RTYPES == 0) { + if (rbtdb->maxtypepername == 0) { return (false); } - return (ntypes >= DNS_RBTDB_MAX_RTYPES); + return (ntypes >= rbtdb->maxtypepername); } static bool @@ -6794,7 +6789,7 @@ find_header: if (!IS_CACHE(rbtdb) && overmaxtype(rbtdb, ntypes)) { free_rdataset(rbtdb, rbtdb->common.mctx, newheader); - return (ISC_R_QUOTA); + return (DNS_R_TOOMANYRECORDS); } newheader->down = NULL; @@ -8623,6 +8618,15 @@ setmaxrrperset(dns_db_t *db, uint32_t maxrrperset) { rbtdb->maxrrperset = maxrrperset; } +static void +setmaxtypepername(dns_db_t *db, uint32_t maxtypepername) { + dns_rbtdb_t *rbtdb = (dns_rbtdb_t *)db; + + REQUIRE(VALID_RBTDB(rbtdb)); + + rbtdb->maxtypepername = maxtypepername; +} + static dns_stats_t * getrrsetstats(dns_db_t *db) { dns_rbtdb_t *rbtdb = (dns_rbtdb_t *)db; @@ -8747,7 +8751,8 @@ static dns_dbmethods_t zone_methods = { attach, NULL, /* getservestalerefresh */ setgluecachestats, adjusthashsize, - setmaxrrperset }; + setmaxrrperset, + setmaxtypepername }; static dns_dbmethods_t cache_methods = { attach, detach, @@ -8800,7 +8805,8 @@ static dns_dbmethods_t cache_methods = { attach, getservestalerefresh, NULL, adjusthashsize, - setmaxrrperset }; + setmaxrrperset, + setmaxtypepername }; isc_result_t dns_rbtdb_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type, diff --git a/lib/dns/sdb.c b/lib/dns/sdb.c index 84cd324fb4..77a5834b76 100644 --- a/lib/dns/sdb.c +++ b/lib/dns/sdb.c @@ -1313,7 +1313,8 @@ static dns_dbmethods_t sdb_methods = { NULL, /* getservestalerefresh */ NULL, /* setgluecachestats */ NULL, /* adjusthashsize */ - NULL /* setmaxrrperset */ + NULL, /* setmaxrrperset */ + NULL /* setmaxtypepername */ }; static isc_result_t diff --git a/lib/dns/sdlz.c b/lib/dns/sdlz.c index 60a1d23b3b..418a4a14ee 100644 --- a/lib/dns/sdlz.c +++ b/lib/dns/sdlz.c @@ -1285,7 +1285,8 @@ static dns_dbmethods_t sdlzdb_methods = { NULL, /* getservestalerefresh */ NULL, /* setgluecachestats */ NULL, /* adjusthashsize */ - NULL /* setmaxrrperset */ + NULL, /* setmaxrrperset */ + NULL /* setmaxtypepername */ }; /* diff --git a/lib/dns/view.c b/lib/dns/view.c index a672aa8bc8..98579f03d9 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -871,6 +871,7 @@ dns_view_setcache(dns_view_t *view, dns_cache_t *cache, bool shared) { INSIST(DNS_DB_VALID(view->cachedb)); dns_cache_setmaxrrperset(view->cache, view->maxrrperset); + dns_cache_setmaxtypepername(view->cache, view->maxtypepername); } bool @@ -2555,3 +2556,12 @@ dns_view_setmaxrrperset(dns_view_t *view, uint32_t value) { dns_cache_setmaxrrperset(view->cache, value); } } + +void +dns_view_setmaxtypepername(dns_view_t *view, uint32_t value) { + REQUIRE(DNS_VIEW_VALID(view)); + view->maxtypepername = value; + if (view->cache != NULL) { + dns_cache_setmaxtypepername(view->cache, value); + } +} diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 5c8d97ed18..e1fb9ab50b 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -277,6 +277,7 @@ struct dns_zone { uint32_t maxrecords; uint32_t maxrrperset; + uint32_t maxtypepername; isc_sockaddr_t *masters; isc_dscp_t *masterdscps; @@ -9959,6 +9960,7 @@ cleanup: } dns_diff_clear(&_sig_diff); + dns_diff_clear(&post_diff); for (i = 0; i < nkeys; i++) { dst_key_free(&zone_keys[i]); @@ -12168,6 +12170,16 @@ dns_zone_setmaxrrperset(dns_zone_t *zone, uint32_t val) { } } +void +dns_zone_setmaxtypepername(dns_zone_t *zone, uint32_t val) { + REQUIRE(DNS_ZONE_VALID(zone)); + + zone->maxtypepername = val; + if (zone->db != NULL) { + dns_db_setmaxtypepername(zone->db, val); + } +} + static bool notify_isqueued(dns_zone_t *zone, unsigned int flags, dns_name_t *name, isc_sockaddr_t *addr, dns_tsigkey_t *key) { @@ -14573,6 +14585,8 @@ ns_query(dns_zone_t *zone, dns_rdataset_t *soardataset, dns_stub_t *stub) { } dns_db_settask(stub->db, zone->task); dns_db_setmaxrrperset(stub->db, zone->maxrrperset); + dns_db_setmaxtypepername(stub->db, + zone->maxtypepername); } result = dns_db_newversion(stub->db, &stub->version); @@ -17295,6 +17309,7 @@ zone_replacedb(dns_zone_t *zone, dns_db_t *db, bool dump) { zone_attachdb(zone, db); dns_db_settask(zone->db, zone->task); dns_db_setmaxrrperset(zone->db, zone->maxrrperset); + dns_db_setmaxtypepername(zone->db, zone->maxtypepername); DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_LOADED | DNS_ZONEFLG_NEEDNOTIFY); return (ISC_R_SUCCESS); @@ -23444,6 +23459,7 @@ dns_zone_makedb(dns_zone_t *zone, dns_db_t **dbp) { dns_db_settask(db, zone->task); dns_db_setmaxrrperset(db, zone->maxrrperset); + dns_db_setmaxtypepername(db, zone->maxtypepername); *dbp = db; diff --git a/lib/isccfg/namedconf.c b/lib/isccfg/namedconf.c index dce30537dd..ac9fc2af5e 100644 --- a/lib/isccfg/namedconf.c +++ b/lib/isccfg/namedconf.c @@ -2239,6 +2239,9 @@ static cfg_clausedef_t zone_clauses[] = { { "max-records-per-type", &cfg_type_uint32, CFG_ZONE_PRIMARY | CFG_ZONE_SECONDARY | CFG_ZONE_MIRROR | CFG_ZONE_STUB | CFG_ZONE_STATICSTUB | CFG_ZONE_REDIRECT }, + { "max-types-per-name", &cfg_type_uint32, + CFG_ZONE_PRIMARY | CFG_ZONE_SECONDARY | CFG_ZONE_MIRROR | + CFG_ZONE_STUB | CFG_ZONE_STATICSTUB | CFG_ZONE_REDIRECT }, { "max-refresh-time", &cfg_type_uint32, CFG_ZONE_SECONDARY | CFG_ZONE_MIRROR | CFG_ZONE_STUB }, { "max-retry-time", &cfg_type_uint32, diff --git a/lib/ns/update.c b/lib/ns/update.c index c5ce1eaf09..0e0bdc9c03 100644 --- a/lib/ns/update.c +++ b/lib/ns/update.c @@ -3112,9 +3112,18 @@ update_action(isc_task_t *task, isc_event_t *event) { dns_diff_clear(&ctx.add_diff); goto failure; } - CHECK(update_one_rr(db, ver, &diff, - DNS_DIFFOP_ADD, - name, ttl, &rdata)); + result = update_one_rr( + db, ver, &diff, DNS_DIFFOP_ADD, + name, ttl, &rdata); + if (result != ISC_R_SUCCESS) { + update_log(client, zone, + LOGLEVEL_PROTOCOL, + "adding an RR " + "failed: %s", + isc_result_totext( + result)); + goto failure; + } } } } else if (update_class == dns_rdataclass_any) { -- 2.45.2