Both utilities were included as one man page, but this caused a problem:
Sphinx directive .. include was used twice on the same file, which
prevented us from using labels (or anything with unique identifier) in
the man pages. This effectivelly prevented linking to them.
Splitting man pages allows us to solve the linking problems and also
clearly make text easier to follow because it does not mention two tools
at the same time.
This change causes duplication of text, but given the frequecy of changes
to these tools I think it is acceptable.
Related: #2799
(cherry picked from commit 2e42414522)
The C17 standard deprecated ATOMIC_VAR_INIT() macro (see [1]). Follow
the suite and remove the ATOMIC_VAR_INIT() usage in favor of simple
assignment of the value as this is what all supported stdatomic.h
implementations do anyway:
* MacOSX.plaform: #define ATOMIC_VAR_INIT(__v) {__v}
* Gcc stdatomic.h: #define ATOMIC_VAR_INIT(VALUE) (VALUE)
1. http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1138r0.pdf
(cherry picked from commit f251d69eba)
Previously, the function(s) in the commit subject could fail for various
reasons - mostly allocation failures, or other functions returning
different return code than ISC_R_SUCCESS. Now, the aforementioned
function(s) cannot ever fail and they would always return ISC_R_SUCCESS.
Change the function(s) to return void and remove the extra checks in
the code that uses them.
(cherry picked from commit d128656d2e)
Previously, the function(s) in the commit subject could fail for various
reasons - mostly allocation failures, or other functions returning
different return code than ISC_R_SUCCESS. Now, the aforementioned
function(s) cannot ever fail and they would always return ISC_R_SUCCESS.
Change the function(s) to return void and remove the extra checks in
the code that uses them.
(cherry picked from commit 8fa27365ec)
The query pointer was detached too early leading to null pointer
reference. Move the query_detach() after the query->canceled check.
(cherry picked from commit 9d8e8a4fcc)
The current implementation of isc_queue uses Michael-Scott lock-free
queue that in turn uses hazard pointers. It was discovered that the way
we use the isc_queue, such complicated mechanism isn't really needed,
because most of the time, we either execute the work directly when on
nmthread (in case of UDP) or schedule the work from the matching
nmthreads.
Replace the current implementation of the isc_queue with a simple locked
ISC_LIST. There's a slight improvement - since copying the whole list
is very lightweight - we move the queue into a new list before we start
the processing and locking just for moving the queue and not for every
single item on the list.
NOTE: There's a room for future improvements - since we don't guarantee
the order in which the netievents are processed, we could have two lists
- one unlocked that would be used when scheduling the work from the
matching thread and one locked that would be used from non-matching
thread.
(cherry picked from commit 6bd025942c)
If the dns_request send callback is delayed, the dst API would get
deinitialized and then the detach from the tsig key would cause an
assertion failure.
Shutdown the isc_managers early, and only then dereference the dst
objects when cleaning up the resources used by nsupdate.
(cherry picked from commit be34b1c535)
For the reference, the _cancel_lookup() function iterates through
the lookup's queries list and detaches them. In the ideal scenario,
that should be the last reference and the query will be destroyed
after that, but it is also possible that we are still expecting a
callback, which also holds a reference (for example, _cancel_lookup()
could have been called from recv_done(), when send_done() was still
not executed).
The start_udp() and start_tcp() functions are currently designed in
slightly different ways: start_udp() creates a new query attachment
`connectquery`, to be called in the callback function, while
start_tcp() does not, which is a bug, but is hidden by the fact
that when the query is being erroneously destroyed prematurely (before
_cancel_lookup() is called) in the result of that, it also gets
de-listed from the lookup's queries' list, so _cancel_lookup() doesn't
even try to detach it.
For better understanding, here's an illustration of the query's
references count changes, and from where it was changed:
UDP
---
1. _new_query() -> refcount = 1 (initial)
2. start_udp() -> refcount = 2 (lookup->current_query)
3. start_udp() -> refcount = 3 (connectquery)
4. udp_ready() -> refcount = 4 (readquery)
5. udp_ready() -> refcount = 5 (sendquery)
6. udp_ready() -> refcount = 4 (lookup->current_query)
7. udp_ready() -> refcount = 3 (connectquery)
8. send_done() -> refcount = 2 (sendquery)
9. recv_done() -> refcount = 1 (readquery)
10. _cancel_lookup() -> refcount = 0 (initial)
11. the query gets destroyed and removed from `lookup->q`
TCP, fortunate scenario
-----------------------
1. _new_query() -> refcount = 1 (initial)
2. start_tcp() -> refcount = 2 (lookup->current_query)
3. launch_next_query() -> refcount = 3 (readquery)
4. launch_next_query() -> refcount = 4 (sendquery)
5. tcp_connected() -> refcount = 3 (lookup->current_query)
6. tcp_connected() -> refcount = 2 (bug, there was no connectquery)
7. send_done() -> refcount = 1 (sendquery)
8. recv_done() -> refcount = 0 (readquery)
9. the query gets prematurely destroyed and removed from `lookup->q`
10. _cancel_lookup() -> the query is not in `lookup->q`
TCP, unfortunate scenario, revealing the bug
--------------------------------------------
1. _new_query() -> refcount = 1 (initial)
2. start_tcp() -> refcount = 2 (lookup->current_query)
3. launch_next_query() -> refcount = 3 (readquery)
4. launch_next_query() -> refcount = 4 (sendquery)
5. tcp_connected() -> refcount = 3 (lookup->current_query)
6. tcp_connected() -> refcount = 2 (bug, there was no connectquery)
7. recv_done() -> refcount = 1 (readquery)
8. _cancel_lookup() -> refcount = 0 (the query was in `lookup->q`)
9. we hit an assertion here when trying to destroy the query, because
sendhandle is not detached (which is done by send_done()).
10. send_done() -> this never happens
This commit does the following:
1. Add a `connectquery` attachment in start_tcp(), like done in
start_udp().
2. Add missing _cancel_lookup() calls for error scenarios, which
were possibly missing because before fixing the bug, calling
_cancel_lookup() and then calling query_detach() would cause
an assertion.
3. Log a debug message and call isc_nm_cancelread(query->readhandle)
for every query in the lookup from inside the _cancel_lookup()
function, like it is done in _cancel_all().
4. Add a `canceled` property for the query which becomes `true` when
the lookup (and subsequently, its queries) are canceled.
5. Use the `canceled` property in the network manager callbacks to
know that the query was canceled, and act like `eresult` was equal
to `ISC_R_CANCELED`.
(cherry picked from commit 4043fe9090)
There was a missing UNLOCK_LOOKUP in the recv_done() callback when
the operation had been canceled. That omission could result in a
deadlock situation.
(cherry picked from commit 98820aef7e)
bad-ksk-without-zsk.conf only has a ksk defined without a
matching zsk for the same algorithm.
bad-zsk-without-ksk.conf only has a zsk defined without a
matching ksk for the same algorithm.
bad-unpaired-keys.conf has two keys of different algorithms
one ksk only and the other zsk only
(cherry picked from commit f23e86b96b)
the "zone" clause can be documented using, for instance,
`cfg_test --zonegrammar primary", which prints only
options that are valid in primary zones. this was not
the method being used when generating the named.conf
man page; instead, "zone" was documented with all possible
options, and no zone types at all.
this commit removes "zone" from the generic documentation
and adds include statements in named.conf.rst so that
correct zone grammars will be included in the man page.
(cherry picked from commit 4ca74eee49)
"masters" and "default-masters" are now flagged so they will
not be included in the named.conf man page, despite being
accepted as valid options by the parser for backward
compatibiility.
(cherry picked from commit 0e57fc160e)
Extend the timeouts system test to ensure that the maximum outgoing
transfer time (max-transfer-time-out) and maximum outgoing transfer idle
time (max-transfer-idle-out) works as expected. This is done by
lowering the limits to 5/1 minutes and testing that the connection has
been dropped while sleeping between the individual XFR messages.
(cherry picked from commit 8fed1b6461)
Test if the TCP connection gets reset when garbage instead of DNS
message is sent.
I'm only happy when it rains
Pour some misery down on me
- Garbage
(cherry picked from commit ebfdb50ac7)
Extend the timeouts system test that bursts the queries for large TXT
record and never read any responses back filling up the server TCP write
buffer. The test should work with the default wmem_max value on
Linux (208k).
(cherry picked from commit b735182ae0)
There was a bug in the checking of the "blackhole" ACL in
dns_request_create*(), causing an address to be treated as included
in the ACL if it was explicitly *excluded*. Thus, leaving "blackhole"
unset had no effect, but setting it to "none" would cause any
destination addresses to be rejected for dns_request purposes. This
would cause zone transfer requests and SOA queries to fail, among
other things.
The bug has been fixed, and "blackhole { none; };" was added to the
xfer system test as a regression test.
(cherry picked from commit 4444b168db)
Replace the hard-coded paths for various BIND 9 files (configuration,
pid, etc.) in the man pages and ARM with compile-time values using the
sphinx-build replace system.
This is more complicated, because the restructured text specification
doesn't allow |substitions| inside ``code-blocks``, so for each specific
file we had to create own substition which is sub-optimal, but it is
only way how to do this without adding Sphinx extension.
(cherry picked from commit b42681c4e9)
The isc_thread_setaffinity call was removed in !5265 and we are not
going to restore it because it was proven that the performance is better
without it. Additionally, remove the already disabled cpu system test.
The isc_thread_setconcurrency function is unused and also calling
pthread_setconcurrency() on Linux has no meaning, formerly it was
added because of Solaris in 2001 and it was removed when taskmgr was
refactored to run on top of netmgr in !4918.
(cherry picked from commit 0500345513)
Fix a missing status=$((status+ret)) in the keyfromlabel system test,
which would ignore the error if ZSK key creation failed.
(cherry picked from commit 7845f51178)
When there are more than one tokens initialized in SoftHSMv2,
care must be taken to correctly identify them.
Use a SoftHSMv2 token label which will uniquely identify the
token used for this test.
Use the "--token-label" parameter for the `pkcs11-tool` program
to make sure that it finds and uses the correct token.
(cherry picked from commit a449709441)
The 'id' variable is either keyfromlabel-ksk or keyfromlabel-zsk and is
set in the 'keygen' and 'keyfromlabel' functions. It should not be used
outside these functions.
(cherry picked from commit 468cf3cdc2)
This test was originally in the pkcs11 system test. While this crash
happened in the native pkcs11 of BIND 9, and that code has been
removed in 9.17, there is no need for this test. Nevertheless, it
doesn't hurt having the test case persist.
(cherry picked from commit bfe287f4a4)
Add a system test for engine_pkcs11 interactions that replaces the
tests that are done in the native PKCS#11 system test.
The native PKCS#11 code was removed in 9.17 but without copying the
pkcs11 system test.
(cherry picked from commit 11a0b41370)
This appears to be left over from the developement phase while
adding reference counting to the lookup structure.
(cherry picked from commit c068c3c771)
the dlzexternal test driver now includes ECS, if present in the
query, in the TXT record returned for QNAME "source-addr".
(cherry picked from commit 79ddedabf8)
Apparently we forgot about DLZ when updating DNS_CLIENTINFO_VERSION
constant for ECS, which is at value "3" since ECS was introduced.
The code in example drivers and tests now hardcodes version numbers
2 (without ECS) and 3 (with ECS) depending on what a given code path
requires.
(cherry picked from commit f81debe1c8)
The keyfromlabel system ECDSA tests sometimes fail. When this happens
the ZSK and KSK key id values differ by 1, which is an indication that
the same key is used for both DNSKEY records.
When the private key is retrieved with 'ENGINE_load_private_key()', the
public key is already set. But sometimes that key differs from the key
which was retrieved with 'ENGINE_load_public_key()'.
The libp11 source code uses id to find the key and without IDs all the
keys are "equal", so it is returning the first key in the array of the
enumerated keys instead of the matching key. In our test we didn't use
'--id', just '--label'. With this change, the system test should no
longer fail intermittently.
Note this is only an issue for ECDSA keys, not RSA keys.
(cherry picked from commit 0af8bbd49b)
Add missing system test for dnssec-keyfromlabel. Test for various
algorithms that we can generate key files from a key that is stored in a
HSM, and that those keys can be used for signing with dnssec-signzone.
(cherry picked from commit eba66665a5)
GitLab CI needs to know about some environment variables that will
tell where OpenSSL and SoftHSM2 is installed. This is done in the
image, making the prepare-softhsm2.sh script obsolete.
The SoftHSM2 module location is system specific.
(cherry picked from commit 221e1bc2a3)
- Removed all code that only runs under CYGWIN, and made all
code that doesn't run under CYGWIN non-optional.
- Removed the $TP variable which was used to add optional
trailing dots to filenames; they're no longer optional.
- Removed references to pssuspend and dos2unix.
- No need to use environment variables for diff and kill.
- Removed uses of "tr -d '\r'"; this was a workaround for
a cygwin regex bug that is no longer needed.
(cherry picked from commit 1d706f328c)
Commit c787a539d2 fixed a certain class of
intermittent system test failures caused by named instances unable to
restart. The root cause was bin/tests/system/stop.pl returning without
waiting for a named instance to remove its lock file.
Later on, it turned out that the above change causes other issues on
Windows due to the way named handles signals on that platform. Commit
761ba4514f intended to address those
issues by making the server_lock_file() subroutine in
bin/tests/system/stop.pl return an empty value on Windows, in order to
prevent the script for waiting for lock file cleanup on that platform.
Note, however, that Windows detection in that subroutine is limited to
checking whether the CYGWIN environment variable is set.
While that environment variable was not set on Unix-like systems before
commit 761ba4514f, another commit
(a33237f070, merged a few weeks later)
changed that by setting the CYGWIN environment variable to an empty
value on Unix-like systems. This made the defined($ENV{'CYGWIN'}) check
in server_lock_file() return true, inadvertently preventing
bin/tests/system/stop.pl from waiting for lock file removal before
exiting on Unix-like systems and therefore reintroducing the original
issue.
Fix by making server_lock_file() only return an empty value when the
CYGWIN environment variable is set to a non-empty value (which is what
bin/tests/system/conf.sh.win32 does). Adjust a similar check in the
pid_file_exists() subroutine in the same way for consistency.
(cherry picked from commit a938db2170)
The echo_*() and cat_*() functions in bin/tests/system/conf.sh.common
call the "read" builtin command without specifying the field separator
to use. This results in leading whitespace getting stripped from each
line of the texts passed to those functions, which mangles e.g. pytest
output, hindering test failure troubleshooting.
Address by setting IFS to an empty value for the "read" calls used in
the aforementioned helper functions.
(cherry picked from commit fb87022115)
The bin/tests/system/start.pl script truncates the named.run file for a
given named instance unless it is invoked with the --restart
command-line option. Ever since Python-based tests were introduced,
bin/tests/system/run.sh may start named instances used by a given system
test multiple times within a single run, causing the
bin/tests/system/start.pl script to truncate some of the log files
written during the test. This makes troubleshooting certain test
failures hard or even impossible.
Fix by calling bin/tests/system/start.pl with the --restart command-line
option for every start_servers() invocation except the first one.
(cherry picked from commit 65abbca79b)
When failure is expected, the `rndc` command in the catz system test
is being called directly instead of using a function, i.e.:
$RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 reconfig \
> /dev/null 2>&1 && ret=1
... instead of:
rndccmd 10.53.0.2 reconfig && ret=1
This is done to suppress messages like "lt-rndc: 'reconfig' failed:
failure" appearing in the message log of the test, because failure
is actually expected, and the appearance of that message can be
confusing.
The port value used in this case is not correct, making the
`rndc reload` command to fail. This error was not detected earlier
only because the failure of the command is actually expected, but
the failure happens for a "wrong" reason, and the test still passes.
Fix the error by using the existing variable instead of the fixed
number.
(cherry picked from commit 5f9d4b5db4)
Test the view reverting code by introducing a faulty dlz configuration
in named.conf and using `rndc reconfig` to check if named handles the
situation correctly.
We use "dlz" because the dlz processing code is located in an ideal
place in the view configuration function for the test to cover the
view reverting code.
This test is specifically added to the catz system test to additionally
cover the catz reconfiguration during the mentioned failed
reconfiguration attempt.
(cherry picked from commit 62337d433f)
If a view configuration error occurs during a named reconfiguration
procedure, BIND can end up having twin views (old and new), with some
zones and internal structures attached to the old one, and others
attached to the new one, which essentially creates chaos.
Implement some additional view reverting mechanisms to avoid the
situation described above:
1. Revert rpz configuration.
2. Revert catz configuration.
3. Revert zones to view attachments.
(cherry picked from commit 3697560f04)
The terms "DNS over HTTPS" and "DNS over TLS" should be hyphenated when
they are used as adjectives and non-hyphenated otherwise. Ensure all
occurrences of these terms in the source tree follow the above rule.
(CHANGES and release notes are intentionally left intact.)
Tweak a related ARM snippet, fixing a typo in the process.
If isc_app_run() gets interrupted by a signal, the global 'rndc_task'
variable may already be detached from (set to NULL) by the time the
outstanding netmgr callbacks are run. This triggers an assertion
failure in isc_task_shutdown(). However, explicitly calling
isc_task_shutdown() from rndc code is redundant because it does not use
isc_task_onshutdown() and the task_shutdown() function gets
automatically called anyway when the task manager gets destroyed (after
isc_app_run() returns). Remove the redundant isc_task_shutdown() calls
to prevent crashes after receiving a signal.
rndc_recvdone() is not treating the ISC_R_CANCELED result code as a
request to stop data processing, which may cause a crash when trying to
dereference ccmsg->buffer. Fix by ensuring ISC_R_CANCELED results in an
early exit from rndc_recvdone().
Make sure the logic for handling ISC_R_CANCELED in rndc_recvnonce()
matches the one present in rndc_recvdone() to ensure consistent behavior
between these two sibling functions.
Sometimes the serving a query or two might fail in the test due to the
listeners not being reinitialised on time. This commit makes the test
suite to wait for reconfiguration message in the log file to detect
the time when the reconfiguration request completed.
gnutls-cli is tricky to script around as it immediately closes the
server connection when its standard input is closed. This prevents
simple shell-based I/O redirection from being used for capturing the DNS
response sent over a TLS connection and the workarounds for this issue
employ non-standard utilities like "timeout".
Instead of resorting to clever shell hacks, reimplement the relevant
check in Python. Exit immediately upon receiving a valid DNS response
or when gnutls-cli exits in order to decrease the test's run time.
Employ dnspython to avoid the need for storing DNS queries in binary
files and to improve test readability. Capture more diagnostic output
to facilitate troubleshooting. Use a pytest fixture instead of an
Autoconf macro to keep test requirements localized.
1. 10 seconds is an unfortunate pick because that reintroduces the
problem described in commit 5307bf64 (for an earlier check).
Change the +tries=3 +timeout=10 to +tries=2 +time=15, so that we
minimize the risk of dig missing any responses sent by the server in
the first 15 seconds while also increasing our chances of the
response arriving in time on machines under heavy load and allowing
it a single retry in case things go awry.
2. The comment about TCP above was misleading: as painfully proven by
GitLab CI, using TCP is no guarantee of receiving a response in a
timely manner. It may help a bit, but it is certainly not a 100%
reliable solution.
Change the dig invocation to just use UDP like in the two prior
tests for consistency (and revise that comment accordingly).
