3421. [bug] Named loops when re-signing if all keys are offline.

[RT #31916]

Squashed commit of the following:

commit f47af0ca6793687b9c8d08fd44b0c091ba5a4f9a
Author: Mark Andrews <marka@isc.org>
Date:   Wed Nov 21 17:45:21 2012 +1100

    dns_dns_zonediff_t -> dns_zonediff_t, clarify comment

commit 344edefc3ee90856a7ff990abe7971925ba843b2
Author: Mark Andrews <marka@isc.org>
Date:   Tue Nov 20 13:12:26 2012 +1100

    commit the zone changes if a keep was marked as being offline

commit cad2c2446ebfc20b6d8c4f6dd0d6596d7106cc0f
Author: Mark Andrews <marka@isc.org>
Date:   Tue Nov 20 13:08:29 2012 +1100

    check for looping when re-signing expiring.example

Conflicts:
	lib/dns/zone.c

Conflicts:
	lib/dns/zone.c
This commit is contained in:
Mark Andrews 2012-11-21 17:48:57 +11:00
parent 40f92e3040
commit 1aefd8ec88
3 changed files with 107 additions and 59 deletions

View file

@ -1,3 +1,6 @@
3421. [bug] Named loops when re-signing if all keys are offline.
[RT #31916]
3420. [bug] Address VPATH compilation issues. [RT #31879]
3419. [bug] Memory leak on validation cancel. [RT #31869]

View file

@ -1282,5 +1282,13 @@ n=`expr $n + 1`
if [ $ret != 0 ]; then echo "I:failed"; fi
status=`expr $status + $ret`
echo "I:check that named doesn't loop when all private keys are not available ($n)"
ret=0
lines=`grep "reading private key file expiring.example" ns3/named.run | wc -l`
test ${lines:-1000} -lt 10 || ret=1
n=`expr $n + 1`
if [ $ret != 0 ]; then echo "I:failed"; fi
status=`expr $status + $ret`
echo "I:exit status: $status"
exit $status

View file

@ -314,6 +314,18 @@ struct dns_zone {
dns_forwardlist_t forwards;
};
typedef struct {
dns_diff_t *diff;
isc_boolean_t offline;
} zonediff_t;
#define zonediff_init(z, d) \
do { \
zonediff_t *_z = (z); \
(_z)->diff = (d); \
(_z)->offline = ISC_FALSE; \
} while (0)
#define DNS_ZONE_FLAG(z,f) (ISC_TF(((z)->flags & (f)) != 0))
#define DNS_ZONE_SETFLAG(z,f) do { \
INSIST(LOCKED_ZONE(z)); \
@ -3686,20 +3698,21 @@ find_zone_keys(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver,
}
static isc_result_t
offline(dns_db_t *db, dns_dbversion_t *ver, dns_diff_t *diff, dns_name_t *name,
dns_ttl_t ttl, dns_rdata_t *rdata)
offline(dns_db_t *db, dns_dbversion_t *ver, zonediff_t *zonediff,
dns_name_t *name, dns_ttl_t ttl, dns_rdata_t *rdata)
{
isc_result_t result;
if ((rdata->flags & DNS_RDATA_OFFLINE) != 0)
return (ISC_R_SUCCESS);
result = update_one_rr(db, ver, diff, DNS_DIFFOP_DELRESIGN,
result = update_one_rr(db, ver, zonediff->diff, DNS_DIFFOP_DELRESIGN,
name, ttl, rdata);
if (result != ISC_R_SUCCESS)
return (result);
rdata->flags |= DNS_RDATA_OFFLINE;
result = update_one_rr(db, ver, diff, DNS_DIFFOP_ADDRESIGN,
result = update_one_rr(db, ver, zonediff->diff, DNS_DIFFOP_ADDRESIGN,
name, ttl, rdata);
zonediff->offline = ISC_TRUE;
return (result);
}
@ -3736,7 +3749,7 @@ set_key_expiry_warning(dns_zone_t *zone, isc_stdtime_t when, isc_stdtime_t now)
*/
static isc_result_t
del_sigs(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name,
dns_rdatatype_t type, dns_diff_t *diff, dst_key_t **keys,
dns_rdatatype_t type, zonediff_t *zonediff, dst_key_t **keys,
unsigned int nkeys, isc_stdtime_t now)
{
isc_result_t result;
@ -3775,7 +3788,7 @@ 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,
result = update_one_rr(db, ver, zonediff->diff,
DNS_DIFFOP_DELRESIGN, name,
rdataset.ttl, &rdata);
dns_rdata_reset(&rdata);
@ -3812,11 +3825,12 @@ del_sigs(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name,
warn = maybe;
if (warn == 0 || warn > timeexpire)
warn = timeexpire;
result = offline(db, ver, diff, name,
rdataset.ttl, &rdata);
result = offline(db, ver, zonediff,
name, rdataset.ttl,
&rdata);
break;
}
result = update_one_rr(db, ver, diff,
result = update_one_rr(db, ver, zonediff->diff,
DNS_DIFFOP_DELRESIGN,
name, rdataset.ttl,
&rdata);
@ -3828,7 +3842,7 @@ del_sigs(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name,
* delete the RRSIG.
*/
if (!found)
result = update_one_rr(db, ver, diff,
result = update_one_rr(db, ver, zonediff->diff,
DNS_DIFFOP_DELRESIGN, name,
rdataset.ttl, &rdata);
dns_rdata_reset(&rdata);
@ -3917,10 +3931,12 @@ add_sigs(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name,
static void
zone_resigninc(dns_zone_t *zone) {
const char *me = "zone_resigninc";
const char *journalfile;
dns_db_t *db = NULL;
dns_dbversion_t *version = NULL;
dns_diff_t sig_diff;
dns_diff_t _sig_diff;
zonediff_t zonediff;
dns_fixedname_t fixed;
dns_name_t *name;
dns_rdataset_t rdataset;
@ -3934,10 +3950,13 @@ zone_resigninc(dns_zone_t *zone) {
unsigned int nkeys = 0;
unsigned int resign;
ENTER;
dns_rdataset_init(&rdataset);
dns_fixedname_init(&fixed);
dns_diff_init(zone->mctx, &sig_diff);
sig_diff.resign = zone->sigresigninginterval;
dns_diff_init(zone->mctx, &_sig_diff);
_sig_diff.resign = zone->sigresigninginterval;
zonediff_init(&zonediff, &_sig_diff);
/*
* Updates are disabled. Pause for 5 minutes.
@ -4016,7 +4035,7 @@ zone_resigninc(dns_zone_t *zone) {
dns_db_resigned(db, &rdataset, version);
dns_rdataset_disassociate(&rdataset);
result = del_sigs(zone, db, version, name, covers, &sig_diff,
result = del_sigs(zone, db, version, name, covers, &zonediff,
zone_keys, nkeys, now);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR,
@ -4024,7 +4043,7 @@ zone_resigninc(dns_zone_t *zone) {
dns_result_totext(result));
break;
}
result = add_sigs(db, version, name, covers, &sig_diff,
result = add_sigs(db, version, name, covers, zonediff.diff,
zone_keys, nkeys, zone->mctx, inception,
expire, check_ksk);
if (result != ISC_R_SUCCESS) {
@ -4049,7 +4068,7 @@ zone_resigninc(dns_zone_t *zone) {
goto failure;
result = del_sigs(zone, db, version, &zone->origin, dns_rdatatype_soa,
&sig_diff, zone_keys, nkeys, now);
&zonediff, zone_keys, nkeys, now);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR,
"zone_resigninc:del_sigs -> %s",
@ -4057,7 +4076,7 @@ zone_resigninc(dns_zone_t *zone) {
goto failure;
}
result = increment_soa_serial(db, version, &sig_diff, zone->mctx);
result = increment_soa_serial(db, version, zonediff.diff, zone->mctx);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR,
"zone_resigninc:increment_soa_serial -> %s",
@ -4070,8 +4089,8 @@ zone_resigninc(dns_zone_t *zone) {
* termination is sensible.
*/
result = add_sigs(db, version, &zone->origin, dns_rdatatype_soa,
&sig_diff, zone_keys, nkeys, zone->mctx, inception,
soaexpire, check_ksk);
zonediff.diff, zone_keys, nkeys, zone->mctx,
inception, soaexpire, check_ksk);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR,
"zone_resigninc:add_sigs -> %s",
@ -4091,7 +4110,7 @@ zone_resigninc(dns_zone_t *zone) {
goto failure;
}
result = dns_journal_write_transaction(journal, &sig_diff);
result = dns_journal_write_transaction(journal, zonediff.diff);
dns_journal_destroy(&journal);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR,
@ -4107,7 +4126,7 @@ zone_resigninc(dns_zone_t *zone) {
dns_db_closeversion(db, &version, ISC_TRUE);
failure:
dns_diff_clear(&sig_diff);
dns_diff_clear(&_sig_diff);
for (i = 0; i < nkeys; i++)
dst_key_free(&zone_keys[i]);
if (version != NULL) {
@ -4730,14 +4749,16 @@ need_nsec_chain(dns_db_t *db, dns_dbversion_t *ver,
*/
static void
zone_nsec3chain(dns_zone_t *zone) {
const char *me = "zone_nsec3chain";
const char *journalfile;
dns_db_t *db = NULL;
dns_dbnode_t *node = NULL;
dns_dbversion_t *version = NULL;
dns_diff_t sig_diff;
dns_diff_t _sig_diff;
dns_diff_t nsec_diff;
dns_diff_t nsec3_diff;
dns_diff_t param_diff;
zonediff_t zonediff;
dns_fixedname_t fixed;
dns_fixedname_t nextfixed;
dns_name_t *name, *nextname;
@ -4763,6 +4784,8 @@ zone_nsec3chain(dns_zone_t *zone) {
isc_boolean_t buildnsecchain;
isc_boolean_t updatensec = ISC_FALSE;
ENTER;
dns_rdataset_init(&rdataset);
dns_fixedname_init(&fixed);
name = dns_fixedname_name(&fixed);
@ -4771,8 +4794,9 @@ zone_nsec3chain(dns_zone_t *zone) {
dns_diff_init(zone->mctx, &param_diff);
dns_diff_init(zone->mctx, &nsec3_diff);
dns_diff_init(zone->mctx, &nsec_diff);
dns_diff_init(zone->mctx, &sig_diff);
sig_diff.resign = zone->sigresigninginterval;
dns_diff_init(zone->mctx, &_sig_diff);
_sig_diff.resign = zone->sigresigninginterval;
zonediff_init(&zonediff, &_sig_diff);
ISC_LIST_INIT(cleanup);
/*
@ -5197,7 +5221,7 @@ zone_nsec3chain(dns_zone_t *zone) {
* the signatures.
*/
result = del_sigs(zone, db, version, &tuple->name,
dns_rdatatype_nsec3, &sig_diff,
dns_rdatatype_nsec3, &zonediff,
zone_keys, nkeys, now);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR,
@ -5206,7 +5230,7 @@ zone_nsec3chain(dns_zone_t *zone) {
goto failure;
}
result = add_sigs(db, version, &tuple->name,
dns_rdatatype_nsec3, &sig_diff, zone_keys,
dns_rdatatype_nsec3, zonediff.diff, zone_keys,
nkeys, zone->mctx, inception, expire,
check_ksk);
if (result != ISC_R_SUCCESS) {
@ -5222,7 +5246,7 @@ zone_nsec3chain(dns_zone_t *zone) {
!dns_name_equal(&tuple->name, &next->name))
next = ISC_LIST_NEXT(next, link);
ISC_LIST_UNLINK(nsec3_diff.tuples, tuple, link);
dns_diff_appendminimal(&sig_diff, &tuple);
dns_diff_appendminimal(zonediff.diff, &tuple);
INSIST(tuple == NULL);
tuple = next;
} while (tuple != NULL);
@ -5236,7 +5260,7 @@ zone_nsec3chain(dns_zone_t *zone) {
* update the signatures.
*/
result = del_sigs(zone, db, version, &tuple->name,
dns_rdatatype_nsec3param, &sig_diff,
dns_rdatatype_nsec3param, &zonediff,
zone_keys, nkeys, now);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR,
@ -5245,7 +5269,7 @@ zone_nsec3chain(dns_zone_t *zone) {
goto failure;
}
result = add_sigs(db, version, &tuple->name,
dns_rdatatype_nsec3param, &sig_diff,
dns_rdatatype_nsec3param, zonediff.diff,
zone_keys, nkeys, zone->mctx, inception,
expire, check_ksk);
if (result != ISC_R_SUCCESS) {
@ -5255,7 +5279,7 @@ zone_nsec3chain(dns_zone_t *zone) {
goto failure;
}
ISC_LIST_UNLINK(param_diff.tuples, tuple, link);
dns_diff_appendminimal(&sig_diff, &tuple);
dns_diff_appendminimal(zonediff.diff, &tuple);
INSIST(tuple == NULL);
}
@ -5267,7 +5291,7 @@ zone_nsec3chain(dns_zone_t *zone) {
tuple != NULL;
tuple = ISC_LIST_HEAD(nsec_diff.tuples)) {
result = del_sigs(zone, db, version, &tuple->name,
dns_rdatatype_nsec, &sig_diff,
dns_rdatatype_nsec, &zonediff,
zone_keys, nkeys, now);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR,
@ -5276,7 +5300,7 @@ zone_nsec3chain(dns_zone_t *zone) {
goto failure;
}
result = add_sigs(db, version, &tuple->name,
dns_rdatatype_nsec, &sig_diff,
dns_rdatatype_nsec, zonediff.diff,
zone_keys, nkeys, zone->mctx, inception,
expire, check_ksk);
if (result != ISC_R_SUCCESS) {
@ -5286,7 +5310,7 @@ zone_nsec3chain(dns_zone_t *zone) {
goto failure;
}
ISC_LIST_UNLINK(nsec_diff.tuples, tuple, link);
dns_diff_appendminimal(&sig_diff, &tuple);
dns_diff_appendminimal(zonediff.diff, &tuple);
INSIST(tuple == NULL);
}
@ -5294,18 +5318,23 @@ zone_nsec3chain(dns_zone_t *zone) {
* If we made no effective changes to the zone then we can just
* cleanup otherwise we need to increment the serial.
*/
if (ISC_LIST_HEAD(sig_diff.tuples) == NULL)
if (ISC_LIST_EMPTY(zonediff.diff->tuples)) {
/*
* No need to call dns_db_closeversion() here as it is
* called with commit = ISC_TRUE below.
*/
goto done;
}
result = del_sigs(zone, db, version, &zone->origin, dns_rdatatype_soa,
&sig_diff, zone_keys, nkeys, now);
&zonediff, zone_keys, nkeys, now);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR, "zone_nsec3chain:"
"del_sigs -> %s", dns_result_totext(result));
goto failure;
}
result = increment_soa_serial(db, version, &sig_diff, zone->mctx);
result = increment_soa_serial(db, version, zonediff.diff, zone->mctx);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR, "zone_nsec3chain:"
"increment_soa_serial -> %s",
@ -5314,8 +5343,8 @@ zone_nsec3chain(dns_zone_t *zone) {
}
result = add_sigs(db, version, &zone->origin, dns_rdatatype_soa,
&sig_diff, zone_keys, nkeys, zone->mctx, inception,
soaexpire, check_ksk);
zonediff.diff, zone_keys, nkeys, zone->mctx,
inception, soaexpire, check_ksk);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR, "zone_nsec3chain:"
"add_sigs -> %s", dns_result_totext(result));
@ -5334,7 +5363,7 @@ zone_nsec3chain(dns_zone_t *zone) {
goto failure;
}
result = dns_journal_write_transaction(journal, &sig_diff);
result = dns_journal_write_transaction(journal, zonediff.diff);
dns_journal_destroy(&journal);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR, "zone_nsec3chain:"
@ -5428,7 +5457,7 @@ zone_nsec3chain(dns_zone_t *zone) {
dns_diff_clear(&param_diff);
dns_diff_clear(&nsec3_diff);
dns_diff_clear(&nsec_diff);
dns_diff_clear(&sig_diff);
dns_diff_clear(&_sig_diff);
if (iterator != NULL)
dns_rdatasetiter_destroy(&iterator);
@ -5532,7 +5561,8 @@ zone_sign(dns_zone_t *zone) {
dns_db_t *db = NULL;
dns_dbnode_t *node = NULL;
dns_dbversion_t *version = NULL;
dns_diff_t sig_diff;
dns_diff_t _sig_diff;
zonediff_t zonediff;
dns_fixedname_t fixed;
dns_fixedname_t nextfixed;
dns_name_t *name, *nextname;
@ -5560,8 +5590,9 @@ zone_sign(dns_zone_t *zone) {
name = dns_fixedname_name(&fixed);
dns_fixedname_init(&nextfixed);
nextname = dns_fixedname_name(&nextfixed);
dns_diff_init(zone->mctx, &sig_diff);
sig_diff.resign = zone->sigresigninginterval;
dns_diff_init(zone->mctx, &_sig_diff);
_sig_diff.resign = zone->sigresigninginterval;
zonediff_init(&zonediff, &_sig_diff);
ISC_LIST_INIT(cleanup);
/*
@ -5681,7 +5712,7 @@ zone_sign(dns_zone_t *zone) {
dns_dbiterator_pause(signing->dbiterator);
CHECK(del_sig(db, version, name, node, nkeys,
signing->algorithm, signing->keyid,
&sig_diff));
zonediff.diff));
goto next_node;
}
/*
@ -5731,8 +5762,8 @@ zone_sign(dns_zone_t *zone) {
CHECK(sign_a_node(db, name, node, version, build_nsec3,
build_nsec, zone_keys[i], inception,
expire, zone->minimum, is_ksk,
&delegation, &sig_diff, &signatures,
zone->mctx));
&delegation, zonediff.diff,
&signatures, zone->mctx));
break;
}
/*
@ -5762,7 +5793,7 @@ zone_sign(dns_zone_t *zone) {
&zone->origin,
zone->minimum,
&secureupdated,
&sig_diff);
zonediff.diff);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone,
ISC_LOG_ERROR,
@ -5774,7 +5805,7 @@ zone_sign(dns_zone_t *zone) {
result = updatesignwithkey(signing, version,
&zone->origin,
zone->privatetype,
&sig_diff);
zonediff.diff);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR,
"updatesignwithkey -> %s",
@ -5810,7 +5841,7 @@ zone_sign(dns_zone_t *zone) {
* the signatures.
*/
result = del_sigs(zone, db, version, &zone->origin,
dns_rdatatype_nsec, &sig_diff, zone_keys,
dns_rdatatype_nsec, &zonediff, zone_keys,
nkeys, now);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR,
@ -5819,7 +5850,7 @@ zone_sign(dns_zone_t *zone) {
goto failure;
}
result = add_sigs(db, version, &zone->origin,
dns_rdatatype_nsec, &sig_diff, zone_keys,
dns_rdatatype_nsec, zonediff.diff, zone_keys,
nkeys, zone->mctx, inception, soaexpire,
check_ksk);
if (result != ISC_R_SUCCESS) {
@ -5836,7 +5867,7 @@ zone_sign(dns_zone_t *zone) {
* the signatures.
*/
result = del_sigs(zone, db, version, &zone->origin,
zone->privatetype, &sig_diff,
zone->privatetype, &zonediff,
zone_keys, nkeys, now);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR,
@ -5845,7 +5876,7 @@ zone_sign(dns_zone_t *zone) {
goto failure;
}
result = add_sigs(db, version, &zone->origin,
zone->privatetype, &sig_diff,
zone->privatetype, zonediff.diff,
zone_keys, nkeys, zone->mctx, inception,
soaexpire, check_ksk);
if (result != ISC_R_SUCCESS) {
@ -5859,13 +5890,19 @@ zone_sign(dns_zone_t *zone) {
/*
* Have we changed anything?
*/
if (ISC_LIST_HEAD(sig_diff.tuples) == NULL)
if (ISC_LIST_EMPTY(zonediff.diff->tuples)) {
/*
* Commit the changes if any key has been marked as offline.
*/
if (zonediff.offline)
dns_db_closeversion(db, &version, ISC_TRUE);
goto pauseall;
}
commit = ISC_TRUE;
result = del_sigs(zone, db, version, &zone->origin, dns_rdatatype_soa,
&sig_diff, zone_keys, nkeys, now);
&zonediff, zone_keys, nkeys, now);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR,
"zone_sign:del_sigs -> %s",
@ -5873,7 +5910,7 @@ zone_sign(dns_zone_t *zone) {
goto failure;
}
result = increment_soa_serial(db, version, &sig_diff, zone->mctx);
result = increment_soa_serial(db, version, zonediff.diff, zone->mctx);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR,
"zone_sign:increment_soa_serial -> %s",
@ -5886,8 +5923,8 @@ zone_sign(dns_zone_t *zone) {
* termination is sensible.
*/
result = add_sigs(db, version, &zone->origin, dns_rdatatype_soa,
&sig_diff, zone_keys, nkeys, zone->mctx, inception,
soaexpire, check_ksk);
zonediff.diff, zone_keys, nkeys, zone->mctx,
inception, soaexpire, check_ksk);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR,
"zone_sign:add_sigs -> %s",
@ -5910,7 +5947,7 @@ zone_sign(dns_zone_t *zone) {
goto failure;
}
result = dns_journal_write_transaction(journal, &sig_diff);
result = dns_journal_write_transaction(journal, zonediff.diff);
dns_journal_destroy(&journal);
if (result != ISC_R_SUCCESS) {
dns_zone_log(zone, ISC_LOG_ERROR,
@ -5977,7 +6014,7 @@ zone_sign(dns_zone_t *zone) {
signing = ISC_LIST_NEXT(signing, link))
dns_dbiterator_pause(signing->dbiterator);
dns_diff_clear(&sig_diff);
dns_diff_clear(&_sig_diff);
for (i = 0; i < nkeys; i++)
dst_key_free(&zone_keys[i]);