From: Colin Vidal Date: Mon, 1 Jun 2026 14:58:45 +0000 (+0200) Subject: Use original query name when caching SERVFAIL X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=66af5b464deb61da1cbb334d37bfe91918ae6944;p=thirdparty%2Fbind9.git Use original query name when caching SERVFAIL Instead of using `client->query.qname` when caching a SERVFAIL answer, use `client->query.origqname` when available. This avoids caching a SERVFAIL against a CNAME target when the failure occurs while the resolver is following the CNAME chain. This is problematic, for instance, when the SERVFAIL is triggered by the `max-query-count` threshold being reached, which would incorrectly prevent legitimate resolution of the CNAME target while in the SERVFAIL cache. Note that if the SERVFAIL genuinely originated from resolving the CNAME target, that specific failure will no longer be cached, and a direct query for the CNAME target will trigger a fresh (likely failing) resolution attempt. However, this is still preferable to the previous behaviour, which would wrongly prevent resolving the CNAME target if it was cached for other reasons (like the example above). --- diff --git a/bin/tests/system/sfcache_cname/ans1/ans.py b/bin/tests/system/sfcache_cname/ans1/ans.py deleted file mode 100644 index 049a2d2009f..00000000000 --- a/bin/tests/system/sfcache_cname/ans1/ans.py +++ /dev/null @@ -1,53 +0,0 @@ -# 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. - - -import dns.name -import dns.rcode -import dns.rdataclass -import dns.rdatatype -import dns.rrset - -from isctest.asyncserver import AsyncDnsServer, QnameQtypeHandler, StaticResponseHandler - - -def rrset( - qname: dns.name.Name | str, - rtype: dns.rdatatype.RdataType, - rdata: str, - ttl: int = 300, -) -> dns.rrset.RRset: - return dns.rrset.from_text(qname, ttl, dns.rdataclass.IN, rtype, rdata) - - -class Tld1Handler(QnameQtypeHandler, StaticResponseHandler): - qnames = ["foo.tld1."] - qtypes = [dns.rdatatype.A] - answer = [rrset("foo.tld1.", dns.rdatatype.CNAME, "tld2.")] - - -class Tld2Handler(QnameQtypeHandler, StaticResponseHandler): - qnames = ["tld2."] - qtypes = [dns.rdatatype.A] - answer = [rrset("tld2.", dns.rdatatype.A, "1.2.3.4")] - - -def main() -> None: - server = AsyncDnsServer(default_aa=True, default_rcode=dns.rcode.NOERROR) - server.install_response_handlers( - Tld1Handler(), - Tld2Handler(), - ) - server.run() - - -if __name__ == "__main__": - main() diff --git a/bin/tests/system/sfcache_cname/ns1/named.conf.j2 b/bin/tests/system/sfcache_cname/ns1/named.conf.j2 new file mode 100644 index 00000000000..9f2271e7092 --- /dev/null +++ b/bin/tests/system/sfcache_cname/ns1/named.conf.j2 @@ -0,0 +1,16 @@ +options { + query-source address @ns.ip@; + notify-source @ns.ip@; + transfer-source @ns.ip@; + port @PORT@; + pid-file "named.pid"; + listen-on { @ns.ip@; }; + listen-on-v6 { none; }; + recursion no; + dnssec-validation no; +}; + +zone "." { + type primary; + file "root.db"; +}; diff --git a/bin/tests/system/sfcache_cname/ns1/root.db b/bin/tests/system/sfcache_cname/ns1/root.db new file mode 100644 index 00000000000..d2357839bea --- /dev/null +++ b/bin/tests/system/sfcache_cname/ns1/root.db @@ -0,0 +1,6 @@ +$TTL 300 +. IN SOA ns. hostmaster. 1 600 600 1200 600 +. NS ns +ns A 10.53.0.1 +foo.tld1. CNAME tld2. +tld2. A 1.2.3.4 diff --git a/bin/tests/system/sfcache_cname/ns2/named.conf.j2 b/bin/tests/system/sfcache_cname/ns2/named.conf.j2 index c501ad6416f..8bc955aceb5 100644 --- a/bin/tests/system/sfcache_cname/ns2/named.conf.j2 +++ b/bin/tests/system/sfcache_cname/ns2/named.conf.j2 @@ -11,16 +11,12 @@ options { max-query-count 2; }; -{% include "_common/controls.conf.j2" %} - zone "tld1." { - type forward; - forward only; - forwarders { 10.53.0.1; }; + type static-stub; + server-addresses { 10.53.0.1; }; }; zone "tld2." { - type forward; - forward only; - forwarders { 10.53.0.1; }; + type static-stub; + server-addresses { 10.53.0.1; }; }; diff --git a/bin/tests/system/sfcache_cname/tests_sfcache_cname.py b/bin/tests/system/sfcache_cname/tests_sfcache_cname.py index df838ddebd5..2c5fbe8363f 100644 --- a/bin/tests/system/sfcache_cname/tests_sfcache_cname.py +++ b/bin/tests/system/sfcache_cname/tests_sfcache_cname.py @@ -18,7 +18,7 @@ import isctest # A SERVFAIL produced while following a CNAME must be cached against the # original query name, not the CNAME target. # -# ans1 serves "foo.tld1 CNAME tld2" and "tld2 A 1.2.3.4"; ns2 forwards +# ns1 serves "foo.tld1 CNAME tld2" and "tld2 A 1.2.3.4"; ns2 forwards # both zones to it with "max-query-count 2". Resolving "foo.tld1/A" # follows the CNAME to "tld2" and then exhausts the query budget, so the # client gets SERVFAIL. That failure must be cached under the original diff --git a/lib/ns/client.c b/lib/ns/client.c index 45fdab1c589..7c38b2a4b0e 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -1030,6 +1030,9 @@ ns_client_error(ns_client_t *client, isc_result_t result) { isc_time_t expire; isc_interval_t i; uint32_t flags = 0; + dns_name_t *qname = client->query.origqname != NULL + ? client->query.origqname + : client->query.qname; if ((message->flags & DNS_MESSAGEFLAG_CD) != 0) { flags = NS_FAILCACHE_CD; @@ -1038,8 +1041,7 @@ ns_client_error(ns_client_t *client, isc_result_t result) { isc_interval_set(&i, client->inner.view->fail_ttl, 0); result = isc_time_nowplusinterval(&expire, &i); if (result == ISC_R_SUCCESS) { - dns_badcache_add(client->inner.view->failcache, - client->query.qname, + dns_badcache_add(client->inner.view->failcache, qname, client->query.qtype, flags, isc_time_seconds(&expire)); }