Ondřej Surý [Tue, 13 Dec 2022 10:02:47 +0000 (11:02 +0100)]
Fix intermittent memory leak in dns_resolver unit
A rdataset could have been left unassociated on the error path in the
resume_dslookup() in the dns_resolver unit. Clone the rdataset after
the error check, so it's not cloned before we check whether we can make
further progress chasing DS records.
Mark Andrews [Tue, 13 Dec 2022 01:03:49 +0000 (12:03 +1100)]
Properly initialise local_ndata in isdotlocal in dig
Remove the trailing '\0' so that the length field of the dns_name_t
structure is correct. The old data just happens to work with
dns_name_issubdomain but would fail with dns_name_equal.
Ondřej Surý [Fri, 9 Dec 2022 07:53:20 +0000 (08:53 +0100)]
Implement proper reference counting for dns_keyfileio_t
Instead of relying on hash table search when using the keys, implement a
proper reference counting in dns_keyfileio_t objects, and attach/detach
the objects to the zone.
Ondřej Surý [Wed, 7 Dec 2022 15:45:33 +0000 (16:45 +0100)]
Release unused key file IO lock objects
Due to off-by-one error in zonemgr_keymgmt_delete, unused key file IO
lock objects were never freed and they were kept until the server
shutdown. Adjust the returned value by -1 to accomodate the fact that
the atomic_fetch_*() functions return the value before the operation and
not current value after the operation.
Mark Andrews [Mon, 21 Nov 2022 00:59:45 +0000 (11:59 +1100)]
Remove different zero TTL handling for rdataset iterator
Zero TTL handling does not need to be different for 'rdatasetiter_first'
and 'rdatasetiter_next' and it interacts badly with 'bind_rdatadataset'
which makes different determinations.
Ondřej Surý [Sun, 13 Nov 2022 10:04:30 +0000 (11:04 +0100)]
Remove isc_resource API and set limits directly in named_os unit
The only function left in the isc_resource API was setting the file
limit. Replace the whole unit with a simple getrlimit to check the
maximum value of RLIMIT_NOFILE and set the maximum back to rlimit_cur.
This is more compatible than trying to set RLIMIT_UNLIMITED on the
RLIMIT_NOFILE as it doesn't work on Linux (see man 5 proc on
/proc/sys/fs/nr_open), neither it does on Darwin kernel (see man 2
getrlimit).
The only place where the maximum value could be raised under privileged
user would be BSDs, but the `named_os_adjustnofile()` were not called
there before. We would apply the increased limits only on Linux and Sun
platforms.
Ondřej Surý [Sun, 13 Nov 2022 09:28:17 +0000 (10:28 +0100)]
Mark setting operating system limits from named.conf as ancient
After deprecating the operating system limits settings (coresize,
datasize, files and stacksize), mark them as ancient and remove the code
that sets the values from config.
Ondřej Surý [Thu, 3 Nov 2022 16:42:12 +0000 (17:42 +0100)]
Propagate the shutdown event to the recursing ns_client(s)
Send the ns_query_cancel() on the recursing clients when we initiate the
named shutdown for faster shutdown.
When we are shutting down the resolver, we cancel all the outstanding
fetches, and the ISC_R_CANCEL events doesn't propagate to the ns_client
callback.
In the future, the better solution how to fix this would be to look at
the shutdown paths and let them all propagate from bottom (loopmgr) to
top (f.e. ns_client).
Ondřej Surý [Tue, 6 Dec 2022 14:59:35 +0000 (15:59 +0100)]
Fix reference counting in get_attached_entry
When get_attached_entry() encounters entry that would be expired, it
needs to get reference to the entry before calling maybe_expire_entry(),
so the ADB entry doesn't get destroyed inside the its own lock.
Michal Nowak [Tue, 22 Nov 2022 09:27:17 +0000 (10:27 +0100)]
Add ASAN- and TSAN-enabled respdiff jobs
Neither of the new CI jobs can reliably pass at the moment; hence they
are defined with "allow_failure: true" until issues in the code base are
resolved.
Mark Andrews [Wed, 30 Nov 2022 08:32:11 +0000 (19:32 +1100)]
Check that restored catalog zone works
Using a restored catalog zone excercised a use-after-free bug.
The test checks that the use-after-free bug is gone and is just
a reasonable behaviour check in its own right.
Mark Andrews [Wed, 30 Nov 2022 07:44:37 +0000 (18:44 +1100)]
Call dns_db_updatenotify_unregister earlier
dns_db_updatenotify_unregister needed to be called earlier to ensure
that listener->onupdate_arg always points to a valid object. The
existing lazy cleanup in rbtdb_free did not ensure that.
Matthijs Mekking [Thu, 17 Nov 2022 13:52:26 +0000 (13:52 +0000)]
Consider non-stale data when in serve-stale mode
With 'stale-answer-enable yes;' and 'stale-answer-client-timeout off;',
consider the following situation:
A CNAME record and its target record are in the cache, then the CNAME
record expires, but the target record is still valid.
When a new query for the CNAME record arrives, and the query fails,
the stale record is used, and then the query "restarts" to follow
the CNAME target. The problem is that the query's multiple stale
options (like DNS_DBFIND_STALEOK) are not reset, so 'query_lookup()'
treats the restarted query as a lookup following a failed lookup,
and returns a SERVFAIL answer when there is no stale data found in the
cache, even if there is valid non-stale data there available.
With this change, query_lookup() now considers non-stale data in the
cache in the first place, and returns it if it is available.
Artem Boldariev [Tue, 22 Nov 2022 13:11:57 +0000 (15:11 +0200)]
TLS: check for sock->recv_cb when handling received data
This commit adds a check if 'sock->recv_cb' might have been nullified
during the call to 'sock->recv_cb'. That could happen, e.g. by an
indirect call to 'isc_nmhandle_close()' from within the callback when
wrapping up.
Artem Boldariev [Fri, 2 Dec 2022 08:41:28 +0000 (10:41 +0200)]
DoH: Avoid accessing non-atomic listener socket flags when accepting
This commit ensures that the non-atomic flags inside a DoH listener
socket object (and associated worker) are accessed when doing accept
for a connection only from within the context of the dedicated thread,
but not other worker threads.
Artem Boldariev [Thu, 1 Dec 2022 20:42:54 +0000 (22:42 +0200)]
TLS: Avoid accessing non-atomic listener socket flags during HS
This commit ensures that the non-atomic flags inside a TLS listener
socket object (and associated worker) are accessed when doing
handshake for a connection only from within the context of the
dedicated thread, but not other worker threads.
Tom Krizek [Wed, 30 Nov 2022 16:44:27 +0000 (17:44 +0100)]
Add libnghttp2 prerequisite for doth system test
While some of these tests are for DoT which doesn't require nghttp2,
the server configs won't allow the server to start without nghttp2
support during compile time.
It might be possible to split these tests into DoT and DoH and only
require nghttp2 for DoH tests, but since almost all of our CI jobs are
compiled with nghttp2, we wouldn't gain a lot of coverage, so it's
probably not worth the effort.
Tom Krizek [Thu, 1 Dec 2022 14:12:28 +0000 (15:12 +0100)]
Use feature-test to detect feature support in system tests
Previously, there were two different ways to detect feature support.
Either through an environment variable set by configure in conf.sh, or
using the feature-test utility.
It is more simple and consistent to have only one way of detecting the
feature support. Using the feature-test utility seems superior the the
environment variables set by configure.
Artem Boldariev [Wed, 30 Nov 2022 20:58:13 +0000 (22:58 +0200)]
TLS: Avoid accessing listener socket flags from other threads
This commit ensures that the flags inside a TLS listener socket
object (and associated worker) are accessed when accepting a
connection only from within the context of the dedicated thread, but
not other worker threads.
Ondřej Surý [Thu, 1 Dec 2022 17:31:05 +0000 (18:31 +0100)]
Honour single read per client isc_nm_read() call in the TLSDNS
The TLSDNS transport was not honouring the single read callback for
TLSDNS client. It would call the read callbacks repeatedly in case the
single TLS read would result in multiple DNS messages in the decoded
buffer.
Ondřej Surý [Fri, 4 Nov 2022 14:04:29 +0000 (15:04 +0100)]
Refactor the dns_resolver fetch context hash tables and locking
This is second in the series of fixing the usage of hashtables in the
dns_adb and the dns_resolver units.
Currently, the fetch buckets (used to hold the fetch context) and zone
buckets (used to hold per-domain counters) would never get cleaned from
the memory. Combined with the fact that the hashtable now grows as
needed (instead of using hashtable as buckets), the memory usage in the
resolver can just grow and it never drops down.
In this commit, the usage of hashtables (hashmaps) has been completely
rewritten, so there are no "buckets" and all the matching conditions are
directly mapped into the hashtable key:
1. For per-domain counter hashtable, this is simple as the lowercase
domain name is used directly as a counter.
2. For fetch context hashtable, this requires copying some extra flags
back and forth in the key.
As we don't hold the "buckets" forever, the cleaning mechanism has been
rewritten as well:
1. For per-domain counter hashtable, this is again much simpler, as we
only need to check whether the usage counter is still zero under the
lock and bail-out on cleaning if the counter is in use.
2. For fetch context hashtable, this is more complicated as the fetch
context cannot be reused after it has been finished. The algorithm
is different, the fetch context is always removed from the
hashtable, but if we find the fetch context that has been marked
as finished in the lookup function, we help with the cleaning from
the hashtable and try again.
Couple of additional changes have been implemented in this refactoring
as those were needed for correct functionality and could not be split
into individual commits (or would not make sense as seperate commits):
1. The dns_resolver_createfetch() has an option to create "unshared"
fetch. The "unshared" fetch will never get matched, so there's
little point in storing the "unshared" fetch in the hashtable.
Therefore the "unshared" fetches are now detached from the
hashtable and live just on their own.
2. Replace the custom reference counting with ISC_REFCOUNT_DECL/IMPL
macros for better tracing.
3. fctx_done_detach() is idempotent, it makes the "final" detach (the
one matching the create function) only once. But that also means
that it has to be called before the detach that kept the fetch
context alive in the callback. A new macro fctx_done_unref() has
been added to allow this code flow:
Doing this the other way around could cause fctx to get destroyed in
the fctx_unref() first and fctx_done_detach() would cause UAF.
4. The resume_qmin() and resume_dslookup() callbacks have been
refactored for more readability and simpler code paths. The
validated() callback has also received some of the simplifications,
but it should be refactored in the future as it is bit of spaghetti
now.
This commit ensures that send callbacks are always called from within
the context of its worker thread even in the case of
shuttigdown/inactive socket, just like TCP transport does and with
which TLS attempts to be as compatible as possible.