Add `dns_delegdb_t`, a qpmulti-based database enabling to lookup a
delegation set (`dns_delegset_t`) from a zonecut name (`dns_name_t`). A
delegation set object essentially contains an expiration time and a list
of delegation (`dns_deleg_t`). Finally, a delegation can be either:
- A list of IP addresses (`isc_netaddrlist_t`), for NS-based delegation
providing glues or DELEG-based delegation using `server-ipv4=` or
`server-ipv6=`;
- Or a list of nameserver names, for NS-based delegation without glues,
or DELEG-based delegation using `server-name=`;
- Or a list of nameserver names, for DELEG-based delegation using
`include-delegparam=`.
The delegation database API provides lookup by closest zonecut,
delegation and delegation set builders as well as insertion of those
newly built delegation set, dumping to a `FILE *`, conversion from an NS
rdataset to a delegation set, deletion of a specific zonecut or all the
sub-tree of a given zonecut.
A memory context is internally used inside the delegation database and
can be constraint to a maximum size. Once it gets close to its maximum
size and a new delegation set is inserted into the database, a
reclamation flow is run internally removing the least recently used
entries.
The delegation set and delegation objects are, once they been inserted
into the database, read-only object. Thus, the caller can use them
without concurrency or locking concerns, and must detached them once its
done with it.
Since it is impossible to increase an isc_statsmulti counter and
retrieve the new counter atomically, and we need the output of
recursclients in order to compute ns_highwater_recursive, we change the
recursclients counter to an isc_stats one.
In the current statistics counter implementation, the statistics are
backed by an array of counters, which are updated via atomic operations.
This leads to contention, especially on high core count
machines.
This commit introduces a new isc_statsmulti_t counter that keeps a
separate array per thread. These counters are then aggregated only when
statistics are queried, shifting work off the critical path.
These changes lead to a ~2% improvement in perflab.
Add a REQUIRE(isc_loop() == loop) assertion to isc_work_enqueue()
to strictly enforce that work is enqueued from the loop it is
assigned to. This loudly prohibits cross-thread queue manipulation
before it inevitably turns into a concurrency debugging nightmare.
Adds text and wire format unit tests to verify the newly enforced
maximum NSEC3 hash length constraints. These tests ensure that hash
lengths up to the 39-byte maximum are accepted, while larger sizes
correctly fail.
NSEC3 hashes are required to fit within a single DNS label. Since there
are 5 bits per label byte without pad characters, the maximum hash size
is floor(63*5/8) (39 bytes).
This patch enforces this maximum length for unknown algorithms, while
strictly enforcing the exact expected digest length for known algorithms
like SHA-1.
Add two short records to example.com.db that cause assertion failures
when converted to wire form.
The checks added to tests.sh are technically not required: the relevant
assertion failures are already hit when the zone is transferred out of
ns1.
Update the relevant unit tests with 1-byte records.
Co-authored-by: Mark Andrews <marka@isc.org>
Instead of the `EVP_MD_CTX` based functions, use either the new
`EVP_MAC` or the old `HMAC_CTX` based functions.
`EVP_MAC` is the recommended way using using MAC functions in post-3.0
while `HMAC_CTX` is used internally by `EVP_MD_CTX`, making the latter
redundant.
Get rid of the OpenSSL-isms that plague the codebase where the hash type
is `EVP_MD *`
By using a proper enum, alongside the cleanup, we also get the ability
to use constants for known hash sizes instead of having a function call
every time.
`EVP_MD_CTX_get0_md` has been removed instead of being adapted since it
wasn't used anymore.
These functions don't need to be called from multiple places and
by making them static we will detect when they are not added to the
list functions to be tested.
Test that closing a writer with a rollback, then opening another
writer and adding a rdataset (while still holding the node reference)
works correctly.
This test checks that the bugfix in the previous commit is correct.
When built with '-Doptimization=1', GCC produces a compiler warning:
In file included from ../lib/isc/include/isc/lib.h:44,
from ../tests/dns/qp_test.c:26:
../tests/dns/qp_test.c: In function ‘check_predecessors_withchain’:
../lib/isc/include/isc/util.h:182:33: error: ‘ival’ may be used uninitialized [-Werror=maybe-uninitialized]
182 | (((a) == (b)) ? (void)0 : (_assert_int_equal(a, b, f, l), abort()))
| ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/cmocka.h🔢5: note: in expansion of macro ‘_assert_int_equal’
1234 | _assert_int_equal(cast_to_largest_integral_type(a), \
| ^~~~~~~~~~~~~~~~~
../tests/dns/qp_test.c:859:18: note: ‘ival’ was declared here
859 | uint32_t ival;
| ^~~~
This is apparently a false positive, because the qpiter_prev_with_name()
function, when returning ISC_R_SUCCESS, sets the 'ival' (passed to
it by a pointer), and the caller checks that the return value is
ISC_R_SUCCESS before using 'ival'.
Initialize the 'ival' variable to 0 anyway to avoid the build error.
The `assert_in_range()` function was deprecated in favor of
`assert_int_in_range()` and `assert_uint_in_range()`. Add compatibility
shims for cmocka<2.0.0 and use the new functions.
Where applicable, use the more detailed CMocka generated JUnit
reports which include subtest results and timings instead of the
one generated by Meson.
Flaky tests also require retrying, so use a wrapper and mark them
with a environment variable. This is done to avoid the need to compute
an intersection of suites in Meson which is not supported out-of-the-box
(`meson test --suite=foo,bar` runs the union of foo and bar).
CMocka uses group names in the JUnit output.
Use dirname_filename as the group name, as there duplicate testnames
(e.g. time exists both in isc/ and dns/)
This is a new seek function for dbiterator that is meant to find an
NSEC3 node in a zone database. The difference with dns_dbiterator_seek
is that if the node does not exist, this seek function will point the
iterator to the next NSEC3 name.
The removal of the foundname and name parameters from various qp.c
functions led to formatting issues. Restore the correct formatting via
clang-format.
Outside of unit tests, the name parameter in dns_qpiter_<...> and
dns_qpchain_<...> is only used in context where the name can be
extracted directly from the underlying node.
This commits modifies the signatures of dns_qpiter_<...> and
dns_qpchain_<...> not to have a name parameter. Where the name parameter
was needed, we now query the node and copy the name directly from it.
This allows us to remove maybe_set_name from qp.c. Besides simplifying
the API, this leads to a performance speedup for NXDOMAIN handling,
as we avoid calling maybe_set_name inside step, and maybe_set_name is
very inefficient.
A copy of the implementation maybe_set_name is retained for the unit
tests.
The `foundname` parameter in dns_qp_lookup is used only in the unit
tests. This commit simplifies the API by removing it, and modifying the
unit tests to extract the name from pval.
This commit implements a batch update function for qpzone. The main
reason for this is speed: using addrdataset would cause a qp transaction
per rrdataset added, leading to a substantial slowdown compared to
RBTDB. The new API results in a qp transaction per applied diff.
Make all non-scalar properties of `cfg_obj_t` allocated values, which
ensures the union size is the width of one pointer. Also reorder the
fields inside `cfg_obj_t` to avoid alignment padding that would increase
the size. As a result, a `cfg_obj_t` instance is now 48 bytes on a
64-bit platform.
Add a static assertion to avoid increasing the size of the struct by
mistake.
The function `parse_sockaddrsub` was taking advantage of the fact that
both sockaddr and sockaddrtls were in the same position, and used to
initialize the sockaddr field independently if this was a -tls one or
not. This doesn't work anymore now that all fields are allocated,
so it has been slightly rewritten to take both cases into account
separately.
previously, there were over 40 separate definitions of CHECK macros, of
which most used "goto cleanup", and the rest "goto failure" or "goto
out". there were another 10 definitions of RETERR, of which most were
identical to CHECK, but some simply returned a result code instead of
jumping to a cleanup label.
this has now been standardized throughout the code base: RETERR is for
returning an error code in the case of an error, and CHECK is for jumping
to a cleanup tag, which is now always called "cleanup". both macros are
defined in isc/util.h.
The isc_stdtime_now() function used by dns_unreachcache_find() to
check if the entry needs to be expired has a one-second resolution,
and the test sleeps for 1 second and then for the amount of the
expiration interval, which in a worst-case scenario can cause the
test to fail, because the entry was expected to be expired but it
wasn't. Sleep for 2 seconds instead of 1 to avoid the timing
resolution issue.
the merging of options and defaults into the effective configuration
broke the mutual inheritance of the allow-recursion, allow-query, and
allow-query-cache ACLs, and of the allow-recursion-on and
allow-query-cache-on ACLs.
this has been corrected by adding a 'cloned' flag to the cfg_obj
structure to indicate whether it was configured explicitly or
cloned from the defaults during parsing. we can then adjust the
ACLs while configuring a view, favoring user-configured values
when they're available over cloned defaults.
currently the adjustments to the ACLs are done in configure_view();
later they'll be moved into the effective configuration and this
special handling can be removed.
Parser test could crash because the `dumpb2` buffer hasn't explicit C
NULL string termination after dumping the configuration tree in it.
`cfg_printx` does not doing this by default.
Fix the test by comparing only the strings written with strncmp.
In order to make upcoming configuration tree changes easier, the
cfg_map_firstclause() and _nextclause() functions have been changed
to return the clause itself rather than only the clause name.
Add a unit test for `cfg_obj_clone` to verify that the cloned tree
indeed has independent child nodes. The test also verifies that the
clone is semantically correct by comparing a text dump of the original
tree and the cloned one.
