Merge branch '3525-key-id-clashes-across-algorithms-cause-problems-with-statistics' into 'main'

Resolve "key id clashes across algorithms cause problems with statistics"

Closes #3525

See merge request isc-projects/bind9!6745
This commit is contained in:
Mark Andrews 2022-09-15 01:04:17 +00:00
commit 5c5f6964ff
6 changed files with 36 additions and 18 deletions

View file

@ -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

View file

@ -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:

View file

@ -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

View file

@ -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
~~~~~~~~~

View file

@ -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 *);

View file

@ -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);
}
}