From 2789554dec0e309b1f725f3987a3ec5163bce5cb Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 20 Jun 2024 14:02:24 +1000 Subject: [PATCH 1/3] Remove false positive qname minimisation error Don't report qname minimisation NXDOMAIN errors when the result is NXDOMAIN. (cherry picked from commit f78beca942ce76dde023ef1ec641924b5fe567e8) --- lib/dns/resolver.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 0c8c85ffeb..26eeeafbb9 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -391,6 +391,7 @@ struct fetchctx { bool minimized; unsigned int qmin_labels; isc_result_t qmin_warning; + bool force_qmin_warning; bool ip6arpaskip; bool forwarding; dns_fixedname_t qminfname; @@ -4180,6 +4181,16 @@ resume_qmin(void *arg) { case DNS_R_NCACHENXRRSET: case DNS_R_CNAME: case DNS_R_DNAME: + /* + * We have previously detected a possible error of an + * incorrect NXDOMAIN and now have a response that + * indicates that it was an actual error. + */ + if (fctx->qmin_warning == DNS_R_NCACHENXDOMAIN || + fctx->qmin_warning == DNS_R_NXDOMAIN) + { + fctx->force_qmin_warning = true; + } /* * Any other result will *not* cause a failure in strict * mode, or cause minimization to be disabled in relaxed @@ -5289,6 +5300,19 @@ validated(void *arg) { covers = fctx->type; } + /* + * Don't report qname minimisation NXDOMAIN errors + * when the result is NXDOMAIN except we have already + * confirmed a higher error. + */ + if (!fctx->force_qmin_warning && + message->rcode == dns_rcode_nxdomain && + (fctx->qmin_warning == DNS_R_NXDOMAIN || + fctx->qmin_warning == DNS_R_NCACHENXDOMAIN)) + { + fctx->qmin_warning = ISC_R_SUCCESS; + } + result = dns_db_findnode(fctx->cache, val->name, true, &node); if (result != ISC_R_SUCCESS) { /* fctx->lock unlocked in noanswer_response */ @@ -6430,6 +6454,18 @@ ncache_message(fetchctx_t *fctx, dns_message_t *message, goto unlock; } + /* + * Don't report qname minimisation NXDOMAIN errors + * when the result is NXDOMAIN except we have already + * confirmed a higher error. + */ + if (!fctx->force_qmin_warning && message->rcode == dns_rcode_nxdomain && + (fctx->qmin_warning == DNS_R_NXDOMAIN || + fctx->qmin_warning == DNS_R_NCACHENXDOMAIN)) + { + fctx->qmin_warning = ISC_R_SUCCESS; + } + /* * If we are asking for a SOA record set the cache time * to zero to facilitate locating the containing zone of From eea196fc78d4803eb37a41bc2f7d453e98c9e7d8 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 25 Jun 2024 16:42:25 +1000 Subject: [PATCH 2/3] Cleanup old clang-format string splitting (cherry picked from commit 6d1c7beb154eaa6390921a64d280f7f76512be18) --- lib/dns/resolver.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 26eeeafbb9..29c4b9b3af 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -1674,9 +1674,8 @@ fctx__done(fetchctx_t *fctx, isc_result_t result, const char *func, if (fctx->qmin_warning != ISC_R_SUCCESS) { isc_log_write(dns_lctx, DNS_LOGCATEGORY_LAME_SERVERS, DNS_LOGMODULE_RESOLVER, ISC_LOG_INFO, - "success resolving '%s' " - "after disabling qname minimization due " - "to '%s'", + "success resolving '%s' after disabling " + "qname minimization due to '%s'", fctx->info, isc_result_totext(fctx->qmin_warning)); } From eb7d7845444c1f7bfa71a8790c3bc0bc4fb95254 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 25 Jun 2024 14:00:51 +1000 Subject: [PATCH 3/3] Test that false positive "success resolving" is not logged (cherry picked from commit 111e28521497b105c3835ee57f92c7971f93a9eb) --- bin/tests/system/qmin/clean.sh | 5 +++-- bin/tests/system/qmin/ns1/root.db | 3 +++ bin/tests/system/qmin/ns5/in-addr.arpa.db | 21 +++++++++++++++++++++ bin/tests/system/qmin/ns5/named.conf.in | 6 ++++++ bin/tests/system/qmin/ns7/named.conf.in | 1 + bin/tests/system/qmin/tests.sh | 11 +++++++++++ 6 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 bin/tests/system/qmin/ns5/in-addr.arpa.db diff --git a/bin/tests/system/qmin/clean.sh b/bin/tests/system/qmin/clean.sh index f5b2546fcf..5d7d603723 100644 --- a/bin/tests/system/qmin/clean.sh +++ b/bin/tests/system/qmin/clean.sh @@ -11,9 +11,10 @@ # See the COPYRIGHT file distributed with this work for additional # information regarding copyright ownership. -rm -f ns*/named.conf rm -f */named.memstats rm -f */named.run */named.run.prev -rm -f dig.out.* rm -f ans*/query.log* +rm -f dig.out.* +rm -f named.run.* +rm -f ns*/named.conf rm -f query*.log diff --git a/bin/tests/system/qmin/ns1/root.db b/bin/tests/system/qmin/ns1/root.db index 325f607ee6..3854fc83c3 100644 --- a/bin/tests/system/qmin/ns1/root.db +++ b/bin/tests/system/qmin/ns1/root.db @@ -39,3 +39,6 @@ ns2.fwd. A 10.53.0.2 $TTL 2 stale. NS ns2.stale. ns2.stale. A 10.53.0.2 + +in-addr.arpa. NS ns5.in-addr.arpa. +ns5.in-addr.arpa. A 10.53.0.5 diff --git a/bin/tests/system/qmin/ns5/in-addr.arpa.db b/bin/tests/system/qmin/ns5/in-addr.arpa.db new file mode 100644 index 0000000000..1866d069b3 --- /dev/null +++ b/bin/tests/system/qmin/ns5/in-addr.arpa.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 20 +@ IN SOA wpk.isc.org. a.root.servers.nil. ( + 2000042100 ; serial + 600 ; refresh + 600 ; retry + 1200 ; expire + 2 ; minimum + ) +@ NS ns5 +ns5 A 10.53.0.5 diff --git a/bin/tests/system/qmin/ns5/named.conf.in b/bin/tests/system/qmin/ns5/named.conf.in index fac3538387..fb3101279a 100644 --- a/bin/tests/system/qmin/ns5/named.conf.in +++ b/bin/tests/system/qmin/ns5/named.conf.in @@ -26,6 +26,7 @@ options { querylog yes; resolver-query-timeout 30000; # 30 seconds dnssec-validation no; + disable-empty-zone 10.in-addr.arpa; }; key rndc_key { @@ -41,3 +42,8 @@ zone "." { type hint; file "../../_common/root.hint"; }; + +zone "in-addr.arpa" { + type primary; + file "in-addr.arpa.db"; +}; diff --git a/bin/tests/system/qmin/ns7/named.conf.in b/bin/tests/system/qmin/ns7/named.conf.in index 917e3e768c..b6bf4c6c6d 100644 --- a/bin/tests/system/qmin/ns7/named.conf.in +++ b/bin/tests/system/qmin/ns7/named.conf.in @@ -26,6 +26,7 @@ options { querylog yes; resolver-query-timeout 30000; # 30 seconds dnssec-validation no; + disable-empty-zone 10.in-addr.arpa; }; key rndc_key { diff --git a/bin/tests/system/qmin/tests.sh b/bin/tests/system/qmin/tests.sh index ff034738e0..d104161f40 100755 --- a/bin/tests/system/qmin/tests.sh +++ b/bin/tests/system/qmin/tests.sh @@ -552,5 +552,16 @@ for ans in ans2 ans3 ans4; do mv -f $ans/query.log query-$ans-$n.log 2>/dev/null if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) +n=$((n + 1)) +echo_i "test that \"success resolving\" is not logged for NXDOMAIN final answer when qname-minimization is in relaxed mode ($n)" +ret=0 +nextpart ns7/named.run >/dev/null +$DIG $DIGOPTS 1.0.53.10.in-addr.arpa ptr @10.53.0.7 >dig.out.test$n || ret=1 +nextpart ns7/named.run >named.run.test$n +grep "status: NXDOMAIN" dig.out.test$n >/dev/null || ret=1 +grep "success resolving" named.run.test$n >/dev/null && ret=1 +if [ $ret != 0 ]; then echo_i "failed"; fi +status=$((status + ret)) + echo_i "exit status: $status" [ $status -eq 0 ] || exit 1