From 927e4c9fecf448bf3894c68fcaf9dc2f89557f3a Mon Sep 17 00:00:00 2001
From: Evan Hunt <each@isc.org>
Date: Tue, 9 Jul 2013 17:39:21 -0700
Subject: [PATCH] [master] address race conditions with removing inline zones

3513.	[bug]		named could crash when deleting inline-signing
			zones with "rndc delzone". [RT #34066]
---
 CHANGES                                |  3 +++
 bin/named/server.c                     | 11 ++++++++---
 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                         | 23 +++++++++++++++--------
 10 files changed, 70 insertions(+), 15 deletions(-)

diff --git a/CHANGES b/CHANGES
index ce50611698..a69a6c54c1 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+3513.	[bug]		named could crash when deleting inline-signing
+			zones with "rndc delzone". [RT #34066]
+
 3512.	[port]		Check whether to use -ljson or -ljson-c. [RT #34115]
 
 3611.	[bug]		Improved resistance to a theoretical authentication
diff --git a/bin/named/server.c b/bin/named/server.c
index c3c9a4cb13..f0de910ed4 100644
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -8481,13 +8481,13 @@ inuse(const char* file, isc_boolean_t first, isc_buffer_t *text) {
 
 	if (file != NULL && isc_file_exists(file) &&
 	    isc_buffer_availablelength(text) >
-	    strlen(file) + (first ? sizeof(INUSEMSG) : 0))
+	    strlen(file) + (first ? sizeof(INUSEMSG) : sizeof("\n")))
 	{
 		if (first)
-			isc__buffer_putstr(text, INUSEMSG);
+			isc_buffer_putstr(text, INUSEMSG);
 		else
 			isc_buffer_putstr(text, "\n");
-		isc__buffer_putstr(text, file);
+		isc_buffer_putstr(text, file);
 		return (ISC_FALSE);
 	}
 	return (first);
@@ -8670,6 +8670,7 @@ ns_server_del_zone(ns_server_t *server, char *args, isc_buffer_t *text) {
 		isc_buffer_putstr(text, "zone ");
 		isc_buffer_putstr(text, zonename);
 		isc_buffer_putstr(text, " and associated files deleted");
+		isc_buffer_putuint8(text, 0);
 	} else if (dns_zone_gettype(mayberaw) == dns_zone_slave ||
 		   dns_zone_gettype(mayberaw) == dns_zone_stub)
 	{
@@ -8688,6 +8689,8 @@ ns_server_del_zone(ns_server_t *server, char *args, isc_buffer_t *text) {
 			file = dns_zone_getjournal(zone);
 			(void)inuse(file, first, text);
 		}
+		if (isc_buffer_availablelength(text) > 0)
+			isc_buffer_putuint8(text, 0);
 	}
 
 	CHECK(dns_zt_unmount(view->zonetable, zone));
@@ -8882,6 +8885,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 036b3544c7..f811830f8f 100644
--- a/bin/rndc/rndc.c
+++ b/bin/rndc/rndc.c
@@ -269,9 +269,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 707ab31e7c..acd04c7e0f 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 9bd4dc1585..ee576c2db9 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 ebfb8d9b9a..5d22e0931e 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 ed9123fd76..4faae800e9 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 987041186d..b727604cfd 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 5ae96107e5..35969134b1 100644
--- a/lib/dns/view.c
+++ b/lib/dns/view.c
@@ -1393,6 +1393,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 643cbc25f9..0a7180ce5e 100644
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -7639,8 +7639,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) {
@@ -13244,12 +13249,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);
@@ -13320,15 +13329,13 @@ receive_secure_db(isc_task_t *task, isc_event_t *event) {
 	/*
 	 * Lock hierarchy: zmgr, zone, raw.
 	 */
-	LOCK_ZONE(zone);
 	INSIST(zone != zone->raw);
-	if (inline_secure(zone))
-		LOCK_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: