From ed460c50b7868e1f797eb609908bb97dd56149fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 27 Nov 2025 14:07:35 +0100 Subject: [PATCH] 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. --- .gitlab-ci.yml | 1 + bin/tests/system/dnssec/tests_validation.py | 3 +++ bin/tests/system/mirror/tests.sh | 6 +++--- bin/tests/system/qmin/tests.sh | 23 --------------------- bin/tests/system/resolver/tests.sh | 8 +++---- bin/tests/system/rpzextra/tests_rpzextra.py | 2 +- bin/tests/system/synthfromdnssec/tests.sh | 6 +++--- lib/dns/resolver.c | 14 +++---------- 8 files changed, 18 insertions(+), 45 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 22cb873438..9a0c47b7b1 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 ad298b39e8..b8f6baa4f8 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 c7f7658fd7..58a634f820 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 7973a60711..6cf8d2b9d0 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 710fac4445..58fe0891b8 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 8dfa8cddf1..aa910f9f7c 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 7a612271de..ce4f409933 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 b3a93f5e2a..e4ae28bbfa 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;