From: Matthijs Mekking Date: Thu, 6 Feb 2020 07:57:13 +0000 (+0100) Subject: Fix kasp bug new KSK on restart [#1593] X-Git-Tag: v9.16.0~26^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b378d0371f5c15f8dd29c0ad6efbb87b3cb62ae5;p=thirdparty%2Fbind9.git Fix kasp bug new KSK on restart [#1593] When you do a restart or reconfig of named, or rndc loadkeys, this triggers the key manager to run. The key manager will check if new keys need to be created. If there is an active key, and key rollover is scheduled far enough away, no new key needs to be created. However, there was a bug that when you just start to sign your zone, it takes a while before the KSK becomes an active key. An active KSK has its DS submitted or published, but before the key manager allows that, the DNSKEY needs to be omnipresent. If you restart named or rndc loadkeys in quick succession when you just started to sign your zone, new keys will be created because the KSK is not yet considered active. Fix is to check for introducing as well as active keys. These keys all have in common that their goal is to become omnipresent. --- diff --git a/CHANGES b/CHANGES index 4fd06eefae8..52d2e1b570f 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,9 @@ +5354. [bug] dnssec-policy created new KSK keys when zone is in + initial stage of signing (the DS is not yet in + rumoured or omnipresent state). Fix by checking + key goals rather than active state when determining + new keys are needed. [GL #1593] + 5353. [doc] Document port and dscp parameters in forwarders configuration option. [GL !914] diff --git a/bin/tests/system/kasp/ns3/named.conf.in b/bin/tests/system/kasp/ns3/named.conf.in index c9ae05894b5..2f78aa81465 100644 --- a/bin/tests/system/kasp/ns3/named.conf.in +++ b/bin/tests/system/kasp/ns3/named.conf.in @@ -107,6 +107,16 @@ zone "pregenerated.kasp" { dnssec-policy "rsasha1"; }; +/* + * A configured dnssec-policy with one rumoured key. + * Bugfix case for GL #1593. + */ +zone "rumoured.kasp" { + type master; + file "rumoured.kasp.db"; + dnssec-policy "rsasha1"; +}; + /* * Different algorithms. */ diff --git a/bin/tests/system/kasp/ns3/setup.sh b/bin/tests/system/kasp/ns3/setup.sh index 3884c2132a4..d5c08bdfced 100644 --- a/bin/tests/system/kasp/ns3/setup.sh +++ b/bin/tests/system/kasp/ns3/setup.sh @@ -43,7 +43,7 @@ U="UNRETENTIVE" # Set up zones that will be initially signed. # for zn in default rsasha1 dnssec-keygen some-keys legacy-keys pregenerated \ - rsasha1-nsec3 rsasha256 rsasha512 ecdsa256 ecdsa384 inherit + rumoured rsasha1-nsec3 rsasha256 rsasha512 ecdsa256 ecdsa384 inherit do setup "${zn}.kasp" cp template.db.in "$zonefile" @@ -72,6 +72,16 @@ zone="pregenerated.kasp" $KEYGEN -k rsasha1 -l policies/kasp.conf $zone > keygen.out.$zone.1 2>&1 $KEYGEN -k rsasha1 -l policies/kasp.conf $zone > keygen.out.$zone.2 2>&1 +zone="rumoured.kasp" +Tpub="now" +Tact="now+1d" +KSK=$($KEYGEN -a RSASHA1 -f KSK -L 1234 $zone 2> keygen.out.$zone.1) +ZSK1=$($KEYGEN -a RSASHA1 -b 2000 -L 1234 $zone 2> keygen.out.$zone.2) +ZSK2=$($KEYGEN -a RSASHA1 -L 1234 $zone 2> keygen.out.$zone.3) +$SETTIME -s -P $Tpub -A $Tact -g $O -k $R $Tpub -r $R $Tpub -d $H $Tpub "$KSK" > settime.out.$zone.1 2>&1 +$SETTIME -s -P $Tpub -A $Tact -g $O -k $R $Tpub -z $R $Tpub "$ZSK1" > settime.out.$zone.2 2>&1 +$SETTIME -s -P $Tpub -A $Tact -g $O -k $R $Tpub -z $R $Tpub "$ZSK2" > settime.out.$zone.2 2>&1 + # # Set up zones that are already signed. # diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh index 7bcd68df906..cbeb9212809 100644 --- a/bin/tests/system/kasp/tests.sh +++ b/bin/tests/system/kasp/tests.sh @@ -1056,6 +1056,17 @@ check_apex check_subdomain dnssec_verify +# +# Zone: rumoured.kasp. +# +# There are three keys in rumoured state. +zone_properties "ns3" "rumoured.kasp" "rsasha1" "1234" "3" "10.53.0.3" +# key_properties, key_timings and key_states same as above. +check_keys +check_apex +check_subdomain +dnssec_verify + # # Zone: secondary.kasp. # diff --git a/lib/dns/dst_api.c b/lib/dns/dst_api.c index 14cff047deb..31594a49a7e 100644 --- a/lib/dns/dst_api.c +++ b/lib/dns/dst_api.c @@ -2468,7 +2468,6 @@ dst_key_is_active(dst_key_t *key, isc_stdtime_t now) return ds_ok && zrrsig_ok && time_ok && !inactive; } - bool dst_key_is_signing(dst_key_t *key, int role, isc_stdtime_t now, isc_stdtime_t *active) { @@ -2582,6 +2581,19 @@ dst_key_is_removed(dst_key_t *key, isc_stdtime_t now, isc_stdtime_t *remove) return state_ok && time_ok; } +dst_key_state_t +dst_key_goal(dst_key_t *key) +{ + dst_key_state_t state; + isc_result_t result; + + result = dst_key_getstate(key, DST_KEY_GOAL, &state); + if (result == ISC_R_SUCCESS) { + return state; + } + return DST_KEY_STATE_HIDDEN; +} + void dst_key_copy_metadata(dst_key_t *to, dst_key_t *from) { diff --git a/lib/dns/include/dst/dst.h b/lib/dns/include/dst/dst.h index afa20686239..44e60e6b129 100644 --- a/lib/dns/include/dst/dst.h +++ b/lib/dns/include/dst/dst.h @@ -1166,6 +1166,18 @@ dst_key_is_removed(dst_key_t *key, isc_stdtime_t now, isc_stdtime_t *remove); * 'key' to be valid. */ +dst_key_state_t +dst_key_goal(dst_key_t *key); +/*%< + * Get the key goal. Should be OMNIPRESENT or HIDDEN. + * This can be used to determine if the key is being introduced or + * is on its way out. + * + * Requires: + * 'key' to be valid. + */ + + void dst_key_copy_metadata(dst_key_t *to, dst_key_t *from); /*%< diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c index 61bb3182e7e..43d95f08751 100644 --- a/lib/dns/keymgr.c +++ b/lib/dns/keymgr.c @@ -1358,7 +1358,7 @@ dns_keymgr_run(const dns_name_t *origin, dns_rdataclass_t rdclass, lifetime); } - if (dst_key_is_active(dkey->key, now)) { + if (dst_key_goal(dkey->key) == OMNIPRESENT) { if (active_key != NULL) { /* * Multiple signing keys match diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in index bd7d8618453..4df371a56fe 100644 --- a/lib/dns/win32/libdns.def.in +++ b/lib/dns/win32/libdns.def.in @@ -1432,6 +1432,7 @@ dst_key_getprivateformat dst_key_getstate dst_key_gettime dst_key_getttl +dst_key_goal dst_key_id dst_key_is_active dst_key_is_published