fixup! fixup! refactor validated()

- there was special-case code in validated() to handle the results
  of a validator started by a CD=1 query. since that never happens,
  the code has been removed.
- the section of code that handles opportunistic caching of
  validated SOA, NS and NSEC data has been split out to a separate
  function.
- the number of goto statements has been reduced considerably.

bin/dig/host.c

@@ -246,8 +246,7 @@ printsection(dns_message_t *msg, dns_section_t sectionid,
 			       (list_type == dns_rdatatype_any ||
 				rdataset->type == list_type)) ||
 			      (list_addresses &&
-			       (rdataset->type == dns_rdatatype_a ||
-				rdataset->type == dns_rdatatype_aaaa ||
+			       (dns_rdatatype_isaddr(rdataset->type) ||
 				rdataset->type == dns_rdatatype_ns ||
 				rdataset->type == dns_rdatatype_ptr))))
 			{

bin/tests/system/dnssec/signer/general/K.+013+23640.key

@@ -0,0 +1,6 @@
+; This is a key-signing key, keyid 23640, for .
+; Created: 20250310185208 (Mon Mar 10 18:52:08 2025)
+; Publish: 20250310185208 (Mon Mar 10 18:52:08 2025)
+; Activate: 20250310185208 (Mon Mar 10 18:52:08 2025)
+; Revoke: 20250310185208 (Mon Mar 10 18:52:08 2025)
+. IN DNSKEY 257 3 13 uKwpRtMH+9iuUk/Xj6LciIP5ZckaBtXaUqxUxzJYexXjvxGZGX4470Jv hq2NCI3HBZQNaCCP/h9sluhIzRGPTA==

bin/tests/system/dnssec/signer/general/K.+013+23640.private

@@ -0,0 +1,7 @@
+Private-key-format: v1.3
+Algorithm: 13 (ECDSAP256SHA256)
+PrivateKey: m5udfGNSijISQ8Tfp4kx09O1em4PErLUw/mCj3SKmqw=
+Created: 20250310185208
+Publish: 20250310185208
+Activate: 20250310185208
+Revoke: 20250310185208

bin/tests/system/dnssec/signer/general/K.+013+23768.key

@@ -0,0 +1,5 @@
+; This is a zone-signing key, keyid 23768, for .
+; Created: 20250310185208 (Mon Mar 10 18:52:08 2025)
+; Publish: 20250310185208 (Mon Mar 10 18:52:08 2025)
+; Activate: 20250310185208 (Mon Mar 10 18:52:08 2025)
+. IN DNSKEY 256 3 13 TFelYtTRBWeA9A307vvuWIcaNwW4txW4RgSELtsi46ZQs24ncRxmxtFf uJuPyVXePNiE4HNI9CIowGUsn5WuBw==

bin/tests/system/dnssec/signer/general/test12.zone

@@ -0,0 +1,37 @@
+; Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+;
+; SPDX-License-Identifier: MPL-2.0
+;
+; This Source Code Form is subject to the terms of the Mozilla Public
+; License, v. 2.0.  If a copy of the MPL was not distributed with this
+; file, you can obtain one at https://mozilla.org/MPL/2.0/.
+;
+; See the COPYRIGHT file distributed with this work for additional
+; information regarding copyright ownership.
+
+;	This is a zone which has two DNSKEY records, both of which have
+; existing private key files available.  They should be loaded automatically
+; and the zone correctly signed.
+;
+$TTL 30	; 30 seconds
+.			IN SOA	a.root.servers.nil. each.isc.org. (
+				2000042101 ; serial
+				600        ; refresh (10 minutes)
+				600        ; retry (10 minutes)
+				1200       ; expire (20 minutes)
+				600        ; minimum (10 minutes)
+				)
+			NS	a.root-servers.nil.
+			DNSKEY	256 3 13 (
+				TFelYtTRBWeA9A307vvuWIcaNwW4txW4RgSELtsi46ZQ
+				s24ncRxmxtFfuJuPyVXePNiE4HNI9CIowGUsn5WuBw==
+				) ; ZSK; alg = ECDSAP256SHA256 ; key id = 23768
+			DNSKEY	257 3 13 (
+				OSmhpULEDCUzHCBeDU5uJXzkCcGuW2qrkQznKRPGhRZN
+				j7ZUIGInGzM5Um5m02ULWt8tKbi55NJUeifKWegQ0g==
+				) ; KSK; alg = ECDSAP256SHA256 ; key id = 22255
+			DNSKEY	385 3 13 (
+				uKwpRtMH+9iuUk/Xj6LciIP5ZckaBtXaUqxUxzJYexXj
+				vxGZGX4470Jvhq2NCI3HBZQNaCCP/h9sluhIzRGPTA==
+				) ; revoked KSK; alg = ECDSAP256SHA256 ; key id = 23768
+a.root-servers.nil.	A	10.53.0.1

bin/tests/system/dnssec/tests.sh

@@ -1564,6 +1564,18 @@ n=$((n + 1))
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status + ret))
 
+echo_ic "revoked KSK ID collides with ZSK ($n)"
+ret=0
+# signing should fail, but should not coredump
+(
+  cd signer/general || exit 0
+  rm -f signed.zone
+  $SIGNER -S -f signed.zone -o . test12.zone >signer.out.$n
+) && ret=1
+n=$((n + 1))
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status + ret))
+
 echo_ic "check that dnssec-signzone rejects excessive NSEC3 iterations ($n)"
 ret=0
 (

bin/tests/system/dyndb/driver/db.c

@@ -232,9 +232,7 @@ addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
 	dns_fixedname_init(&name);
 	CHECK(dns__db_addrdataset(sampledb->db, node, version, now, rdataset,
 				  options, addedrdataset DNS__DB_FLARG_PASS));
-	if (rdataset->type == dns_rdatatype_a ||
-	    rdataset->type == dns_rdatatype_aaaa)
-	{
+	if (dns_rdatatype_isaddr(rdataset->type)) {
 		CHECK(dns_db_nodefullname(sampledb->db, node,
 					  dns_fixedname_name(&name)));
 		CHECK(syncptrs(sampledb->inst, dns_fixedname_name(&name),
@@ -263,9 +261,7 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
 		goto cleanup;
 	}
 
-	if (rdataset->type == dns_rdatatype_a ||
-	    rdataset->type == dns_rdatatype_aaaa)
-	{
+	if (dns_rdatatype_isaddr(rdataset->type)) {
 		CHECK(dns_db_nodefullname(sampledb->db, node,
 					  dns_fixedname_name(&name)));
 		CHECK(syncptrs(sampledb->inst, dns_fixedname_name(&name),

lib/dns/adb.c

@@ -567,7 +567,7 @@ import_rdataset(dns_adbname_t *adbname, dns_rdataset_t *rdataset,
 		rdataset->ttl = ttlclamp(rdataset->ttl);
 	}
 
-	REQUIRE(rdtype == dns_rdatatype_a || rdtype == dns_rdatatype_aaaa);
+	REQUIRE(dns_rdatatype_isaddr(rdtype));
 
 	for (result = dns_rdataset_first(rdataset); result == ISC_R_SUCCESS;
 	     result = dns_rdataset_next(rdataset))
@@ -2557,7 +2557,7 @@ dbfind_name(dns_adbname_t *adbname, isc_stdtime_t now, dns_rdatatype_t rdtype) {
 	adb = adbname->adb;
 
 	REQUIRE(DNS_ADB_VALID(adb));
-	REQUIRE(rdtype == dns_rdatatype_a || rdtype == dns_rdatatype_aaaa);
+	REQUIRE(dns_rdatatype_isaddr(rdtype));
 
 	fname = dns_fixedname_initname(&foundname);
 	dns_rdataset_init(&rdataset);

lib/dns/catz.c

@@ -1516,8 +1516,7 @@ catz_process_primaries(dns_catz_zone_t *catz, dns_ipkeylist_t *ipkl,
 	}
 	/* else - 'simple' case - without labels */
 
-	if (value->type != dns_rdatatype_a && value->type != dns_rdatatype_aaaa)
-	{
+	if (!dns_rdatatype_isaddr(value->type)) {
 		return ISC_R_FAILURE;
 	}
 

lib/dns/dnssec.c

@@ -1428,29 +1428,35 @@ addkey(dns_dnsseckeylist_t *keylist, dst_key_t **newkey, bool savekeys,
 
 	if (key != NULL) {
 		/*
-		 * Found a match.  If the old key was only public and the
-		 * new key is private, replace the old one; otherwise
-		 * leave it.  But either way, mark the key as having
-		 * been found in the zone.
+		 * Found a match. If we already had a private key, then
+		 * the new key can't be an improvement. If the existing
+		 * key was public-only but the new key is too, then it's
+		 * still not an improvement. Mark the old key as having
+		 * been found in the zone and stop.
 		 */
-		if (dst_key_isprivate(key->key)) {
-			dst_key_free(newkey);
-		} else if (dst_key_isprivate(*newkey)) {
-			dst_key_free(&key->key);
-			key->key = *newkey;
+		if (dst_key_isprivate(key->key) || !dst_key_isprivate(*newkey))
+		{
+			key->source = dns_keysource_zoneapex;
+			return;
 		}
 
-		key->source = dns_keysource_zoneapex;
-		return;
+		/*
+		 * However, if the old key was public-only, and the new key
+		 * is private, then we're throwing away the old key.
+		 */
+		dst_key_free(&key->key);
+		ISC_LIST_UNLINK(*keylist, key, link);
+		dns_dnsseckey_destroy(mctx, &key);
 	}
 
+	/* Store the new key. */
 	dns_dnsseckey_create(mctx, newkey, &key);
+	key->source = dns_keysource_zoneapex;
 	key->pubkey = pubkey_only;
 	if (key->legacy || savekeys) {
 		key->force_publish = true;
 		key->force_sign = dst_key_isprivate(key->key);
 	}
-	key->source = dns_keysource_zoneapex;
 	ISC_LIST_APPEND(*keylist, key, link);
 	*newkey = NULL;
 }

lib/dns/include/dns/keytable.h

@@ -205,9 +205,9 @@ dns_keytable_finddeepestmatch(dns_keytable_t *keytable, const dns_name_t *name,
  *\li	Any other result indicates an error.
  */
 
-isc_result_t
+bool
 dns_keytable_issecuredomain(dns_keytable_t *keytable, const dns_name_t *name,
-			    dns_name_t *foundname, bool *wantdnssecp);
+			    dns_name_t *foundname);
 /*%<
  * Is 'name' at or beneath a trusted key?
  *
@@ -219,20 +219,11 @@ dns_keytable_issecuredomain(dns_keytable_t *keytable, const dns_name_t *name,
  *
  *\li	'foundanme' is NULL or is a pointer to an initialized dns_name_t
  *
- *\li	'*wantsdnssecp' is a valid bool.
- *
  * Ensures:
  *
- *\li	On success, *wantsdnssecp will be true if and only if 'name'
- *	is at or beneath a trusted key.  If 'foundname' is not NULL, then
- *	it will be updated to contain the name of the closest enclosing
- *	trust anchor.
- *
- * Returns:
- *
- *\li	ISC_R_SUCCESS
- *
- *\li	Any other result is an error.
+ *\li	Returns true if and only if 'name' is at or beneath a trusted key.
+ *	If 'foundname' is not NULL, then it will be updated to contain
+ *	the name of the closest enclosing trust anchor.
  */
 
 isc_result_t

lib/dns/include/dns/ncache.h

@@ -54,26 +54,24 @@
 isc_result_t
 dns_ncache_add(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node,
 	       dns_rdatatype_t covers, isc_stdtime_t now, dns_ttl_t minttl,
-	       dns_ttl_t maxttl, dns_rdataset_t *addedrdataset);
-isc_result_t
-dns_ncache_addoptout(dns_message_t *message, dns_db_t *cache,
-		     dns_dbnode_t *node, dns_rdatatype_t covers,
-		     isc_stdtime_t now, dns_ttl_t minttl, dns_ttl_t maxttl,
-		     bool optout, dns_rdataset_t *addedrdataset);
+	       dns_ttl_t maxttl, bool optout, bool secure,
+	       dns_rdataset_t *addedrdataset);
 /*%<
  * Convert the authority data from 'message' into a negative cache
  * rdataset, and store it in 'cache' at 'node' with a TTL limited to
  * 'maxttl'.
  *
- * \li dns_ncache_add produces a negative cache entry with a trust of no
- *     more than answer
- * \li dns_ncache_addoptout produces a negative cache entry which will have
- *     a trust of secure if all the records that make up the entry are secure.
+ * \li If 'secure' is true and all the records that make up the entry
+ *     are secure, then dns_ncache_add produces a negative cache entry
+ *     with trust level secure.
+ * \li If 'secure' is false, the negative cache entry's trust level
+ *     will be capped at answer.
  *
  * The 'covers' argument is the RR type whose nonexistence we are caching,
  * or dns_rdatatype_any when caching a NXDOMAIN response.
  *
- * 'optout' indicates a DNS_RDATASETATTR_OPTOUT should be set.
+ * 'optout' indicates DNS_RDATASETATTR_OPTOUT should be set. This only
+ * applies in secure zones; if 'secure' is false, 'optout' is ignored.
  *
  * Note:
  *\li	If 'addedrdataset' is not NULL, then it will be attached to the added

lib/dns/include/dns/rdata.h

@@ -113,6 +113,36 @@ struct dns_rdata {
 	ISC_LINK(dns_rdata_t) link;
 };
 
+/*%
+ * Rdatatype attributes.
+ */
+enum {
+	/*% only one may exist for a name */
+	DNS_RDATATYPEATTR_SINGLETON = 1 << 0,
+	/*% requires no other data be present */
+	DNS_RDATATYPEATTR_EXCLUSIVE = 1 << 1,
+	/*% Is a meta type */
+	DNS_RDATATYPEATTR_META = 1 << 2,
+	/*% Is a DNSSEC type, like RRSIG or NSEC */
+	DNS_RDATATYPEATTR_DNSSEC = 1 << 3,
+	/*% Is a zone cut authority type */
+	DNS_RDATATYPEATTR_ZONECUTAUTH = 1 << 4,
+	/*% Is reserved (unusable) */
+	DNS_RDATATYPEATTR_RESERVED = 1 << 5,
+	/*% Is an unknown type */
+	DNS_RDATATYPEATTR_UNKNOWN = 1 << 6,
+	/*% Is META, and can only be in a question section */
+	DNS_RDATATYPEATTR_QUESTIONONLY = 1 << 7,
+	/*% Is META, and can NOT be in a question section */
+	DNS_RDATATYPEATTR_NOTQUESTION = 1 << 8,
+	/*% Is present at zone cuts in the parent, not the child */
+	DNS_RDATATYPEATTR_ATPARENT = 1 << 9,
+	/*% Can exist along side a CNAME */
+	DNS_RDATATYPEATTR_ATCNAME = 1 << 10,
+	/*% Follow additional */
+	DNS_RDATATYPEATTR_FOLLOWADDITIONAL = 1 << 11,
+};
+
 #define DNS_RDATA_INIT                        \
 	{                                     \
 		.data = NULL,                 \
@@ -530,16 +560,28 @@ dns_rdata_freestruct(void *source);
  *	dns_rdata_tostruct().
  */
 
-bool
-dns_rdatatype_ismeta(dns_rdatatype_t type);
+unsigned int
+dns_rdatatype_attributes(dns_rdatatype_t rdtype);
 /*%<
+ * Return attributes for the given type.
+ *
+ * Requires:
+ *\li	'rdtype' are known.
+ *
+ * Returns:
+ *\li	a bitmask of the rdatatype attribute flags, defined above.
+ */
+
+/*%
  * Return true iff the rdata type 'type' is a meta-type
  * like ANY or AXFR.
  */
+static inline bool
+dns_rdatatype_ismeta(dns_rdatatype_t type) {
+	return (dns_rdatatype_attributes(type) & DNS_RDATATYPEATTR_META) != 0;
+}
 
-bool
-dns_rdatatype_issingleton(dns_rdatatype_t type);
-/*%<
+/*%
  * Return true iff the rdata type 'type' is a singleton type,
  * like CNAME or SOA.
  *
@@ -547,34 +589,108 @@ dns_rdatatype_issingleton(dns_rdatatype_t type);
  * \li	'type' is a valid rdata type.
  *
  */
+static inline bool
+dns_rdatatype_issingleton(dns_rdatatype_t type) {
+	return (dns_rdatatype_attributes(type) & DNS_RDATATYPEATTR_SINGLETON) !=
+	       0;
+}
 
-bool
-dns_rdataclass_ismeta(dns_rdataclass_t rdclass);
-/*%<
- * Return true iff the rdata class 'rdclass' is a meta-class
- * like ANY or NONE.
+/*%
+ * Return true iff rdata of type 'type' can not appear in the question
+ * section of a properly formatted message.
+ *
+ * Requires:
+ * \li	'type' is a valid rdata type.
+ *
  */
+static inline bool
+dns_rdatatype_notquestion(dns_rdatatype_t type) {
+	return (dns_rdatatype_attributes(type) &
+		DNS_RDATATYPEATTR_NOTQUESTION) != 0;
+}
 
-bool
-dns_rdatatype_isdnssec(dns_rdatatype_t type);
-/*%<
+/*%
+ * Return true iff rdata of type 'type' can only appear in the question
+ * section of a properly formatted message.
+ *
+ * Requires:
+ * \li	'type' is a valid rdata type.
+ *
+ */
+static inline bool
+dns_rdatatype_questiononly(dns_rdatatype_t type) {
+	return (dns_rdatatype_attributes(type) &
+		DNS_RDATATYPEATTR_QUESTIONONLY) != 0;
+}
+
+/*%
+ * Return true iff rdata of type 'type' can appear beside a cname.
+ *
+ * Requires:
+ * \li	'type' is a valid rdata type.
+ *
+ */
+static inline bool
+dns_rdatatype_atcname(dns_rdatatype_t type) {
+	return (dns_rdatatype_attributes(type) & DNS_RDATATYPEATTR_ATCNAME) !=
+	       0;
+}
+
+/*%
+ * Return true iff rdata of type 'type' should appear at the parent of
+ * a zone cut.
+ *
+ * Requires:
+ * \li	'type' is a valid rdata type.
+ *
+ */
+static inline bool
+dns_rdatatype_atparent(dns_rdatatype_t type) {
+	return (dns_rdatatype_attributes(type) & DNS_RDATATYPEATTR_ATPARENT) !=
+	       0;
+}
+
+/*%
+ * Return true if adding a record of type 'type' to the ADDITIONAL section
+ * of a message can itself trigger the addition of still more data to the
+ * additional section.
+ *
+ * (For example: adding SRV to the ADDITIONAL section may trigger
+ * the addition of address records associated with that SRV.)
+ *
+ * Requires:
+ * \li	'type' is a valid rdata type.
+ *
+ */
+static inline bool
+dns_rdatatype_followadditional(dns_rdatatype_t type) {
+	return (dns_rdatatype_attributes(type) &
+		DNS_RDATATYPEATTR_FOLLOWADDITIONAL) != 0;
+}
+
+/*%
  * Return true iff 'type' is one of the DNSSEC
  * rdata types that may exist alongside a CNAME record.
  *
  * Requires:
  * \li	'type' is a valid rdata type.
  */
+static inline bool
+dns_rdatatype_isdnssec(dns_rdatatype_t type) {
+	return (dns_rdatatype_attributes(type) & DNS_RDATATYPEATTR_DNSSEC) != 0;
+}
 
-bool
-dns_rdatatype_iskeymaterial(dns_rdatatype_t type);
-/*%<
+/*%
  * Return true iff the rdata type 'type' is a DNSSEC key
  * related type, like DNSKEY, CDNSKEY, or CDS.
  */
+static inline bool
+dns_rdatatype_iskeymaterial(dns_rdatatype_t type) {
+	return type == dns_rdatatype_dnskey || type == dns_rdatatype_cdnskey ||
+	       type == dns_rdatatype_cds;
+}
 
-bool
-dns_rdatatype_iszonecutauth(dns_rdatatype_t type);
-/*%<
+/*%
  * Return true iff rdata of type 'type' is considered authoritative
  * data (not glue) in the NSEC chain when it occurs in the parent zone
  * at a zone cut.
@@ -583,16 +699,68 @@ dns_rdatatype_iszonecutauth(dns_rdatatype_t type);
  * \li	'type' is a valid rdata type.
  *
  */
+static inline bool
+dns_rdatatype_iszonecutauth(dns_rdatatype_t type) {
+	return (dns_rdatatype_attributes(type) &
+		DNS_RDATATYPEATTR_ZONECUTAUTH) != 0;
+}
 
-bool
-dns_rdatatype_isknown(dns_rdatatype_t type);
-/*%<
+/*%
  * Return true iff the rdata type 'type' is known.
  *
  * Requires:
  * \li	'type' is a valid rdata type.
  *
  */
+static inline bool
+dns_rdatatype_isknown(dns_rdatatype_t type) {
+	return (dns_rdatatype_attributes(type) & DNS_RDATATYPEATTR_UNKNOWN) ==
+	       0;
+}
+
+/*%
+ * Return true iff a query for the rdata type can have multiple
+ * unrelated answers in a response: ANY, RRSIG, or SIG.
+ */
+static inline bool
+dns_rdatatype_ismulti(dns_rdatatype_t type) {
+	return type == dns_rdatatype_any || type == dns_rdatatype_rrsig ||
+	       type == dns_rdatatype_sig;
+}
+
+/*%
+ * Return true iff the rdata type is a signature: either RRSIG or SIG.
+ */
+static inline bool
+dns_rdatatype_issig(dns_rdatatype_t type) {
+	return type == dns_rdatatype_rrsig || type == dns_rdatatype_sig;
+}
+
+/*%
+ * Return true iff the rdata type is an address: either A or AAAA.
+ */
+static inline bool
+dns_rdatatype_isaddr(dns_rdatatype_t type) {
+	return type == dns_rdatatype_a || type == dns_rdatatype_aaaa;
+}
+
+/*%
+ * Return true iff the rdata type is an alias: either CNAME or DNAME.
+ */
+static inline bool
+dns_rdatatype_isalias(dns_rdatatype_t type) {
+	return type == dns_rdatatype_cname || type == dns_rdatatype_dname;
+}
+
+/*%
+ * Return true iff the rdata class 'rdclass' is a meta-class
+ * like ANY or NONE.
+ */
+static inline bool
+dns_rdataclass_ismeta(dns_rdataclass_t rdclass) {
+	return rdclass == dns_rdataclass_reserved0 ||
+	       rdclass == dns_rdataclass_none || rdclass == dns_rdataclass_any;
+}
 
 isc_result_t
 dns_rdata_additionaldata(dns_rdata_t *rdata, const dns_name_t *owner,
@@ -653,101 +821,6 @@ dns_rdata_digest(dns_rdata_t *rdata, dns_digestfunc_t digest, void *arg);
  *\li	Many other results are possible if not successful.
  */
 
-bool
-dns_rdatatype_questiononly(dns_rdatatype_t type);
-/*%<
- * Return true iff rdata of type 'type' can only appear in the question
- * section of a properly formatted message.
- *
- * Requires:
- * \li	'type' is a valid rdata type.
- *
- */
-
-bool
-dns_rdatatype_notquestion(dns_rdatatype_t type);
-/*%<
- * Return true iff rdata of type 'type' can not appear in the question
- * section of a properly formatted message.
- *
- * Requires:
- * \li	'type' is a valid rdata type.
- *
- */
-
-bool
-dns_rdatatype_atparent(dns_rdatatype_t type);
-/*%<
- * Return true iff rdata of type 'type' should appear at the parent of
- * a zone cut.
- *
- * Requires:
- * \li	'type' is a valid rdata type.
- *
- */
-
-bool
-dns_rdatatype_atcname(dns_rdatatype_t type);
-/*%<
- * Return true iff rdata of type 'type' can appear beside a cname.
- *
- * Requires:
- * \li	'type' is a valid rdata type.
- *
- */
-
-bool
-dns_rdatatype_followadditional(dns_rdatatype_t type);
-/*%<
- * Return true if adding a record of type 'type' to the ADDITIONAL section
- * of a message can itself trigger the addition of still more data to the
- * additional section.
- *
- * (For example: adding SRV to the ADDITIONAL section may trigger
- * the addition of address records associated with that SRV.)
- *
- * Requires:
- * \li	'type' is a valid rdata type.
- *
- */
-
-unsigned int
-dns_rdatatype_attributes(dns_rdatatype_t rdtype);
-/*%<
- * Return attributes for the given type.
- *
- * Requires:
- *\li	'rdtype' are known.
- *
- * Returns:
- *\li	a bitmask consisting of the following flags.
- */
-
-/*% only one may exist for a name */
-#define DNS_RDATATYPEATTR_SINGLETON 0x00000001U
-/*% requires no other data be present */
-#define DNS_RDATATYPEATTR_EXCLUSIVE 0x00000002U
-/*% Is a meta type */
-#define DNS_RDATATYPEATTR_META 0x00000004U
-/*% Is a DNSSEC type, like RRSIG or NSEC */
-#define DNS_RDATATYPEATTR_DNSSEC 0x00000008U
-/*% Is a zone cut authority type */
-#define DNS_RDATATYPEATTR_ZONECUTAUTH 0x00000010U
-/*% Is reserved (unusable) */
-#define DNS_RDATATYPEATTR_RESERVED 0x00000020U
-/*% Is an unknown type */
-#define DNS_RDATATYPEATTR_UNKNOWN 0x00000040U
-/*% Is META, and can only be in a question section */
-#define DNS_RDATATYPEATTR_QUESTIONONLY 0x00000080U
-/*% Is META, and can NOT be in a question section */
-#define DNS_RDATATYPEATTR_NOTQUESTION 0x00000100U
-/*% Is present at zone cuts in the parent, not the child */
-#define DNS_RDATATYPEATTR_ATPARENT 0x00000200U
-/*% Can exist along side a CNAME */
-#define DNS_RDATATYPEATTR_ATCNAME 0x00000400U
-/*% Follow additional */
-#define DNS_RDATATYPEATTR_FOLLOWADDITIONAL 0x00000800U
-
 dns_rdatatype_t
 dns_rdata_covers(dns_rdata_t *rdata);
 /*%<

lib/dns/include/dns/rdataset.h

@@ -689,3 +689,30 @@ dns_rdataset_equals(const dns_rdataset_t *rdataset1,
  * \li	'rdataset1' is a valid rdataset.
  * \li	'rdataset2' is a valid rdataset.
  */
+
+/*%
+ * Returns true if the rdataset is of type 'type', or type RRSIG
+ * and covers 'type'.
+ */
+static inline bool
+dns_rdataset_matchestype(const dns_rdataset_t *rdataset,
+			 const dns_rdatatype_t type) {
+	REQUIRE(DNS_RDATASET_VALID(rdataset));
+
+	return rdataset->type == type ||
+	       (rdataset->type == dns_rdatatype_rrsig &&
+		rdataset->covers == type);
+}
+
+/*%
+ * Returns true if the rdataset is of type 'type', or type RRSIG
+ * and covers 'type'.
+ */
+static inline bool
+dns_rdataset_issigtype(const dns_rdataset_t *rdataset,
+		       const dns_rdatatype_t type) {
+	REQUIRE(DNS_RDATASET_VALID(rdataset));
+
+	return rdataset->type == dns_rdatatype_rrsig &&
+	       rdataset->covers == type;
+}

lib/dns/include/dns/view.h

@@ -986,13 +986,12 @@ dns_view_getsecroots(dns_view_t *view, dns_keytable_t **ktp);
  *\li	ISC_R_NOTFOUND
  */
 
-isc_result_t
+bool
 dns_view_issecuredomain(dns_view_t *view, const dns_name_t *name,
-			isc_stdtime_t now, bool checknta, bool *ntap,
-			bool *secure_domain);
+			isc_stdtime_t now, bool checknta, bool *ntap);
 /*%<
  * Is 'name' at or beneath a trusted key, and not covered by a valid
- * negative trust anchor?  Put answer in '*secure_domain'.
+ * negative trust anchor, and DNSSEC validation is enabled?
  *
  * If 'checknta' is false, ignore the NTA table in determining
  * whether this is a secure domain. If 'checknta' is not false, and if
@@ -1001,10 +1000,6 @@ dns_view_issecuredomain(dns_view_t *view, const dns_name_t *name,
  *
  * Requires:
  * \li	'view' is valid.
- *
- * Returns:
- *\li	ISC_R_SUCCESS
- *\li	Any other value indicates failure
  */
 
 bool

lib/dns/keytable.c

@@ -530,13 +530,14 @@ dns_keytable_finddeepestmatch(dns_keytable_t *keytable, const dns_name_t *name,
 	return result;
 }
 
-isc_result_t
+bool
 dns_keytable_issecuredomain(dns_keytable_t *keytable, const dns_name_t *name,
-			    dns_name_t *foundname, bool *wantdnssecp) {
+			    dns_name_t *foundname) {
 	isc_result_t result;
 	dns_qpread_t qpr;
 	dns_keynode_t *keynode = NULL;
 	void *pval = NULL;
+	bool secure = false;
 
 	/*
 	 * Is 'name' at or beneath a trusted key?
@@ -544,7 +545,6 @@ dns_keytable_issecuredomain(dns_keytable_t *keytable, const dns_name_t *name,
 
 	REQUIRE(VALID_KEYTABLE(keytable));
 	REQUIRE(dns_name_isabsolute(name));
-	REQUIRE(wantdnssecp != NULL);
 
 	dns_qpmulti_query(keytable->table, &qpr);
 	result = dns_qp_lookup(&qpr, name, NULL, NULL, NULL, &pval, NULL);
@@ -553,16 +553,12 @@ dns_keytable_issecuredomain(dns_keytable_t *keytable, const dns_name_t *name,
 		if (foundname != NULL) {
 			dns_name_copy(&keynode->name, foundname);
 		}
-		*wantdnssecp = true;
-		result = ISC_R_SUCCESS;
-	} else if (result == ISC_R_NOTFOUND) {
-		*wantdnssecp = false;
-		result = ISC_R_SUCCESS;
+		secure = true;
 	}
 
 	dns_qpread_destroy(keytable->table, &qpr);
 
-	return result;
+	return secure;
 }
 
 static isc_result_t

lib/dns/master.c

@@ -1914,7 +1914,7 @@ load_text(dns_loadctx_t *lctx) {
 			}
 		}
 
-		if (type == dns_rdatatype_rrsig || type == dns_rdatatype_sig) {
+		if (dns_rdatatype_issig(type)) {
 			covers = dns_rdata_covers(&rdata[rdcount]);
 		} else {
 			covers = 0;

lib/dns/ncache.c

@@ -50,12 +50,6 @@ atomic_getuint8(isc_buffer_t *b) {
 	return ret;
 }
 
-static isc_result_t
-addoptout(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node,
-	  dns_rdatatype_t covers, isc_stdtime_t now, dns_ttl_t minttl,
-	  dns_ttl_t maxttl, bool optout, bool secure,
-	  dns_rdataset_t *addedrdataset);
-
 static isc_result_t
 copy_rdataset(dns_rdataset_t *rdataset, isc_buffer_t *buffer) {
 	isc_result_t result;
@@ -107,25 +101,8 @@ copy_rdataset(dns_rdataset_t *rdataset, isc_buffer_t *buffer) {
 isc_result_t
 dns_ncache_add(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node,
 	       dns_rdatatype_t covers, isc_stdtime_t now, dns_ttl_t minttl,
-	       dns_ttl_t maxttl, dns_rdataset_t *addedrdataset) {
-	return addoptout(message, cache, node, covers, now, minttl, maxttl,
-			 false, false, addedrdataset);
-}
-
-isc_result_t
-dns_ncache_addoptout(dns_message_t *message, dns_db_t *cache,
-		     dns_dbnode_t *node, dns_rdatatype_t covers,
-		     isc_stdtime_t now, dns_ttl_t minttl, dns_ttl_t maxttl,
-		     bool optout, dns_rdataset_t *addedrdataset) {
-	return addoptout(message, cache, node, covers, now, minttl, maxttl,
-			 optout, true, addedrdataset);
-}
-
-static isc_result_t
-addoptout(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node,
-	  dns_rdatatype_t covers, isc_stdtime_t now, dns_ttl_t minttl,
-	  dns_ttl_t maxttl, bool optout, bool secure,
-	  dns_rdataset_t *addedrdataset) {
+	       dns_ttl_t maxttl, bool optout, bool secure,
+	       dns_rdataset_t *addedrdataset) {
 	isc_result_t result;
 	isc_buffer_t buffer;
 	isc_region_t r;
@@ -143,14 +120,17 @@ addoptout(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node,
 	/*
 	 * Convert the authority data from 'message' into a negative cache
 	 * rdataset, and store it in 'cache' at 'node'.
+	 *
+	 * We assume that all data in the authority section has been
+	 * validated by the caller.
 	 */
 
 	REQUIRE(message != NULL);
 
 	/*
-	 * We assume that all data in the authority section has been
-	 * validated by the caller.
+	 * If 'secure' is false, ignore 'optout'.
 	 */
+	optout = optout && secure;
 
 	/*
 	 * Initialize the list.

lib/dns/qpcache.c

@@ -513,8 +513,7 @@ need_headerupdate(dns_slabheader_t *header, isc_stdtime_t now) {
 #if DNS_QPDB_LIMITLRUUPDATE
 	if (header->type == dns_rdatatype_ns ||
 	    (header->trust == dns_trust_glue &&
-	     (header->type == dns_rdatatype_a ||
-	      header->type == dns_rdatatype_aaaa)))
+	     dns_rdatatype_isaddr(header->type)))
 	{
 		/*
 		 * Glue records are updated if at least DNS_QPDB_LRUUPDATE_GLUE

lib/dns/qpzone.c

@@ -2696,13 +2696,25 @@ step(qpz_search_t *search, dns_qpiter_t *it, direction_t direction,
 	while (result == ISC_R_SUCCESS) {
 		isc_rwlock_t *nlock = &qpdb->buckets[node->locknum].lock;
 		isc_rwlocktype_t nlocktype = isc_rwlocktype_none;
+		dns_slabheader_t *header_next = NULL;
 
 		NODE_RDLOCK(nlock, &nlocktype);
-		for (header = node->data; header != NULL; header = header->next)
+		for (header = node->data; header != NULL; header = header_next)
 		{
-			if (header->serial <= search->serial &&
-			    !IGNORE(header) && !NONEXISTENT(header))
-			{
+			header_next = header->next;
+			while (header != NULL) {
+				if (header->serial <= search->serial &&
+				    !IGNORE(header))
+				{
+					if (NONEXISTENT(header)) {
+						header = NULL;
+					}
+					break;
+				} else {
+					header = header->down;
+				}
+			}
+			if (header != NULL) {
 				break;
 			}
 		}

lib/dns/rdata.c

@@ -2354,113 +2354,6 @@ dns_rdata_covers(dns_rdata_t *rdata) {
 	return covers_sig(rdata);
 }
 
-bool
-dns_rdatatype_ismeta(dns_rdatatype_t type) {
-	if ((dns_rdatatype_attributes(type) & DNS_RDATATYPEATTR_META) != 0) {
-		return true;
-	}
-	return false;
-}
-
-bool
-dns_rdatatype_issingleton(dns_rdatatype_t type) {
-	if ((dns_rdatatype_attributes(type) & DNS_RDATATYPEATTR_SINGLETON) != 0)
-	{
-		return true;
-	}
-	return false;
-}
-
-bool
-dns_rdatatype_notquestion(dns_rdatatype_t type) {
-	if ((dns_rdatatype_attributes(type) & DNS_RDATATYPEATTR_NOTQUESTION) !=
-	    0)
-	{
-		return true;
-	}
-	return false;
-}
-
-bool
-dns_rdatatype_questiononly(dns_rdatatype_t type) {
-	if ((dns_rdatatype_attributes(type) & DNS_RDATATYPEATTR_QUESTIONONLY) !=
-	    0)
-	{
-		return true;
-	}
-	return false;
-}
-
-bool
-dns_rdatatype_atcname(dns_rdatatype_t type) {
-	if ((dns_rdatatype_attributes(type) & DNS_RDATATYPEATTR_ATCNAME) != 0) {
-		return true;
-	}
-	return false;
-}
-
-bool
-dns_rdatatype_atparent(dns_rdatatype_t type) {
-	if ((dns_rdatatype_attributes(type) & DNS_RDATATYPEATTR_ATPARENT) != 0)
-	{
-		return true;
-	}
-	return false;
-}
-
-bool
-dns_rdatatype_followadditional(dns_rdatatype_t type) {
-	if ((dns_rdatatype_attributes(type) &
-	     DNS_RDATATYPEATTR_FOLLOWADDITIONAL) != 0)
-	{
-		return true;
-	}
-	return false;
-}
-
-bool
-dns_rdataclass_ismeta(dns_rdataclass_t rdclass) {
-	if (rdclass == dns_rdataclass_reserved0 ||
-	    rdclass == dns_rdataclass_none || rdclass == dns_rdataclass_any)
-	{
-		return true;
-	}
-
-	return false; /* Assume it is not a meta class. */
-}
-
-bool
-dns_rdatatype_isdnssec(dns_rdatatype_t type) {
-	if ((dns_rdatatype_attributes(type) & DNS_RDATATYPEATTR_DNSSEC) != 0) {
-		return true;
-	}
-	return false;
-}
-
-bool
-dns_rdatatype_iskeymaterial(dns_rdatatype_t type) {
-	return type == dns_rdatatype_dnskey || type == dns_rdatatype_cdnskey ||
-	       type == dns_rdatatype_cds;
-}
-
-bool
-dns_rdatatype_iszonecutauth(dns_rdatatype_t type) {
-	if ((dns_rdatatype_attributes(type) & DNS_RDATATYPEATTR_ZONECUTAUTH) !=
-	    0)
-	{
-		return true;
-	}
-	return false;
-}
-
-bool
-dns_rdatatype_isknown(dns_rdatatype_t type) {
-	if ((dns_rdatatype_attributes(type) & DNS_RDATATYPEATTR_UNKNOWN) == 0) {
-		return true;
-	}
-	return false;
-}
-
 void
 dns_rdata_exists(dns_rdata_t *rdata, dns_rdatatype_t type) {
 	REQUIRE(rdata != NULL);

lib/dns/sdlz.c

@@ -752,7 +752,7 @@ findrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
 	UNUSED(now);
 	UNUSED(sigrdataset);
 
-	if (type == dns_rdatatype_sig || type == dns_rdatatype_rrsig) {
+	if (dns_rdatatype_issig(type)) {
 		return ISC_R_NOTIMPLEMENTED;
 	}
 

lib/dns/view.c

@@ -1533,41 +1533,31 @@ dns_view_ntacovers(dns_view_t *view, isc_stdtime_t now, const dns_name_t *name,
 	return dns_ntatable_covered(view->ntatable_priv, now, name, anchor);
 }
 
-isc_result_t
+bool
 dns_view_issecuredomain(dns_view_t *view, const dns_name_t *name,
-			isc_stdtime_t now, bool checknta, bool *ntap,
-			bool *secure_domain) {
-	isc_result_t result;
+			isc_stdtime_t now, bool checknta, bool *ntap) {
 	bool secure = false;
 	dns_fixedname_t fn;
 	dns_name_t *anchor;
 
 	REQUIRE(DNS_VIEW_VALID(view));
 
-	if (view->secroots_priv == NULL) {
-		return ISC_R_NOTFOUND;
+	if (!view->enablevalidation || view->secroots_priv == NULL) {
+		return false;
 	}
 
 	anchor = dns_fixedname_initname(&fn);
-
-	result = dns_keytable_issecuredomain(view->secroots_priv, name, anchor,
-					     &secure);
-	if (result != ISC_R_SUCCESS) {
-		return result;
-	}
+	secure = dns_keytable_issecuredomain(view->secroots_priv, name, anchor);
 
 	SET_IF_NOT_NULL(ntap, false);
 	if (checknta && secure && view->ntatable_priv != NULL &&
 	    dns_ntatable_covered(view->ntatable_priv, now, name, anchor))
 	{
-		if (ntap != NULL) {
-			*ntap = true;
-		}
+		SET_IF_NOT_NULL(ntap, true);
 		secure = false;
 	}
 
-	*secure_domain = secure;
-	return ISC_R_SUCCESS;
+	return secure;
 }
 
 void

lib/ns/query.c

@@ -3318,9 +3318,7 @@ rpz_find_p(ns_client_t *client, dns_name_t *self_name, dns_rdatatype_t qtype,
 			}
 			dns_db_detachnode(*dbp, nodep);
 
-			if (qtype == dns_rdatatype_rrsig ||
-			    qtype == dns_rdatatype_sig)
-			{
+			if (dns_rdatatype_issig(qtype)) {
 				result = DNS_R_NXRRSET;
 			} else {
 				result = dns_db_findext(*dbp, p_name, *versionp,
@@ -5044,9 +5042,7 @@ qctx_init(ns_client_t *client, dns_fetchresponse_t **frespp,
 	/*
 	 * If it's an RRSIG or SIG query, we'll iterate the node.
 	 */
-	if (qctx->qtype == dns_rdatatype_rrsig ||
-	    qctx->qtype == dns_rdatatype_sig)
-	{
+	if (dns_rdatatype_issig(qctx->qtype)) {
 		qctx->type = dns_rdatatype_any;
 	}
 
@@ -5439,8 +5435,7 @@ ns__query_start(query_ctx_t *qctx) {
 	 */
 	if (qctx->view->root_key_sentinel &&
 	    qctx->client->query.restarts == 0 &&
-	    (qctx->qtype == dns_rdatatype_a ||
-	     qctx->qtype == dns_rdatatype_aaaa) &&
+	    (dns_rdatatype_isaddr(qctx->qtype)) &&
 	    (qctx->client->message->flags & DNS_MESSAGEFLAG_CD) == 0)
 	{
 		root_key_sentinel_detect(qctx);
@@ -6495,9 +6490,7 @@ query_resume(query_ctx_t *qctx) {
 	}
 	INSIST(qctx->rdataset != NULL);
 
-	if (qctx->qtype == dns_rdatatype_rrsig ||
-	    qctx->qtype == dns_rdatatype_sig)
-	{
+	if (dns_rdatatype_issig(qctx->qtype)) {
 		qctx->type = dns_rdatatype_any;
 	} else {
 		qctx->type = qctx->qtype;
@@ -7735,8 +7728,7 @@ query_respond_any(query_ctx_t *qctx) {
 		} else if (qctx->view->minimal_any && !TCP(qctx->client) &&
 			   !WANTDNSSEC(qctx->client) &&
 			   qctx->qtype == dns_rdatatype_any &&
-			   (qctx->rdataset->type == dns_rdatatype_sig ||
-			    qctx->rdataset->type == dns_rdatatype_rrsig))
+			   (dns_rdatatype_issig(qctx->rdataset->type)))
 		{
 			CCTRACE(ISC_LOG_DEBUG(5), "query_respond_any: "
 						  "minimal-any skip signature");
@@ -7781,9 +7773,7 @@ query_respond_any(query_ctx_t *qctx) {
 			 * Remember the first RRtype we find so we
 			 * can skip others with minimal-any.
 			 */
-			if (qctx->rdataset->type == dns_rdatatype_sig ||
-			    qctx->rdataset->type == dns_rdatatype_rrsig)
-			{
+			if (dns_rdatatype_issig(qctx->rdataset->type)) {
 				onetype = qctx->rdataset->covers;
 			} else {
 				onetype = qctx->rdataset->type;
@@ -7847,9 +7837,7 @@ query_respond_any(query_ctx_t *qctx) {
 		 * At least one matching rdataset was found
 		 */
 		query_addauth(qctx);
-	} else if (qctx->qtype == dns_rdatatype_rrsig ||
-		   qctx->qtype == dns_rdatatype_sig)
-	{
+	} else if (dns_rdatatype_issig(qctx->qtype)) {
 		/*
 		 * No matching rdatasets were found, but we got
 		 * here on a search for RRSIG/SIG, so that's okay.
@@ -9918,8 +9906,7 @@ query_coveringnsec(query_ctx_t *qctx) {
 			goto cleanup;
 		}
 		if (!ISC_LIST_EMPTY(qctx->view->dns64) &&
-		    (qctx->type == dns_rdatatype_a ||
-		     qctx->type == dns_rdatatype_aaaa)) /* XXX not yet */
+		    dns_rdatatype_isaddr(qctx->type)) /* XXX not yet */
 		{
 			goto cleanup;
 		}
@@ -9978,8 +9965,7 @@ query_coveringnsec(query_ctx_t *qctx) {
 			goto cleanup;
 		}
 		if (!ISC_LIST_EMPTY(qctx->view->dns64) &&
-		    (qctx->type == dns_rdatatype_a ||
-		     qctx->type == dns_rdatatype_aaaa)) /* XXX not yet */
+		    dns_rdatatype_isaddr(qctx->type)) /* XXX not yet */
 		{
 			goto cleanup;
 		}
@@ -11329,8 +11315,7 @@ query_glueanswer(query_ctx_t *qctx) {
 
 	if (!ISC_LIST_EMPTY(secs[DNS_SECTION_ANSWER]) ||
 	    qctx->client->message->rcode != dns_rcode_noerror ||
-	    (qctx->qtype != dns_rdatatype_a &&
-	     qctx->qtype != dns_rdatatype_aaaa))
+	    !dns_rdatatype_isaddr(qctx->qtype))
 	{
 		return;
 	}

lib/ns/update.c

@@ -1165,9 +1165,7 @@ temp_check(isc_mem_t *mctx, dns_diff_t *temp, dns_db_t *db,
 					   *    this name and type */
 
 			*typep = type = t->rdata.type;
-			if (type == dns_rdatatype_rrsig ||
-			    type == dns_rdatatype_sig)
-			{
+			if (dns_rdatatype_issig(type)) {
 				covers = dns_rdata_covers(&t->rdata);
 			} else if (type == dns_rdatatype_any) {
 				dns_db_detachnode(db, &node);

lib/ns/xfrout.c

@@ -135,9 +135,7 @@ log_rr(dns_name_t *name, dns_rdata_t *rdata, uint32_t ttl) {
 	rdl.type = rdata->type;
 	rdl.rdclass = rdata->rdclass;
 	rdl.ttl = ttl;
-	if (rdata->type == dns_rdatatype_sig ||
-	    rdata->type == dns_rdatatype_rrsig)
-	{
+	if (dns_rdatatype_issig(rdata->type)) {
 		rdl.covers = dns_rdata_covers(rdata);
 	} else {
 		rdl.covers = dns_rdatatype_none;
@@ -1553,9 +1551,7 @@ sendstream(xfrout_ctx_t *xfr) {
 		msgrdl->type = rdata->type;
 		msgrdl->rdclass = rdata->rdclass;
 		msgrdl->ttl = ttl;
-		if (rdata->type == dns_rdatatype_sig ||
-		    rdata->type == dns_rdatatype_rrsig)
-		{
+		if (dns_rdatatype_issig(rdata->type)) {
 			msgrdl->covers = dns_rdata_covers(rdata);
 		} else {
 			msgrdl->covers = dns_rdatatype_none;

tests/dns/db_test.c

@@ -351,8 +351,40 @@ ISC_LOOP_TEST_IMPL(version) {
 	result = dns_db_find(db, name, ver, dns_rdatatype_a, 0, 0, &node,
 			     foundname, &rdataset, NULL);
 	assert_int_equal(result, ISC_R_SUCCESS);
-	dns_rdataset_disassociate(&rdataset);
 	dns_db_detachnode(db, &node);
+
+	/* Now we create a node with an empty parent */
+	result = dns_db_newversion(db, &new);
+	dns_test_namefromstring("long.ent.name.test.test.", &fname);
+	result = dns_db_findnode(db, name, true, &node);
+	assert_int_equal(result, ISC_R_SUCCESS);
+	result = dns_db_addrdataset(db, node, new, 0, &rdataset, 0, NULL);
+	assert_int_equal(result, ISC_R_SUCCESS);
+	dns_rdataset_disassociate(&rdataset);
+	dns_rdataset_init(&rdataset);
+
+	/* look up the ENT; it should be empty */
+	dns_test_namefromstring("ent.name.test.test.", &fname);
+	dns_db_detachnode(db, &node);
+	result = dns_db_find(db, name, new, dns_rdatatype_a, 0, 0, &node,
+			     foundname, &rdataset, NULL);
+	assert_int_equal(result, DNS_R_EMPTYNAME);
+
+	/* ... but then we roll it back... */
+	dns_db_closeversion(db, &new, false);
+
+	/* ... and the ENT should be NXDOMAIN now */
+	dns_test_namefromstring("ent.name.test.test.", &fname);
+	result = dns_db_find(db, name, ver, dns_rdatatype_a, 0, 0, &node,
+			     foundname, &rdataset, NULL);
+	assert_int_equal(result, DNS_R_NXDOMAIN);
+
+	if (dns_rdataset_isassociated(&rdataset)) {
+		dns_rdataset_disassociate(&rdataset);
+	}
+	if (node != NULL) {
+		dns_db_detachnode(db, &node);
+	}
 	dns_db_closeversion(db, &ver, false);
 
 	dns_db_detach(&db);

tests/dns/dbversion_test.c

@@ -363,6 +363,154 @@ ISC_RUN_TEST_IMPL(getnsec3parameters) {
 		db1, v2, &hash, &flags, &iterations, salt, &salt_length));
 }
 
+/*
+ * Check that the correct node contents are found after a rollback.
+ */
+ISC_RUN_TEST_IMPL(rollback) {
+	isc_result_t res;
+	dns_rdata_t rdata1 = DNS_RDATA_INIT, rdata2 = DNS_RDATA_INIT;
+	dns_rdataset_t input1 = DNS_RDATASET_INIT;
+	dns_rdataset_t input2 = DNS_RDATASET_INIT;
+	dns_rdataset_t rdataset1 = DNS_RDATASET_INIT;
+	dns_rdataset_t rdataset2 = DNS_RDATASET_INIT;
+	dns_rdatalist_t rdatalist1, rdatalist2;
+	dns_rdata_t out1 = DNS_RDATA_INIT, out2 = DNS_RDATA_INIT;
+	dns_dbnode_t *node = NULL;
+	char *txt1 = (char *)"\006text 1";
+	char *txt2 = (char *)"\006text 2";
+	size_t len1 = strlen(txt1), len2 = strlen(txt2);
+	char buf[1024];
+	isc_buffer_t b;
+
+	UNUSED(state);
+
+	isc_buffer_init(&b, buf, sizeof(buf));
+
+	/* Set up two rdatasets to insert */
+	rdata1.rdclass = dns_rdataclass_in;
+	rdata1.type = dns_rdatatype_txt;
+	rdata2 = rdata1;
+
+	rdata1.length = len1;
+	rdata1.data = (unsigned char *)txt1;
+	rdata2.length = len2;
+	rdata2.data = (unsigned char *)txt2;
+
+	dns_rdatalist_init(&rdatalist1);
+	rdatalist1.rdclass = dns_rdataclass_in;
+	rdatalist1.type = dns_rdatatype_txt;
+	rdatalist1.ttl = 3600;
+	rdatalist2 = rdatalist1;
+
+	ISC_LIST_APPEND(rdatalist1.rdata, &rdata1, link);
+	ISC_LIST_APPEND(rdatalist2.rdata, &rdata2, link);
+
+	dns_rdatalist_tordataset(&rdatalist1, &input1);
+	dns_rdatalist_tordataset(&rdatalist2, &input2);
+
+	/* db1: Insert the first version ("text 1"), and commit */
+	res = dns_db_findnode(db1, dns_rootname, true, &node);
+	assert_int_equal(res, ISC_R_SUCCESS);
+	res = dns_db_addrdataset(db1, node, v1, 0, &input1, 0, NULL);
+	assert_int_equal(res, ISC_R_SUCCESS);
+	dns_db_closeversion(db1, &v1, true); /* commit */
+	assert_null(v1);
+	dns_db_detachnode(db1, &node);
+	assert_null(node);
+
+	/* db2: Insert the first version ("text 1"), and commit */
+	res = dns_db_findnode(db2, dns_rootname, true, &node);
+	assert_int_equal(res, ISC_R_SUCCESS);
+	res = dns_db_addrdataset(db2, node, v2, 0, &input1, 0, NULL);
+	assert_int_equal(res, ISC_R_SUCCESS);
+	dns_db_closeversion(db2, &v2, true); /* commit */
+	assert_null(v2);
+	dns_db_detachnode(db2, &node);
+	assert_null(node);
+
+	/* Reopen the versions */
+	dns_db_newversion(db1, &v1);
+	assert_non_null(v1);
+	dns_db_newversion(db2, &v2);
+	assert_non_null(v2);
+
+	/* db1: Insert the second version ("text 2"), and roll back */
+	res = dns_db_findnode(db1, dns_rootname, true, &node);
+	assert_int_equal(res, ISC_R_SUCCESS);
+	res = dns_db_addrdataset(db1, node, v1, 0, &input2, 0, NULL);
+	assert_int_equal(res, ISC_R_SUCCESS);
+	dns_db_closeversion(db1, &v1, false); /* rollback */
+	assert_null(v1);
+	dns_db_detachnode(db1, &node);
+	assert_null(node);
+
+	/* db2: Insert the second version ("text 2"), and commit */
+	res = dns_db_findnode(db2, dns_rootname, true, &node);
+	assert_int_equal(res, ISC_R_SUCCESS);
+	res = dns_db_addrdataset(db2, node, v2, 0, &input2, 0, NULL);
+	assert_int_equal(res, ISC_R_SUCCESS);
+	dns_db_closeversion(db2, &v2, true); /* commit */
+	assert_null(v2);
+	dns_db_detachnode(db2, &node);
+	assert_null(node);
+
+	/* db1: Look it up and check that the first version is found */
+	dns_db_currentversion(db1, &v1);
+	assert_non_null(v1);
+	res = dns_db_findnode(db1, dns_rootname, true, &node);
+	assert_int_equal(res, ISC_R_SUCCESS);
+	res = dns_db_findrdataset(db1, node, v1, dns_rdatatype_txt, 0, 0,
+				  &rdataset1, NULL);
+	assert_int_equal(res, ISC_R_SUCCESS);
+
+	/* db1: Convert result to text */
+	res = dns_rdataset_first(&rdataset1);
+	assert_int_equal(res, ISC_R_SUCCESS);
+	dns_rdataset_current(&rdataset1, &out1);
+
+	res = dns_rdata_totext(&out1, NULL, &b);
+	assert_int_equal(res, ISC_R_SUCCESS);
+	isc_buffer_putuint8(&b, 0);
+
+	/* db1: We should have "text 1" */
+	assert_string_equal(buf, "\"text 1\"");
+
+	dns_rdataset_disassociate(&rdataset1);
+
+	dns_db_closeversion(db1, &v1, true);
+	assert_null(v1);
+	dns_db_detachnode(db1, &node);
+	assert_null(node);
+
+	/* db2: Look it up and check that the second version is found */
+	dns_db_currentversion(db2, &v2);
+	assert_non_null(v2);
+	res = dns_db_findnode(db2, dns_rootname, true, &node);
+	assert_int_equal(res, ISC_R_SUCCESS);
+	res = dns_db_findrdataset(db2, node, v2, dns_rdatatype_txt, 0, 0,
+				  &rdataset2, NULL);
+	assert_int_equal(res, ISC_R_SUCCESS);
+
+	/* db2: Convert result to text */
+	res = dns_rdataset_first(&rdataset2);
+	assert_int_equal(res, ISC_R_SUCCESS);
+	dns_rdataset_current(&rdataset2, &out2);
+	isc_buffer_init(&b, buf, sizeof(buf));
+	res = dns_rdata_totext(&out2, NULL, &b);
+	assert_int_equal(res, ISC_R_SUCCESS);
+	isc_buffer_putuint8(&b, 0);
+
+	/* db2: We should have "text 2" */
+	assert_string_equal(buf, "\"text 2\"");
+
+	dns_rdataset_disassociate(&rdataset2);
+
+	dns_db_closeversion(db2, &v2, true);
+	assert_null(v2);
+	dns_db_detachnode(db2, &node);
+	assert_null(node);
+}
+
 ISC_TEST_LIST_START
 ISC_TEST_ENTRY_CUSTOM(find, setup_test, teardown_test)
 ISC_TEST_ENTRY_CUSTOM(allrdatasets, setup_test, teardown_test)
@@ -373,6 +521,7 @@ ISC_TEST_ENTRY_CUSTOM(addrdataset, setup_test, teardown_test)
 ISC_TEST_ENTRY_CUSTOM(getnsec3parameters, setup_test, teardown_test)
 ISC_TEST_ENTRY_CUSTOM(attachversion, setup_test, teardown_test)
 ISC_TEST_ENTRY_CUSTOM(closeversion, setup_test, teardown_test)
+ISC_TEST_ENTRY_CUSTOM(rollback, setup_test, teardown_test)
 ISC_TEST_LIST_END
 
 ISC_TEST_MAIN

tests/dns/keytable_test.c

@@ -535,7 +535,6 @@ ISC_LOOP_TEST_IMPL(find) {
 
 /* check issecuredomain() */
 ISC_LOOP_TEST_IMPL(issecuredomain) {
-	bool issecure;
 	const char **n;
 	const char *names[] = { "example.com", "sub.example.com",
 				"null.example", "sub.null.example", NULL };
@@ -550,22 +549,16 @@ ISC_LOOP_TEST_IMPL(issecuredomain) {
 	 * of installing a null key).
 	 */
 	for (n = names; *n != NULL; n++) {
-		assert_int_equal(dns_keytable_issecuredomain(keytable,
-							     str2name(*n), NULL,
-							     &issecure),
-				 ISC_R_SUCCESS);
-		assert_true(issecure);
+		assert_true(dns_keytable_issecuredomain(keytable, str2name(*n),
+							NULL));
 	}
 
 	/*
 	 * If the key table has no entry (not even a null one) for a domain or
 	 * any of its ancestors, that domain is considered insecure.
 	 */
-	assert_int_equal(dns_keytable_issecuredomain(keytable,
-						     str2name("example.org"),
-						     NULL, &issecure),
-			 ISC_R_SUCCESS);
-	assert_false(issecure);
+	assert_false(dns_keytable_issecuredomain(
+		keytable, str2name("example.org"), NULL));
 
 	destroy_tables();
 
@@ -595,7 +588,7 @@ ISC_LOOP_TEST_IMPL(dump) {
 /* check negative trust anchors */
 ISC_LOOP_TEST_IMPL(nta) {
 	isc_result_t result;
-	bool issecure, covered;
+	bool covered;
 	dns_fixedname_t fn;
 	dns_name_t *keyname = dns_fixedname_name(&fn);
 	unsigned char digest[ISC_MAX_MD_SIZE];
@@ -626,20 +619,15 @@ ISC_LOOP_TEST_IMPL(nta) {
 	assert_int_equal(result, ISC_R_SUCCESS);
 
 	/* Should be secure */
-	result = dns_view_issecuredomain(myview,
-					 str2name("test.secure.example"), now,
-					 true, &covered, &issecure);
-	assert_int_equal(result, ISC_R_SUCCESS);
+	assert_true(dns_view_issecuredomain(
+		myview, str2name("test.secure.example"), now, true, &covered));
 	assert_false(covered);
-	assert_true(issecure);
 
 	/* Should not be secure */
-	result = dns_view_issecuredomain(myview,
-					 str2name("test.insecure.example"), now,
-					 true, &covered, &issecure);
-	assert_int_equal(result, ISC_R_SUCCESS);
+	assert_false(dns_view_issecuredomain(myview,
+					     str2name("test.insecure.example"),
+					     now, true, &covered));
 	assert_true(covered);
-	assert_false(issecure);
 
 	/* NTA covered */
 	covered = dns_view_ntacovers(myview, now, str2name("insecure.example"),
@@ -652,38 +640,30 @@ ISC_LOOP_TEST_IMPL(nta) {
 	assert_false(covered);
 
 	/* As of now + 2, the NTA should be clear */
-	result = dns_view_issecuredomain(myview,
-					 str2name("test.insecure.example"),
-					 now + 2, true, &covered, &issecure);
-	assert_int_equal(result, ISC_R_SUCCESS);
+	assert_true(dns_view_issecuredomain(myview,
+					    str2name("test.insecure.example"),
+					    now + 2, true, &covered));
 	assert_false(covered);
-	assert_true(issecure);
 
 	/* Now check deletion */
-	result = dns_view_issecuredomain(myview, str2name("test.new.example"),
-					 now, true, &covered, &issecure);
-	assert_int_equal(result, ISC_R_SUCCESS);
+	assert_true(dns_view_issecuredomain(
+		myview, str2name("test.new.example"), now, true, &covered));
 	assert_false(covered);
-	assert_true(issecure);
 
 	result = dns_ntatable_add(ntatable, str2name("new.example"), false, now,
 				  3600);
 	assert_int_equal(result, ISC_R_SUCCESS);
 
-	result = dns_view_issecuredomain(myview, str2name("test.new.example"),
-					 now, true, &covered, &issecure);
-	assert_int_equal(result, ISC_R_SUCCESS);
+	assert_false(dns_view_issecuredomain(
+		myview, str2name("test.new.example"), now, true, &covered));
 	assert_true(covered);
-	assert_false(issecure);
 
 	result = dns_ntatable_delete(ntatable, str2name("new.example"));
 	assert_int_equal(result, ISC_R_SUCCESS);
 
-	result = dns_view_issecuredomain(myview, str2name("test.new.example"),
-					 now, true, &covered, &issecure);
-	assert_int_equal(result, ISC_R_SUCCESS);
+	assert_true(dns_view_issecuredomain(
+		myview, str2name("test.new.example"), now, true, &covered));
 	assert_false(covered);
-	assert_true(issecure);
 
 	isc_loopmgr_shutdown(loopmgr);