Mark Andrews [Tue, 10 Jan 2023 06:15:09 +0000 (17:15 +1100)]
Don't perform arithmetic on NULL pointers
When node is NULL when calling getparent() et al. they return NULL
but performing arithmetic on the NULL pointer is undefined. Check
if 'node' or 'header' is NULL and skip the adjustment.
query.c:9394:26: warning: Dereference of null pointer [core.NullDereference]
if (!qctx->nxrewrite || qctx->rpz_st->m.rpz->addsoa) {
^~~~~~~~~~~~~~~~~~~
1 warning generated.
The warning above is for qctx->rpz_st potentially being a NULL pointer
when query_nxdomain() is called from query_resume(). This is a false
positive because none of the database lookup result codes currently
causing query_nxdomain() to be called (DNS_R_EMPTYWILD, DNS_R_NXDOMAIN)
can be returned by a database lookup following a recursive resolution
attempt. Add a NULL check nevertheless in order to future-proof the
code and silence Clang Static Analyzer.
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.
Tom Krizek [Wed, 16 Nov 2022 16:28:27 +0000 (17:28 +0100)]
Don't check algorithm support during configure step
The 9.16 version of ./configure calls bin/tests/system/cleanall.sh
unless --without-make-clean is used. The cleanall.sh script then
includes bin/tests/system/conf.sh, which includes
bin/tests/system/conf.sh.common. At that point, dnssec-keygen which is
used to detect algorithm support isn't compiled, so it can't be used.
More importantly, algorithm selection for system tests during the
./configure phase is irrelevant, so it can be safely skipped.
Tom Krizek [Wed, 16 Nov 2022 13:37:01 +0000 (14:37 +0100)]
Ensure test interpreters are defined before common config
Nothing from conf.sh.common is required to set these values. On the
contrary, a Python interpreter needs to be set in order to randomize the
algorithm set (which happens in conf.sh.common).
Tom Krizek [Mon, 7 Nov 2022 14:58:40 +0000 (15:58 +0100)]
Force quiet mode when using testcrypto.sh directly
When testcrypto.sh is used as a standalone script, always use quiet mode
to avoid using undefined commands (such as echo_i) which require
inclusion of the entire conf.sh machinery.
Tom Krizek [Mon, 31 Oct 2022 11:18:07 +0000 (12:18 +0100)]
ci: disable algorithm support checking in softhsm
The algorithm support detection script doesn't seem to work when using
the SoftHSM module. For some reason, dnssec-keygen returns 'crypto
failure'. Since the tests themselves pass, this is likely to be some
bug/definiency in the test scripts that check algorithm support that get
confused by SoftHSM.
Since this issue only happens for the system:gcc:softhsm2.6 job in the
9.16 branch, use a workaround to not introduce this new feature for
this particular problematic job.
Tom Krizek [Wed, 26 Oct 2022 14:20:57 +0000 (16:20 +0200)]
Randomize algorithm selection for mkeys test
Use the ALGORITHM_SET option to use randomly selected default algorithm
in this test. Make sure the test works by using variables instead of
hard-coding values.
Tom Krizek [Tue, 25 Oct 2022 15:45:16 +0000 (17:45 +0200)]
Script for random algorithm selection in system tests
Multiple algorithm sets can be defined in this script. These can be
selected via the ALGORITHM_SET environment variable. For compatibility
reasons, "stable" set contains the currently used algorithms, since our
system tests need some changes before being compatible with randomly
selected algorithms.
The script operation is similar to the get_ports.py - environment
variables are created and then printed out as `export NAME=VALUE`
commands, to be interpreted by shell. Once we support pytest runner for
system tests, this should be a fixture instead.
Tom Krizek [Tue, 25 Oct 2022 16:00:27 +0000 (18:00 +0200)]
Export env variables in system tests
Certain variables have to be exported in order for the system tests to
work. It makes little sense to export the variables in one place/script
while they're defined in another place.
Since it makes no harm, export all the variables to make the behaviour
more predictable and consistent. Previously, some variables were
exported as environment variables, while others were just shell
variables which could be used once the configuration was sourced from
another script. However, they wouldn't be exposed to spawned processes.
For simplicity sake (and for the upcoming effort to run system tests
with pytest), export all variables that are used. TESTS, PARALLEL_UNIX
and SUBDIRS variables are automake-specific, aren't used anywhere else
and thus not exported.
Tom Krizek [Tue, 25 Oct 2022 12:05:07 +0000 (14:05 +0200)]
Support testcrypto.sh usage without including conf.sh
The only variable really needed for the script to work is the path to
the $KEYGEN binary. Allow setting this via an environment variable to
avoid loading conf.sh (and causing a chicken-egg problem). Also make
testcrypto.sh executable to allow its use from conf.sh.
Tom Krizek [Fri, 14 Oct 2022 09:12:53 +0000 (11:12 +0200)]
Use common name convention for pytest files
It is better to use consistent file names to avoid issue with sorting
etc.
Using underscore in filenames as opposed to dash was chosen because it
seems more common in pytest/python to use underscore for filenames.
Also rename the bin/tests/system/timeouts/tests-tcp.py file to
bin/tests/system/timeouts/tests_tcp_timeouts.py to avoid pytest name
collision (there can't be two files named tests_tcp.py).
Tom Krizek [Thu, 15 Dec 2022 16:52:52 +0000 (17:52 +0100)]
danger: check backport commits for original commit IDs
A full backport must have all the commit from the original MR and the
original commit IDs must be referenced in the backport commit messages.
If the criteria above is not met, the MR should be marked as a partial
backport. In that case, any discrepencies are only logged as informative
messages rather than failures.
Tom Krizek [Thu, 15 Dec 2022 16:51:24 +0000 (17:51 +0100)]
danger: check that original MR has been merged
When checking a backport MR, ensure that the original MR has been merged
already. This is vital for followup checks that verify commit IDs from
original commits are present in backport commit messages.
Tom Krizek [Thu, 15 Dec 2022 16:48:34 +0000 (17:48 +0100)]
danger: check backport links to the original MR
When doing archeology, it is much easier to find stuff if it's properly
linked. This check ensures that backport MR are linked to their original
MR via a "Backport of !XXXX" message.
The regular expression is fairly broad and has been tested to accept the
following variants of the message:
Backport of MR !XXXX
Backport of: !XXXX
backport of mr !XXXX
Backport of !XXXX
Backport of https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/XXXX
Aram Sargsyan [Wed, 14 Dec 2022 14:40:31 +0000 (14:40 +0000)]
Fix logging a uint32_t SOA serial value in dns_catz_update_from_db()
The dns_catz_update_from_db() function prints serial number as a signed
number (with "%d" in the format string), but the `vers` variable's type
is 'uint32_t'. This breaks serials bigger than 2^31.
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ý [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).
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.
query.c:9394:26: warning: Dereference of null pointer [core.NullDereference]
if (!qctx->nxrewrite || qctx->rpz_st->m.rpz->addsoa) {
^~~~~~~~~~~~~~~~~~~
1 warning generated.
The warning above is for qctx->rpz_st potentially being a NULL pointer
when query_nxdomain() is called from query_resume(). This is a false
positive because none of the database lookup result codes currently
causing query_nxdomain() to be called (DNS_R_EMPTYWILD, DNS_R_NXDOMAIN)
can be returned by a database lookup following a recursive resolution
attempt. Add a NULL check nevertheless in order to future-proof the
code and silence Clang Static Analyzer.
projects / thirdparty / bind9.git / log
