Ondřej Surý [Wed, 21 Feb 2024 12:32:09 +0000 (13:32 +0100)]
Simplify the parent cleaning in the prune_tree() mechanism
Instead of juggling with node locks in a cycle, cleanup the node we are
just pruning and send any the parent that's also subject to the pruning
to the prune tree via normal way (e.g. enqueue pruning on the parent).
This simplifies the code and also spreads the pruning load across more
event loop ticks which is better for lock contention as less things run
in a tight loop.
In some older BIND 9 branches, the extra queuing overhead eliminated by
this change could be remotely exploited to cause excessive memory use.
Due to architectural shift, this branch is not vulnerable to that issue,
but applying the fix to the latter is nevertheless deemed prudent for
consistency and to make the code future-proof.
However, it turned out that having a single queue for the nodes to be
pruned increased lock contention to a level where cleaning up nodes from
the RBTDB took too long, causing the amount of memory used by the cache
to grow indefinitely over time.
This commit reverts the change to the pruning mechanism introduced by
commit 24381cc36d8528f5a4046fb2614451aeac4cdfc1 as BIND branches newer
than 9.16 were not affected by the excessive event queueing overhead
issue mentioned in the log message for the above commit.
Artem Boldariev [Thu, 22 Feb 2024 17:42:04 +0000 (19:42 +0200)]
Improve documentation on ephemeral TLS configuration
This commit improves the documentation on the ephemeral TLS
configuration and describes in more detail what is happening with TLS
configurations on reconfiguration in general.
Michal Nowak [Fri, 23 Feb 2024 13:51:23 +0000 (14:51 +0100)]
Watch logs from start in dialup system test
When the first parametrized test takes a bit longer than usual, the zone
transfer in ns3 may succeed before the second parametrized test is even
started, and then watch_log_from_here() won't find the "Transfer status:
success" message in the named log. Using watch_log_from_start() instead
makes sure the test is more stable.
Mark Andrews [Thu, 22 Feb 2024 23:12:47 +0000 (10:12 +1100)]
Do not use header_prev in expire_lru_headers
dns__cacherbt_expireheader can unlink / free header_prev underneath
it. Use ISC_LIST_TAIL after calling dns__cacherbt_expireheader
instead to get the next pointer to be processed.
Artem Boldariev [Mon, 19 Feb 2024 18:02:38 +0000 (20:02 +0200)]
Do not lock workers when using -T transferslowly/transferstuck
This commit ensures that worker threads are not sleeping (by using
select()) when '-T transferslowly/transferstuck' test options are
used. This commit converts synchronous implementation of the code into
an asynchronous one based on timers.
Artem Boldariev [Mon, 12 Feb 2024 20:51:39 +0000 (22:51 +0200)]
DoT: do not crash resolver on TLS context creation failure
The resolver's code was not ready to failures when trying to establish
a connection via TCP-based transports (e.g. when creating TLS contexts
before establishing a TLS connection).
Tom Krizek [Thu, 15 Feb 2024 13:55:56 +0000 (14:55 +0100)]
Don't include temp testdir on each log line
This was mostly an artifact to tell which log lines belong to which test
from the time when the test output could be all mingled together. Now
this info is reduntant, because the pytest logger already includes both
the system test name, and the specific test.
Tom Krizek [Thu, 15 Feb 2024 13:47:13 +0000 (14:47 +0100)]
Add utility logging functions to isctest.log
Unify the different loggers (conftest, module, test) into a single
interface. Remove the need to select the proper logger by automatically
selecting the most-specific logger currently available.
This also removes the need to use the logger/mlogger fixtures manually
and pass these around. This was especially annoying and unwieldy when
splitting the test cases into functions, because logger had to always be
passed around. Instead, it is now possible to use the
isctest.log.(debug,info,warning,error) functions.
Tom Krizek [Thu, 15 Feb 2024 12:57:42 +0000 (13:57 +0100)]
Move watchlog module into isctest.log package
Preparation for further logging improvements - keep the watchlog
contents in a separate module inside isctest.log. Export the names in
the log package so the imports don't change for the users of these
classes.
Tom Krizek [Tue, 13 Feb 2024 15:42:16 +0000 (16:42 +0100)]
Remove accidentally duplicated RNDCExecutor code
This code has probably been accidentally added during some rebase. The
actual RNDCExecutor and related classes are in isctest/rndc.py. Remove
the duplicated and unused code from isctest/log.py, as it doesn't belong
there.
Aram Sargsyan [Wed, 7 Feb 2024 09:22:55 +0000 (09:22 +0000)]
Address scan-build warnings
The warnings (see below) seem to be false-positives. Address them
by adding runtime checks.
resolver.c:1627:10: warning: Access to field 'tid' results in a dereference of a null pointer (loaded from variable 'fctx') [core.NullDereference]
1627 | REQUIRE(fctx->tid == isc_tid());
| ^~~~~~~~~
../../lib/isc/include/isc/util.h:332:34: note: expanded from macro 'REQUIRE'
332 | #define REQUIRE(e) ISC_REQUIRE(e)
| ^
../../lib/isc/include/isc/assertions.h:45:11: note: expanded from macro 'ISC_REQUIRE'
45 | ((void)((cond) || \
| ^~~~
resolver.c:10335:6: warning: Access to field 'depth' results in a dereference of a null pointer (loaded from variable 'fctx') [core.NullDereference]
10335 | if (fctx->depth > depth) {
| ^~~~~~~~~~~
2 warnings generated.
Evan Hunt [Wed, 14 Feb 2024 20:58:01 +0000 (12:58 -0800)]
fix several bugs in the RBTDB dbiterator implementation
- the DNS_DB_NSEC3ONLY and DNS_DB_NONSEC3 flags are mutually
exclusive; it never made sense to set both at the same time.
to enforce this, it is now a fatal error to do so. the
dbiterator implementation has been cleaned up to remove
code that treated the two as independent: if nonsec3 is
true, we can be certain nsec3only is false, and vice versa.
- previously, iterating a database backwards omitted
NSEC3 records even if DNS_DB_NONSEC3 had not been set. this
has been corrected.
- when an iterator reaches the origin node of the NSEC3 tree, we
need to skip over it and go to the next node in the sequence.
the NSEC3 origin node is there for housekeeping purposes and
never contains data.
- the dbiterator_test unit test has been expanded, several
incorrect expectations have been fixed. (for example, the
expected number of iterations has been reduced by one; we were
previously counting the NSEC3 origin node and we should not
have been doing so.)
Michał Kępień [Wed, 14 Feb 2024 13:49:49 +0000 (14:49 +0100)]
Swap CHANGES entries 6343 and 6344
Fix a CHANGES entries numbering issue that was inadvertently introduced
when change 6344 was backported. This makes the affected CHANGES
numbers consistent across all branches and releases again.
Michał Kępień [Wed, 14 Feb 2024 13:49:49 +0000 (14:49 +0100)]
Retroactively add release note for CVE-2023-50868
A release note for CVE-2023-50868 was not included in BIND 9.19.21, even
though that vulnerability was already addressed in that release (by the
fix for CVE-2023-50387). Retroactively add a relevant release note for
BIND 9.19.21.
Michał Kępień [Wed, 14 Feb 2024 13:49:49 +0000 (14:49 +0100)]
Mention CVE-2023-50868 in CHANGES entry 6322
Since CVE-2023-50868 does not have a dedicated fix in BIND 9, mention
its CVE identifier in the CHANGES entry for CVE-2023-50387 (KeyTrap),
which accompanied the code change that addresses both of these
vulnerabilities.
Evan Hunt [Thu, 5 Oct 2023 01:14:55 +0000 (18:14 -0700)]
clean up dns_rbt
- create_node() in rbt.c cannot fail
- the dns_rbt_*name() functions, which are wrappers around
dns_rbt_[add|find|delete]node(), were never used except in tests.
this change isn't really necessary since RBT is likely to go away
eventually anyway. but keeping the API as simple as possible while it
persists is a good thing, and may reduce confusion while QPDB is being
developed from RBTDB code.
Evan Hunt [Thu, 5 Oct 2023 00:49:51 +0000 (17:49 -0700)]
move DNS_RBT_NSEC_* to db.h
these values pertain to whether a node is in the main, nsec, or nsec3
tree of an RBTDB. they need to be moved to a more generic location so
they can also be used by QPDB.
(this is in db.h rather than db_p.h because rbt.c needs access to it.
technically, that's a layer violation, but it's a long-existing one;
refactoring to get rid of it would be a large hassle, and eventually
we expect to remove rbt.c anyway.)
Evan Hunt [Sun, 1 Oct 2023 08:06:49 +0000 (01:06 -0700)]
separate generic DB helpers into db_p.h
when the QPDB is implemented, we will need to have both qpdb_p.h and
rbtdb_p.h. in order to prevent name collisions or code duplication,
this commit adds a generic private header file, db_p.h, containing
structures and macros that will be used by both databases.
some functions and structs have been renamed to more specifically refer
to the RBT database, in order to avoid namespace collision with similar
things that will be needed by the QPDB later.
Evan Hunt [Mon, 6 Nov 2023 16:02:49 +0000 (17:02 +0100)]
refactor wildcard matching
refactor the wildcard matching code to make it a bit easier to
understand, in hopes that it will reduce the difficulty of converting
from RBTDB to QPDB later.
there are also some minor optimizations: previously, after stepping
backward to find the predecessor, we stepped back foward *from* the
predecessor to find the successor. we now reset the rbtnode chain to
its original starting point before stepping forward; this eliminates
some unnecessary processing. and, if neither predecessor nor successor
is found, we return early rather than carrying on with an unnecessary
effort to match labels.
Coverity detected that address->type.sa was too small when copying
a struct sockaddr_sin6, use the alterative union element
address->type.sin6 instead.
Ondřej Surý [Sun, 11 Feb 2024 08:13:43 +0000 (09:13 +0100)]
Add a system test for mixed-case data for the same owner
We were missing a test where a single owner name would have multiple
types with a different case. The generated RRSIGs and NSEC records will
then have different case than the signed records and message parser have
to cope with that and treat everything as the same owner.
Ondřej Surý [Sat, 10 Feb 2024 23:49:32 +0000 (00:49 +0100)]
Fix case insensitive matching in isc_ht hash table implementation
The case insensitive matching in isc_ht was basically completely broken
as only the hashvalue computation was case insensitive, but the key
comparison was always case sensitive.
Aydın Mercan [Tue, 19 Dec 2023 07:41:15 +0000 (10:41 +0300)]
Convert rwlock in isc_log_t to RCU
The isc_log_t contains a isc_logconfig_t that is swapped, dereferenced
or accessed its fields through a mutex. Instead of protecting it with a
rwlock, use RCU.
Ondřej Surý [Thu, 8 Feb 2024 11:31:09 +0000 (12:31 +0100)]
Fix UAF in ccmsg.c when reading stopped before sending
When shutting down the whole server, the reading could stop and detach
from controlconnection before sending is done. If send callback then
detaches from the last controlconnection handle, the ccmsg would be
invalidated after the send callback and thus we must not access ccmsg
after calling the send_cb().
Ondřej Surý [Thu, 8 Feb 2024 11:31:09 +0000 (12:31 +0100)]
Add isc_nm_read_stop() and remove .reading member from ccmsg
We need to stop reading when calling isc_ccmsg_disconnect() as the
reading handle doesn't have to be last because sending might be in
progress. After that, we can safely remove .reading member because the
reading would not be called after the disconnect has been called.
The ccmsg_senddone() should also not call the recv callback if the
sending failed, that's the job of the caller's send callback - in fact
it already does that, so the code in ccmsg_senddone() was superfluous.
To reduce memory pressure, we can add light per-loop (netmgr worker)
memory pools for isc_nmsocket_t structures. This will help in
situations where there's a lot of churn creating and destroying the
nmsockets.
Reduce the isc_nmsocket_t size from 1840 to 1208 bytes
Embedding isc_nmsocket_h2_t directly inside isc_nmsocket_t had increased
the size of isc_nmsocket_t to 1840 bytes. Making the isc_nmsocket_h2_t
to be a pointer to the structure and allocated on demand allows us to
reduce the size to 1208 bytes. While there are still some possible
reductions in the isc_nmsocket_t (embedded tlsstream, streamdns
structures), this was the far biggest drop in the memory usage.
Reduce struct isc__nm_uvreq size from 1560 to 560 bytes
The uv_req union member of struct isc__nm_uvreq contained libuv request
types that we don't use. Turns out that uv_getnameinfo_t is 1000 bytes
big and unnecessarily enlarged the whole structure. Remove all the
unused members from the uv_req union.
After removing sockaddr_unix from isc_sockaddr, we can also remove
sockaddr_storage and reduce the isc_sockaddr size from 152 bytes to just
48 bytes needed to hold IPv6 addresses.
Tom Krizek [Tue, 6 Feb 2024 09:21:45 +0000 (10:21 +0100)]
Support older junit XML format in test result processing
When running `make check` on a platform which has older (but still
supported) pytest, e.g. 3.4.2 on EL8, the junit to trs conversion would
fail because the junit format has different structure. Make the junit
XML processing more lenient to support both the older and newer junit
XML formats.
Tom Krizek [Tue, 6 Feb 2024 14:35:49 +0000 (15:35 +0100)]
Use a single local port for ditch.pl
The ditch.pl script is used to generate burst traffic without waiting
for the responses. When running other tests in parallel, this can result
in a ephemeral port clash, since the ditch.pl process closes the socket
immediately. In rare occasions when the message ID also clashes with
other tests' queries, it might result in an UnexpectedSource error from
dnspython.
Use a dedicated port EXTRAPORT8 which is reserved for each test as a
source port for the burst traffic.