Move dns_dnssec_findzonekeys from the dnssec.{c,h} source code to
zone.{c,h} (the header file already commented that this should be done
inside dns_zone_t).
Alter the function in such a way, that keys are searched for in the
key stores if a 'dnssec-policy' (kasp) is attached to the zone,
otherwise keep using the zone's key-directory.
When writing key files to disk, use the internally stored directory.
Add an access function 'dst_key_directory()'.
Most calls to keymgr functions no longer need to provide the
key-directory value. Only 'dns_keymgr_run' still needs access to
the zone's key-directory in case the key-store is set to the built-in
key-directory.
Refactor dns_dnssec_findmatchingkeys and dns_dnssec_keylistfromrdataset
to take into account the key store directories in case the zone is using
dnssec-policy (kasp). Add 'kasp' and 'keystores' parameters.
This requires the keystorelist to be stored inside the zone structure.
The calls to these functions in the DNSSEC tools can use NULL as the
kasp value, as dnssec-signzone does not (yet) support dnssec-policy,
and key collision is checked inside the directory where it is created.
If there is a keystore configured with a PKCS#11 URI, zones that
are using a dnssec-policy that uses such a keystore should create keys
via the PKCS#11 interface. Those keys are generally stored inside an
HSM.
Some changes to the code are required, to store the engine reference
into the keystore.
When creating the kasp structure, instead of storing the name of the
key store on keys, store a reference to the key store object instead.
This requires to build the keystore list prior to creating the kasp
structures, in the dnssec tools, the check code and the server code.
We will create a builtin keystore called "key-directory" which means
use the zone's key-directory as the key store.
The check code changes, because now the keystore is looked up before
creating the kasp structure (and if the keystore is not found, this
is an error). Instead of looking up the keystore after all
'dnssec-policy' clauses have been read.
Add checkconf check to ensure that the used key-store in the keys
section exists. Error if that is not the case. We also don't allow
the special keyword 'key-directory' as that is internally used to
signal that the zone's key-directory should be used.
Add code for configuring keystore objects. Add this to the "kaspconf"
code, as it is related to 'dnssec-policy' and it is too small to create
a separate file for it.
If the DNSKEY, CDNSKEY or CDS RRset had different TTLs then the
filtering of these RRset resulted in dns_diff_apply failing with
"not exact". Identify tuple pairs that are just TTL changes and
allow them through the filter.
Clang Static Analyzer is unable to grasp that when dns_rbt_addnode()
returns ISC_R_EXISTS, it always sets the pointer passed to it via its
'nodep' parameter to a non-NULL value. Add an extra safety check in the
conditional expression used in dns_rbt_addname() to silence that
warning.
it was possible for fix_iterator() to get stuck in a loop while
trying to find the predecessor of a missing node. this has been
fixed and a regression test has been added.
the fix_iterator() function moves an iterator so that it points
to the predecessor of the searched-for name when that name doesn't
exist in the database. the tests only checked the correctness of
the top of the stack, however, and missed some cases where interior
branches in the stack could be missing or duplicated. in these
cases, the iterator would produce inconsistent results when walked.
the predecessors test case in qp_test has been updated to walk
each iterator to the end and ensure that the expected number of
nodes are found.
When building NSEC3 chains update the NSEC3PARAM TTL to match
the SOA minimum. Delete all records using the old TTL then
re-add them using the new TTL.
Just remove the key from consideration as it is being removed.
The old code could leak a key reference as dst_free_key was not
called every time we continued. This simplification will address
this as well.
Check the tid and cancel the request immediately or pass it to the
appropriate loop for processing. Call request->cb directly from
req_sendevent as it is now always called with the correct tid.
The xfrin_end() function is run when a zone transfer is finished or
canceled. One of the actions it takes for incremental transfers (IXFR)
is calling dns_journal_destroy() on the zone journal structure that is
stored in the relevant zone transfer context (xfr->ixfr.journal). That
immediately invalidates that structure as it is not reference-counted.
However, since the changes present in the IXFR stream are applied to the
journal asynchronously (via isc_work_enqueue()), it is possible that
some zone changes may still be in the process of being written to the
journal by the time xfrin_end() destroys the relevant structure. Such a
scenario leads to crashes.
Fix by not destroying the zone journal structure until the entire zone
transfer context is destroyed. xfrin_destroy() already conditionally
calls dns_journal_destroy() and when the former is called, all
asynchronous work for a given zone transfer process is guaranteed to be
complete.
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.
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.
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.
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.
For example, consider the following qp-trie:
branch: "[e]", "[m]":
- leaf: "a.b.c.d.e"
- branch: "moo[g]", "moo[k]", "moo[n]":
- leaf: "moog"
- branch: "mook[e]", "mook[o]"
- leaf: "mooker"
- leaf: "mooko"
- leaf: "moon"
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.
Unit tests to cover the scenario are now added.
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.
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.
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.
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.
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.
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.
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.
This commit fixes the issue.
The main intention of PROXY protocol is to pass endpoints information
to a back-end server (in our case - BIND). That means that it is a
valid way to spoof endpoints information, as the addresses and ports
extracted from PROXYv2 headers, from the point of view of BIND, are
used instead of the real connection addresses.
Of course, an ability to easily spoof endpoints information can be
considered a security issue when used uncontrollably. To resolve that,
we introduce 'allow-proxy' and 'allow-proxy-on' ACL options. These are
the only ACL options in BIND that work with real PROXY connections
addresses, allowing a DNS server operator to specify from what clients
and on which interfaces he or she is willing to accept PROXY
headers. By default, for security reasons we do not allow to accept
them.
BIND 9 will now treat the response as insecure when processing NSEC3
records with iterations larger than 50.
Earlier, we limited the number of iterations to 150 (in #2445).
RFC 9276 says: Because there has been a large growth of open (public)
DNSSEC validating resolvers that are subject to compute resource
constraints when handling requests from anonymous clients, this
document recommends that validating resolvers reduce their iteration
count limits over time. Specifically, validating resolver operators and
validating resolver software implementers are encouraged to continue
evaluating NSEC3 iteration count deployment trends and lower their
acceptable iteration limits over time.
After evaluation, we decided that the next major BIND release should
lower the maximum allowed NSEC3 iterations to 50, which should be
fine for 99,87% of the domain names.
Matthijs Mekking
Mark Andrews
Artem Boldariev
Michał Kępień
Evan Hunt
Aram Sargsyan