Mark Andrews [Tue, 7 Dec 2021 03:26:10 +0000 (14:26 +1100)]
Test validation deadlock recovery
Check that the following scenarios do not cause fetches to hang:
- An undelegated unsigned zone with CNAME to apex where the parent
zone is signed
- An NSEC3 zone with only DNSKEY correctly signed
Evan Hunt [Tue, 30 Nov 2021 19:17:01 +0000 (11:17 -0800)]
check for validator/fetch dependency loops
It was possible for a deadlock to occur when a fetch started
a validator which started another fetch for the same name.
To address this, we now store pointers from the valdiator object
to the originating fetch, and from the fetch object to the originating
validator (if any), so that potential loops can be detected.
Evan Hunt [Thu, 2 Dec 2021 20:04:42 +0000 (12:04 -0800)]
prevent a shutdown hang on non-matching TCP responses
When a non-matching DNS response is received by the resolver,
it calls dns_dispatch_getnext() to resume reading. This is necessary
for UDP but not for TCP, because TCP connections automatically
resume reading after any valid DNS response.
This commit adds a 'tcpreading' flag to TCP dispatches, so that
`dispatch_getnext()` can be called multiple times without subsequent
calls having any effect.
Ondřej Surý [Mon, 6 Dec 2021 10:10:17 +0000 (11:10 +0100)]
Stop leaking mutex in nmworker and cond in nm socket
On FreeBSD, the pthread primitives are not solely allocated on stack,
but part of the object lives on the heap. Missing pthread_*_destroy
causes the heap memory to grow and in case of fast lived object it's
possible to run out-of-memory.
Properly destroy the leaking mutex (worker->lock) and
the leaking condition (sock->cond).
Ondřej Surý [Tue, 7 Dec 2021 19:35:58 +0000 (20:35 +0100)]
Reduce the number of hazard pointers
Previously, we set the number of the hazard pointers to be 4 times the
number of workers because the dispatch ran on the old socket code.
Since the old socket code was removed there's a smaller number of
threads, namely:
Ondřej Surý [Tue, 7 Dec 2021 10:15:27 +0000 (11:15 +0100)]
Fix the isc_hp initialization and memory usage
Previously, the isc_hp_init() could not lower the value of
isc__hp_max_threads, but because of a mistake the isc__hp_max_threads
would be set to HP_MAX_THREADS (e.g. 128 threads) thus it would be
always set to 128. This would result in increased memory usage even
when small number of workers were in use.
Change the default value of isc__hp_max_threads to be 1.
Additionally, enforce the max_hps value in isc_hp_new() to be smaller or
equal to HP_MAX_HPS. The only user is isc_queue which uses just 1
hazard pointer, so it's only theoretical issue.
Petr Špaček [Mon, 6 Dec 2021 15:24:57 +0000 (16:24 +0100)]
Mark broken-nsec option as deprecated
It's unclear if we are going to keep it or not, so let's mark it as
deprecated for a good measure. It's easier to un-deprecate it than the
other way around.
Evan Hunt [Thu, 2 Dec 2021 01:02:05 +0000 (17:02 -0800)]
restore the fetch lifetime timer
the lifetime expiry timer for the fetch context was removed
when we switched to using in-band netmgr timeouts. however,
it turns out some dependency loops can occur between a fetch
and the ADB the validator; these deadlocks were formerly broken
when the timer fired, and now there's no timer. we can fix these
errors individually, but in the meantime we don't want the server
to get hung at shutdown because of dangling fetches.
this commit puts back a single timer, which fires two seconds
after the fetch should have completed, and shuts it down. it also
logs a message at level INFO so we know about the problems when
they occur.
Mark Andrews [Wed, 1 Dec 2021 13:34:38 +0000 (00:34 +1100)]
Reject NSEC records with next field with \000 label
A number of DNS implementation produce NSEC records with bad type
maps that don't contain types that exist at the name leading to
NODATA responses being synthesize instead of the records in the
zone. NSEC records with these bad type maps often have the NSEC
NSEC field set to '\000.QNAME'. We look for the first label of
this pattern.
e.g.
example.com NSEC \000.example.com SOA NS NSEC RRSIG
example.com RRRSIG NSEC ...
example.com SOA ...
example.com RRRSIG SOA ...
example.com NS ...
example.com RRRSIG NS ...
example.com A ...
example.com RRRSIG A ...
A is missing from the type map.
This introduces a temporary option 'reject-000-label' to control
this behaviour.
Mark Andrews [Wed, 1 Dec 2021 03:52:31 +0000 (14:52 +1100)]
Add server christmas tree test
This sets as many server options as possible at once to detect
cut-and-paste bugs when implementing new server options in peer.c.
Most of the accessor functions are similar and it is easy to miss
updating a macro name or structure element name when adding new
accessor functions.
checkconf/setup.sh is there to minimise the difference to branches
with optional server options where the list is updated at runtime.
Mark Andrews [Wed, 1 Dec 2021 03:10:47 +0000 (14:10 +1100)]
Allow servers that emit broken NSEC records to be identified
'server <prefix> { broken-nsec yes; };' can now be used to stop
NSEC records from negative responses from servers in the given
prefix being cached and hence available to synth-from-dnssec.
Mark Andrews [Fri, 26 Nov 2021 23:41:38 +0000 (10:41 +1100)]
Add synthesis of NODATA at wildcard
The old code rejected NSEC that proved the wildcard name existed
(exists). The new code rejects NSEC that prove that the wildcard
name exists and that the type exists (exists && data) but accept
NSEC that prove the wildcard name exists.
query_synthnxdomain (renamed query_synthnxdomainnodata) already
took the NSEC records and added the correct records to the message
body for NXDOMAIN or NODATA responses with the above change. The
only additional change needed was to ensure the correct RCODE is
set.
Mark Andrews [Fri, 5 Nov 2021 22:30:48 +0000 (09:30 +1100)]
Ignore NSEC records without RRSIG and NSEC present
dns_nsec_noexistnodata now checks that RRSIG and NSEC are
present in the type map. Both types should be present in
a correctly constructed NSEC record. This check is in
addition to similar checks in resolver.c and validator.c.
Mark Andrews [Wed, 20 Oct 2021 01:01:00 +0000 (12:01 +1100)]
Extend dns_db_nodecount to access auxilary rbt node counts
dns_db_nodecount can now be used to get counts from the auxilary
rbt databases. The existing node count is returned by
tree=dns_dbtree_main. The nsec and nsec3 node counts by dns_dbtree_nsec
and dns_dbtree_nsec3 respectively.
Mark Andrews [Tue, 19 Oct 2021 22:22:50 +0000 (09:22 +1100)]
Allow "black lies" to be cached
"black lies" differ from "white lies" in that the owner name of the
NSEC record matches the QNAME and the intent is to return NODATA
instead of NXDOMAIN for all types. Caching this NSEC does not lead
to unexpected behaviour on synthesis when the QNAME matches the
NSEC owner which it does for the the general "white lie" response.
"black lie" QNAME NSEC \000.QNAME NSEC RRSIG
"white lie" QNAME- NSEC QNAME+ NSEC RRSIG
where QNAME- is a name that is close to QNAME but sorts before QNAME
and QNAME+ is a that is close to QNAME but sorts after QNAME.
Black lies are safe to cache as they don't bring into existence
names that are not intended to exist. "Black lies" intentional change
NXDOMAIN to NODATA. "White lies" bring QNAME- into existence and named
would synthesis NODATA for QNAME+ if it is queried for that name
instead of discovering the, presumable, NXDOMAIN response.
Note rejection NSEC RRsets with NEXT names starting with the label
'\000' renders this change ineffective (see reject-000-label).
Mark Andrews [Tue, 19 Oct 2021 03:42:48 +0000 (14:42 +1100)]
Check that minimal NSEC records are not cached
construct a test zone which contains a minimal NSEC record,
emit priming queries for this record, and then check that
a respose that would be synthesised from it isn't.
Mark Andrews [Fri, 15 Oct 2021 03:47:07 +0000 (14:47 +1100)]
Count DNS_R_COVERINGNSEC as a cache {query}hit
Note when synthesising answer involving wildcards we look in the
cache multiple times, once for the QNAME and once for the wildcard
name which is constucted by looking at the names from the covering
NSEC return by the QNAME miss.
Mark Andrews [Wed, 29 Sep 2021 03:55:46 +0000 (13:55 +1000)]
Rework rbtdb.c:find_coveringnsec() to use the auxilary nsec rbt
this improves the performance of looking for NSEC and RRSIG(NSEC)
records in the cache by skipping lots of nodes in the main trees
in the cache without these records present. This is a simplified
version of previous_closest_nsec() which uses the same underlying
mechanism to look for NSEC and RRSIG(NSEC) records in authorative
zones.
The auxilary NSEC tree was already being maintained as a side effect
of looking for the covering NSEC in large zones where there can be
lots of glue records that needed to be skipped. Nodes are added
to the tree whenever a NSEC record is added to the primary tree.
They are removed when the corresponding node is removed from the
primary tree.
Having nodes in the NSEC tree w/o NSEC records in the primary tree
should not impact on synth-from-dnssec efficiency as that node would
have held the NSEC we would have been needed to synthesise the
response. Removing the node when the NSEC RRset expires would only
cause rbtdb to return a NSEC which would be rejected at a higher
level.
Ondřej Surý [Wed, 1 Dec 2021 16:41:20 +0000 (17:41 +0100)]
Improve the logging on failed TCP accept
Previously, when TCP accept failed, we have logged a message with
ISC_LOG_ERROR level. One common case, how this could happen is that the
client hits TCP client quota and is put on hold and when resumed, the
client has already given up and closed the TCP connection. In such
case, the named would log:
TCP connection failed: socket is not connected
This message was quite confusing because it actually doesn't say that
it's related to the accepting the TCP connection and also it logs
everything on the ISC_LOG_ERROR level.
Change the log message to "Accepting TCP connection failed" and for
specific error states lower the severity of the log message to
ISC_LOG_INFO.
Ondřej Surý [Wed, 1 Dec 2021 11:22:59 +0000 (12:22 +0100)]
Add TCP connection reset test
The TCP connection reset test starts mock UDP and TCP server which
always returns empty DNS answer with TC bit set over UDP and resets the
TCP connection after five seconds.
When tested without the fix, the DNS query to 10.53.0.2 times out and
the ns2 server hangs at shutdown.
Evan Hunt [Tue, 30 Nov 2021 08:57:27 +0000 (09:57 +0100)]
On non-matching answer, check for missed timeout
A TCP connection may be held open past its proper timeout if it's
receiving a stream of DNS responses that don't match any queries.
In this case, we now check whether the oldest query should have timed
out.
Ondřej Surý [Mon, 29 Nov 2021 08:59:33 +0000 (09:59 +0100)]
Tear down the TCP connection on too many unexpected DNS messages
When the outgoing TCP dispatch times-out active response, we might still
receive the answer during the lifetime of the connection. Previously,
we would just ignore any non-matching DNS answers, which would allow the
server to feed us with otherwise valid DNS answer and keep the
connection open.
Add a counter for timed-out DNS queries over TCP and tear down the whole
TCP connection if we receive unexpected number of DNS answers.
Ondřej Surý [Fri, 26 Nov 2021 08:14:58 +0000 (09:14 +0100)]
Shutdown all TCP connection on invalid DNS message
Previously, when invalid DNS message is received over TCP we throw the
garbage DNS message away and continued looking for valid DNS message
that would match our outgoing queries. This logic makes sense for UDP,
because anyone can send DNS message over UDP.
Change the logic that the TCP connection is closed when we receive
garbage, because the other side is acting malicious.
Ondřej Surý [Fri, 26 Nov 2021 08:14:58 +0000 (09:14 +0100)]
Shutdown all active TCP connections on error
When outgoing TCP connection was prematurely terminated (f.e. with
connection reset), the dispatch code would not cleanup the resources
used by such connection leading to dangling dns_dispentry_t entries.
Ondřej Surý [Wed, 1 Dec 2021 09:20:31 +0000 (10:20 +0100)]
Add an idna test that _ and * characters are preserved
Add a idna that checks whether non-character letters like _ and * are
preserved when IDN is enabled. This wasn't the case when
UseSTD3ASCIIRules were enabled, f.e. _ from _tcp would get mangled to
tcp.
Artem Boldariev [Wed, 1 Dec 2021 10:50:21 +0000 (12:50 +0200)]
Increase startup timeout for servers in system tests
This change is made in particular to address the issue with 'doth'
system tests where servers are unable to iniitalise in time in CI
system under high load (that happened particularly often for Debian
Buster cross32 configuration).
The right solution, is, of course, to (re)use TLS context sparingly,
while right now we create too many of them.
Artem Boldariev [Tue, 30 Nov 2021 08:42:23 +0000 (10:42 +0200)]
TLS context handling code: Fix an abort on ancient OpenSSL version
There was a logical bug when setting a list of enabled TLS protocols,
which may lead to a crash (an abort()) on systems with ancient OpenSSL
versions.
The problem was due to the fact that we were INSIST()ing on supporting
all of the TLS versions, while checking only for mentioned in the
configuration was implied.
Artem Boldariev [Mon, 29 Nov 2021 10:50:35 +0000 (12:50 +0200)]
Add transport-acl system test
This commit adds a new system-test: transport-acl system test. It is
intended to test the new, extended syntax for ACLs, the one where port
or transport protocol can be specified. Currently, it includes the
tests only using allow-transfer statement, as this extended syntax is
used only there, at least for now.
Artem Boldariev [Tue, 23 Nov 2021 13:04:51 +0000 (15:04 +0200)]
Mention that the allow-transfer option has been extended
This commit updates both the reference manual and release notes with
the information that 'allow-transfer' has been extended with
additional "port" and "transport" options.
Artem Boldariev [Mon, 22 Nov 2021 13:31:15 +0000 (15:31 +0200)]
Extend the 'doth' system test to test extended allow-transfer option
This commit extends the 'doth' system test to verify that the new
extended 'allow-transfer' option syntax featuring 'port' and
'transport' parameters is supported and works as expected. That is, it
restricts the primary server to allow zone transfers only via XoT.
Additionally to that, it extends the 'checkonf' test with more
configuration file examples featuring the new syntax.
Artem Boldariev [Fri, 12 Nov 2021 14:53:13 +0000 (16:53 +0200)]
Integrate extended ACLs syntax featuring 'port' and 'transport' opts
This commit completes the integration of the new, extended ACL syntax
featuring 'port' and 'transport' options.
The runtime presentation and ACL loading code are extended to allow
the syntax to be used beyond the 'allow-transfer' option (e.g. in
'acl' definitions and other 'allow-*' options) and can be used to
ultimately extend the ACL support with transport-only
ACLs (e.g. 'transport-acl tls-acl port 853 transport tls'). But, due
to fundamental nature of such a change, it has not been completed as a
part of 9.17.X release series due to it being close to 9.18 stable
release status. That means that we do not have enough time to fully
test it.
The complete integration is planned as a part of 9.19.X release
series.
The code was manually verified to work as expected by temporarily
enabling the extended syntax for 'acl' statements and 'allow-query'
options, including ACL merging, negated ACLs.
Artem Boldariev [Thu, 4 Nov 2021 14:52:49 +0000 (16:52 +0200)]
Extend ACL syntax handling code with 'port' and 'transport' options
This commit extends ACL syntax handling code with 'port' and
'transport' options. Currently, the extended syntax is available only
for allow-transfer options.