fix: usr: Glues from different parent are rejected

The changes making BIND 9 parent-centric !11621 introduced an issue where it could be possible, when processing a referral, to use the glue to a nameserver which has a different parent than the zonecut. For instance:

	AUTHORITY
	test.example.		NS	ns.test.example.
	test.example.		NS	ns.foo.example.
	test.example.		NS	ns.bar.
	
	ADDITIONAL
	ns.bar.			A	1.2.3.4
	ns.foo.example.		A	5.6.7.8
	ns.test.example.	A	9.8.7.6

In such situation, only the glues for `ns.foo.example.` and `ns.test.example.` should be used, and the glue from `ns.bar.` must be ignored as this is not either a sub-domain or a sibling domain, the parent is different (`bar.` instead of `example.`). This is now fixed.

Sibling glue and cyclic sibling glues are defined in RFC 9471 section 2.2 and section 2.3.

Merge branch 'colin/cyclic-glues-test' into 'main'

See merge request isc-projects/bind9!11873

bin/tests/system/cyclic_glue/ans5/ans.py

@@ -0,0 +1,74 @@
+"""
+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.
+"""
+
+from dns import name, rcode, rdataclass, rdatatype, rrset
+
+from isctest.asyncserver import AsyncDnsServer, QnameHandler, StaticResponseHandler
+
+
+def build_rrset(
+    qname: name.Name | str,
+    rtype: rdatatype.RdataType,
+    rdata: str,
+    ttl: int = 300,
+) -> rrset.RRset:
+    return rrset.from_text(qname, ttl, rdataclass.IN, rtype, rdata)
+
+
+class BrokenFooHandler(QnameHandler, StaticResponseHandler):
+    qnames = ["a.foo.test."]
+    qtypes = [rdatatype.A]
+    authority = [
+        build_rrset("foo.test.", rdatatype.NS, "ns.foo.test."),
+        build_rrset("foo.test.", rdatatype.NS, "ns.bar.test."),
+        build_rrset("foo.test.", rdatatype.NS, "ns.test2."),
+    ]
+    additional = [
+        build_rrset("ns.foo.test.", rdatatype.A, "10.53.0.3"),
+        # These glues don't belong, as they're outside the
+        # delegated domain. However, only the latest will be
+        # ignored by the resolver (the former, being a sibling
+        # glue, is still used.)
+        build_rrset("ns.bar.test.", rdatatype.A, "10.53.0.3"),
+        build_rrset("ns.test2.", rdatatype.A, "10.10.10.10"),
+    ]
+
+
+class BrokenBarHandler(QnameHandler, StaticResponseHandler):
+    qnames = ["a.bar.test."]
+    qtypes = [rdatatype.A]
+    authority = [
+        build_rrset("bar.test.", rdatatype.NS, "ns.bar.test."),
+        # This NS is valid but outside the bar.test domain.
+        build_rrset("bar.test.", rdatatype.NS, "ns2.foo.test."),
+        # This NS is wrong, it's not the qname.
+        # It will be ignored by the resolver.
+        build_rrset("bar.test2.", rdatatype.NS, "ns.test2."),
+    ]
+    additional = [
+        build_rrset("ns.bar.test.", rdatatype.A, "10.53.0.3"),
+        build_rrset("ns2.foo.test.", rdatatype.A, "10.53.0.4"),
+        # The glue is then ignored as well, is it doesn't match
+        # any of the valid NS above.
+        build_rrset("ns.test2.", rdatatype.A, "10.10.10.10"),
+    ]
+
+
+def main() -> None:
+    server = AsyncDnsServer(default_aa=True, default_rcode=rcode.NOERROR)
+    server.install_response_handlers(BrokenFooHandler(), BrokenBarHandler())
+    server.run()
+
+
+if __name__ == "__main__":
+    main()

bin/tests/system/cyclic_glue/ns1/named.conf.j2

@@ -0,0 +1,26 @@
+options {
+	query-source address 10.53.0.1;
+	notify-source 10.53.0.1;
+	transfer-source 10.53.0.1;
+	port @PORT@;
+	pid-file "named.pid";
+	listen-on { 10.53.0.1; };
+	listen-on-v6 { none; };
+	recursion no;
+	notify yes;
+	dnssec-validation no;
+};
+
+zone "." {
+	type primary;
+	file "root.db";
+};
+
+key rndc_key {
+	secret "1234abcd8765";
+	algorithm @DEFAULT_HMAC@;
+};
+
+controls {
+	inet 10.53.0.1 port @CONTROLPORT@ allow { any; } keys { rndc_key; };
+};

bin/tests/system/cyclic_glue/ns1/root.db.j2

@@ -0,0 +1,20 @@
+{% set broken_ns = broken_ns | default(False) %}
+
+$TTL 300
+. 			IN SOA	gson.nominum.com. a.root.servers.nil. (
+				2000042100   	; serial
+				600         	; refresh
+				600         	; retry
+				1200    	; expire
+				600       	; minimum
+				)
+.			NS	a.root-servers.nil.
+a.root-servers.nil.	A	10.53.0.1
+
+test.			NS	ns.test.
+
+{% if broken_ns %}
+ns.test.		A	10.53.0.5
+{% else %}
+ns.test.		A	10.53.0.2
+{% endif %}

bin/tests/system/cyclic_glue/ns2/named.conf.j2

@@ -0,0 +1,18 @@
+options {
+	query-source address 10.53.0.2;
+	notify-source 10.53.0.2;
+	transfer-source 10.53.0.2;
+	port @PORT@;
+	pid-file "named.pid";
+	listen-on { 10.53.0.2; };
+	listen-on-v6 { none; };
+	recursion no;
+	notify yes;
+	dnssec-validation no;
+	minimal-responses no;
+};
+
+zone "test" {
+	type primary;
+	file "test.db";
+};

bin/tests/system/cyclic_glue/ns2/test.db

@@ -0,0 +1,16 @@
+$TTL 300	; 5 minutes
+@			IN SOA	mname1. . (
+				2000042407 ; serial
+				20         ; refresh (20 seconds)
+				20         ; retry (20 seconds)
+				1814400    ; expire (3 weeks)
+				3600       ; minimum (1 hour)
+				)
+			NS	ns
+ns			A	10.53.0.2
+
+foo			NS 	ns.bar
+ns.foo			A	10.53.0.3
+
+bar			NS	ns.foo
+ns.bar			A	10.53.0.3

bin/tests/system/cyclic_glue/ns3/named.conf.j2

@@ -0,0 +1,22 @@
+options {
+	query-source address 10.53.0.3;
+	notify-source 10.53.0.3;
+	transfer-source 10.53.0.3;
+	port @PORT@;
+	pid-file "named.pid";
+	listen-on { 10.53.0.3; };
+	listen-on-v6 { none; };
+	recursion no;
+	notify yes;
+	dnssec-validation no;
+};
+
+zone "foo.test" {
+	type primary;
+	file "zones.db";
+};
+
+zone "bar.test" {
+	type primary;
+	file "zones.db";
+};

bin/tests/system/cyclic_glue/ns3/zones.db

@@ -0,0 +1,11 @@
+$TTL 300	; 5 minutes
+@			IN SOA	mname1. . (
+				2000042407 ; serial
+				20         ; refresh (20 seconds)
+				20         ; retry (20 seconds)
+				1814400    ; expire (3 weeks)
+				3600       ; minimum (1 hour)
+				)
+			NS	ns
+ns			A	10.53.0.3
+a			A	10.53.0.99

bin/tests/system/cyclic_glue/ns4/named.conf.j2

@@ -0,0 +1,25 @@
+options {
+	query-source address 10.53.0.4;
+	notify-source 10.53.0.4;
+	transfer-source 10.53.0.4;
+	port @PORT@;
+	pid-file "named.pid";
+	listen-on { 10.53.0.4; };
+	listen-on-v6 { none; };
+	recursion yes;
+	dnssec-validation no;
+};
+
+zone "." {
+	type hint;
+	file "../../_common/root.hint";
+};
+
+key rndc_key {
+	secret "1234abcd8765";
+	algorithm @DEFAULT_HMAC@;
+};
+
+controls {
+	inet 10.53.0.4 port @CONTROLPORT@ allow { any; } keys { rndc_key; };
+};

bin/tests/system/cyclic_glue/tests_cyclic_glue.py

@@ -0,0 +1,85 @@
+# 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.
+
+from re import compile as Re
+
+import isctest
+
+
+def run_queries(ns):
+    msg = isctest.query.create("a.foo.test.", "A")
+    res = isctest.query.udp(msg, ns.ip)
+    isctest.check.noerror(res)
+
+    msg = isctest.query.create("a.bar.test.", "A")
+    res = isctest.query.udp(msg, ns.ip)
+    isctest.check.noerror(res)
+
+
+def test_cyclic_glues(ns1, ns4, templates):
+    run_queries(ns4)
+    ns4.rndc("dumpdb -deleg")
+    dump = isctest.text.TextFile("ns4/named_dump.db")
+
+    # The test is using the correctly-behaving ns2 server.
+    assert len(dump.grep(Re("test. .* DELEG server-ipv4=10.53.0.2"))) == 1
+    assert len(dump.grep(Re("test. .* DELEG server-ipv4=10.53.0.5"))) == 0
+
+    # We've sent queries for both foo.test and bar.test and got a
+    # single in-domain address from each:
+    assert len(dump.grep(Re("foo.test. [0-9]* DELEG server-ipv4=10.53.0.3"))) == 1
+    assert len(dump.grep(Re("foo.test. [0-9]* DELEG"))) == 1
+
+    assert len(dump.grep(Re("bar.test. [0-9]* DELEG server-ipv4=10.53.0.3"))) == 1
+    assert len(dump.grep(Re("bar.test. [0-9]* DELEG"))) == 1
+
+    # in total we should have test, foo.test, and bar.test, nothing else:
+    assert len(dump.grep(Re("test. [0-9]* DELEG"))) == 3
+
+    templates.render("ns1/root.db", {"broken_ns": True})
+    with ns1.watch_log_from_here() as watcher:
+        ns1.rndc("reload")
+        watcher.wait_for_line("running")
+
+    with ns4.watch_log_from_here() as watcher:
+        ns4.rndc("flush")
+        watcher.wait_for_line("flushing caches in all views succeeded")
+
+    run_queries(ns4)
+    ns4.rndc("dumpdb -deleg")
+    dump = isctest.text.TextFile("ns4/named_dump.db")
+
+    # The test is now using the broken ans5 server.
+    assert len(dump.grep(Re("test. [0-9]* DELEG server-ipv4=10.53.0.2"))) == 0
+    assert len(dump.grep(Re("test. [0-9]* DELEG server-ipv4=10.53.0.5"))) == 1
+
+    # The bar and foo delegations include names and glue records
+    # that are out of bailiwick; we need to ensure that we are not
+    # using the address, but only its name.
+    assert len(dump.grep(Re("10.10.10.10"))) == 0
+    assert len(dump.grep(Re("test2. "))) == 0
+    assert len(dump.grep(Re("foo.test. [0-9]* DELEG server-name=ns.test2."))) == 1
+
+    # There should in principle be only one of these, but there is no guard
+    # to prevent duplicates when two glues for two different owner names
+    # (in this case, ns.foo.test and ns.bar.test) both point to the same
+    # IP address.
+    assert len(dump.grep(Re("foo.test. [0-9]* DELEG server-ipv4="))) > 0
+
+    # ns.bar.test is in-domain and should be stored as an address,
+    # not a server-name.
+    assert len(dump.grep(Re("bar.test. [0-9]* DELEG server-name=ns.bar.test."))) == 0
+    assert len(dump.grep(Re("bar.test. [0-9]* DELEG server-ipv4=10.53.0.3"))) == 1
+
+    # Since ns2.foo.test. came from the same parent (sibling glue) we have
+    # its address, NOT its server-name.
+    assert len(dump.grep(Re("bar.test. [0-9]* DELEG server-name=ns2.foo.test."))) == 0
+    assert len(dump.grep(Re("bar.test. [0-9]* DELEG server-ipv4=10.53.0.4"))) == 1

lib/dns/resolver.c

@@ -6689,12 +6689,26 @@ cache_delegns(respctx_t *rctx) {
 	dns_delegdb_t *delegdb = fctx->res->view->deleg;
 	dns_delegset_t *delegset = NULL;
 	dns_ttl_t ttl = rctx->ns_rdataset->ttl;
+	dns_fixedname_t fparent;
+	dns_name_t *parent = dns_fixedname_initname(&fparent);
+	size_t labels;
 	isc_result_t result;
 
 	FCTXTRACE("cache_delegns");
 
 	dns_delegset_allocset(delegdb, &delegset);
 
+	/*
+	 * The top of the delegated zone is `rctx->ns_name`. So truncating
+	 * the first label gives the common parent domain allowed to get
+	 * glues (this allows in-domain and sibling, but not different
+	 * parents).
+	 */
+	labels = dns_name_countlabels(rctx->ns_name);
+	if (labels > 1) {
+		dns_name_getlabelsequence(rctx->ns_name, 1, labels - 1, parent);
+	}
+
 	DNS_RDATASET_FOREACH(rctx->ns_rdataset) {
 		dns_rdataset_t *gluerdataset = NULL;
 		dns_rdata_t rdata = DNS_RDATA_INIT;
@@ -6715,20 +6729,26 @@ cache_delegns(respctx_t *rctx) {
 		INSIST(rdata.type == dns_rdatatype_ns);
 		dns_rdata_tostruct(&rdata, &ns, NULL);
 
-		result = dns_message_findname(
-			rctx->query->rmessage, DNS_SECTION_ADDITIONAL, &ns.name,
-			dns_rdatatype_a, 0, NULL, &gluerdataset);
-		if (result == ISC_R_SUCCESS) {
-			cache_delegglue(delegset, deleg, &ttl, gluerdataset);
-			gluerdataset = NULL;
-		}
+		if (labels > 1 && dns_name_issubdomain(&ns.name, parent)) {
+			result = dns_message_findname(rctx->query->rmessage,
+						      DNS_SECTION_ADDITIONAL,
+						      &ns.name, dns_rdatatype_a,
+						      0, NULL, &gluerdataset);
+			if (result == ISC_R_SUCCESS) {
+				cache_delegglue(delegset, deleg, &ttl,
+						gluerdataset);
+				gluerdataset = NULL;
+			}
 
-		result = dns_message_findname(
-			rctx->query->rmessage, DNS_SECTION_ADDITIONAL, &ns.name,
-			dns_rdatatype_aaaa, 0, NULL, &gluerdataset);
-		if (result == ISC_R_SUCCESS) {
-			cache_delegglue6(delegset, deleg, &ttl, gluerdataset);
-			gluerdataset = NULL;
+			result = dns_message_findname(
+				rctx->query->rmessage, DNS_SECTION_ADDITIONAL,
+				&ns.name, dns_rdatatype_aaaa, 0, NULL,
+				&gluerdataset);
+			if (result == ISC_R_SUCCESS) {
+				cache_delegglue6(delegset, deleg, &ttl,
+						 gluerdataset);
+				gluerdataset = NULL;
+			}
 		}
 
 		if (ISC_LIST_EMPTY(deleg->addresses)) {