From 63edc4435f8ddefbbabbf9731f2b44d59d68c40b Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Mon, 3 Mar 2025 12:07:03 +0100 Subject: [PATCH] Fix wrong usage of safety intervals in keymgr There are a couple of cases where the safety intervals are added inappropriately: 1. When setting the PublishCDS/SyncPublish timing metadata, we don't need to add the publish-safety value if we are calculating the time when the zone is completely signed for the first time. This value is for when the DNSKEY has been published and we add a safety interval before considering the DNSKEY omnipresent. 2. The retire-safety value should only be added to ZSK rollovers if there is an actual rollover happening, similar to adding the sign delay. 3. The retire-safety value should only be added to KSK rollovers if there is an actual rollover happening. We consider the new DS omnipresent a bit later, so that we are forced to keep the old DS a bit longer. --- bin/tests/system/kasp/ns3/setup.sh | 11 +- bin/tests/system/kasp/ns6/setup.sh | 82 +++++----- bin/tests/system/kasp/tests.sh | 245 ++++++++++++++--------------- lib/dns/keymgr.c | 58 ++++--- 4 files changed, 194 insertions(+), 202 deletions(-) diff --git a/bin/tests/system/kasp/ns3/setup.sh b/bin/tests/system/kasp/ns3/setup.sh index 85a88f5856..bd7ee71ded 100644 --- a/bin/tests/system/kasp/ns3/setup.sh +++ b/bin/tests/system/kasp/ns3/setup.sh @@ -350,10 +350,9 @@ setup step2.enable-dnssec.autosign TpubN="now-900s" # RRSIG TTL: 12 hour (43200 seconds) # zone-propagation-delay: 5 minutes (300 seconds) -# retire-safety: 20 minutes (1200 seconds) # Already passed time: -900 seconds -# Total: 43800 seconds -TsbmN="now+43800s" +# Total: 42600 seconds +TsbmN="now+42600s" keytimes="-P ${TpubN} -P sync ${TsbmN} -A ${TpubN}" CSK=$($KEYGEN -k enable-dnssec -l policies/autosign.conf $keytimes $zone 2>keygen.out.$zone.1) $SETTIME -s -g $O -k $R $TpubN -r $R $TpubN -d $H $TpubN -z $R $TpubN "$CSK" >settime.out.$zone.1 2>&1 @@ -365,10 +364,10 @@ $SIGNER -S -z -x -s now-1h -e now+30d -o $zone -O raw -f "${zonefile}.signed" $i # Step 3: # The zone signatures have been published long enough to become OMNIPRESENT. setup step3.enable-dnssec.autosign -# Passed time since publications: 43800 + 900 = 44700 seconds. -TpubN="now-44700s" +# Passed time since publications: 42600 + 900 = 43500 seconds. +TpubN="now-43500s" # The key is secure for using in chain of trust when the DNSKEY is OMNIPRESENT. -TcotN="now-43800s" +TcotN="now-42600s" # We can submit the DS now. TsbmN="now" keytimes="-P ${TpubN} -P sync ${TsbmN} -A ${TpubN}" diff --git a/bin/tests/system/kasp/ns6/setup.sh b/bin/tests/system/kasp/ns6/setup.sh index 312a70518b..fcdabad355 100644 --- a/bin/tests/system/kasp/ns6/setup.sh +++ b/bin/tests/system/kasp/ns6/setup.sh @@ -127,9 +127,9 @@ setup step2.algorithm-roll.kasp # The time passed since the new algorithm keys have been introduced is 3 hours. TactN="now-3h" TpubN1="now-3h" -# Tsbm(N+1) = TpubN1 + Ipub = now + TTLsig + Dprp + publish-safety = -# now - 3h + 6h + 1h + 1h = now + 5h -TsbmN1="now+5h" +# Tsbm(N+1) = TpubN1 + Ipub = now + TTLsig + Dprp = +# now - 3h + 6h + 1h = now + 4h +TsbmN1="now+4h" ksk1times="-P ${TactN} -A ${TactN} -P sync ${TactN} -I now" zsk1times="-P ${TactN} -A ${TactN} -I now" ksk2times="-P ${TpubN1} -A ${TpubN1} -P sync ${TsbmN1}" @@ -156,11 +156,11 @@ $SIGNER -S -x -s now-1h -e now+2w -o $zone -O raw -f "${zonefile}.signed" $infil # Step 3: # The zone signatures are also OMNIPRESENT. setup step3.algorithm-roll.kasp -# The time passed since the new algorithm keys have been introduced is 9 hours. -TactN="now-9h" -TretN="now-6h" -TpubN1="now-9h" -TsbmN1="now-1h" +# The time passed since the new algorithm keys have been introduced is 7 hours. +TactN="now-7h" +TretN="now-3h" +TpubN1="now-7h" +TsbmN1="now" ksk1times="-P ${TactN} -A ${TactN} -P sync ${TactN} -I ${TretN}" zsk1times="-P ${TactN} -A ${TactN} -I ${TretN}" ksk2times="-P ${TpubN1} -A ${TpubN1} -P sync ${TsbmN1}" @@ -188,11 +188,11 @@ $SIGNER -S -x -s now-1h -e now+2w -o $zone -O raw -f "${zonefile}.signed" $infil # The DS is swapped and can become OMNIPRESENT. setup step4.algorithm-roll.kasp # The time passed since the DS has been swapped is 29 hours. -TactN="now-38h" -TretN="now-35h" -TpubN1="now-38h" -TsbmN1="now-30h" -TactN1="now-29h" +TactN="now-36h" +TretN="now-33h" +TpubN1="now-36h" +TsbmN1="now-29h" +TactN1="now-27h" ksk1times="-P ${TactN} -A ${TactN} -P sync ${TactN} -I ${TretN}" zsk1times="-P ${TactN} -A ${TactN} -I ${TretN}" ksk2times="-P ${TpubN1} -A ${TpubN1} -P sync ${TsbmN1}" @@ -220,12 +220,12 @@ $SIGNER -S -x -s now-1h -e now+2w -o $zone -O raw -f "${zonefile}.signed" $infil # The DNSKEY is removed long enough to be HIDDEN. setup step5.algorithm-roll.kasp # The time passed since the DNSKEY has been removed is 2 hours. -TactN="now-40h" -TretN="now-37h" +TactN="now-38h" +TretN="now-35h" TremN="now-2h" -TpubN1="now-40h" -TsbmN1="now-32h" -TactN1="now-31h" +TpubN1="now-38h" +TsbmN1="now-31h" +TactN1="now-29h" ksk1times="-P ${TactN} -A ${TactN} -P sync ${TactN} -I ${TretN}" zsk1times="-P ${TactN} -A ${TactN} -I ${TretN}" ksk2times="-P ${TpubN1} -A ${TpubN1} -P sync ${TsbmN1}" @@ -253,13 +253,13 @@ $SIGNER -S -x -s now-1h -e now+2w -o $zone -O raw -f "${zonefile}.signed" $infil # The RRSIGs have been removed long enough to be HIDDEN. setup step6.algorithm-roll.kasp # Additional time passed: 7h. -TactN="now-47h" -TretN="now-44h" +TactN="now-45h" +TretN="now-42h" TremN="now-7h" -TpubN1="now-47h" -TsbmN1="now-39h" -TactN1="now-38h" -TdeaN="now-9h" +TpubN1="now-45h" +TsbmN1="now-38h" +TactN1="now-36h" +TdeaN="now-7h" ksk1times="-P ${TactN} -A ${TactN} -P sync ${TactN} -I ${TretN}" zsk1times="-P ${TactN} -A ${TactN} -I ${TretN}" ksk2times="-P ${TpubN1} -A ${TpubN1} -P sync ${TsbmN1}" @@ -324,11 +324,11 @@ $SIGNER -S -x -z -s now-1h -e now+2w -o $zone -O raw -f "${zonefile}.signed" $in # Step 3: # The zone signatures are also OMNIPRESENT. setup step3.csk-algorithm-roll.kasp -# The time passed since the new algorithm keys have been introduced is 9 hours. -TactN="now-9h" -TretN="now-6h" -TpubN1="now-9h" -TactN1="now-6h" +# The time passed since the new algorithm keys have been introduced is 7 hours. +TactN="now-7h" +TretN="now-3h" +TpubN1="now-7h" +TactN1="now-3h" csktimes="-P ${TactN} -A ${TactN} -P sync ${TactN} -I ${TretN}" newtimes="-P ${TpubN1} -A ${TpubN1}" CSK1=$($KEYGEN -k csk-algoroll -l policies/csk1.conf $csktimes $zone 2>keygen.out.$zone.1) @@ -347,10 +347,10 @@ $SIGNER -S -x -z -s now-1h -e now+2w -o $zone -O raw -f "${zonefile}.signed" $in # The DS is swapped and can become OMNIPRESENT. setup step4.csk-algorithm-roll.kasp # The time passed since the DS has been swapped is 29 hours. -TactN="now-38h" -TretN="now-35h" -TpubN1="now-38h" -TactN1="now-35h" +TactN="now-36h" +TretN="now-33h" +TpubN1="now-36h" +TactN1="now-33h" TsubN1="now-29h" csktimes="-P ${TactN} -A ${TactN} -P sync ${TactN} -I ${TretN}" newtimes="-P ${TpubN1} -A ${TpubN1}" @@ -370,11 +370,11 @@ $SIGNER -S -x -z -s now-1h -e now+2w -o $zone -O raw -f "${zonefile}.signed" $in # The DNSKEY is removed long enough to be HIDDEN. setup step5.csk-algorithm-roll.kasp # The time passed since the DNSKEY has been removed is 2 hours. -TactN="now-40h" -TretN="now-37h" +TactN="now-38h" +TretN="now-35h" TremN="now-2h" -TpubN1="now-40h" -TactN1="now-37h" +TpubN1="now-38h" +TactN1="now-35h" TsubN1="now-31h" csktimes="-P ${TactN} -A ${TactN} -P sync ${TactN} -I ${TretN}" newtimes="-P ${TpubN1} -A ${TpubN1}" @@ -394,12 +394,12 @@ $SIGNER -S -x -z -s now-1h -e now+2w -o $zone -O raw -f "${zonefile}.signed" $in # The RRSIGs have been removed long enough to be HIDDEN. setup step6.csk-algorithm-roll.kasp # Additional time passed: 7h. -TactN="now-47h" -TretN="now-44h" +TactN="now-45h" +TretN="now-42h" TdeaN="now-9h" TremN="now-7h" -TpubN1="now-47h" -TactN1="now-44h" +TpubN1="now-45h" +TactN1="now-42h" TsubN1="now-38h" csktimes="-P ${TactN} -A ${TactN} -P sync ${TactN} -I ${TretN}" newtimes="-P ${TpubN1} -A ${TpubN1}" diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh index 2215666bb3..fa64c69f58 100644 --- a/bin/tests/system/kasp/tests.sh +++ b/bin/tests/system/kasp/tests.sh @@ -275,9 +275,8 @@ set_keytimes_csk_policy() { set_keytime "KEY1" "ACTIVE" "${created}" # The DS can be published if the DNSKEY and RRSIG records are # OMNIPRESENT. This happens after max-zone-ttl (1d) plus - # publish-safety (1h) plus zone-propagation-delay (300s) = - # 86400 + 3600 + 300 = 90300. - set_addkeytime "KEY1" "SYNCPUBLISH" "${created}" 90300 + # zone-propagation-delay (300s) = 86400 + 300 = 86700. + set_addkeytime "KEY1" "SYNCPUBLISH" "${created}" 86700 # Key lifetime is unlimited, so not setting RETIRED and REMOVED. } @@ -769,9 +768,8 @@ set_keytimes_algorithm_policy() { # The DS can be published if the DNSKEY and RRSIG records are # OMNIPRESENT. This happens after max-zone-ttl (1d) plus - # publish-safety (1h) plus zone-propagation-delay (300s) = - # 86400 + 3600 + 300 = 90300. - set_addkeytime "KEY1" "SYNCPUBLISH" "${published}" 90300 + # zone-propagation-delay (300s) = 86400 + 300 = 86700. + set_addkeytime "KEY1" "SYNCPUBLISH" "${published}" 86700 # Key lifetime is 10 years, 315360000 seconds. set_addkeytime "KEY1" "RETIRED" "${published}" 315360000 # The key is removed after the retire time plus DS TTL (1d), @@ -1720,10 +1718,10 @@ published=$(awk '{print $3}' syncpublish) { syncpublish = zrrsig_present; } @@ -272,7 +271,6 @@ keymgr_prepublication_time(dns_dnsseckey_t *key, dns_kasp_t *kasp, dns_ttl_t ttlsig = dns_kasp_zonemaxttl(kasp, true); syncpub2 = pub + ttlsig + - dns_kasp_publishsafety(kasp) + dns_kasp_zonepropagationdelay(kasp); } @@ -1286,6 +1284,7 @@ keymgr_transition_time(dns_dnsseckey_t *key, int type, isc_result_t ret; isc_stdtime_t lastchange, dstime, nexttime = now; dns_ttl_t ttlsig = dns_kasp_zonemaxttl(kasp, true); + uint32_t dsstate; /* * No need to wait if we move things into an uncertain state. @@ -1355,15 +1354,12 @@ keymgr_transition_time(dns_dnsseckey_t *key, int type, * records. This translates to: * * Dsgn + zone-propagation-delay + max-zone-ttl. - * - * We will also add the retire-safety interval. */ nexttime = lastchange + ttlsig + - dns_kasp_zonepropagationdelay(kasp) + - dns_kasp_retiresafety(kasp); + dns_kasp_zonepropagationdelay(kasp); /* - * Only add the sign delay Dsgn if there is an actual - * predecessor or successor key. + * Only add the sign delay Dsgn and retire-safety if + * there is an actual predecessor or successor key. */ uint32_t tag; ret = dst_key_getnum(key->key, DST_NUM_PREDECESSOR, @@ -1373,7 +1369,8 @@ keymgr_transition_time(dns_dnsseckey_t *key, int type, DST_NUM_SUCCESSOR, &tag); } if (ret == ISC_R_SUCCESS) { - nexttime += dns_kasp_signdelay(kasp); + nexttime += dns_kasp_signdelay(kasp) + + dns_kasp_retiresafety(kasp); } break; default: @@ -1399,35 +1396,36 @@ keymgr_transition_time(dns_dnsseckey_t *key, int type, * This translates to: * * parent-propagation-delay + parent-ds-ttl. - * - * We will also add the retire-safety interval. */ case OMNIPRESENT: - /* Make sure DS has been seen in the parent. */ - ret = dst_key_gettime(key->key, DST_TIME_DSPUBLISH, - &dstime); - if (ret != ISC_R_SUCCESS || dstime > now) { - /* Not yet, try again in an hour. */ - nexttime = now + 3600; - } else { - nexttime = - dstime + dns_kasp_dsttl(kasp) + - dns_kasp_parentpropagationdelay(kasp) + - dns_kasp_retiresafety(kasp); - } - break; case HIDDEN: - /* Make sure DS has been withdrawn from the parent. */ - ret = dst_key_gettime(key->key, DST_TIME_DSDELETE, - &dstime); + /* Make sure DS has been seen in/withdrawn from the + * parent. */ + dsstate = next_state == HIDDEN ? DST_TIME_DSDELETE + : DST_TIME_DSPUBLISH; + ret = dst_key_gettime(key->key, dsstate, &dstime); if (ret != ISC_R_SUCCESS || dstime > now) { /* Not yet, try again in an hour. */ nexttime = now + 3600; } else { nexttime = dstime + dns_kasp_dsttl(kasp) + - dns_kasp_parentpropagationdelay(kasp) + - dns_kasp_retiresafety(kasp); + dns_kasp_parentpropagationdelay(kasp); + /* + * Only add the retire-safety if there is an + * actual predecessor or successor key. + */ + uint32_t tag; + ret = dst_key_getnum(key->key, + DST_NUM_PREDECESSOR, &tag); + if (ret != ISC_R_SUCCESS) { + ret = dst_key_getnum(key->key, + DST_NUM_SUCCESSOR, + &tag); + } + if (ret == ISC_R_SUCCESS) { + nexttime += dns_kasp_retiresafety(kasp); + } } break; default: