From: Ondřej Surý Date: Thu, 27 Nov 2025 13:07:35 +0000 (+0100) Subject: Change the QNAME minimization algorithm to follow the standard X-Git-Tag: v9.21.16~21^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ed460c50b7868e1f797eb609908bb97dd56149fa;p=thirdparty%2Fbind9.git Change the QNAME minimization algorithm to follow the standard In !9155, the QNAME minimization was changed to not leak the query type to the parent name server. This violates RFC 9156 Section 3, step (3) and it is not necessary. It also breaks some (weird) authoritative DNS setups, especially when CNAMEs are involved. Also there is really no privacy leak with query type. --- diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 22cb873438d..9a0c47b7b1e 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -2032,6 +2032,7 @@ respdiff:recent-named: CFLAGS: "${CFLAGS_COMMON} -DISC_TRACK_PTHREADS_OBJECTS" EXTRA_CONFIGURE: "-Doptimization=g" MAX_DISAGREEMENTS_PERCENTAGE: "0.1" + allow_failure: true # GL!11293 # Performance tests diff --git a/bin/tests/system/dnssec/tests_validation.py b/bin/tests/system/dnssec/tests_validation.py index ad298b39e86..b8f6baa4f8d 100644 --- a/bin/tests/system/dnssec/tests_validation.py +++ b/bin/tests/system/dnssec/tests_validation.py @@ -737,6 +737,9 @@ def test_cache(ns4): for rrset in res2.answer: assert rrset.ttl <= 300 + # first query for a NS record, to cache NSEC and RRSIG(NSEC) + msg = isctest.query.create("normalthenrrsig.secure.example", "NS") + isctest.query.tcp(msg, "10.53.0.4") # query for a record, then follow it with a query for the # corresponding RRSIG, check that it's answered from the cache msg = isctest.query.create("normalthenrrsig.secure.example", "A") diff --git a/bin/tests/system/mirror/tests.sh b/bin/tests/system/mirror/tests.sh index c7f7658fd73..58a634f8206 100644 --- a/bin/tests/system/mirror/tests.sh +++ b/bin/tests/system/mirror/tests.sh @@ -386,7 +386,7 @@ $DIG $DIGOPTS @10.53.0.3 foo.initially-unavailable. A >dig.out.ns3.test$n.1 2>&1 grep "NOERROR" dig.out.ns3.test$n.1 >/dev/null || ret=1 grep "flags:.* ad" dig.out.ns3.test$n.1 >/dev/null || ret=1 # Sanity check: the authoritative server should have been queried. -nextpart ns2/named.run | grep "query 'foo.initially-unavailable/NS/IN'" >/dev/null || ret=1 +nextpart ns2/named.run | grep "query 'foo.initially-unavailable/A/IN'" >/dev/null || ret=1 # Reconfigure ns2 so that the zone can be mirrored on ns3. sed '/^zone "initially-unavailable" {$/,/^};$/ { s/10.53.0.254/10.53.0.3/ @@ -404,7 +404,7 @@ $DIG $DIGOPTS @10.53.0.3 foo.initially-unavailable. A >dig.out.ns3.test$n.2 2>&1 grep "NOERROR" dig.out.ns3.test$n.2 >/dev/null || ret=1 grep "flags:.* ad" dig.out.ns3.test$n.2 >/dev/null || ret=1 # Ensure the authoritative server was not queried. -nextpart ns2/named.run | grep "query 'foo.initially-unavailable/NS/IN'" >/dev/null && ret=1 +nextpart ns2/named.run | grep "query 'foo.initially-unavailable/A/IN'" >/dev/null && ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) @@ -435,7 +435,7 @@ $DIG $DIGOPTS @10.53.0.3 foo.initially-unavailable. A >dig.out.ns3.test$n 2>&1 | grep "NOERROR" dig.out.ns3.test$n >/dev/null || ret=1 grep "flags:.* ad" dig.out.ns3.test$n >/dev/null || ret=1 # Sanity check: the authoritative server should have been queried. -nextpart ns2/named.run | grep "query 'foo.initially-unavailable/NS/IN'" >/dev/null || ret=1 +nextpart ns2/named.run | grep "query 'foo.initially-unavailable/A/IN'" >/dev/null || ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) diff --git a/bin/tests/system/qmin/tests.sh b/bin/tests/system/qmin/tests.sh index 7973a607118..6cf8d2b9d07 100755 --- a/bin/tests/system/qmin/tests.sh +++ b/bin/tests/system/qmin/tests.sh @@ -127,14 +127,12 @@ ADDR a.bit.longer.ns.name.good. ADDR ns2.good. ADDR ns3.good. ADDR ns3.good. -NS a.bit.longer.ns.name.good. NS bit.longer.ns.name.good. NS boing.good. NS good. NS longer.ns.name.good. NS name.good. NS ns.name.good. -NS ns3.good. NS zoop.boing.good. __EOF cat <<__EOF | diff ans3/query.log - >/dev/null || ret=1 @@ -167,13 +165,11 @@ ADDR a.bit.longer.ns.name.good. ADDR ns2.good. ADDR ns3.good. ADDR ns3.good. -NS a.bit.longer.ns.name.good. NS bit.longer.ns.name.good. NS boing.good. NS longer.ns.name.good. NS name.good. NS ns.name.good. -NS ns3.good. NS zoop.boing.good. __EOF cat <<__EOF | diff ans3/query.log - >/dev/null || ret=1 @@ -225,7 +221,6 @@ ADDR ns3.bad. ADDR ns3.bad. NS boing.bad. NS name.bad. -NS ns3.bad. __EOF cat <<__EOF | diff ans3/query.log - >/dev/null || ret=1 ADDR icky.icky.icky.ptang.zoop.boing.bad. @@ -276,7 +271,6 @@ ADDR ns3.ugly. NS boing.ugly. NS name.ugly. NS name.ugly. -NS ns3.ugly. __EOF echo "ADDR icky.icky.icky.ptang.zoop.boing.ugly." | diff ans3/query.log - >/dev/null || ret=1 echo "ADDR icky.icky.icky.ptang.zoop.boing.ugly." | diff ans4/query.log - >/dev/null || ret=1 @@ -308,13 +302,11 @@ ADDR a.bit.longer.ns.name.slow. ADDR ns2.slow. ADDR ns3.slow. ADDR ns3.slow. -NS a.bit.longer.ns.name.slow. NS bit.longer.ns.name.slow. NS boing.slow. NS longer.ns.name.slow. NS name.slow. NS ns.name.slow. -NS ns3.slow. NS slow. NS zoop.boing.slow. __EOF @@ -348,7 +340,6 @@ NS 8.f.4.0.1.0.0.2.ip6.arpa. NS 0.0.0.0.8.f.4.0.1.0.0.2.ip6.arpa. NS 0.0.0.0.0.0.8.f.4.0.1.0.0.2.ip6.arpa. NS 0.0.0.0.0.0.0.0.8.f.4.0.1.0.0.2.ip6.arpa. -NS 1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.f.4.0.1.0.0.2.ip6.arpa. PTR 1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.f.4.0.1.0.0.2.ip6.arpa. __EOF for ans in ans2 ans3 ans4; do mv -f $ans/query.log query-$ans-$n.log 2>/dev/null || true; done @@ -371,14 +362,12 @@ ADDR a.bit.longer.ns.name.good. ADDR ns2.good. ADDR ns3.good. ADDR ns3.good. -NS a.bit.longer.ns.name.good. NS bit.longer.ns.name.good. NS boing.good. NS good. NS longer.ns.name.good. NS name.good. NS ns.name.good. -NS ns3.good. NS zoop.boing.good. __EOF cat <<__EOF | diff ans3/query.log - >/dev/null || ret=1 @@ -460,7 +449,6 @@ grep "a\.b\.stale\..*1.*IN.*TXT.*hooray" dig.out.test$n >/dev/null || ret=1 sleep 1 sort ans2/query.log >ans2/query.log.sorted cat <<__EOF | diff ans2/query.log.sorted - >/dev/null || ret=1 -ADDR ns.a.b.stale. ADDR ns.b.stale. ADDR ns2.stale. NS b.stale. @@ -469,9 +457,7 @@ __EOF test -f ans3/query.log && ret=1 sort ans4/query.log >ans4/query.log.sorted cat <<__EOF | diff ans4/query.log.sorted - >/dev/null || ret=1 -ADDR ns.a.b.stale. ADDR ns.b.stale. -NS a.b.stale. NS b.stale. TXT a.b.stale. __EOF @@ -490,7 +476,6 @@ grep "a\.b\.stale\..*1.*IN.*TXT.*hooray" dig.out.test$n >/dev/null || ret=1 sleep 1 sort ans2/query.log >ans2/query.log.sorted cat <<__EOF | diff ans2/query.log.sorted - >/dev/null || ret=1 -ADDR ns.a.b.stale. ADDR ns.b.stale. ADDR ns2.stale. NS b.stale. @@ -498,9 +483,7 @@ __EOF test -f ans3/query.log && ret=1 sort ans4/query.log >ans4/query.log.sorted cat <<__EOF | diff ans4/query.log.sorted - >/dev/null || ret=1 -ADDR ns.a.b.stale. ADDR ns.b.stale. -NS a.b.stale. TXT a.b.stale. __EOF for ans in ans2 ans3 ans4; do mv -f $ans/query.log query-$ans-$n.log 2>/dev/null || true; done @@ -536,7 +519,6 @@ grep "a\.b\.stale\..*1.*IN.*TXT.*hooray" dig.out.test$n >/dev/null || ret=1 sleep 1 sort ans2/query.log >ans2/query.log.sorted cat <<__EOF | diff ans2/query.log.sorted - >/dev/null || ret=1 -ADDR ns.a.b.stale. ADDR ns.b.stale. ADDR ns2.stale. NS b.stale. @@ -545,9 +527,7 @@ __EOF test -f ans3/query.log && ret=1 sort ans4/query.log >ans4/query.log.sorted cat <<__EOF | diff ans4/query.log.sorted - >/dev/null || ret=1 -ADDR ns.a.b.stale. ADDR ns.b.stale. -NS a.b.stale. NS b.stale. TXT a.b.stale. __EOF @@ -566,7 +546,6 @@ grep "a\.b\.stale\..*1.*IN.*TXT.*hooray" dig.out.test$n >/dev/null || ret=1 sleep 1 sort ans2/query.log >ans2/query.log.sorted cat <<__EOF | diff ans2/query.log.sorted - >/dev/null || ret=1 -ADDR ns.a.b.stale. ADDR ns.b.stale. ADDR ns2.stale. NS b.stale. @@ -574,9 +553,7 @@ __EOF test -f ans3/query.log && ret=1 sort ans4/query.log >ans4/query.log.sorted cat <<__EOF | diff ans4/query.log.sorted - >/dev/null || ret=1 -ADDR ns.a.b.stale. ADDR ns.b.stale. -NS a.b.stale. TXT a.b.stale. __EOF for ans in ans2 ans3 ans4; do mv -f $ans/query.log query-$ans-$n.log 2>/dev/null || true; done diff --git a/bin/tests/system/resolver/tests.sh b/bin/tests/system/resolver/tests.sh index 710fac4445c..58fe0891b8b 100755 --- a/bin/tests/system/resolver/tests.sh +++ b/bin/tests/system/resolver/tests.sh @@ -749,10 +749,10 @@ if ${FEATURETEST} --enable-querytrace; then grep "status: SERVFAIL" dig.ns5.out.${n} >/dev/null || ret=1 check_namedrun() { nextpartpeek ns5/named.run >nextpart.out.${n} - grep 'resolving tcpalso.no-questions/NS for [^:]*: empty question section, accepting it anyway as TC=1' nextpart.out.${n} >/dev/null || return 1 - grep '(tcpalso.no-questions/NS): connecting via TCP' nextpart.out.${n} >/dev/null || return 1 - grep 'resolving tcpalso.no-questions/NS for [^:]*: empty question section$' nextpart.out.${n} >/dev/null || return 1 - grep '(tcpalso.no-questions/NS): nextitem' nextpart.out.${n} >/dev/null || return 1 + grep 'resolving tcpalso.no-questions/A for [^:]*: empty question section, accepting it anyway as TC=1' nextpart.out.${n} >/dev/null || return 1 + grep '(tcpalso.no-questions/A): connecting via TCP' nextpart.out.${n} >/dev/null || return 1 + grep 'resolving tcpalso.no-questions/A for [^:]*: empty question section$' nextpart.out.${n} >/dev/null || return 1 + grep '(tcpalso.no-questions/A): nextitem' nextpart.out.${n} >/dev/null || return 1 return 0 } retry_quiet 12 check_namedrun || ret=1 diff --git a/bin/tests/system/rpzextra/tests_rpzextra.py b/bin/tests/system/rpzextra/tests_rpzextra.py index 8dfa8cddf16..aa910f9f7cd 100644 --- a/bin/tests/system/rpzextra/tests_rpzextra.py +++ b/bin/tests/system/rpzextra/tests_rpzextra.py @@ -114,8 +114,8 @@ def test_rpz_passthru_logging(): expected_rcode=dns.rcode.NOERROR, ) assert res_allowed_any.answer == [ - dns.rrset.from_text("allowed.", 300, "IN", "A", "10.53.0.2"), dns.rrset.from_text("allowed.", 300, "IN", "NS", "ns1.allowed."), + dns.rrset.from_text("allowed.", 300, "IN", "A", "10.53.0.2"), ] # The comparison above doesn't compare the TTL values, and we want to # make sure that the "passthru" rpz doesn't cap the TTL with max-policy-ttl. diff --git a/bin/tests/system/synthfromdnssec/tests.sh b/bin/tests/system/synthfromdnssec/tests.sh index 7a612271de6..ce4f4099335 100644 --- a/bin/tests/system/synthfromdnssec/tests.sh +++ b/bin/tests/system/synthfromdnssec/tests.sh @@ -443,10 +443,10 @@ for ns in 2 4 5 6; do check_status NOERROR dig.out.ns${ns}.test$n || ret=1 if [ ${synth} = yes ]; then check_synth_cname b.wild-cname.example. dig.out.ns${ns}.test$n || ret=1 - nextpart ns1/named.run | grep b.wild-cname.example/NS >/dev/null && ret=1 + nextpart ns1/named.run | grep b.wild-cname.example/A >/dev/null && ret=1 else check_nosynth_cname b.wild-cname.example. dig.out.ns${ns}.test$n || ret=1 - nextpart ns1/named.run | grep b.wild-cname.example/NS >/dev/null || ret=1 + nextpart ns1/named.run | grep b.wild-cname.example/A >/dev/null || ret=1 fi grep "ns1.example.*.IN.A" dig.out.ns${ns}.test$n >/dev/null || ret=1 digcomp wildcname.out dig.out.ns${ns}.test$n || ret=1 @@ -561,7 +561,7 @@ for ns in 2 4 5 6; do check_ad_flag no dig.out.ns${ns}.test$n || ret=1 check_status NOERROR dig.out.ns${ns}.test$n || ret=1 check_nosynth_cname b.wild-cname.insecure.example dig.out.ns${ns}.test$n || ret=1 - nextpart ns1/named.run | grep b.wild-cname.insecure.example/NS >/dev/null || ret=1 + nextpart ns1/named.run | grep b.wild-cname.insecure.example/A >/dev/null || ret=1 grep "ns1.insecure.example.*.IN.A" dig.out.ns${ns}.test$n >/dev/null || ret=1 digcomp insecure.wildcname.out dig.out.ns${ns}.test$n || ret=1 n=$((n + 1)) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index b3a93f5e2a6..e4ae28bbfa1 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -10072,7 +10072,7 @@ fctx_minimize_qname(fetchctx_t *fctx) { } else if (fctx->qmin_labels < 35) { fctx->qmin_labels = 35; } else { - fctx->qmin_labels = nlabels + 1; + fctx->qmin_labels = nlabels; } } else if (fctx->qmin_labels > DNS_QMIN_MAXLABELS) { fctx->qmin_labels = DNS_NAME_MAXLABELS; @@ -10112,18 +10112,10 @@ fctx_minimize_qname(fetchctx_t *fctx) { break; } break; - } while (fctx->qmin_labels <= nlabels); + } while (fctx->qmin_labels < nlabels); } - /* - * DS lookups come from the parent zone so we don't need to do a - * NS lookup at the QNAME. If the QTYPE is NS we are not leaking - * the type if we just do the final NS lookup. - */ - if (fctx->qmin_labels < nlabels || - (fctx->type != dns_rdatatype_ns && fctx->type != dns_rdatatype_ds && - fctx->qmin_labels == nlabels)) - { + if (fctx->qmin_labels < nlabels) { dns_name_copy(&name, fctx->qminname); fctx->qmintype = dns_rdatatype_ns; fctx->minimized = true;