From cb40c5229aae51c97e9de6f60a58b55c9ffb2447 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Tue, 14 Aug 2018 09:22:43 +0200 Subject: [PATCH 1/2] Queue "rndc signing -nsec3param ..." requests if needed If "rndc signing -nsec3param ..." is ran for a zone which has not yet been loaded or transferred (i.e. its "db" field is NULL), it will be silently ignored by named despite rndc logging an "nsec3param request queued" message, which is misleading. Prevent this by keeping a per-zone queue of NSEC3PARAM change requests which arrive before a zone is loaded or transferred and processing that queue once the raw version of an inline-signed zone becomes available. --- bin/tests/system/inline/ns2/named.conf.in | 1 + bin/tests/system/inline/tests.sh | 52 ++++++++++++++++------- lib/dns/zone.c | 44 ++++++++++++++++++- 3 files changed, 79 insertions(+), 18 deletions(-) diff --git a/bin/tests/system/inline/ns2/named.conf.in b/bin/tests/system/inline/ns2/named.conf.in index dbe5967755..c4da5080cf 100644 --- a/bin/tests/system/inline/ns2/named.conf.in +++ b/bin/tests/system/inline/ns2/named.conf.in @@ -48,6 +48,7 @@ zone "retransfer3" { type master; file "retransfer3.db"; allow-update { any; }; + allow-transfer { none; }; // changed dynamically by tests.sh notify no; }; diff --git a/bin/tests/system/inline/tests.sh b/bin/tests/system/inline/tests.sh index 5bff5254f4..262594fc30 100755 --- a/bin/tests/system/inline/tests.sh +++ b/bin/tests/system/inline/tests.sh @@ -27,22 +27,6 @@ do sleep 1 done -# Loop until retransfer3 has been transferred. -for i in 1 2 3 4 5 6 7 8 9 0 -do - ans=0 - $RNDCCMD 10.53.0.3 signing -nsec3param 1 0 0 - retransfer3 > /dev/null 2>&1 || ans=1 - [ $ans = 0 ] && break - sleep 1 -done - -for i in 1 2 3 4 5 6 7 8 9 0 -do - nsec3param=`$DIG $DIGOPTS +nodnssec +short @10.53.0.3 nsec3param retransfer3.` - test "$nsec3param" = "1 0 0 -" && break - sleep 1 -done - n=`expr $n + 1` echo_i "checking that rrsigs are replaced with ksk only ($n)" ret=0 @@ -770,9 +754,45 @@ done if [ $ret != 0 ]; then echo_i "failed"; fi status=`expr $status + $ret` +n=`expr $n + 1` +echo_i "check 'rndc signing -nsec3param' requests are queued for zones which are not loaded ($n)" +ret=0 +# The "retransfer3" zone is configured with "allow-transfer { none; };" on ns2, +# which means it should not yet be available on ns3. +$DIG $DIGOPTS @10.53.0.3 retransfer3 SOA > dig.out.ns3.pre.test$n +grep "status: SERVFAIL" dig.out.ns3.pre.test$n > /dev/null || ret=1 +# Switch the zone to NSEC3. An "NSEC3 -> NSEC -> NSEC3" sequence is used purely +# to test that multiple queued "rndc signing -nsec3param" requests are handled +# properly. +$RNDCCMD 10.53.0.3 signing -nsec3param 1 0 0 - retransfer3 > /dev/null 2>&1 || ret=1 +$RNDCCMD 10.53.0.3 signing -nsec3param none retransfer3 > /dev/null 2>&1 || ret=1 +$RNDCCMD 10.53.0.3 signing -nsec3param 1 0 0 - retransfer3 > /dev/null 2>&1 || ret=1 +# Reconfigure ns2 to allow outgoing transfers for the "retransfer3" zone. +sed "s|\(allow-transfer { none; };.*\)|// \1|;" ns2/named.conf > ns2/named.conf.new +mv ns2/named.conf.new ns2/named.conf +$RNDCCMD 10.53.0.2 reconfig || ret=1 +# Request ns3 to retransfer the "retransfer3" zone. +$RNDCCMD 10.53.0.3 retransfer retransfer3 || ret=1 +# Wait until ns3 finishes building the NSEC3 chain for "retransfer3". There is +# no need to immediately set ret=1 if the expected message does not appear in +# the log within the time limit because the query we will send shortly will +# detect problems anyway. +for i in 0 1 2 3 4 5 6 7 8 9 +do + grep "add.*retransfer3.*NSEC3PARAM 1 0 0 -" ns3/named.run > /dev/null && break + sleep 1 +done +# Check whether "retransfer3" uses NSEC3 as requested. +$DIG $DIGOPTS @10.53.0.3 nonexist.retransfer3 A > dig.out.ns3.post.test$n +grep "status: NXDOMAIN" dig.out.ns3.post.test$n > /dev/null || ret=1 +grep "NSEC3" dig.out.ns3.post.test$n > /dev/null || ret=1 +if [ $ret != 0 ]; then echo_i "failed"; fi +status=`expr $status + $ret` + n=`expr $n + 1` echo_i "check rndc retransfer of a inline nsec3 slave retains nsec3 ($n)" ret=0 +$RNDCCMD 10.53.0.3 signing -nsec3param 1 0 0 - retransfer3 > /dev/null 2>&1 || ret=1 for i in 0 1 2 3 4 5 6 7 8 9 do ans=0 diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 2150a98f3f..488108b13e 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -357,6 +357,10 @@ struct dns_zone { */ dns_signinglist_t signing; dns_nsec3chainlist_t nsec3chain; + /*% + * List of outstanding NSEC3PARAM change requests. + */ + isc_eventlist_t setnsec3param_queue; /*% * Signing / re-signing quantum stopping parameters. */ @@ -1039,6 +1043,7 @@ dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx) { zone->isselfarg = NULL; ISC_LIST_INIT(zone->signing); ISC_LIST_INIT(zone->nsec3chain); + ISC_LIST_INIT(zone->setnsec3param_queue); zone->signatures = 10; zone->nodes = 100; zone->privatetype = (dns_rdatatype_t)0xffffU; @@ -1116,6 +1121,7 @@ zone_free(dns_zone_t *zone) { isc_mem_t *mctx = NULL; dns_signing_t *signing; dns_nsec3chain_t *nsec3chain; + isc_event_t *setnsec3param_event; dns_include_t *include; REQUIRE(DNS_ZONE_VALID(zone)); @@ -1149,6 +1155,12 @@ zone_free(dns_zone_t *zone) { } /* Unmanaged objects */ + while (!ISC_LIST_EMPTY(zone->setnsec3param_queue)) { + setnsec3param_event = ISC_LIST_HEAD(zone->setnsec3param_queue); + ISC_LIST_UNLINK(zone->setnsec3param_queue, setnsec3param_event, + ev_link); + isc_event_free(&setnsec3param_event); + } for (signing = ISC_LIST_HEAD(zone->signing); signing != NULL; signing = ISC_LIST_HEAD(zone->signing)) { @@ -14750,6 +14762,8 @@ receive_secure_db(isc_task_t *task, isc_event_t *event) { unsigned int oldserial = 0; bool have_oldserial = false; nsec3paramlist_t nsec3list; + isc_event_t *setnsec3param_event; + dns_zone_t *dummy; UNUSED(task); @@ -14874,6 +14888,18 @@ receive_secure_db(isc_task_t *task, isc_event_t *event) { zone_needdump(zone, 0); /* XXXMPA */ UNLOCK_ZONE(zone->raw); + /* + * Process any queued NSEC3PARAM change requests. + */ + while (!ISC_LIST_EMPTY(zone->setnsec3param_queue)) { + setnsec3param_event = ISC_LIST_HEAD(zone->setnsec3param_queue); + ISC_LIST_UNLINK(zone->setnsec3param_queue, setnsec3param_event, + ev_link); + dummy = NULL; + zone_iattach(zone, &dummy); + isc_task_send(zone->task, &setnsec3param_event); + } + failure: UNLOCK_ZONE(zone); if (result != ISC_R_SUCCESS) @@ -19093,8 +19119,22 @@ dns_zone_setnsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags, np->nsec = false; } - zone_iattach(zone, &dummy); - isc_task_send(zone->task, &e); + /* + * setnsec3param() will silently return early if the zone does not yet + * have a database. Prevent that by queueing the event up if zone->db + * is NULL. All events queued here are subsequently processed by + * receive_secure_db() if it ever gets called or simply freed by + * zone_free() otherwise. + */ + ZONEDB_LOCK(&zone->dblock, isc_rwlocktype_read); + if (zone->db != NULL) { + zone_iattach(zone, &dummy); + isc_task_send(zone->task, &e); + } else { + ISC_LIST_APPEND(zone->setnsec3param_queue, e, ev_link); + e = NULL; + } + ZONEDB_UNLOCK(&zone->dblock, isc_rwlocktype_read); failure: if (e != NULL) From eed6778be4493955e16393b07bec59a8da6e728d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Tue, 14 Aug 2018 09:22:43 +0200 Subject: [PATCH 2/2] Add CHANGES entry 5008. [bug] "rndc signing -nsec3param ..." requests were silently ignored for zones which were not yet loaded or transferred. [GL #468] --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index 791851b185..1464989720 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5008. [bug] "rndc signing -nsec3param ..." requests were silently + ignored for zones which were not yet loaded or + transferred. [GL #468] + 5007. [cleanup] Replace custom ISC boolean and integer data types with C99 stdint.h and stdbool.h types. [GL #9]