From 82f72ae2497ecea4966189cd2db02458a20dc07a Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Wed, 17 Mar 2021 15:57:34 +0100 Subject: [PATCH 1/2] Rekey immediately after rndc checkds/rollover Call 'dns_zone_rekey' after a 'rndc dnssec -checkds' or 'rndc dnssec -rollover' command is received, because such a command may influence the next key event. Updating the keys immediately avoids unnecessary rollover delays. The kasp system test no longer needs to call 'rndc loadkeys' after a 'rndc dnssec -checkds' or 'rndc dnssec -rollover' command. --- CHANGES | 4 ++++ bin/named/server.c | 12 ++++++++++++ bin/tests/system/kasp.sh | 25 ------------------------- doc/notes/notes-current.rst | 4 ++++ 4 files changed, 20 insertions(+), 25 deletions(-) diff --git a/CHANGES b/CHANGES index 313883173b..765a297a8c 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5607. [bug] Rekey after 'rndc dnssec -checkds' or 'rndc dnssec + -rollover' command is received, because such a command + may influence the next key event. [GL #2488] + 5606. [bug] CDS/CDNSKEY DELETE records were not removed when a zone transitioned from secure to insecure. "named-checkzone" should not complain if such records exist in an diff --git a/bin/named/server.c b/bin/named/server.c index ef05daec98..8ec8b4343a 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -15122,6 +15122,12 @@ named_server_dnssec(named_server_t *server, isc_lex_t *lex, switch (result) { case ISC_R_SUCCESS: + /* + * Rekey after checkds command because the next key + * event may have changed. + */ + dns_zone_rekey(zone, false); + if (use_keyid) { char tagbuf[6]; snprintf(tagbuf, sizeof(tagbuf), "%u", keyid); @@ -15166,6 +15172,12 @@ named_server_dnssec(named_server_t *server, isc_lex_t *lex, switch (result) { case ISC_R_SUCCESS: + /* + * Rekey after rollover command because the next key + * event may have changed. + */ + dns_zone_rekey(zone, false); + if (use_keyid) { char tagbuf[6]; snprintf(tagbuf, sizeof(tagbuf), "%u", keyid); diff --git a/bin/tests/system/kasp.sh b/bin/tests/system/kasp.sh index 863facc648..a619c0e6a9 100644 --- a/bin/tests/system/kasp.sh +++ b/bin/tests/system/kasp.sh @@ -1037,25 +1037,6 @@ check_cdslog() { status=$((status+ret)) } -# -# Utility to call after 'rndc dnssec -checkds|-rollover'. -# -_loadkeys_on() { - _server=$1 - _dir=$2 - _zone=$3 - - nextpart $_dir/named.run > /dev/null - _rndccmd $_server loadkeys $_zone in $_view > rndc.dnssec.loadkeys.out.$_zone.$n - - if [ "${DYNAMIC}" = "yes" ]; then - wait_for_log 20 "zone ${_zone}/IN: next key event" $_dir/named.run || return 1 - else - # inline-signing zone adds "(signed)" - wait_for_log 20 "zone ${_zone}/IN (signed): next key event" $_dir/named.run || return 1 - fi -} - # Tell named that the DS for the key in given zone has been seen in the # parent (this does not actually has to be true, we just issue the command # to make named believe it can continue with the rollover). @@ -1085,10 +1066,6 @@ rndc_checkds() { _rndccmd $_server dnssec -checkds $_keycmd $_whencmd $_what $_zone in $_view > rndc.dnssec.checkds.out.$_zone.$n || _log_error "rndc dnssec -checkds${_keycmd}${_whencmd} ${_what} zone ${_zone} failed" - if [ "$ret" -eq 0 ]; then - _loadkeys_on $_server $_dir $_zone || _log_error "loadkeys zone ${_zone} failed ($n)" - fi - test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) } @@ -1113,8 +1090,6 @@ rndc_rollover() { _rndccmd $_server dnssec -rollover -key $_keyid $_whencmd $_zone in $_view > rndc.dnssec.rollover.out.$_zone.$n || _log_error "rndc dnssec -rollover (key ${_keyid} when ${_when}) zone ${_zone} failed" - _loadkeys_on $_server $_dir $_zone || _log_error "loadkeys zone ${_zone} failed ($n)" - test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) } diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 2ebdff3f27..96d83f5363 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -39,6 +39,10 @@ Feature Changes Bug Fixes ~~~~~~~~~ +- When calling ``rndc dnssec -rollover`` or ``rndc checkds -checkds``, + ``named`` now updates the keys immediately, avoiding unnecessary rollover + delays. [#2488] + - Dynamic zones with ``dnssec-policy`` that were frozen could not be thawed. This has been fixed. [GL #2523] From 82d667e1d5a753752199c007fa2b81002ee03a7e Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Thu, 18 Mar 2021 10:47:56 +0100 Subject: [PATCH 2/2] Fix some intermittent kasp failures When calling "rndc dnssec -checkds", it may take some milliseconds before the appropriate changes have been written to the state file. Add retry_quiet mechanisms to allow the write operation to finish. Also retry_quiet the check for the next key event. A "rndc dnssec" command may trigger a zone_rekey event and this will write out a new "next key event" log line, but it may take a bit longer than than expected in the tests. --- bin/tests/system/kasp/tests.sh | 44 ++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh index 05025e5af2..6e5a906d92 100644 --- a/bin/tests/system/kasp/tests.sh +++ b/bin/tests/system/kasp/tests.sh @@ -473,17 +473,24 @@ dnssec_verify basefile=$(key_get KEY1 BASEFILE) +_wait_for_metadata() { + _expr=$1 + _file=$2 + grep "$_expr" $_file > /dev/null || return 1 + return 0 +} + n=$((n+1)) echo_i "checkds publish correctly sets DSPublish for zone $ZONE ($n)" rndc_checkds "$SERVER" "$DIR" "-" "20190102121314" "published" "$ZONE" -grep "DSPublish: 20190102121314" "${basefile}.state" > /dev/null || log_error "DSPublish not set in ${basefile}" +retry_quiet 3 _wait_for_metadata "DSPublish: 20190102121314" "${basefile}.state" || log_error "bad DSPublish in ${basefile}.state" test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) n=$((n+1)) echo_i "checkds withdraw correctly sets DSRemoved for zone $ZONE ($n)" rndc_checkds "$SERVER" "$DIR" "-" "20200102121314" "withdrawn" "$ZONE" -grep "DSRemoved: 20200102121314" "${basefile}.state" > /dev/null || log_error "DSRemoved not set in ${basefile}" +retry_quiet 3 _wait_for_metadata "DSRemoved: 20200102121314" "${basefile}.state" || log_error "bad DSRemoved in ${basefile}.state" test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) @@ -551,8 +558,8 @@ status=$((status+ret)) n=$((n+1)) echo_i "checkds withdrawn does not set DSRemoved for zone $ZONE (multiple KSK) ($n)" rndc_checkds "$SERVER" "$DIR" "-" "20190102121314" "withdrawn" "$ZONE" -grep "DSRemoved:" "${basefile1}.state" > /dev/null && log_error "DSPublish incorrectly set in ${basefile1}" -grep "DSRemoved:" "${basefile2}.state" > /dev/null && log_error "DSPublish incorrectly set in ${basefile2}" +grep "DSRemoved:" "${basefile1}.state" > /dev/null && log_error "DSRemoved incorrectly set in ${basefile1}" +grep "DSRemoved:" "${basefile2}.state" > /dev/null && log_error "DSRemoved incorrectly set in ${basefile2}" test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) @@ -575,7 +582,7 @@ status=$((status+ret)) n=$((n+1)) echo_i "checkds published -key correctly sets DSPublish for key $(key_get KEY1 ID) zone $ZONE (multiple KSK) ($n)" rndc_checkds "$SERVER" "$DIR" KEY1 "20190102121314" "published" "$ZONE" -grep "DSPublish: 20190102121314" "${basefile1}.state" > /dev/null || log_error "DSPublish not set in ${basefile1}" +retry_quiet 3 _wait_for_metadata "DSPublish: 20190102121314" "${basefile1}.state" || log_error "bad DSPublish in ${basefile1}.state" grep "DSPublish:" "${basefile2}.state" > /dev/null && log_error "DSPublish incorrectly set in ${basefile2}" test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) @@ -583,8 +590,8 @@ status=$((status+ret)) n=$((n+1)) echo_i "checkds withdrawn -key correctly sets DSRemoved for key $(key_get KEY2 ID) zone $ZONE (multiple KSK) ($n)" rndc_checkds "$SERVER" "$DIR" KEY2 "20200102121314" "withdrawn" "$ZONE" -grep "DSRemoved: 20200102121314" "${basefile2}.state" > /dev/null || log_error "DSRemoved not set in ${basefile2}" grep "DSRemoved:" "${basefile1}.state" > /dev/null && log_error "DSPublish incorrectly set in ${basefile1}" +retry_quiet 3 _wait_for_metadata "DSRemoved: 20200102121314" "${basefile2}.state" || log_error "bad DSRemoved in ${basefile2}.state" test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) @@ -623,14 +630,14 @@ basefile=$(key_get KEY1 BASEFILE) n=$((n+1)) echo_i "checkds publish correctly sets DSPublish for zone $ZONE ($n)" rndc_checkds "$SERVER" "$DIR" "-" "20190102121314" "published" "$ZONE" -grep "DSPublish: 20190102121314" "${basefile}.state" > /dev/null || log_error "DSPublish not set in ${basefile}" +retry_quiet 3 _wait_for_metadata "DSPublish: 20190102121314" "${basefile}.state" || log_error "bad DSPublish in ${basefile}.state" test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) n=$((n+1)) echo_i "checkds withdraw correctly sets DSRemoved for zone $ZONE ($n)" rndc_checkds "$SERVER" "$DIR" "-" "20200102121314" "withdrawn" "$ZONE" -grep "DSRemoved: 20200102121314" "${basefile}.state" > /dev/null || log_error "DSRemoved not set in ${basefile}" +retry_quiet 3 _wait_for_metadata "DSRemoved: 20200102121314" "${basefile}.state" || log_error "bad DSRemoved in ${basefile}.state" test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) @@ -2023,13 +2030,10 @@ check_apex check_subdomain dnssec_verify -check_next_key_event() { +_check_next_key_event() { _expect=$1 - n=$((n+1)) - echo_i "check next key event for zone ${ZONE} ($n)" - ret=0 - grep "zone ${ZONE}.*: next key event in .* seconds" "${DIR}/named.run" > "keyevent.out.$ZONE.test$n" || log_error "no next key event for zone ${ZONE}" + grep "zone ${ZONE}.*: next key event in .* seconds" "${DIR}/named.run" > "keyevent.out.$ZONE.test$n" || return 1 # Get the latest next key event. if [ "${DYNAMIC}" = "yes" ]; then @@ -2044,11 +2048,21 @@ check_next_key_event() { _expectmin=$((_expect-next_key_event_threshold)) _expectmax=$((_expect+next_key_event_threshold)) - test $_expectmin -le "$_time" || log_error "bad next key event time ${_time} for zone ${ZONE} (expect ${_expect})" - test $_expectmax -ge "$_time" || log_error "bad next key event time ${_time} for zone ${ZONE} (expect ${_expect})" + test $_expectmin -le "$_time" || return 1 + test $_expectmax -ge "$_time" || return 1 + return 0 +} + +check_next_key_event() { + n=$((n+1)) + echo_i "check next key event for zone ${ZONE} ($n)" + ret=0 + + retry_quiet 3 _check_next_key_event $1 || log_error "bad next key event time for zone ${ZONE} (expect ${_expect})" test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) + } # Next key event is when the DNSKEY RRset becomes OMNIPRESENT: DNSKEY TTL plus