diff --git a/CHANGES b/CHANGES index 52981c221f..dbd9318f31 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,10 @@ +5373. [bug] Collecting DNSSEC signing operations introduced by + GL #513 (change 5254) allocated counters for every + possible key id per zone which results in a lot of + wasted memory. Fix by tracking up to four keys + per zone, rotate counters when keys are replaced. + [GL #1179] + 5372. [bug] Fix migration from existing DNSSEC key files using auto-dnssec maintain to dnssec-policy. [GL #1706] diff --git a/bin/named/statschannel.c b/bin/named/statschannel.c index c3568b1216..2bbe01de8a 100644 --- a/bin/named/statschannel.c +++ b/bin/named/statschannel.c @@ -1814,7 +1814,6 @@ zone_xmlrender(dns_zone_t *zone, void *arg) { isc_stats_t *gluecachestats; dns_stats_t *rcvquerystats; dns_stats_t *dnssecsignstats; - dns_stats_t *dnssecrefreshstats; uint64_t nsstat_values[ns_statscounter_max]; uint64_t gluecachestats_values[dns_gluecachestatscounter_max]; @@ -1880,6 +1879,7 @@ zone_xmlrender(dns_zone_t *zone, void *arg) { dnssecsignstats = dns_zone_getdnssecsignstats(zone); if (dnssecsignstats != NULL) { + /* counters type="dnssec-sign"*/ TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "counters")); TRY0(xmlTextWriterWriteAttribute( @@ -1887,7 +1887,7 @@ zone_xmlrender(dns_zone_t *zone, void *arg) { ISC_XMLCHAR "dnssec-sign")); dumparg.result = ISC_R_SUCCESS; - dns_dnssecsignstats_dump(dnssecsignstats, + dns_dnssecsignstats_dump(dnssecsignstats, false, dnssecsignstat_dump, &dumparg, 0); if (dumparg.result != ISC_R_SUCCESS) { @@ -1896,10 +1896,8 @@ zone_xmlrender(dns_zone_t *zone, void *arg) { /* counters type="dnssec-sign"*/ TRY0(xmlTextWriterEndElement(writer)); - } - dnssecrefreshstats = dns_zone_getdnssecrefreshstats(zone); - if (dnssecrefreshstats != NULL) { + /* counters type="dnssec-refresh"*/ TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "counters")); TRY0(xmlTextWriterWriteAttribute( @@ -1907,7 +1905,7 @@ zone_xmlrender(dns_zone_t *zone, void *arg) { ISC_XMLCHAR "dnssec-refresh")); dumparg.result = ISC_R_SUCCESS; - dns_dnssecsignstats_dump(dnssecrefreshstats, + dns_dnssecsignstats_dump(dnssecsignstats, true, dnssecsignstat_dump, &dumparg, 0); if (dumparg.result != ISC_R_SUCCESS) { @@ -2626,7 +2624,6 @@ zone_jsonrender(dns_zone_t *zone, void *arg) { isc_stats_t *gluecachestats; dns_stats_t *rcvquerystats; dns_stats_t *dnssecsignstats; - dns_stats_t *dnssecrefreshstats; uint64_t nsstat_values[ns_statscounter_max]; uint64_t gluecachestats_values[dns_gluecachestatscounter_max]; @@ -2714,7 +2711,7 @@ zone_jsonrender(dns_zone_t *zone, void *arg) { dumparg.type = isc_statsformat_json; dumparg.arg = counters; dumparg.result = ISC_R_SUCCESS; - dns_dnssecsignstats_dump(dnssecsignstats, + dns_dnssecsignstats_dump(dnssecsignstats, false, dnssecsignstat_dump, &dumparg, 0); if (dumparg.result != ISC_R_SUCCESS) { @@ -2730,8 +2727,7 @@ zone_jsonrender(dns_zone_t *zone, void *arg) { } } - dnssecrefreshstats = dns_zone_getdnssecrefreshstats(zone); - if (dnssecrefreshstats != NULL) { + if (dnssecsignstats != NULL) { stats_dumparg_t dumparg; json_object *counters = json_object_new_object(); CHECKMEM(counters); @@ -2739,7 +2735,7 @@ zone_jsonrender(dns_zone_t *zone, void *arg) { dumparg.type = isc_statsformat_json; dumparg.arg = counters; dumparg.result = ISC_R_SUCCESS; - dns_dnssecsignstats_dump(dnssecrefreshstats, + dns_dnssecsignstats_dump(dnssecsignstats, true, dnssecsignstat_dump, &dumparg, 0); if (dumparg.result != ISC_R_SUCCESS) { diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c index 6ceb710346..7cad6c6b7a 100644 --- a/bin/named/zoneconf.c +++ b/bin/named/zoneconf.c @@ -890,7 +890,6 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig, isc_stats_t *zoneqrystats; dns_stats_t *rcvquerystats; dns_stats_t *dnssecsignstats; - dns_stats_t *dnssecrefreshstats; dns_zonestat_level_t statlevel = dns_zonestat_none; int seconds; dns_zone_t *mayberaw = (raw != NULL) ? raw : zone; @@ -1187,18 +1186,15 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig, zoneqrystats = NULL; rcvquerystats = NULL; dnssecsignstats = NULL; - dnssecrefreshstats = NULL; if (statlevel == dns_zonestat_full) { RETERR(isc_stats_create(mctx, &zoneqrystats, ns_statscounter_max)); RETERR(dns_rdatatypestats_create(mctx, &rcvquerystats)); RETERR(dns_dnssecsignstats_create(mctx, &dnssecsignstats)); - RETERR(dns_dnssecsignstats_create(mctx, &dnssecrefreshstats)); } dns_zone_setrequeststats(zone, zoneqrystats); dns_zone_setrcvquerystats(zone, rcvquerystats); dns_zone_setdnssecsignstats(zone, dnssecsignstats); - dns_zone_setdnssecrefreshstats(zone, dnssecrefreshstats); if (zoneqrystats != NULL) { isc_stats_detach(&zoneqrystats); @@ -1212,10 +1208,6 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig, dns_stats_detach(&dnssecsignstats); } - if (dnssecrefreshstats != NULL) { - dns_stats_detach(&dnssecrefreshstats); - } - /* * Configure master functionality. This applies * to primary masters (type "master") and slaves diff --git a/lib/dns/include/dns/stats.h b/lib/dns/include/dns/stats.h index 4526e68e2d..79c3fe70a0 100644 --- a/lib/dns/include/dns/stats.h +++ b/lib/dns/include/dns/stats.h @@ -684,9 +684,11 @@ dns_rcodestats_increment(dns_stats_t *stats, dns_opcode_t code); */ void -dns_dnssecsignstats_increment(dns_stats_t *stats, dns_keytag_t id); +dns_dnssecsignstats_increment(dns_stats_t *stats, dns_keytag_t id, + bool refresh); /*%< - * Increment the statistics counter for the DNSKEY 'id'. + * Increment the statistics counter for the DNSKEY 'id'. If 'refresh' is set + * to true, update the refresh counter, otherwise update the sign counter. * * Requires: *\li 'stats' is a valid dns_stats_t created by dns_dnssecsignstats_create(). @@ -737,7 +739,7 @@ dns_rdatasetstats_dump(dns_stats_t *stats, dns_rdatatypestats_dumper_t dump_fn, */ void -dns_dnssecsignstats_dump(dns_stats_t * stats, +dns_dnssecsignstats_dump(dns_stats_t *stats, bool refresh, dns_dnssecsignstats_dumper_t dump_fn, void *arg, unsigned int options); /*%< diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h index c5c363ab0e..093e467598 100644 --- a/lib/dns/include/dns/zone.h +++ b/lib/dns/include/dns/zone.h @@ -1957,9 +1957,6 @@ dns_zone_setrcvquerystats(dns_zone_t *zone, dns_stats_t *stats); void dns_zone_setdnssecsignstats(dns_zone_t *zone, dns_stats_t *stats); - -void -dns_zone_setdnssecrefreshstats(dns_zone_t *zone, dns_stats_t *stats); /*%< * Set additional statistics sets to zone. These are attached to the zone * but are not counted in the zone module; only the caller updates the @@ -1979,9 +1976,6 @@ dns_zone_getrcvquerystats(dns_zone_t *zone); dns_stats_t * dns_zone_getdnssecsignstats(dns_zone_t *zone); - -dns_stats_t * -dns_zone_getdnssecrefreshstats(dns_zone_t *zone); /*%< * Get the additional statistics for zone, if one is installed. * @@ -1993,17 +1987,6 @@ dns_zone_getdnssecrefreshstats(dns_zone_t *zone); * otherwise NULL. */ -/*%< - * Set additional statistics sets to zone. These are attached to the zone - * but are not counted in the zone module; only the caller updates the - * counters. - * - * Requires: - * \li 'zone' to be a valid zone. - * - *\li stats is a valid statistics. - */ - void dns_zone_dialup(dns_zone_t *zone); /*%< diff --git a/lib/dns/stats.c b/lib/dns/stats.c index 50f9308f23..605be5f71a 100644 --- a/lib/dns/stats.c +++ b/lib/dns/stats.c @@ -20,6 +20,7 @@ #include #include +#include #include #include #include @@ -93,8 +94,18 @@ typedef enum { */ #define RDTYPECOUNTER_MAXVAL 0x0602 -/* dnssec maximum key id */ -static int dnssec_keyid_max = 65535; +/* + * DNSSEC sign statistics. + * + * Per key we maintain 3 counters. The first is actually no counter but + * a key id reference. The second is the number of signatures the key created. + * The third is the number of signatures refreshed by the key. + */ + +/* Maximum number of keys to keep track of for DNSSEC signing statistics. */ +static int dnssec_max_keys = 4; +/* Attribute to signal whether a counter is actually a key id. */ +#define DNSSECSIGNSTATS_IS_KEY 0x10000 struct dns_stats { unsigned int magic; @@ -228,7 +239,11 @@ isc_result_t dns_dnssecsignstats_create(isc_mem_t *mctx, dns_stats_t **statsp) { REQUIRE(statsp != NULL && *statsp == NULL); - return (create_stats(mctx, dns_statstype_dnssec, dnssec_keyid_max, + /* + * Create two counters per key, one is the key id, the other two are + * the actual counters for creating and refreshing signatures. + */ + return (create_stats(mctx, dns_statstype_dnssec, dnssec_max_keys * 3, statsp)); } @@ -342,10 +357,64 @@ dns_rcodestats_increment(dns_stats_t *stats, dns_rcode_t code) { } void -dns_dnssecsignstats_increment(dns_stats_t *stats, dns_keytag_t id) { +dns_dnssecsignstats_increment(dns_stats_t *stats, dns_keytag_t id, + bool refresh) { + isc_statscounter_t operation; + uint32_t kval; + REQUIRE(DNS_STATS_VALID(stats) && stats->type == dns_statstype_dnssec); - isc_stats_increment(stats->counters, (isc_statscounter_t)id); + kval = (uint32_t)id; + kval |= DNSSECSIGNSTATS_IS_KEY; + + /* What operation are we counting? */ + if (refresh) { + operation = (isc_statscounter_t)dnssec_max_keys * 2; + } else { + operation = (isc_statscounter_t)dnssec_max_keys; + } + + /* Look up correct counter. */ + for (int i = 0; i < dnssec_max_keys; i++) { + uint32_t counter = isc_stats_get_counter(stats->counters, i); + if (counter == kval) { + /* Match */ + isc_stats_increment(stats->counters, operation + i); + return; + } + } + + /* No match found. Store key in unused slot. */ + for (int i = 0; i < dnssec_max_keys; i++) { + uint32_t counter = isc_stats_get_counter(stats->counters, i); + if (counter == 0) { + isc_stats_set(stats->counters, kval, i); + isc_stats_increment(stats->counters, operation + i); + return; + } + } + + /* No room, rotate keys. */ + for (int i = 1; i < dnssec_max_keys; i++) { + uint32_t keyv = isc_stats_get_counter(stats->counters, i); + uint32_t sign = isc_stats_get_counter(stats->counters, + (dnssec_max_keys + i)); + uint32_t refr = isc_stats_get_counter( + stats->counters, (dnssec_max_keys * 2 + i)); + + isc_stats_set(stats->counters, keyv, i - 1); + isc_stats_set(stats->counters, sign, dnssec_max_keys + i - 1); + isc_stats_set(stats->counters, refr, + dnssec_max_keys * 2 + i - 1); + } + + /* Reset counters for new key. */ + isc_stats_set(stats->counters, kval, dnssec_max_keys - 1); + isc_stats_set(stats->counters, 0, 2 * dnssec_max_keys - 1); + isc_stats_set(stats->counters, 0, 3 * dnssec_max_keys - 1); + + /* And increment the counter for the given operation. */ + isc_stats_increment(stats->counters, operation + dnssec_max_keys - 1); } /*% @@ -452,8 +521,41 @@ dnssec_dumpcb(isc_statscounter_t counter, uint64_t value, void *arg) { dnssecarg->fn((dns_keytag_t)counter, value, dnssecarg->arg); } +static void +dnssec_statsdump(isc_stats_t *stats, bool refresh, isc_stats_dumper_t dump_fn, + void *arg, unsigned int options) { + int i; + isc_statscounter_t operation; + + if (refresh) { + operation = (isc_statscounter_t)dnssec_max_keys * 2; + } else { + operation = (isc_statscounter_t)dnssec_max_keys; + } + + for (i = 0; i < dnssec_max_keys; i++) { + uint32_t kval, val; + dns_keytag_t id; + + kval = isc_stats_get_counter(stats, i); + if (kval == 0) { + continue; + } + + val = isc_stats_get_counter(stats, (operation + i)); + if ((options & ISC_STATSDUMP_VERBOSE) == 0 && val == 0) { + continue; + } + + id = (dns_keytag_t)kval; + id &= ~DNSSECSIGNSTATS_IS_KEY; + + dump_fn((isc_statscounter_t)id, val, arg); + } +} + void -dns_dnssecsignstats_dump(dns_stats_t *stats, +dns_dnssecsignstats_dump(dns_stats_t *stats, bool refresh, dns_dnssecsignstats_dumper_t dump_fn, void *arg0, unsigned int options) { dnssecsigndumparg_t arg; @@ -462,7 +564,9 @@ dns_dnssecsignstats_dump(dns_stats_t *stats, arg.fn = dump_fn; arg.arg = arg0; - isc_stats_dump(stats->counters, dnssec_dumpcb, &arg, options); + + dnssec_statsdump(stats->counters, refresh, dnssec_dumpcb, &arg, + options); } static void diff --git a/lib/dns/update.c b/lib/dns/update.c index e2a062e0c3..476c3e4b17 100644 --- a/lib/dns/update.c +++ b/lib/dns/update.c @@ -1259,8 +1259,8 @@ add_sigs(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db, added_sig = true; /* Update DNSSEC sign statistics. */ if (dnssecsignstats != NULL) { - dns_dnssecsignstats_increment(dnssecsignstats, - dst_key_id(keys[i])); + dns_dnssecsignstats_increment( + dnssecsignstats, dst_key_id(keys[i]), false); } } if (!added_sig) { diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in index 71c85d9ef7..50e42bae36 100644 --- a/lib/dns/win32/libdns.def.in +++ b/lib/dns/win32/libdns.def.in @@ -1194,7 +1194,6 @@ dns_zone_getchecknames dns_zone_getclass dns_zone_getdb dns_zone_getdbtype -dns_zone_getdnssecrefreshstats dns_zone_getdnssecsignstats dns_zone_getexpiretime dns_zone_getfile @@ -1298,7 +1297,6 @@ dns_zone_setclass dns_zone_setdb dns_zone_setdbtype dns_zone_setdialup -dns_zone_setdnssecrefreshstats dns_zone_setdnssecsignstats dns_zone_setfile dns_zone_setflag diff --git a/lib/dns/zone.c b/lib/dns/zone.c index aefb71b866..23c8a5f92b 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -338,7 +338,6 @@ struct dns_zone { isc_stats_t *requeststats; dns_stats_t *rcvquerystats; dns_stats_t *dnssecsignstats; - dns_stats_t *dnssecrefreshstats; uint32_t notifydelay; dns_isselffunc_t isself; void *isselfarg; @@ -1091,7 +1090,6 @@ dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx) { zone->requeststats = NULL; zone->rcvquerystats = NULL; zone->dnssecsignstats = NULL; - zone->dnssecrefreshstats = NULL; zone->notifydelay = 5; zone->isself = NULL; zone->isselfarg = NULL; @@ -1270,9 +1268,6 @@ zone_free(dns_zone_t *zone) { if (zone->dnssecsignstats != NULL) { dns_stats_detach(&zone->dnssecsignstats); } - if (zone->dnssecrefreshstats != NULL) { - dns_stats_detach(&zone->dnssecrefreshstats); - } if (zone->db != NULL) { zone_detachdb(zone); } @@ -6752,7 +6747,6 @@ add_sigs(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, dns_zone_t *zone, dns_dbnode_t *node = NULL; dns_kasp_t *kasp = dns_zone_getkasp(zone); dns_stats_t *dnssecsignstats; - dns_stats_t *dnssecrefreshstats; dns_rdataset_t rdataset; dns_rdata_t sig_rdata = DNS_RDATA_INIT; unsigned char data[1024]; /* XXX */ @@ -6926,16 +6920,13 @@ add_sigs(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, dns_zone_t *zone, /* Update DNSSEC sign statistics. */ dnssecsignstats = dns_zone_getdnssecsignstats(zone); - dnssecrefreshstats = dns_zone_getdnssecrefreshstats(zone); if (dnssecsignstats != NULL) { + /* Generated a new signature. */ dns_dnssecsignstats_increment( - dns_zone_getdnssecsignstats(zone), - dst_key_id(keys[i])); - } - if (dnssecrefreshstats != NULL) { + dnssecsignstats, dst_key_id(keys[i]), false); + /* This is a refresh. */ dns_dnssecsignstats_increment( - dns_zone_getdnssecrefreshstats(zone), - dst_key_id(keys[i])); + dnssecsignstats, dst_key_id(keys[i]), true); } } @@ -7396,7 +7387,6 @@ sign_a_node(dns_db_t *db, dns_zone_t *zone, dns_name_t *name, dns_rdataset_t rdataset; dns_rdata_t rdata = DNS_RDATA_INIT; dns_stats_t *dnssecsignstats; - dns_stats_t *dnssecrefreshstats; isc_buffer_t buffer; unsigned char data[1024]; @@ -7515,16 +7505,13 @@ sign_a_node(dns_db_t *db, dns_zone_t *zone, dns_name_t *name, /* Update DNSSEC sign statistics. */ dnssecsignstats = dns_zone_getdnssecsignstats(zone); - dnssecrefreshstats = dns_zone_getdnssecrefreshstats(zone); if (dnssecsignstats != NULL) { - dns_dnssecsignstats_increment( - dns_zone_getdnssecsignstats(zone), - dst_key_id(key)); - } - if (dnssecrefreshstats != NULL) { - dns_dnssecsignstats_increment( - dns_zone_getdnssecrefreshstats(zone), - dst_key_id(key)); + /* Generated a new signature. */ + dns_dnssecsignstats_increment(dnssecsignstats, + dst_key_id(key), false); + /* This is a refresh. */ + dns_dnssecsignstats_increment(dnssecsignstats, + dst_key_id(key), true); } (*signatures)--; @@ -18458,17 +18445,6 @@ dns_zone_setdnssecsignstats(dns_zone_t *zone, dns_stats_t *stats) { UNLOCK_ZONE(zone); } -void -dns_zone_setdnssecrefreshstats(dns_zone_t *zone, dns_stats_t *stats) { - REQUIRE(DNS_ZONE_VALID(zone)); - - LOCK_ZONE(zone); - if (stats != NULL && zone->dnssecrefreshstats == NULL) { - dns_stats_attach(stats, &zone->dnssecrefreshstats); - } - UNLOCK_ZONE(zone); -} - dns_stats_t * dns_zone_getdnssecsignstats(dns_zone_t *zone) { REQUIRE(DNS_ZONE_VALID(zone)); @@ -18476,13 +18452,6 @@ dns_zone_getdnssecsignstats(dns_zone_t *zone) { return (zone->dnssecsignstats); } -dns_stats_t * -dns_zone_getdnssecrefreshstats(dns_zone_t *zone) { - REQUIRE(DNS_ZONE_VALID(zone)); - - return (zone->dnssecrefreshstats); -} - isc_stats_t * dns_zone_getrequeststats(dns_zone_t *zone) { /*