Evan Hunt [Tue, 25 Feb 2025 22:41:41 +0000 (14:41 -0800)]
set eresult based on the type in ncache_adderesult()
when the caching of a negative record failed because of the
presence of a positive one, ncache_adderesult() could override
this to ISC_R_SUCCESS. this could cause CNAME and DNAME responses
to be handled incorrectly. ncache_adderesult() now sets the result
code correctly in such cases.
Evan Hunt [Wed, 26 Feb 2025 00:43:46 +0000 (00:43 +0000)]
fix: dev: Remove 'target' from dns_adb
When a server name turns out to be a CNAME or DNAME, the ADB does not use it, but the `dns_adbname` structure still stored a copy of the target name. This is unnecessary and the code has been removed.
Evan Hunt [Fri, 21 Feb 2025 09:24:42 +0000 (01:24 -0800)]
remove 'target' from dns_adb
the target name parameter to dns_adb_createfind() was always passed as
NULL, so we can safely remove it.
relatedly, the 'target' field in the dns_adbname structure was never
referenced after being set. the 'expire_target' field was used, but
only as a way to check whether an ADB name represents a CNAME or DNAME,
and that information can be stored as a single flag.
The dual-stack-servers configuration option was not working as expected; the specified servers were not being used when they should have been, leading to resolution failures. This has been fixed.
Closes #5019
Merge branch '5019-dual-stack-servers-wasn-t-working-in-all-cases' into 'main'
Mark Andrews [Fri, 1 Nov 2024 03:45:43 +0000 (14:45 +1100)]
Removing now unneeded priming queries
Now that fctx_try is being called when adb returns DNS_ADB_NOMOREADDRESSES
we don't need these priming queries for the dual-stack-servers test
to succeed.
Mark Andrews [Fri, 1 Nov 2024 02:54:52 +0000 (13:54 +1100)]
Fix dual-stack-servers
Named was stopping nameserver address resolution attempts too soon
when dual stack servers are configured. Dual stack servers are
used when there are *not* addresses for the server in a particular
address family so find->status == DNS_ADB_NOMOREADDRESSES is not a
sufficient stopping condition when dual stack servers are available.
Call fctx_try to see if the alternate servers can be used.
Mark Andrews [Tue, 25 Feb 2025 23:39:40 +0000 (23:39 +0000)]
fix: usr: Relax private DNSKEY and RRSIG constraints
DNSKEY, KEY, RRSIG and SIG constraints have been relaxed to allow empty key and signature material after the algorithm identifier for PRIVATEOID and PRIVATEDNS. It is arguable whether this falls within the expected use of these types as no key material is shared and the signatures are ineffective but these are private algorithms and they can be totally insecure.
Closes #5167
Merge branch '5167-relax-private-dnskey-constraints' into 'main'
Mark Andrews [Thu, 6 Feb 2025 23:01:57 +0000 (10:01 +1100)]
Relax private DNSKEY and RRSIG constraints
DNSKEY, KEY, RRSIG and SIG constraints have been relaxed to allow
empty key and signature material after the algorithm identifier for
PRIVATEOID and PRIVATEDNS. It is arguable whether this falls within
the expected use of these types as no key material is shared and
the signatures are ineffective but these are private algorithms and
they can be totally insecure.
Evan Hunt [Tue, 25 Feb 2025 22:40:55 +0000 (22:40 +0000)]
fix: dev: Prevent a reference leak when using plugins
The `NS_QUERY_DONE_BEGIN` and `NS_QUERY_DONE_SEND` plugin hooks could cause a reference leak if they returned `NS_HOOK_RETURN` without cleaning up the query context properly.
Closes #2094
Merge branch '2094-plugin-reference-leak' into 'main'
Evan Hunt [Wed, 22 Jan 2025 01:57:00 +0000 (17:57 -0800)]
prevent a reference leak from the ns_query_done hooks
if the NS_QUERY_DONE_BEGIN or NS_QUERY_DONE_SEND hook is
used in a plugin and returns NS_HOOK_RETURN, some of the
cleanup in ns_query_done() can be skipped over, leading
to reference leaks that can cause named to hang on shut
down.
this has been addressed by adding more housekeeping
code after the cleanup: tag in ns_query_done().
Evan Hunt [Tue, 25 Feb 2025 21:34:31 +0000 (21:34 +0000)]
fix: dev: Simplify some dns_name API calls
Several functions in the `dns_name` module have had parameters removed, that were rarely or never used:
- `dns_name_fromtext()` and `dns_name_concatenate()` no longer take a target buffer.
- `dns_name_towire()` no longer takes a compression offset pointer; this is now part of the compression context.
- `dns_name_towire()` with a `NULL` compression context will copy name data directly into a buffer with no processing.
Evan Hunt [Sat, 22 Feb 2025 08:11:38 +0000 (00:11 -0800)]
simplify dns_name_fromtext() interface
previously, dns_name_fromtext() took both a target name and an
optional target buffer parameter, which could override the name's
dedicated buffer. this interface is unnecessarily complex.
we now have two functions, dns_name_fromtext() to convert text
into a dns_name that has a dedicated buffer, and dns_name_wirefromtext()
to convert text into uncompressed DNS wire format and append it to a
target buffer.
in cases where it really is necessary to have both, we can use
dns_name_fromtext() to load the dns_name, then dns_name_towire()
to append the wire format to the target buffer.
Evan Hunt [Fri, 21 Feb 2025 21:36:57 +0000 (13:36 -0800)]
avoid the 'target' buffer in dns_name_fromtext()
dns_name_fromtext() stores the converted name in the 'name'
passed to it, and optionally also copies it in wire format to
a buffer 'target'. this makes the interface unnecessarily
complex, and could be simplified by having a different function
for each purpose. as a first step, remove uses of the target
buffer in calls to dns_name_fromtext() where it wasn't actually
needed.
Evan Hunt [Fri, 21 Feb 2025 08:56:47 +0000 (00:56 -0800)]
remove 'target' parameter from dns_name_concatenate()
the target buffer passed to dns_name_concatenate() was never
used (except for one place in dig, where it wasn't actually
needed, and has already been removed in a prior commit).
we can safely remove the parameter.
Evan Hunt [Sat, 22 Feb 2025 02:14:55 +0000 (18:14 -0800)]
remove the 'name_coff' parameter in dns_name_towire()
this parameter was added as a (minor) optimization for
cases where dns_name_towire() is run repeatedly with the
same compression context, as when rendering all of the rdatas
in an rdataset. it is currently only used in one place.
we now simplify the interface by removing the extra parameter.
the compression offset value is now part of the compression
context, and can be activated when needed by calling
dns_compress_setmultiuse(). multiuse mode is automatically
deactivated by any subsequent call to dns_compress_permitted().
Evan Hunt [Tue, 25 Feb 2025 18:37:49 +0000 (18:37 +0000)]
fix: dev: Save time when creating a slab from another slab
The `dns_rdataslab_fromrdataset()` function creates a slab from an rdataset. If the source rdataset already uses a slab, then no processing is necessary; we can just copy the existing slab to a new location.
Evan Hunt [Tue, 25 Feb 2025 05:30:10 +0000 (21:30 -0800)]
save time when creating a slab from another slab
the dns_rdataslab_fromrdataset() function creates a slab
from an rdataset. if the source rdataset already uses a slab,
then no processing is necessary; we can just copy the existing
slab to a new location.
Ondřej Surý [Tue, 25 Feb 2025 15:38:08 +0000 (15:38 +0000)]
fix: usr: Fix assertion failure when dumping recursing clients
Previously, if a new counter was added to the hashtable
while dumping recursing clients via the `rndc recursing`
command, and `fetches-per-zone` was enabled, an assertion
failure could occur. This has been fixed.
Closes #5200
Merge branch '5200-destroy-iterator-inside-the-rwlock' into 'main'
Ondřej Surý [Tue, 25 Feb 2025 11:13:56 +0000 (12:13 +0100)]
Destroy the hashmap iterator inside the rwlock
Previously, the hashmap iterator for fetches-per-zone was destroy
outside the rwlock. This could lead to an assertion failure due to a
timing race with the internal rehashing of the hashmap table as the
rehashing process requires no iterators to be running when rehashing the
hashmap table. This has been fixed by moving the destruction of the
iterator inside the read locked section.
Ondřej Surý [Mon, 24 Feb 2025 13:29:55 +0000 (14:29 +0100)]
Squash set_offsets() and dns_name_offsets() into single function
The third argument to set_offsets() was only used in
dns_name_fromregion() and not really needed. We can remove the third
argument and then manually check whether the last label is root label.
Ondřej Surý [Mon, 24 Feb 2025 13:17:22 +0000 (14:17 +0100)]
Remove target buffer from dns_name_downcase()
There was just a single use of passing an extra buffer to
dns_name_downcase() which have been replaced by simple call to
isc_ascii_lowercase() and the 'target' argument from dns_name_downcase()
function has been removed.
Ondřej Surý [Mon, 24 Feb 2025 12:58:13 +0000 (13:58 +0100)]
Remove MAKE_EMPTY() macro from dns_name unit
The MAKE_EMPTY() macro was clearing up the output variable in case of
the failure. However, this was breaking the usual design pattern that
the output variables are left in indeterminate state or we don't touch
them at all when a failure occurs. Remove the macro and change the
dns_name_downcase() to not touch the name contents until success.
Ondřej Surý [Fri, 21 Feb 2025 11:10:02 +0000 (12:10 +0100)]
Cleanup the usage of dns_offsets_t vs unsigned char * pointers
There was a back-and-forth between static arrays and the pointers to the
offsets. Since we are now only using the static arrays, we can cleanup
the usage of the pointers that would previously point either to the
static array or name->offsets if available.
Ondřej Surý [Fri, 21 Feb 2025 11:09:28 +0000 (12:09 +0100)]
Remove offsets from the dns_name and dns_fixedname structures
The offsets were meant to speed-up the repeated dns_name operations, but
it was experimentally proven that there's actually no real-world
benefit. Remove the offsets and labels fields from the dns_name and the
static offsets fields to save 128 bytes from the fixedname in favor of
calculating labels and offsets only when needed.
Alessio Podda [Tue, 25 Feb 2025 10:29:00 +0000 (10:29 +0000)]
chg: usr: Drop malformed notify messages early instead of decompressing them
The DNS header shows if a message has multiple questions or invalid
NOTIFY sections. We can drop these messages early, right after parsing
the question. This matches RFC 9619 for multi-question messages and
Unbound's handling of NOTIFY. We still parse the question to include it in
our FORMERR response.
Add drop_msg_early() function to check for these conditions:
- Messages with more than one question, as required by RFC 9619
- NOTIFY query messages containing answer sections (like Unbound)
- NOTIFY messages containing authority sections (like Unbound)
Closes #5158, #3656
Merge branch '5158-early-formerr-on-bad-notify-or-bad-qdcount' into 'main'
alessio [Wed, 29 Jan 2025 09:37:33 +0000 (10:37 +0100)]
Drop malformed notify messages early instead of decompressing them
The DNS header shows if a message has multiple questions or invalid
NOTIFY sections. We can drop these messages early, right after parsing
the question. This matches RFC 9619 for multi-question messages and
Unbound's handling of NOTIFY.
To further add further robustness, we include an additional check for
unknown opcodes, and also drop those messages early.
Add early_sanity_check() function to check for these conditions:
- Messages with more than one question, as required by RFC 9619
- NOTIFY query messages containing answer sections (like Unbound)
- NOTIFY messages containing authority sections (like Unbound)
- Unknown opcodes.
Mark Andrews [Tue, 25 Feb 2025 06:16:45 +0000 (06:16 +0000)]
fix: test: Handle example3.db being modified in upforwd system test
The zone file for example3 (ns1/example3.db) can be modified in the
upforwd test as example3 is updated as part of the test. Whether
the zone is written out or not by the end of the test is timing
dependent. Rename ns1/example3.db to ns1/example3.db.in and copy it to
ns1/example3.db in setup so we don't trigger post test changes checks.
Closes #5180
Merge branch '5180-create-example3-in-setup' into 'main'
Mark Andrews [Tue, 25 Feb 2025 01:28:58 +0000 (12:28 +1100)]
Handle example3.db being modified in upforwd system test
The zone file for example3 (ns1/example3.db) can be modified in the
upforwd test as example3 is updated as part of the test. Whether
the zone is written out or not by the end of the test is timing
dependent. Rename ns1/example3.db to ns1/example3.db.in and copy
it to ns1/example3.db in setup so we don't trigger post test changes
checks.
Evan Hunt [Mon, 24 Feb 2025 23:39:23 +0000 (23:39 +0000)]
fix: dev: Fix a logic error in cache_name()
A change in 6aba56ae8 (checking whether a rejected RRset was identical
to the data it would have replaced, so that we could still cache a
signature) inadvertently introduced cases where processing of a
response would continue when previously it would have been skipped.
Closes #5197
Merge branch '5197-cache_name-logic-error' into 'main'
Evan Hunt [Mon, 24 Feb 2025 22:30:39 +0000 (14:30 -0800)]
Fix a logic error in cache_name()
A change in 6aba56ae8 (checking whether a rejected RRset was identical
to the data it would have replaced, so that we could still cache a
signature) inadvertently introduced cases where processing of a
response would continue when previously it would have been skipped.
Ondřej Surý [Mon, 24 Feb 2025 22:24:51 +0000 (22:24 +0000)]
fix: dev: Acquire the database reference before possibly last node release
Acquire the database reference in the detachnode() to prevent the last
reference to be release while the NODE_LOCK being locked. The NODE_LOCK
is locked/unlocked inside the RCU critical section, thus it is most
probably this should not pose a problem as the database uses call_rcu
memory reclamation, but this it is still safer to acquire the reference
before releasing the node.
Closes #5194
Merge branch '5194-fix-assertion-failure-while-reference-counting-qpdb' into 'main'
Ondřej Surý [Mon, 24 Feb 2025 14:55:18 +0000 (15:55 +0100)]
Acquire the database reference before possibly last node release
Acquire the database refernce in the detachnode() to prevent the last
reference to be release while the NODE_LOCK being locked. The NODE_LOCK
is locked/unlocked inside the RCU critical section, thus it is most
probably this should not pose a problem as the database uses call_rcu
memory reclamation, but this it is still safer to acquire the reference
before releasing the node.
Ondřej Surý [Sat, 22 Feb 2025 18:06:21 +0000 (18:06 +0000)]
chg: dev: Move the library initialization and shutdown to executables
Instead of relying on unreliable order of execution of the library
constructors and destructors, move them to individual binaries. The
advantage is that the execution time and order will remain constant and
will not depend on the dynamic load dependency solver.
Merge branch 'ondrej/move-the-constructors-destructors-to-binaries' into 'main'
Ondřej Surý [Tue, 4 Feb 2025 18:17:28 +0000 (19:17 +0100)]
Explicitly create and shutdown the call_rcu_thread
As the default_call_rcu_thread can't be forced to flush all the work
during the executable shutdown, create one call_rcu_thread explicitly
and assign it to the all created threads.
This allows this explicit call_rcu_thread to be unassociated from the
main thread and freed before the executable destructor exits.
Ondřej Surý [Tue, 4 Feb 2025 12:17:31 +0000 (13:17 +0100)]
Move the library init and shutdown to executables
Instead of relying on unreliable order of execution of the library
constructors and destructors, move them to individual binaries. The
advantage is that the execution time and order will remain constant and
will not depend on the dynamic load dependency solver.
This requires more work, but that was mitigated by a simple requirement,
any executable using libisc and libdns, must include <isc/lib.h> and
<dns/lib.h> respectively (in this particular order). In turn, these two
headers must not be included from within any library as they contain
inlined functions marked with constructor/destructor attributes.
Ondřej Surý [Fri, 21 Feb 2025 21:26:18 +0000 (21:26 +0000)]
fix:usr: Dump the active resolver fetches from dns_resolver_dumpfetches()
Previously, active resolver fetches were only dumped when the `fetches-per-zone` configuration option was enabled. Now, active resolver fetches are dumped along with the number of `clients-per-server` counters per resolver fetch.
Merge branch 'ondrej/make-dns_resolver_dumpfetches-dump-fetches' into 'main'
Ondřej Surý [Mon, 17 Feb 2025 13:58:28 +0000 (14:58 +0100)]
Dump the fetches from dns_resolver_dumpfetches()
Previously, the dns_resolver_dumpfetches() would go over the fetch
counters. Alas, because of the earlier optimization, the fetch counters
would be increased only when fetches-per-zone was not 0, otherwise the
whole counting was skipped for performance reasons.
Instead of using the auxiliary fetch counters hash table, use the real
hash table that stores the fetch contexts to dump the ongoing fetches to
the recursing file.
Additionally print more information about the fetch context like start
and expiry times, number of fetch responses, number of queries and count
of allowed and dropped fetches.
Ondřej Surý [Fri, 21 Feb 2025 21:24:56 +0000 (21:24 +0000)]
fix:usr: Fix the data race causing a permanent active client increase
Previously, a data race could cause a newly created fetch context for a new client to be used
before it had been fully initialized, which would cause the query to become stuck; queries for the same
data would be either paused indefinitely or dropped because of
the `clients-per-query` limit. This has been fixed.
Closes #5053
Merge branch '5053-fetch-context-create-data-race' into 'main'
Ondřej Surý [Wed, 19 Feb 2025 05:28:46 +0000 (06:28 +0100)]
Fix the fetch context hash table lock ordering
The order of the fetch context hash table rwlock and the individual
fetch context was reversed when calling the release_fctx() function.
This was causing a problem when iterating the hash table, and thus the
ordering has been corrected in a way that the hash table rwlock is now
always locked on the outside and the fctx lock is the interior lock.
Ondřej Surý [Mon, 17 Feb 2025 13:58:28 +0000 (14:58 +0100)]
Add isc_timer_running() function to check status of timer
In the next commit, we need to know whether the timer has been started
or stopped. Add isc_timer_running() function that returns true if the
timer has been started.
Arаm Sаrgsyаn [Fri, 21 Feb 2025 11:45:00 +0000 (11:45 +0000)]
fix: usr: Fix RPZ race condition during a reconfiguration
With RPZ in use, `named` could terminate unexpectedly because of a race condition when a reconfiguration command was received using `rndc`. This has been fixed.
Closes #5146
Merge branch '5146-rpz-reconfig-bug-fix' into 'main'
Aram Sargsyan [Wed, 5 Feb 2025 09:36:09 +0000 (09:36 +0000)]
Fix RPZ bug when resuming a query during a reconfiguration
After a reconfiguration the old view can be left without a valid
'rpzs' member, because when the RPZ is not changed during the named
reconfiguration 'rpzs' "migrate" from the old view into the new
view, so when a query resumes it can find that 'qctx->view->rpzs'
is NULL which query_resume() currently doesn't expect to happen if
it's recursing and 'qctx->rpz_st' is not NULL.
Fix the issue by adding a NULL-check. In order to not split the log
message to two different log messages depending on whether
'qctx->view->rpzs' is NULL or not, change the message to not log
the RPZ policy's "version" which is just a runtime counter and is
most likely not very useful for the users.
Ondřej Surý [Fri, 21 Feb 2025 09:51:49 +0000 (09:51 +0000)]
chg:nil: Cleanup the isc_counter unit
The isc_counter_create() doesn't need the return value (it was always
ISC_R_SUCCESS), use the macros to implement the reference counting,
little style cleanup, and expand the unit test.
Merge branch 'ondrej/cleanup-isc_counter-unit' into 'main'
Ondřej Surý [Wed, 19 Feb 2025 05:49:38 +0000 (06:49 +0100)]
Cleanup the isc_counter unit
The isc_counter_create() doesn't need the return value (it was always
ISC_R_SUCCESS), use the macros to implement the reference counting,
little style cleanup, and expand the unit test.
Mark Andrews [Fri, 21 Feb 2025 02:57:46 +0000 (02:57 +0000)]
fix: usr: Remove NSEC/DS/NSEC3 RRSIG check from dns_message_parse
Previously, when parsing responses, named incorrectly rejected responses without matching RRSIG records for NSEC/DS/NSEC3 records in the authority section. This rejection, if appropriate, should have been left for the validator to determine and has been fixed.
Closes #5185
Merge branch '5185-remove-rrsig-check-from-dns_message_parse' into 'main'
Mark Andrews [Tue, 18 Feb 2025 23:34:47 +0000 (10:34 +1100)]
Remove check for missing RRSIG records from getsection
Checking whether the authority section is properly signed should
be left to the validator. Checking in getsection (dns_message_parse)
was way too early and resulted in resolution failures of lookups
that should have otherwise succeeded.
Arаm Sаrgsyаn [Thu, 20 Feb 2025 14:24:17 +0000 (14:24 +0000)]
fix: usr: Implement sig0key-checks-limit and sig0message-checks-limit
Previously a hard-coded limitation of maximum two key or message
verification checks were introduced when checking the message's
SIG(0) signature. It was done in order to protect against possible
DoS attacks. The logic behind choosing the number 2 was that more
than a single key should only be required during key rotations, and
in that case two keys are enough. But later it became apparent that
there are other use cases too where even more keys are required, see
issue number #5050 in GitLab.
This change introduces two new configuration options for the views,
`sig0key-checks-limit` and `sig0message-checks-limit`, which define how
many keys are allowed to be checked to find a matching key, and how
many message verifications are allowed to take place once a matching
key has been found. The latter protects against expensive cryptographic
operations when there are keys with colliding tags and algorithm
numbers, with default being 2, and the former protects against a bit
less expensive key parsing operations and defaults to 16.
Closes #5050
Merge branch '5050-sig0-let-considering-more-than-two-keys' into 'main'
Aram Sargsyan [Tue, 21 Jan 2025 13:20:12 +0000 (13:20 +0000)]
Implement sig0key-checks-limit and sig0message-checks-limit
Previously a hard-coded limitation of maximum two key or message
verification checks were introduced when checking the message's
SIG(0) signature. It was done in order to protect against possible
DoS attacks. The logic behind choosing the number two was that more
than one key should only be required only during key rotations, and
in that case two keys are enough. But later it became apparent that
there are other use cases too where even more keys are required, see
issue number #5050 in GitLab.
This change introduces two new configuration options for the views,
sig0key-checks-limit and sig0message-checks-limit, which define how
many keys are allowed to be checked to find a matching key, and how
many message verifications are allowed to take place once a matching
key has been found. The latter protects against expensive cryptographic
operations when there are keys with colliding tags and algorithm
numbers, with default being 2, and the former protects against a bit
less expensive key parsing operations and defaults to 16.
Arаm Sаrgsyаn [Thu, 20 Feb 2025 12:19:46 +0000 (12:19 +0000)]
fix: dev: Fix isc_quota bug
Running jobs which were entered into the isc_quota queue is the
responsibility of the isc_quota_release() function, which, when
releasing a previously acquired quota, checks whether the queue
is empty, and if it's not, it runs a job from the queue without touching
the 'quota->used' counter. This mechanism is susceptible to a possible
hangup of a newly queued job in case when between the time a decision
has been made to queue it (because used >= max) and the time it was
actually queued, the last quota was released. Since there is no more
quotas to be released (unless arriving in the future), the newly
entered job will be stuck in the queue.
Fix the issue by adding checks in both isc_quota_release() and
isc_quota_acquire_cb() to make sure that the described hangup does
not happen. Also see code comments.
Aram Sargsyan [Thu, 6 Feb 2025 15:44:00 +0000 (15:44 +0000)]
Fix isc_quota bug
Running jobs which were entered into the isc_quota queue is the
responsibility of the isc_quota_release() function, which, when
releasing a previously acquired quota, checks whether the queue
is empty, and if it's not, it runs a job from the queue without touching
the 'quota->used' counter. This mechanism is susceptible to a possible
hangup of a newly queued job in case when between the time a decision
has been made to queue it (because used >= max) and the time it was
actually queued, the last quota was released. Since there is no more
quotas to be released (unless arriving in the future), the newly
entered job will be stuck in the queue.
Fix the wrong memory ordering for 'quota->used', as the relaxed
ordering doesn't ensure that data modifications made by one thread
are visible in other threads.
Add checks in both isc_quota_release() and isc_quota_acquire_cb()
to make sure that the described hangup does not happen. Also see
code comments.
Arаm Sаrgsyаn [Thu, 20 Feb 2025 10:31:47 +0000 (10:31 +0000)]
new: usr: Implement the min-transfer-rate-in configuration option
A new option 'min-transfer-rate-in <bytes> <minutes>' has been added
to the view and zone configurations. It can abort incoming zone
transfers which run very slowly due to network related issues, for
example. The default value is set to 10240 bytes in 5 minutes.
Closes #3914
Merge branch '3914-detect-and-restart-stalled-zone-transfers' into 'main'
Aram Sargsyan [Wed, 27 Nov 2024 10:34:40 +0000 (10:34 +0000)]
Expose the incoming transfers' rates in the statistics channel
Expose the average transfer rate (in bytes-per-second) during the
last full 'min-transfer-rate-in <bytes> <minutes>' minutes interval.
If no such interval has passed yet, then the overall average rate is
reported instead.
Aram Sargsyan [Mon, 10 Jun 2024 16:50:37 +0000 (16:50 +0000)]
Test the new min-transfer-rate-in configuration option
Add a new big zone, run a zone transfer in slow mode, and check
whether the zone transfer gets canceled because 100000 bytes are
not transferred in 5 seconds (as it's running in slow mode).
Aram Sargsyan [Mon, 10 Jun 2024 16:48:26 +0000 (16:48 +0000)]
Implement the min-transfer-rate-in configuration option
This new option sets a minimum amount of transfer rate for
an incoming zone transfer that will abort a transfer, which
for some network related reasons run very slowly.
Evan Hunt [Thu, 20 Feb 2025 02:12:12 +0000 (02:12 +0000)]
fix: dev: Do not cache signatures for rejected data
The cache has been updated so that if new data is rejected - for example, because there was already existing data at a higher trust level - then its covering RRSIG will also be rejected.
Closes #5132
Merge branch '5132-improve-cd-behavior' into 'main'
Evan Hunt [Fri, 24 Jan 2025 01:16:30 +0000 (17:16 -0800)]
Check whether a rejected rrset is different
Add a new dns_rdataset_equals() function to check whether two
rdatasets are equal in DNSSEC terms.
When an rdataset being cached is rejected because its trust
level is lower than the existing rdataset, we now check to see
whether the rejected data was identical to the existing data.
This allows us to cache a potentially useful RRSIG when handling
CD=1 queries, while still rejecting RRSIGs that would definitely
have resulted in a validation failure.
Evan Hunt [Wed, 19 Feb 2025 23:43:41 +0000 (23:43 +0000)]
fix: dev: Clean up dns_rdataslab module
Rdata slabs used in the QP databases are usually prepended with a slab header, but are sometimes "raw", containing only the rdata and no header. Previously, to allow for them to be used both ways, functions that operated on them took a `reservelen` argument, which would be set to either the header length or to zero, and skipped over that many bytes at the beginning of the buffer. Most such functions were never used on the raw form. To make the code clearer, each of these functions now operates on full slabs with headers, and an alternate "raw" version of the function has been added in cases where that was needed.
In addition, the `dns_rdataslab_merge()` and `_subtract()` functions have been rewritten for clarity and efficiency, and a minor bug has been fixed in `dns_rdataslab_equal()` and `_equalx()`, which could cause an incorrect result if both slabs being compared had zero length.
Merge branch 'each-refactor-rdataslab' into 'main'
Ondřej Surý [Wed, 19 Feb 2025 12:59:23 +0000 (13:59 +0100)]
Remove the "raw" version of the dns_slabheader API
The "raw" version of the header was used for the noqname and the closest
proofs to save around 152 bytes of the dns_slabheader_t while bringing
an additional complexity. Remove the raw version of the dns_slabheader
API at the slight expense of having unused dns_slabheader_t data sitting
in front of the proofs.
Evan Hunt [Mon, 10 Feb 2025 04:43:39 +0000 (20:43 -0800)]
refactor dns_rdataslab_subtract() for efficiency
reduce the number of rdata comparisons needed by walking
through the original slab once to determine whether the rdata
in it is duplicated in the slab to be subtracted, and then
write out the rdatas that aren't. previously, this was
done twice: once when determining the size of the target buffer
and then again when copying data into it.
Evan Hunt [Mon, 10 Feb 2025 04:03:01 +0000 (20:03 -0800)]
refactor dns_rdataslab_merge() for efficiency
when merging two rdata slabs, we now check once to see
whether an item in the new slab has a duplicate in the
old. previously this was done twice; once to determine the
size of the target buffer required, and then again when
copying the data into it.
we also minimize the number of rdata comparisons necessary,
by remembering which items in the old slab have already been
found to be duplicates.
The function name dns_slabheader_fromrdataset() was too similar
to dns_rdataslab_fromrdataset(). Instead, we now have an rdataset
method 'getheader' which is implemented for slab-type rdatasets.
A new NOHEADER rdataset attribute is set for rdatasets using
raw slabs (i.e., noqname and closest encloser proofs); when
called on rdatasets with that flag set, dns_rdataset_getheader()
returns NULL.
Evan Hunt [Sat, 8 Feb 2025 05:06:34 +0000 (21:06 -0800)]
initialize header in dns_rdataslab_fromrdataset()
when dns_rdataslab_fromrdataset() is run, in addition to
allocating space for a slab header, it also partially
initializes it, setting the type match rdataset->type and
rdataset->covers, the trust to rdataset->trust, and the TTL to
rdataset->ttl.
Evan Hunt [Sat, 8 Feb 2025 04:07:53 +0000 (20:07 -0800)]
clarify dns_rdataslab_fromrdataset()
there are now two functions for creating an rdataslab from an
rdataset: dns_rdataslab_fromrdataset() creates a full slab (including
space for a slab header), and dns_rdataslab_raw_fromrdataset() creates
a raw slab.
Evan Hunt [Sat, 8 Feb 2025 05:21:52 +0000 (21:21 -0800)]
more rdataslab refactoring
- there are now two functions for getting rdataslab size:
dns_rdataslab_size() is for full slabs and dns_rdataslab_sizeraw()
for raw slabs. there is no longer a need for a reservelen parameter.
- dns_rdataslab_count() also no longer takes a reservelen parameter.
(currently it's never used for raw slabs, so there is no _countraw()
function.)
- dns_rdataslab_rdatasize() has been removed, because
dns_rdataslab_sizeraw() can do the same thing.
- dns_rdataslab_merge() and dns_rdataslab_subtract() both take
slabheader parameters instead of character buffers, and the
reservelen parameter has been removed.
Evan Hunt [Thu, 6 Feb 2025 23:50:52 +0000 (15:50 -0800)]
fix and simplify dns_rdataset_equal() and _equalx()
if both rdataslabs being compared have zero length, return true.
also, since these functions are only ever called on slabheaders
with sizeof(dns_slabheader_t) as the reserve length, we can
simplify the API: remove the reservelen argument, and pass the
slabs as type dns_slabheader_t * instead of unsigned char *.
Ondřej Surý [Fri, 31 Jan 2025 14:47:33 +0000 (15:47 +0100)]
Add .up pointer to slabheader
The dns_slabheader object uses the 'next' pointer for two purposes.
In the first header for any given type, 'next' points to the first
header for the next type. But 'down' points to the next header of
the same type, and in that record, 'next' points back up.
This design made the code confusing to read. We now use a union
so that the 'next' pointer can also be called 'up'.
Artem Boldariev [Wed, 19 Feb 2025 16:39:36 +0000 (16:39 +0000)]
fix: dev: Post [CVE-2024-12705] Performance Drop Fixes
This merge request fixes a [performance drop](https://gitlab.isc.org/isc-projects/bind9/-/pipelines/216728) after merging the fixes for #4795, in particular in 9.18.
The MR [fixes the problem](https://gitlab.isc.org/isc-projects/bind9/-/pipelines/219825) without affecting performance for the newer versions, in particular for [the development version](https://gitlab.isc.org/isc-projects/bind9/-/pipelines/220619).
Merge branch 'artem-doh-performance-drop' into 'main'
Artem Boldariev [Wed, 19 Feb 2025 10:28:37 +0000 (12:28 +0200)]
DoH: http_send_outgoing() return value is not used
The value returned by http_send_outgoing() is not used anywhere, so we
make it not return anything (void). Probably it is an omission from
older times.
Artem Boldariev [Thu, 13 Feb 2025 13:05:10 +0000 (15:05 +0200)]
DoH: change how the active streams number is calculated
This commit changes the way how the number of active HTTP streams is
calculated and allows it to scale with the values of the maximum
amount of streams per connection, instead of effectively capping at
STREAM_CLIENTS_PER_CONN.
The original limit, which is intended to define the pipelining limit
for TCP/DoT. However, it appeared to be too restrictive for DoH, as it
works quite differently and implements pipelining at protocol level by
the means of multiplexing multiple streams. That renders each stream
to be effectively a separate connection from the point of view of the
rest of the codebase.
Artem Boldariev [Thu, 13 Feb 2025 12:53:18 +0000 (14:53 +0200)]
DoH: Track the amount of in flight outgoing data
Previously we would limit the amount of incoming data to process based
solely on the presence of not completed send requests. That worked,
however, it was found to severely degrade performance in certain
cases, as was revealed during extended testing.
Now we switch to keeping track of how much data is in flight (or ready
to be in flight) and limit the amount of processed incoming data when
the amount of in flight data surpasses the given threshold, similarly
to like we do in other transports.
Ondřej Surý [Sun, 2 Feb 2025 23:06:48 +0000 (00:06 +0100)]
Unify how we handle database version in the cache
Database versions are not used in cache databases. Some places in
qpcache.c required the version argument to be NULL; others marked it
as UNUSED. Unify all cases to require version to be NULL.
Ondřej Surý [Sun, 2 Feb 2025 19:22:29 +0000 (20:22 +0100)]
Refactor the search in qpcache_findrdataset()
Add new related_headers() function that simplifies the code
flow in qpcache_findrdataset(). Also use check_stale_header() function
to remove code duplication.
Ondřej Surý [Sun, 2 Feb 2025 19:07:42 +0000 (20:07 +0100)]
Refactor simple slabheader matching
Add a helper function both_headers() that unifies the slabheader
matching for simple type: it returns true when both the type and
the matching RRSIG have been found.