Fix CDS (non-)publication

The CDS/CDNSKEY record will be published when the DS is in the
rumoured state. However, with the introduction of the rndc '-checkds'
command, the logic in the keymgr was changed to prevent the DS
state to go in RUMOURED unless the specific command was given. Hence,
the CDS was never published before it was seen in the parent.

Initially I thought this was a policy approval rule, however it is
actually a DNSSEC timing rule. Remove the restriction from
'keymgr_policy_approval' and update the 'keymgr_transition_time'
function. When looking to move the DS state to OMNIPRESENT it will
no longer calculate the state from its last change, but from when
the DS was seen in the parent, "DS Publish". If the time was not set,
default to next key event of an hour.

Similarly for moving the DS state to HIDDEN, the time to wait will
be derived from the "DS Delete" time, not from when the DS state
last changed.

(cherry picked from commit c8205bfa0e838c7dae35a24d526e4ccd00614f85)

diff --git a/CHANGES b/CHANGES
index efc1a1c010623f06722fe9667d8db87016f917d8..4fbbfe4000dd9228accec8eae4941c4a91343131 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+5500.  [bug]           Fix (non-)publication of CDS and CDNSKEY records.
+                       [GL #2103]
+
 5499.  [func]          Add '-P ds' and '-D ds' arguments to dnssec-settime.
 
 5497.  [bug]           dig +bufsize=0 failed to disable EDNS as a side

diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh
index 032966a92a41f296ed7132bc4c0e0b85e802c144..ab27405455b66213f82524696abea0bd77d607e5 100644 (file)
--- a/bin/tests/system/kasp/tests.sh
+++ b/bin/tests/system/kasp/tests.sh
@@ -2701,8 +2701,9 @@ check_next_key_event 43800
 set_zone "step3.enable-dnssec.autosign"
 set_policy "enable-dnssec" "1" "300"
 set_server "ns3" "10.53.0.3"
-# All signatures should be omnipresent.
+# All signatures should be omnipresent, so the DS can be submitted.
 set_keystate "KEY1" "STATE_ZRRSIG" "omnipresent"
+set_keystate "KEY1" "STATE_DS"     "rumoured"
 
 # Various signing policy checks.
 check_keys
@@ -2723,7 +2724,6 @@ dnssec_verify
 
 # The DS can be introduced. We ignore any parent registration delay, so set
 # the DS publish time to now.
-set_keystate "KEY1" "STATE_DS" "rumoured"
 rndc_checkds "$SERVER" "$DIR" KEY1 "now" "published" "$ZONE"
 # Next key event is when the DS can move to the OMNIPRESENT state.  This occurs
 # when the parent propagation delay have passed, plus the DS TTL and retire
@@ -3144,6 +3144,9 @@ set_server "ns3" "10.53.0.3"
 # Check keys before we tell named that we saw the DS has been replaced.
 set_keystate "KEY3" "STATE_DNSKEY" "omnipresent"
 set_keystate "KEY3" "STATE_KRRSIG" "omnipresent"
+# The old DS (KEY1) can be withdrawn and the new DS (KEY3) can be introduced.
+set_keystate "KEY1" "STATE_DS"     "unretentive"
+set_keystate "KEY3" "STATE_DS"     "rumoured"
 
 # Various signing policy checks.
 check_keys
@@ -3167,9 +3170,6 @@ check_apex
 check_subdomain
 dnssec_verify
 
-# The old DS (KEY1) can be withdrawn and the new DS (KEY3) can be introduced.
-set_keystate "KEY1" "STATE_DS"     "unretentive"
-set_keystate "KEY3" "STATE_DS"     "rumoured"
 # We ignore any parent registration delay, so set the DS publish time to now.
 rndc_checkds "$SERVER" "$DIR" KEY1 "now" "withdrawn" "$ZONE"
 rndc_checkds "$SERVER" "$DIR" KEY3 "now" "published"  "$ZONE"
@@ -3389,6 +3389,9 @@ set_keystate "KEY1" "STATE_ZRRSIG" "unretentive"
 set_keystate "KEY2" "STATE_DNSKEY" "omnipresent"
 set_keystate "KEY2" "STATE_KRRSIG" "omnipresent"
 set_keystate "KEY2" "STATE_ZRRSIG" "rumoured"
+# The old DS (KEY1) can be withdrawn and the new DS (KEY2) can be introduced.
+set_keystate "KEY1" "STATE_DS"     "unretentive"
+set_keystate "KEY2" "STATE_DS"     "rumoured"
 
 # Various signing policy checks.
 check_keys
@@ -3419,9 +3422,6 @@ set_zonesigning  "KEY1" "no"
 set_zonesigning  "KEY2" "yes"
 dnssec_verify
 
-# The old DS (KEY1) can be withdrawn and the new DS (KEY2) can be introduced.
-set_keystate "KEY1" "STATE_DS"     "unretentive"
-set_keystate "KEY2" "STATE_DS"     "rumoured"
 # We ignore any parent registration delay, so set the DS publish time to now.
 rndc_checkds "$SERVER" "$DIR" KEY1 "now" "withdrawn" "$ZONE"
 rndc_checkds "$SERVER" "$DIR" KEY2 "now" "published"  "$ZONE"
@@ -3707,6 +3707,9 @@ set_zonesigning  "KEY2" "yes"
 set_keystate     "KEY2" "STATE_DNSKEY" "omnipresent"
 set_keystate     "KEY2" "STATE_KRRSIG" "omnipresent"
 set_keystate     "KEY2" "STATE_ZRRSIG" "rumoured"
+# The old DS (KEY1) can be withdrawn and the new DS (KEY2) can be introduced.
+set_keystate     "KEY1" "STATE_DS" "unretentive"
+set_keystate     "KEY2" "STATE_DS" "rumoured"
 
 # Various signing policy checks.
 check_keys
@@ -3737,9 +3740,6 @@ set_zonesigning  "KEY1" "no"
 set_zonesigning  "KEY2" "yes"
 dnssec_verify
 
-# The old DS (KEY1) can be withdrawn and the new DS (KEY2) can be introduced.
-set_keystate     "KEY1" "STATE_DS" "unretentive"
-set_keystate     "KEY2" "STATE_DS" "rumoured"
 # We ignore any parent registration delay, so set the DS publish time to now.
 rndc_checkds "$SERVER" "$DIR" KEY1 "now" "withdrawn" "$ZONE"
 rndc_checkds "$SERVER" "$DIR" KEY2 "now" "published" "$ZONE"
@@ -4710,9 +4710,6 @@ check_apex
 check_subdomain
 dnssec_verify
 
-# It is time to swap the DS.
-set_keystate "KEY1" "STATE_DS"     "unretentive"
-set_keystate "KEY3" "STATE_DS"     "rumoured"
 # Tell named we "saw" the parent swap the DS and see if the next key event is
 # scheduled at the correct time.
 rndc_checkds "$SERVER" "$DIR" KEY1 "now" "withdrawn" "$ZONE"
@@ -5033,6 +5030,9 @@ set_server "ns6" "10.53.0.6"
 # The ECDSAP256SHA256 key is introducing. The DNSKEY RRset and all signatures
 # are now omnipresent, so the DS can be introduced.
 set_keystate "KEY2" "STATE_ZRRSIG" "omnipresent"
+# The old DS (KEY1) can be withdrawn and the new DS (KEY2) can be introduced.
+set_keystate "KEY1" "STATE_DS"     "unretentive"
+set_keystate "KEY2" "STATE_DS"     "rumoured"
 
 # Various signing policy checks.
 check_keys
@@ -5060,9 +5060,6 @@ check_apex
 check_subdomain
 dnssec_verify
 
-# The old DS (KEY1) can be withdrawn and the new DS (KEY2) can be introduced.
-set_keystate "KEY1" "STATE_DS"     "unretentive"
-set_keystate "KEY2" "STATE_DS"     "rumoured"
 # We ignore any parent registration delay, so set the DS publish time to now.
 rndc_checkds "$SERVER" "$DIR" KEY1 "now" "withdrawn" "$ZONE"
 rndc_checkds "$SERVER" "$DIR" KEY2 "now" "published" "$ZONE"

diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c
index 1a24786c3956d40835ac043a9bf9fa8d70b154b2..635dafe69d9f709312468f26ebdb7a232c219a72 100644 (file)
--- a/lib/dns/keymgr.c
+++ b/lib/dns/keymgr.c
@@ -955,15 +955,10 @@ keymgr_have_rrsig(dns_dnsseckeylist_t *keyring, dns_dnsseckey_t *key, int type,
  * 2. Double-KSK rollover method (in case of a KSK)
  *    - First introduce the DNSKEY record, as well as the KRRSIG records.
  *    - Only if the DNSKEY record is OMNIPRESENT, suggest to introduce the DS.
- *
- * Also check the DS Publish or Delete times, to see if the DS record
- * already reached the parent.
  */
 static bool
 keymgr_policy_approval(dns_dnsseckeylist_t *keyring, dns_dnsseckey_t *key,
-                      int type, dst_key_state_t next, isc_stdtime_t now) {
-       isc_result_t ret;
-       isc_stdtime_t dstime;
+                      int type, dst_key_state_t next) {
        dst_key_state_t dnskeystate = HIDDEN;
        dst_key_state_t ksk_present[4] = { OMNIPRESENT, NA, OMNIPRESENT,
                                           OMNIPRESENT };
@@ -975,7 +970,7 @@ keymgr_policy_approval(dns_dnsseckeylist_t *keyring, dns_dnsseckey_t *key,
        dst_key_state_t ksk_retired[4] = { UNRETENTIVE, NA, NA, OMNIPRESENT };
        dst_key_state_t na[4] = { NA, NA, NA, NA }; /* successor n/a */
 
-       if (next != RUMOURED && next != UNRETENTIVE) {
+       if (next != RUMOURED) {
                /*
                 * Local policy only adds an extra barrier on transitions to
                 * the RUMOURED and UNRETENTIVE states.
@@ -988,9 +983,6 @@ keymgr_policy_approval(dns_dnsseckeylist_t *keyring, dns_dnsseckey_t *key,
                /* No restrictions. */
                return (true);
        case DST_KEY_ZRRSIG:
-               if (next != RUMOURED) {
-                       return (true);
-               }
                /* Make sure the DNSKEY record is OMNIPRESENT. */
                (void)dst_key_getstate(key->key, DST_KEY_DNSKEY, &dnskeystate);
                if (dnskeystate == OMNIPRESENT) {
@@ -1011,35 +1003,13 @@ keymgr_policy_approval(dns_dnsseckeylist_t *keyring, dns_dnsseckey_t *key,
                                  keyring, key, type, next, ksk_retired,
                                  ksk_rumoured, true, true)));
        case DST_KEY_KRRSIG:
-               if (next != RUMOURED) {
-                       return (true);
-               }
                /* Only introduce if the DNSKEY is also introduced. */
                (void)dst_key_getstate(key->key, DST_KEY_DNSKEY, &dnskeystate);
                return (dnskeystate != HIDDEN);
        case DST_KEY_DS:
-               if (next == RUMOURED) {
-                       /* Make sure the DNSKEY record is OMNIPRESENT. */
-                       (void)dst_key_getstate(key->key, DST_KEY_DNSKEY,
-                                              &dnskeystate);
-                       if (dnskeystate != OMNIPRESENT) {
-                               return (false);
-                       }
-                       /* 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) {
-                               return (false);
-                       }
-               } else if (next == UNRETENTIVE) {
-                       /* Make sure DS has been withdrawn from the parent. */
-                       ret = dst_key_gettime(key->key, DST_TIME_DSDELETE,
-                                             &dstime);
-                       if (ret != ISC_R_SUCCESS || dstime > now) {
-                               return (false);
-                       }
-               }
-               return (true);
+               /* Make sure the DNSKEY record is OMNIPRESENT. */
+               (void)dst_key_getstate(key->key, DST_KEY_DNSKEY, &dnskeystate);
+               return (dnskeystate == OMNIPRESENT);
        default:
                return (false);
        }
@@ -1110,7 +1080,7 @@ keymgr_transition_time(dns_dnsseckey_t *key, int type,
                       dst_key_state_t next_state, dns_kasp_t *kasp,
                       isc_stdtime_t now, isc_stdtime_t *when) {
        isc_result_t ret;
-       isc_stdtime_t lastchange, nexttime = now;
+       isc_stdtime_t lastchange, dstime, nexttime = now;
 
        /*
         * No need to wait if we move things into an uncertain state.
@@ -1208,30 +1178,52 @@ keymgr_transition_time(dns_dnsseckey_t *key, int type,
                break;
        case DST_KEY_DS:
                switch (next_state) {
+               /*
+                * RFC 7583: The successor DS record is published in
+                * the parent zone and after the registration delay
+                * (Dreg), the time taken after the DS record has been
+                * submitted to the parent zone manager for it to be
+                * placed in the zone.  Key N (the predecessor) must
+                * remain in the zone until any caches that contain a
+                * copy of the DS RRset have a copy containing the new
+                * DS record. This interval is the retire interval
+                * (Iret), given by:
+                *
+                *      Iret = DprpP + TTLds
+                *
+                * 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:
-                       /*
-                        * RFC 7583: The successor DS record is published in
-                        * the parent zone and after the registration delay
-                        * (Dreg), the time taken after the DS record has been
-                        * submitted to the parent zone manager for it to be
-                        * placed in the zone.  Key N (the predecessor) must
-                        * remain in the zone until any caches that contain a
-                        * copy of the DS RRset have a copy containing the new
-                        * DS record. This interval is the retire interval
-                        * (Iret), given by:
-                        *
-                        *      Iret = DprpP + TTLds
-                        *
-                        * This translates to:
-                        *
-                        *      parent-propagation-delay + parent-ds-ttl.
-                        *
-                        * We will also add the retire-safety interval.
-                        */
-                       nexttime = lastchange + dns_kasp_dsttl(kasp) +
-                                  dns_kasp_parentpropagationdelay(kasp) +
-                                  dns_kasp_retiresafety(kasp);
+                       /* Make sure DS has been withdrawn from the parent. */
+                       ret = dst_key_gettime(key->key, DST_TIME_DSDELETE,
+                                             &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;
                default:
                        nexttime = now;
@@ -1319,7 +1311,7 @@ transition:
 
                        /* Is the transition allowed according to policy? */
                        if (!keymgr_policy_approval(keyring, dkey, i,
-                                                   next_state, now)) {
+                                                   next_state)) {
                                /* No, please respect rollover methods. */
                                isc_log_write(
                                        dns_lctx, DNS_LOGCATEGORY_DNSSEC,