diff --git a/bin/tests/system/inline/ns3/named.conf.j2 b/bin/tests/system/inline/ns3/named.conf.j2 index 55e2048117..fd28e9da0b 100644 --- a/bin/tests/system/inline/ns3/named.conf.j2 +++ b/bin/tests/system/inline/ns3/named.conf.j2 @@ -57,6 +57,15 @@ zone "bits" { sig-signing-signatures 1; // force incremental processing }; +zone "incremental-updates" { + type primary; + file "incremental-updates.db"; + inline-signing yes; + dnssec-policy inline; + sig-signing-signatures 1; // force incremental processing + allow-update { any; }; +}; + server 10.53.0.4 { request-ixfr no; }; zone "noixfr" { diff --git a/bin/tests/system/inline/setup.sh b/bin/tests/system/inline/setup.sh index c1b38e8606..50d2a07347 100644 --- a/bin/tests/system/inline/setup.sh +++ b/bin/tests/system/inline/setup.sh @@ -31,6 +31,7 @@ cp ns3/primary.db.in ns3/nsec3.db cp ns3/primary.db.in ns3/externalkey.db cp ns3/primary.db.in ns3/delayedkeys.db cp ns3/primary.db.in ns3/removedkeys-primary.db +cp ns3/primary.db.in ns3/incremental-updates.db cp ns3/include.db.in ns3/include.db mkdir ns3/removedkeys diff --git a/bin/tests/system/inline/tests_inline_incremental_updates.py b/bin/tests/system/inline/tests_inline_incremental_updates.py new file mode 100644 index 0000000000..fdc3c18d63 --- /dev/null +++ b/bin/tests/system/inline/tests_inline_incremental_updates.py @@ -0,0 +1,65 @@ +# Copyright (C) Internet Systems Consortium, Inc. ("ISC") +# +# SPDX-License-Identifier: MPL-2.0 +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, you can obtain one at https://mozilla.org/MPL/2.0/. +# +# See the COPYRIGHT file distributed with this work for additional +# information regarding copyright ownership. + +import random +import threading + +import dns.update +import pytest + +pytestmark = pytest.mark.extra_artifacts( + [ + "K*", + "*.out*", + "*/*.out*", + "ns*/K*", + "ns*/dsset-*", + "ns*/*.bk", + "ns*/*.db", + "ns*/*.jbk", + "ns*/*.jnl", + "ns*/*.nzd", + "ns*/*.signed", + "ns*/trusted.conf", + "ns3/delayedkeys.conf", + "ns3/removedkeys", + ] +) + + +def worker(server, count) -> None: + zone = "incremental-updates" + for i in range(count): + try: + sub = random.randrange(1000000) + update_msg = dns.update.UpdateMessage(zone) + update_msg.add(f"a-{sub}-{i}.{zone}.", 300, "A", "10.0.0.1") + server.nsupdate(update_msg) + except Exception: # pylint: disable=broad-exception-caught + break + + +def test_inline_incremental_updates(ns3): + """ + Flood the server with updates to check how 'receive secure serial' + is coping with quick incremental updates. + """ + threads_n = 10 + updates_n = 10 + threads = [ + threading.Thread(target=worker, args=(ns3, updates_n), daemon=True) + for _ in range(threads_n) + ] + + for thread in threads: + thread.start() + for thread in threads: + thread.join() diff --git a/lib/dns/include/dns/update.h b/lib/dns/include/dns/update.h index ec406dca0c..b0fdf17fb5 100644 --- a/lib/dns/include/dns/update.h +++ b/lib/dns/include/dns/update.h @@ -63,4 +63,7 @@ isc_result_t dns_update_signaturesinc(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *oldver, dns_dbversion_t *newver, dns_diff_t *diff, uint32_t sigvalidityinterval, - dns_update_state_t **state); + dns_update_state_t **statep); + +void +dns_update_state_clear(dns_update_state_t **statep, bool destroy); diff --git a/lib/dns/update.c b/lib/dns/update.c index e1cb886713..276a06f762 100644 --- a/lib/dns/update.c +++ b/lib/dns/update.c @@ -1310,6 +1310,7 @@ dns_update_signatures(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db, struct dns_update_state { unsigned int magic; + isc_mem_t *mctx; dns_diff_t diffnames; dns_diff_t affected; dns_diff_t sig_diff; @@ -1366,10 +1367,9 @@ dns_update_signaturesinc(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db, dns_diff_t *diff, uint32_t sigvalidityinterval, dns_update_state_t **statep) { isc_result_t result = ISC_R_SUCCESS; - dns_update_state_t mystate, *state = NULL; + dns_update_state_t mystate = { 0 }, *state = NULL; dns_difftuple_t *tuple = NULL; bool flag, build_nsec; - unsigned int i; dns_rdata_soa_t soa; dns_rdata_t rdata = DNS_RDATA_INIT; dns_rdataset_t rdataset; @@ -1385,7 +1385,10 @@ dns_update_signaturesinc(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db, state = &mystate; } else { state = isc_mem_get(diff->mctx, sizeof(*state)); + state->mctx = NULL; + isc_mem_attach(diff->mctx, &state->mctx); } + state->magic = STATE_MAGIC; dns_diff_init(diff->mctx, &state->diffnames); dns_diff_init(diff->mctx, &state->affected); @@ -1444,7 +1447,6 @@ dns_update_signaturesinc(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db, */ CHECK(dns_diff_sort(diff, temp_order)); state->state = sign_updates; - state->magic = STATE_MAGIC; SET_IF_NOT_NULL(statep, state); } else { REQUIRE(DNS_STATE_VALID(*statep)); @@ -2005,6 +2007,18 @@ cleanup: dns_db_detachnode(&node); } + dns_update_state_clear(&state, state != &mystate); + SET_IF_NOT_NULL(statep, NULL); + + return result; +} + +void +dns_update_state_clear(dns_update_state_t **statep, bool destroy) { + REQUIRE(DNS_STATE_VALID(*statep)); + + dns_update_state_t *state = *statep; + dns_diff_clear(&state->sig_diff); dns_diff_clear(&state->nsec_diff); dns_diff_clear(&state->nsec_mindiff); @@ -2013,17 +2027,15 @@ cleanup: dns_diff_clear(&state->diffnames); dns_diff_clear(&state->work); - for (i = 0; i < state->nkeys; i++) { + for (size_t i = 0; i < state->nkeys; i++) { dst_key_free(&state->zone_keys[i]); } - if (state != &mystate) { + if (destroy) { *statep = NULL; state->magic = 0; - isc_mem_put(diff->mctx, state, sizeof(*state)); + isc_mem_putanddetach(&state->mctx, state, sizeof(*state)); } - - return result; } static isc_stdtime_t diff --git a/lib/dns/zone.c b/lib/dns/zone.c index d6a597abfd..d50cbbaf34 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -593,6 +593,10 @@ dns__zone_free(dns_zone_t *zone) { isc_mem_put(zone->mctx, include, sizeof *include); } + if (zone->rss_state != NULL) { + dns_update_state_clear(&zone->rss_state, true); + } + if (zone->masterfile != NULL) { isc_mem_free(zone->mctx, zone->masterfile); } @@ -13799,7 +13803,6 @@ struct rss { dns_zone_t *zone; dns_db_t *db; uint32_t serial; - ISC_LINK(struct rss) link; }; static void @@ -14392,18 +14395,51 @@ receive_secure_serial(void *arg) { LOCK_ZONE(zone); - /* - * The receive_secure_serial() is loop-serialized for the zone. Make - * sure there's no processing currently running. - */ + if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING) || + !dns__zone_inline_secure(zone)) + { + /* + * If this is a callback for a new secure serial that was + * never processed and the zone is shutting down, then just + * free 'rss_next' and return. + */ + if (rss == zone->rss_next) { + isc_mem_put(zone->mctx, rss, sizeof(*rss)); + zone->rss_next = NULL; + UNLOCK_ZONE(zone); + dns_zone_idetach(&zone); + return; + } - INSIST(zone->rss == NULL || zone->rss == rss); - - if (zone->rss != NULL) { - INSIST(zone->rss == rss); + /* Otherwise, this is an ongoing processing, do the cleanup. */ UNLOCK_ZONE(zone); + CLEANUP(ISC_R_SHUTTINGDOWN); + } + + /* + * The receive_secure_serial() is loop-serialized for the zone, but it + * is possible for new serial to arrive before the old processing is + * completed, because the old call could have been rescheduled if + * dns_update_signaturesinc() below returned DNS_R_CONTINUE. + */ + if (zone->rss != NULL) { + /* There is an existing processing */ + UNLOCK_ZONE(zone); + if (zone->rss != rss) { + /* + * A new 'rss' is sandwiched between a previous call + * with the old 'zone->rss' and a future scheduled + * call with the old 'zone->rss'. Reschedule the new + * 'rss' so the old one can be completed first. + */ + isc_async_run(zone->loop, receive_secure_serial, rss); + return; + } } else { - zone->rss = rss; + /* New arrival */ + INSIST(rss == zone->rss_next); + zone->rss = MOVE_OWNERSHIP(zone->rss_next); + dns_diff_init(zone->mctx, &zone->rss_diff); /* @@ -14519,6 +14555,7 @@ receive_secure_serial(void *arg) { &log, zone, zone->rss_db, zone->rss_oldver, zone->rss_newver, &zone->rss_diff, zone->sigvalidityinterval, &zone->rss_state); if (result == DNS_R_CONTINUE) { + /* Signing quantum reached, reschedule the next chunk. */ if (rjournal != NULL) { dns_journal_destroy(&rjournal); } @@ -14580,7 +14617,7 @@ cleanup: if (zone->rss_raw != NULL) { dns_zone_detach(&zone->rss_raw); } - if (result != ISC_R_SUCCESS) { + if (result != ISC_R_SUCCESS && result != ISC_R_SHUTTINGDOWN) { LOCK_ZONE(zone); dns__zone_set_resigntime(zone); timenow = isc_time_now(); @@ -14623,15 +14660,24 @@ static isc_result_t zone_send_secureserial(dns_zone_t *zone, uint32_t serial) { struct rss *rss = NULL; - rss = isc_mem_get(zone->secure->mctx, sizeof(*rss)); - *rss = (struct rss){ - .serial = serial, - .link = ISC_LINK_INITIALIZER, - }; - INSIST(LOCKED_ZONE(zone->secure)); - zone_iattach(zone->secure, &rss->zone); - isc_async_run(zone->secure->loop, receive_secure_serial, rss); + + if (zone->secure->rss_next != NULL) { + /* + * New version came earlier than the previous one had a + * chance to be processed, just update the serial. + */ + zone->secure->rss_next->serial = serial; + } else { + rss = isc_mem_get(zone->secure->mctx, sizeof(*rss)); + *rss = (struct rss){ + .serial = serial, + }; + zone_iattach(zone->secure, &rss->zone); + + zone->secure->rss_next = rss; + isc_async_run(zone->secure->loop, receive_secure_serial, rss); + } DNS_ZONE_CLRFLAG(zone, DNS_ZONEFLG_SENDSECURE); return ISC_R_SUCCESS; @@ -15057,7 +15103,7 @@ zone_send_securedb(dns_zone_t *zone, dns_db_t *db) { struct rss *rss = NULL; rss = isc_mem_get(zone->secure->mctx, sizeof(*rss)); - *rss = (struct rss){ .link = ISC_LINK_INITIALIZER }; + *rss = (struct rss){ 0 }; INSIST(LOCKED_ZONE(zone->secure)); zone_iattach(zone->secure, &rss->zone); diff --git a/lib/dns/zone_p.h b/lib/dns/zone_p.h index f7428c3a24..4e40944358 100644 --- a/lib/dns/zone_p.h +++ b/lib/dns/zone_p.h @@ -582,6 +582,7 @@ struct dns_zone { dns_db_t *rss_db; dns_zone_t *rss_raw; struct rss *rss; + struct rss *rss_next; dns_update_state_t *rss_state; isc_stats_t *gluecachestats;