Ondřej Surý [Tue, 6 Jun 2023 10:48:23 +0000 (12:48 +0200)]
Fix extra detach when dns_validator create_fetch() detects deadlock
When create_fetch() in the dns_validator unit detects deadlock, it
returns DNS_R_NOVALIDSIG, but it didn't attach to the validator. The
other condition to returning result != ISC_R_SUCCESS would be error from
dns_resolver_createfetch(). The caller (in two places out of three)
would detect the error condition and always detach from the validator.
Move the dns_validator_detach() on dns_resolver_createfetch() error
condition to create_fetch() function and cleanup the extra detaches in
seek_dnskey() and get_dsset().
Artem Boldariev [Fri, 2 Jun 2023 11:28:50 +0000 (14:28 +0300)]
Use appropriately sized send buffers for DNS messages over TCP
This commit changes send buffers allocation strategy for stream based
transports. Before that change we would allocate a dynamic buffers
sized at 64Kb even when we do not need that much. That could lead to
high memory usage on server. Now we resize the send buffer to match
the size of the actual data, freeing the memory at the end of the
buffer for being reused later.
Mark Andrews [Wed, 31 May 2023 02:40:37 +0000 (12:40 +1000)]
Use rcu methods to lock access view->zonetable
dns_view_find* may be called after the final call to dns_view_detach
is made which detaches view->zonetable to permit the server to
shutdown. We need to detect if view->zonetable is NULL during this
stage and appropriately recover.
Ondřej Surý [Thu, 1 Jun 2023 11:38:42 +0000 (13:38 +0200)]
Disable URCU inlining if inlined rcu_dereference() fails to compile
In some cases, the inlined version rcu_dereference() would not compile
when working on pointer to opaque struct (namely Ubuntu Jammy). Detect
such condition in the autoconf and disable the inlining of the small
functions if it breaks the build.
Aram Sargsyan [Sat, 27 May 2023 11:01:28 +0000 (11:01 +0000)]
Fix a clients-per-query miscalculation bug
The number of clients per query is calculated using the pending
fetch responses in the list. The dns_resolver_createfetch() function
includes every item in the list when deciding whether the limit is
reached (i.e. fctx->spilled is true). Then, when the limit is reached,
there is another calculation in fctx_sendevents(), when deciding
whether it is needed to increase the limit, but this time the TRYSTALE
responses are not included in the calculation (because of early break
from the loop), and because of that the limit is never increased.
A single client can have more than one associated response/event in the
list (currently max. two), and calculating them as separate "clients"
is unexpected. E.g. if 'stale-answer-enable' is enabled and
'stale-answer-client-timeout' is enabled and is larger than 0, then
each client will have two events, which will effectively halve the
clients-per-query limit.
Fix the dns_resolver_createfetch() function to calculate only the
regular FETCHDONE responses/events.
Change the fctx_sendevents() function to also calculate only FETCHDONE
responses/events. Currently, this second change doesn't have any impact,
because the TRYSTALE events were already skipped, but having the same
condition in both places will help prevent similar bugs in the future
if a new type of response/event is ever added.
Aram Sargsyan [Mon, 29 May 2023 17:34:13 +0000 (17:34 +0000)]
Light refactoring of the fetchlimit system test
Prepare the fetchlimit system test for adding a clients-per-query
check. Change some functions and commands to accept a destination
NS IP address instead of using the hardcoded 10.53.0.3.
Remove the code implementing nonstardard behaviors that were formerly
needed to allow GSS-TSIG to work with Windows 2000, which passed
End-of-Life in 2010.
Deprecate the "oldgsstsig" command and "-o" command line option
to nsupdate; these are now treated as synonyms for "gsstsig" and "-g"
respectively.
Michal Nowak [Fri, 26 May 2023 08:50:58 +0000 (10:50 +0200)]
Change images for TSAN jobs
Fedora 38 and Debian "bullseye" images were "forked" to images used only
for TSAN CI jobs. The new images contain TSAN-aware liburcu that does
not fit well with ASAN CI jobs for which original images were also used.
Also, drop liburcu-related TSAN suppressions because they are
unnecessary with TSAN-aware liburcu.
Michal Nowak [Wed, 26 Apr 2023 09:21:28 +0000 (11:21 +0200)]
Look for core files in $TOP_BUILDDIR
The get_core_dumps.sh script couldn't find and process core files of
out-of-tree configurations because it looked for them in the source
instead of the build directory.
We recently fixed a bug where in some cases (when following an
expired CNAME for example), named could return SERVFAIL if the target
record is still valid (see isc-projects/bind9#3678, and
isc-projects/bind9!7096). We fixed this by considering non-stale
RRsets as well during the stale lookup.
However, this triggered a new bug because despite the answer from
cache not being stale, the lookup may be triggered by serve-stale.
If the answer from database is not stale, the fix in
isc-projects/bind9!7096 erroneously skips the serve-stale logic.
Add 'answer_found' checks to the serve-stale logic to fix this issue.
Add a test case where when priming the cache with a slow authoritative
resolver, the stale-answer-client-timeout option should not return
a delegation to the client (it should wait until an applicable answer
is found, if no entry is found in the cache).
Since we don't use networking directly but rather via libuv, these
configure options were no-op. Remove the configure checks for epoll
(Linux), kqueue (BSDs) and /dev/poll (Solaris).
Mark Andrews [Fri, 26 May 2023 01:09:33 +0000 (11:09 +1000)]
Add regression test for [GL # 4090]
These insertions are added to produce a radix tree that will trigger
the INSIST reported in [GL #4090]. Due to fixes added since BIND 9.9
an extra insert in needed to ensure node->parent is non NULL.
Mark Andrews [Fri, 26 May 2023 00:28:39 +0000 (10:28 +1000)]
Move isc_mem_put to after node is checked for equality
isc_mem_put NULL's the pointer to the memory being freed. The
equality test 'parent->r == node' was accidentally being turned
into a test against NULL.
This commit ensures that access to the TLS context cache within zone
manager is properly synchronised.
Previously there was a possibility for it to get unexpectedly
NULLified for a brief moment by a call to
dns_zonemgr_set_tlsctx_cache() from one thread, while being accessed
from another (e.g. from got_transfer_quota()). This behaviour could
lead to server abort()ing on configuration reload (under very rare
circumstances).
Tom Krizek [Wed, 24 May 2023 11:58:01 +0000 (13:58 +0200)]
Disable rrl check in slow environments
The check for 'would limit' log message is triggered by sending at least
three messages within one second. However, in extremely slow conditions
(currently when running with clang+TSAN in CI), the individual queries
might take too much time to send enough of them within one second.
Since this is a pretty rare condition, let's just silently skip this
test in environments where a single query takes more than 500 ms, since
there's no way to perform the check under such conditions.
Evan Hunt [Tue, 16 May 2023 22:35:00 +0000 (15:35 -0700)]
fix handling of TCP timeouts
when a TCP dispatch times out, we call tcp_recv() with a result
value of ISC_R_TIMEDOUT; this cancels the oldest dispatch
entry in the dispatch's active queue, plus any additional entries
that have waited longer than their configured timeouts. if, at
that point, there were more dispatch entries still on the active
queue, it resumes reading, but until now it failed to restart
the timer.
this has been corrected: we now calculate a new timeout
based on the oldest dispatch entry still remaining. this
requires us to initialize the start time of each dispatch entry
when it's first added to the queue.
in order to ensure that the handling of timed-out requests is
consistent, we now calculate the runtime of each dispatch
entry based on the same value for 'now'.
incidentally also fixed a compile error that turned up when
DNS_DISPATCH_TRACE was turned on.
Evan Hunt [Sun, 21 May 2023 19:59:38 +0000 (12:59 -0700)]
prevent TSIG keys from being added to multiple rings
it was possible to add a TSIG key to more than one TSIG
keyring at a time, and this was in fact happening with the
session key, which was generated once and then added to the
keyrings for each view as it was configured.
this has been corrected and a REQUIRE added to dns_tsigkeyring_add()
to prevent it from happening again.
Aram Sargsyan [Wed, 24 May 2023 14:26:04 +0000 (14:26 +0000)]
Fix an interfacemgr use-after-free error in zoneconf.c:isself()
The 'named_g_server->interfacemgr' pointer is saved in the zone
structure using dns_zone_setisself(), as a void* argument to be
passed to the isself() callback, so there is no attach/detach,
and when shutting down, the interface manager can be destroyed
by the shutdown_server(), running in exclusive mode, and causing
isself() to crash when trying to use the pointer.
Instead of keeping the interface manager pointer in the zone
structure, just check and use the 'named_g_server->interfacemgr'
itself, as it was implemented originally in the 3aca8e5bf3740bbcc3bb13dde242d7cc369abb27 commit. Later, in the 8eb88aafee951859264e36c315b1289cd8c2088b commit, the code was
changed to pass the interface manager pointer using the additional
void* argument, but the commit message doesn't mention if there
was any practical reason for that.
Additionally, don't pass the interfacemgr pointer to the
ns_interfacemgr_getaclenv() function before it is checked
against NULL.
The 'update-nsec3.example' requires to be DNSSEC maintained via
dynamic update. Commit 03b22983cd20cec51ad8b9f25f2e7d0e472dc79c adds
checks to make sure the raw zone is not signed. So the test case neesd
to be updated to allow for DNSSEC maintenance.
A zone in multisigner model 2 should also be possible to remove
previously added DNSKEY, CDS and CDNSKEY records from the zone operated
by the other provider.
Add a test case where updates are being made against a hidden primary
and two bump in the wire signers (the providers in the multisigner
model) serve the zone.
The test covers the same cases as for two primary providers that is:
- Add DNSKEY
- Remove (previously added) DNSKEY
- Add CDNSKEY
- Remove (previously added) CDNSKEY
- Add CDS
- Remove (previously added) CDS
Mark Andrews [Wed, 12 Oct 2022 06:01:57 +0000 (17:01 +1100)]
Don't sign the raw zone
The raw zone is not supposed to be signed. DNSKEY records in a raw zone
should not trigger zone signing. The update code needs to be able to
identify when it is working on a raw zone. Add dns_zone_israw() and
dns_zone_issecure() enable it to do this. Also, we need to check the
case for 'auto-dnssec maintain'.
A zone in multisigner model 2 should also be possible to publish the
CDS and CDNSKEY records from their KSK into the zone operated by the
other provider.
For inline-signing zones, sometimes kasp was not detected because
the function was called on the raw (unsigned) version of the zone,
but the kasp is only set on the secure (signed) version of the zone.
Fix the dns_zone_getkasp() function to check whether the zone
structure is inline_raw(), and if so, use the kasp from the
secure version.
In zone.c we can access the kasp pointer directly.
When synchronizing the journal or database from the unsigned version of
the zone to the secure version of the zone, allow DNSKEY records to be
synced, because these may be added by the user with the sole intent to
publish the record (not used for signing). This may be the case for
example in the multisigner model 2 (RFC 8901).
Additional code needs to be added to ensure that we do not remove DNSKEY
records that are under our control. Keys under our control are keys that
are used for signing the zone and thus that we have key files for.
Same counts for CDNSKEY and CDS (records that are derived from keys).
Add a new system test to test multisigner model use cases. This
initial test just tests a small part of the model 2, and uses two
providers for the same zone, ns3 and ns4, each with their own unique
key set. This commit tests that each provider can import their ZSK
of the other provider into their DNSKEY RRset, using dynamic update.
Both providers use dnssec-policy, ns3 applies the DNSSEC records
directly, while ns4 uses inline-signing.
Mark Andrews [Wed, 10 May 2023 04:50:59 +0000 (14:50 +1000)]
Silence Coverity USE_AFTER_FREE warning
Use current used pointer - 16 instead of a saved pointer as Coverity
thinks the memory may be freed between assignment and use of 'cp'.
isc_buffer_put{mem,uint{8,16,32}} can theoretically free the memory
if there is a dynamic buffer in use but that is not the case here.
Tom Krizek [Mon, 22 May 2023 12:20:29 +0000 (14:20 +0200)]
Reorder dead primary checks in upforwd test
The check which attempts to forward dynamic update to a dead primary may
trigger a timing issue #4080. For some reason, this has manifested under
the pytest runner, while the test still passes with the legacy runner.
Move the dead primary check closer to the end of the test to avoid
hitting this issue before we have a proper fix.
Tom Krizek [Tue, 9 May 2023 12:20:02 +0000 (14:20 +0200)]
Tear down module logger handler in system tests
The module-level logger has a handler that writes into a temporary
directory. Ensure the logging output is flushed and the handler is
closed before attempting to remove this temporary directory.
Tom Krizek [Tue, 9 May 2023 11:32:13 +0000 (13:32 +0200)]
Rewrite run.sh to invoke pytest in a system test directory
Previously, run.sh tried to use pytest's -k option for test selection.
The downside was that this filter expression matched any test case with
the given substring, rather than executing a system test suite with the
given name.
The run.sh has been rewritten to invoke pytest from a system test
directory instead. This behaves more consistently with the run.sh from
legacy system test framework.
run.sh is now also a shell script to avoid confusion regarding its
file extension.
Tom Krizek [Fri, 14 Apr 2023 12:40:51 +0000 (14:40 +0200)]
Add .log.txt to gitignore
It can be useful to append the .txt extension to logs. When this
extension is used, GitLab is able to set the proper content type on such
artifacts in CI. This makes it possible to display those files directly
in the browser rather than having to download them.
Tom Krizek [Wed, 12 Apr 2023 15:29:26 +0000 (17:29 +0200)]
Remove "which" declaration from env vars in EL8+ tests
EL8+ systems declare "which" function using environment variables in the
/etc/profile.d/which2.sh file. Because of our suboptimal environment
variable detection, which is required in order to support the legacy
runner, these variables are picked up by the pytest runner.
If subprocesses are spawned with these environment variables set, it
will cause the following issue when they spawn yet another subprocess:
/bin/sh: which: line 1: syntax error: unexpected end of file
/bin/sh: error importing function definition for `which'
Tom Krizek [Wed, 12 Apr 2023 12:22:27 +0000 (14:22 +0200)]
Capture log output during pytest runner setup
Instantiate a new logger that is used during pytest initialization /
configuration. This logging isn't handled by pytest itself, since it
happens outside of any tests or fixtures.
Root logger can't be reused for this purpose, because that would
duplicate the logs. Instead, create a conftest-specific logger for this
purpose.
Unfortunately, this introduces another log file,
pytest.conftest.log.txt, which contains only the logging from pytest
initialization. However, unless one is debugging the runner /
environment, there should be no need to investigate this file.
Tom Krizek [Wed, 5 Apr 2023 10:39:56 +0000 (12:39 +0200)]
Execute long running system tests first
In order to take the most advantage of parallel execution of tests,
ensure certain long running tests are scheduled first.
The list of tests considered long-running was created empirically. In
addition to the test run time, its position in the default
(alphabetical) ordering was also taken into account.
Tom Krizek [Tue, 4 Apr 2023 15:44:10 +0000 (17:44 +0200)]
Add test specific logger for pytests
The logger fixture is provided as a test-level logging facility which
can be easily passed to tests to enable capturing and/or displaying
messages from tests written in Python.
While this works optimally with the pytest runner, messages on INFO
level or above will also be visible when using the legacy runner.
projects / thirdparty / bind9.git / log
