Merge branch '392-send-upstream-tat-queries-for-locally-served-zones' into 'master'

Send upstream trust anchor telemetry queries for locally served zones

Closes #392

See merge request isc-projects/bind9!484

CHANGES

@@ -1,3 +1,6 @@
+4994.	[bug]		Trust anchor telemetry queries were not being sent
+			upstream for locally served zones. [GL #392]
+
 4993.	[cleanup]	Remove support for silently ignoring 'no-change' deltas
 			from BIND 8 when processing an IXFR stream. 'no-change'
 			deltas will now trigger a fallback to AXFR as the

bin/named/server.c

@@ -6500,35 +6500,34 @@ struct dotat_arg {
 	isc_task_t *task;
 };
 
-static void
-dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) {
-	isc_result_t result;
+/*%
+ * Prepare the QNAME for the TAT query to be sent by processing the trust
+ * anchors present at 'keynode' of 'keytable'.  Store the result in 'dst' and
+ * the domain name which 'keynode' is associated with in 'origin'.
+ *
+ * A maximum of 12 key IDs can be reported in a single TAT query due to the
+ * 63-octet length limit for any single label in a domain name.  If there are
+ * more than 12 keys configured at 'keynode', only the first 12 will be
+ * reported in the TAT query.
+ */
+static isc_result_t
+get_tat_qname(dns_name_t *dst, const dns_name_t **origin,
+	      dns_keytable_t *keytable, dns_keynode_t *keynode)
+{
 	dns_keynode_t *firstnode = keynode;
 	dns_keynode_t *nextnode;
 	unsigned int i, n = 0;
-	char label[64], namebuf[DNS_NAME_FORMATSIZE];
-	dns_fixedname_t fixed;
-	dns_name_t *tatname;
-	isc_uint16_t ids[12]; /* Only 12 id's will fit in a label. */
-	int m;
-	ns_tat_t *tat;
-	dns_name_t *name = NULL;
-	struct dotat_arg *dotat_arg = arg;
-	dns_view_t *view;
-	isc_task_t *task;
+	isc_uint16_t ids[12];
 	isc_textregion_t r;
+	char label[64];
+	int m;
 
-	REQUIRE(keytable != NULL);
-	REQUIRE(keynode != NULL);
-	REQUIRE(arg != NULL);
-
-	view = dotat_arg->view;
-	task = dotat_arg->task;
+	REQUIRE(origin != NULL && *origin == NULL);
 
 	do {
 		dst_key_t *key = dns_keynode_key(keynode);
 		if (key != NULL) {
-			name = dst_key_name(key);
+			*origin = dst_key_name(key);
 			if (n < (sizeof(ids)/sizeof(ids[0]))) {
 				ids[n] = dst_key_id(key);
 				n++;
@@ -6543,7 +6542,7 @@ dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) {
 	} while (keynode != NULL);
 
 	if (n == 0) {
-		return;
+		return (DNS_R_EMPTYNAME);
 	}
 
 	if (n > 1) {
@@ -6559,18 +6558,42 @@ dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) {
 	r.length = sizeof(label);
 	m = snprintf(r.base, r.length, "_ta");
 	if (m < 0 || (unsigned)m > r.length) {
-		return;
+		return (ISC_R_FAILURE);
 	}
 	isc_textregion_consume(&r, m);
 	for (i = 0; i < n; i++) {
 		m = snprintf(r.base, r.length, "-%04x", ids[i]);
 		if (m < 0 || (unsigned)m > r.length) {
-			return;
+			return (ISC_R_FAILURE);
 		}
 		isc_textregion_consume(&r, m);
 	}
+
+	return (dns_name_fromstring2(dst, label, *origin, 0, NULL));
+}
+
+static void
+dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) {
+	struct dotat_arg *dotat_arg = arg;
+	char namebuf[DNS_NAME_FORMATSIZE];
+	const dns_name_t *origin = NULL;
+	dns_fixedname_t fixed, fdomain;
+	dns_name_t *tatname, *domain;
+	dns_rdataset_t nameservers;
+	isc_result_t result;
+	dns_view_t *view;
+	isc_task_t *task;
+	ns_tat_t *tat;
+
+	REQUIRE(keytable != NULL);
+	REQUIRE(keynode != NULL);
+	REQUIRE(arg != NULL);
+
+	view = dotat_arg->view;
+	task = dotat_arg->task;
+
 	tatname = dns_fixedname_initname(&fixed);
-	result = dns_name_fromstring2(tatname, label, name, 0, NULL);
+	result = get_tat_qname(tatname, &origin, keytable, keynode);
 	if (result != ISC_R_SUCCESS) {
 		return;
 	}
@@ -6594,12 +6617,54 @@ dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) {
 	isc_mem_attach(dotat_arg->view->mctx, &tat->mctx);
 	isc_task_attach(task, &tat->task);
 
-	result = dns_resolver_createfetch(view->resolver, tatname,
-					  dns_rdatatype_null, NULL, NULL,
-					  NULL, NULL, 0, 0, 0, NULL, tat->task,
-					  tat_done, tat, &tat->rdataset,
-					  &tat->sigrdataset, &tat->fetch);
+	/*
+	 * TAT queries should be sent to the authoritative servers for a given
+	 * zone.  If this function is called for a keytable node corresponding
+	 * to a locally served zone, calling dns_resolver_createfetch() with
+	 * NULL 'domain' and 'nameservers' arguments will cause 'tatname' to be
+	 * resolved locally, without sending any TAT queries upstream.
+	 *
+	 * Work around this issue by calling dns_view_findzonecut() first.  If
+	 * the zone is served locally, the NS RRset for the given domain name
+	 * will be retrieved from local data; if it is not, the deepest zone
+	 * cut we have for it will be retrieved from cache.  In either case,
+	 * passing the results to dns_resolver_createfetch() will prevent it
+	 * from returning NXDOMAIN for 'tatname' while still allowing it to
+	 * chase down any potential delegations returned by upstream servers in
+	 * order to eventually find the destination host to send the TAT query
+	 * to.
+	 *
+	 * 'origin' holds the domain name at 'keynode', i.e. the domain name
+	 * for which the trust anchors to be reported by this TAT query are
+	 * defined.
+	 *
+	 * After the dns_view_findzonecut() call, 'domain' will hold the
+	 * deepest zone cut we can find for 'origin' while 'nameservers' will
+	 * hold the NS RRset at that zone cut.
+	 */
+	domain = dns_fixedname_initname(&fdomain);
+	dns_rdataset_init(&nameservers);
+	result = dns_view_findzonecut(view, origin, domain, 0, 0, ISC_TRUE,
+				      ISC_TRUE, &nameservers, NULL);
+	if (result != ISC_R_SUCCESS) {
+		goto done;
+	}
 
+	result = dns_resolver_createfetch(view->resolver, tatname,
+					  dns_rdatatype_null, domain,
+					  &nameservers, NULL, NULL, 0, 0, 0,
+					  NULL, tat->task, tat_done, tat,
+					  &tat->rdataset, &tat->sigrdataset,
+					  &tat->fetch);
+
+	/*
+	 * dns_resolver_createfetch() creates its own copies of 'domain' and
+	 * 'nameservers'; clean up the latter (the former points into a
+	 * dst_key_t structure and thus must not be freed).
+	 */
+	dns_rdataset_disassociate(&nameservers);
+
+ done:
 	if (result != ISC_R_SUCCESS) {
 		isc_task_detach(&tat->task);
 		isc_mem_putanddetach(&tat->mctx, tat, sizeof(*tat));

bin/tests/system/mirror/ns3/named.args

@@ -0,0 +1 @@
+-X named.lock -m record,size,mctx -T clienttest -c named.conf -d 99 -g -U 4 -T tat=1

bin/tests/system/mirror/ns3/named.conf.in

@@ -28,6 +28,7 @@ options {
 	listen-on-v6 { none; };
 	recursion yes;
 	allow-query-cache { 10.53.0.1; };
+	trust-anchor-telemetry yes;
 };
 
 zone "." {

bin/tests/system/mirror/tests.sh

@@ -404,5 +404,13 @@ nextpart ns3/named.run | grep "No correct RSASHA256 signature for verify-reconfi
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
+n=`expr $n + 1`
+echo_i "ensuring trust anchor telemetry queries are sent upstream for a mirror zone ($n)"
+ret=0
+# ns3 is started with "-T tat=1", so TAT queries should have already been sent.
+grep "_ta-[-0-9a-f]*/NULL" ns1/named.run > /dev/null || ret=1
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
 echo_i "exit status: $status"
 [ $status -eq 0 ] || exit 1

util/copyrights

@@ -1573,6 +1573,7 @@
 ./bin/tests/system/mirror/ns2/sign.sh		SH	2018
 ./bin/tests/system/mirror/ns2/sub.example.db.in	ZONE	2018
 ./bin/tests/system/mirror/ns2/verify.db.in	ZONE	2018
+./bin/tests/system/mirror/ns3/named.args	X	2018
 ./bin/tests/system/mirror/ns3/named.conf.in	CONF-C	2018
 ./bin/tests/system/mirror/setup.sh		SH	2018
 ./bin/tests/system/mirror/tests.sh		SH	2018