diff --git a/CHANGES b/CHANGES index 2ec07f881f..46bc523037 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,10 @@ + 617. [bug] When using dynamic update to add a new RR to an + existing RRset with a different TTL, the journal + entries generated from the update did not include + explicit deletions and re-additions of the existing + RRs to update their TTL to the new value. + 616. [func] dnssec-signzone -t output now includes performance statistics. diff --git a/bin/named/update.c b/bin/named/update.c index e8775cb488..0510ded98a 100644 --- a/bin/named/update.c +++ b/bin/named/update.c @@ -15,7 +15,7 @@ * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: update.c,v 1.77 2000/12/09 02:17:03 bwelling Exp $ */ +/* $Id: update.c,v 1.78 2000/12/16 00:58:01 gson Exp $ */ #include @@ -160,7 +160,6 @@ static void updatedone_action(isc_task_t *task, isc_event_t *event); static isc_result_t send_forward_event(ns_client_t *client, dns_zone_t *zone); /**************************************************************************/ - /* * Update a single RR in version 'ver' of 'db' and log the * update in 'diff'. @@ -204,6 +203,30 @@ do_one_tuple(dns_difftuple_t **tuple, return (ISC_R_SUCCESS); } +/* + * Perform the updates in 'updates' in version 'ver' of 'db' and log the + * update in 'diff'. + * + * Ensures: + * 'updates' is empty. + */ +static isc_result_t +do_diff(dns_diff_t *updates, dns_db_t *db, dns_dbversion_t *ver, + dns_diff_t *diff) +{ + isc_result_t result; + while (! ISC_LIST_EMPTY(updates->tuples)) { + dns_difftuple_t *t = ISC_LIST_HEAD(updates->tuples); + ISC_LIST_UNLINK(updates->tuples, t, link); + CHECK(do_one_tuple(&t, db, ver, diff)); + } + return (ISC_R_SUCCESS); + + failure: + dns_diff_clear(diff); + return (result); +} + static isc_result_t update_one_rr(dns_db_t *db, dns_dbversion_t *ver, dns_diff_t *diff, dns_diffop_t op, dns_name_t *name, @@ -536,59 +559,6 @@ rr_count(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, count_rr_action, countp)); } -/* - * Context struct for matching_rr_exists(). - */ - -typedef struct { - rr_predicate *predicate; - dns_db_t *db; - dns_dbversion_t *ver; - dns_name_t *name; - dns_rdata_t *update_rr; -} matching_rr_exists_ctx_t; - -/* - * Helper function for matching_rr_exists(). - */ - -static isc_result_t -matching_rr_exists_action(void *data, rr_t *rr) { - matching_rr_exists_ctx_t *ctx = data; - if ((*ctx->predicate)(ctx->update_rr, &rr->rdata)) - return (ISC_R_EXISTS); - return (ISC_R_SUCCESS); -} - -/* - * Compare the 'update_rr' with all RRs in the RRset specified by 'db', - * 'ver', 'name', and 'type' using 'predicate'. If the predicate returns - * true for at least one of them, set '*exists' to ISC_TRUE. Otherwise, - * set it to ISC_FALSE. - */ -static isc_result_t -matching_rr_exists(rr_predicate *predicate, - dns_db_t *db, - dns_dbversion_t *ver, - dns_name_t *name, - dns_rdatatype_t type, - dns_rdatatype_t covers, - dns_rdata_t *update_rr, - isc_boolean_t *exists) -{ - isc_result_t result; - matching_rr_exists_ctx_t ctx; - ctx.predicate = predicate; - ctx.db = db; - ctx.ver = ver; - ctx.name = name; - ctx.update_rr = update_rr; - result = foreach_rr(db, ver, name, type, covers, - matching_rr_exists_action, &ctx); - RETURN_EXISTENCE_FLAG; -} - - /* * Context struct and helper function for name_exists(). */ @@ -1006,6 +976,79 @@ delete_if(rr_predicate *predicate, delete_if_action, &ctx)); } +/**************************************************************************/ +/* + * Prepare an RR for the addition of the new RR 'ctx->update_rr', + * with TTL 'ctx->update_rr_ttl', to its rdataset, by deleting + * the RRs if it is replaced by the new RR or has a conflicting TTL. + * The necessary changes are appended to ctx->del_diff and ctx->add_diff; + * we need to do all deletions before any additions so that we don't run + * into transient states with conflicting TTLs. + */ + +typedef struct { + dns_db_t *db; + dns_dbversion_t *ver; + dns_diff_t *diff; + dns_name_t *name; + dns_rdata_t *update_rr; + dns_ttl_t update_rr_ttl; + isc_boolean_t ignore_add; + dns_diff_t del_diff; + dns_diff_t add_diff; +} add_rr_prepare_ctx_t; + +static isc_result_t +add_rr_prepare_action(void *data, rr_t *rr) { + isc_result_t result = ISC_R_SUCCESS; + add_rr_prepare_ctx_t *ctx = data; + dns_difftuple_t *tuple = NULL; + + /* + * If the update RR is a "duplicate" of the update RR, + * the update should be silently ignored. + */ + if (dns_rdata_compare(&rr->rdata, ctx->update_rr) == 0 && + rr->ttl == ctx->update_rr_ttl) { + ctx->ignore_add = ISC_TRUE; + } + + /* + * If this RR is "equal" to the update RR, it should + * be deleted before the update RR is added. + */ + if (replaces_p(ctx->update_rr, &rr->rdata)) { + CHECK(dns_difftuple_create(ctx->del_diff.mctx, + DNS_DIFFOP_DEL, ctx->name, + rr->ttl, + &rr->rdata, + &tuple)); + dns_diff_append(&ctx->del_diff, &tuple); + return (ISC_R_SUCCESS); + } + + /* + * If this RR differs in TTL from the update RR, + * its TTL must be adjusted. + */ + if (rr->ttl != ctx->update_rr_ttl) { + CHECK(dns_difftuple_create(ctx->del_diff.mctx, + DNS_DIFFOP_DEL, ctx->name, + rr->ttl, + &rr->rdata, + &tuple)); + dns_diff_append(&ctx->del_diff, &tuple); + CHECK(dns_difftuple_create(ctx->add_diff.mctx, + DNS_DIFFOP_ADD, ctx->name, + ctx->update_rr_ttl, + &rr->rdata, + &tuple)); + dns_diff_append(&ctx->add_diff, &tuple); + } + failure: + return (result); +} + /**************************************************************************/ /* * Miscellaneous subroutines. @@ -2214,29 +2257,34 @@ update_action(isc_task_t *task, isc_event_t *event) { } soa_serial_changed = ISC_TRUE; } - /* - * Add an RR. If an identical RR already exists, - * do nothing. If a similar but not identical - * CNAME, SOA, or WKS exists, remove it first. - */ - CHECK(matching_rr_exists(rr_equal_p, db, ver, name, - rdata.type, covers, &rdata, - &flag)); - if (! flag) { - isc_log_write(UPDATE_PROTOCOL_LOGARGS, - "adding an RR"); - CHECK(delete_if(replaces_p, db, ver, name, - rdata.type, covers, &rdata, - &diff)); - result = update_one_rr(db, ver, &diff, - DNS_DIFFOP_ADD, - name, ttl, &rdata); - if (result != ISC_R_SUCCESS) - FAIL(result); - } else { - isc_log_write(UPDATE_PROTOCOL_LOGARGS, - "attempt to add existing RR " - "ignored"); + + isc_log_write(UPDATE_PROTOCOL_LOGARGS, "adding an RR"); + + /* Prepare the affected RRset for the addition. */ + { + add_rr_prepare_ctx_t ctx; + ctx.db = db; + ctx.ver = ver; + ctx.diff = &diff; + ctx.name = name; + ctx.update_rr = &rdata; + ctx.update_rr_ttl = ttl; + ctx.ignore_add = ISC_FALSE; + dns_diff_init(mctx, &ctx.del_diff); + dns_diff_init(mctx, &ctx.add_diff); + CHECK(foreach_rr(db, ver, name, rdata.type, covers, + add_rr_prepare_action, &ctx)); + + if (ctx.ignore_add) { + dns_diff_clear(&ctx.del_diff); + dns_diff_clear(&ctx.add_diff); + } else { + CHECK(do_diff(&ctx.del_diff, db, ver, &diff)); + CHECK(do_diff(&ctx.add_diff, db, ver, &diff)); + CHECK(update_one_rr(db, ver, &diff, + DNS_DIFFOP_ADD, + name, ttl, &rdata)); + } } } else if (update_class == dns_rdataclass_any) { if (rdata.type == dns_rdatatype_any) { diff --git a/bin/tests/system/nsupdate/update_test.pl b/bin/tests/system/nsupdate/update_test.pl index 0b9b3da82d..9240651bc4 100644 --- a/bin/tests/system/nsupdate/update_test.pl +++ b/bin/tests/system/nsupdate/update_test.pl @@ -37,7 +37,7 @@ # # perl -MCPAN -e "install Net::DNS" # -# $Id: update_test.pl,v 1.4 2000/08/01 01:16:14 tale Exp $ +# $Id: update_test.pl,v 1.5 2000/12/16 00:58:03 gson Exp $ # use Getopt::Std; @@ -375,12 +375,40 @@ section("Updating TTLs only"); test("NOERROR", ["update", rr_add("t.$zone 300 A 73.80.65.49")]); ($a) = $res->query("t.$zone", "A")->answer; -assert($a->ttl == 300); +$ttl = $a->ttl; +assert($ttl == 300, "incorrect TTL value $ttl != 300"); test("NOERROR", ["update", rr_del("t.$zone 300 A 73.80.65.49"), rr_add("t.$zone 301 A 73.80.65.49")]); ($a) = $res->query("t.$zone", "A")->answer; -assert($a->ttl == 301); +$ttl = $a->ttl; +assert($ttl == 301, "incorrect TTL value $ttl != 301"); + +# Add an RR that is identical to an existing one except for the TTL. +# RFC2136 is not clear about what this should do; it says "duplicate RRs +# will be silently ignored" but is an RR differing only in TTL +# to be considered a duplicate or not? The test assumes that it +# should not be considered a duplicate. +test("NOERROR", ["update", rr_add("t.$zone 302 A 73.80.65.50")]); +($a) = $res->query("t.$zone", "A")->answer; +$ttl = $a->ttl; +assert($ttl == 302, "incorrect TTL value $ttl != 302"); + +section("TTL normalization"); + +# The desired behaviour is that the old RRs get their TTL +# changed to match the new one. RFC2136 does not explicitly +# specify this, but I think it makes more sense than the +# alternatives. + +test("NOERROR", ["update", rr_add("t.$zone 303 A 73.80.65.51")]); +(@answers) = $res->query("t.$zone", "A")->answer; +$nanswers = scalar @answers; +assert($nanswers == 3, "wrong number of answers $nanswers != 3"); +foreach $a (@answers) { + $ttl = $a->ttl; + assert($ttl == 303, "incorrect TTL value $ttl != 303"); +} section("Obscuring existing data by zone cut"); test("NOERROR", ["update", rr_add("a.u.$zone 300 A 73.80.65.49")]);