diff --git a/CHANGES b/CHANGES index 9583c7030a..a98a447cf0 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5423. [bug] Fix a bug in keymgr_key_has_successor: it would + return a false positive if any other key in the + keyring has a successor. [GL #1845] + 5422. [bug] When using dnssec-policy, print correct keytiming metadata. [GL #1843] diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh index b0f2d727e2..6547053d05 100644 --- a/bin/tests/system/kasp/tests.sh +++ b/bin/tests/system/kasp/tests.sh @@ -2533,6 +2533,7 @@ set_keyalgorithm "KEY3" "13" "ECDSAP256SHA256" "256" set_keysigning "KEY3" "no" set_zonesigning "KEY3" "no" # Key states. +set_keystate "KEY2" "GOAL" "hidden" set_keystate "KEY3" "GOAL" "omnipresent" set_keystate "KEY3" "STATE_DNSKEY" "rumoured" set_keystate "KEY3" "STATE_ZRRSIG" "hidden" @@ -2570,7 +2571,6 @@ set_server "ns3" "10.53.0.3" # ZSK (KEY2) no longer is actively signing, RRSIG state in UNRETENTIVE. # New ZSK (KEY3) is now actively signing, RRSIG state in RUMOURED. set_zonesigning "KEY2" "no" -set_keystate "KEY2" "GOAL" "hidden" set_keystate "KEY2" "STATE_ZRRSIG" "unretentive" set_zonesigning "KEY3" "yes" set_keystate "KEY3" "STATE_DNSKEY" "omnipresent" @@ -2749,6 +2749,7 @@ set_keyalgorithm "KEY3" "13" "ECDSAP256SHA256" "256" set_keysigning "KEY3" "yes" set_zonesigning "KEY3" "no" # Key states. +set_keystate "KEY1" "GOAL" "hidden" set_keystate "KEY3" "GOAL" "omnipresent" set_keystate "KEY3" "STATE_DNSKEY" "rumoured" set_keystate "KEY3" "STATE_KRRSIG" "rumoured" @@ -2792,7 +2793,6 @@ set_zone "step3.ksk-doubleksk.autosign" set_policy "ksk-doubleksk" "3" "7200" set_server "ns3" "10.53.0.3" # KSK (KEY1) DS will be removed, so it is UNRETENTIVE. -set_keystate "KEY1" "GOAL" "hidden" set_keystate "KEY1" "STATE_DS" "unretentive" # New KSK (KEY3) has its DS submitted. set_keystate "KEY3" "STATE_DNSKEY" "omnipresent" @@ -2978,6 +2978,7 @@ set_keyalgorithm "KEY2" "13" "ECDSAP256SHA256" "256" set_keysigning "KEY2" "yes" set_zonesigning "KEY2" "no" # Key states. +set_keystate "KEY1" "GOAL" "hidden" set_keystate "KEY2" "GOAL" "omnipresent" set_keystate "KEY2" "STATE_DNSKEY" "rumoured" set_keystate "KEY2" "STATE_KRRSIG" "rumoured" @@ -3019,7 +3020,6 @@ set_server "ns3" "10.53.0.3" set_zonesigning "KEY1" "no" set_zonesigning "KEY2" "yes" # CSK (KEY1) DS and ZRRSIG will be removed, so it is UNRETENTIVE. -set_keystate "KEY1" "GOAL" "hidden" set_keystate "KEY1" "STATE_ZRRSIG" "unretentive" set_keystate "KEY1" "STATE_DS" "unretentive" # New CSK (KEY2) has its DS submitted, and is signing, so the DS and ZRRSIG @@ -3277,6 +3277,7 @@ set_keyalgorithm "KEY2" "13" "ECDSAP256SHA256" "256" set_keysigning "KEY2" "yes" set_zonesigning "KEY2" "no" # Key states. +set_keystate "KEY1" "GOAL" "hidden" set_keystate "KEY2" "GOAL" "omnipresent" set_keystate "KEY2" "STATE_DNSKEY" "rumoured" set_keystate "KEY2" "STATE_KRRSIG" "rumoured" @@ -3315,7 +3316,6 @@ set_policy "csk-roll2" "2" "3600" set_server "ns3" "10.53.0.3" # CSK (KEY1) DS and ZRRSIG will be removed, so it is UNRETENTIVE. set_zonesigning "KEY1" "no" -set_keystate "KEY1" "GOAL" "hidden" set_keystate "KEY1" "STATE_ZRRSIG" "unretentive" set_keystate "KEY1" "STATE_DS" "unretentive" # New CSK (KEY2) has its DS submitted, and is signing, so the DS and ZRRSIG diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 4f71ff1e04..c371e2e5cd 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -128,3 +128,7 @@ Bug Fixes - ``named`` could crash with an assertion failure if the name of a database node was looked up while the database was being modified. [GL #1857] + +- Fix a bug in dnssec-policy keymgr where the check if a key has a + successor would return a false positive if any other key in the + keyring has a successor. [GL #1845] diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c index 9a5480e24c..7a0831db50 100644 --- a/lib/dns/keymgr.c +++ b/lib/dns/keymgr.c @@ -633,11 +633,16 @@ keymgr_key_exists_with_state(dns_dnsseckeylist_t *keyring, dns_dnsseckey_t *key, * Check if a key has a successor. */ static bool -keymgr_key_has_successor(dns_dnsseckey_t *key, dns_dnsseckeylist_t *keyring) { - /* Don't worry about key states. */ - dst_key_state_t na[4] = { NA, NA, NA, NA }; - return (keymgr_key_exists_with_state(keyring, key, DST_KEY_DNSKEY, NA, - na, na, true, true)); +keymgr_key_has_successor(dns_dnsseckey_t *predecessor, + dns_dnsseckeylist_t *keyring) { + for (dns_dnsseckey_t *successor = ISC_LIST_HEAD(*keyring); + successor != NULL; successor = ISC_LIST_NEXT(successor, link)) + { + if (keymgr_key_is_successor(predecessor->key, successor->key)) { + return (true); + } + } + return (false); } /*