From 791d26b99eefc2f8e4c344d4945bf3b8e0963458 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 22 Sep 2022 14:03:17 +0200 Subject: [PATCH 1/5] Clean up the "glue" system test Bring the "glue" system test up to speed with other system tests: add check numbering, ensure test artifacts are preserved upon failure, improve error reporting, make the test fail upon unexpected errors, address ShellCheck warnings. --- bin/tests/system/glue/clean.sh | 4 ++-- bin/tests/system/glue/tests.sh | 29 +++++++++++++++++++---------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/bin/tests/system/glue/clean.sh b/bin/tests/system/glue/clean.sh index 4d84f068f8..d6ec3455d6 100644 --- a/bin/tests/system/glue/clean.sh +++ b/bin/tests/system/glue/clean.sh @@ -15,9 +15,9 @@ # Clean up after glue tests. # -rm -f dig.out rm -f */named.conf rm -f */named.memstats rm -f */named.run -rm -f ns*/named.lock +rm -f dig.out rm -f ns*/managed-keys.bind* +rm -f ns*/named.lock diff --git a/bin/tests/system/glue/tests.sh b/bin/tests/system/glue/tests.sh index 9ecf39b6a8..e761839a77 100644 --- a/bin/tests/system/glue/tests.sh +++ b/bin/tests/system/glue/tests.sh @@ -13,21 +13,30 @@ . ../conf.sh -# -# Do glue tests. -# +set -e -DIGOPTS="+norec -p ${PORT}" +dig_with_opts() { + "$DIG" +norec -p "${PORT}" "$@" +} status=0 +n=0 -echo_i "testing that a ccTLD referral gets a full glue set from the root zone" -$DIG $DIGOPTS @10.53.0.1 foo.bar.fi. A >dig.out || status=1 -digcomp --lc fi.good dig.out || status=1 +n=$((n+1)) +echo_i "testing that a ccTLD referral gets a full glue set from the root zone ($n)" +ret=0 +dig_with_opts @10.53.0.1 foo.bar.fi. A > dig.out.$n || ret=1 +digcomp --lc fi.good dig.out.$n || ret=1 +if [ "$ret" -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) -echo_i "testing that we don't find out-of-zone glue" -$DIG $DIGOPTS @10.53.0.1 example.net. a > dig.out || status=1 -digcomp noglue.good dig.out || status=1 +n=$((n+1)) +echo_i "testing that we don't find out-of-zone glue ($n)" +ret=0 +dig_with_opts @10.53.0.1 example.net. A > dig.out.$n || ret=1 +digcomp noglue.good dig.out.$n || ret=1 +if [ "$ret" -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) echo_i "exit status: $status" [ $status -eq 0 ] || exit 1 From 18143493744234eee02e5334bfd717d7d62ff1a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 22 Sep 2022 14:03:17 +0200 Subject: [PATCH 2/5] Add tests for broken glueless referrals If an NS RRset at the parent side of a delegation point only contains in-bailiwick NS records, at least one glue record should be included in every referral response sent for such a delegation point or else clients will need to send follow-up queries in order to determine name server addresses. In certain edge cases (when the total size of a referral response without glue records was just below to the UDP packet size limit), named failed to adhere to that rule by sending non-truncated, glueless referral responses. Add tests attempting to trigger that bug in several different scenarios, covering all possible combinations of the following factors: - type of zone (signed, unsigned), - glue record type (A, AAAA, both). --- bin/tests/system/glue/clean.sh | 4 + bin/tests/system/glue/ns1/named.conf.in | 11 ++ bin/tests/system/glue/ns1/sign.sh | 27 +++++ .../system/glue/ns1/tc-test-signed.db.in | 55 +++++++++ bin/tests/system/glue/ns1/tc-test-unsigned.db | 112 ++++++++++++++++++ bin/tests/system/glue/setup.sh | 2 + bin/tests/system/glue/tests.sh | 48 ++++++++ 7 files changed, 259 insertions(+) create mode 100644 bin/tests/system/glue/ns1/sign.sh create mode 100644 bin/tests/system/glue/ns1/tc-test-signed.db.in create mode 100644 bin/tests/system/glue/ns1/tc-test-unsigned.db diff --git a/bin/tests/system/glue/clean.sh b/bin/tests/system/glue/clean.sh index d6ec3455d6..138980e90a 100644 --- a/bin/tests/system/glue/clean.sh +++ b/bin/tests/system/glue/clean.sh @@ -19,5 +19,9 @@ rm -f */named.conf rm -f */named.memstats rm -f */named.run rm -f dig.out +rm -f ns*/K* +rm -f ns*/dsset-* rm -f ns*/managed-keys.bind* rm -f ns*/named.lock +rm -f ns*/tc-test-signed.db +rm -f ns*/tc-test-signed.db.signed diff --git a/bin/tests/system/glue/ns1/named.conf.in b/bin/tests/system/glue/ns1/named.conf.in index 4d1ef75611..61195f9136 100644 --- a/bin/tests/system/glue/ns1/named.conf.in +++ b/bin/tests/system/glue/ns1/named.conf.in @@ -32,7 +32,18 @@ zone "root-servers.nil" { type primary; file "root-servers.nil.db"; }; + zone "net" { type primary; file "net.db"; }; + +zone "tc-test-unsigned" { + type master; + file "tc-test-unsigned.db"; +}; + +zone "tc-test-signed" { + type master; + file "tc-test-signed.db.signed"; +}; diff --git a/bin/tests/system/glue/ns1/sign.sh b/bin/tests/system/glue/ns1/sign.sh new file mode 100644 index 0000000000..64250065e6 --- /dev/null +++ b/bin/tests/system/glue/ns1/sign.sh @@ -0,0 +1,27 @@ +#!/bin/sh -e + +# 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. + +. ../../conf.sh + +zone=tc-test-signed +infile=tc-test-signed.db.in +zonefile=tc-test-signed.db + +# The signing algorithm and key sizes used here are NOT arbitrary - they have +# been carefully chosen to ensure that the signed referral response checked in +# the test will be around 512 bytes in size with glue records excluded. Please +# keep this in mind when updating signing algorithms used in system tests. +keyname=$($KEYGEN -q -a RSASHA256 -b 2048 -n zone $zone) +cat "$infile" "$keyname.key" > "$zonefile" + +$SIGNER -P -o $zone $zonefile > /dev/null diff --git a/bin/tests/system/glue/ns1/tc-test-signed.db.in b/bin/tests/system/glue/ns1/tc-test-signed.db.in new file mode 100644 index 0000000000..5c0181dc3a --- /dev/null +++ b/bin/tests/system/glue/ns1/tc-test-signed.db.in @@ -0,0 +1,55 @@ +; 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. + +; CAUTION: Contents of this zone were carefully crafted so that the responses +; to the queries used in the "glue" system test have a very specific size. +; Editing this zone is not recommended as it may break the relevant checks. + +$TTL 300 +@ IN SOA ns hostmaster ( + 1 + 3600 + 1800 + 1814400 + 3600 + ) + NS a +a A 10.53.0.1 + +subdomain-a NS 0123456789.subdomain-a + NS 0123456.subdomain-a + NS 0123.subdomain-a + +0123456789.subdomain-a A 10.53.0.1 +0123456.subdomain-a A 10.53.0.1 +0123.subdomain-a A 10.53.0.1 + +subdomain-aaaa NS 0123456789.subdomain-aaaa + NS 0123456.subdomain-aaaa + NS 0123.subdomain-aaaa + +0123456789.subdomain-aaaa AAAA fd92:7065:b8e:ffff::1 +0123456.subdomain-aaaa AAAA fd92:7065:b8e:ffff::1 +0123.subdomain-aaaa AAAA fd92:7065:b8e:ffff::1 + +subdomain-both NS 0123456789.subdomain-both + NS 0123456.subdomain-both + NS 0123.subdomain-both + NS 0.subdomain-both + +0123456789.subdomain-both A 10.53.0.1 + AAAA fd92:7065:b8e:ffff::1 +0123456.subdomain-both A 10.53.0.1 + AAAA fd92:7065:b8e:ffff::1 +0123.subdomain-both A 10.53.0.1 + AAAA fd92:7065:b8e:ffff::1 +0.subdomain-both A 10.53.0.1 + AAAA fd92:7065:b8e:ffff::1 diff --git a/bin/tests/system/glue/ns1/tc-test-unsigned.db b/bin/tests/system/glue/ns1/tc-test-unsigned.db new file mode 100644 index 0000000000..de326c0acc --- /dev/null +++ b/bin/tests/system/glue/ns1/tc-test-unsigned.db @@ -0,0 +1,112 @@ +; 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. + +; CAUTION: Contents of this zone were carefully crafted so that the responses +; to the queries used in the "glue" system test have a very specific size. +; Editing this zone is not recommended as it may break the relevant checks. + +$TTL 300 +@ IN SOA ns hostmaster ( + 1 + 3600 + 1800 + 1814400 + 3600 + ) + NS a +a A 10.53.0.1 + +subdomain-a NS abcdefghijklmnopqrstuvwxyz.subdomain-a + NS bcdefghijklmnopqrstuvwxyz.subdomain-a + NS cdefghijklmnopqrstuvwxyz.subdomain-a + NS defghijklmnopqrstuvwxyz.subdomain-a + NS efghijklmnopqrstuvwxyz.subdomain-a + NS fghijklmnopqrstuvwxyz.subdomain-a + NS ghijklmnopqrstuvwxyz.subdomain-a + NS hijklmnopqrstuvwxyz.subdomain-a + NS ijklmnopqrstuvwxyz.subdomain-a + NS jklmnopqrstuvwxyz.subdomain-a + NS klmnopqrstuvwxyz.subdomain-a + NS lmnopqrstuvwxyz.subdomain-a + NS mnopqrstuvwxyz.subdomain-a + +abcdefghijklmnopqrstuvwxyz.subdomain-a A 10.53.0.1 +bcdefghijklmnopqrstuvwxyz.subdomain-a A 10.53.0.1 +cdefghijklmnopqrstuvwxyz.subdomain-a A 10.53.0.1 +defghijklmnopqrstuvwxyz.subdomain-a A 10.53.0.1 +efghijklmnopqrstuvwxyz.subdomain-a A 10.53.0.1 +fghijklmnopqrstuvwxyz.subdomain-a A 10.53.0.1 +ghijklmnopqrstuvwxyz.subdomain-a A 10.53.0.1 +hijklmnopqrstuvwxyz.subdomain-a A 10.53.0.1 +ijklmnopqrstuvwxyz.subdomain-a A 10.53.0.1 +jklmnopqrstuvwxyz.subdomain-a A 10.53.0.1 +klmnopqrstuvwxyz.subdomain-a A 10.53.0.1 +lmnopqrstuvwxyz.subdomain-a A 10.53.0.1 +mnopqrstuvwxyz.subdomain-a A 10.53.0.1 + +subdomain-aaaa NS abcdefghijklmnopqrstuvwxyz.subdomain-aaaa + NS bcdefghijklmnopqrstuvwxyz.subdomain-aaaa + NS cdefghijklmnopqrstuvwxyz.subdomain-aaaa + NS defghijklmnopqrstuvwxyz.subdomain-aaaa + NS efghijklmnopqrstuvwxyz.subdomain-aaaa + NS fghijklmnopqrstuvwxyz.subdomain-aaaa + NS ghijklmnopqrstuvwxyz.subdomain-aaaa + NS hijklmnopqrstuvwxyz.subdomain-aaaa + NS ijklmnopqrstuvwxyz.subdomain-aaaa + NS jklmnopqrstuvwxyz.subdomain-aaaa + NS klmnopqrstuvwxyz.subdomain-aaaa + NS lmnopqrstuvwxyz.subdomain-aaaa + NS mnopqrstuvwxyz.subdomain-aaaa + +abcdefghijklmnopqrstuvwxyz.subdomain-aaaa AAAA fd92:7065:b8e:ffff::1 +bcdefghijklmnopqrstuvwxyz.subdomain-aaaa AAAA fd92:7065:b8e:ffff::1 +cdefghijklmnopqrstuvwxyz.subdomain-aaaa AAAA fd92:7065:b8e:ffff::1 +defghijklmnopqrstuvwxyz.subdomain-aaaa AAAA fd92:7065:b8e:ffff::1 +efghijklmnopqrstuvwxyz.subdomain-aaaa AAAA fd92:7065:b8e:ffff::1 +fghijklmnopqrstuvwxyz.subdomain-aaaa AAAA fd92:7065:b8e:ffff::1 +ghijklmnopqrstuvwxyz.subdomain-aaaa AAAA fd92:7065:b8e:ffff::1 +hijklmnopqrstuvwxyz.subdomain-aaaa AAAA fd92:7065:b8e:ffff::1 +ijklmnopqrstuvwxyz.subdomain-aaaa AAAA fd92:7065:b8e:ffff::1 +jklmnopqrstuvwxyz.subdomain-aaaa AAAA fd92:7065:b8e:ffff::1 +klmnopqrstuvwxyz.subdomain-aaaa AAAA fd92:7065:b8e:ffff::1 +lmnopqrstuvwxyz.subdomain-aaaa AAAA fd92:7065:b8e:ffff::1 +mnopqrstuvwxyz.subdomain-aaaa AAAA fd92:7065:b8e:ffff::1 + +subdomain-both NS abcdefghijklmnopqrstuvwxyz.subdomain-both + NS bcdefghijklmnopqrstuvwxyz.subdomain-both + NS cdefghijklmnopqrstuvwxyz.subdomain-both + NS defghijklmnopqrstuvwxyz.subdomain-both + NS efghijklmnopqrstuvwxyz.subdomain-both + NS fghijklmnopqrstuvwxyz.subdomain-both + NS ghijklmnopqrstuvwxyz.subdomain-both + NS hijklmnopqrstuvwxyz.subdomain-both + NS ijklmnopqrstuvwxyz.subdomain-both + NS jklmnopqrstuvwxyz.subdomain-both + NS klmnopqrstuvwxyz.subdomain-both + NS lmnopqrstuvwxyz.subdomain-both + NS mnopqrstuvwxyz.subdomain-both + +abcdefghijklmnopqrstuvwxyz.subdomain-both A 10.53.0.1 + AAAA fd92:7065:b8e:ffff::1 +bcdefghijklmnopqrstuvwxyz.subdomain-both A 10.53.0.1 + AAAA fd92:7065:b8e:ffff::1 +cdefghijklmnopqrstuvwxyz.subdomain-both A 10.53.0.1 + AAAA fd92:7065:b8e:ffff::1 +defghijklmnopqrstuvwxyz.subdomain-both A 10.53.0.1 + AAAA fd92:7065:b8e:ffff::1 +efghijklmnopqrstuvwxyz.subdomain-both A 10.53.0.1 + AAAA fd92:7065:b8e:ffff::1 +fghijklmnopqrstuvwxyz.subdomain-both A 10.53.0.1 + AAAA fd92:7065:b8e:ffff::1 +ghijklmnopqrstuvwxyz.subdomain-both A 10.53.0.1 + AAAA fd92:7065:b8e:ffff::1 +hijklmnopqrstuvwxyz.subdomain-both A 10.53.0.1 + AAAA fd92:7065:b8e:ffff::1 diff --git a/bin/tests/system/glue/setup.sh b/bin/tests/system/glue/setup.sh index 82240a7c1b..1cac6e7efc 100644 --- a/bin/tests/system/glue/setup.sh +++ b/bin/tests/system/glue/setup.sh @@ -14,3 +14,5 @@ . ../conf.sh copy_setports ns1/named.conf.in ns1/named.conf + +( cd ns1 && $SHELL sign.sh ) diff --git a/bin/tests/system/glue/tests.sh b/bin/tests/system/glue/tests.sh index e761839a77..4d750b04be 100644 --- a/bin/tests/system/glue/tests.sh +++ b/bin/tests/system/glue/tests.sh @@ -38,5 +38,53 @@ digcomp noglue.good dig.out.$n || ret=1 if [ "$ret" -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) +n=$((n+1)) +echo_i "testing truncation for unsigned referrals close to UDP packet size limit (A glue) ($n)" +ret=0 +dig_with_opts @10.53.0.1 +ignore +noedns foo.subdomain-a.tc-test-unsigned. > dig.out.$n || ret=1 +grep -q "flags:[^;]* tc" dig.out.$n || ret=1 +if [ "$ret" -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +n=$((n+1)) +echo_i "testing truncation for unsigned referrals close to UDP packet size limit (AAAA glue) ($n)" +ret=0 +dig_with_opts @10.53.0.1 +ignore +noedns foo.subdomain-aaaa.tc-test-unsigned. > dig.out.$n || ret=1 +grep -q "flags:[^;]* tc" dig.out.$n || ret=1 +if [ "$ret" -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +n=$((n+1)) +echo_i "testing truncation for unsigned referrals close to UDP packet size limit (A+AAAA glue) ($n)" +ret=0 +dig_with_opts @10.53.0.1 +ignore +noedns foo.subdomain-both.tc-test-unsigned. > dig.out.$n || ret=1 +grep -q "flags:[^;]* tc" dig.out.$n || ret=1 +if [ "$ret" -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +n=$((n+1)) +echo_i "testing truncation for signed referrals close to UDP packet size limit (A glue) ($n)" +ret=0 +dig_with_opts @10.53.0.1 +ignore +dnssec +bufsize=512 foo.subdomain-a.tc-test-signed. > dig.out.$n || ret=1 +grep -q "flags:[^;]* tc" dig.out.$n || ret=1 +if [ "$ret" -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +n=$((n+1)) +echo_i "testing truncation for signed referrals close to UDP packet size limit (AAAA glue) ($n)" +ret=0 +dig_with_opts @10.53.0.1 +ignore +dnssec +bufsize=512 foo.subdomain-aaaa.tc-test-signed. > dig.out.$n || ret=1 +grep -q "flags:[^;]* tc" dig.out.$n || ret=1 +if [ "$ret" -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +n=$((n+1)) +echo_i "testing truncation for signed referrals close to UDP packet size limit (A+AAAA glue) ($n)" +ret=0 +dig_with_opts @10.53.0.1 +ignore +dnssec +bufsize=512 foo.subdomain-both.tc-test-signed. > dig.out.$n || ret=1 +grep -q "flags:[^;]* tc" dig.out.$n || ret=1 +if [ "$ret" -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + echo_i "exit status: $status" [ $status -eq 0 ] || exit 1 From d977eae211fd9f5e255d01a0676814af35f1903c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 22 Sep 2022 14:03:17 +0200 Subject: [PATCH 3/5] Mark required glue during glue cache processing If an NS RRset at the parent side of a delegation point only contains in-bailiwick NS records, at least one glue record should be included in every referral response sent for such a delegation point or else clients will need to send follow-up queries in order to determine name server addresses. In certain edge cases (when the total size of a referral response without glue records was just below to the UDP packet size limit), named failed to adhere to that rule by sending non-truncated, glueless referral responses. Fix the problem by marking all in-bailiwick NS records processed by glue_nsdname_cb() (the dns_rdataset_additionaldata() callback used by RBTDB code while iterating over an NS RRset when dns_rdataset_addglue() is called) with the DNS_RDATASETATTR_REQUIRED flag. Note that for simplicity, glue RRsets for all in-bailiwick NS records are marked this way, even though dns_message_rendersection() only checks the attributes for the first rdataset associated with the first name added to the ADDITIONAL section. --- lib/dns/rbtdb.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index f6e8444e17..38d485dbc3 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -9407,6 +9407,7 @@ typedef struct { rbtdb_glue_t *glue_list; dns_rbtdb_t *rbtdb; rbtdb_version_t *rbtversion; + dns_name_t *nodename; } rbtdb_glue_additionaldata_ctx_t; static void @@ -9629,6 +9630,25 @@ glue_nsdname_cb(void *arg, const dns_name_t *name, dns_rdatatype_t qtype, } } + /* + * If the currently processed NS record is in-bailiwick, mark any glue + * RRsets found for it with DNS_RDATASETATTR_REQUIRED. Note that for + * simplicity, glue RRsets for all in-bailiwick NS records are marked + * this way, even though dns_message_rendersection() only checks the + * attributes for the first rdataset associated with the first name + * added to the ADDITIONAL section. + */ + if (glue != NULL && dns_name_issubdomain(name, ctx->nodename)) { + if (dns_rdataset_isassociated(&glue->rdataset_a)) { + glue->rdataset_a.attributes |= + DNS_RDATASETATTR_REQUIRED; + } + if (dns_rdataset_isassociated(&glue->rdataset_aaaa)) { + glue->rdataset_aaaa.attributes |= + DNS_RDATASETATTR_REQUIRED; + } + } + if (glue != NULL) { glue->next = ctx->glue_list; ctx->glue_list = glue; @@ -9666,6 +9686,7 @@ rdataset_addglue(dns_rdataset_t *rdataset, dns_dbversion_t *version, dns_rbtdb_t *rbtdb = rdataset->private1; dns_rbtnode_t *node = rdataset->private2; rbtdb_version_t *rbtversion = version; + dns_fixedname_t nodename; uint32_t idx; rbtdb_glue_table_node_t *cur; bool found = false; @@ -9811,6 +9832,14 @@ no_glue: ctx.rbtdb = rbtdb; ctx.rbtversion = rbtversion; + /* + * Get the owner name of the NS RRset - it will be necessary for + * identifying required glue in glue_nsdname_cb() (by determining which + * NS records in the delegation are in-bailiwick). + */ + ctx.nodename = dns_fixedname_initname(&nodename); + nodefullname((dns_db_t *)rbtdb, node, ctx.nodename); + RWLOCK(&rbtversion->glue_rwlock, isc_rwlocktype_write); maybe_rehash_gluetable(rbtversion); From 68a004501aa116e8631c924375161ae8fa428828 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 22 Sep 2022 14:03:17 +0200 Subject: [PATCH 4/5] Ensure required cached glue is rendered When looking for required glue, dns_message_rendersection() only processes the first rdataset associated with the first name added to the ADDITIONAL section. If the DNS_RDATASETATTR_REQUIRED attribute is set for an rdataset which is located somewhere else (i.e. the name it is associated with is preceded by another name in the ADDITIONAL section), it will not be honored, i.e. the TC bit will not be set even if the rdataset does not fit into the response. Check the attributes of each processed rdataset while appending names to a referral response based on a glue cache entry. If a given rdataset is marked with DNS_RDATASETATTR_REQUIRED, make sure the name it is associated with is added to the response at the beginning of the ADDITIONAL section, not its end. Note that using ISC_LIST_PREPEND() instead of ISC_LIST_APPEND() is not necessary when associating the rdataset with its owner name because the dns_name_t structures are initialized just before the glue rdatasets are associated with them and therefore they are empty at that point, which means no other (non-required) rdataset can precede the glue rdatasets within the dns_name_t structure owning them. --- lib/dns/rbtdb.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index 38d485dbc3..53fbb2822f 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -9680,6 +9680,8 @@ glue_nsdname_cb(void *arg, const dns_name_t *name, dns_rdatatype_t qtype, return (result); } +#define IS_REQUIRED_GLUE(r) (((r)->attributes & DNS_RDATASETATTR_REQUIRED) != 0) + static isc_result_t rdataset_addglue(dns_rdataset_t *rdataset, dns_dbversion_t *version, dns_message_t *msg) { @@ -9764,6 +9766,7 @@ restart: dns_rdataset_t *rdataset_aaaa = NULL; dns_rdataset_t *sigrdataset_aaaa = NULL; dns_name_t *gluename = dns_fixedname_name(&ge->fixedname); + bool prepend_name = false; dns_message_gettempname(msg, &name); @@ -9788,6 +9791,9 @@ restart: if (rdataset_a != NULL) { dns_rdataset_clone(&ge->rdataset_a, rdataset_a); ISC_LIST_APPEND(name->list, rdataset_a, link); + if (IS_REQUIRED_GLUE(rdataset_a)) { + prepend_name = true; + } } if (sigrdataset_a != NULL) { @@ -9798,6 +9804,9 @@ restart: if (rdataset_aaaa != NULL) { dns_rdataset_clone(&ge->rdataset_aaaa, rdataset_aaaa); ISC_LIST_APPEND(name->list, rdataset_aaaa, link); + if (IS_REQUIRED_GLUE(rdataset_aaaa)) { + prepend_name = true; + } } if (sigrdataset_aaaa != NULL) { dns_rdataset_clone(&ge->sigrdataset_aaaa, @@ -9806,6 +9815,23 @@ restart: } dns_message_addname(msg, name, DNS_SECTION_ADDITIONAL); + + /* + * When looking for required glue, dns_message_rendersection() + * only processes the first rdataset associated with the first + * name added to the ADDITIONAL section. dns_message_addname() + * performs an append on the list of names in a given section, + * so if any glue record was marked as required, we need to + * move the name it is associated with to the beginning of the + * list for the ADDITIONAL section or else required glue might + * not be rendered. + */ + if (prepend_name) { + ISC_LIST_UNLINK(msg->sections[DNS_SECTION_ADDITIONAL], + name, link); + ISC_LIST_PREPEND(msg->sections[DNS_SECTION_ADDITIONAL], + name, link); + } } no_glue: From 07721836a4f11b4e52b3d00969616e339bf46933 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 22 Sep 2022 14:03:17 +0200 Subject: [PATCH 5/5] Add CHANGES and release notes for [GL #1967] --- CHANGES | 4 ++++ doc/notes/notes-current.rst | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/CHANGES b/CHANGES index 55468d151c..2790440a74 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5977. [bug] named could incorrectly return non-truncated, glueless + referrals for responses whose size was close to the UDP + packet size limit. [GL #1967] + 5976. [cleanup] isc_timer_t objects are now created, started and destroyed in a particular loop, and timer callbacks run in that loop. isc_timer_stop() can still be called diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 1bdaa66242..1d2dd322c0 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -62,3 +62,7 @@ Bug Fixes - An assertion failure was fixed in ``named`` that was caused by aborting the statistics channel connection while sending statistics data to the client. :gl:`#3542` + +- :iscman:`named` could incorrectly return non-truncated, glueless + referrals for responses whose size was close to the UDP packet size + limit. :gl:`#1967`