From: Evan Hunt <each@isc.org>
Date: Mon, 21 Mar 2011 19:54:03 +0000 (+0000)
Subject: 3087.	[bug]		DDNS updates using SIG(0) with update-policy match
X-Git-Tag: v9.9.0a1~440
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0994d3a21baeedf28cbf7e461b3bd8de5f9a6654;p=thirdparty%2Fbind9.git

3087.	[bug]		DDNS updates using SIG(0) with update-policy match
			type "external" could cause a crash. [RT #23735]
---

diff --git a/CHANGES b/CHANGES
index 83744ae8df2..2d265701ff5 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+3087.	[bug]		DDNS updates using SIG(0) with update-policy match
+			type "external" could cause a crash. [RT #23735]
+
 3086.	[bug]		Running dnssec-settime -f on an old-style key will
 			now force an update to the new key format even if no
 			other change has been specified, using "-P now -A now"
diff --git a/bin/nsupdate/nsupdate.c b/bin/nsupdate/nsupdate.c
index 6dbeb999ec6..c7c3521b9d9 100644
--- a/bin/nsupdate/nsupdate.c
+++ b/bin/nsupdate/nsupdate.c
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: nsupdate.c,v 1.194 2011/03/11 06:11:22 marka Exp $ */
+/* $Id: nsupdate.c,v 1.195 2011/03/21 19:54:02 each Exp $ */
 
 /*! \file */
 
@@ -145,7 +145,7 @@ static dns_name_t tmpzonename;
 static dns_name_t restart_master;
 static dns_tsig_keyring_t *gssring = NULL;
 static dns_tsigkey_t *tsigkey = NULL;
-static dst_key_t *sig0key;
+static dst_key_t *sig0key = NULL;
 static lwres_context_t *lwctx = NULL;
 static lwres_conf_t *lwconf;
 static isc_sockaddr_t *servers;
@@ -2880,6 +2880,9 @@ cleanup(void) {
 	}
 #endif
 
+	if (sig0key != NULL)
+		dst_key_free(&sig0key);
+
 	ddebug("Shutting down task manager");
 	isc_taskmgr_destroy(&taskmgr);
 
diff --git a/bin/tests/system/logfileconfig/ns1/named.conf b/bin/tests/system/logfileconfig/ns1/named.conf
index 81264267a93..5fb93b70cdd 100644
--- a/bin/tests/system/logfileconfig/ns1/named.conf
+++ b/bin/tests/system/logfileconfig/ns1/named.conf
@@ -14,7 +14,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: named.conf,v 1.5 2011/03/11 17:19:05 each Exp $ */
+/* $Id: named.conf,v 1.6 2011/03/21 19:54:02 each Exp $ */
 
 options {
         query-source address 10.53.0.1;
diff --git a/bin/tests/system/tsiggss/clean.sh b/bin/tests/system/tsiggss/clean.sh
index 9196e8b9e6e..eeb9aab5e56 100644
--- a/bin/tests/system/tsiggss/clean.sh
+++ b/bin/tests/system/tsiggss/clean.sh
@@ -4,5 +4,8 @@
 #
 
 rm -f ns1/*.jnl ns1/update.txt ns1/auth.sock
+rm -f ns1/*.db ns1/K*.key ns1/K*.private
+rm -f ns1/_default.tsigkeys
 rm -f */named.memstats
 rm -f authsock.pid
+rm -f ns1/core
diff --git a/bin/tests/system/tsiggss/ns1/example.nil.db b/bin/tests/system/tsiggss/ns1/example.nil.db.in
similarity index 100%
rename from bin/tests/system/tsiggss/ns1/example.nil.db
rename to bin/tests/system/tsiggss/ns1/example.nil.db.in
diff --git a/bin/tests/system/tsiggss/setup.sh b/bin/tests/system/tsiggss/setup.sh
index d5bf723ff5b..38c449213dd 100644
--- a/bin/tests/system/tsiggss/setup.sh
+++ b/bin/tests/system/tsiggss/setup.sh
@@ -14,9 +14,17 @@
 # OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 # PERFORMANCE OF THIS SOFTWARE.
 
-# $Id: setup.sh,v 1.3 2010/12/20 23:47:20 tbox Exp $
+# $Id: setup.sh,v 1.4 2011/03/21 19:54:02 each Exp $
 
 SYSTEMTESTTOP=..
 . $SYSTEMTESTTOP/conf.sh
 
-rm -f ns1/*.jnl 
+RANDFILE="random.data"
+
+
+rm -f ns1/*.jnl ns1/K*.key ns1/K*.private ns1/_default.tsigkeys
+
+../../../tools/genrandom 400 $RANDFILE
+
+key=`$KEYGEN -Cq -K ns1 -a DSA -b 512 -r $RANDFILE -n HOST -T KEY key.example.nil.`
+cat ns1/example.nil.db.in ns1/${key}.key > ns1/example.nil.db
diff --git a/bin/tests/system/tsiggss/tests.sh b/bin/tests/system/tsiggss/tests.sh
index 753de811b7c..005d0af150a 100644
--- a/bin/tests/system/tsiggss/tests.sh
+++ b/bin/tests/system/tsiggss/tests.sh
@@ -24,7 +24,7 @@ update add $host $cmd
 send
 EOF
     echo "I:testing update for $host $type $cmd"
-    $NSUPDATE -g ns1/update.txt || {
+    $NSUPDATE -g ns1/update.txt > /dev/null 2>&1 || {
 	echo "I:update failed for $host $type $cmd"
 	return 1
     }
@@ -55,13 +55,25 @@ test_update testdenied.example.nil. TXT "86400 TXT helloworld" "helloworld" || s
 
 echo "I:testing external update policy"
 test_update testcname.example.nil. TXT "86400 CNAME testdenied.example.nil" "testdenied" && status=1
-perl ./authsock.pl --type=CNAME --path=ns1/auth.sock --pidfile=authsock.pid --timeout=120 &
+perl ./authsock.pl --type=CNAME --path=ns1/auth.sock --pidfile=authsock.pid --timeout=120 > /dev/null 2>&1 &
 sleep 1
 test_update testcname.example.nil. TXT "86400 CNAME testdenied.example.nil" "testdenied" || status=1
 test_update testcname.example.nil. TXT "86400 A 10.53.0.13" "10.53.0.13" && status=1
 
+echo "I:testing external policy with SIG(0) key"
+ret=0
+$NSUPDATE -R random.data -k ns1/Kkey.example.nil.*.private <<END > /dev/null 2>&1 || ret=1
+server 10.53.0.1 5300
+zone example.nil
+update add fred.example.nil 120 cname foo.bar.
+send
+END
+output=`$DIG $DIGOPTS +short cname fred.example.nil.`
+[ -n "$output" ] || ret=1
+[ $ret -eq 0 ] || echo "I:failed"
+status=`expr $status + $ret`
+
 [ $status -eq 0 ] && echo "I:tsiggss tests all OK"
 
 kill `cat authsock.pid`
-
 exit $status
diff --git a/lib/dns/dst_api.c b/lib/dns/dst_api.c
index d43e3705606..1a26cf0bbef 100644
--- a/lib/dns/dst_api.c
+++ b/lib/dns/dst_api.c
@@ -31,7 +31,7 @@
 
 /*
  * Principal Author: Brian Wellington
- * $Id: dst_api.c,v 1.58 2011/03/17 01:40:39 each Exp $
+ * $Id: dst_api.c,v 1.59 2011/03/21 19:54:03 each Exp $
  */
 
 /*! \file */
@@ -1785,5 +1785,6 @@ dst__entropy_status(void) {
 
 isc_buffer_t *
 dst_key_tkeytoken(const dst_key_t *key) {
+	REQUIRE(VALID_KEY(key));
 	return (key->key_tkeytoken);
 }
diff --git a/lib/dns/include/dst/dst.h b/lib/dns/include/dst/dst.h
index d1e0d750ad2..d71ab1af3b6 100644
--- a/lib/dns/include/dst/dst.h
+++ b/lib/dns/include/dst/dst.h
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: dst.h,v 1.32 2011/03/17 01:40:39 each Exp $ */
+/* $Id: dst.h,v 1.33 2011/03/21 19:54:03 each Exp $ */
 
 #ifndef DST_DST_H
 #define DST_DST_H 1
@@ -864,6 +864,9 @@ dst_key_tkeytoken(const dst_key_t *key);
 /*%<
  * Return the token from the TKEY request, if any.  If this key was
  * not negotiated via TKEY, return NULL.
+ *
+ * Requires:
+ *	"key" is a valid key.
  */
 
 
diff --git a/lib/dns/sdlz.c b/lib/dns/sdlz.c
index 55b85c30eb2..25e9f986c40 100644
--- a/lib/dns/sdlz.c
+++ b/lib/dns/sdlz.c
@@ -50,7 +50,7 @@
  * USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: sdlz.c,v 1.32 2011/03/11 06:11:24 marka Exp $ */
+/* $Id: sdlz.c,v 1.33 2011/03/21 19:54:03 each Exp $ */
 
 /*! \file */
 
@@ -1680,7 +1680,7 @@ dns_sdlzssumatch(dns_name_t *signer, dns_name_t *name, isc_netaddr_t *tcpaddr,
 	char b_addr[ISC_NETADDR_FORMATSIZE];
 	char b_type[DNS_RDATATYPE_FORMATSIZE];
 	char b_key[DST_KEY_FORMATSIZE];
-	isc_buffer_t *tkey_token;
+	isc_buffer_t *tkey_token = NULL;
 	isc_region_t token_region;
 	isc_uint32_t token_len = 0;
 	isc_boolean_t ret;
@@ -1695,28 +1695,27 @@ dns_sdlzssumatch(dns_name_t *signer, dns_name_t *name, isc_netaddr_t *tcpaddr,
 	 * Format the request elements. sdlz operates on strings, not
 	 * structures
 	 */
-	if (signer)
+	if (signer != NULL)
 		dns_name_format(signer, b_signer, sizeof(b_signer));
 	else
 		b_signer[0] = 0;
 
 	dns_name_format(name, b_name, sizeof(b_name));
 
-	if (tcpaddr)
+	if (tcpaddr != NULL)
 		isc_netaddr_format(tcpaddr, b_addr, sizeof(b_addr));
 	else
 		b_addr[0] = 0;
 
 	dns_rdatatype_format(type, b_type, sizeof(b_type));
 
-	if (key)
+	if (key != NULL) {
 		dst_key_format(key, b_key, sizeof(b_key));
-	else
+		tkey_token = dst_key_tkeytoken(key);
+	} else
 		b_key[0] = 0;
 
-	tkey_token = dst_key_tkeytoken(key);
-
-	if (tkey_token) {
+	if (tkey_token != NULL) {
 		isc_buffer_region(tkey_token, &token_region);
 		token_len = token_region.length;
 	}
@@ -1724,7 +1723,7 @@ dns_sdlzssumatch(dns_name_t *signer, dns_name_t *name, isc_netaddr_t *tcpaddr,
 	MAYBE_LOCK(imp);
 	ret = imp->methods->ssumatch(b_signer, b_name, b_addr, b_type, b_key,
 				     token_len,
-				     token_len ? token_region.base : NULL,
+				     token_len != 0 ? token_region.base : NULL,
 				     imp->driverarg, dbdata);
 	MAYBE_UNLOCK(imp);
 	return (ret);
diff --git a/lib/dns/ssu_external.c b/lib/dns/ssu_external.c
index 72ab58ba1c5..701c673e0fd 100644
--- a/lib/dns/ssu_external.c
+++ b/lib/dns/ssu_external.c
@@ -14,7 +14,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: ssu_external.c,v 1.7 2011/01/13 07:05:57 marka Exp $ */
+/* $Id: ssu_external.c,v 1.8 2011/03/21 19:54:03 each Exp $ */
 
 /*
  * This implements external update-policy rules.  This allows permission
@@ -128,7 +128,7 @@ dns_ssu_external_match(dns_name_t *identity,
 	char b_addr[ISC_NETADDR_FORMATSIZE];
 	char b_type[DNS_RDATATYPE_FORMATSIZE];
 	char b_key[DST_KEY_FORMATSIZE];
-	isc_buffer_t *tkey_token;
+	isc_buffer_t *tkey_token = NULL;
 	int fd;
 	const char *sock_path;
 	size_t req_len;
@@ -154,33 +154,32 @@ dns_ssu_external_match(dns_name_t *identity,
 	if (fd == -1)
 		return (ISC_FALSE);
 
-	tkey_token = dst_key_tkeytoken(key);
+	if (key != NULL) {
+		dst_key_format(key, b_key, sizeof(b_key));
+		tkey_token = dst_key_tkeytoken(key);
+	} else
+		b_key[0] = 0;
+
+	if (tkey_token != NULL) {
+		isc_buffer_region(tkey_token, &token_region);
+		token_len = token_region.length;
+	}
 
 	/* Format the request elements */
-	if (signer)
+	if (signer != NULL)
 		dns_name_format(signer, b_signer, sizeof(b_signer));
 	else
 		b_signer[0] = 0;
 
 	dns_name_format(name, b_name, sizeof(b_name));
 
-	if (tcpaddr)
+	if (tcpaddr != NULL)
 		isc_netaddr_format(tcpaddr, b_addr, sizeof(b_addr));
 	else
 		b_addr[0] = 0;
 
 	dns_rdatatype_format(type, b_type, sizeof(b_type));
 
-	if (key)
-		dst_key_format(key, b_key, sizeof(b_key));
-	else
-		b_key[0] = 0;
-
-	if (tkey_token) {
-		isc_buffer_region(tkey_token, &token_region);
-		token_len = token_region.length;
-	}
-
 	/* Work out how big the request will be */
 	req_len = sizeof(isc_uint32_t)     + /* Format version */
 		  sizeof(isc_uint32_t)     + /* Length */
diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c
index c9c32c29866..e72e147357a 100644
--- a/lib/dns/tsig.c
+++ b/lib/dns/tsig.c
@@ -16,7 +16,7 @@
  */
 
 /*
- * $Id: tsig.c,v 1.147 2011/01/11 23:47:13 tbox Exp $
+ * $Id: tsig.c,v 1.148 2011/03/21 19:54:03 each Exp $
  */
 /*! \file */
 #include <config.h>
@@ -619,7 +619,7 @@ restore_key(dns_tsig_keyring_t *ring, isc_stdtime_t now, FILE *fp) {
 	result = dns_tsigkey_createfromkey(name, algorithm, dstkey,
 					   ISC_TRUE, creator, inception,
 					   expire, ring->mctx, ring, NULL);
-	if (result != ISC_R_SUCCESS && dstkey != NULL)
+	if (dstkey != NULL)
 		dst_key_free(&dstkey);
 	return (result);
 }