Merge branch '1967-prevent-generating-broken-glueless-referrals' into 'main'

Prevent generating broken glueless referrals

Closes #1967

See merge request isc-projects/bind9!4122
This commit is contained in:
Michał Kępień 2022-09-22 12:23:22 +00:00
commit 62813df44b
10 changed files with 343 additions and 12 deletions

View file

@ -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

View file

@ -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

View file

@ -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";
};

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -14,3 +14,5 @@
. ../conf.sh
copy_setports ns1/named.conf.in ns1/named.conf
( cd ns1 && $SHELL sign.sh )

View file

@ -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

View file

@ -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`

View file

@ -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);