Ondřej Surý [Fri, 14 Oct 2022 10:54:57 +0000 (12:54 +0200)]
Make the pthread_rwlock implementation header-only macros [2/2]
While using mutrace, the phtread-rwlock based isc_rwlock implementation
would be all tracked in the rwlock.c unit losing all useful information
as all rwlocks would be traced in a single place. Rewrite the
pthread_rwlock based implementation to be header-only macros, so we can
use mutrace to properly track the rwlock contention without heavily
patching mutrace to understand the libisc synchronization primitives.
Ondřej Surý [Fri, 14 Oct 2022 10:03:02 +0000 (12:03 +0200)]
Remove one level of indirection from isc_rwlock [1/2]
Instead of checking the PTHREAD_RUNTIME_CHECK from the header, move it
to the pthread_rwlock implementation functions. The internal isc_rwlock
actually cannot fail, so the checks in the header was useless anyway.
Ondřej Surý [Fri, 14 Oct 2022 12:35:44 +0000 (14:35 +0200)]
Add strong rwlock consistency checks to dns_rbtdb
The dns_rbtdb unit already tracks the state of the node and tree rwlocks
during the top level function and passes the states of the locks to the
called functions.
Add the tree locking family of macros modeled after node locking macros,
and expand both to track the state of the lock in an external variable.
Additionally, in developer mode, add precondition to the macros, so the
lock is in required state - this should cause an assertion failure on
double locking instead of the thread getting stuck.
Ondřej Surý [Thu, 13 Oct 2022 19:10:04 +0000 (21:10 +0200)]
Remove isc_rwlock_downgrade usage in rbtdb.c
The only place where isc_rwlock_downgrade was being used was the
decrement_reference() where the code tries either relocks the node
rwlock to write and then tries to upgrade the tree lock. When returning
from the function it tries to restore the locks into a previous state
which is nice, but kind of moot, because at every use of
decrement_reference() the node locks is immediately or almost
immeditately unlocked, and same holds for the tree lock.
Instead of trying to restore the node and tree lock into the initial
state, the decrement_reference now returns the state of the locks, so
the caller can then use the right unlock operation (read or write).
Only when the tree lock was originally unlocked, the decrement_reference
unlocks the tree lock before returning to the caller.
Ondřej Surý [Thu, 6 Oct 2022 12:22:20 +0000 (14:22 +0200)]
Refactor the privilege dropping
On Linux, the libcap is now mandatory. It makes things simpler for us.
System without {set,get}res{uid,gid} now have compatibility shim using
setreuid/setregid or seteuid/setegid to setup effective UID/GID, so the
same code can be called all the time (including on Linux).
Artem Boldariev [Wed, 26 Oct 2022 12:41:30 +0000 (15:41 +0300)]
TLS DNS unit tests: do not share the port with TCP DNS tests
TLS DNS unit tests were sharing the port with TCP DNS tests by
mistake. That could have caused conflicts between the two, when
running the unit tests in parallel. This commit fixes that.
Artem Boldariev [Wed, 26 Oct 2022 12:17:54 +0000 (15:17 +0300)]
TCP and TLS DNS tests: properly pass connection callback
After the loop manager refactoring TCP DNS and TLS DNS unit tests
ended up broken.
The problem is that in these unit tests the code is written in such a
way that for establishing a new connection tcpdns_connect() and
tlsdns_connect() functions are used. However, in these tests as a
connection callback function connect_connect_cb() is used. The
function logic is responsible for determining the function for
establishing subsequent connection.
To do so, it called get_stream_connect_function() ... which can return
only tcp_connect() or tls_connect(), not tcpdns_connect() or
tlsdns_connect(). That is definitely *not* what was implied.
All this time the unit tests were testing something, but now what was
intended.
This commit fixes the problem by passing the tcpdns_connect() and
tlsdns_connect() function pointers to connect_connect_cb().
Ondřej Surý [Thu, 6 Oct 2022 16:00:06 +0000 (18:00 +0200)]
Rescan interfaces before dropping privileges
The ns_interfacemgr_scan() now requires the loopmgr to be running, so we
need to end exclusive mode for the rescan and then begin it again.
This is relatively safe operation (because the scan happens on the timer
anyway), but we need to ensure that we won't load the configuration from
different threads. This is already the case because the initial load
happens on the main thread and the control channel also listens just on
the main loop.
Aram Sargsyan [Mon, 31 Oct 2022 12:49:05 +0000 (12:49 +0000)]
Don't trust a placeholder KEYDATA record
When named starts it creates an empty KEYDATA record in the managed-keys
zone as a placeholder, then schedules a key refresh. If key refresh
fails for some reason (e.g. connectivity problems), named will load the
placeholder key into secroots as a trusted key during the next startup,
which will break the chain of trust, and named will never recover from
that state until managed-keys.bind and managed-keys.bind.jnl files are
manually deleted before (re)starting named again.
Before calling load_secroots(), check that we are not dealing with a
placeholder.
Evan Hunt [Sun, 23 Oct 2022 18:39:44 +0000 (11:39 -0700)]
Call dns_resolver_createfetch() asynchronously in zone_refreshkeys()
Because dns_resolver_createfetch() locks the view, it was necessary
to unlock the zone in zone_refreshkeys() before calling it in order
to maintain the lock order, and relock afterward. this permitted a race
with dns_zone_synckeyzone().
This commit moves the call to dns_resolver_createfetch() into a separate
function which is called asynchronously after the zone has been
unlocked.
The keyfetch object now attaches to the zone to ensure that
it won't be shut down before the asynchronous call completes.
This necessitated refactoring dns_zone_detach() so it always runs
unlocked. For managed zones it now schedules zone_shutdown() to
run asynchronously, and for unmanaged zones, it requires the last
dns_zone_detach() to be run without loopmgr running.
Ondřej Surý [Mon, 31 Oct 2022 14:46:30 +0000 (14:46 +0000)]
Don't enforce jemalloc on NetBSD
The NetBSD system allocator is in fact based on the jemalloc, but it
doesn't export the extended interface, so we can't use that. Remove
the jemalloc enforcement for the NetBSD.
Evan Hunt [Sun, 30 Oct 2022 01:05:39 +0000 (18:05 -0700)]
isc_async_run() runs events in reverse order
when more than one event was scheduled in the isc_aysnc queue,
they were executed in reverse order. we need to pull events
off the back of queue instead the front, so that uv_loop will
run them in the right order.
note that isc_job_run() has the same behavior, because it calls
uv_idle_start() directly. in that case we just document it so
it'll be less surprising in the future.
Ondřej Surý [Wed, 26 Oct 2022 10:40:43 +0000 (12:40 +0200)]
Move the zone loading to the offloaded threads
Instead of doing incremental zone loading with fixed quantum - 100
loaded lines per event, move the zone loading process to the offloaded
libuv threads using isc_work_enqueue() API.
This has the advantage that the thread scheduling is given back to the
operating system that understands blocking operations, and the zone
loading operation doesn't block the networking threads directly.
Mark Andrews [Thu, 6 Oct 2022 06:31:40 +0000 (17:31 +1100)]
Test named's check-svcb behaviour with UPDATE
Checks that malformed _dns SVCB records are rejected unless
check-svcb is set to no, in which case they are accepted. Both
missing ALPN and missing DOHPATH are checked for.
Mark Andrews [Wed, 5 Oct 2022 06:25:21 +0000 (17:25 +1100)]
Add checking of _dns SVCB records constraints to nsupdate
_dns SVBC records have additional constrains which should be checked
when records are being added. This adds those constraint checks but
allows the user to override them using 'check-svcb no'.
Mark Andrews [Wed, 5 Oct 2022 06:18:24 +0000 (17:18 +1100)]
Add dns_rdata_checksvcb
dns_rdata_checksvcb performs data entry checks on SVCB records.
In particular that _dns SVBC record have an 'alpn' and if that 'alpn'
parameter indicates HTTP is in use that 'dophath' is present.
Matthijs Mekking [Wed, 26 Oct 2022 08:02:36 +0000 (10:02 +0200)]
Fix update forwarding bug
The wrong tls configuration was picked here. It should be of the
primary that is selected by forward->which, not zone->curprimary.
This bug may cause BIND to select the wrong primary when retrieving
the TLS settings, or cause a crash in case the wrongly selected primary
has no TLS settings.
Matthijs Mekking [Wed, 26 Oct 2022 07:51:21 +0000 (09:51 +0200)]
Add new upforwd system test
Add a new upforwd system test that checks if update forwarding still
works if the first primary is badly configured.
We cannot reuse the 'example.' zone for this test because that
checks if update forwarding works for DoT. What transport is used
in the new test is of no relevance.
Update the system test to use different known good file names for
the different zones that are being tested.
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.
Matthijs Mekking [Wed, 26 Oct 2022 07:55:55 +0000 (09:55 +0200)]
Fix config bug related to port setting
There are three levels there for the port value, with increasing
priority:
1. The default ports, defined by 'port' and 'tls-port' config options.
2. The primaries-level default port: primaries port <number> { ... };
3. The primaries element-level port: primaries { <address> port
<number>; ... };"
In 'named_config_getipandkeylist()', the 'def_port' and 'def_tlsport'
variables are extracted from level 1. The 'port' variable is extracted
from the level 2. Currently if that is unset, it defaults to the
default port ('def_port' or 'def_tlsport' depending on the transport
used), but overrides the level 2 port setting for the next primaries in
the list.
Update the code such that we inherit the port only if the level 3 port
is not set, and inherit from the default ports if the level 2 port is
also not set.
Matthijs Mekking [Wed, 26 Oct 2022 14:55:05 +0000 (16:55 +0200)]
Add xfer system test case
Add a test case that if the first primary fails, the fallback of a
second primary on plain DNS works. This is mainly to test that the port
configuration inheritance works correctly.
Evan Hunt [Mon, 24 Oct 2022 20:54:39 +0000 (13:54 -0700)]
Fix an error when building with --disable-doh
The netievent handler for isc_nmsocket_set_tlsctx() was inadvertently
ifdef'd out when BIND was built with --disable-doh, resulting in an
assertion failure on startup when DoT was configured.
Michał Kępień [Mon, 24 Oct 2022 09:05:02 +0000 (11:05 +0200)]
Bump Sphinx version to 5.3.0
Make the Sphinx version listed in doc/arm/requirements.txt match the
version currently used in GitLab CI, so that Read the Docs builds the
documentation using the same Python software versions as those used in
GitLab CI.
Aram Sargsyan [Thu, 20 Oct 2022 10:38:50 +0000 (10:38 +0000)]
Getting the "prefetch" setting from the configuration cannot fail
The "prefetch" setting is in "defaultconf" so it cannot fail, use
INSIST to confirm that.
The 'trigger' and 'eligible' variables are now prefixed with
'prefetch_' and their declaration moved to an upper level, because
there is no more additional code block after this change.
Aram Sargsyan [Tue, 18 Oct 2022 13:21:01 +0000 (13:21 +0000)]
Fix prefetch "trigger" value's documentation in ARM
For the prefetch "trigger" parameter ARM states that when a cache
record with a lower TTL value is encountered during query processing,
it is refreshed. But in reality, the record is refreshed when the TTL
value is lower or equal to the configured "trigger" value.
Fix the documentation to make it match with with the code.
Aram Sargsyan [Tue, 18 Oct 2022 12:59:47 +0000 (12:59 +0000)]
Match prefetch eligibility behavior with ARM
ARM states that the "eligibility" TTL is the smallest original TTL
value that is accepted for a record to be eligible for prefetching,
but the code, which implements the condition doesn't behave in that
manner for the edge case when the TTL is equal to the configured
eligibility value.
Fix the code to check that the TTL is greater than, or equal to the
configured eligibility value, instead of just greater than it.
Aram Sargsyan [Tue, 18 Oct 2022 13:06:19 +0000 (13:06 +0000)]
Add another prefetch check in the resolver system test
The test triggers a prefetch, but fails to check if it acutally
happened, which prevented it from catching a bug when the record's
TTL value matches the configured prefetch eligibility value.
Check that prefetch happened by comparing the TTL values.
Tony Finch [Wed, 19 Oct 2022 10:20:24 +0000 (11:20 +0100)]
Delete the `render` benchmark
Instead of fixing a Coverity complaint (and other style nits),
delete it because it needs input data that can't be generated
with the tools that ship with BIND.
Aram Sargsyan [Fri, 21 Oct 2022 08:08:55 +0000 (08:08 +0000)]
Call dns_adb_endudpfetch() on error path, if required
For UDP queries, after calling dns_adb_beginudpfetch() in fctx_query(),
make sure that dns_adb_endudpfetch() is also called on error path, in
order to adjust the quota back.
Aram Sargsyan [Fri, 21 Oct 2022 08:08:47 +0000 (08:08 +0000)]
Always call dns_adb_endudpfetch() in fctx_cancelquery() for UDP queries
It is currently possible that dns_adb_endudpfetch() is not
called in fctx_cancelquery() for a UDP query, which results
in quotas not being adjusted back.
Always call dns_adb_endudpfetch() for UDP queries.
Artem Boldariev [Wed, 19 Oct 2022 12:26:48 +0000 (15:26 +0300)]
Fix named failing to start on Solaris systems with hundreds of CPUs
This commit fixes a startup issue on Solaris systems with
many (reportedly > 510) CPUs by bumping RLIMIT_NOFILE. This appears to
be a regression from 9.11.
Ondřej Surý [Tue, 18 Oct 2022 19:46:16 +0000 (21:46 +0200)]
Replace some raw nc usage in statschannel system test with curl
For tests where the TCP connection might get interrupted abruptly,
replace the nc with curl as the data sent from server to client might
get lost because of abrupt TCP connection. This happens when the TCP
connection gets closed during sending the large request to the server.
As we already require curl for other system tests, replace the nc usage
in the statschannel test with curl that actually understands the
HTTP/1.1 protocol, so the same connection is reused for sending the
consequtive requests, but without client-side "pipelining".
For the record, the server doesn't support parallel processing of the
pipelined request, so it's a bit misnomer here, because what we are
actually testing is that we process all requests received in a single
TCP read callback.