From a50ce0f80bd665545389cfd91df31d3f4fe66b04 Mon Sep 17 00:00:00 2001 From: Scott Mann Date: Thu, 19 May 2011 00:31:57 +0000 Subject: [PATCH] Fix for RT #23136 task 1. --- CHANGES | 3 + bin/tests/system/dnssec/ns3/named.conf | 8 +- bin/tests/system/dnssec/ns3/sign.sh | 12 ++- bin/tests/system/dnssec/tests.sh | 12 ++- lib/dns/include/dns/db.h | 10 +- lib/dns/zone.c | 124 ++++++++++++++++++++++--- 6 files changed, 151 insertions(+), 18 deletions(-) diff --git a/CHANGES b/CHANGES index 5395919d4b..140e6183c4 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +3114. [bug] Retain signed RRSET if key is inactive and there is + no replacement key. [RT #23136 task 1] + 3113. [doc] Document the relationship between serial-query-rate and NOTIFY messages. diff --git a/bin/tests/system/dnssec/ns3/named.conf b/bin/tests/system/dnssec/ns3/named.conf index 856dd3a238..4e320bbccb 100644 --- a/bin/tests/system/dnssec/ns3/named.conf +++ b/bin/tests/system/dnssec/ns3/named.conf @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: named.conf,v 1.46 2011/03/21 01:02:39 marka Exp $ */ +/* $Id: named.conf,v 1.47 2011/05/19 00:31:57 smann Exp $ */ // NS3 @@ -223,4 +223,10 @@ zone "nsec3chain-test" { masters { 10.53.0.2; }; }; +zone "expiring.example" { + type master; + allow-update { any; }; + file "expiring.example.db.signed"; +}; + include "trusted.conf"; diff --git a/bin/tests/system/dnssec/ns3/sign.sh b/bin/tests/system/dnssec/ns3/sign.sh index f197929a0a..58e9ad1811 100644 --- a/bin/tests/system/dnssec/ns3/sign.sh +++ b/bin/tests/system/dnssec/ns3/sign.sh @@ -15,7 +15,7 @@ # OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR # PERFORMANCE OF THIS SOFTWARE. -# $Id: sign.sh,v 1.40 2011/03/31 15:58:51 each Exp $ +# $Id: sign.sh,v 1.41 2011/05/19 00:31:57 smann Exp $ SYSTEMTESTTOP=../.. . $SYSTEMTESTTOP/conf.sh @@ -370,3 +370,13 @@ echo '$INCLUDE "'"$signedfile"'"' >> $zonefile : > $signedfile $SIGNER -P -S -r $RANDFILE -D -o $zone $zonefile > /dev/null 2>&1 +zone="expiring.example." +infile="expiring.example.db.in" +zonefile="expiring.example.db" +signedfile="expiring.example.db.signed" +kskname=`$KEYGEN -q -r $RANDFILE $zone` +zskname=`$KEYGEN -q -r $RANDFILE -f KSK $zone` +cp $infile $zonefile +$SIGNER -S -r $RANDFILE -e now+1mi -o $zone $zonefile > /dev/null 2>&1 +rm -f ${zskname}.private ${kskname}.private + diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index 4adf74fb37..58e8637e93 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -15,7 +15,7 @@ # OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR # PERFORMANCE OF THIS SOFTWARE. -# $Id: tests.sh,v 1.88 2011/03/24 02:10:23 marka Exp $ +# $Id: tests.sh,v 1.89 2011/05/19 00:31:57 smann Exp $ SYSTEMTESTTOP=.. . $SYSTEMTESTTOP/conf.sh @@ -1315,7 +1315,7 @@ n=`expr $n + 1` if [ $ret != 0 ]; then echo "I:failed"; fi status=`expr $status + $ret` -echo "I:check dnssec-dsfromkey from stdin($n)" +echo "I:check dnssec-dsfromkey from stdin ($n)" ret=0 $DIG $DIGOPTS dnskey algroll. @10.53.0.2 | \ $DSFROMKEY -f - algroll. > dig.out.ns2.test$n || ret=1 @@ -1324,5 +1324,13 @@ n=`expr $n + 1` if [ $ret != 0 ]; then echo "I:failed"; fi status=`expr $status + $ret` +echo "I:testing soon-to-expire RRSIGs without a replacement private key ($n)" +ret=0 +$DIG +noall +answer +dnssec +nottl -p 5300 expiring.example ns @10.53.0.3 | grep RRSIG > dig.out.ns3.test$n 2>&1 +# there must be a signature here +[ -s dig.out.ns3.test$n ] || ret=1 +if [ $ret != 0 ]; then echo "I:failed"; fi +status=`expr $status + $ret` + echo "I:exit status: $status" exit $status diff --git a/lib/dns/include/dns/db.h b/lib/dns/include/dns/db.h index bc98553628..5a5e5e9977 100644 --- a/lib/dns/include/dns/db.h +++ b/lib/dns/include/dns/db.h @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: db.h,v 1.104 2011/01/13 04:59:25 tbox Exp $ */ +/* $Id: db.h,v 1.105 2011/05/19 00:31:57 smann Exp $ */ #ifndef DNS_DB_H #define DNS_DB_H 1 @@ -1441,7 +1441,9 @@ dns_db_setsigningtime(dns_db_t *db, dns_rdataset_t *rdataset, * * Requires: * \li 'db' is a valid zone database. - * \li 'rdataset' to be associated with 'db'. + * \li 'rdataset' is or is to be associated with 'db'. + * \li 'rdataset' is not pending removed from the heap via an + * uncommitted call to dns_db_resigned(). * * Returns: * \li #ISC_R_SUCCESS @@ -1472,7 +1474,9 @@ dns_db_resigned(dns_db_t *db, dns_rdataset_t *rdataset, * Mark 'rdataset' as not being available to be returned by * dns_db_getsigningtime(). If the changes associated with 'version' * are committed this will be permanent. If the version is not committed - * this change will be rolled back when the version is closed. + * this change will be rolled back when the version is closed. Until + * 'version' is either committed or rolled back, 'rdataset' can no longer + * be acted upon by dns_db_setsigningtime(). * * Requires: * \li 'db' is a valid zone database. diff --git a/lib/dns/zone.c b/lib/dns/zone.c index aaee6345c7..bb93d2431b 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: zone.c,v 1.604 2011/05/06 21:23:51 each Exp $ */ +/* $Id: zone.c,v 1.605 2011/05/19 00:31:57 smann Exp $ */ /*! \file */ @@ -109,12 +109,20 @@ #define NSEC3REMOVE(x) (((x) & DNS_NSEC3FLAG_REMOVE) != 0) +/*% + * Key flags + */ +#define REVOKE(x) ((dst_key_flags(x) & DNS_KEYFLAG_REVOKE) != 0) +#define KSK(x) ((dst_key_flags(x) & DNS_KEYFLAG_KSK) != 0) +#define ALG(x) dst_key_alg(x) + /* * Default values. */ #define DNS_DEFAULT_IDLEIN 3600 /*%< 1 hour */ #define DNS_DEFAULT_IDLEOUT 3600 /*%< 1 hour */ #define MAX_XFER_TIME (2*3600) /*%< Documented default is 2 hours */ +#define RESIGN_DELAY 3600 /*%< 1 hour */ #ifndef DNS_MAX_EXPIRE #define DNS_MAX_EXPIRE 14515200 /*%< 24 weeks */ @@ -214,6 +222,7 @@ struct dns_zone { isc_uint32_t expire; isc_uint32_t minimum; isc_stdtime_t key_expiry; + isc_stdtime_t log_key_expired_timer; char *keydirectory; isc_uint32_t maxrefresh; @@ -663,6 +672,8 @@ static isc_result_t delete_nsec(dns_db_t *db, dns_dbversion_t *ver, dns_dbnode_t *node, dns_name_t *name, dns_diff_t *diff); static void zone_rekey(dns_zone_t *zone); +static isc_boolean_t delsig_ok(dns_rdata_rrsig_t *rrsig_ptr, + dst_key_t **keys, unsigned int nkeys); #define ENTER zone_debuglog(zone, me, 1, "enter") @@ -810,6 +821,7 @@ dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx) { zone->timer = NULL; zone->idlein = DNS_DEFAULT_IDLEIN; zone->idleout = DNS_DEFAULT_IDLEOUT; + zone->log_key_expired_timer = 0; ISC_LIST_INIT(zone->notifies); isc_sockaddr_any(&zone->notifysrc4); isc_sockaddr_any6(&zone->notifysrc6); @@ -3611,6 +3623,39 @@ zone_postload(dns_zone_t *zone, dns_db_t *db, isc_time_t loadtime, resume_signingwithkey(zone); resume_addnsec3chain(zone); } + + if (zone->type == dns_zone_master && + dns_zone_isdynamic(zone, ISC_TRUE) && + dns_db_issecure(db)) { + dns_name_t *name; + dns_fixedname_t fixed; + dns_rdataset_t next; + + dns_rdataset_init(&next); + dns_fixedname_init(&fixed); + name = dns_fixedname_name(&fixed); + + result = dns_db_getsigningtime(db, &next, name); + if (result == ISC_R_SUCCESS) { + isc_stdtime_t timenow; + char namebuf[DNS_NAME_FORMATSIZE]; + char typebuf[DNS_RDATATYPE_FORMATSIZE]; + + isc_stdtime_get(&timenow); + dns_name_format(name, namebuf, sizeof(namebuf)); + dns_rdatatype_format(next.covers, + typebuf, sizeof(typebuf)); + dns_zone_log(zone, ISC_LOG_DEBUG(3), + "next resign: %s/%s in %d seconds", + namebuf, typebuf, + next.resign - timenow); + dns_rdataset_disassociate(&next); + } else + dns_zone_log(zone, ISC_LOG_WARNING, + "signed dynamic zone has no " + "resign event scheduled"); + } + zone_settimer(zone, &now); } @@ -4604,6 +4649,31 @@ set_key_expiry_warning(dns_zone_t *zone, isc_stdtime_t when, isc_stdtime_t now) } } +/* + * Helper function to del_sigs(). We don't want to delete RRSIGs that + * have no new key. + */ +static isc_boolean_t +delsig_ok(dns_rdata_rrsig_t *rrsig_ptr, dst_key_t **keys, unsigned int nkeys) { + unsigned int i = 0; + + for (i = 0; i < nkeys; i++) { + if ((rrsig_ptr->algorithm == dst_key_alg(keys[i])) && + (rrsig_ptr->keyid != dst_key_id(keys[i]))) { + if ((dst_key_isprivate(keys[i])) && !KSK(keys[i])) { + /* + * Success - found a private key, which + * means it is an active key and thus, it + * is OK to delete the RRSIG + */ + return (ISC_TRUE); + } + } + } + + return (ISC_FALSE); +} + /* * Delete expired RRsigs and any RRsigs we are about to re-sign. * See also update.c:del_keysigs(). @@ -4653,13 +4723,49 @@ del_sigs(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, RUNTIME_CHECK(result == ISC_R_SUCCESS); if (type != dns_rdatatype_dnskey) { - result = update_one_rr(db, ver, diff, + if(delsig_ok(&rrsig, keys, nkeys)) { + result = update_one_rr(db, ver, diff, DNS_DIFFOP_DELRESIGN, name, rdataset.ttl, &rdata); - dns_rdata_reset(&rdata); - if (result != ISC_R_SUCCESS) - break; - continue; + dns_db_resigned(db, &rdataset, ver); + dns_rdata_reset(&rdata); + if (result != ISC_R_SUCCESS) + break; + continue; + } else { + /* + * At this point, we've got an RRSIG, + * which is signed by an inactive key. + * An administrator needs to provide a new + * key/alg, but until that time, we want to + * keep the old RRSIG. Resetting the timer + * here will ensure that we don't + * constantly recheck this expired record. + * + * Note: dns_db_setsigningtime() will + * assert if called after dns_db_resigned(). + */ + isc_stdtime_t recheck = now + RESIGN_DELAY; + dns_db_setsigningtime(db, &rdataset, recheck); + + /* + * log the key id and algorithm of + * the inactive key with no replacement + */ + if((isc_log_getdebuglevel(dns_lctx) > 3) || + (zone->log_key_expired_timer <= now)) { + dns_zone_log(zone, ISC_LOG_WARNING, + "del_sigs(): " + "keyid: %u/algorithm: %u " + "is not active and there " + "is no replacement. " + "Not deleting.", + rrsig.keyid, + rrsig.algorithm); + zone->log_key_expired_timer = now + + 3600; + } + } } /* @@ -4763,10 +4869,6 @@ add_sigs(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, goto failure; } -#define REVOKE(x) ((dst_key_flags(x) & DNS_KEYFLAG_REVOKE) != 0) -#define KSK(x) ((dst_key_flags(x) & DNS_KEYFLAG_KSK) != 0) -#define ALG(x) dst_key_alg(x) - for (i = 0; i < nkeys; i++) { isc_boolean_t both = ISC_FALSE; @@ -4922,7 +5024,6 @@ zone_resigninc(dns_zone_t *zone) { break; } - dns_db_resigned(db, &rdataset, version); dns_rdataset_disassociate(&rdataset); result = del_sigs(zone, db, version, name, covers, &sig_diff, @@ -4933,6 +5034,7 @@ zone_resigninc(dns_zone_t *zone) { dns_result_totext(result)); break; } + result = add_sigs(db, version, name, covers, &sig_diff, zone_keys, nkeys, zone->mctx, inception, expire, check_ksk, keyset_kskonly);