Tony Finch [Tue, 14 Feb 2023 16:13:16 +0000 (16:13 +0000)]
Use a qp-trie for the zone table
This change makes the zone table lock-free for reads. Previously, the
zone table used a red-black tree, which is not thread safe, so the hot
read path acquired both the per-view mutex and the per-zonetable
rwlock. (The double locking was to fix to cleanup races on shutdown.)
One visible difference is that zones are not necessarily shut down
promptly: it depends on when the qp-trie garbage collector cleans up
the zone table. The `catz` system test checks several times that zones
have been deleted; the test now checks for zones to be removed from
the server configuration, instead of being fully shut down. The catz
test does not churn through enough zones to trigger a gc, so the zones
are not fully detached until the server exits.
After this change, it is still possible to improve the way we handle
changes to the zone table, for instance, batching changes, or better
compaction heuristics.
Tony Finch [Fri, 3 Mar 2023 12:05:51 +0000 (12:05 +0000)]
Compact more in dns_qp_compact(DNS_QPGC_ALL)
Commit 0858514ae8 enriched dns_qp_compact() to give callers more
control over how thoroughly the trie should be compacted.
In the DNS_QPGC_ALL case, if the trie is small it might be compacted
to a new position in the same memory chunk. In this situation it will
still be holding references to old leaf objects which have been
removed from the trie but will not be completely detached until the
chunk containing the references is freed.
This change resets the qp-trie allocator to a fresh chunk before a
DNS_QPGC_ALL compaction, so all the old memory chunks will be
evacuated and old leaf objects can be detached sooner.
Tony Finch [Thu, 2 Mar 2023 13:30:24 +0000 (13:30 +0000)]
Support for off-loop read-ony qp-trie transactions
It is sometimes necessary to access a qp-trie outside an isc_loop,
such as in tests or an isc_work callback. The best option was to use
a `dns_qpmulti_write()` transaction, but that has overheads that are
not necessary for read-only access, such as committing a new version
of the trie even when nothing changed.
So this commit adds a `dns_qpmulti_read()` transaction, which is
nearly as lightweight as a query transaction, but it takes the mutex
like a write transaction.
Tony Finch [Fri, 10 Feb 2023 16:53:31 +0000 (16:53 +0000)]
Support for finding the longest parent domain in a qp-trie
This is the first of the "fancy" searches that know how the DNS
namespace maps on to the structure of a qp-trie. For example, it will
find the closest enclosing zone in the zone tree.
In the check_algorithm() function openssleddsa_alg_info() is
called with two known variants of the 'algorithm' argument, and
both are expected to return a non-NULL value.
Add an INSIST to suppress the following GCC 12 analyzer report:
openssleddsa_link.c: In function 'raw_key_to_ossl':
openssleddsa_link.c:92:13: error: dereference of NULL 'alginfo' [CWE-476] [-Werror=analyzer-null-dereference]
92 | int pkey_type = alginfo->pkey_type;
| ^~~~~~~~~
Mark Andrews [Tue, 4 Apr 2023 01:01:36 +0000 (11:01 +1000)]
Remove 'inst != NULL' from cleanup check in plugin_register
'inst' is guarenteed to be non NULL at this point.
358 *instp = inst;
359
360cleanup:
CID 281450 (#2 of 2): Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking inst suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
361 if (result != ISC_R_SUCCESS && inst != NULL) {
362 plugin_destroy((void **)&inst);
363 }
364
365 return (result);
Bump the requirement in the shutdown test to dnspython 2.0.0
The dnspython.Resolve.resolve() requires at least dnspython >= 2.0.0,
this wasn't enforced in the shutdown system test leading to infinite
loop waiting for the server start due to failing resolve() call.
Ondřej Surý [Wed, 22 Mar 2023 14:11:17 +0000 (15:11 +0100)]
Add test for RPZ in multiple views
This adds rudimentary test for response-policy zones in multiple
views. Different combinations are tested:
- two views with response-policy inherited from options {};
- two views view explicit response-policy using same RPZ zone name
- two views view explicit response-policy using secondary RPZ zone
Ondřej Surý [Thu, 30 Mar 2023 19:19:17 +0000 (21:19 +0200)]
Change dns_adbentry_overquota() to dns_adb_overquota()
The dns_adbentry_overquota() was violating the layers accessing the
adbentry struct members directly. Change it to dns_adb_overquota() to
match the dns_adb API.
Attach catzs to catz instead of doing this explicitly
Instead of explicitly adding a reference to catzs (catalog zones) when
calling the update callback, attach the catzs to the catz (catalog zone)
object to keep it referenced for the whole time the catz exists.
As we are now using dispatch instead of netmgr for XFR TCP connection,
the xfrin_recv_done() will be called when cancelling the dispatch with
ISC_R_CANCELED. This could lead to double detach from the dns_xfrin_t,
one in the xfrin_recv_done() and one in the dns_xfrin_shutdown().
Remove the extra detach from the dns_xfrin_shutdown() and rely on the
dispatch read callback to be always called.
use ISC_REFCOUNT_IMPL for external dns_zone references
use the ISC_REFCOUNT implementation for dns_zone_attach() and
_detach(). (this applies only to external zone references, not
to dns_zone_iattach() and dns_zone_idetach().)
use dns_zone_ref() where previously a dummy zone object had been
used to increment the reference count.
Mark Andrews [Thu, 24 Nov 2022 03:18:20 +0000 (14:18 +1100)]
Reduce the number of verifiations required
In selfsigned_dnskey only call dns_dnssec_verify if the signature's
key id matches a revoked key, the trust is pending and the key
matches a trust anchor. Previously named was calling dns_dnssec_verify
unconditionally resulted in busy work.
Aram Sargsyan [Wed, 28 Dec 2022 10:20:37 +0000 (10:20 +0000)]
nsupdate: use the configurable timeout and retry values for all queries
The 'nsupdate' tool, when sending SOA queries, uses a hard-coded value
3 UDP retries and of 5 seconds of timeout for UDP queries, and 100
seconds of timeout for TCP queries.
Use the timeout and retry values which can be configured using the
-t, -u, -r command line options, and which are already used for
sending the update query.
Aram Sargsyan [Mon, 12 Dec 2022 12:12:13 +0000 (12:12 +0000)]
Do not resend TCP requests
The req_response() function is using 'udpcount' variable to resend
the request 'udpcount' times on timeout even for TCP requests,
which does not make sense, as it would use the same connection.
Add a condition to use the resend logic only for UDP requests.
Aram Sargsyan [Mon, 12 Dec 2022 11:55:09 +0000 (11:55 +0000)]
Synchronize dns_request_createraw() and dns_request_create() UDP timeout
The dns_request_createraw() function, unlike dns_request_create(), when
calculating the UDP timeout value, doesn't check that 'udpretries' is
not zero, and that is the more logical behavior, because the calculation
formula uses division to 'udpretries + 1', where '1' is the first try.
Change the dns_request_create() function to remove the 'udpretries != 0'
condition.
Add a 'REQUIRE(udpretries != UINT_MAX)' check to protect from a division
by zero.
Make the 'request->udpcount' field to represent the number of tries,
instead of the number of retries.
Aram Sargsyan [Wed, 28 Dec 2022 15:55:43 +0000 (15:55 +0000)]
Add nsupdate timeout tests
* nsupdate should take 12 seconds (one try and three retries with
3 second timeout for each), UDP mode
* nsupdate -u 4 -r 1 should take 8 seconds (one try and one retry with
4 second timeout for each), UDP mode
* nsupdate -u 0 -t 8 -r 1 should also take 8 seconds, UDP mode
* nsupdate -u 4 -t 30 -r 1 should also take 8 seconds, as -u takes
precedence over -t, UDP mode
* nsupdate -t 8 -v should also take 8 seconds, TCP mode
Tony Finch [Mon, 3 Apr 2023 13:20:20 +0000 (14:20 +0100)]
More dns_qpkey_t safety checks
My original idea had been that the core qp-trie code would be mostly
independent of the storage for keys, so I did not make it check at run
time that key lengths are sensible. However, the qp-trie search
routines need to get keys out of leaf objects, for which they provide
storage on the stack, which is particularly dangerous for unchecked
buffer overflows. So this change checks that key lengths are in bounds
at the API boundary between the qp-trie code and the rest of BIND, and
there is no more pretence that keys might be longer.
Matthijs Mekking [Wed, 29 Mar 2023 09:26:22 +0000 (11:26 +0200)]
Make checkds system test more resilient
The checkds system test could fail if some parent secondary servers did
not yet loaded all the zones before ns9 started sending DS queries. This
leads to SERVFAIL responses, while the test case expects good DS
responses. In order to mitigate against this issue, call 'rndc loadkeys'
to quickly restart the checkds procedure again.
Also refactor the checkds system test, to get rid of the many zone
name duplications. Update the functions 'zone_check' and
'keystate_check' to make the zone name an FQDN so we can just pass
the 'zone' variable into the function.
Matthijs Mekking [Tue, 28 Mar 2023 14:57:58 +0000 (16:57 +0200)]
Determine checkds default from config
If the 'checkds' option is not explicitly set, check if there are
'parental-agents' for the zone configured. If so, default to "explicit",
otherwise default to "yes".
Matthijs Mekking [Tue, 28 Mar 2023 14:47:16 +0000 (16:47 +0200)]
Add two checkds test servers
Add two new checkds test servers, that are hidden secondaries (hidden
as in not published in the NS RRset), that can be used specifically
for testing explicitly configured parental-agents.
Matthijs Mekking [Tue, 28 Mar 2023 13:55:51 +0000 (15:55 +0200)]
Implement auto parental-agents (checkds yes)
Implement the new feature, automatic parental-agents. This is enabled
with 'checkds yes'.
When set to 'yes', instead of querying the explicit configured
parental agents, look up the parental agents by resolving the parent
NS records. The found parent NS RRset is considered to be the list
of parental agents that should be queried during a KSK rollover,
looking up the DS RRset corresponding to the key signing keys.
For each NS record, look up the addresses in the ADB. These addresses
will be used to send the DS requests. Count the number of servers and
keep track of how many good DS responses were seen.
Matthijs Mekking [Tue, 28 Mar 2023 12:35:57 +0000 (14:35 +0200)]
Add test cases for 'checkds no'
Add test cases for when checkds is disabled. Copy the test cases that
would have resulted in a DSPublish or DSRemoved and make sure that
with 'checkds no' the metadata is not set.
Matthijs Mekking [Tue, 28 Mar 2023 10:00:56 +0000 (12:00 +0200)]
Add test cases for 'checkds yes'
Add the test cases for automatic parental-agents, i.e. when 'checkds'
is set to 'yes'. Split out the special cases that use a reference
or a resolver as parental-agent so that the common use cases can be
tested with the same function.
Matthijs Mekking [Fri, 24 Mar 2023 16:22:24 +0000 (17:22 +0100)]
Update checkds system test
Make the checkds system test more structured with the many more test
cases to come. Add a README for clarity.
Update the 'has_signed_apex_nsec' helper function so it can take any
domain name regardless of the number of labels.
Change the DNS tree structure such that we have different TLD names
for the various test scenarios, because we need servers that respond
differently to DS queries. Note that this isn't applicable to the
existing "checkds explicit" test cases, but is preparation work for
testing "checkds yes" (automatic parental agents).
Add a trust-anchor to the server that will be querying for parent
NS records.
Mark Andrews [Wed, 8 Mar 2023 05:05:03 +0000 (16:05 +1100)]
Silence NULL pointer dereferene false positive
Only attempt to digest 'in' if it is non NULL. This will prevent
false positives about NULL pointer dereferences against 'in' and
should also speed up the processing.
Artem Boldariev [Mon, 26 Dec 2022 15:42:49 +0000 (17:42 +0200)]
Stream DNS: avoid memory copying/buffer resizing when reading data
This commit optimises isc_dnsstream_assembler_t in such a way that
memory copying and reallocation are avoided when receiving one or more
complete DNS messages at once. We try to handle the data from the
messages directly, without storing them in an intermediate memory
buffer.
In write_public_key() and write_key_state(), there were left-over checks
for result, that were effectively dead code after the last refactoring.
Remove those.
Tony Finch [Thu, 16 Mar 2023 14:31:46 +0000 (14:31 +0000)]
Use isc_histo for the message size statistics
This should have no functional effects.
The message size stats are specified by RSSAC002 so it's best not
to mess around with how they appear in the statschannel. But it's
worth changing the implementation to use general-purpose histograms,
to reduce code size and benefit from sharded counters.
Tony Finch [Mon, 20 Mar 2023 15:05:36 +0000 (15:05 +0000)]
Simplify histogram quantiles
The `isc_histosummary_t` functions were written in the early days of
`hg64` and carried over when I brought `hg64` into BIND. They were
intended to be useful for graphing cumulative frequency distributions
and the like, but in practice whatever draws charts is better off with
a raw histogram export. Especially because of the poor performance of
the old functions.
The replacement `isc_histo_quantiles()` function is intended for
providing a few quantile values in BIND's stats channel, when the user
does not want the full histogram. Unlike the old functions, the caller
provides all the query fractions up-front, so that the values can be
found in a single scan instead of a scan per value. The scan is from
larger values to smaller, since larger quantiles are usually more
interesting, so the scan can bail out early.
Tony Finch [Thu, 16 Mar 2023 09:46:15 +0000 (09:46 +0000)]
Add per-thread sharded histograms for heavy loads
Although an `isc_histo_t` is thread-safe, it can suffer
from cache contention under heavy load. To avoid this,
an `isc_histomulti_t` contains a histogram per thread,
so updates are local and low-contention.
Tony Finch [Wed, 8 Mar 2023 09:55:42 +0000 (09:55 +0000)]
Add isc_histo for histogram statistics
This is an adaptation of my `hg64` experiments for use in BIND.
As well as renaming everything according to ISC style, I have
written some more extensive tests that ensure the edge cases are
correct and the fenceposts are in the right places.
I have added utility functions for working with precision in terms of
decimal significant figures as well as this code's native binary.
Remove the reference to setting the DF-flag as we don't do that right
now. Rephrase the paragraph that the default value should not be
causing fragmentation.
Ondřej Surý [Thu, 30 Mar 2023 15:35:00 +0000 (17:35 +0200)]
Cleanup the last Windows / MSC ifdefs and comments
Cleanup the remnants of MS Compiler bits from <isc/refcount.h>, printing
the information in named/main.c, and cleanup some comments about Windows
that no longer apply.
The bits in picohttpparser.{h,c} were left out, because it's not our
code.
Tom Krizek [Mon, 13 Mar 2023 12:36:24 +0000 (13:36 +0100)]
Find errors in dig output in system tests
Facilitate faster system test failure identification and debugging by
checking any dig outputs for errors, which are typically indicative of
CI runner network / load issues.
projects / thirdparty / bind9.git / log
