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/bin/tests/system/glue/clean.sh b/bin/tests/system/glue/clean.sh index 4d84f068f8..138980e90a 100644 --- a/bin/tests/system/glue/clean.sh +++ b/bin/tests/system/glue/clean.sh @@ -15,9 +15,13 @@ # 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*/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 9ecf39b6a8..4d750b04be 100644 --- a/bin/tests/system/glue/tests.sh +++ b/bin/tests/system/glue/tests.sh @@ -13,21 +13,78 @@ . ../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)) + +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 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` diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index f6e8444e17..53fbb2822f 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; @@ -9660,12 +9680,15 @@ 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) { 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; @@ -9743,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); @@ -9767,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) { @@ -9777,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, @@ -9785,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: @@ -9811,6 +9858,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);