Matthijs Mekking [Wed, 13 Dec 2023 08:38:17 +0000 (09:38 +0100)]
Remove kasp mutex lock
Multiple zones should be able to read the same key and signing policy
at the same time. Since writing the kasp lock only happens during
reconfiguration, and the complete kasp list is being replaced, there
is actually no need for a lock. Reference counting ensures that a kasp
structure is not destroyed when still being attached to one or more
zones.
This significantly improves the load configuration time.
Mark Andrews [Mon, 18 Dec 2023 00:23:21 +0000 (11:23 +1100)]
Regression check for missing RRSIGs
When transitioning from NSEC3 to NSEC the added records where not
being signed because the wrong time was being used to determine if
a key should be used or not. Check that these records are actually
signed.
Mark Andrews [Thu, 14 Dec 2023 22:42:10 +0000 (09:42 +1100)]
Use 'now' rather than 'inception' in 'add_sigs'
When kasp support was added 'inception' was used as a proxy for
'now' and resulted in signatures not being generated or the wrong
signatures being generated. 'inception' is the time to be set
in the signatures being generated and is usually in the past to
allow for clock skew. 'now' determines what keys are to be used
for signing.
Michał Kępień [Mon, 18 Dec 2023 14:11:39 +0000 (15:11 +0100)]
"trust-anchor-telemetry" is no longer experimental
Remove the CFG_CLAUSEFLAG_EXPERIMENTAL flag from the
"trust-anchor-telemetry" statement as the behavior of the latter has not
been changed since its initial implementation and there are currently no
plans to do so. This silences a relevant log message that was emitted
even when the feature was explicitly disabled.
Michał Kępień [Mon, 18 Dec 2023 10:33:43 +0000 (11:33 +0100)]
Fix reference counting in do_nsfetch()
Each function queuing a do_nsfetch() call using isc_async_run() is
expected to increase the given zone's internal reference count
(zone->irefs), which is then correspondingly decreased in either
do_nsfetch() itself (when the dns_resolver_createfetch() fails) or in
nsfetch_done() (when recursion is finished).
However, do_nsfetch() can also return early if either the zone itself or
the relevant view's resolver object is being shut down. In that case,
do_nsfetch() simply returns without decreasing the internal reference
count for the zone. This leaves a dangling zone reference around, which
leads to hangs during named shutdown.
Fix by executing the same cleanup code for early returns from
do_nsfetch() as for a failed dns_resolver_createfetch() call in that
function as the reference count will not be decreased in nsfetch_done()
in any of these cases.
Michał Kępień [Mon, 18 Dec 2023 10:07:04 +0000 (11:07 +0100)]
Prevent an infinite loop in shutdown_listener()
The loop in shutdown_listener() assumes that the reference count for
every controlconnection_t object on the listener->connections linked
list will drop down to zero after the conn_shutdown() call in the loop's
body. However, when the timing is just right, some netmgr callbacks for
a given control connection may still be awaiting processing by the same
event loop that executes shutdown_listener() when the latter is run.
Since these netmgr callbacks must be run in order for the reference
count for the relevant controlconnection_t objects to drop to zero, when
the scenario described above happens, shutdown_listener() runs into an
infinite loop due to one of the controlconnection_t objects on the
listener->connections linked list never going away from the head of that
list.
Fix by safely iterating through the listener->connections list and
initiating shutdown for all controlconnection_t objects found. This
allows any pending netmgr callbacks to be run by the same event loop in
due course, i.e. after shutdown_listener() returns.
Aram Sargsyan [Tue, 12 Dec 2023 14:54:40 +0000 (14:54 +0000)]
Fix a statschannel system test zone loadtime issue
The check_loaded() function compares the zone's loadtime value and
an expected loadtime value, which is based on the zone file's mtime
extracted from the filesystem.
For the secondary zones there may be cases, when the zone file isn't
ready yet before the zone transfer is complete and the zone file is
dumped to the disk, so a so zero value mtime is retrieved.
In such cases wait one second and retry until timeout. Also modify
the affected check to allow a possible difference of the same amount
of seconds as the chosen timeout value.
Aram Sargsyan [Fri, 15 Dec 2023 09:43:36 +0000 (09:43 +0000)]
Use atomic store operations instead of atomic initialize
The atomic_init() function makes sense to use with structure's
members when creating a new instance of a strucutre. In other
places, use atomic store operations instead, in order to avoid
data races.
Move the code to find the predecessor into one function, as it is shares
quite some similarities: In both cases we first need to find the
immediate predecessor/successor, then we need to find the immediate
predecessor if the iterator is not already pointing at it.
This one is similar to the bug when searching for a key, reaching a
dead-end branch that doesn't match, because the branch offset point
is after the point where the search key differs.
This fixes the case where we are multiple levels deep. In other
words, we had a more-than-one matches *after* the point where the
search key differs.
If searching for a key "monky", we would reach the branch with
twigs "moo[k]" and "moo[n]". The key matches on the 'k' on offset=4,
and reaches the branch with twigs "mook[e]" and "mook[o]". This time
we cannot find a twig that matches our key at offset=5, there is no
twig for 'y'. The closest name we found was "mooker".
Note that on a branch it can't detect it is on a dead branch because the
key is not encapsulated in a branch node.
In the previous code we considered "mooker" to be the successor of
"monky" and so we needed to the predecessor of "mooker" to find the
predecessor for "monky". However, since the search key alread differed
before entering this branch, this is not enough. We would be left with
"moog" as the predecessor of "monky", while in this example "a.b.c.d.e"
is the actual predecessor.
Instead, we need to go up a level, find the predecessor and check
again if we are on the right branch, and repeat the process until we
are.
There was yet another edge case in which an iterator could be
positioned at the wrong node after dns_qp_lookup(). When searching for
a key, it's possible to reach a leaf that matches at the given offset,
but because the offset point is *after* the point where the search key
differs from the leaf's contents, we are now at the wrong leaf.
In other words, the bug fixed the previous commit for dead-end branches
must also be applied on matched leaves.
For example, if searching for the key "monpop", we could reach a branch
containing "moop" and "moor". the branch offset point - i.e., the point
after which the branch's leaves differ from each other - is the
fourth character ("p" or "r"). The search key matches the fourth
character "p", and takes that twig to the next node (which can be
a branch for names starting with "moop", or could be a leaf node for
"moop").
The old code failed to detect this condition, and would have
incorrectly left the iterator pointing at some successor, and not
at the predecessor of the "moop".
To find the right predecessor in this case, we need to get to the
previous branch and get the previous from there.
This has been fixed and the unit test now includes several new
scenarios for testing search names that match and unmatch on the
offset but have a different character before the offset.
Tom Krizek [Fri, 8 Dec 2023 15:31:46 +0000 (16:31 +0100)]
Handle empty MR description in dangerfile
A merge request might have no description at all (i.e. None, rather than
an empty string). This might happen when the MR is created via an API.
Check a description is present before trying to find a backport string
in it.
Michał Kępień [Thu, 7 Dec 2023 12:23:22 +0000 (13:23 +0100)]
Improve determining the lines added by a branch
Since the list of lines added to Git-tracked text files in a given
branch is not part of the Danger DSL [1], it is determined using custom
code in dangerfile.py. The current implementation of that logic is less
than perfect as it examines the diff between the current tip of the
target branch and the source branch rather than the diff between the
merge base of the two branches and the source branch. Consider a Git
history like this:
* F (target)
...
* E
* D
* C
| * B (source)
|/
* A (merge base)
If danger-python or Hazard are run for commit B, the current logic for
determining the list of added lines in dangerfile.py examines the diff
between commits F and B rather than between commits A and B. Therefore,
the added_lines() function returns not just the lines added by commit B
on top of commit A, but also the list of lines that were removed between
commits A and F, which leads to confusing results.
Fix by using the triple-dot diff operator in the Git invocation whose
output is used as the source of information for determining the list of
lines added by a given branch.
Since Hazard fetches the target branch itself when it is run, remove the
explicit "git fetch" invocation that fetches the target branch from
GitLab (shortening its local history to a single commit in the process)
before "git diff" is invoked.
Tom Krizek [Thu, 7 Dec 2023 15:42:43 +0000 (16:42 +0100)]
Temporarily allow failure of respdiff-long:tsan job
There was a change in the branch that uncovered preexisting data races
when testing with respdiff under TSAN. This isn't a new issue that
should stop the releases. Allow the check to temporarily fail until the
underlying issue GL #4475 is addressed.
Mark Andrews [Tue, 5 Dec 2023 02:15:03 +0000 (13:15 +1100)]
Also cleanup the space for the rbt nodes
As we are in overmem state we want to free more memory than we are
adding so we need to add in an allowance for the rbtnodes that may
have been added and the names stored with them. There is the node
for the owner name and a possible ENT node if there was a node split.
Mark Andrews [Thu, 23 Nov 2023 04:47:35 +0000 (15:47 +1100)]
Process the combined LRU lists in LRU order
Only cleanup headers that are less than equal to the rbt's last_used
time. Adjust the rbt's last_used time when the target cleaning was
not achieved to the oldest value of the remaining set of headers.
When updating delegating NS and glue records last_used was not being
updated when it should have been.
When adding zero TTL records to the tail of the LRU lists set
last_used to rbtdb->last_used + 1 rather than now. This appoximately
preserves the lists LRU order.
Evan Hunt [Wed, 1 Nov 2023 16:09:16 +0000 (17:09 +0100)]
remove some long-deprecated arguments from dig
certain dig options which were deprecated and became nonoperational
several releases ago still had documentation in the dig man page and
warnings printed when they were used: these included +mapped,
+sigchase, +topdown, +unexpected, +trusted-key, and the -i and -n
options. these are now all fatal errors.
another option was described as deprecated in the man page, but
the code to print a warning was never added. it has been added now.
Evan Hunt [Tue, 31 Oct 2023 14:09:05 +0000 (15:09 +0100)]
deprecate resolver-retry-interval and resolver-nonbackoff-tries
these options control default timing of retries in the resolver
for experimental purposes; they are not known to useful in production
environments. they will be removed in the future; for now, we
only log a warning if they are used.
Evan Hunt [Mon, 4 Dec 2023 19:21:40 +0000 (11:21 -0800)]
fix another dns_qp_lookup() iterator bug
there was another edge case in which an iterator could be positioned at
the wrong node after dns_qp_lookup(). when searching for a key, it's
possible to reach a dead-end branch that doesn't match, because the
branch offset point is *after* the point where the search key differs
from the branch's contents.
for example, if searching for the key "mop", we could reach a branch
containing "moon" and "moor". the branch offset point - i.e., the
point after which the branch's leaves differ from each other - is the
fourth character ("n" or "r"). however, both leaves differ from the
search key at position *three* ("o" or "p"). the old code failed to
detect this condition, and would have incorrectly left the iterator
pointing at some lower value and not at "moor".
this has been fixed and the unit test now includes this scenario.
Evan Hunt [Sun, 3 Dec 2023 22:02:29 +0000 (14:02 -0800)]
fix dns_qp_lookup() iterator bug
in some cases it was possible for the iterator to be positioned in the
wrong place by dns_qp_lookup(). previously, when a leaf node was found
which matched the search key at its parent branch's offset point, but
did not match after that point, the code incorrectly assumed the leaf
it had found was a successor to the searched-for name, and stepped the
iterator back to find a predecessor. however, it was possible for the
non-matching leaf to be the predecessor, in which case stepping the
iterator back was wrong.
(for example: a branch contains "aba" and "abcd", and we are searching
for "abcde". we step down to the twig matching the letter "c" in
position 3. "abcd" is the predecessor of "abcde", so the iterator is
already correctly positioned, but because the twig was an exact match,
we would have moved it back one step to "aba".)
this previously went unnoticed due to a mistake in the qp_test unit
test, which had the wrong expected result for the test case that should
have detected the error. both the code and the test have been fixed.
Evan Hunt [Sun, 5 Nov 2023 12:42:42 +0000 (13:42 +0100)]
add an iterator argument to dns_qp_lookup()
the 'predecessor' argument to dns_qp_lookup() turns out not to
be sufficient for our needs: the predecessor node in a QP database
could have become "empty" (for the current version) because of an
update or because cache data expired, and in that case the caller
would have to iterate more than one step back to find the predecessor
node that it needs.
it may also be necessary for a caller to iterate forward, in
order to determine whether a node has any children.
for both of these reasons, we now replace the 'predecessor'
argument with an 'iter' argument. if set, this points to memory
with enough space for a dns_qpiter object.
when an exact match is found by the lookup, the iterator will be
pointing to the matching node. if not, it will be pointing to the
lexical predecessor of the nae that was searched for.
a dns_qpiter_current() method has been added for examining
the current value of the iterator without moving it in either
direction.
Artem Boldariev [Mon, 4 Dec 2023 13:12:57 +0000 (15:12 +0200)]
doth test: add a secondary NS instance that reuses a 'tls' entry
This commit extends the 'doth' system tests with additional secondary
NS instance that reuses the same 'tls' entry for connecting the the
primary to download zones. This configurations were known to crash
secondaries in some cases.
Artem Boldariev [Mon, 4 Dec 2023 12:28:28 +0000 (14:28 +0200)]
Fix TLS certs store deletion on concurrent access
During initialisation or reconfiguration, it is possible that multiple
threads are trying to create a TLS context and associated data (like
TLS certs store) concurrently. In some cases, a thread might be too
late to add newly created data to the TLS contexts cache, in which
case it needs to be discarded. In the code that handles that case, it
was not taken into account that, in some cases, the TLS certs store
could not have been created or should not be deleted, as it is being
managed by the TLS contexts cache already. Deleting the store in such
cases might lead to crashes.
projects / thirdparty / bind9.git / log
