diff --git a/CHANGES b/CHANGES index 3c752cc442..2d214f7d91 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +5634. [bug] Don't roll keys when the private key file is offline. + [GL #2596] + 5633. [doc] Inline-signing was incorrectly described as being inherited from the options / view levels and was incorrectly accepted at those levels without effect. diff --git a/bin/named/server.c b/bin/named/server.c index 6342ff31a1..e306fd63b5 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -14935,8 +14935,8 @@ named_server_dnssec(named_server_t *server, isc_lex_t *lex, isc_result_t result = ISC_R_SUCCESS; dns_zone_t *zone = NULL; dns_kasp_t *kasp = NULL; - dns_dnsseckeylist_t keys; - dns_dnsseckey_t *key; + dns_dnsseckeylist_t keys, dnskeys; + dns_dnsseckey_t *key, *key_next = NULL; char *ptr, *zonetext = NULL; const char *msg = NULL; /* variables for -checkds */ @@ -14953,6 +14953,11 @@ named_server_dnssec(named_server_t *server, isc_lex_t *lex, isc_stdtime_t now, when; isc_time_t timenow, timewhen; const char *dir; + dns_name_t *origin; + dns_db_t *db = NULL; + dns_dbnode_t *node = NULL; + dns_dbversion_t *version = NULL; + dns_rdataset_t keyset; /* Skip the command name. */ ptr = next_token(lex, text); @@ -14971,7 +14976,9 @@ named_server_dnssec(named_server_t *server, isc_lex_t *lex, now = isc_time_seconds(&timenow); when = now; + ISC_LIST_INIT(dnskeys); ISC_LIST_INIT(keys); + dns_rdataset_init(&keyset); if (strcasecmp(ptr, "-status") == 0) { status = true; @@ -15084,13 +15091,46 @@ named_server_dnssec(named_server_t *server, isc_lex_t *lex, /* Get DNSSEC keys. */ dir = dns_zone_getkeydirectory(zone); + origin = dns_zone_getorigin(zone); + CHECK(dns_zone_getdb(zone, &db)); + CHECK(dns_db_findnode(db, origin, false, &node)); + dns_db_currentversion(db, &version); + /* Get keys from private key files. */ LOCK(&kasp->lock); - result = dns_dnssec_findmatchingkeys(dns_zone_getorigin(zone), dir, now, + result = dns_dnssec_findmatchingkeys(origin, dir, now, dns_zone_getmctx(zone), &keys); UNLOCK(&kasp->lock); if (result != ISC_R_SUCCESS && result != ISC_R_NOTFOUND) { goto cleanup; } + /* Get public keys (dnskeys). */ + result = dns_db_findrdataset(db, node, version, dns_rdatatype_dnskey, + dns_rdatatype_none, 0, &keyset, NULL); + if (result == ISC_R_SUCCESS) { + CHECK(dns_dnssec_keylistfromrdataset( + origin, dir, dns_zone_getmctx(zone), &keyset, NULL, + NULL, false, false, &dnskeys)); + } else if (result != ISC_R_NOTFOUND) { + CHECK(result); + } + /* Add new 'dnskeys' to 'keys'. */ + for (dns_dnsseckey_t *k1 = ISC_LIST_HEAD(dnskeys); k1 != NULL; + k1 = key_next) { + dns_dnsseckey_t *k2 = NULL; + key_next = ISC_LIST_NEXT(k1, link); + + for (k2 = ISC_LIST_HEAD(keys); k2 != NULL; + k2 = ISC_LIST_NEXT(k2, link)) { + if (dst_key_compare(k1->key, k2->key)) { + break; + } + } + /* No match found, add the new key. */ + if (k2 == NULL) { + ISC_LIST_UNLINK(dnskeys, k1, link); + ISC_LIST_APPEND(keys, k1, link); + } + } if (status) { /* @@ -15210,6 +15250,24 @@ cleanup: (void)putnull(text); } + if (dns_rdataset_isassociated(&keyset)) { + dns_rdataset_disassociate(&keyset); + } + if (node != NULL) { + dns_db_detachnode(db, &node); + } + if (version != NULL) { + dns_db_closeversion(db, &version, false); + } + if (db != NULL) { + dns_db_detach(&db); + } + + while (!ISC_LIST_EMPTY(dnskeys)) { + key = ISC_LIST_HEAD(dnskeys); + ISC_LIST_UNLINK(dnskeys, key, link); + dns_dnsseckey_destroy(dns_zone_getmctx(zone), &key); + } while (!ISC_LIST_EMPTY(keys)) { key = ISC_LIST_HEAD(keys); ISC_LIST_UNLINK(keys, key, link); diff --git a/bin/tests/system/kasp.sh b/bin/tests/system/kasp.sh index 94e395fb4d..d21839a08b 100644 --- a/bin/tests/system/kasp.sh +++ b/bin/tests/system/kasp.sh @@ -59,6 +59,7 @@ VIEW2="4xILSZQnuO1UKubXHkYUsvBRPu8=" # EXPECT_ZRRSIG # EXPECT_KRRSIG # LEGACY +# PRIVATE key_key() { echo "${1}__${2}" @@ -112,6 +113,7 @@ key_clear() { key_set "$1" "EXPECT_ZRRSIG" 'no' key_set "$1" "EXPECT_KRRSIG" 'no' key_set "$1" "LEGACY" 'no' + key_set "$1" "PRIVATE" 'yes' } # Start clear. @@ -303,6 +305,7 @@ check_key() { _dnskey_ttl="$DNSKEY_TTL" _lifetime=$(key_get "$1" LIFETIME) _legacy=$(key_get "$1" LEGACY) + _private=$(key_get "$1" PRIVATE) _published=$(key_get "$1" PUBLISHED) _active=$(key_get "$1" ACTIVE) @@ -341,7 +344,9 @@ check_key() { # Check file existence. [ -s "$KEY_FILE" ] || ret=1 - [ -s "$PRIVATE_FILE" ] || ret=1 + if [ "$_private" = "yes" ]; then + [ -s "$PRIVATE_FILE" ] || ret=1 + fi if [ "$_legacy" = "no" ]; then [ -s "$STATE_FILE" ] || ret=1 fi @@ -352,7 +357,9 @@ check_key() { grep "; Created:" "$KEY_FILE" > "${ZONE}.${KEY_ID}.${_alg_num}.created" || _log_error "mismatch created comment in $KEY_FILE" KEY_CREATED=$(awk '{print $3}' < "${ZONE}.${KEY_ID}.${_alg_num}.created") - grep "Created: ${KEY_CREATED}" "$PRIVATE_FILE" > /dev/null || _log_error "mismatch created in $PRIVATE_FILE" + if [ "$_private" = "yes" ]; then + grep "Created: ${KEY_CREATED}" "$PRIVATE_FILE" > /dev/null || _log_error "mismatch created in $PRIVATE_FILE" + fi if [ "$_legacy" = "no" ]; then grep "Generated: ${KEY_CREATED}" "$STATE_FILE" > /dev/null || _log_error "mismatch generated in $STATE_FILE" fi @@ -363,8 +370,10 @@ check_key() { grep "This is a ${_role2} key, keyid ${_key_id}, for ${_zone}." "$KEY_FILE" > /dev/null || _log_error "mismatch top comment in $KEY_FILE" grep "${_zone}\. ${_dnskey_ttl} IN DNSKEY ${_flags} 3 ${_alg_num}" "$KEY_FILE" > /dev/null || _log_error "mismatch DNSKEY record in $KEY_FILE" # Now check the private key file. - grep "Private-key-format: v1.3" "$PRIVATE_FILE" > /dev/null || _log_error "mismatch private key format in $PRIVATE_FILE" - grep "Algorithm: ${_alg_num} (${_alg_string})" "$PRIVATE_FILE" > /dev/null || _log_error "mismatch algorithm in $PRIVATE_FILE" + if [ "$_private" = "yes" ]; then + grep "Private-key-format: v1.3" "$PRIVATE_FILE" > /dev/null || _log_error "mismatch private key format in $PRIVATE_FILE" + grep "Algorithm: ${_alg_num} (${_alg_string})" "$PRIVATE_FILE" > /dev/null || _log_error "mismatch algorithm in $PRIVATE_FILE" + fi # Now check the key state file. if [ "$_legacy" = "no" ]; then grep "This is the state of key ${_key_id}, for ${_zone}." "$STATE_FILE" > /dev/null || _log_error "mismatch top comment in $STATE_FILE" @@ -444,6 +453,8 @@ check_timingmetadata() { _key_file="${_base_file}.key" _private_file="${_base_file}.private" _state_file="${_base_file}.state" + _legacy=$(key_get "$1" LEGACY) + _private=$(key_get "$1" PRIVATE) _published=$(key_get "$1" PUBLISHED) _syncpublish=$(key_get "$1" SYNCPUBLISH) @@ -459,13 +470,17 @@ check_timingmetadata() { if [ "$_published" = "none" ]; then grep "; Publish:" "${_key_file}" > /dev/null && _log_error "unexpected publish comment in ${_key_file}" - grep "Publish:" "${_private_file}" > /dev/null && _log_error "unexpected publish in ${_private_file}" + if [ "$_private" = "yes" ]; then + grep "Publish:" "${_private_file}" > /dev/null && _log_error "unexpected publish in ${_private_file}" + fi if [ "$_legacy" = "no" ]; then grep "Published: " "${_state_file}" > /dev/null && _log_error "unexpected publish in ${_state_file}" fi else grep "; Publish: $_published" "${_key_file}" > /dev/null || _log_error "mismatch publish comment in ${_key_file} (expected ${_published})" - grep "Publish: $_published" "${_private_file}" > /dev/null || _log_error "mismatch publish in ${_private_file} (expected ${_published})" + if [ "$_private" = "yes" ]; then + grep "Publish: $_published" "${_private_file}" > /dev/null || _log_error "mismatch publish in ${_private_file} (expected ${_published})" + fi if [ "$_legacy" = "no" ]; then grep "Published: $_published" "${_state_file}" > /dev/null || _log_error "mismatch publish in ${_state_file} (expected ${_published})" fi @@ -473,13 +488,17 @@ check_timingmetadata() { if [ "$_syncpublish" = "none" ]; then grep "; SyncPublish:" "${_key_file}" > /dev/null && _log_error "unexpected syncpublish comment in ${_key_file}" - grep "SyncPublish:" "${_private_file}" > /dev/null && _log_error "unexpected syncpublish in ${_private_file}" + if [ "$_private" = "yes" ]; then + grep "SyncPublish:" "${_private_file}" > /dev/null && _log_error "unexpected syncpublish in ${_private_file}" + fi if [ "$_legacy" = "no" ]; then grep "PublishCDS: " "${_state_file}" > /dev/null && _log_error "unexpected syncpublish in ${_state_file}" fi else grep "; SyncPublish: $_syncpublish" "${_key_file}" > /dev/null || _log_error "mismatch syncpublish comment in ${_key_file} (expected ${_syncpublish})" - grep "SyncPublish: $_syncpublish" "${_private_file}" > /dev/null || _log_error "mismatch syncpublish in ${_private_file} (expected ${_syncpublish})" + if [ "$_private" = "yes" ]; then + grep "SyncPublish: $_syncpublish" "${_private_file}" > /dev/null || _log_error "mismatch syncpublish in ${_private_file} (expected ${_syncpublish})" + fi if [ "$_legacy" = "no" ]; then grep "PublishCDS: $_syncpublish" "${_state_file}" > /dev/null || _log_error "mismatch syncpublish in ${_state_file} (expected ${_syncpublish})" fi @@ -487,13 +506,17 @@ check_timingmetadata() { if [ "$_active" = "none" ]; then grep "; Activate:" "${_key_file}" > /dev/null && _log_error "unexpected active comment in ${_key_file}" - grep "Activate:" "${_private_file}" > /dev/null && _log_error "unexpected active in ${_private_file}" + if [ "$_private" = "yes" ]; then + grep "Activate:" "${_private_file}" > /dev/null && _log_error "unexpected active in ${_private_file}" + fi if [ "$_legacy" = "no" ]; then grep "Active: " "${_state_file}" > /dev/null && _log_error "unexpected active in ${_state_file}" fi else grep "; Activate: $_active" "${_key_file}" > /dev/null || _log_error "mismatch active comment in ${_key_file} (expected ${_active})" - grep "Activate: $_active" "${_private_file}" > /dev/null || _log_error "mismatch active in ${_private_file} (expected ${_active})" + if [ "$_private" = "yes" ]; then + grep "Activate: $_active" "${_private_file}" > /dev/null || _log_error "mismatch active in ${_private_file} (expected ${_active})" + fi if [ "$_legacy" = "no" ]; then grep "Active: $_active" "${_state_file}" > /dev/null || _log_error "mismatch active in ${_state_file} (expected ${_active})" fi @@ -501,13 +524,17 @@ check_timingmetadata() { if [ "$_retired" = "none" ]; then grep "; Inactive:" "${_key_file}" > /dev/null && _log_error "unexpected retired comment in ${_key_file}" - grep "Inactive:" "${_private_file}" > /dev/null && _log_error "unexpected retired in ${_private_file}" + if [ "$_private" = "yes" ]; then + grep "Inactive:" "${_private_file}" > /dev/null && _log_error "unexpected retired in ${_private_file}" + fi if [ "$_legacy" = "no" ]; then grep "Retired: " "${_state_file}" > /dev/null && _log_error "unexpected retired in ${_state_file}" fi else grep "; Inactive: $_retired" "${_key_file}" > /dev/null || _log_error "mismatch retired comment in ${_key_file} (expected ${_retired})" - grep "Inactive: $_retired" "${_private_file}" > /dev/null || _log_error "mismatch retired in ${_private_file} (expected ${_retired})" + if [ "$_private" = "yes" ]; then + grep "Inactive: $_retired" "${_private_file}" > /dev/null || _log_error "mismatch retired in ${_private_file} (expected ${_retired})" + fi if [ "$_legacy" = "no" ]; then grep "Retired: $_retired" "${_state_file}" > /dev/null || _log_error "mismatch retired in ${_state_file} (expected ${_retired})" fi @@ -515,13 +542,17 @@ check_timingmetadata() { if [ "$_revoked" = "none" ]; then grep "; Revoke:" "${_key_file}" > /dev/null && _log_error "unexpected revoked comment in ${_key_file}" - grep "Revoke:" "${_private_file}" > /dev/null && _log_error "unexpected revoked in ${_private_file}" + if [ "$_private" = "yes" ]; then + grep "Revoke:" "${_private_file}" > /dev/null && _log_error "unexpected revoked in ${_private_file}" + fi if [ "$_legacy" = "no" ]; then grep "Revoked: " "${_state_file}" > /dev/null && _log_error "unexpected revoked in ${_state_file}" fi else grep "; Revoke: $_revoked" "${_key_file}" > /dev/null || _log_error "mismatch revoked comment in ${_key_file} (expected ${_revoked})" - grep "Revoke: $_revoked" "${_private_file}" > /dev/null || _log_error "mismatch revoked in ${_private_file} (expected ${_revoked})" + if [ "$_private" = "yes" ]; then + grep "Revoke: $_revoked" "${_private_file}" > /dev/null || _log_error "mismatch revoked in ${_private_file} (expected ${_revoked})" + fi if [ "$_legacy" = "no" ]; then grep "Revoked: $_revoked" "${_state_file}" > /dev/null || _log_error "mismatch revoked in ${_state_file} (expected ${_revoked})" fi @@ -529,13 +560,17 @@ check_timingmetadata() { if [ "$_removed" = "none" ]; then grep "; Delete:" "${_key_file}" > /dev/null && _log_error "unexpected removed comment in ${_key_file}" - grep "Delete:" "${_private_file}" > /dev/null && _log_error "unexpected removed in ${_private_file}" + if [ "$_private" = "yes" ]; then + grep "Delete:" "${_private_file}" > /dev/null && _log_error "unexpected removed in ${_private_file}" + fi if [ "$_legacy" = "no" ]; then grep "Removed: " "${_state_file}" > /dev/null && _log_error "unexpected removed in ${_state_file}" fi else grep "; Delete: $_removed" "${_key_file}" > /dev/null || _log_error "mismatch removed comment in ${_key_file} (expected ${_removed})" - grep "Delete: $_removed" "${_private_file}" > /dev/null || _log_error "mismatch removed in ${_private_file} (expected ${_removed})" + if [ "$_private" = "yes" ]; then + grep "Delete: $_removed" "${_private_file}" > /dev/null || _log_error "mismatch removed in ${_private_file} (expected ${_removed})" + fi if [ "$_legacy" = "no" ]; then grep "Removed: $_removed" "${_state_file}" > /dev/null || _log_error "mismatch removed in ${_state_file} (expected ${_removed})" fi @@ -672,7 +707,7 @@ _check_keys() { # Check key files. _ids=$(get_keyids "$DIR" "$ZONE") for _id in $_ids; do - # There are three key files with the same algorithm. + # There are multiple key files with the same algorithm. # Check them until a match is found. ret=0 echo_i "check key id $_id" diff --git a/bin/tests/system/kasp/ns3/named.conf.in b/bin/tests/system/kasp/ns3/named.conf.in index 6e6f7bfa06..a2731099e9 100644 --- a/bin/tests/system/kasp/ns3/named.conf.in +++ b/bin/tests/system/kasp/ns3/named.conf.in @@ -252,6 +252,15 @@ zone "unfresh-sigs.autosign" { dnssec-policy "autosign"; }; +/* + * Zone that has missing private KSK. + */ +zone "ksk-missing.autosign" { + type primary; + file "ksk-missing.autosign.db"; + dnssec-policy "autosign"; +}; + /* * Zone that has missing private ZSK. */ diff --git a/bin/tests/system/kasp/ns3/setup.sh b/bin/tests/system/kasp/ns3/setup.sh index 55e862856c..7e45193438 100644 --- a/bin/tests/system/kasp/ns3/setup.sh +++ b/bin/tests/system/kasp/ns3/setup.sh @@ -194,7 +194,25 @@ private_type_record $zone $DEFAULT_ALGORITHM_NUMBER "$KSK" >> "$infile" private_type_record $zone $DEFAULT_ALGORITHM_NUMBER "$ZSK" >> "$infile" $SIGNER -S -x -s now-1w -e now+1w -o $zone -O full -f $zonefile $infile > signer.out.$zone.1 2>&1 -# These signatures are already expired, and the private ZSK is missing. +# These signatures are still good, but the private KSK is missing. +setup ksk-missing.autosign +T="now-6mo" +ksktimes="-P $T -A $T -P sync $T" +zsktimes="-P $T -A $T" +KSK=$($KEYGEN -a $DEFAULT_ALGORITHM -L 300 -f KSK $ksktimes $zone 2> keygen.out.$zone.1) +ZSK=$($KEYGEN -a $DEFAULT_ALGORITHM -L 300 $zsktimes $zone 2> keygen.out.$zone.2) +$SETTIME -s -g $O -d $O $T -k $O $T -r $O $T "$KSK" > settime.out.$zone.1 2>&1 +$SETTIME -s -g $O -k $O $T -z $O $T "$ZSK" > settime.out.$zone.2 2>&1 +cat template.db.in "${KSK}.key" "${ZSK}.key" > "$infile" +private_type_record $zone $DEFAULT_ALGORITHM_NUMBER "$KSK" >> "$infile" +private_type_record $zone $DEFAULT_ALGORITHM_NUMBER "$ZSK" >> "$infile" +$SIGNER -S -x -s now-1w -e now+1w -o $zone -O full -f $zonefile $infile > signer.out.$zone.1 2>&1 +echo "KSK: yes" >> "${KSK}".state +echo "ZSK: no" >> "${KSK}".state +echo "Lifetime: 63072000" >> "${KSK}".state # PT2Y +rm -f "${KSK}".private + +# These signatures are still good, but the private ZSK is missing. setup zsk-missing.autosign T="now-6mo" ksktimes="-P $T -A $T -P sync $T" @@ -206,7 +224,10 @@ $SETTIME -s -g $O -k $O $T -z $O $T "$ZSK" > settime.out.$zone.2 2>&1 cat template.db.in "${KSK}.key" "${ZSK}.key" > "$infile" private_type_record $zone $DEFAULT_ALGORITHM_NUMBER "$KSK" >> "$infile" private_type_record $zone $DEFAULT_ALGORITHM_NUMBER "$ZSK" >> "$infile" -$SIGNER -PS -x -s now-2w -e now-1mi -o $zone -O full -f $zonefile $infile > signer.out.$zone.1 2>&1 +$SIGNER -S -x -s now-1w -e now+1w -o $zone -O full -f $zonefile $infile > signer.out.$zone.1 2>&1 +echo "KSK: no" >> "${ZSK}".state +echo "ZSK: yes" >> "${ZSK}".state +echo "Lifetime: 31536000" >> "${ZSK}".state # PT1Y rm -f "${ZSK}".private # These signatures are already expired, and the private ZSK is retired. diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh index be49ac7574..d8a96c3d9f 100644 --- a/bin/tests/system/kasp/tests.sh +++ b/bin/tests/system/kasp/tests.sh @@ -325,6 +325,27 @@ retry_quiet 10 update_is_signed "10.0.0.11" "10.0.0.44" || ret=1 test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) +# Move the private key file, a rekey event should not introduce replacement +# keys. +ret=0 +echo_i "test that if private key files are inaccessible this doesn't trigger a rollover ($n)" +basefile=$(key_get KEY1 BASEFILE) +mv "${basefile}.private" "${basefile}.offline" +rndccmd 10.53.0.3 loadkeys "$ZONE" > /dev/null || log_error "rndc loadkeys zone ${ZONE} failed" +wait_for_log 3 "offline, policy default" $DIR/named.run || ret=1 +mv "${basefile}.offline" "${basefile}.private" +test "$ret" -eq 0 || echo_i "failed" +status=$((status+ret)) + +# Nothing has changed. +check_keys +check_dnssecstatus "$SERVER" "$POLICY" "$ZONE" +set_keytimes_csk_policy +check_keytimes +check_apex +check_subdomain +dnssec_verify + # # Zone: dynamic.kasp # @@ -1339,6 +1360,25 @@ check_subdomain dnssec_verify check_rrsig_refresh +# +# Zone: ksk-missing.autosign. +# +set_zone "ksk-missing.autosign" +set_policy "autosign" "2" "300" +set_server "ns3" "10.53.0.3" +# Key properties, timings and states same as above. +# Skip checking the private file, because it is missing. +key_set "KEY1" "PRIVATE" "no" + +check_keys +check_dnssecstatus "$SERVER" "$POLICY" "$ZONE" +check_apex +check_subdomain +dnssec_verify + +# Restore the PRIVATE variable. +key_set "KEY1" "PRIVATE" "yes" + # # Zone: zsk-missing.autosign. # @@ -1346,7 +1386,25 @@ set_zone "zsk-missing.autosign" set_policy "autosign" "2" "300" set_server "ns3" "10.53.0.3" # Key properties, timings and states same as above. -# TODO. +# Skip checking the private file, because it is missing. +key_set "KEY2" "PRIVATE" "no" + +check_keys +check_dnssecstatus "$SERVER" "$POLICY" "$ZONE" +# For the apex, we expect the SOA to be signed with the KSK because the ZSK is +# offline. Temporary treat KEY1 as a zone signing key too. +set_keyrole "KEY1" "csk" +set_zonesigning "KEY1" "yes" +set_zonesigning "KEY2" "no" +check_apex +set_keyrole "KEY1" "ksk" +set_zonesigning "KEY1" "no" +set_zonesigning "KEY2" "yes" +check_subdomain +dnssec_verify + +# Restore the PRIVATE variable. +key_set "KEY2" "PRIVATE" "yes" # # Zone: zsk-retired.autosign. diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index b9b48a46c4..aa3cdde892 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -82,3 +82,6 @@ Bug Fixes between the new keys too. :gl:`#2628` - Update ZONEMD to match RFC 8976. :gl:`#2658` + +- With ``dnssec-policy```, don't roll keys if the private key file is offline. + :gl:`#2596` diff --git a/lib/bind9/check.c b/lib/bind9/check.c index ee00d2482a..464797e8a3 100644 --- a/lib/bind9/check.c +++ b/lib/bind9/check.c @@ -890,6 +890,9 @@ kasp_name_allowed(const cfg_listelt_t *element) { if (strcmp("none", name) == 0) { return (false); } + if (strcmp("insecure", name) == 0) { + return (false); + } if (strcmp("default", name) == 0) { return (false); } diff --git a/lib/dns/dnssec.c b/lib/dns/dnssec.c index 97316b7d33..f2a107b5fd 100644 --- a/lib/dns/dnssec.c +++ b/lib/dns/dnssec.c @@ -1657,7 +1657,7 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, const char *directory, dns_dnsseckeylist_t *keylist) { dns_rdataset_t keys; dns_rdata_t rdata = DNS_RDATA_INIT; - dst_key_t *pubkey = NULL, *privkey = NULL; + dst_key_t *dnskey = NULL, *pubkey = NULL, *privkey = NULL; isc_result_t result; REQUIRE(keyset != NULL && dns_rdataset_isassociated(keyset)); @@ -1680,27 +1680,38 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, const char *directory, goto skip; } - RETERR(dns_dnssec_keyfromrdata(origin, &rdata, mctx, &pubkey)); - dst_key_setttl(pubkey, keys.ttl); + RETERR(dns_dnssec_keyfromrdata(origin, &rdata, mctx, &dnskey)); + dst_key_setttl(dnskey, keys.ttl); - if (!is_zone_key(pubkey) || - (dst_key_flags(pubkey) & DNS_KEYTYPE_NOAUTH) != 0) { + if (!is_zone_key(dnskey) || + (dst_key_flags(dnskey) & DNS_KEYTYPE_NOAUTH) != 0) { goto skip; } /* Corrupted .key file? */ - if (!dns_name_equal(origin, dst_key_name(pubkey))) { + if (!dns_name_equal(origin, dst_key_name(dnskey))) { goto skip; } if (publickey) { - RETERR(addkey(keylist, &pubkey, savekeys, mctx)); + RETERR(addkey(keylist, &dnskey, savekeys, mctx)); goto skip; } + /* Try to read the public key. */ result = dst_key_fromfile( - dst_key_name(pubkey), dst_key_id(pubkey), - dst_key_alg(pubkey), + dst_key_name(dnskey), dst_key_id(dnskey), + dst_key_alg(dnskey), (DST_TYPE_PUBLIC | DST_TYPE_STATE), + directory, mctx, &pubkey); + if (result == ISC_R_FILENOTFOUND || result == ISC_R_NOPERM) { + result = ISC_R_SUCCESS; + } + RETERR(result); + + /* Now read the private key. */ + result = dst_key_fromfile( + dst_key_name(dnskey), dst_key_id(dnskey), + dst_key_alg(dnskey), (DST_TYPE_PUBLIC | DST_TYPE_PRIVATE | DST_TYPE_STATE), directory, mctx, &privkey); @@ -1711,22 +1722,22 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, const char *directory, */ if (result == ISC_R_FILENOTFOUND) { uint32_t flags; - flags = dst_key_flags(pubkey); + flags = dst_key_flags(dnskey); if ((flags & DNS_KEYFLAG_REVOKE) != 0) { - dst_key_setflags(pubkey, + dst_key_setflags(dnskey, flags & ~DNS_KEYFLAG_REVOKE); result = dst_key_fromfile( - dst_key_name(pubkey), - dst_key_id(pubkey), dst_key_alg(pubkey), + dst_key_name(dnskey), + dst_key_id(dnskey), dst_key_alg(dnskey), (DST_TYPE_PUBLIC | DST_TYPE_PRIVATE | DST_TYPE_STATE), directory, mctx, &privkey); if (result == ISC_R_SUCCESS && - dst_key_pubcompare(pubkey, privkey, false)) + dst_key_pubcompare(dnskey, privkey, false)) { dst_key_setflags(privkey, flags); } - dst_key_setflags(pubkey, flags); + dst_key_setflags(dnskey, flags); } } @@ -1739,8 +1750,8 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, const char *directory, isc_buffer_init(&buf, filename, NAME_MAX); result2 = dst_key_getfilename( - dst_key_name(pubkey), dst_key_id(pubkey), - dst_key_alg(pubkey), + dst_key_name(dnskey), dst_key_id(dnskey), + dst_key_alg(dnskey), (DST_TYPE_PUBLIC | DST_TYPE_PRIVATE | DST_TYPE_STATE), directory, mctx, &buf); @@ -1748,13 +1759,13 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, const char *directory, char namebuf[DNS_NAME_FORMATSIZE]; char algbuf[DNS_SECALG_FORMATSIZE]; - dns_name_format(dst_key_name(pubkey), namebuf, + dns_name_format(dst_key_name(dnskey), namebuf, sizeof(namebuf)); - dns_secalg_format(dst_key_alg(pubkey), algbuf, + dns_secalg_format(dst_key_alg(dnskey), algbuf, sizeof(algbuf)); snprintf(filename, sizeof(filename) - 1, "key file for %s/%s/%d", namebuf, - algbuf, dst_key_id(pubkey)); + algbuf, dst_key_id(dnskey)); } isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, @@ -1765,7 +1776,13 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, const char *directory, } if (result == ISC_R_FILENOTFOUND || result == ISC_R_NOPERM) { - RETERR(addkey(keylist, &pubkey, savekeys, mctx)); + if (pubkey != NULL) { + RETERR(addkey(keylist, &pubkey, savekeys, + mctx)); + } else { + RETERR(addkey(keylist, &dnskey, savekeys, + mctx)); + } goto skip; } RETERR(result); @@ -1779,10 +1796,13 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, const char *directory, * Whatever the key's default TTL may have * been, the rdataset TTL takes priority. */ - dst_key_setttl(privkey, dst_key_getttl(pubkey)); + dst_key_setttl(privkey, dst_key_getttl(dnskey)); RETERR(addkey(keylist, &privkey, savekeys, mctx)); skip: + if (dnskey != NULL) { + dst_key_free(&dnskey); + } if (pubkey != NULL) { dst_key_free(&pubkey); } @@ -1809,6 +1829,9 @@ failure: if (dns_rdataset_isassociated(&keys)) { dns_rdataset_disassociate(&keys); } + if (dnskey != NULL) { + dst_key_free(&dnskey); + } if (pubkey != NULL) { dst_key_free(&pubkey); } diff --git a/lib/dns/dst_api.c b/lib/dns/dst_api.c index 9692ac66be..1c48a26d22 100644 --- a/lib/dns/dst_api.c +++ b/lib/dns/dst_api.c @@ -569,8 +569,8 @@ dst_key_fromnamedfile(const char *filename, const char *dirname, int type, isc_mem_t *mctx, dst_key_t **keyp) { isc_result_t result; dst_key_t *pubkey = NULL, *key = NULL; - char *newfilename = NULL; - int newfilenamelen = 0; + char *newfilename = NULL, *statefilename = NULL; + int newfilenamelen = 0, statefilenamelen = 0; isc_lex_t *lex = NULL; REQUIRE(dst_initialized); @@ -604,9 +604,39 @@ dst_key_fromnamedfile(const char *filename, const char *dirname, int type, newfilename = NULL; RETERR(result); + /* + * Read the state file, if requested by type. + */ + if ((type & DST_TYPE_STATE) != 0) { + statefilenamelen = strlen(filename) + 7; + if (dirname != NULL) { + statefilenamelen += strlen(dirname) + 1; + } + statefilename = isc_mem_get(mctx, statefilenamelen); + result = addsuffix(statefilename, statefilenamelen, dirname, + filename, ".state"); + INSIST(result == ISC_R_SUCCESS); + } + + pubkey->kasp = false; + if ((type & DST_TYPE_STATE) != 0) { + result = dst_key_read_state(statefilename, mctx, &pubkey); + if (result == ISC_R_SUCCESS) { + pubkey->kasp = true; + } else if (result == ISC_R_FILENOTFOUND) { + /* Having no state is valid. */ + result = ISC_R_SUCCESS; + } + RETERR(result); + } + if ((type & (DST_TYPE_PRIVATE | DST_TYPE_PUBLIC)) == DST_TYPE_PUBLIC || (pubkey->key_flags & DNS_KEYFLAG_TYPEMASK) == DNS_KEYTYPE_NOKEY) { + if (statefilename != NULL) { + isc_mem_put(mctx, statefilename, statefilenamelen); + } + result = computeid(pubkey); if (result != ISC_R_SUCCESS) { dst_key_free(&pubkey); @@ -636,32 +666,6 @@ dst_key_fromnamedfile(const char *filename, const char *dirname, int type, RETERR(DST_R_UNSUPPORTEDALG); } - /* - * Read the state file, if requested by type. - */ - if ((type & DST_TYPE_STATE) != 0) { - newfilenamelen = strlen(filename) + 7; - if (dirname != NULL) { - newfilenamelen += strlen(dirname) + 1; - } - newfilename = isc_mem_get(mctx, newfilenamelen); - result = addsuffix(newfilename, newfilenamelen, dirname, - filename, ".state"); - INSIST(result == ISC_R_SUCCESS); - - key->kasp = false; - result = dst_key_read_state(newfilename, mctx, &key); - if (result == ISC_R_SUCCESS) { - key->kasp = true; - } else if (result == ISC_R_FILENOTFOUND) { - /* Having no state is valid. */ - result = ISC_R_SUCCESS; - } - isc_mem_put(mctx, newfilename, newfilenamelen); - newfilename = NULL; - RETERR(result); - } - newfilenamelen = strlen(filename) + 9; if (dirname != NULL) { newfilenamelen += strlen(dirname) + 1; @@ -678,6 +682,20 @@ dst_key_fromnamedfile(const char *filename, const char *dirname, int type, RETERR(key->func->parse(key, lex, pubkey)); isc_lex_destroy(&lex); + key->kasp = false; + if ((type & DST_TYPE_STATE) != 0) { + result = dst_key_read_state(statefilename, mctx, &key); + if (result == ISC_R_SUCCESS) { + key->kasp = true; + } else if (result == ISC_R_FILENOTFOUND) { + /* Having no state is valid. */ + result = ISC_R_SUCCESS; + } + isc_mem_put(mctx, statefilename, statefilenamelen); + statefilename = NULL; + } + RETERR(result); + RETERR(computeid(key)); if (pubkey->key_id != key->key_id) { @@ -695,6 +713,9 @@ out: if (newfilename != NULL) { isc_mem_put(mctx, newfilename, newfilenamelen); } + if (statefilename != NULL) { + isc_mem_put(mctx, statefilename, statefilenamelen); + } if (lex != NULL) { isc_lex_destroy(&lex); } diff --git a/lib/dns/include/dns/keymgr.h b/lib/dns/include/dns/keymgr.h index 6c7e17ceee..ae4ab5da13 100644 --- a/lib/dns/include/dns/keymgr.h +++ b/lib/dns/include/dns/keymgr.h @@ -26,12 +26,12 @@ ISC_LANG_BEGINDECLS isc_result_t dns_keymgr_run(const dns_name_t *origin, dns_rdataclass_t rdclass, const char *directory, isc_mem_t *mctx, - dns_dnsseckeylist_t *keyring, dns_kasp_t *kasp, - isc_stdtime_t now, isc_stdtime_t *nexttime); + dns_dnsseckeylist_t *keyring, dns_dnsseckeylist_t *dnskeys, + dns_kasp_t *kasp, isc_stdtime_t now, isc_stdtime_t *nexttime); /*%< - * Manage keys in 'keylist' and update timing data according to 'kasp' policy. + * Manage keys in 'keyring' and update timing data according to 'kasp' policy. * Create new keys for 'origin' if necessary in 'directory'. Append all such - * keys, along with use hints gleaned from their metadata, onto 'keylist'. + * keys, along with use hints gleaned from their metadata, onto 'keyring'. * * Update key states and store changes back to disk. Store when to run next * in 'nexttime'. diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c index b4efb3aeb2..02dbd711e2 100644 --- a/lib/dns/keymgr.c +++ b/lib/dns/keymgr.c @@ -1640,8 +1640,9 @@ static isc_result_t keymgr_key_rollover(dns_kasp_key_t *kaspkey, dns_dnsseckey_t *active_key, dns_dnsseckeylist_t *keyring, dns_dnsseckeylist_t *newkeys, const dns_name_t *origin, dns_rdataclass_t rdclass, - dns_kasp_t *kasp, uint32_t lifetime, isc_stdtime_t now, - isc_stdtime_t *nexttime, isc_mem_t *mctx) { + dns_kasp_t *kasp, uint32_t lifetime, bool rollover, + isc_stdtime_t now, isc_stdtime_t *nexttime, + isc_mem_t *mctx) { char keystr[DST_KEY_FORMATSIZE]; isc_stdtime_t retire = 0, active = 0, prepub = 0; dns_dnsseckey_t *new_key = NULL; @@ -1723,6 +1724,20 @@ keymgr_key_rollover(dns_kasp_key_t *kaspkey, dns_dnsseckey_t *active_key, /* It is time to do key rollover, we need a new key. */ + /* + * If rollover is not allowed, warn. + */ + if (!rollover) { + dst_key_format(active_key->key, keystr, sizeof(keystr)); + isc_log_write(dns_lctx, DNS_LOGCATEGORY_DNSSEC, + DNS_LOGMODULE_DNSSEC, ISC_LOG_WARNING, + "keymgr: DNSKEY %s (%s) is offline in policy %s, " + "cannot start rollover", + keystr, keymgr_keyrole(active_key->key), + dns_kasp_getname(kasp)); + return (ISC_R_SUCCESS); + } + /* * Check if there is a key available in pool because keys * may have been pregenerated with dnssec-keygen. @@ -1929,8 +1944,8 @@ keymgr_purge_keyfile(dst_key_t *key, const char *dir, int type) { isc_result_t dns_keymgr_run(const dns_name_t *origin, dns_rdataclass_t rdclass, const char *directory, isc_mem_t *mctx, - dns_dnsseckeylist_t *keyring, dns_kasp_t *kasp, - isc_stdtime_t now, isc_stdtime_t *nexttime) { + dns_dnsseckeylist_t *keyring, dns_dnsseckeylist_t *dnskeys, + dns_kasp_t *kasp, isc_stdtime_t now, isc_stdtime_t *nexttime) { isc_result_t result = ISC_R_SUCCESS; dns_dnsseckeylist_t newkeys; dns_kasp_key_t *kkey; @@ -1974,8 +1989,17 @@ dns_keymgr_run(const dns_name_t *origin, dns_rdataclass_t rdclass, dst_key_format(dkey->key, keystr, sizeof(keystr)); isc_log_write(dns_lctx, DNS_LOGCATEGORY_DNSSEC, DNS_LOGMODULE_DNSSEC, ISC_LOG_DEBUG(1), - "keymgr: keyring: dnskey %s (policy %s)", - keystr, dns_kasp_getname(kasp)); + "keymgr: keyring: %s (policy %s)", keystr, + dns_kasp_getname(kasp)); + } + for (dns_dnsseckey_t *dkey = ISC_LIST_HEAD(*dnskeys); + dkey != NULL; dkey = ISC_LIST_NEXT(dkey, link)) + { + dst_key_format(dkey->key, keystr, sizeof(keystr)); + isc_log_write(dns_lctx, DNS_LOGCATEGORY_DNSSEC, + DNS_LOGMODULE_DNSSEC, ISC_LOG_DEBUG(1), + "keymgr: dnskeys: %s (policy %s)", keystr, + dns_kasp_getname(kasp)); } } @@ -2029,6 +2053,7 @@ dns_keymgr_run(const dns_name_t *origin, dns_rdataclass_t rdclass, { uint32_t lifetime = dns_kasp_key_lifetime(kkey); dns_dnsseckey_t *active_key = NULL; + bool rollover_allowed = true; /* Do we have keys available for this kasp key? */ for (dns_dnsseckey_t *dkey = ISC_LIST_HEAD(*keyring); @@ -2089,10 +2114,43 @@ dns_keymgr_run(const dns_name_t *origin, dns_rdataclass_t rdclass, } } + if (active_key == NULL) { + /* + * We didn't found an active key, perhaps the .private + * key file is offline. If so, we don't want to create + * a successor key. Check if we have an appropriate + * state file. + */ + for (dns_dnsseckey_t *dnskey = ISC_LIST_HEAD(*dnskeys); + dnskey != NULL; + dnskey = ISC_LIST_NEXT(dnskey, link)) + { + if (keymgr_dnsseckey_kaspkey_match(dnskey, + kkey)) { + /* Found a match. */ + dst_key_format(dnskey->key, keystr, + sizeof(keystr)); + isc_log_write( + dns_lctx, + DNS_LOGCATEGORY_DNSSEC, + DNS_LOGMODULE_DNSSEC, + ISC_LOG_DEBUG(1), + "keymgr: DNSKEY %s (%s) " + "offline, policy %s", + keystr, + keymgr_keyrole(dnskey->key), + dns_kasp_getname(kasp)); + rollover_allowed = false; + active_key = dnskey; + break; + } + } + } + /* See if this key requires a rollover. */ - RETERR(keymgr_key_rollover(kkey, active_key, keyring, &newkeys, - origin, rdclass, kasp, lifetime, now, - nexttime, mctx)); + RETERR(keymgr_key_rollover( + kkey, active_key, keyring, &newkeys, origin, rdclass, + kasp, lifetime, rollover_allowed, now, nexttime, mctx)); } /* Walked all kasp key configurations. Append new keys. */ diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 7ccacf6e80..9553ade3e8 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -6438,7 +6438,7 @@ set_key_expiry_warning(dns_zone_t *zone, isc_stdtime_t when, */ static bool delsig_ok(dns_rdata_rrsig_t *rrsig_ptr, dst_key_t **keys, unsigned int nkeys, - bool *warn) { + bool kasp, bool *warn) { unsigned int i = 0; isc_result_t ret; bool have_ksk = false, have_zsk = false; @@ -6482,12 +6482,27 @@ delsig_ok(dns_rdata_rrsig_t *rrsig_ptr, dst_key_t **keys, unsigned int nkeys, *warn = true; } + if (have_pksk && have_pzsk) { + return (true); + } + /* - * It's okay to delete a signature if there is an active key - * with the same algorithm to replace it. + * Deleting the SOA RRSIG is always okay. + */ + if (rrsig_ptr->covered == dns_rdatatype_soa) { + return (true); + } + + /* + * It's okay to delete a signature if there is an active key with the + * same algorithm to replace it, unless that violates the DNSSEC + * policy. */ if (have_pksk || have_pzsk) { - return (true); + if (kasp && have_pzsk) { + return (true); + } + return (!kasp); } /* @@ -6521,6 +6536,7 @@ del_sigs(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, dns_rdataset_t rdataset; unsigned int i; dns_rdata_rrsig_t rrsig; + bool kasp = (dns_zone_getkasp(zone) != NULL); bool found; int64_t timewarn = 0, timemaybe = 0; @@ -6563,7 +6579,7 @@ del_sigs(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, type != dns_rdatatype_cdnskey) { bool warn = false, deleted = false; - if (delsig_ok(&rrsig, keys, nkeys, &warn)) { + if (delsig_ok(&rrsig, keys, nkeys, kasp, &warn)) { result = update_one_rr(db, ver, zonediff->diff, DNS_DIFFOP_DELRESIGN, name, rdataset.ttl, @@ -6807,6 +6823,8 @@ add_sigs(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, dns_zone_t *zone, isc_stdtime_t when; bool ksk = false; bool zsk = false; + bool have_ksk = false; + bool have_zsk = false; kresult = dst_key_getbool(keys[i], DST_BOOL_KSK, &ksk); if (kresult != ISC_R_SUCCESS) { @@ -6821,6 +6839,56 @@ add_sigs(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, dns_zone_t *zone, } } + have_ksk = ksk; + have_zsk = zsk; + both = have_ksk && have_zsk; + + for (j = 0; j < nkeys; j++) { + if (both) { + break; + } + + if (j == i || ALG(keys[i]) != ALG(keys[j])) { + continue; + } + + /* + * Don't consider inactive keys or offline keys. + */ + if (!dst_key_isprivate(keys[j])) { + continue; + } + if (dst_key_inactive(keys[j])) { + continue; + } + + if (REVOKE(keys[j])) { + continue; + } + + if (!have_ksk) { + kresult = dst_key_getbool(keys[j], + DST_BOOL_KSK, + &have_ksk); + if (kresult != ISC_R_SUCCESS) { + if (KSK(keys[j])) { + have_ksk = true; + } + } + } + if (!have_zsk) { + kresult = dst_key_getbool(keys[j], + DST_BOOL_ZSK, + &have_zsk); + if (kresult != ISC_R_SUCCESS) { + if (!KSK(keys[j])) { + have_zsk = true; + } + } + } + both = have_ksk && have_zsk; + } + if (type == dns_rdatatype_dnskey || type == dns_rdatatype_cdnskey || type == dns_rdatatype_cds) @@ -6836,7 +6904,13 @@ add_sigs(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, dns_zone_t *zone, /* * Other RRsets are signed with ZSK. */ - continue; + if (type != dns_rdatatype_soa && + type != zone->privatetype) { + continue; + } + if (have_zsk) { + continue; + } } else if (!dst_key_is_signing(keys[i], DST_BOOL_ZSK, inception, &when)) { /* @@ -19971,22 +20045,20 @@ zone_rekey(dns_zone_t *zone) { isc_result_totext(result)); } - if (kasp != NULL && - (result == ISC_R_SUCCESS || result == ISC_R_NOTFOUND)) { - result = dns_keymgr_run(&zone->origin, zone->rdclass, dir, mctx, - &keys, kasp, now, &nexttime); - if (result != ISC_R_SUCCESS) { - if (kasp != NULL) { - UNLOCK(&kasp->lock); - } - dnssec_log(zone, ISC_LOG_ERROR, - "zone_rekey:dns_dnssec_keymgr failed: %s", - isc_result_totext(result)); - goto failure; - } - } - if (kasp != NULL) { + if (result == ISC_R_SUCCESS || result == ISC_R_NOTFOUND) { + result = dns_keymgr_run(&zone->origin, zone->rdclass, + dir, mctx, &keys, &dnskeys, + kasp, now, &nexttime); + if (result != ISC_R_SUCCESS) { + dnssec_log(zone, ISC_LOG_ERROR, + "zone_rekey:dns_dnssec_keymgr " + "failed: %s", + isc_result_totext(result)); + UNLOCK(&kasp->lock); + goto failure; + } + } UNLOCK(&kasp->lock); } @@ -20006,8 +20078,7 @@ zone_rekey(dns_zone_t *zone) { /* * Only update DNSKEY TTL if we have a policy. */ - if (kasp != NULL && strcmp(dns_kasp_getname(kasp), "none") != 0) - { + if (kasp != NULL) { ttl = dns_kasp_dnskeyttl(kasp); } @@ -20343,6 +20414,10 @@ failure: * Something went wrong; try again in ten minutes or * after a key refresh interval, whichever is shorter. */ + dnssec_log(zone, ISC_LOG_DEBUG(3), + "zone_rekey failure: %s (retry in %u seconds)", + isc_result_totext(result), + ISC_MIN(zone->refreshkeyinterval, 600)); isc_interval_set(&ival, ISC_MIN(zone->refreshkeyinterval, 600), 0); isc_time_nowplusinterval(&zone->refreshkeytime, &ival);