Now that this function also creates the DNSKEY record for the KSKs,
as well as other associated records such as CDS and CDNSKEY, rename
the function to something slightly better.
Creating the KSR happens on the "ZSK side". The KSK is offline and while
the public key and state file may be present, draft-icann-dnssec-keymgmt-01.txt
suggest that the KSR only contains ZSKs.
This is also what knot dns does, so it would also be in the spirit of
interoperability.
The final line in a KSR ";; KeySigningRequest generated at ..." was
missing the version number, that has now been fixed.
Thanks Libor Peltan for reporting.
The expireheader() call in the expire_ttl_headers() function
is erroneous as it passes the 'nlocktypep' and 'tlocktypep'
arguments in a wrong order, which then causes an assertion
failure.
Fix the order of the arguments so it corresponds to the function's
prototype.
in QP keys, characters that are not common in DNS names are
encoded as two-octet sequences. this caused a glitch in iterator
positioning when some lookups failed.
consider the case where we're searching for "\009" (represented
in a QP key as {0x03, 0x0c}) and a branch exists for "\000"
(represented as {0x03, 0x03}). we match on the 0x03, and continue
to search down. at the point where we find we have no match,
we need to pop back up to the branch before the 0x03 - which may
be multiple levels up the stack - before we position the iterator.
there were some structure names used in qpcache.c and qpzone.c that
were too similar to each other and could be confusing when debugging.
they have been changed as follows:
in qcache.c:
- changed_t was unused, and has been removed
- search_t -> qpc_search_t
- qpdb_rdatasetiter_t -> qpc_rditer_t
- qpdb_dbiterator_t -> qpc_dbiter_t
in qpzone.c:
- qpdb_changed_t -> qpz_changed_t
- qpdb_changedlist_t -> qpz_changedlist_t
- qpdb_version_t -> qpz_version_t
- qpdb_versionlist_t -> qpz_versionlist_t
- qpdb_search_t -> qpz_search_t
- qpdb_load_t -> qpz_search_t
when calling dns_qp_lookup() from qpcache, instead of passing
'foundname' so that a name would be constructed from the QP key,
we now just use the name field in the node data. this makes
dns_qp_lookup() run faster.
the same optimization has also been added to qpzone.
the documentation for dns_qp_lookup() has been updated to
discuss this performance consideration.
when the cache is over memory, we purge from the LRU list until
we've freed the approximate amount of memory to be added. this
approximation could fail because the memory allocated for nodenames
wasn't being counted.
add a dns_name_size() function so we can look up the size of nodenames,
then add that to the purgesize calculation.
in a cache database, unlike zones, NSEC3 records are stored in
the main tree. it is not necessary to maintain a separate 'nsec3'
tree, nor to have code in the dbiterator implementation to traverse
from one tree to another.
(if we ever implement synth-from-dnssec using NSEC3 records, we'll
need to revert this change. in the meantime, simpler code is better.)
the QP database doesn't support relative names as the RBTDB did, so
there's no need for a 'new_origin' flag or to handle `DNS_R_NEWORIGIN`
result codes.
- remove unneeded struct members and misleading comments.
- remove unused parameters for static functions.
- rename 'find_callback' to 'delegating' for consistency with qpzone;
the find callback mechanism is not used in QP databases.
- change dns_qpdata_t to qpcnode_t (QP cache node), and dns_qpdb_t to
qpcache_t, as these types are only accessed locally.
- also change qpdata_t in qpzone.c to qpznode_t (QP zone node), for
consistency.
- make the refcount declarations for qpcnode_t and qpznode_t static,
using the new ISC_REFCOUNT_STATIC macros.
In qpcache (and rbtdb), there are some functions that acquire
neither the tree lock nor the node lock when calling newref().
In theory, this could lead to a race in which a new reference
is added to a node that was about to be deleted.
We now detect this condition by passing the current tree and node
lock status to newref(). If the node was previously unreferenced
and we don't hold at least one read lock, we will assert.
The FreeBSD autoscaler has been configured to utilize the new "instance"
GitLab Runner executor to spawn "stress" test CI jobs on AWS EC2
dynamically. A shared GitLab Runner named "freebsd-instance-autoscaler"
has been set up in GitLab CI/CD to communicate with EC2, provisioning VM
instances on demand based on a FreeBSD 13 AMI image. This image is the
same as the one previously used for FreeBSD "stress" tests before the
implementation of autoscaling (specifically, the
"freebsd13-amd64-bind9stress.aws.lab.isc.org" GitLab Runner in CI/CD).
Since the BIND 9 QA repository has been made public, adjust the relevant
URLs and paths used in .gitlab-ci.yml so that they work with the public
version of that repository.
Because some tests don't have a legtimate handle, provide a temporary
return early that should be fixed and removed before squashing. This
short circuiting is still correct until DoQ/DoH3 support is introduced.
GCC might fail to compile because it expects a return after UNREACHABLE.
It should ideally just work anyway since UNREACHABLE is either a
noreturn or UB (__builtin_unreachable / C23 unreachable).
Either way, it should be optimized almost always so the fallback is
free or basically free anyway when it isn't optimized out.
the previous commit introduced a possible race in getsigningtime()
where the rdataset header could change between being found on the
heap and being bound.
getsigningtime() now looks at the first element of the heap, gathers the
locknum, locks the respective lock, and retrieves the header from the
heap again. If the locknum has changed, it will rinse and repeat.
Theoretically, this could spin forever, but practically, it almost never
will as the heap changes on the zone are very rare.
we simplify matters further by changing the dns_db_getsigningtime()
API call. instead of passing back a bound rdataset, we pass back the
information the caller actually needed: the resigning time, owner name
and type of the rdataset that was first on the heap.
in RBTDB, the heap was used by zone databases for resigning, and
by the cache for TTL-based cache cleaning. the cache use case required
very frequent updates, so there was a separate heap for each of the
node lock buckets.
qpzone is for zones only, so it doesn't need to support the cache
use case; the heap will only be touched when the zone is updated or
incrementally signed. we can simplify the code by using only a single
heap.
fix_iterator() and related functions are quite difficult to read.
perhaps it would be a little clearer if we didn't assign values
to variables that won't subsequently be used, or unnecessarily
pop the stack and then push the same value back onto it.
also, in dns_qp_lookup() we previously called fix_iterator(),
removed the leaf from the top of the iterator stack, and then
added it back on. this would be clearer if we just push the leaf
onto the stack when we need to, but leave the stack alone when
it's already complete.
under some circumstances it was possible for the iterator to
be set to the first leaf in a set of twigs, when it should have
been set to the last.
a unit test has been added to test this scenario. if there is a
a tree containing the following values: {".", "abb.", "abc."}, and
we query for "acb.", previously the iterator would be positioned at
"abb." instead of "abc.".
the tree structure is:
branch (offset 1, ".")
branch (offset 3, ".ab")
leaf (".abb")
leaf (".abc")
we find the branch with offset 3 (indicating that its twigs differ
from each other in the third position of the label, "abB" vs "abC").
but the search key differs from the found keys at position 2
("aC" vs "aB"). we look up the bit value in position 3 of the
search key ("B"), and incorrectly follow it onto the wrong twig
("abB").
to correct for this, we need to check for the case where the search
key is greater than the found key in a position earlier than the
branch offset. if it is, then we need to pop from the current leaf
to its parent, and get the greatest leaf from there.
a further change is needed to ensure that we don't do this twice;
when we've moved to a new leaf and the point of difference between
it and the search key even earlier than before, then we're definitely
at a predecessor node and there's no need to continue the loop.
