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 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/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 ~~~~~~~~~ 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); } }