diff --git a/bin/tests/system/cyclic_glue/ans5/ans.py b/bin/tests/system/cyclic_glue/ans5/ans.py new file mode 100644 index 0000000000..4be5b9d382 --- /dev/null +++ b/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() diff --git a/bin/tests/system/cyclic_glue/ns1/named.conf.j2 b/bin/tests/system/cyclic_glue/ns1/named.conf.j2 new file mode 100644 index 0000000000..44f64403e4 --- /dev/null +++ b/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; }; +}; diff --git a/bin/tests/system/cyclic_glue/ns1/root.db.j2 b/bin/tests/system/cyclic_glue/ns1/root.db.j2 new file mode 100644 index 0000000000..1dfdfd137f --- /dev/null +++ b/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 %} diff --git a/bin/tests/system/cyclic_glue/ns2/named.conf.j2 b/bin/tests/system/cyclic_glue/ns2/named.conf.j2 new file mode 100644 index 0000000000..6acab2ecb5 --- /dev/null +++ b/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"; +}; diff --git a/bin/tests/system/cyclic_glue/ns2/test.db b/bin/tests/system/cyclic_glue/ns2/test.db new file mode 100644 index 0000000000..6b4db7f57d --- /dev/null +++ b/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 diff --git a/bin/tests/system/cyclic_glue/ns3/named.conf.j2 b/bin/tests/system/cyclic_glue/ns3/named.conf.j2 new file mode 100644 index 0000000000..5bc355ce26 --- /dev/null +++ b/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"; +}; diff --git a/bin/tests/system/cyclic_glue/ns3/zones.db b/bin/tests/system/cyclic_glue/ns3/zones.db new file mode 100644 index 0000000000..5999613e92 --- /dev/null +++ b/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 diff --git a/bin/tests/system/cyclic_glue/ns4/named.conf.j2 b/bin/tests/system/cyclic_glue/ns4/named.conf.j2 new file mode 100644 index 0000000000..6888622874 --- /dev/null +++ b/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; }; +}; diff --git a/bin/tests/system/cyclic_glue/tests_cyclic_glue.py b/bin/tests/system/cyclic_glue/tests_cyclic_glue.py new file mode 100644 index 0000000000..ff71db7a6b --- /dev/null +++ b/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 diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 3e5018f241..19c4c29f0d 100644 --- a/lib/dns/resolver.c +++ b/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)) {