From c2cb8c8fc0486a0a673a7fc8f6e01f5522955a7f Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 9 Jul 2013 17:50:43 -0700 Subject: [PATCH] [v9_9] address race conditions with removing inline zones 3513. [bug] named could crash when deleting inline-signing zones with "rndc delzone". [RT #34066] (cherry picked from commit 927e4c9fecf448bf3894c68fcaf9dc2f89557f3a) --- CHANGES | 3 +++ bin/named/server.c | 2 ++ bin/rndc/rndc.c | 7 ++++--- bin/tests/system/inline/clean.sh | 4 ++++ bin/tests/system/inline/ns2/named.conf | 5 ++++- bin/tests/system/inline/ns3/named.conf | 1 + bin/tests/system/inline/ns3/sign.sh | 13 +++++++++++++ bin/tests/system/inline/tests.sh | 16 ++++++++++++++++ lib/dns/view.c | 2 ++ lib/dns/zone.c | 24 ++++++++++++++++-------- 10 files changed, 65 insertions(+), 12 deletions(-) diff --git a/CHANGES b/CHANGES index 3bf7d19b8f..68897146d0 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +3513. [bug] named could crash when deleting inline-signing + zones with "rndc delzone". [RT #34066] + 3611. [bug] Improved resistance to a theoretical authentication attack based on differential timing. [RT #33939] diff --git a/bin/named/server.c b/bin/named/server.c index f28504758a..67a23c5817 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -8442,6 +8442,8 @@ ns_server_signing(ns_server_t *server, char *args, isc_buffer_t *text) { isc_buffer_add(text, n); } + if (!first && isc_buffer_availablelength(text) > 0) + isc_buffer_putuint8(text, 0); if (result == ISC_R_NOMORE) result = ISC_R_SUCCESS; diff --git a/bin/rndc/rndc.c b/bin/rndc/rndc.c index 7d66fbfa7b..ba2c3f6d55 100644 --- a/bin/rndc/rndc.c +++ b/bin/rndc/rndc.c @@ -265,9 +265,10 @@ rndc_recvdone(isc_task_t *task, isc_event_t *event) { progname, isc_result_totext(result)); result = isccc_cc_lookupstring(data, "text", &textmsg); - if (result == ISC_R_SUCCESS) - printf("%s\n", textmsg); - else if (result != ISC_R_NOTFOUND) + if (result == ISC_R_SUCCESS) { + if (strlen(textmsg) != 0U) + printf("%s\n", textmsg); + } else if (result != ISC_R_NOTFOUND) fprintf(stderr, "%s: parsing response failed: %s\n", progname, isc_result_totext(result)); diff --git a/bin/tests/system/inline/clean.sh b/bin/tests/system/inline/clean.sh index ec71fd3b42..d0d8ad4dcd 100644 --- a/bin/tests/system/inline/clean.sh +++ b/bin/tests/system/inline/clean.sh @@ -71,3 +71,7 @@ rm -f dig.out.ns*.test* rm -f signing.out* rm -f freeze.test* rm -f thaw.test* +rm -f */*.nzf +rm -f ns3/test-?.bk +rm -f ns3/test-?.bk.signed +rm -f ns3/test-?.bk.signed.jnl diff --git a/bin/tests/system/inline/ns2/named.conf b/bin/tests/system/inline/ns2/named.conf index 0da29e6c67..f252f0d17e 100644 --- a/bin/tests/system/inline/ns2/named.conf +++ b/bin/tests/system/inline/ns2/named.conf @@ -18,7 +18,9 @@ // NS2 -controls { /* empty */ }; +include "../../common/rndc.key"; + +controls { inet 10.53.0.2 port 9953 allow { any; } keys { rndc_key; }; }; options { query-source address 10.53.0.2; @@ -31,6 +33,7 @@ options { recursion no; notify yes; notify-delay 0; + allow-new-zones yes; }; zone "bits" { diff --git a/bin/tests/system/inline/ns3/named.conf b/bin/tests/system/inline/ns3/named.conf index 346d2b0236..88c7148550 100644 --- a/bin/tests/system/inline/ns3/named.conf +++ b/bin/tests/system/inline/ns3/named.conf @@ -34,6 +34,7 @@ options { notify yes; try-tcp-refresh no; notify-delay 0; + allow-new-zones yes; }; zone "bits" { diff --git a/bin/tests/system/inline/ns3/sign.sh b/bin/tests/system/inline/ns3/sign.sh index b620902632..41ebfb30a5 100644 --- a/bin/tests/system/inline/ns3/sign.sh +++ b/bin/tests/system/inline/ns3/sign.sh @@ -73,3 +73,16 @@ rm -f K${zone}.+*+*.private keyname=`$KEYGEN -q -r $RANDFILE -a RSASHA1 -b 768 -n zone $zone` keyname=`$KEYGEN -q -r $RANDFILE -a RSASHA1 -b 1024 -n zone -f KSK $zone` $DSFROMKEY -T 1200 $keyname >> ../ns1/root.db + +for s in a c d h k l m q z +do + zone=test-$s + keyname=`$KEYGEN -q -r $RANDFILE -a RSASHA1 -b 768 -n zone $zone` +done + +for s in b f i o p t v +do + zone=test-$s + keyname=`$KEYGEN -q -r $RANDFILE -a RSASHA1 -b 768 -n zone $zone` + keyname=`$KEYGEN -q -r $RANDFILE -a RSASHA1 -b 1024 -n zone -f KSK $zone` +done diff --git a/bin/tests/system/inline/tests.sh b/bin/tests/system/inline/tests.sh index e2cd0b6d14..182a2c5978 100644 --- a/bin/tests/system/inline/tests.sh +++ b/bin/tests/system/inline/tests.sh @@ -776,4 +776,20 @@ done if [ $ret != 0 ]; then echo "I:failed"; fi status=`expr $status + $ret` +ret=0 +echo "I:test add/del zone combinations" +for zone in a b c d e f g h i j k l m n o p q r s t u v w x y z +do +$RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 addzone test-$zone \ + '{ type master; file "bits.db.in"; allow-transfer { any; }; };' +$DIG $DIGOPTS @10.53.0.2 -p 5300 test-$zone SOA > dig.out.ns2.$zone.test$n +grep "status: NOERROR," dig.out.ns2.$zone.test$n > /dev/null || { ret=1; cat dig.out.ns2.$zone.test$n; } +$RNDC -c ../common/rndc.conf -s 10.53.0.3 -p 9953 addzone test-$zone \ + '{ type slave; masters { 10.53.0.2; }; file "'test-$zone.bk'"; inline-signing yes; auto-dnssec maintain; allow-transfer { any; }; };' +$RNDC -c ../common/rndc.conf -s 10.53.0.3 -p 9953 delzone test-$zone +done + +if [ $ret != 0 ]; then echo "I:failed"; fi +status=`expr $status + $ret` + exit $status diff --git a/lib/dns/view.c b/lib/dns/view.c index 7daf64fae7..142b09edbd 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -1427,6 +1427,8 @@ dns_viewlist_findzone(dns_viewlist_t *list, dns_name_t *name, dns_zone_t **zp = NULL;; REQUIRE(list != NULL); + REQUIRE(zonep != NULL && *zonep == NULL); + for (view = ISC_LIST_HEAD(*list); view != NULL; view = ISC_LIST_NEXT(view, link)) { diff --git a/lib/dns/zone.c b/lib/dns/zone.c index bd2ad74a0b..e8334060b6 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -7340,8 +7340,13 @@ zone_sign(dns_zone_t *zone) { } ZONEDB_LOCK(&zone->dblock, isc_rwlocktype_read); - dns_db_attach(zone->db, &db); + if (zone->db != NULL) + dns_db_attach(zone->db, &db); ZONEDB_UNLOCK(&zone->dblock, isc_rwlocktype_read); + if (db == NULL) { + result = ISC_R_FAILURE; + goto failure; + } result = dns_db_newversion(db, &version); if (result != ISC_R_SUCCESS) { @@ -12891,12 +12896,16 @@ receive_secure_db(isc_task_t *task, isc_event_t *event) { rawdb = ((struct secure_event *)event)->db; isc_event_free(&event); - REQUIRE(inline_secure(zone)); - dns_fixedname_init(&fname); name = dns_fixedname_name(&fname); dns_rdataset_init(&rdataset); + LOCK_ZONE(zone); + if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING) || !inline_secure(zone)) { + result = ISC_R_SHUTTINGDOWN; + goto unlock; + } + TIME_NOW(&loadtime); if (zone->db != NULL) { result = dns_db_getsoaserial(zone->db, NULL, &oldserial); @@ -12967,14 +12976,13 @@ receive_secure_db(isc_task_t *task, isc_event_t *event) { /* * Lock hierarchy: zmgr, zone, raw. */ - LOCK_ZONE(zone); - if (inline_secure(zone)) - LOCK_ZONE(zone->raw); + INSIST(zone != zone->raw); + LOCK_ZONE(zone->raw); DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_NEEDNOTIFY); result = zone_postload(zone, db, loadtime, ISC_R_SUCCESS); zone_needdump(zone, 0); /* XXXMPA */ - if (inline_secure(zone)) - UNLOCK_ZONE(zone->raw); + UNLOCK_ZONE(zone->raw); + unlock: UNLOCK_ZONE(zone); failure: