From 07f72afd13e609c301cab136a632c6369c638e70 Mon Sep 17 00:00:00 2001 From: Alessio Podda Date: Mon, 13 Apr 2026 15:55:38 +0200 Subject: [PATCH 01/14] Add xfr quota starvation system test Add a starvation test that tries to starve the XFR quota with unautorized requests. (cherry picked from commit 086cf2312c76c7de20856bcc5946135aab426435) --- bin/tests/system/xferquota/ns1/named.conf.in | 2 + bin/tests/system/xferquota/ns3/named.conf.in | 46 ++++++++++++++++ bin/tests/system/xferquota/ns3/quota.db | 22 ++++++++ bin/tests/system/xferquota/ns3/root.db | 21 ++++++++ bin/tests/system/xferquota/setup.sh | 1 + bin/tests/system/xferquota/tests_xferquota.py | 53 +++++++++++++++++++ 6 files changed, 145 insertions(+) create mode 100644 bin/tests/system/xferquota/ns3/named.conf.in create mode 100644 bin/tests/system/xferquota/ns3/quota.db create mode 100644 bin/tests/system/xferquota/ns3/root.db create mode 100644 bin/tests/system/xferquota/tests_xferquota.py diff --git a/bin/tests/system/xferquota/ns1/named.conf.in b/bin/tests/system/xferquota/ns1/named.conf.in index c9f19f92d0..a62a4c59fe 100644 --- a/bin/tests/system/xferquota/ns1/named.conf.in +++ b/bin/tests/system/xferquota/ns1/named.conf.in @@ -21,6 +21,8 @@ options { listen-on-v6 { none; }; recursion no; notify yes; + + transfers-out 3; }; key rndc_key { diff --git a/bin/tests/system/xferquota/ns3/named.conf.in b/bin/tests/system/xferquota/ns3/named.conf.in new file mode 100644 index 0000000000..4a0d70ca55 --- /dev/null +++ b/bin/tests/system/xferquota/ns3/named.conf.in @@ -0,0 +1,46 @@ +/* + * 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. + */ + +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; + dnssec-validation no; + + transfers-out 1; + allow-transfer { 10.53.0.2; }; +}; + +key rndc_key { + secret "1234abcd8765"; + algorithm @DEFAULT_HMAC@; +}; + +controls { + inet 10.53.0.3 port @CONTROLPORT@ allow { any; } keys { rndc_key; }; +}; + +zone "." { + type primary; + file "root.db"; +}; + +zone "quota." { + type primary; + file "quota.db"; +}; diff --git a/bin/tests/system/xferquota/ns3/quota.db b/bin/tests/system/xferquota/ns3/quota.db new file mode 100644 index 0000000000..12a67d3d2a --- /dev/null +++ b/bin/tests/system/xferquota/ns3/quota.db @@ -0,0 +1,22 @@ +; 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. + +$TTL 300 +@ IN SOA ns1.quota. hostmaster.quota. ( + 1 ; serial + 3600 ; refresh + 1800 ; retry + 604800 ; expire + 600 ; minimum + ) + IN NS ns1.quota. +ns1 IN A 10.53.0.3 +www IN A 10.0.0.1 diff --git a/bin/tests/system/xferquota/ns3/root.db b/bin/tests/system/xferquota/ns3/root.db new file mode 100644 index 0000000000..a5ff0fc697 --- /dev/null +++ b/bin/tests/system/xferquota/ns3/root.db @@ -0,0 +1,21 @@ +; 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. + +$TTL 300 +. IN SOA ns.root. hostmaster.root. ( + 1 ; serial + 3600 ; refresh + 1800 ; retry + 604800 ; expire + 600 ; minimum + ) +. NS a.root-servers.nil. +a.root-servers.nil. A 10.53.0.3 diff --git a/bin/tests/system/xferquota/setup.sh b/bin/tests/system/xferquota/setup.sh index c8c488d5fb..10daf80563 100644 --- a/bin/tests/system/xferquota/setup.sh +++ b/bin/tests/system/xferquota/setup.sh @@ -24,3 +24,4 @@ cp -f ns1/changing1.db ns1/changing.db copy_setports ns1/named.conf.in ns1/named.conf copy_setports ns2/named.conf.in ns2/named.conf +copy_setports ns3/named.conf.in ns3/named.conf diff --git a/bin/tests/system/xferquota/tests_xferquota.py b/bin/tests/system/xferquota/tests_xferquota.py new file mode 100644 index 0000000000..9d74000e0b --- /dev/null +++ b/bin/tests/system/xferquota/tests_xferquota.py @@ -0,0 +1,53 @@ +# 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 multiprocessing +import time + +import dns.message +import dns.query +import dns.zone + + +def _flood_unauthorized_axfrs(port, duration): + """Child process: send unauthorized AXFR requests for `duration` seconds.""" + deadline = time.monotonic() + duration + while time.monotonic() < deadline: + try: + msg = dns.message.make_query("quota.", "AXFR") + dns.query.tcp(msg, "10.53.0.3", port=port, timeout=2, source="10.53.0.1") + except Exception: # pylint: disable=broad-exception-caught + pass + + +def test_xfrquota_unauthorized_no_starve(named_port): + """Unauthorized AXFR clients must not consume XFR-out quota (GL #3859). + + ns3 is configured with transfers-out 1 and allow-transfer { 10.53.0.2; }. + We flood AXFR requests from unauthorized source processes (10.53.0.1) and + verify that an authorized client (10.53.0.2) can still transfer. + """ + with multiprocessing.Pool(10) as pool: + pool.starmap_async(_flood_unauthorized_axfrs, [(named_port, 5)] * 10) + + # Give the flood a moment to saturate + time.sleep(1) + + # Try an authorized AXFR from 10.53.0.2 multiple times to increase + # the chance of hitting the race window where quota is consumed. + zone = dns.zone.Zone("quota.") + dns.query.inbound_xfr( + "10.53.0.3", + zone, + port=named_port, + timeout=10, + source="10.53.0.2", + ) From d2cca91835f494cc699c3f1777a916e6f5152df9 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Thu, 16 Apr 2026 15:17:46 +0000 Subject: [PATCH 02/14] Apply XFR-out quota after ACL is checked Unauthorized clients can consume XFR-out quota and block authorized XFR clients. Apply the quota after ACL is checked. (cherry picked from commit e8268b76a8a1cde039131234b04b197ee7b46220) --- lib/ns/xfrout.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/ns/xfrout.c b/lib/ns/xfrout.c index 271b462d64..4f57d4c176 100644 --- a/lib/ns/xfrout.c +++ b/lib/ns/xfrout.c @@ -764,16 +764,6 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) { ns_client_log(client, DNS_LOGCATEGORY_XFER_OUT, NS_LOGMODULE_XFER_OUT, ISC_LOG_DEBUG(6), "%s request", mnemonic); - /* - * Apply quota. - */ - result = isc_quota_attach(&client->sctx->xfroutquota, "a); - if (result != ISC_R_SUCCESS) { - isc_log_write(XFROUT_COMMON_LOGARGS, ISC_LOG_WARNING, - "%s request denied: %s", mnemonic, - isc_result_totext(result)); - goto failure; - } /* * Interpret the question section. @@ -945,6 +935,18 @@ got_soa: FAILC(DNS_R_FORMERR, "attempted AXFR over UDP"); } + /* + * Apply quota after ACL is checked, so that unauthorized clients + * can not starve the authorized clients. + */ + result = isc_quota_attach(&client->sctx->xfroutquota, "a); + if (result != ISC_R_SUCCESS) { + isc_log_write(XFROUT_COMMON_LOGARGS, ISC_LOG_WARNING, + "%s request denied: %s", mnemonic, + isc_result_totext(result)); + goto failure; + } + /* * Look up the requesting server in the peer table. */ @@ -1218,6 +1220,7 @@ failure: } /* XXX kludge */ if (xfr != NULL) { + /* The quota will be released in xfrout_ctx_destroy(). */ xfrout_fail(xfr, result, "setting up zone transfer"); } else if (result != ISC_R_SUCCESS) { ns_client_log(client, DNS_LOGCATEGORY_XFER_OUT, From c78042a013c04bd5db894543a6a02a7ed59e44c4 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Tue, 7 Apr 2026 22:18:10 +0200 Subject: [PATCH 03/14] Refactor incrementing query counters Move the logic incrementing the query counter and the global query counter into a dedicated helper function. (cherry picked from commit 00345dde8feadf6601c864f000d99e42986159d9) --- lib/dns/resolver.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index a823f5a705..2fdadceb08 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -4294,6 +4294,21 @@ fctx_nextaddress(fetchctx_t *fctx) { return (addrinfo); } +static isc_result_t +incr_query_counters(fetchctx_t *fctx) { + isc_result_t result; + + result = isc_counter_increment(fctx->qc); + if (result != ISC_R_SUCCESS) { + isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER, + DNS_LOGMODULE_RESOLVER, ISC_LOG_DEBUG(3), + "exceeded max queries resolving '%s'", + fctx->info); + } + + return result; +} + static void fctx_try(fetchctx_t *fctx, bool retrying, bool badcache) { isc_result_t result; @@ -4433,12 +4448,8 @@ fctx_try(fetchctx_t *fctx, bool retrying, bool badcache) { return; } - result = isc_counter_increment(fctx->qc); + result = incr_query_counters(fctx); if (result != ISC_R_SUCCESS) { - isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER, - DNS_LOGMODULE_RESOLVER, ISC_LOG_DEBUG(3), - "exceeded max queries resolving '%s'", - fctx->info); fctx_done(fctx, DNS_R_SERVFAIL, __LINE__); return; } From eb73df0cebc8909a030a0db66d4366e2181bd3f5 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Tue, 7 Apr 2026 22:18:58 +0200 Subject: [PATCH 04/14] rctx_resend() increment query counters Calls to `rctx_resend()` are done internally within the resolver, in flow which are not supposed to happens more than once. For instance, if some query fails, and a specific flag "F" wasn't set, then set the flag and try again. This wouldn't occur more than once because if the query fails the next attempt, the flag "F" would be set already, so the resolver would move to the next server (or give up). However, a subtle bug missing checking a flag, for instance, could lead to an unbounded loop re-trying to query the same server. This is now impossible as `rctx_resend()` also increment the query counters (so if such case occurs, it would stop once the maximum limit is reached). (cherry picked from commit b863694b32f8f764ae7475939888aebe99425b90) --- lib/dns/resolver.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 2fdadceb08..4fbe31ad81 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -10107,6 +10107,13 @@ rctx_resend(respctx_t *rctx, dns_adbaddrinfo_t *addrinfo) { unsigned int bucketnum; FCTXTRACE("resend"); + + result = incr_query_counters(fctx); + if (result != ISC_R_SUCCESS) { + fctx_done(fctx, DNS_R_SERVFAIL, __LINE__); + return; + } + inc_stats(fctx->res, dns_resstatscounter_retry); fctx_increference(fctx); result = fctx_query(fctx, addrinfo, rctx->retryopts); From 20d79632c20ba81e66dec9a9e158ac17909ede1a Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Thu, 5 Feb 2026 09:46:01 +0100 Subject: [PATCH 05/14] Limit the number of addresses returned per ADB find Add a hard limit on the number of addresses that ADB returns from a single NS lookup (dns_adbfind_t). This mitigates a flood attack where an attacker controls a zone with many addresses for a nameserver, each returning an invalid response. The global max-query count (default 50) also limits this, but significant harm can be done before that limit is reached. The default limit is now 6 (v4 and/or v6) addresses for an ADB find (so, ADB looking up for A/AAAA addresses of a name server name). (cherry picked from commit 9b61b194c58c02d4fbbbe439eceeec8fd2a2a54c) --- lib/dns/adb.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/lib/dns/adb.c b/lib/dns/adb.c index 6f98feca45..99e83100de 100644 --- a/lib/dns/adb.c +++ b/lib/dns/adb.c @@ -86,6 +86,12 @@ #define DNS_ADB_MINADBSIZE (1024U * 1024U) /*%< 1 Megabyte */ +/* + * Default for the per-find address limit, the sum of the number of A and AAAA + * RR from an ADB NS name resolution + */ +#define DEFAULT_ADDRSLIMIT 6 + typedef ISC_LIST(dns_adbname_t) dns_adbnamelist_t; typedef struct dns_adbnamehook dns_adbnamehook_t; typedef ISC_LIST(dns_adbnamehook_t) dns_adbnamehooklist_t; @@ -2227,6 +2233,7 @@ copy_namehook_lists(dns_adb_t *adb, dns_adbfind_t *find, dns_adbaddrinfo_t *addrinfo; dns_adbentry_t *entry; int bucket; + size_t count = 0; bucket = DNS_ADB_INVALIDBUCKET; @@ -2261,6 +2268,13 @@ copy_namehook_lists(dns_adb_t *adb, dns_adbfind_t *find, inc_entry_refcnt(adb, entry, false); ISC_LIST_APPEND(find->list, addrinfo, publink); addrinfo = NULL; + + if (++count >= DEFAULT_ADDRSLIMIT) { + DP(ISC_LOG_DEBUG(3), "skipping addresses"); + UNLOCK(&adb->entrylocks[bucket]); + return; + } + nextv4: UNLOCK(&adb->entrylocks[bucket]); bucket = DNS_ADB_INVALIDBUCKET; @@ -2299,6 +2313,13 @@ copy_namehook_lists(dns_adb_t *adb, dns_adbfind_t *find, inc_entry_refcnt(adb, entry, false); ISC_LIST_APPEND(find->list, addrinfo, publink); addrinfo = NULL; + + if (++count >= DEFAULT_ADDRSLIMIT) { + DP(ISC_LOG_DEBUG(3), "skipping addresses"); + UNLOCK(&adb->entrylocks[bucket]); + return; + } + nextv6: UNLOCK(&adb->entrylocks[bucket]); bucket = DNS_ADB_INVALIDBUCKET; From 5d7f468c7784d8806e230b784f5e63cb4c6c76ce Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Wed, 4 Feb 2026 10:18:42 +0100 Subject: [PATCH 06/14] Remove duplicate addresses from the resolver SLIST The SLIST (essentially `fctx->finds`, forwarders and dual-stack alternatives aside) can have duplicate server addresses when multiple in-domain nameservers share the same IP addresses: sub.example. NS ns1.sub.example. sub.example. NS ns2.sub.example. ns1.sub.example. A 1.2.3.4 ns1.sub.example. A 5.6.7.8 ns2.sub.example. A 1.2.3.4 ns2.sub.example. A 5.6.7.8 If both 1.2.3.4 and 5.6.7.8 fail to return a valid answer, the resolver would query each address twice. The problem is fixed by replacing the two-phase server selection (sort each find list by SRTT, sort finds by head SRTT) with a single linear scan in nextaddress() that finds the lowest-SRTT unmarked, non-duplicate address across all find lists. The old approach had a correctness bug: after sorting, the resolver picked the next address from the "current" find list rather than globally. For example, with find lists [1, 15, 26] and [3, 4, 5], the second pick would be SRTT 15 instead of the correct SRTT 3. The new approach is both simpler and correct: each call to nextaddress() walks all addresses, skips marked and duplicate entries, and returns the one with the lowest SRTT. While this walk is repeated for each server attempt, it operates on a small bounded list and is negligible compared to the network I/O of querying the server. (cherry picked from commit ced6b66fafa1badfd044d569a6d3cfbdb600f5a7) --- lib/dns/resolver.c | 220 ++++++++++++++++++--------------------------- 1 file changed, 89 insertions(+), 131 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 4fbe31ad81..ee49128825 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -319,7 +319,16 @@ struct fetchctx { dns_message_t *qmessage; ISC_LIST(resquery_t) queries; dns_adbfindlist_t finds; - dns_adbfind_t *find; + /* + * This is a state to keep track of the latest upstream server which is + * being queried. See `nextaddress()`. + * + * `addrinfo` is basically a copy of `foundaddrinfo` but came from the + * response of the query, so fields like the SRTT/timing might have been + * altered. So it might be possible (?) to wrap those two in an union + * for clarity (and memory saving). + */ + dns_adbaddrinfo_t *foundaddrinfo; /* * altfinds are names and/or addresses of dual stack servers that * should be used when iterative resolution to a server is not @@ -1544,7 +1553,7 @@ fctx_cleanupfinds(fetchctx_t *fctx) { ISC_LIST_UNLINK(fctx->finds, find, publink); dns_adb_destroyfind(&find); } - fctx->find = NULL; + fctx->foundaddrinfo = NULL; } static void @@ -3509,88 +3518,6 @@ add_bad(fetchctx_t *fctx, dns_message_t *rmessage, dns_adbaddrinfo_t *addrinfo, dns_result_totext(reason), namebuf, typebuf, classbuf, addrbuf); } -/* - * Sort addrinfo list by RTT. - */ -static void -sort_adbfind(dns_adbfind_t *find, unsigned int bias) { - dns_adbaddrinfo_t *best, *curr; - dns_adbaddrinfolist_t sorted; - unsigned int best_srtt, curr_srtt; - - /* Lame N^2 bubble sort. */ - ISC_LIST_INIT(sorted); - while (!ISC_LIST_EMPTY(find->list)) { - best = ISC_LIST_HEAD(find->list); - best_srtt = best->srtt; - if (isc_sockaddr_pf(&best->sockaddr) != AF_INET6) { - best_srtt += bias; - } - curr = ISC_LIST_NEXT(best, publink); - while (curr != NULL) { - curr_srtt = curr->srtt; - if (isc_sockaddr_pf(&curr->sockaddr) != AF_INET6) { - curr_srtt += bias; - } - if (curr_srtt < best_srtt) { - best = curr; - best_srtt = curr_srtt; - } - curr = ISC_LIST_NEXT(curr, publink); - } - ISC_LIST_UNLINK(find->list, best, publink); - ISC_LIST_APPEND(sorted, best, publink); - } - find->list = sorted; -} - -/* - * Sort a list of finds by server RTT. - */ -static void -sort_finds(dns_adbfindlist_t *findlist, unsigned int bias) { - dns_adbfind_t *best, *curr; - dns_adbfindlist_t sorted; - dns_adbaddrinfo_t *addrinfo, *bestaddrinfo; - unsigned int best_srtt, curr_srtt; - - /* Sort each find's addrinfo list by SRTT. */ - for (curr = ISC_LIST_HEAD(*findlist); curr != NULL; - curr = ISC_LIST_NEXT(curr, publink)) - { - sort_adbfind(curr, bias); - } - - /* Lame N^2 bubble sort. */ - ISC_LIST_INIT(sorted); - while (!ISC_LIST_EMPTY(*findlist)) { - best = ISC_LIST_HEAD(*findlist); - bestaddrinfo = ISC_LIST_HEAD(best->list); - INSIST(bestaddrinfo != NULL); - best_srtt = bestaddrinfo->srtt; - if (isc_sockaddr_pf(&bestaddrinfo->sockaddr) != AF_INET6) { - best_srtt += bias; - } - curr = ISC_LIST_NEXT(best, publink); - while (curr != NULL) { - addrinfo = ISC_LIST_HEAD(curr->list); - INSIST(addrinfo != NULL); - curr_srtt = addrinfo->srtt; - if (isc_sockaddr_pf(&addrinfo->sockaddr) != AF_INET6) { - curr_srtt += bias; - } - if (curr_srtt < best_srtt) { - best = curr; - best_srtt = curr_srtt; - } - curr = ISC_LIST_NEXT(curr, publink); - } - ISC_LIST_UNLINK(*findlist, best, publink); - ISC_LIST_APPEND(sorted, best, publink); - } - *findlist = sorted; -} - static void findname(fetchctx_t *fctx, const dns_name_t *name, in_port_t port, unsigned int options, unsigned int flags, isc_stdtime_t now, @@ -4052,8 +3979,6 @@ out: * We've found some addresses. We might still be looking * for more addresses. */ - sort_finds(&fctx->finds, res->view->v6bias); - sort_finds(&fctx->altfinds, 0); result = ISC_R_SUCCESS; } @@ -4128,6 +4053,80 @@ possibly_mark(fetchctx_t *fctx, dns_adbaddrinfo_t *addr) { } } +static dns_adbaddrinfo_t * +nextaddress(fetchctx_t *fctx) { + dns_adbaddrinfo_t *prevai = fctx->foundaddrinfo, *lowestsrttai = NULL; + unsigned int v6bias = fctx->res->view->v6bias, lowestsrtt = 0; + + /* + * Let's walk through the list of dns_adbaddrinfo_t to find the best + * next server address to query. This is linear on the number of + * dns_adbaddrinfo_t which are grouped in find list (for each ADB find). + */ + for (dns_adbfind_t *find = ISC_LIST_HEAD(fctx->finds); find != NULL; + find = ISC_LIST_NEXT(find, publink)) + { + for (dns_adbaddrinfo_t *ai = ISC_LIST_HEAD(find->list); + ai != NULL; ai = ISC_LIST_NEXT(ai, publink)) + { + /* + * This address has been marked already, skip it. + */ + if (!UNMARKED(ai)) { + continue; + } + + /* + * This address is the same as the previously used + * address, it's a duplicate, mark it and skip it! + */ + if (prevai != NULL) { + if (prevai->entry == ai->entry) { + ai->flags |= FCTX_ADDRINFO_MARK; + continue; + } + } + + /* + * Mark and skip this address if incompatible (i.e. IPv6 + * address on a v4 only server, or for ACL reason, etc.) + */ + possibly_mark(fctx, ai); + if (!UNMARKED(ai)) { + continue; + } + + /* + * This address hasn't been tried yet and is a + * good candidate. Let's keep track of it if it + * has the lowest SRTT so far (or if there is no + * address with lowest SRTT found yet). + */ + unsigned int aisrtt = ai->srtt; + + if (isc_sockaddr_pf(&ai->sockaddr) != AF_INET6) { + aisrtt += v6bias; + } + + if (lowestsrttai == NULL || aisrtt < lowestsrtt) { + lowestsrttai = ai; + lowestsrtt = aisrtt; + continue; + } + } + } + + /* + * This is the next address to query. If this is NULL, we're done. + */ + if (lowestsrttai != NULL) { + lowestsrttai->flags |= FCTX_ADDRINFO_MARK; + } + fctx->foundaddrinfo = lowestsrttai; + + return lowestsrttai; +} + static dns_adbaddrinfo_t * fctx_nextaddress(fetchctx_t *fctx) { dns_adbfind_t *find, *start; @@ -4150,7 +4149,6 @@ fctx_nextaddress(fetchctx_t *fctx) { possibly_mark(fctx, addrinfo); if (UNMARKED(addrinfo)) { addrinfo->flags |= FCTX_ADDRINFO_MARK; - fctx->find = NULL; fctx->forwarding = true; /* @@ -4171,49 +4169,9 @@ fctx_nextaddress(fetchctx_t *fctx) { fctx->forwarding = false; FCTX_ATTR_SET(fctx, FCTX_ATTR_TRIEDFIND); - find = fctx->find; - if (find == NULL) { - find = ISC_LIST_HEAD(fctx->finds); - } else { - find = ISC_LIST_NEXT(find, publink); - if (find == NULL) { - find = ISC_LIST_HEAD(fctx->finds); - } - } - - /* - * Find the first unmarked addrinfo. - */ - addrinfo = NULL; - if (find != NULL) { - start = find; - do { - for (addrinfo = ISC_LIST_HEAD(find->list); - addrinfo != NULL; - addrinfo = ISC_LIST_NEXT(addrinfo, publink)) - { - if (!UNMARKED(addrinfo)) { - continue; - } - possibly_mark(fctx, addrinfo); - if (UNMARKED(addrinfo)) { - addrinfo->flags |= FCTX_ADDRINFO_MARK; - break; - } - } - if (addrinfo != NULL) { - break; - } - find = ISC_LIST_NEXT(find, publink); - if (find == NULL) { - find = ISC_LIST_HEAD(fctx->finds); - } - } while (find != start); - } - - fctx->find = find; - if (addrinfo != NULL) { - return (addrinfo); + faddrinfo = nextaddress(fctx); + if (faddrinfo != NULL) { + return faddrinfo; } /* @@ -5239,7 +5197,7 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type, ISC_LIST_INIT(fctx->bad_edns); ISC_LIST_INIT(fctx->validators); fctx->validator = NULL; - fctx->find = NULL; + fctx->foundaddrinfo = NULL; fctx->altfind = NULL; fctx->pending = 0; fctx->restarts = 0; From 58c646c4b95efcda759cf194a534290eef38135e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 18 Mar 2026 00:10:35 +0100 Subject: [PATCH 07/14] Fix GSS-API context leak in TKEY negotiation Reject multi-round GSS-API negotiation (GSS_S_CONTINUE_NEEDED) in dst_gssapi_acceptctx(). Each call to gss_accept_sec_context() allocates a context inside the GSS library; without this fix, the context handle was passed back to process_gsstkey() which did not store it persistently, leaking it on every incomplete negotiation. An unauthenticated attacker could exhaust server memory by sending repeated TKEY queries with GSSAPI tokens, each leaking one GSS context. The leaked memory is allocated by the GSS library via malloc(), bypassing BIND's memory accounting. In practice, Kerberos/SPNEGO (the only mechanism used with BIND) completes in a single round, so rejecting continuation does not affect real-world deployments. See RFC 3645 Section 4.1.3. (cherry picked from commit 3883058bf284de2889e4e3676767e58ad91a0ae3) --- lib/dns/gssapictx.c | 109 +++++++++++++++++++---------------- lib/dns/include/dst/gssapi.h | 17 +++--- lib/dns/tkey.c | 29 ++++------ 3 files changed, 76 insertions(+), 79 deletions(-) diff --git a/lib/dns/gssapictx.c b/lib/dns/gssapictx.c index f2a64e8979..c7bc5d1810 100644 --- a/lib/dns/gssapictx.c +++ b/lib/dns/gssapictx.c @@ -608,7 +608,14 @@ dst_gssapi_initctx(const dns_name_t *name, isc_buffer_t *intoken, GSS_SPNEGO_MECHANISM, flags, 0, NULL, gintokenp, NULL, &gouttoken, &ret_flags, NULL); - if (gret != GSS_S_COMPLETE && gret != GSS_S_CONTINUE_NEEDED) { + switch (gret) { + case GSS_S_COMPLETE: + result = ISC_R_SUCCESS; + break; + case GSS_S_CONTINUE_NEEDED: + result = DNS_R_CONTINUE; + break; + default: gss_err_message(mctx, gret, minor, err_message); if (err_message != NULL && *err_message != NULL) { gss_log(3, "Failure initiating security context: %s", @@ -634,12 +641,6 @@ dst_gssapi_initctx(const dns_name_t *name, isc_buffer_t *intoken, RETERR(isc_buffer_copyregion(outtoken, &r)); } - if (gret == GSS_S_COMPLETE) { - result = ISC_R_SUCCESS; - } else { - result = DNS_R_CONTINUE; - } - out: if (gouttoken.length != 0U) { (void)gss_release_buffer(&minor, &gouttoken); @@ -664,15 +665,10 @@ dst_gssapi_acceptctx(dns_gss_cred_id_t cred, const char *gssapi_keytab, char buf[1024]; REQUIRE(outtoken != NULL && *outtoken == NULL); + REQUIRE(*ctxout == NULL); REGION_TO_GBUFFER(*intoken, gintoken); - if (*ctxout == NULL) { - context = GSS_C_NO_CONTEXT; - } else { - context = *ctxout; - } - if (gssapi_keytab != NULL) { #if defined(ISC_PLATFORM_GSSAPI_KRB5_HEADER) || defined(WIN32) gret = gsskrb5_register_acceptor_identity(gssapi_keytab); @@ -717,8 +713,15 @@ dst_gssapi_acceptctx(dns_gss_cred_id_t cred, const char *gssapi_keytab, switch (gret) { case GSS_S_COMPLETE: - case GSS_S_CONTINUE_NEEDED: break; + /* + * RFC 3645 4.1.3: we don't handle GSS_S_CONTINUE_NEEDED + * Multi-round GSS-API negotiation is not supported. + */ + case GSS_S_CONTINUE_NEEDED: + gss_log(3, "multi-round GSS-API negotiation not supported"); + (void)gss_delete_sec_context(&minor, &context, NULL); + FALLTHROUGH; case GSS_S_DEFECTIVE_TOKEN: case GSS_S_DEFECTIVE_CREDENTIAL: case GSS_S_BAD_SIG: @@ -731,7 +734,7 @@ dst_gssapi_acceptctx(dns_gss_cred_id_t cred, const char *gssapi_keytab, case GSS_S_BAD_MECH: case GSS_S_FAILURE: result = DNS_R_INVALIDTKEY; - /* fall through */ + FALLTHROUGH; default: gss_log(3, "failed gss_accept_sec_context: %s", gss_error_tostring(gret, minor, buf, sizeof(buf))); @@ -749,52 +752,56 @@ dst_gssapi_acceptctx(dns_gss_cred_id_t cred, const char *gssapi_keytab, (void)gss_release_buffer(&minor, &gouttoken); } - if (gret == GSS_S_COMPLETE) { - gret = gss_display_name(&minor, gname, &gnamebuf, NULL); - if (gret != GSS_S_COMPLETE) { - gss_log(3, "failed gss_display_name: %s", - gss_error_tostring(gret, minor, buf, - sizeof(buf))); - RETERR(ISC_R_FAILURE); - } + INSIST(gret == GSS_S_COMPLETE); - /* - * Compensate for a bug in Solaris8's implementation - * of gss_display_name(). Should be harmless in any - * case, since principal names really should not - * contain null characters. - */ - if (gnamebuf.length > 0U && - ((char *)gnamebuf.value)[gnamebuf.length - 1] == '\0') - { - gnamebuf.length--; - } + gret = gss_display_name(&minor, gname, &gnamebuf, NULL); + if (gret != GSS_S_COMPLETE) { + gss_log(3, "failed gss_display_name: %s", + gss_error_tostring(gret, minor, buf, sizeof(buf))); + result = ISC_R_FAILURE; + goto out; + } - gss_log(3, "gss-api source name (accept) is %.*s", - (int)gnamebuf.length, (char *)gnamebuf.value); + /* + * Compensate for a bug in Solaris8's implementation + * of gss_display_name(). Should be harmless in any + * case, since principal names really should not + * contain null characters. + */ + if (gnamebuf.length > 0U && + ((char *)gnamebuf.value)[gnamebuf.length - 1] == '\0') + { + gnamebuf.length--; + } - GBUFFER_TO_REGION(gnamebuf, r); - isc_buffer_init(&namebuf, r.base, r.length); - isc_buffer_add(&namebuf, r.length); + gss_log(3, "gss-api source name (accept) is %.*s", (int)gnamebuf.length, + (char *)gnamebuf.value); - RETERR(dns_name_fromtext(principal, &namebuf, dns_rootname, 0, - NULL)); + GBUFFER_TO_REGION(gnamebuf, r); + isc_buffer_init(&namebuf, r.base, r.length); + isc_buffer_add(&namebuf, r.length); - if (gnamebuf.length != 0U) { - gret = gss_release_buffer(&minor, &gnamebuf); - if (gret != GSS_S_COMPLETE) { - gss_log(3, "failed gss_release_buffer: %s", - gss_error_tostring(gret, minor, buf, - sizeof(buf))); - } - } - } else { - result = DNS_R_CONTINUE; + result = dns_name_fromtext(principal, &namebuf, dns_rootname, 0, NULL); + if (result != ISC_R_SUCCESS) { + goto out; } *ctxout = context; out: + if (result != ISC_R_SUCCESS && context != GSS_C_NO_CONTEXT) { + (void)gss_delete_sec_context(&minor, &context, NULL); + } + + if (gnamebuf.length != 0U) { + gret = gss_release_buffer(&minor, &gnamebuf); + if (gret != GSS_S_COMPLETE) { + gss_log(3, "failed gss_release_buffer: %s", + gss_error_tostring(gret, minor, buf, + sizeof(buf))); + } + } + if (gname != NULL) { gret = gss_release_name(&minor, &gname); if (gret != GSS_S_COMPLETE) { diff --git a/lib/dns/include/dst/gssapi.h b/lib/dns/include/dst/gssapi.h index e783f20bd3..c325c4b097 100644 --- a/lib/dns/include/dst/gssapi.h +++ b/lib/dns/include/dst/gssapi.h @@ -115,20 +115,17 @@ dst_gssapi_acceptctx(dns_gss_cred_id_t cred, const char *gssapi_keytab, * generated by gss_accept_sec_context() to be sent to the * initiator * 'context' is a valid pointer to receive the generated context handle. - * On the initial call, it should be a pointer to NULL, which - * will be allocated as a dns_gss_ctx_id_t. Subsequent calls - * should pass in the handle generated on the first call. - * Call dst_gssapi_releasecred to delete the context and free - * the memory. * * Requires: - * 'outtoken' to != NULL && *outtoken == NULL. + * 'outtoken' != NULL && *outtoken == NULL. + * 'context' != NULL && *context == NULL. * * Returns: - * ISC_R_SUCCESS msg was successfully updated to include the - * query to be sent - * DNS_R_CONTINUE transaction still in progress - * other an error occurred while building the message + * ISC_R_SUCCESS msg was successfully updated to include + * the query to be sent + * DNS_R_INVALIDTKEY an error occurred while accepting the + * context + * ISC_R_FAILURE other error occurred */ isc_result_t diff --git a/lib/dns/tkey.c b/lib/dns/tkey.c index d91ed3123b..7f39fbdd31 100644 --- a/lib/dns/tkey.c +++ b/lib/dns/tkey.c @@ -534,19 +534,9 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin, return (ISC_R_SUCCESS); } - /* - * XXXDCL need to check for key expiry per 4.1.1 - * XXXDCL need a way to check fully established, perhaps w/key_flags - */ - intoken.base = tkeyin->key; intoken.length = tkeyin->keylen; - result = dns_tsigkey_find(&tsigkey, name, &tkeyin->algorithm, ring); - if (result == ISC_R_SUCCESS) { - gss_ctx = dst_key_getgssctx(tsigkey->key); - } - principal = dns_fixedname_initname(&fixed); /* @@ -555,7 +545,7 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin, result = dst_gssapi_acceptctx(tctx->gsscred, tctx->gssapi_keytab, &intoken, &outtoken, &gss_ctx, principal, tctx->mctx); - if (result == DNS_R_INVALIDTKEY) { + if (result != ISC_R_SUCCESS) { if (tsigkey != NULL) { dns_tsigkey_detach(&tsigkey); } @@ -564,11 +554,11 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin, */ return (ISC_R_SUCCESS); } - if (result != DNS_R_CONTINUE && result != ISC_R_SUCCESS) { - goto failure; - } + /* - * XXXDCL Section 4.1.3: Limit GSS_S_CONTINUE_NEEDED to 10 times. + * Multi-round GSS-API negotiation (GSS_S_CONTINUE_NEEDED) is + * rejected in dst_gssapi_acceptctx(), so if we reach here the + * negotiation is complete and the principal must be set. */ isc_stdtime_get(&now); @@ -639,6 +629,10 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin, return (ISC_R_SUCCESS); failure: + if (dstkey == NULL && gss_ctx != NULL) { + dst_gssapi_deletectx(tctx->mctx, &gss_ctx); + } + if (tsigkey != NULL) { dns_tsigkey_detach(&tsigkey); } @@ -1576,9 +1570,8 @@ dns_tkey_gssnegotiate(dns_message_t *qmsg, dns_message_t *rmsg, NULL)); /* - * XXXSRA This seems confused. If we got CONTINUE from initctx, - * the GSS negotiation hasn't completed yet, so we can't sign - * anything yet. + * GSS negotiation is complete (CONTINUE returned earlier). + * Create the TSIG key from the established context. */ RETERR(dns_tsigkey_createfromkey( From 7eef47ce676672f65b4988b1b5c086c4f30a9b70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 10 Apr 2026 12:51:31 +0200 Subject: [PATCH 08/14] Fix output token and GSS context leaks in TKEY/GSS-API error paths In dst_gssapi_acceptctx(), rename outtoken to outtokenp (matching BIND convention for output pointer parameters) and free the allocated output token buffer on error in the cleanup path. In process_gsstkey(), route the empty-principal error path through cleanup via CLEANUP() instead of returning early, so that the output token, GSS context, and TSIG key are all freed consistently by the existing cleanup block. (cherry picked from commit f2240d2d06a1a68b622bd6b00a52c6fe4274426d) --- lib/dns/gssapictx.c | 15 +++++++++++---- lib/dns/tkey.c | 25 ++++++++++++------------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/lib/dns/gssapictx.c b/lib/dns/gssapictx.c index c7bc5d1810..880a245e06 100644 --- a/lib/dns/gssapictx.c +++ b/lib/dns/gssapictx.c @@ -651,7 +651,7 @@ out: isc_result_t dst_gssapi_acceptctx(dns_gss_cred_id_t cred, const char *gssapi_keytab, - isc_region_t *intoken, isc_buffer_t **outtoken, + isc_region_t *intoken, isc_buffer_t **outtokenp, dns_gss_ctx_id_t *ctxout, dns_name_t *principal, isc_mem_t *mctx) { isc_region_t r; @@ -664,7 +664,7 @@ dst_gssapi_acceptctx(dns_gss_cred_id_t cred, const char *gssapi_keytab, isc_result_t result; char buf[1024]; - REQUIRE(outtoken != NULL && *outtoken == NULL); + REQUIRE(outtokenp != NULL && *outtokenp == NULL); REQUIRE(*ctxout == NULL); REGION_TO_GBUFFER(*intoken, gintoken); @@ -745,10 +745,13 @@ dst_gssapi_acceptctx(dns_gss_cred_id_t cred, const char *gssapi_keytab, } if (gouttoken.length > 0U) { - isc_buffer_allocate(mctx, outtoken, + isc_buffer_allocate(mctx, outtokenp, (unsigned int)gouttoken.length); GBUFFER_TO_REGION(gouttoken, r); - RETERR(isc_buffer_copyregion(*outtoken, &r)); + result = isc_buffer_copyregion(*outtokenp, &r); + if (result != ISC_R_SUCCESS) { + goto out; + } (void)gss_release_buffer(&minor, &gouttoken); } @@ -789,6 +792,10 @@ dst_gssapi_acceptctx(dns_gss_cred_id_t cred, const char *gssapi_keytab, *ctxout = context; out: + if (result != ISC_R_SUCCESS && *outtokenp != NULL) { + isc_buffer_free(outtokenp); + } + if (result != ISC_R_SUCCESS && context != GSS_C_NO_CONTEXT) { (void)gss_delete_sec_context(&minor, &context, NULL); } diff --git a/lib/dns/tkey.c b/lib/dns/tkey.c index 7f39fbdd31..9eba35078a 100644 --- a/lib/dns/tkey.c +++ b/lib/dns/tkey.c @@ -546,13 +546,10 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin, &intoken, &outtoken, &gss_ctx, principal, tctx->mctx); if (result != ISC_R_SUCCESS) { - if (tsigkey != NULL) { - dns_tsigkey_detach(&tsigkey); - } tkeyout->error = dns_tsigerror_badkey; - tkey_log("process_gsstkey(): dns_tsigerror_badkey"); /* XXXSRA - */ - return (ISC_R_SUCCESS); + tkey_log("process_gsstkey(): dns_tsigerror_badkey"); + result = ISC_R_SUCCESS; + goto failure; } /* @@ -564,9 +561,11 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin, isc_stdtime_get(&now); if (dns_name_countlabels(principal) == 0U) { - if (tsigkey != NULL) { - dns_tsigkey_detach(&tsigkey); - } + tkeyout->error = dns_tsigerror_badkey; + tkey_log("process_gsstkey(): " + "completed context with empty principal"); + result = ISC_R_SUCCESS; + goto failure; } else if (tsigkey == NULL) { #ifdef GSSAPI OM_uint32 gret, minor, lifetime; @@ -645,10 +644,10 @@ failure: isc_buffer_free(&outtoken); } - tkey_log("process_gsstkey(): %s", isc_result_totext(result)); /* XXXSRA - */ - - return (result); + if (result != ISC_R_SUCCESS) { + tkey_log("process_gsstkey(): %s", isc_result_totext(result)); + } + return result; } static isc_result_t From e577560f65dbc6109fca8a597d16568a1cd8987c Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 3 Mar 2026 14:00:38 -0800 Subject: [PATCH 09/14] Disable recursion for non-IN classes Force recursion off, and set allow-recursion/allow-recursion-on ACLs to none, for views with a class other than IN. Log a configuration warning if recursion is explicitly enabled for a non-IN view. This addresses YWH-PGM40640-74 and YWH-PGM40640-75 by preventing any attempt at recursive processing in a class-CHAOS view, ensuring that server addresses used for recursive queries and received in recursive responses are of the expected format. Fixes: isc-projects/bind9#5780 Fixes: isc-projects/bind9#5781 (cherry picked from commit 70532a37a1aec761e8a12444852866ce9d9d5fcc) (cherry picked from commit cf0d5a4e385525e21f2ae39098b1ab90c1137a2a) --- bin/named/server.c | 40 +++++-------------- bin/tests/system/checkconf/tests.sh | 12 ++++++ .../checkconf/warn-chaos-recursion.conf | 25 ++++++++++++ bin/tests/system/resolver/tests.sh | 8 ++-- lib/bind9/check.c | 22 ++++++++-- 5 files changed, 72 insertions(+), 35 deletions(-) create mode 100644 bin/tests/system/checkconf/warn-chaos-recursion.conf diff --git a/bin/named/server.c b/bin/named/server.c index 444bb9be05..fc8b3e8a0c 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -4215,6 +4215,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, obj = NULL; result = named_config_get(maps, "max-cache-size", &obj); INSIST(result == ISC_R_SUCCESS); + /* * If "-T maxcachesize=..." is in effect, it overrides any other * "max-cache-size" setting found in configuration, either implicit or @@ -4965,34 +4966,15 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, } /* - * We have default hints for class IN if we need them. + * We have default root hints for class IN if we need them. + * Each view gets its own rootdb so a priming response only + * writes into that view's copy. Other classes don't support + * recursion and don't need hints. */ if (view->rdclass == dns_rdataclass_in && view->hints == NULL) { dns_view_sethints(view, named_g_server->in_roothints); } - /* - * If we still have no hints, this is a non-IN view with no - * "hints zone" configured. Issue a warning, except if this - * is a root server. Root servers never need to consult - * their hints, so it's no point requiring users to configure - * them. - */ - if (view->hints == NULL) { - dns_zone_t *rootzone = NULL; - (void)dns_view_findzone(view, dns_rootname, &rootzone); - if (rootzone != NULL) { - dns_zone_detach(&rootzone); - need_hints = false; - } - if (need_hints) { - isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, - "no root hints for view '%s'", - view->name); - } - } - /* * Configure the view's TSIG keys. */ @@ -5102,7 +5084,8 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, obj = NULL; result = named_config_get(maps, "recursion", &obj); INSIST(result == ISC_R_SUCCESS); - view->recursion = cfg_obj_asboolean(obj); + view->recursion = (view->rdclass == dns_rdataclass_in && + cfg_obj_asboolean(obj)); obj = NULL; result = named_config_get(maps, "qname-minimization", &obj); @@ -5201,14 +5184,13 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, CHECK(configure_view_acl(vconfig, config, NULL, "allow-query-cache-on", NULL, actx, named_g_mctx, &view->cacheonacl)); - if (strcmp(view->name, "_bind") != 0 && - view->rdclass != dns_rdataclass_chaos) - { - /* named.conf only */ + if (view->rdclass != dns_rdataclass_in) { + dns_acl_none(named_g_mctx, &view->recursionacl); + dns_acl_none(named_g_mctx, &view->recursiononacl); + } else { CHECK(configure_view_acl(vconfig, config, NULL, "allow-recursion", NULL, actx, named_g_mctx, &view->recursionacl)); - /* named.conf only */ CHECK(configure_view_acl(vconfig, config, NULL, "allow-recursion-on", NULL, actx, named_g_mctx, &view->recursiononacl)); diff --git a/bin/tests/system/checkconf/tests.sh b/bin/tests/system/checkconf/tests.sh index c8f883e38d..cc7d37d28e 100644 --- a/bin/tests/system/checkconf/tests.sh +++ b/bin/tests/system/checkconf/tests.sh @@ -481,6 +481,7 @@ $CHECKCONF -l good.conf \ | grep -v "is not implemented" \ | grep -v "is not recommended" \ | grep -v "no longer exists" \ + | grep -v "recursion will be disabled" \ | grep -v "is obsolete" >checkconf.out$n || ret=1 diff good.zonelist checkconf.out$n >diff.out$n || ret=1 if [ $ret -ne 0 ]; then @@ -763,5 +764,16 @@ status=$(expr $status + $ret) rmdir keys +n=$((n + 1)) +echo_i "check 'recursion yes;' is warned and disabled in a non-IN view ($n)" +ret=0 +$CHECKCONF warn-chaos-recursion.conf >checkconf.out$n 2>&1 || ret=1 +grep -F "recursion will be disabled" checkconf.out$n >/dev/null || ret=1 +if [ $ret != 0 ]; then + echo_i "failed" + ret=1 +fi +status=$((status + ret)) + echo_i "exit status: $status" [ $status -eq 0 ] || exit 1 diff --git a/bin/tests/system/checkconf/warn-chaos-recursion.conf b/bin/tests/system/checkconf/warn-chaos-recursion.conf new file mode 100644 index 0000000000..e384533c0a --- /dev/null +++ b/bin/tests/system/checkconf/warn-chaos-recursion.conf @@ -0,0 +1,25 @@ +/* + * 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. + */ + +options { + directory "."; +}; + +view chaos ch { + match-clients { any; }; + recursion yes; + zone "." { + type hint; + file "chaos.hints"; + }; +}; diff --git a/bin/tests/system/resolver/tests.sh b/bin/tests/system/resolver/tests.sh index 761a728eba..09ace25f2a 100755 --- a/bin/tests/system/resolver/tests.sh +++ b/bin/tests/system/resolver/tests.sh @@ -905,10 +905,12 @@ if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) n=$((n + 1)) -echo_i "checking NXDOMAIN is returned when querying non existing domain in CH class ($n)" +echo_i "checking REFUSED is returned when querying non existing domain in CH class ($n)" ret=0 -dig_with_opts @10.53.0.1 id.hostname txt ch >dig.ns1.out.${n} || ret=1 -grep "status: NXDOMAIN" dig.ns1.out.${n} >/dev/null || ret=1 +dig_with_opts @10.53.0.1 hostname.chaostest txt ch >dig.ns1.out.1.${n} || ret=1 +grep "status: NOERROR" dig.ns1.out.1.${n} >/dev/null || ret=1 +dig_with_opts @10.53.0.1 id.hostname txt ch >dig.ns1.out.2.${n} || ret=1 +grep "status: REFUSED" dig.ns1.out.2.${n} >/dev/null || ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) diff --git a/lib/bind9/check.c b/lib/bind9/check.c index db87964ce0..bf623cb5a3 100644 --- a/lib/bind9/check.c +++ b/lib/bind9/check.c @@ -2306,13 +2306,17 @@ check_mirror_zone_notify(const cfg_obj_t *zoptions, const char *znamestr, */ static bool check_recursion(const cfg_obj_t *config, const cfg_obj_t *voptions, - const cfg_obj_t *goptions, isc_log_t *logctx, - cfg_aclconfctx_t *actx, isc_mem_t *mctx) { + dns_rdataclass_t vclass, const cfg_obj_t *goptions, + isc_log_t *logctx, cfg_aclconfctx_t *actx, isc_mem_t *mctx) { dns_acl_t *acl = NULL; const cfg_obj_t *obj; isc_result_t result; bool retval = true; + if (vclass != dns_rdataclass_in) { + return false; + } + /* * Check the "recursion" option first. */ @@ -2892,7 +2896,8 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, * contradicts the purpose of the former. */ if (ztype == CFG_ZONE_MIRROR && - !check_recursion(config, voptions, goptions, logctx, actx, mctx)) + !check_recursion(config, voptions, zclass, goptions, logctx, actx, + mctx)) { cfg_obj_log(zoptions, logctx, ISC_LOG_ERROR, "zone '%s': mirror zones cannot be used if " @@ -4645,6 +4650,17 @@ check_viewconf(const cfg_obj_t *config, const cfg_obj_t *voptions, cfg_aclconfctx_create(mctx, &actx); + if (vclass != dns_rdataclass_in) { + if (check_recursion(config, voptions, dns_rdataclass_in, + options, logctx, actx, mctx)) + { + cfg_obj_log(opts, logctx, ISC_LOG_WARNING, + "recursion will be disabled for " + "non-IN view '%s'", + viewname); + } + } + if (voptions != NULL) { (void)cfg_map_get(voptions, "zone", &zones); } else { From bec30ad70d17e36241df9da259bb2ac85b3c3435 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 4 Mar 2026 13:24:52 -0800 Subject: [PATCH 10/14] Disable UPDATE and NOTIFY for non-IN classes Return NOTIMP for UPDATE and NOTIFY requests received for views with a class other than IN. Only QUERY is now supported for non-IN views such as CHAOS. When running dns dns_rdata_tostruct() with types that are only defined for class IN, ensure that the class is correct before proceeding. Add an assertion that any zone being updated is of class IN. (Note that previously, a DLZ zone could have its class value set incorrectly to NONE; this has been fixed.) This addresses YWH-PGM40640-70 and YWH-PGM40640-73 (as well as any similar problems that might have occurred in the future) by minimizing the code paths that can be reached by rdata classes other than IN, so it is safe for the implementation to assume that rdatatypes that are only defined for class IN, such as SVCB or WKS, have been parsed and validated, and not accepted as unknown/opaque data. Fixes: isc-projects/bind9#5777 Fixes: isc-projects/bind9#5779 (cherry picked from commit 9ae24c32bec16c0f64225ef04f34670018bf0765) (cherry picked from commit a2ca2408b3ff031c426c5dc785f550c9d30bf4aa) --- bin/named/server.c | 2 ++ lib/dns/adb.c | 3 ++- lib/ns/client.c | 8 ++++++++ lib/ns/update.c | 38 +++++++++++++++++--------------------- 4 files changed, 29 insertions(+), 22 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index fc8b3e8a0c..f27ed21572 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -1932,10 +1932,12 @@ dlzconfigure_callback(dns_view_t *view, dns_dlzdb_t *dlzdb, dns_zone_t *zone) { dns_rdataclass_t zclass = view->rdclass; isc_result_t result; + dns_zone_setclass(zone, zclass); result = dns_zonemgr_managezone(named_g_server->zonemgr, zone); if (result != ISC_R_SUCCESS) { return (result); } + dns_zone_setstats(zone, named_g_server->zonestats); return (named_zone_configure_writeable_dlz(dlzdb, zone, zclass, diff --git a/lib/dns/adb.c b/lib/dns/adb.c index 99e83100de..fefec56e50 100644 --- a/lib/dns/adb.c +++ b/lib/dns/adb.c @@ -932,7 +932,8 @@ import_rdataset(dns_adbname_t *adbname, dns_rdataset_t *rdataset, INSIST(DNS_ADB_VALID(adb)); rdtype = rdataset->type; - INSIST((rdtype == dns_rdatatype_a) || (rdtype == dns_rdatatype_aaaa)); + REQUIRE(rdtype == dns_rdatatype_a || rdtype == dns_rdatatype_aaaa); + if (rdtype == dns_rdatatype_a) { findoptions = DNS_ADBFIND_INET; } else { diff --git a/lib/ns/client.c b/lib/ns/client.c index 5f958fdf34..fed6a57176 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -2194,6 +2194,10 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, break; case dns_opcode_update: CTRACE("update"); + if (client->view->rdclass != dns_rdataclass_in) { + ns_client_error(client, DNS_R_NOTIMP); + break; + } #ifdef HAVE_DNSTAP dns_dt_send(client->view, DNS_DTTYPE_UQ, &client->peeraddr, &client->destsockaddr, TCP_CLIENT(client), NULL, @@ -2204,6 +2208,10 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, break; case dns_opcode_notify: CTRACE("notify"); + if (client->view->rdclass != dns_rdataclass_in) { + ns_client_error(client, DNS_R_NOTIMP); + break; + } ns_client_settimeout(client, 60); ns_notify_start(client, handle); break; diff --git a/lib/ns/update.c b/lib/ns/update.c index d9cea2c4c6..30a9206a66 100644 --- a/lib/ns/update.c +++ b/lib/ns/update.c @@ -1294,7 +1294,10 @@ replaces_p(dns_rdata_t *update_rr, dns_rdata_t *db_rr) { return (true); } } - if (db_rr->type == dns_rdatatype_wks) { + + if (db_rr->rdclass == dns_rdataclass_in && + db_rr->type == dns_rdatatype_wks) + { /* * Compare the address and protocol fields only. These * form the first five bytes of the RR data. Do a @@ -1438,8 +1441,7 @@ failure: * 'rdata', and 'ttl', respectively. */ static void -get_current_rr(dns_message_t *msg, dns_section_t section, - dns_rdataclass_t zoneclass, dns_name_t **name, +get_current_rr(dns_message_t *msg, dns_section_t section, dns_name_t **name, dns_rdata_t *rdata, dns_rdatatype_t *covers, dns_ttl_t *ttl, dns_rdataclass_t *update_class) { dns_rdataset_t *rdataset; @@ -1455,7 +1457,7 @@ get_current_rr(dns_message_t *msg, dns_section_t section, dns_rdataset_current(rdataset, rdata); INSIST(dns_rdataset_next(rdataset) == ISC_R_NOMORE); *update_class = rdata->rdclass; - rdata->rdclass = zoneclass; + rdata->rdclass = dns_rdataclass_in; } /*% @@ -1557,7 +1559,6 @@ send_update_event(ns_client_t *client, dns_zone_t *zone) { dns_message_t *request = client->message; dns_aclenv_t *env = ns_interfacemgr_getaclenv(client->manager->interface->mgr); - dns_rdataclass_t zoneclass; dns_rdatatype_t covers; dns_name_t *zonename = NULL; dns_db_t *db = NULL; @@ -1565,10 +1566,12 @@ send_update_event(ns_client_t *client, dns_zone_t *zone) { CHECK(dns_zone_getdb(zone, &db)); zonename = dns_db_origin(db); - zoneclass = dns_db_class(db); dns_zone_getssutable(zone, &ssutable); dns_db_currentversion(db, &ver); + /* Updates are only supported for class IN. */ + INSIST(dns_zone_getclass(zone) == dns_rdataclass_in); + /* * Update message processing can leak record existence information * so check that we are allowed to query this zone. Additionally, @@ -1609,13 +1612,13 @@ send_update_event(ns_client_t *client, dns_zone_t *zone) { dns_ttl_t ttl; dns_rdataclass_t update_class; - get_current_rr(request, DNS_SECTION_UPDATE, zoneclass, &name, + get_current_rr(request, DNS_SECTION_UPDATE, &name, &rdata, &covers, &ttl, &update_class); if (!dns_name_issubdomain(name, zonename)) { FAILC(DNS_R_NOTZONE, "update RR is outside zone"); } - if (update_class == zoneclass) { + if (update_class == dns_rdataclass_in) { /* * Check for meta-RRs. The RFC2136 pseudocode says * check for ANY|AXFR|MAILA|MAILB, but the text adds @@ -2755,9 +2758,7 @@ update_action(isc_task_t *task, isc_event_t *event) { isc_mem_t *mctx = client->mctx; dns_rdatatype_t covers; dns_message_t *request = client->message; - dns_rdataclass_t zoneclass; dns_name_t *zonename = NULL; - dns_ssutable_t *ssutable = NULL; dns_fixedname_t tmpnamefixed; dns_name_t *tmpname = NULL; dns_zoneopt_t options; @@ -2776,10 +2777,9 @@ update_action(isc_task_t *task, isc_event_t *event) { CHECK(dns_zone_getdb(zone, &db)); zonename = dns_db_origin(db); - zoneclass = dns_db_class(db); - dns_zone_getssutable(zone, &ssutable); options = dns_zone_getoptions(zone); + INSIST(dns_zone_getclass(zone) == dns_rdataclass_in); /* * Get old and new versions now that queryacl has been checked. */ @@ -2800,8 +2800,8 @@ update_action(isc_task_t *task, isc_event_t *event) { dns_rdataclass_t update_class; bool flag; - get_current_rr(request, DNS_SECTION_PREREQUISITE, zoneclass, - &name, &rdata, &covers, &ttl, &update_class); + get_current_rr(request, DNS_SECTION_PREREQUISITE, &name, &rdata, + &covers, &ttl, &update_class); if (ttl != 0) { PREREQFAILC(DNS_R_FORMERR, "prerequisite TTL is not " @@ -2868,7 +2868,7 @@ update_action(isc_task_t *task, isc_event_t *event) { "satisfied"); } } - } else if (update_class == zoneclass) { + } else if (update_class == dns_rdataclass_in) { /* "temp += rr;" */ result = temp_append(&temp, name, &rdata); if (result != ISC_R_SUCCESS) { @@ -2927,10 +2927,10 @@ update_action(isc_task_t *task, isc_event_t *event) { dns_rdataclass_t update_class; bool flag; - get_current_rr(request, DNS_SECTION_UPDATE, zoneclass, &name, + get_current_rr(request, DNS_SECTION_UPDATE, &name, &rdata, &covers, &ttl, &update_class); - if (update_class == zoneclass) { + if (update_class == dns_rdataclass_in) { /* * RFC1123 doesn't allow MF and MD in master zones. */ @@ -3516,10 +3516,6 @@ common: dns_db_detach(&db); } - if (ssutable != NULL) { - dns_ssutable_detach(&ssutable); - } - isc_task_detach(&task); uev->result = result; if (zone != NULL) { From 986533b5ae8e855e23430bd7f6af7552d82e0ece Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 4 Mar 2026 10:46:58 +0100 Subject: [PATCH 11/14] Validate DNS message CLASS early in request processing Reject requests with unsupported or misused CLASS values before further processing. Only IN, CH, HS, RESERVED0 (for DNS Cookies), ANY (for TKEY negotiation), and NONE (for DNS UPDATE) are accepted; all other classes return NOTIMP. Misuse of NONE or ANY outside their allowed contexts returns FORMERR. This adds further protection against bugs of the same general class as YWH-PGM40640-70 and YWH-PGM40640-73. (cherry picked from commit d41865a458b9ecd76be4097ac1bea1005cad72db) (cherry picked from commit 1c8016c91c3674929f87cbe7ad09f3670e05ad4e) --- bin/tests/system/unknown/tests.sh | 12 +++---- lib/ns/client.c | 54 +++++++++++++++++++++++++++---- 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/bin/tests/system/unknown/tests.sh b/bin/tests/system/unknown/tests.sh index b14548a039..092ae2d308 100644 --- a/bin/tests/system/unknown/tests.sh +++ b/bin/tests/system/unknown/tests.sh @@ -75,8 +75,8 @@ n=$((n + 1)) echo_i "querying for various representations of a CLASS10 TYPE1 record ($n)" for i in 1 2; do ret=0 - $DIG +short $DIGOPTS @10.53.0.1 a$i.example a class10 >dig.out.$i.test$n || ret=1 - echo '\# 4 0A000001' | $DIFF - dig.out.$i.test$n || ret=1 + $DIG $DIGOPTS @10.53.0.1 a$i.example a class10 >dig.out.$i.test$n || ret=1 + grep -q "NOTIMP" dig.out.$i.test$n || ret=1 if [ $ret != 0 ]; then echo_i "#$i failed" fi @@ -87,8 +87,8 @@ n=$((n + 1)) echo_i "querying for various representations of a CLASS10 TXT record ($n)" for i in 1 2 3 4; do ret=0 - $DIG +short $DIGOPTS @10.53.0.1 txt$i.example txt class10 >dig.out.$i.test$n || ret=1 - echo '"hello"' | $DIFF - dig.out.$i.test$n || ret=1 + $DIG $DIGOPTS @10.53.0.1 txt$i.example txt class10 >dig.out.$i.test$n || ret=1 + grep -q "NOTIMP" dig.out.$i.test$n || ret=1 if [ $ret != 0 ]; then echo_i "#$i failed" fi @@ -99,8 +99,8 @@ n=$((n + 1)) echo_i "querying for various representations of a CLASS10 TYPE123 record ($n)" for i in 1 2; do ret=0 - $DIG +short $DIGOPTS @10.53.0.1 unk$i.example type123 class10 >dig.out.$i.test$n || ret=1 - echo '\# 1 00' | $DIFF - dig.out.$i.test$n || ret=1 + $DIG $DIGOPTS @10.53.0.1 unk$i.example type123 class10 >dig.out.$i.test$n || ret=1 + grep -q "NOTIMP" dig.out.$i.test$n || ret=1 if [ $ret != 0 ]; then echo_i "#$i failed" fi diff --git a/lib/ns/client.c b/lib/ns/client.c index fed6a57176..cbc8a04836 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -43,6 +43,7 @@ #include #include #include +#include #include #include #include @@ -1945,7 +1946,9 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, } } - if (client->message->rdclass == 0) { + char classbuf[DNS_RDATACLASS_FORMATSIZE]; + switch (client->message->rdclass) { + case dns_rdataclass_reserved0: if ((client->attributes & NS_CLIENTATTR_WANTCOOKIE) != 0 && client->message->opcode == dns_opcode_query && client->message->counts[DNS_SECTION_QUESTION] == 0U) @@ -1966,12 +1969,49 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, return; } + ns_client_dumpmessage(client, + "message class could not be determined"); + ns_client_error(client, notimp ? DNS_R_NOTIMP : DNS_R_FORMERR); + isc_task_unpause(client->task); + return; + case dns_rdataclass_in: + break; + case dns_rdataclass_chaos: + break; + case dns_rdataclass_hs: + break; + case dns_rdataclass_none: + if (client->message->opcode != dns_opcode_update) { + ns_client_dumpmessage(client, + "message class NONE can be only " + "used in DNS updates"); + ns_client_error(client, DNS_R_FORMERR); + isc_task_unpause(client->task); + return; + } + break; + case dns_rdataclass_any: + /* + * Required for TKEY negotiation. + */ + if (client->message->tkey == 0) { + ns_client_dumpmessage(client, + "message class ANY can be only " + "used for TKEY negotiation"); + ns_client_error(client, DNS_R_FORMERR); + isc_task_unpause(client->task); + return; + } + break; + default: + dns_rdataclass_format(client->message->rdclass, classbuf, + sizeof(classbuf)); + ns_client_dumpmessage(client, NULL); ns_client_log(client, NS_LOGCATEGORY_CLIENT, NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(1), - "message class could not be determined"); - ns_client_dumpmessage(client, "message class could not be " - "determined"); - ns_client_error(client, notimp ? DNS_R_NOTIMP : DNS_R_FORMERR); + "invalid message class: %s", classbuf); + + ns_client_error(client, DNS_R_NOTIMP); isc_task_unpause(client->task); return; } @@ -2010,7 +2050,7 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, ns_client_log(client, NS_LOGCATEGORY_CLIENT, NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(1), "no matching view in class '%s'", classname); - ns_client_dumpmessage(client, "no matching view in class"); + ns_client_dumpmessage(client, NULL); ns_client_error(client, notimp ? DNS_R_NOTIMP : DNS_R_REFUSED); isc_task_unpause(client->task); return; @@ -2762,7 +2802,7 @@ ns_client_dumpmessage(ns_client_t *client, const char *reason) { int len = 1024; isc_result_t result; - if (!isc_log_wouldlog(ns_lctx, ISC_LOG_DEBUG(1))) { + if (!isc_log_wouldlog(ns_lctx, ISC_LOG_DEBUG(1)) || reason == NULL) { return; } From e7468f6be6dc98f86508627f4f333b6d09d2ac31 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 4 Mar 2026 10:00:56 +1100 Subject: [PATCH 12/14] Reject meta-classes in UPDATE and NOTIFY messages NOTIFY and UPDATE messages must specify a data class in the QUESTION/ZONE section. NONE and ANY are meta-classes and not appropriate here. Return FORMERR if either is used. Rejecting messages with a query class of NONE addresses YWH-PGM40640-72, YWH-PGM40640-82, and YWH-PGM40640-83. Rejecting messages with a query class of ANY addresses YWH-PGM40640-87, YWH-PGM40640-88, and YWH-PGM40640-117. Fixes: isc-projects/bind9#5778 Fixes: isc-projects/bind9#5782 Fixes: isc-projects/bind9#5783 Fixes: isc-projects/bind9#5797 Fixes: isc-projects/bind9#5798 Fixes: isc-projects/bind9#5853 (cherry picked from commit 7de5160517ae69196d1c323b8627b267cdd10761) (cherry picked from commit 3c44de9e6252ec1c7742ef02ecc0d6cbf1cde5e9) --- lib/dns/message.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/dns/message.c b/lib/dns/message.c index 225c9d7576..1fc1152616 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -1087,6 +1087,17 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, rdtype = isc_buffer_getuint16(source); rdclass = isc_buffer_getuint16(source); + /* + * Notify and update messages need to specify the data class. + */ + if ((msg->opcode == dns_opcode_update || + msg->opcode == dns_opcode_notify) && + (rdclass == dns_rdataclass_none || + rdclass == dns_rdataclass_any)) + { + DO_ERROR(DNS_R_FORMERR); + } + /* * If this class is different than the one we already read, * this is an error. From e60ee54f7d650783d3b6dd7fdd749c64c6593353 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 17 Mar 2026 13:24:43 -0700 Subject: [PATCH 13/14] Skip "deny-answer-address" for non-IN addresses Ensure that we don't attempt an ACL match for answer addresses when handling a class-CHAOS zone. This is an additional line of defense for YWH-PGM40640-74. (cherry picked from commit 4cd3d8e6d866143ddc62df821a1007bf3ee7f083) (cherry picked from commit fa60101e910346e64fa2a684b903fbcb84d8243b) --- lib/dns/resolver.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index ee49128825..35a05ddef5 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -7347,9 +7347,16 @@ is_answeraddress_allowed(dns_view_t *view, dns_name_t *name, } /* - * Otherwise, search the filter list for a match for each address - * record. If a match is found, the address should be filtered, - * so should the entire answer. + * deny-answer-address doesn't apply to non-IN classes. + */ + if (rdataset->rdclass != dns_rdataclass_in) { + return true; + } + + /* + * Otherwise, search the filter list for a match for each + * address record. If a match is found, the address should be + * filtered, so should the entire answer. */ for (result = dns_rdataset_first(rdataset); result == ISC_R_SUCCESS; result = dns_rdataset_next(rdataset)) From cd3a72e414d35c2ae573f4faea1148a95db7d5ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 6 May 2026 10:12:35 +0200 Subject: [PATCH 14/14] Pass empty string instead of NULL to ns_client_dumpmessage() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two new call sites added by the CLASS-validation work passed NULL as the reason, but ns_client_dumpmessage() bails out early on a NULL reason — so the message dump never happened. The intent was to dump the message and let the follow-up ns_client_log() carry the reason text, so pass "" to suppress the prefix without short-circuiting the dump. (cherry picked from commit 8af39c360407a92ef31bf233b9df760d1bb9fb5f) --- lib/ns/client.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ns/client.c b/lib/ns/client.c index cbc8a04836..c65ebbbdd8 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -2006,7 +2006,7 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, default: dns_rdataclass_format(client->message->rdclass, classbuf, sizeof(classbuf)); - ns_client_dumpmessage(client, NULL); + ns_client_dumpmessage(client, ""); ns_client_log(client, NS_LOGCATEGORY_CLIENT, NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(1), "invalid message class: %s", classbuf); @@ -2050,7 +2050,7 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, ns_client_log(client, NS_LOGCATEGORY_CLIENT, NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(1), "no matching view in class '%s'", classname); - ns_client_dumpmessage(client, NULL); + ns_client_dumpmessage(client, ""); ns_client_error(client, notimp ? DNS_R_NOTIMP : DNS_R_REFUSED); isc_task_unpause(client->task); return; @@ -2343,7 +2343,7 @@ ns__client_setup(ns_client_t *client, ns_clientmgr_t *mgr, bool new) { * The caller is responsible for that. */ - REQUIRE(NS_CLIENT_VALID(client) || (new &&client != NULL)); + REQUIRE(NS_CLIENT_VALID(client) || (new && client != NULL)); REQUIRE(VALID_MANAGER(mgr) || !new); if (new) {