From b1ef1ded697f0e53439943ec533d19820a3988bc Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 8 Sep 2022 09:48:27 +1000 Subject: [PATCH 1/3] 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); } } From b15309e10dd809648ee16042a3940ba864965da9 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 8 Sep 2022 09:59:10 +1000 Subject: [PATCH 2/3] Add a CHANGES note for [GL #3525] --- CHANGES | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGES b/CHANGES index 74144751cf..68770f4101 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +5969. [bug] DNSSEC signing statistics failed to identify the + algorithm involved. The key names have been changed + to be the algorithm number followed by "+" followed + by the key id (e.g. "8+54274"). [GL #3525] + 5968. [cleanup] Remove 'resolve' binary from tests. [GL !6733] 5967. [cleanup] Flagged the obsolete "random-device" option as From 91488a9b6d0a653c74a8bf2f73960dc99693dc55 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 8 Sep 2022 10:01:25 +1000 Subject: [PATCH 3/3] Add a release note for [GL #3525] --- doc/notes/notes-current.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index e1f3761303..2d7f9218c5 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -46,6 +46,12 @@ Feature Changes or pass it through unchanged, instead of stopping with an error message. You can use the ``idna2`` utility for checking IDNA syntax. :gl:`#3485`. +- The DNSSEC signing data included in zone statistics identified + keys only by the key ID; this caused confusion when two keys using + different algorithms had the same ID. Zone statistics now identify + keys using the algorithm number, followed by "+", followed by the + key ID: for example, "8+54274". :gl:`#3525` + Bug Fixes ~~~~~~~~~