Mark Andrews [Thu, 2 May 2024 05:31:18 +0000 (15:31 +1000)]
Address qp/rbtdb backup file style differences
qp and rbtdb produce stylistically different backup files. This
was causing the xferquota system test to fail. This has been
addressed by making the test independent of the stylistic differences.
Petr Špaček [Tue, 7 May 2024 11:24:43 +0000 (13:24 +0200)]
Update KSK root sentinel references
The mechanism was published as RFC 8509. I've briefly looked at diff
between versions -08 and the RFC and did not find significant protocol
change. Quick manual check confirms what we seem to comply with the
published protocol.
Petr Špaček [Tue, 7 May 2024 11:11:03 +0000 (13:11 +0200)]
Mention RFC 9276 Guidance for NSEC3 Parameter Settings
Draft was eventually published as RFC 9276 but we did not update our
docs. Also add couple mentions in relevant places in the ARM and
dnssec-signzone man page, mainly around "do not touch" places.
Mark Andrews [Thu, 18 Jan 2024 07:54:09 +0000 (18:54 +1100)]
Remove infinite loop on ISC_R_NOFILE
When parsing a zonefile named-checkzone (and others) could loop
infinitely if a directory was $INCLUDED. Record the error and treat
as EOF when looking for multiple errors.
Mark Andrews [Fri, 19 Jan 2024 01:37:10 +0000 (12:37 +1100)]
Address infinite loop when processing $GENERATE
In nibble mode if the value to be converted was negative the parser
would loop forever. Process the value as an unsigned int instead
of as an int to prevent sign extension when shifting.
Matthijs Mekking [Wed, 31 Jan 2024 12:55:29 +0000 (13:55 +0100)]
Update hours-vs-days test
This test is outdated because it tested the 'sig-validity-interval'
option that has been replaced by dnssec-policy's 'signatures-validity',
'signatures-refresh', and 'signatures-jitter' options.
Nevertheless, it tests if the jitter is spread correctly.
Update the test to make use of 'signatures-jitter', set the value
to 1 day (meaning resign in 499 days since 'signatures-validity' is
set to 500 days).
Note that this previously changed erroneously the refresh value to
449 days (should have been 499 days, but that is not allowed by
checkconf, since it is above 90% of 'signatures-validity').
Matthijs Mekking [Tue, 30 Jan 2024 16:21:34 +0000 (17:21 +0100)]
Fix dnssec system test iterations too high
After we have changed the maximum allowed iterations to 51 for signing,
the NSEC3 chain has changed and requires one more NSEC to be returned
in the answer (plus corresponding RRSIG). So the expected number or
records in the authority section is now 8.
Matthijs Mekking [Tue, 30 Jan 2024 16:10:28 +0000 (17:10 +0100)]
Offline KSK and dnssec-policy not yet supported
If the key is offline and the keymgr runs, it will treat it as a missing key,
and generate a new key (according to the policy). Fix the test by putting
back the KSK temporarily when we run 'rndc loadkeys'.
Matthijs Mekking [Tue, 30 Jan 2024 15:16:47 +0000 (16:16 +0100)]
Fix ZSK rollovers in dnssec system test
1. When generating keys, don't set timing metadata. Otherwise keys
are considered to be in use and won't be selected when dnssec-policy
starts a new key rollover.
2. Add an extra check to make sure the new ZSK (zsk2) is prepublished.
Also add a check to make sure it has become active.
3. When using dnssec-settime, add -s to also write to key state files.
Tom Krizek [Fri, 19 Jan 2024 15:01:47 +0000 (16:01 +0100)]
Ensure dnssec test doesn't leak queries to root servers
The config was recently modified to ensure ns4 won't leak any queries to
root servers. However, the test wasn't executed and it turns out the way
this was handled actually broke the test case. Add our custom root hint
to both of the views to ensure the test can still pass without leaking
any queries.
An RPZ response's SOA record TTL is set to 1 instead of the SOA TTL,
a boolean value is passed on to query_addsoa, which is supposed to be
a TTL value. I don't see what value is appropriate to be used for
overriding, so we will pass UINT32_MAX.
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.
Aram Sargsyan [Thu, 2 May 2024 08:33:51 +0000 (08:33 +0000)]
Fix a bug in expireheader() call arguments order
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
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.
Evan Hunt [Thu, 14 Mar 2024 23:46:52 +0000 (16:46 -0700)]
include the nodenames when calculating memory to purge
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.
Evan Hunt [Wed, 13 Mar 2024 05:19:47 +0000 (22:19 -0700)]
simplify qpcache iterators
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.)
clean up unnecessary dbiterator code related to origin
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.
Evan Hunt [Tue, 12 Mar 2024 08:05:07 +0000 (01:05 -0700)]
more cleanups in qpcache.c
- 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.
Ondřej Surý [Tue, 26 Mar 2024 13:13:24 +0000 (14:13 +0100)]
Improve the reference counting checks in newref()
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.
Michal Nowak [Thu, 4 Apr 2024 10:59:50 +0000 (12:59 +0200)]
Use FreeBSD autoscaler for "stress" tests
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).
Michał Kępień [Fri, 26 Apr 2024 16:43:07 +0000 (18:43 +0200)]
Update URLs and paths for the BIND 9 QA repository
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.
Aydın Mercan [Thu, 15 Feb 2024 10:30:42 +0000 (13:30 +0300)]
Provide an early escape hatch for ns_client_transport_type
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.
Aydın Mercan [Wed, 7 Feb 2024 08:35:59 +0000 (11:35 +0300)]
Add fallback to ns_client_get_type despite unreachable
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.
Ondřej Surý [Tue, 26 Mar 2024 00:23:19 +0000 (17:23 -0700)]
avoid a race in the qpzone getsigningtime() implementation
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.
Evan Hunt [Wed, 13 Mar 2024 22:59:53 +0000 (15:59 -0700)]
simplify qpzone database by using only one heap for resigning
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.
simplify code by removing return values where possible
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.