Michal Nowak [Mon, 5 May 2025 10:00:01 +0000 (10:00 +0000)]
chg: ci: Disable linkcheck on www.gnu.org
The check fails with the following error for some time:
broken https://www.gnu.org/software/libidn/#libidn2 - HTTPSConnectionPool(host='www.gnu.org', port=443): Max retries exceeded with url: /software/libidn/ (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x7f5bd4c14590>: Failed to establish a new connection: [Errno 111] Connection refused'))
Merge branch 'mnowak/linkcheck-disable-www-gnu-org' into 'main'
Michal Nowak [Mon, 5 May 2025 09:50:03 +0000 (11:50 +0200)]
Disable linkcheck on www.gnu.org
The check fails with the following error for some time:
broken https://www.gnu.org/software/libidn/#libidn2 - HTTPSConnectionPool(host='www.gnu.org', port=443): Max retries exceeded with url: /software/libidn/ (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x7f5bd4c14590>: Failed to establish a new connection: [Errno 111] Connection refused'))
Mark Andrews [Fri, 2 May 2025 07:09:56 +0000 (07:09 +0000)]
fix: dev: fix the ksr two-tone test
The two-tone ksr subtest (test_ksr_twotone) depended on the dnssec-policy keys algorithm values in named.conf being entered in numerical order. As the algorithms used in the test can be selected randomly this does not always happen. Sort the dnssec-policy keys by algorithm when adding them to the key list from named.conf.
Closes #5286
Merge branch '5286-ksr-two-tone-test-only-work-by-luck' into 'main'
Mark Andrews [Tue, 29 Apr 2025 06:59:15 +0000 (16:59 +1000)]
Don't depend on keys being sorted
Extract each section of the bundle and check that the expected
records are there. The old code was assuming that the records in
each section where in a particular order which didn't happen in
practice.
Over the past few years, some of the initial decisions made about which
GitLab CI jobs to run for all merge requests and which of them to run
just for scheduled/web-triggered pipelines turned out to be less than
ideal in practice: test coverage was found to be too lax in some areas
and on the other hand unnecessarily repetitive in others. For example,
compilation failures for certain build types that are not exercised for
every merge request (e.g. FIPS-enabled builds) turned out to be much
more common in practice than e.g. test failures happening only on a
subset of releases of a given Linux distribution.
To limit excessive resource use while retaining broad test coverage,
adjust GitLab CI job triggering rules for merge request pipelines as
follows:
- run all possible build jobs for every merge request; compilation
failures triggered for build flavors that were only tested in
scheduled pipelines turned out to be surprisingly commonplace and
became a nuisance over time, particularly given that the run times
of build jobs are much lower than those of test jobs,
- for every merge request, run at least one system & unit test job for
each build flavor (e.g. sanitizer-enabled, FIPS-enabled,
out-of-tree, tarball-based, etc.),
- limit the amount of test jobs run for each distinct operating
system; for example, only run system & unit test jobs for Ubuntu
24.04 Noble Numbat in merge request pipelines, skipping those for
Ubuntu 22.04 Jammy Jellyfish and Ubuntu 20.04 Focal Fossa (while
still running them in other pipeline types, e.g. in scheduled
pipelines),
- ensure every merge request is tested on Oracle Linux 8, which is the
operating system with the oldest package versions out of the systems
that are still supported by this BIND 9 branch,
- decrease the number of test jobs run with sanitizers enabled while
still testing with both ASAN and TSAN and both GCC and Clang for
every merge request.
These changes do not affect the set of jobs created for any other
pipeline type (triggered by a schedule, by a GitLab API call, by the web
interface, etc.); only merge request pipelines are affected.
---
Since understanding the impact of this MR just by looking at the diff is
arguably challenging, I prepared some tables showing which jobs are
currently triggered for every merge request and what the new state of
things will be after this MR gets merged.
**Legend:**
- :chart_with_upwards_trend: - job was *not* run for every merge
request before, but will be
- :chart_with_downwards_trend: - job was run for every merge request
before, but will *not* be any longer
Over the past few years, some of the initial decisions made about which
GitLab CI jobs to run for all merge requests and which of them to run
just for scheduled/web-triggered pipelines turned out to be less than
ideal in practice: test coverage was found to be too lax in some areas
and on the other hand unnecessarily repetitive in others. For example,
compilation failures for certain build types that are not exercised for
every merge request (e.g. FIPS-enabled builds) turned out to be much
more common in practice than e.g. test failures happening only on a
subset of releases of a given Linux distribution.
To limit excessive resource use while retaining broad test coverage,
adjust GitLab CI job triggering rules for merge request pipelines as
follows:
- run all possible build jobs for every merge request; compilation
failures triggered for build flavors that were only tested in
scheduled pipelines turned out to be surprisingly commonplace and
became a nuisance over time, particularly given that the run times
of build jobs are much lower than those of test jobs,
- for every merge request, run at least one system & unit test job for
each build flavor (e.g. sanitizer-enabled, FIPS-enabled,
out-of-tree, tarball-based, etc.),
- limit the amount of test jobs run for each distinct operating
system; for example, only run system & unit test jobs for Ubuntu
24.04 Noble Numbat in merge request pipelines, skipping those for
Ubuntu 22.04 Jammy Jellyfish and Ubuntu 20.04 Focal Fossa (while
still running them in other pipeline types, e.g. in scheduled
pipelines),
- ensure every merge request is tested on Oracle Linux 8, which is the
operating system with the oldest package versions out of the systems
that are still supported by this BIND 9 branch,
- decrease the number of test jobs run with sanitizers enabled while
still testing with both ASAN and TSAN and both GCC and Clang for
every merge request.
These changes do not affect the set of jobs created for any other
pipeline type (triggered by a schedule, by a GitLab API call, by the web
interface, etc.); only merge request pipelines are affected.
Michal Nowak [Tue, 29 Apr 2025 10:55:28 +0000 (10:55 +0000)]
rem: ci: Drop OpenBSD from the CI
With the ongoing process of moving CI workloads to AWS, OpenBSD poses a
challenge, as there is no OpenBSD AMI image in the AWS catalog. Building
our image from scratch is disproportionately complicated, given that
OpenBSD is not a common deployment platform for BIND 9. Otherwise,
OpenBSD stays at the "Best-Effort" level of support.
Merge branch 'mnowak/drop-openbsd-from-ci' into 'main'
Michal Nowak [Wed, 9 Apr 2025 12:05:42 +0000 (14:05 +0200)]
Drop OpenBSD from the CI
With the ongoing process of moving CI workloads to AWS, OpenBSD poses a
challenge, as there is no OpenBSD AMI image in the AWS catalog. Building
our image from scratch is disproportionately complicated, given that
OpenBSD is not a common deployment platform for BIND 9. Otherwise,
OpenBSD stays at the "Best-Effort" level of support.
fix: dev: Call rcu_barrier earlier in the destructor
If a call_rcu thread is running, there is a possible race condition
where the destructors run before all call_rcu callbacks have finished
running. This can happen, for example, if the call_rcu callback tries to
log something after the logging context has been torn down.
In !10394, we tried to counter this by explicitely creating a call_rcu
thread an shutting it down before running the destructors, but it is
possible for things to "slip" and end up on the default call_rcu thread.
As a quickfix, this commit moves an rcu_barrier() that was in the mem
context destructor earlier, so that it "protects" all libisc
destructors.
Closes #5296
Merge branch '5296-join-rcu-thread-on-shutdown' into 'main'
Your Name [Fri, 25 Apr 2025 08:41:29 +0000 (10:41 +0200)]
Call rcu_barrier earlier in the destructor
If a call_rcu thread is running, there is a possible race condition
where the destructors run before all call_rcu callbacks have finished
running. This can happen, for example, if the call_rcu callback tries to
log something after the logging context has been torn down.
In !10394, we tried to counter this by explicitely creating a call_rcu
thread an shutting it down before running the destructors, but it is
possible for things to "slip" and end up on the default call_rcu thread.
As a quickfix, this commit moves an rcu_barrier() that was in the mem
context destructor earlier, so that it "protects" all libisc
destructors.
These tests do not easily fit in the standard test case framework, so they go into their own suite.
- zsk retired case
- checkds cases
- reload/restart
- inheritance tests
Merge branch 'matthijs-pytest-rewrite-kasp-system-test-4' into 'main'
Matthijs Mekking [Mon, 17 Mar 2025 16:16:29 +0000 (17:16 +0100)]
Convert kasp inheritance tests
These tests ensure that if dnssec-policy is set on a higher level, the
zone is still signed (or unsigned) as expected. Or if a higher level
has an override, the new policy is honored as expected.
The new `tcp-primaries-timeout` configuration option works the same way
as the older `tcp-initial-timeout` option, but applies only to the TCP
connections made to the primary servers, so that the timeout value can
be set separately for them. By default, it's set to 150, which is 15
seconds.
Closes #3649
Merge branch '3649-configurable-xfr-tcp-timeouts' into 'main'
Aram Sargsyan [Tue, 11 Mar 2025 12:24:43 +0000 (12:24 +0000)]
Fix delv default timeout value
The isc_nm_getinitialtimeout() function (and also the previously used
isc_nm_gettimeouts() function) returns timeout value(s) in milliseconds,
while the dns_request_create() function expects timeout values in
seconds. Fix the bug by dividing the timeout value by MS_PER_SEC.
There is no added test, because it turns out delv doesn't support
setting custom timeout values (as opposed to what is suggested in
its man page). Tests should be added later when the '+timeout=T'
option is implemented.
Aram Sargsyan [Tue, 11 Mar 2025 12:10:51 +0000 (12:10 +0000)]
Separate the single setter/getter functions for TCP timeouts
Previously all kinds of TCP timeouts had a single getter and setter
functions. Separate each timeout to its own getter/setter functions,
because in majority of cases only one is required at a time, and it's
not optimal expanding those functions every time a new timeout value
is implemented.
Aram Sargsyan [Tue, 18 Feb 2025 19:13:35 +0000 (19:13 +0000)]
Fix the notify system test after the newly applied timeout value
Since notify messages now use the configured 'tcp-initial-timeout'
connect timeout value, the existing "checking notify retries expire
within 30 seconds" check in the "notify" system test is failing. Set
the 'tcp-initial-timeout' option for ns3 to the previously hardcoded
value of 15 seconds for the test to pass successfully.
Aram Sargsyan [Tue, 18 Feb 2025 17:13:39 +0000 (17:13 +0000)]
Use the configured TCP connect timeout in checkds_send_toaddr()
The checkds_send_toaddr() function uses hardcoded timeout values
for both UDP and TCP, however, with TCP named has configurable
timeout values. Slightly refactor the timeouts calculation part
and use the configured 'tcp-initial-timeout' value as the connect
timeout.
Aram Sargsyan [Tue, 18 Feb 2025 17:07:15 +0000 (17:07 +0000)]
Use the configured TCP connect timeout in notify_send_toaddr()
The notify_send_toaddr() function uses hardcoded timeout values
for both UDP and TCP, however, with TCP named has configurable
timeout values. Slightly refactor the timeouts calculation part
and use the configured 'tcp-initial-timeout' value as the connect
timeout.
Aram Sargsyan [Tue, 18 Feb 2025 14:44:29 +0000 (14:44 +0000)]
Implement tcp-primaries-timeout
The new 'tcp-primaries-timeout' configuration option works the same way
as the existing 'tcp-initial-timeout' option, but applies only to the
TCP connections made to the primary servers, so that the timeout value
can be set separately for them. The default is 15 seconds.
Also, while accommodating zone.c's code to support the new option, make
a light refactoring with the way UDP timeouts are calculated by using
definitions instead of hardcoded values.
Matthijs Mekking [Mon, 17 Mar 2025 14:32:43 +0000 (15:32 +0100)]
Convert keystore and rumoured kasp test cases
For 'keystore.kasp', a setting 'key-directories' is used. If set, this
will expect a list of two directories, the first one is where the KSKs
will be stored, the second in the list is the ZSK key directory. This
may be expanded in the future to test more complex key storage cases.
The 'rumoured.kasp' zone is weird, the key timings can never match
those key states. But it is a regression test for an early day bug,
so we convert it, but skip the expected key times check.
Matthijs Mekking [Mon, 17 Mar 2025 13:53:55 +0000 (14:53 +0100)]
Convert more kasp test cases to pytest
These test cases follow the same pattern as many other, but all require
some additional checks. These are set in "additional-tests".
The "zsk-missing.autosign" zone is special handled, as it expects the
KSK to sign the SOA RRset (because the ZSK is unavailable).
The kasp/ns3/setup.sh script is updated so the SyncPublish is not set
(named will initialize it correctly). For the test zones that have
missing private key files we do need to set the expected key timing
metadata.
Remove the counterparts for the newly added test from the kasp shell
tests script.
Matthijs Mekking [Mon, 17 Mar 2025 10:52:18 +0000 (11:52 +0100)]
Update kasp check_signatures for dnssec-policy
The check_signatures code was initially created to be suitable for
the ksr system test, to test the Offline KSK feature. For that, a
key is expected to be signing if the current time is between
the timing metadata Active and Retired.
With dnssec-policy, the key timing metadata is indicative, the key
states determine the actual signing behavior.
Update the check_signatures function so that by default the signing
is derived from the key states (ksigning and zsigning). Add an
argument 'offline_ksk', if set the make sure that the zsigning is set
if the current time is between the Active and Retired timing metadata,
and for ksigning we just use the timing metadata (as the key is offline,
we cannot check the key states).
Another (upcoming) test case is where key files are missing. When the
ZSK private key file is missing, the KSK takes over. Add an argument
'zsk_missing', when set to True the expected zone signing (zsigning)
is reversed.
Matthijs Mekking [Fri, 14 Mar 2025 16:28:28 +0000 (17:28 +0100)]
Two more kasp test cases converted to pytest
The zone 'pregenerated.kasp' is a case where there already exist more
keys than required. For this we set the 'pregenerated' setting. This
will change the 'keydir_to_keylist' function behavior: Only keys in use
are considered. A key is in use if all of the states are either
undefined, or set to 'hidden'.
The 'some-keys.kasp' zone is similar to 'pregenerated.kasp', except
only some keys have been pregenerated.
Matthijs Mekking [Fri, 14 Mar 2025 16:11:14 +0000 (17:11 +0100)]
Convert many kasp test cases to pytst
Write python-based tests for the many test cases from the kasp system
test. These test cases all follow the same pattern:
- Wait until the zone is signed.
- Check the keys from the key-directory against expected properties.
- Set the expected key timings derived from when the key was created.
- Check the key timing metadata against expected timings.
- Check the 'rndc dnssec -status' output.
- Check the apex is signed correctly.
- Check a subdomain is signed correctly.
- Verify that the zone is DNSSEC correct.
Remove the counterparts for the newly added test from the kasp shell
tests script.
fix: dev: Fix a date race in qpcache_addrdataset()
The 'qpnode->nsec' structure member isn't protected by a lock and
there's a data race between the reading and writing parts in the
qpcache_addrdataset() function. Use a node read lock for accessing
'qpnode->nsec' in qpcache_addrdataset(). Add an additional
'qpnode->nsec != DNS_DB_NSEC_HAS_NSEC' check under a write lock
to be sure that no other competing thread changed it in the time
when the read lock is unlocked and a write lock is not acquired
yet.
Closes #5285
Merge branch '5285-data-race-in-qpcache_addrdataset' into 'main'
The 'qpnode->nsec' structure member isn't protected by a lock and
there's a data race between the reading and writing parts in the
qpcache_addrdataset() function. Use a node read lock for accessing
'qpnode->nsec' in qpcache_addrdataset(). Add an additional
'qpnode->nsec != DNS_DB_NSEC_HAS_NSEC' check under a write lock
to be sure that no other competing thread changed it in the time
when the read lock is unlocked and a write lock is not acquired
yet.
fix: usr: Fix a serve-stale issue with a delegated zone
When ``stale-answer-client-timeout 0`` option was enabled, it could be ignored
when resolving a zone which is a delegation of an authoritative zone belonging
to the resolver. This has been fixed.
Closes #5275
Merge branch '5275-stale-answer-client-timeout-0-and-delegation-fix' into 'main'
Test 'stale-answer-client-timeout 0' with a delegation
Add a new test which gets an answer for a delegated zone, then
checks whether the 'stale-answer-client-timeout 0' mode (i.e. the
'stalefirst' mode) works for it.
When 'stale-answer-client-timeout' is 0, named is allowed to return
a stale answer immediately, while also initiating a new query to get
the real answer. This mode is activated in ns__query_start() by setting
the 'qctx->options.stalefirst' optoin to 'true' before calling the
query_lookup() function, but not when the zone is known to be
authoritative to the server. When the zone is authoritative, and
query_looup() finds out that the requested name is a delegation,
then before proceeding with the query, named tries to look it up
in the cache first. Here comes the issue that it doesn't consider
enabling 'qctx->options.stalefirst' in this case, and so the
'stale-answer-client-timeout 0' setting doesn't work for those
delegated zones - instead of immediately returning the stale answer
(if it exists), named tries to resolve it.
Fix this issue by enabling 'qctx->options.stalefirst' in the
query_zone_delegation() function just before named looks up the name
in the cache using a new query_lookup() call. Also, if nothing was
found in the cache, don't initiate another query_lookup() from inside
query_notfound(), and let query_notfound() do its work, i.e. it will
call query_delegation() for further processing.
Mark Andrews [Tue, 22 Apr 2025 00:58:14 +0000 (00:58 +0000)]
fix: usr: Fix EDNS yaml output
`dig` was producing invalid YAML when displaying some EDNS options. This has been corrected.
Several other improvements have been made to the display of EDNS option data:
- We now use the correct name for the UPDATE-LEASE option, which was previously displayed as "UL", and split it into separate LEASE and LEASE-KEY components in YAML mode.
- Human-readable durations are now displayed as comments in YAML mode so as not to interfere with machine parsing.
- KEY-TAG options are now displayed as an array of integers in YAML mode.
- EDNS COOKIE options are displayed as separate CLIENT and SERVER components, and cookie STATUS is a retrievable variable in YAML mode.
Closes #5014
Merge branch '5014-improve-edns-yaml-processing' into 'main'
Mark Andrews [Wed, 30 Oct 2024 03:37:32 +0000 (14:37 +1100)]
Split EDNS COOKIE YAML into separate parts
Split the YAML display of the EDNS COOKIE option into CLIENT and SERVER
parts. The STATUS of the EDNS COOKIE in the reply is now a YAML element
rather than a comment.
Mark Andrews [Tue, 29 Oct 2024 03:32:54 +0000 (14:32 +1100)]
Change the name and YAML format of EDNS UL
The offical EDNS option name for "UL" is "UPDATE-LEASE". We now
emit "UPDATE-LEASE" instead of "UL", when printing messages, but
"UL" has been retained as an alias on the command line.
Update leases consist of 1 or 2 values, LEASE and KEY-LEASE. These
components are now emitted separately so they can be easily extracted
from YAML output. Tests have been added to check YAML correctness.
Mark Andrews [Tue, 29 Oct 2024 05:45:41 +0000 (16:45 +1100)]
Add YAML escaping where needed
When rendering text, such as domain names or the EXTRA-TEXT
field of the EDE option, backslashes and quotation marks must
be escaped to ensure that the emitted message is valid YAML.
Mark Andrews [Tue, 29 Oct 2024 23:41:21 +0000 (10:41 +1100)]
Collapse common switch cases when emitting EDNS options
The CHAIN and REPORT-CHANNEL EDNS options are both domain names, so they
can be combined. THE CLIENT-TAG and SERVER-TAG EDNS options are both 16
bit integers, so they can be combined.
fix: usr: Disable own memory context for libxml2 on macOS
Apple broke custom memory allocation functions in the system-wide libxml2 starting with macOS Sequoia 15.4. Usage of the custom memory allocation functions has been disabled on macOS.
Closes #5268
Merge branch '5268-disable-libxml2-memory-management-on-macos' into 'main'
Disable own memory context for libxml2 on macOS 15.4 Sequoia
The custom allocation API for libxml2 is deprecated starting in macOS
Sequoia 15.4, iOS 18.4, tvOS 18.4, visionOS 2.4, and tvOS 18.4.
Disable the memory function override for libxml2 when
LIBXML_HAS_DEPRECATED_MEMORY_ALLOCATION_FUNCTIONS is defined as Apple
broke the system-wide libxml2 starting with macOS Sequoia 15.4.
isctest.util was not imported so file_contents_contain could not be
found. And rename verify_keys to check_keys because it asserts in
isctest.run.retry_with_timeout.
Matthijs Mekking [Fri, 14 Mar 2025 12:42:30 +0000 (13:42 +0100)]
Convert some special kasp test cases to pytest
This converts a special characters test case, a max-zone-ttl error
check, and two cases of insecure zones.
We no longer assert for having more than one DNSKEY and/or RRSIG
records. If the zone is insecure, this is no longer always true. And
we already check for the expected number of records in the
check_dnskeys/check_signatures functions.
Matthijs Mekking [Fri, 14 Mar 2025 12:08:44 +0000 (13:08 +0100)]
Convert dynamic zone test cases to pytest
This commit deals with converting the dynamic zone test cases to
pytest. The tests for 'inline-signing.kasp' are similar to the default
case, so these are added to 'test_kasp_default'.
Unfortunately I need to add sleep calls in between freezing, updating,
and thawing a zone. Without it the intermittent failures are too
frequent.
Matthijs Mekking [Fri, 14 Mar 2025 11:52:36 +0000 (12:52 +0100)]
Convert kasp default test cases to pytest
This commit deals with converting the test cases related to the default
dnssec-policy.
This requires a new method 'check_update_is_signed'. This method will
be used in future tests as well, and checks if an expected record is
in the zone and is properly signed.
Remove the counterparts for the newly added test from the kasp shell
tests script.
Matthijs Mekking [Fri, 14 Mar 2025 11:19:36 +0000 (12:19 +0100)]
Convert kasp dnssectools tests to pytest
Convert the first couple of tests from 'kasp/tests.sh' to
'kasp/tests_kasp.py', those are test cases related to 'dnssec-keygen'
and 'dnssec-settime'.
For this, we also add a new KeyProperties method,
'policy_to_properties', that takes a list of strings which represent
the keys according to the dnssec-policy and the expected key states.
fix: dev: Move the call_rcu_thread explicit create and shutdown to isc_loop
When isc__thread_initialize() is called from a library constructor, it
could be called before we fork the main process. This happens with
named, and then we have the call_rcu_thread attached to the pre-fork
process and not the post-fork process, which means that the initial
process will never shutdown, because there's noone to tell it so.
Move the isc__thread_initialize() and isc__thread_shutdown() to the
isc_loop unit where we call it before creating the extra thread and
after joining all the extra threads respectively.
Closes #5281
Merge branch '5281-move-call_rcu-thread-ctor-dtor-to-main-thread' into 'main'
Move the call_rcu_thread explicit create and shutdown to isc_loop
When isc__thread_initialize() is called from a library constructor, it
could be called before we fork the main process. This happens with
named, and then we have the call_rcu_thread attached to the pre-fork
process and not the post-fork process, which means that the initial
process will never shutdown, because there's noone to tell it so.
Move the isc__thread_initialize() and isc__thread_shutdown() to the
isc_loop unit where we call it before creating the extra thread and
after joining all the extra threads respectively.
The QPDB_VIRTUAL value was introduced to allow the clients (presumably
ns_clients) that has been running for some time to access the cached
data that was valid at the time of its inception. The default value
of 5 minutes is way longer than longevity of the ns_client object as
the resolver will give up after 2 minutes.
Reduce the value to 10 seconds to accomodate to honour the original
more closely, but still allow some leeway for clients that started some
time in the past.
Our measurements show that even setting this value to 0 has no
statistically significant effect, thus the value of 10 seconds should be
on the safe side.
Merge branch 'ondrej/reduce-QPDB_VIRTUAL' into 'main'
Ondřej Surý [Fri, 21 Mar 2025 05:17:55 +0000 (06:17 +0100)]
Reduce QPDB_VIRTUAL to 10 seconds
The *DB_VIRTUAL value was introduced to allow the clients (presumably
ns_clients) that has been running for some time to access the cached
data that was valid at the time of its inception. The default value
of 5 minutes is way longer than longevity of the ns_client object as
the resolver will give up after 2 minutes.
Reduce the value to 10 seconds to accomodate to honour the original
more closely, but still allow some leeway for clients that started some
time in the past.
Our measurements show that even setting this value to 0 has no
statistically significant effect, thus the value of 10 seconds should be
on the safe side.
Many of the system tests now use jinja2 template engine. Adding jinja2
as a hard dependency is preferable than potentially silently skipping
many system tests.
Nicki Křížek [Fri, 4 Oct 2024 14:44:13 +0000 (16:44 +0200)]
Replace selected setup.sh system test files
These setup.sh scripts only do templating and copying files. Both of
these can be replaced with either jinja templates, or using plain files.
Since each test invocation creates its own temporary directory, copying
files to ensure a "clean" state is no longer necessary.
In cases where named writes some content to the files, a jinja template
can be used instead of a plain file to avoid an artifact check which
would detect a change to a git-tracked file.
Nicki Křížek [Tue, 1 Oct 2024 13:13:06 +0000 (15:13 +0200)]
Replace the trivial setup.sh system test files
All these setup files only use copy_setports function which can be done
with jinja2 templates instead -- simply by renaming the .in files to
.j2, without any other changes. The pytest runner will render these
templates during test setup without any need for an additional script.
Mark Andrews [Tue, 15 Apr 2025 03:11:01 +0000 (03:11 +0000)]
fix: usr: Return DNS COOKIE and NSID with BADVERS
This change allows the client to identify the server that returns the
BADVERS and to provide a DNS SERVER COOKIE to be included in the
resend of the request.
Closes #5235
Merge branch '5235-return-the-server-cookie-when-returning-badvers' into 'main'
Mark Andrews [Fri, 28 Mar 2025 01:08:37 +0000 (12:08 +1100)]
Check DNS COOKIE, NSID and BADVERS
DNS COOKIE and NSID should also be being processed when returning
BADVERS. Check that this has actually occured by looking for the
cookie and nsid in the response.
Mark Andrews [Fri, 25 Oct 2024 03:43:03 +0000 (14:43 +1100)]
Process NSID and DNS COOKIE options when returning BADVERS
This will help identify the broken server if we happen to break
EDNS version negotiation. It will also help protect the client
from spoofed BADVERSION responses.
With `dnssec-policy` you can pregenerate keys and if they are eligible, rather than creating a new key, a key is selected from the pregenerated keys. A key is eligible if it is unused, i.e it has no key timing metadata set.
Merge branch 'matthijs-clarify-pregenerating-keys' into 'main'
With dnssec-policy you can pregenerate keys and if they are eligible,
rather than creating a new key, a key is selected from the pregenerated
keys. A key is eligible if it is unused, i.e it has no key timing
metadata set.
Michal Nowak [Mon, 14 Apr 2025 10:56:13 +0000 (10:56 +0000)]
fix: test: Fix check_pid() in runtime system test on FreeBSD
The original check_pid() always returned 0 on FreeBSD, even if the
process was still running. This makes the "verifying that named checks
for conflicting named processes" check fail on FreeBSD with TSAN.
Merge branch 'mnowak/fix-runtime-pid-check' into 'main'
Michal Nowak [Thu, 3 Apr 2025 11:38:03 +0000 (13:38 +0200)]
Fix check_pid() in runtime system test on FreeBSD
The original check_pid() always returned 0 on FreeBSD, even if the
process was still running. This makes the "verifying that named checks
for conflicting named processes" check fail on FreeBSD with TSAN.
Michał Kępień [Fri, 11 Apr 2025 14:51:14 +0000 (14:51 +0000)]
chg: test: Use isctest.asyncserver in the "forward" test
Replace the custom DNS servers used in the "forward" system test with new
code based on the isctest.asyncserver module.
For ans6, instead of configuring the responses to send at runtime, set
them up when the server is started. Make sure the server supports
toggling response sending at runtime to enable simulating forwarder
timeouts as required by one of the checks.
For ans11, put most of the responses to be provided by that server into
a zone file, only retaining code modifying zone-based answers in the
form of a response handler, to improve code readability. Use explicit
domain names instead of variables as that server only handles a single
domain and fixed strings improve readability in this case. Make sure
the server supports toggling response sending at runtime to enable
simulating forwarder timeouts as required by one of the checks.
Migrate sendcmd() and its uses to the new way of sending control queries
to custom servers used in system tests.
Depends on !10339
Merge branch 'michal/forward-asyncserver' into 'main'
Michał Kępień [Fri, 11 Apr 2025 14:18:50 +0000 (09:18 -0500)]
Use isctest.asyncserver in the "forward" test
Replace the custom DNS servers used in the "forward" system test with
new code based on the isctest.asyncserver module.
For ans6, instead of configuring the responses to send at runtime, set
them up when the server is started. Make sure the server supports
toggling response sending at runtime to enable simulating forwarder
timeouts as required by one of the checks.
For ans11, put most of the responses to be provided by that server into
a zone file, only retaining code modifying zone-based answers in the
form of a response handler, to improve code readability. Use explicit
domain names instead of variables as that server only handles a single
domain and fixed strings improve readability in this case. Make sure
the server supports toggling response sending at runtime to enable
simulating forwarder timeouts as required by one of the checks.
Migrate sendcmd() and its uses to the new way of sending control queries
to custom servers used in system tests.
Michał Kępień [Fri, 11 Apr 2025 14:17:22 +0000 (14:17 +0000)]
new: test: Add support for control commands to isctest.asyncserver
Some BIND 9 system tests need to dynamically change custom server
behavior at runtime. Existing custom servers typically use a separate
TCP socket for listening to control commands, which mimics what `named`
does, but adds extra complexity to the custom server's networking code
for no gain (given the purpose at hand). There is also no common way of
performing typical runtime actions (like toggling response dropping)
across all custom servers.
Instead of listening on a separate TCP socket in `asyncserver.py`, make
it detect DNS queries to a "magic" domain (`_control.`) on the same port
as the one it uses for receiving "production" DNS traffic. This enables
query/response logging code to be reused for control traffic, clearly
denotes behavior changes in packet captures, facilitates implementing
commonly used features as reusable chunks of code (by making them "own"
distinct subdomains of the control domain), voids the need for separate
tools sending control commands, and enables using DNS facilities for
returning information to the user (e.g. RCODE for status codes, TXT
records for additional information, etc.).
Merge branch 'michal/asyncserver-control-commands' into 'main'
Michał Kępień [Fri, 11 Apr 2025 14:14:57 +0000 (09:14 -0500)]
Add control command for toggling response dropping
Implement a reusable control command that makes it possible to
dynamically disable/enable sending responses to clients. This is a
typical use case for custom DNS servers employed in various BIND 9
system tests.
Michał Kępień [Fri, 11 Apr 2025 14:14:57 +0000 (09:14 -0500)]
Implement control query handling
Some BIND 9 system tests need to dynamically change custom server
behavior at runtime. Existing custom servers typically use a separate
TCP socket for listening to control commands, which mimics what named
does, but adds extra complexity to the custom server's networking code
for no gain (given the purpose at hand). There is also no common way of
performing typical runtime actions (like toggling response dropping)
across all custom servers.
Instead of listening on a separate TCP socket in asyncserver.py, make it
detect DNS queries to a "magic" domain ("_control.") on the same port as
the one it uses for receiving "production" DNS traffic. This enables
query/response logging code to be reused for control traffic, clearly
denotes behavior changes in packet captures, facilitates implementing
commonly used features as reusable chunks of code (by making them "own"
distinct subdomains of the control domain), voids the need for separate
tools sending control commands, and enables using DNS facilities for
returning information to the user (e.g. RCODE for status codes, TXT
records for additional information, etc.).
Michał Kępień [Fri, 11 Apr 2025 14:14:57 +0000 (09:14 -0500)]
Add debug logs for response handler matching
With multiple and/or dynamically managed response handlers at play, it
becomes useful for debugging purposes to know which handler (if any) was
used for preparing each response sent by the server. Add debug logs
providing that information. Make class name the default string
representation of each response handler to prettify logs.
Michał Kępień [Fri, 11 Apr 2025 14:14:57 +0000 (09:14 -0500)]
Make response handler management more flexible
Extend AsyncDnsServer.install_response_handler() so that the provided
response handler can be inserted at the beginning of the handler list.
This enables installing a response handler that takes priority over all
previously installed handlers.
Add a new method, AsyncDnsServer.uninstall_response_handler(), which
enables removing a previously installed response handler.
Together, these two methods provide full control over the response
handler list at runtime.
Michał Kępień [Fri, 11 Apr 2025 14:14:57 +0000 (09:14 -0500)]
Avoid global namespace pollution
Add a main() function to all custom servers based on isctest.asyncserver
and move server startup code there. This prevents redefining variables
from outer scope in custom server code as it evolves.
Michał Kępień [Fri, 11 Apr 2025 14:14:57 +0000 (09:14 -0500)]
Gracefully handle invalid queries
Prevent custom servers based on asyncserver.py from exiting prematurely
due to unhandled exceptions raised as a result of attempting to parse
invalid queries sent by clients.
Michał Kępień [Fri, 11 Apr 2025 14:14:57 +0000 (09:14 -0500)]
Fix Python 3.6 StreamWriter compatibility issue
The StreamWriter.wait_closed() method was introduced in Python 3.7, so
attempting to use it with Python 3.6 raises an exception. This has not
been noticed before because awaiting StreamWriter.wait_closed() is the
last action taken for each TCP connection and unhandled exceptions were
not causing the scripts based on AsyncServer to exit prematurely until
the previous commit.
As per Python documentation [1], awaiting StreamWriter.wait_closed()
after calling StreamWriter.close() is recommended, but not mandatory, so
try to use it if it is available, without taking any fallback action in
case it isn't.
Michał Kępień [Fri, 11 Apr 2025 14:14:57 +0000 (09:14 -0500)]
Ensure uncaught exceptions kill custom servers
Uncaught exceptions raised by tasks running on event loops are not
handled by Python's default exception handler, so they do not cause
scripts to die immediately with a non-zero exit code. Set up an
exception handler for AsyncServer code that makes any uncaught exception
the result of the Future that the top-level coroutine awaits. This
ensures that any uncaught exceptions cause scripts based on AsyncServer
to immediately exit with an error, enabling the system test framework to
fail tests in which custom servers encounter unforeseen problems.
Matthijs Mekking [Fri, 14 Mar 2025 10:10:22 +0000 (11:10 +0100)]
Introduce pytest check_next_key_event, get_keyids
For the kasp tests we need a new utility that can retrieve a list of
Keys from a given directory, belonging to a specific zone. This is
'keydir_to_keylist' and is the replacement of 'kasp.sh:get_keyids()'.
'next_key_event_eqauls' is a method to check when the next key event is
scheduled, needed for the rollover tests, and is the equivalent of shell
script 'check_next_key_event'.
Matthijs Mekking [Fri, 14 Mar 2025 09:51:36 +0000 (10:51 +0100)]
Introduce pytest verify_keys and check_keytimes
This commit introduces replacements for the 'check_keys' and
'check_keytimes' from the shell test library. 'check_keys' is renamed
to 'verify_keys' because it does not assert.
For that, we introduce more functions for the class Key. The
'match_properties' function is used in 'verify_keys' to see if a set of
KeyProperties match the Key. This speficially ignores timing metadata.
The function resembles what is in 'kasp.sh:check_key()'.
The 'match_timingmetadata' function is used in 'check_keytimes' to see
if the timing metadata of a set of KeyProperties match the Key. The
values are checked in all three key files (except if the private key is
not available (set with properties["private"]), or if it is a legacy key
(set with properties["legacy"]).
An additional check function is added, to check if the key relationships
are set correctly. It follows a similar pattern as 'check_keytimes'. If
"Predecessor" and/or "Successor" are expected to be set in the state
file, this function checks so, and also verifies that they are not set
if they should not be.
Matthijs Mekking [Fri, 14 Mar 2025 09:38:43 +0000 (10:38 +0100)]
Update class Key
Because we want to check the metadata in all three files, a new
value in the Key class is added: 'privatefile'. The 'get_metadata'
function is adapted so that we can also check metadata in other files.
Introduce methods to easily retrieve the TTL and public DNSKEY record
from the keyfile.
When checking if the CDS is equal to the expected value, use the DNSKEY
TTL instead of hardcoded 3600.
Matthijs Mekking [Fri, 14 Mar 2025 09:23:37 +0000 (10:23 +0100)]
Introduce class KeyProperties
In isctest.kasp, introduce a new class 'KeyProperties' that can be used
to check if a Key matches expected properties. Properties are for the
time being divided in three parts: 'properties' that contain some
attributes of the expected properties (such as are we dealing with a
legacy key, is the private key available, and other things that do not
fit the metadata exactly), 'metadata' that contains expected metadata
(such as 'Algorithm', 'Lifetime', 'Length'), and 'timing', which is
metadata of the class KeyTimingMetadata.
The 'default()' method fills in the expected properties for the default
DNSSEC policy.
The 'set_expected_times()' sets the expected timing metadata, derived
from when the key was created. This method can take an offset to push
the expected timing metadata a duration in the future or back into the
past. If 'pregenerated=True', derive the expected timing metadata from
the 'Publish' metadata derived from the keyfile, rather than from the
'Created' metadata.
The calculations in the 'Ipub', 'IpubC' and 'Iret' methods are derived
from RFC 7583 DNSSEC Key Rollover Timing Considerations.
Matthijs Mekking [Thu, 13 Mar 2025 14:46:39 +0000 (15:46 +0100)]
Move test code that can be reused to isctest
This is the first step of converting the kasp system test to pytest.
Well, perhaps not the first, because earlier the ksr system test was
already converted to pytest and then the `isctest/kasp.py` library
was already introduced. Lots of this code can be reused for the kasp
pytest code.
First of all, 'check_file_contents_equal' is moved out of the ksr test
and into the 'check' library. This feels the most appropriate place
for this function to be reused in other tests. Then, 'keystr_to_keylist'
is moved to the 'kasp' library.
Introduce two new methods that are unused in this point of time, but
we are going to need them for the kasp system test. 'zone_contains'
will be used to check if a signature exists in the zonefile. This way
we can tell whether the signature has been reused or refreshed.
'file_contents_contain' will be used to check if the comment and public
DNSKEY record in the keyfile is correct.