From b1ef1ded697f0e53439943ec533d19820a3988bc Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 8 Sep 2022 09:48:27 +1000 Subject: [PATCH] Emit key algorithm + key id in dnssec signing statsistics If there was a collision of key id across algorithms it was not possible to determine where counter applies to which algorithm for xml statistics while for json only one of the values was emitted. The key names are now "+" (e.g. "8+54274"). --- bin/named/statschannel.c | 8 ++++++-- bin/tests/system/statschannel/ns2/sign.sh | 24 +++++++++++++++-------- lib/dns/include/dns/stats.h | 2 +- lib/dns/stats.c | 9 ++------- 4 files changed, 25 insertions(+), 18 deletions(-) diff --git a/bin/named/statschannel.c b/bin/named/statschannel.c index 11e50e1689..3fc907f1dc 100644 --- a/bin/named/statschannel.c +++ b/bin/named/statschannel.c @@ -1713,7 +1713,7 @@ cleanup: #if defined(EXTENDED_STATS) static void -dnssecsignstat_dump(dns_keytag_t tag, uint64_t val, void *arg) { +dnssecsignstat_dump(uint32_t kval, uint64_t val, void *arg) { FILE *fp; char tagbuf[64]; stats_dumparg_t *dumparg = arg; @@ -1725,7 +1725,11 @@ dnssecsignstat_dump(dns_keytag_t tag, uint64_t val, void *arg) { json_object *zoneobj, *obj; #endif /* ifdef HAVE_JSON_C */ - snprintf(tagbuf, sizeof(tagbuf), "%u", tag); + /* + * kval is '(algorithm << 16) | keyid'. + */ + snprintf(tagbuf, sizeof(tagbuf), "%u+%u", (kval >> 16) & 0xff, + kval & 0xffff); switch (dumparg->type) { case isc_statsformat_file: diff --git a/bin/tests/system/statschannel/ns2/sign.sh b/bin/tests/system/statschannel/ns2/sign.sh index f9429cadaf..d93a9b8694 100644 --- a/bin/tests/system/statschannel/ns2/sign.sh +++ b/bin/tests/system/statschannel/ns2/sign.sh @@ -23,8 +23,10 @@ ksk=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -f KSK "$zone") zsk=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" "$zone") # Sign deliberately with a very short expiration date. "$SIGNER" -P -S -x -O full -e "now"+1s -o "$zone" -f "$zonefile" "$infile" > "signzone.out.$zone" 2>&1 -keyfile_to_key_id "$ksk" > dnssec.ksk.id -keyfile_to_key_id "$zsk" > dnssec.zsk.id +id=$(keyfile_to_key_id "$ksk") +echo "$DEFAULT_ALGORITHM_NUMBER+$id" > dnssec.ksk.id +id=$(keyfile_to_key_id "$zsk") +echo "$DEFAULT_ALGORITHM_NUMBER+$id" > dnssec.zsk.id zone=manykeys. infile=manykeys.db.in @@ -37,9 +39,15 @@ ksk14=$("$KEYGEN" -q -a ECDSAP384SHA384 -b 384 -f KSK "$zone") zsk14=$("$KEYGEN" -q -a ECDSAP384SHA384 -b 384 "$zone") # Sign deliberately with a very short expiration date. "$SIGNER" -S -x -O full -e "now"+1s -o "$zone" -f "$zonefile" "$infile" > "signzone.out.$zone" 2>&1 -keyfile_to_key_id "$ksk8" > manykeys.ksk8.id -keyfile_to_key_id "$zsk8" > manykeys.zsk8.id -keyfile_to_key_id "$ksk13" > manykeys.ksk13.id -keyfile_to_key_id "$zsk13" > manykeys.zsk13.id -keyfile_to_key_id "$ksk14" > manykeys.ksk14.id -keyfile_to_key_id "$zsk14" > manykeys.zsk14.id +id=$(keyfile_to_key_id "$ksk8") +echo "8+$id" > manykeys.ksk8.id +id=$(keyfile_to_key_id "$zsk8") +echo "8+$id" > manykeys.zsk8.id +id=$(keyfile_to_key_id "$ksk13") +echo "13+$id" > manykeys.ksk13.id +id=$(keyfile_to_key_id "$zsk13") +echo "13+$id" > manykeys.zsk13.id +id=$(keyfile_to_key_id "$ksk14") +echo "14+$id" > manykeys.ksk14.id +id=$(keyfile_to_key_id "$zsk14") +echo "14+$id" > manykeys.zsk14.id diff --git a/lib/dns/include/dns/stats.h b/lib/dns/include/dns/stats.h index 1f57810e73..e6e39775ad 100644 --- a/lib/dns/include/dns/stats.h +++ b/lib/dns/include/dns/stats.h @@ -510,7 +510,7 @@ typedef enum { typedef void (*dns_generalstats_dumper_t)(isc_statscounter_t, uint64_t, void *); typedef void (*dns_rdatatypestats_dumper_t)(dns_rdatastatstype_t, uint64_t, void *); -typedef void (*dns_dnssecsignstats_dumper_t)(dns_keytag_t, uint64_t, void *); +typedef void (*dns_dnssecsignstats_dumper_t)(uint32_t, uint64_t, void *); typedef void (*dns_opcodestats_dumper_t)(dns_opcode_t, uint64_t, void *); typedef void (*dns_rcodestats_dumper_t)(dns_rcode_t, uint64_t, void *); diff --git a/lib/dns/stats.c b/lib/dns/stats.c index 4f58ef0d77..a636a44528 100644 --- a/lib/dns/stats.c +++ b/lib/dns/stats.c @@ -107,8 +107,6 @@ typedef enum { /* Maximum number of keys to keep track of for DNSSEC signing statistics. */ static int dnssecsign_num_keys = 4; static int dnssecsign_block_size = 3; -/* Key id mask */ -#define DNSSECSIGNSTATS_KEY_ID_MASK 0x0000FFFF struct dns_stats { unsigned int magic; @@ -536,7 +534,7 @@ static void dnssec_dumpcb(isc_statscounter_t counter, uint64_t value, void *arg) { dnssecsigndumparg_t *dnssecarg = arg; - dnssecarg->fn((dns_keytag_t)counter, value, dnssecarg->arg); + dnssecarg->fn((uint32_t)counter, value, dnssecarg->arg); } static void @@ -548,7 +546,6 @@ dnssec_statsdump(isc_stats_t *stats, dnssecsignstats_type_t operation, for (i = 0; i < num_keys; i++) { int idx = dnssecsign_block_size * i; uint32_t kval, val; - dns_keytag_t id; kval = isc_stats_get_counter(stats, idx); if (kval == 0) { @@ -560,9 +557,7 @@ dnssec_statsdump(isc_stats_t *stats, dnssecsignstats_type_t operation, continue; } - id = (dns_keytag_t)kval & DNSSECSIGNSTATS_KEY_ID_MASK; - - dump_fn((isc_statscounter_t)id, val, arg); + dump_fn(kval, val, arg); } }