4379. [bug] An INSIST could be triggered if a zone contains

RRSIG records with expiry fields that loop using serial number arithmetic. [RT #40571] (cherry picked from commit 9268297baa)
2016-05-27 15:24:30 +10:00
parent 0017ccff05
commit 19a4c5035c
4 changed files with 117 additions and 11 deletions
--- a/4
+++ b/4
@@ -1,3 +1,7 @@
+4379.   [bug]           An INSIST could be triggered if a zone contains
+			RRSIG records with expiry fields that loop
+			using serial number arithmetic. [RT #40571]
+
 4378.	[contrib]	#include <isc/string.h> for strlcat in zone2ldap.c.
 			[RT #42525]

--- a/bin/tests/system/checkzone/tests.sh
+++ b/bin/tests/system/checkzone/tests.sh
@@ -141,5 +141,34 @@ n=`expr $n + 1`
 if [ $ret != 0 ]; then echo "I:failed"; fi
 status=`expr $status + $ret`

+echo "I:checking that expirations that loop using serial arithmetic are handled ($n)"
+ret=0
+q=-q
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1
+n=`expr $n + 1`
+if [ $ret != 0 ]; then echo "I:failed"; fi
+status=`expr $status + $ret`
+
 echo "I:exit status: $status"
 exit $status
--- a/bin/tests/system/checkzone/zones/crashzone.db
+++ b/bin/tests/system/checkzone/zones/crashzone.db
@@ -0,0 +1,53 @@
+dyn.example.net.	7200	IN SOA	ns1.example.net. hostmaster.example.net. (
+					6  ; serial
+					43200      ; refresh (12 hours)
+					1800       ; retry (30 minutes)
+					1209600    ; expire (2 weeks)
+					7200       ; minimum (2 hours)
+					)
+			7200	RRSIG	SOA 7 3 7200 2010	20100225214229 30323 dyn.example.net.
+			7200	NS	ns1.example.net.
+			7200	NS	ns2.example.net.
+			3600	RRSIG	DNSKEY 7 3 3600 20100227180048 (
+					20100221180048 52935 dyn.example.net.
+					MuyIUCa3XlttWuSnaQegQnRgTrTsx0Mj4EGI
+					fwtZs2H3L079Y/brqMvtlIGxtlr9meLg43oo
+					jX1w48ilerzf1PwYhtVpFefZTgmClK0h2ej4
+					Ho9Qh4/6snesVj06kWsQDkhuVs58zHmhRtEy
+					P4YlqP/R1CAk166RhwSmGuSx1O8= )
+			0	NSEC3PARAM 1 0 10 76931F
+ns1.dyn.example.net.	7200	IN A	1.0.0.5
+			7200	AAAA	2001:db8::53
+			7200	RRSIG	AAAA 7 4 7200 20100227180048 (
+					20100221180048 30323 dyn.example.net.
+					dk1DfG0y9qjCi3VD4e9B1NGKWEig7q8hFdaR
+					3hElCIzGlflvgHRiE7iTJxDMB+kTA0by4BMZ
+					yssUuXP2FMlB2g== )
+ns2.dyn.example.net.	7200	IN A	1.2.0.6
+y.dyn.example.net.	7200	IN A	1.2.3.5
+z.dyn.example.net.	7200	IN A	1.2.3.6
+A54T6DKFVU4QCKFFNJ0KEU0FH0I4OJSN.dyn.example.net. 7200 IN NSEC3	1 0 10 76931F AJHVGTICN6K0VDA53GCHFMT219SRRQLM A RRSIG
+ò		7200	RRSIG	NSEC3 7 4 7200 00100227180048 (
+					20100221180048 30323 dyn.example.net.
+					9BhZcQdLwRPU/Dz38uMis/nCcddyhKEm0Zb+
+					Mhh3V3OsGI202cebTaxbwVEbQQOeowpUmf8l
+					AmK/cNX7+IS2rw== )
+AJHVGTICN6K0VDA53GCHFMT219SRRQLM.dyn.example.net. 7200 IN NSEC3	1 0 10 76931F FQ7RBG86KRMACA1NAAKP2KQRQALBA0C7 A RRSIG
+FQ7RBG86KRMACA1NAAKP2KQRQALBA0C7.dyn.example.net.  7200	RRSIG	NSEC3 7 4 7200 20100227180048 (
+					20100221180048 30323 dyn.example.net.
+					577WZnTQemStx+diON9rEGXAGnU7C0KLjrFL
+					VyhocnBnNtxJS8eRMSWvb9XuYCMNhYKOurtt
+					Ar4qh4VW1+unmA== )
+I7A7A184GGMI35K1E3IR650LKO7NOB5R.dyn.example.net. 7200 IN NSEC3	1 0 10 76931F IMQ912BREQP1POLAH3RMONG;UED541AS A RRSIG
+IMQ912BREQP1POLAH3RMONG3UED541AS.dyn.example.net. 7200 IN NSEC3	1 0 10 76931F S3USV4M1HLVJ8F88EDSG8N9PVQRQ20N7 A RRSIG
+			7200	RRSIG	NSEC3 7 4 7200 20100227180048 (
+					20100221180048 30323 dyn.example.net.
+					smsg35snQ9PpeG2r8ZGxBl44pwSReh/1rIil
+					u/n8aa5nKbBpkqtbcc7q1OpUgb1Q7+Tl/wes
+					kB6bJA== )
+S3USV4M1HLVJ8F88EDSG8N9PVQRQ20N7.dyn.example.net.  7200	RRSIG	NSEC3 7 4 7200 20100227180048 (
+					20100221180048 30323 dyn.example.net.
+					XalRIESpdeVK1aNbwu9ym2SpK981Y127rKua
+					xsoals0Zn2tTjF9wpOYVGVOto3FcWBbyKD1g
+					69BTRlv634UIOw== )
+T320G5LC07QE1BLR074KORIJTG9DPTI9.dyn.example.net. 7200 IN NSEC3	1 0 10 76931F A54T6DKFVU4QCAFFNJ0KEU0FH0I4OJSN NS SOA RRSIG DNSKEY NSEC3PARAM
--- a/lib/dns/rbtdb.c
+++ b/lib/dns/rbtdb.c
@@ -70,6 +70,7 @@
 #include <dns/rdatastruct.h>
 #include <dns/result.h>
 #include <dns/stats.h>
+#include <dns/time.h>
 #include <dns/version.h>
 #include <dns/view.h>
 #include <dns/zone.h>
@@ -412,6 +413,7 @@ typedef struct rdatasetheader {
 	unsigned int 			is_mmapped : 1;
 	unsigned int 			next_is_relative : 1;
 	unsigned int 			node_is_relative : 1;
+	unsigned int 			resign_lsb : 1;
 	/*%<
 	 * We don't use the LIST macros, because the LIST structure has
 	 * both head and tail pointers, and is doubly linked.
@@ -1048,7 +1050,8 @@ resign_sooner(void *v1, void *v2) {
 	rdatasetheader_t *h1 = v1;
 	rdatasetheader_t *h2 = v2;

-	if (isc_serial_lt(h1->resign, h2->resign))
+	if (h1->resign < h2->resign ||
+	    (h1->resign == h2->resign && h1->resign_lsb < h2->resign_lsb))
 		return (ISC_TRUE);
 	return (ISC_FALSE);
 }
@@ -3197,7 +3200,7 @@ bind_rdataset(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node,
 	 */
 	if (RESIGN(header)) {
 		rdataset->attributes |=  DNS_RDATASETATTR_RESIGN;
-		rdataset->resign = header->resign;
+		rdataset->resign = (header->resign << 1) | header->resign_lsb;
 	} else
 		rdataset->resign = 0;
 }
@@ -5559,7 +5562,8 @@ printnode(dns_db_t *db, dns_dbnode_t *node, FILE *out) {
 					current->rdh_ttl,
 					current->trust,
 					current->attributes,
-					current->resign);
+					(current->resign << 1) |
+					current->resign_lsb);
 				current = current->down;
 			} while (current != NULL);
 		}
@@ -6182,8 +6186,11 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_version_t *rbtversion,
 				update_newheader(newheader, header);
 				if (loading && RESIGN(newheader) &&
 				    RESIGN(header) &&
-				    header->resign < newheader->resign)
+				    header->resign < newheader->resign) {
 					newheader->resign = header->resign;
+					newheader->resign_lsb =
+							header->resign_lsb;
+				}
 			} else {
 				free_rdataset(rbtdb, rbtdb->common.mctx,
 					      newheader);
@@ -6589,12 +6596,17 @@ addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,

 		if ((rdataset->attributes & DNS_RDATASETATTR_RESIGN) != 0) {
 			newheader->attributes |= RDATASET_ATTR_RESIGN;
-			newheader->resign = rdataset->resign;
-		} else
+			newheader->resign =
+			            dns_time64_from32(rdataset->resign) >> 1;
+			newheader->resign_lsb = rdataset->resign & 0x1;
+		} else {
 			newheader->resign = 0;
+			newheader->resign_lsb = 0;
+		}
 	} else {
 		newheader->serial = 1;
 		newheader->resign = 0;
+		newheader->resign_lsb = 0;
 		if ((rdataset->attributes & DNS_RDATASETATTR_PREFETCH) != 0)
 			newheader->attributes |= RDATASET_ATTR_PREFETCH;
 		if ((rdataset->attributes & DNS_RDATASETATTR_NEGATIVE) != 0)
@@ -6773,9 +6785,12 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
 	newheader->node = rbtnode;
 	if ((rdataset->attributes & DNS_RDATASETATTR_RESIGN) != 0) {
 		newheader->attributes |= RDATASET_ATTR_RESIGN;
-		newheader->resign = rdataset->resign;
-	} else
+		newheader->resign = dns_time64_from32(rdataset->resign) >> 1;
+		newheader->resign_lsb = rdataset->resign & 0x1;
+	} else {
 		newheader->resign = 0;
+		newheader->resign_lsb = 0;
+	}

 	NODE_LOCK(&rbtdb->node_locks[rbtnode->locknum].lock,
 		  isc_rwlocktype_write);
@@ -6864,6 +6879,7 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
 			newheader->additional_glue = NULL;
 			newheader->node = rbtnode;
 			newheader->resign = 0;
+			newheader->resign_lsb = 0;
 			newheader->last_used = 0;
 		} else {
 			free_rdataset(rbtdb, rbtdb->common.mctx, newheader);
@@ -7178,9 +7194,12 @@ loading_addrdataset(void *arg, dns_name_t *name, dns_rdataset_t *rdataset) {
 	newheader->node = node;
 	if ((rdataset->attributes & DNS_RDATASETATTR_RESIGN) != 0) {
 		newheader->attributes |= RDATASET_ATTR_RESIGN;
-		newheader->resign = rdataset->resign;
-	} else
+		newheader->resign = dns_time64_from32(rdataset->resign) >> 1;
+		newheader->resign_lsb = rdataset->resign & 0x1;
+	} else {
 		newheader->resign = 0;
+		newheader->resign_lsb = 0;
+	}

 	result = add32(rbtdb, node, rbtdb->current_version, newheader,
 		       DNS_DBADD_MERGE, ISC_TRUE, NULL, 0);
@@ -7843,7 +7862,8 @@ setsigningtime(dns_db_t *db, dns_rdataset_t *rdataset, isc_stdtime_t resign) {
 		  isc_rwlocktype_write);

 	oldresign = header->resign;
-	header->resign = resign;
+	header->resign = dns_time64_from32(resign) >> 1;
+	header->resign_lsb = resign & 0x1;
 	if (header->heap_index != 0) {
 		INSIST(RESIGN(header));
 		if (resign == 0) {