]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
dnssec: do not publish CDS records when -Psync is in the future
authorMark Andrews <marka@isc.org>
Tue, 21 Jan 2020 23:04:16 +0000 (23:04 +0000)
committerMark Andrews <marka@isc.org>
Tue, 21 Jan 2020 23:04:16 +0000 (23:04 +0000)
This is a bug I encountered when trying to schedule an algorithm
rollover. My plan, for a zone whose maximum TTL is 48h, was to sign
with the new algorithm and schedule a change of CDS records for more
than 48 hours in the future, roughly like this:

    $ dnssec-keygen -a 13 -fk -Psync now+50h $zone
    $ dnssec-keygen -a 13 $zone
    $ dnssec-settime -Dsync now+50h $zone_ksk_old

However the algorithm 13 CDS was published immediately, which could
have made the zone bogus.

To reveal the bug using the `smartsign` test, this change just adds a
KSK with all its times in the future, so it should not affect the
existing checks at all. But the final check (that there are no CDS or
CDSNSKEY records after -Dsync) fails with the old `syncpublish()`
logic, because the future key's sync records appear early. With the
new `syncpublish()` logic the future key does not affect the test, as
expected, and it now passes.

(cherry picked from commit 4227b7969b27c50cf6f45e8d5a776edab74b097b)

CHANGES
bin/tests/system/smartsign/tests.sh
lib/dns/dnssec.c

diff --git a/CHANGES b/CHANGES
index 0d0d636d7923cf15036052fffd93180f91c0af52..1e43cf83050c358a5082a2506088f9c606b89742 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+5348.  [bug]           dnssec-settime -Psync was not being honoured.
+                       [GL !2893]
+
 5339.  [bug]           With some libmaxminddb versions, named could erroneously
                        match an IP address not belonging to any subnet defined
                        in a given GeoIP2 database to one of the existing
index e1a85d877acffe8a98028bfeec867f85f37aa9eb..73d0cb155e21c8cb5e7a3b452b144f04af892ec4 100644 (file)
@@ -55,6 +55,9 @@ cksk4=`$REVOKE $cksk3`
 echo_i "setting up sync key"
 cksk5=`$KEYGEN -q -a rsasha1 -fk -P now+1mo -A now+1mo -Psync now $czone`
 
+echo_i "and future sync key"
+cksk6=`$KEYGEN -q -a rsasha1 -fk -P now+1mo -A now+1mo -Psync now+1mo $czone`
+
 echo_i "generating parent keys"
 pzsk=`$KEYGEN -q -a rsasha1 $pzone`
 pksk=`$KEYGEN -q -a rsasha1 -fk $pzone`
@@ -348,6 +351,7 @@ awk 'BEGIN { r=1 } $2 == "CDS" { r=0 } END { exit r }' $cfile.signed || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
+# this also checks that the future sync record is not yet published
 echo_i "checking sync record deletion"
 ret=0
 $SETTIME -P now -A now -Dsync now ${cksk5} > /dev/null
index 81aa91f27ecc90c413443537b1c30f759f770569..d10e274e8c6413b79240309082a09a8b9de6b0c0 100644 (file)
@@ -650,6 +650,7 @@ syncpublish(dst_key_t *key, isc_stdtime_t now) {
        isc_result_t result;
        isc_stdtime_t when;
        int major, minor;
+       bool publish;
 
        /*
         * Is this an old-style key?
@@ -663,16 +664,16 @@ syncpublish(dst_key_t *key, isc_stdtime_t now) {
        if (major == 1 && minor <= 2)
                return (false);
 
+       publish = false;
        result = dst_key_gettime(key, DST_TIME_SYNCPUBLISH, &when);
-       if (result != ISC_R_SUCCESS)
-               return (false);
-
+       if (result == ISC_R_SUCCESS && when <= now) {
+               publish = true;
+       }
        result = dst_key_gettime(key, DST_TIME_SYNCDELETE, &when);
-       if (result != ISC_R_SUCCESS)
-               return (true);
-       if (when <= now)
-               return (false);
-       return (true);
+       if (result == ISC_R_SUCCESS && when < now) {
+               publish = false;
+       }
+       return (publish);
 }
 
 /*%<