Replace the two-pass "random start index and wrap around" logic in
fctx_getaddresses_nameservers() with a statistically sound Fisher-Yates
shuffle.
The previous implementation picked a random starting node and did two
passes over the linked list to find query candidates. The new logic
extracts the available nameservers into a bounded, stack-allocated array
of dns_rdata_t structures.
This array is then randomized in-place using a Fisher-Yates shuffle.
Finally, the shuffled array is traversed sequentially to launch fetches
until the dynamic quota (fctx->pending_running >= fetches_allowed) is
reached.
This guarantees a fair random distribution for outbound queries while
properly respecting dynamic query limits, entirely within O(1) memory
and without the overhead of linked-list pointer shuffling or multiple
dataset traversals.
(cherry picked from commit 3c33e7d937)
A debug message that logs a PKCS#11 object has been generated was
erroneously logged at error level. This has been fixed.
(cherry picked from commit 5bd6322739)
When selecting nameserver addresses to be looked up we where
always selecting them in dnssec name order from the start of
the nameserver rrset. This could lead to resolution failure
despite there being address that could be resolved for the
other names. Use a random starting point when selecting which
names to lookup.
(cherry picked from commit b78052119a)
If an invalid SKR file is imported, reading the time from the token
buffer might overflow the buffer on the local stack. This has been
fixed by removing the intermediate buffer and parsing the lexer token
directly.
(cherry picked from commit 8ab4827a0c)
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.
(cherry picked from commit 3801d0ebbf)
When .next_length is longer than NSEC3_MAX_HASH_LENGTH, it causes a
harmless out-of-bound read of the isdelegation() stack. This patch
fixes the issue by skipping NSEC3 records with an oversized hash length
during validation.
(cherry picked from commit 67b4fb56e4)
A regression was introduced when adding the EDE code for unsupported
DNSKEY and DS algorithms. When the parent has both supported and
unsupported algorithm in the DS record, the validator would treat the
supported DS algorithm as insecure when validating DNSKEY records
instead of BOGUS. This has not security impact as the rest of the child
zone correctly ends with BOGUS status, but it is incorrect and thus the
regression has been fixed.
(cherry picked from commit f983a64152)
An attacker controlling a malicious DNS server returns a DNAME record,
and the we stores a pointer to resp->foundname, frees the response
structure, then uses the dangling pointer in dns_name_fullcompare()
possibly causing invalid match. Only the `delv`is affected. This has
been fixed.
(cherry picked from commit 9135b71a7a)
The fetch loop detection occured in two places: when
`dns_resolver_createfetch()` is invoked (looking up through the parent
fetches chain and stops the fetch if a parent fetch is the same qname and
qtype) and right after calling `dns_adb_findname()` in the resolver
(stops the fetch if the current fetch is the same name from the ADB
lookup, and ADB lookup needs to fetch it).
Regarding fetch loop detection at the `dns_resulver_createfetch()`
entry, there are case where both qname and qtype are similar but the
zonecut is different. This will then query different name servers and
get different responses. For instance, the following delegation
parent-side (both for `foo.example.` and `dnshost.example.`):
foo.example. 3600 NS ns.dnshost.example.
dnshost.example. 3600 NS ns.dnshost.example.
ns.dnshost.example. 3600 A 1.2.3.4
Then the child-side of `dnshost.example.`:
dnshost.example. 300 NS ns.dnshost.example.
ns.dnshost.example. 300 A 1.2.3.4
Then the child-side of `foo.example.`:
foo.example 3600 NS ns.dnshost.example.
a.foo.example 300 A 5.6.7.8
Obviously, there is a misconfiguration between the parent-side and the
child-side of `dnshost.example` (the mismatch of the TTL), but, this
happens...
Because the resolver is currently child-centric, the parent-side
delegation's glue of `dnshost.example.` will be overriden by the
child-side of the delegation. Once both A records will expires, the
resolver will attempt to find out the A RRs but will start from the
`foo.example.` zonecut, as the delegation itself is still valid.
Then the resolver will attempt to resolve `ns.dnshost.example.`, still
using the `foo.example.` zonecut, which will immediately trigger another
attempt to resolve `ns.foo.example.` (because the A RR is expired). This
is, however _not_ a loop, because the second attempt will have
`dnshost.example.` zonecut. And this changes everything, because the
resolver detects the A name is in-domain, and pass a flag to ADB so
`dns_view_find()` won't use the cache. As a result, the zonecut will be
`.`, and the hints (root servers) will be queried instead.
From that point, they'll return the parent-side delegation, which
includes the glue for `ns.dnshost.example/A`, and the resolution can
continue. Previously, this wouldn't be possible because a loop would be
detected from the second attempt to looking `ns.foo.example/A` and would
result in a SERVFAIL.
Now, the loop detection is relaxed as the loop is detected if the qname,
qtype _and_ zonecut are equals.
This commit also changes the way the loop detection post
`dns_adb_createfind()` works. From the same example above, there would
be two ADB fetches with the same name, but with two different ADB flags
(the first one without DNS_ADB_STARTATZONE, the second one with that
flag). It means that there will be two fetches out of those two ADB
lookups, both legit, and not a loop (i.e. it won't be stuck). To
differenciate between a find which has a pending fetch (which could be
from another find the current find has been attached to), a new find
option `DNS_ADBFIND_STARTEDFETCH` is introduced, which tells that the
current has did started a fetch.
That way, if a find doesn't have `DNS_ADBFIND_STARTEDFETCH` option but
has pending fetches, we know this is a find attached to a similar find
so this is a loop. Otherwise, with `DNS_ADBFIND_STARTEDFETCH`, we know
that even if there is a pending fetch, this is not a loop as the fetch
has just been started
(cherry picked from commit f623ab1fb3)
ADB entry window and ADB min cache time can be tweaked using `named -T
adbentrywindow=<unsigned int>` and `named -T adbmincache=<unsigned
int>`.
While those values doesn't needs to be exposed to the operator, this can
be needed to be able to system test ADB behaviors without having to wait
as long as those values are by default.
(cherry picked from commit e5f963262a)
This better reflects the true nature of the function as we are reading
the ephemeral port range which is not related to UDP at all.
(cherry picked from commit 295139f8ca)
The function was already marked as never failing, always returning
ISC_R_SUCCESS, so there was a lot of dead code around checking whether
the result would be ISC_R_SUCCESS. This has been cleaned up.
(cherry picked from commit c3ec414d88)
Prevent retrying the notify over TCP in case the source address is not
available or the source vs the destination address family mismatch or
when the destination address has been blackholed. Properly log the
hard notify failures.
(cherry picked from commit 5a5bc6de22)
When dns_request_create() fails in notify_send_toaddr() the TSIG key was
not cleared when retrying over TCP causing assertion failure. Set the
TSIG key to NULL in the dns_message to prevent the assertion failure.
(cherry picked from commit ee3391a146)
In dst_gssapi_acceptctx(), the gnamebuf could leak a little bit of
memory if dns_name_fromtext() would theoretically fail. This would
require a Kerberos principal with invalid DNS name.
(cherry picked from commit 3ad87f1ad6)
The description in the protobuf specification is not a list of request
types to process but rather a list of examples to qualify the
description of whether the time indicates when the message is received
or sent.
(cherry picked from commit 479c737517)
Non qp/rbt databases might not implement the
dns_db_(begin|commit|abort)update methods. This commit ensures that we
return ISC_R_NOTIMPLEMENTED in those cases.
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.
(cherry picked from commit da53708dcb)
This commit adds a layer of indirection to the apply_diff logic used by
IXFR and resigning by having the database updates go through a vtable.
We do this in three steps:
- We extend dns_rdatacallbacks_t vtable to allow subtraction and
resigning.
- We add a new set of api (begin|commit|abort)update to the dbmethods
vtable, that model an incremental update that can be aborted.
- We extract the core logic of diff_apply into a function that
satisfies the new interface.
- We make diff_apply use this new function, and log the results.
The intent of this commit is to allow databases to expose a batch
incremental update implementation, just like they expose a custom
batch creation implementation through (begin|end)load.
(cherry picked from commit e36dc0ca76)
The setresign method is not diff specific, it only returns the minimum
resign time of an rdataset. Move it to rdataset.c to simplify late
refactoring.
(cherry picked from commit 6f726ae3db)
Make the API tighter. The idea of this commit is to highlight the
distinction between a database transaction and a journal transaction,
and ensure we run dns_zone_verifydb on error.
Done to simplify a later refactor.
(cherry picked from commit 399f0c191a)
The zone_loaddone() function disables database notifications for
a catalog zones and response policy zones (RPZ) when loading had
failed. Howerer, the 'result != ISC_R_SUCCESS' check is insufficient,
because the DNS_R_SEENINCLUDE result also indicates success.
Add a second condition for the "if" block.
(cherry picked from commit 31290eccb1)
uint16_tobuffer was used instead of uint8_tobuffer when adding the
scheme to the buffer. This produced a record that was one octet
too long. This has been fixed.
(cherry picked from commit 3180e50459)
Previously, the isc_ht hash table module was case-sensitive, but now
it supports case-insensitive mode. Use the case-insensitive mode
for the catalog zones' entry names.
(cherry picked from commit 6f4b5d6958)
Previously, the isc_ht hash table module was case-sensitive, but now
it supports case-insensitive mode. Use the case-insensitive mode
for catalog zone names.
(cherry picked from commit 0e0ba06dbf)
Fix incorrect length checks in the towire_*() methods for BRID and HHIT
records to prevent assertion failures when trying to serve short
records.
(cherry picked from commit 14e299995f)
When switching from NSEC3 opt-out to NSEC, add NSEC records if we saw an
RR. This corrects a mistake in style cleanups done in commit
308ab1b4a5.
(cherry picked from commit 6f285bff6a)
A catalog zone is updated in an offloaded thread, which is not
stopped during a reconfiguration in an exclusive mode, and so
can cause a race condition with it.
Waiting for the offloaded threads to complete their work before
entering into the exclusive mode can potentially cause unwanted
delays, because offloaded threads are generally "allowed" to take
a longer amount of time before they complete.
Add a dns_catz_zone_prereconfig()/dns_catz_zone_postreconfig() pair
of functions which currently just lock the catalog zone when
reconfiguring it. The change should eliminate the race.
As a side note, there was already a similar pair of functions,
dns_catz_prereconfig() and dns_catz_postreconfig() which are called
before and after reconfiguring a 'dns_catz_zones_t' object.
Below are the stack traces of the reconfiguration thread which has
asserted, and a catalog zone update thread which was caught in the
middle of its work despite the fact that the exclusive mode is
turned on.
Stack trace of thread 23859:
#0 0x00007f80e7b8e52f raise (libc.so.6)
#1 0x00007f80e7b61e65 abort (libc.so.6)
#2 0x0000000000422558 assertion_failed (named)
#3 0x00007f80eaa6799e isc_assertion_failed (libisc-9.18.41.so)
#4 0x00007f80ea5bc788 dns_catz_entry_getname (libdns-9.18.41.so)
#5 0x000000000042ce0e catz_reconfigure (named)
#6 0x000000000042d3c5 configure_catz_zone (named)
#7 0x000000000042d7a4 configure_catz (named)
#8 0x0000000000430645 configure_view (named)
#9 0x000000000043d998 load_configuration (named)
#10 0x000000000044184f loadconfig (named)
#11 0x0000000000442525 named_server_reconfigcommand (named)
#12 0x000000000041b277 named_control_docommand (named)
#13 0x000000000041c74a control_command (named)
#14 0x00007f80eaa912ae task_run (libisc-9.18.41.so)
#15 0x00007f80eaa914cd isc_task_run (libisc-9.18.41.so)
#16 0x00007f80eaa46435 isc__nm_async_task (libisc-9.18.41.so)
#17 0x00007f80eaa467aa process_netievent (libisc-9.18.41.so)
#18 0x00007f80eaa475a6 process_queue (libisc-9.18.41.so)
#19 0x00007f80eaa46227 process_all_queues (libisc-9.18.41.so)
#20 0x00007f80eaa462a1 async_cb (libisc-9.18.41.so)
#21 0x00007f80e8d01893 uv__async_io.part.3 (libuv.so.1)
#22 0x00007f80e8d13ac4 uv__io_poll (libuv.so.1)
#23 0x00007f80e8d023fb uv_run (libuv.so.1)
#24 0x00007f80eaa45ced nm_thread (libisc-9.18.41.so)
#25 0x00007f80eaa9bda3 isc__trampoline_run (libisc-9.18.41.so)
#26 0x00007f80e7f1e1ca start_thread (libpthread.so.0)
#27 0x00007f80e7b798d3 __clone (libc.so.6)
...
...
Stack trace of thread 23912:
#0 0x00007f80ea5bc2da dns_catz_options_setdefault (libdns-9.18.41.so)
#1 0x00007f80ea5bd411 dns__catz_zones_merge (libdns-9.18.41.so)
#2 0x00007f80ea5c3c2f dns__catz_update_cb (libdns-9.18.41.so)
#3 0x00007f80eaa4fee9 isc__nm_work_run (libisc-9.18.41.so)
#4 0x00007f80eaa9bda3 isc__trampoline_run (libisc-9.18.41.so)
#5 0x00007f80eaa4ff48 isc__nm_work_cb (libisc-9.18.41.so)
#6 0x00007f80e8cfc75e worker (libuv.so.1)
#7 0x00007f80e7f1e1ca start_thread (libpthread.so.0)
#8 0x00007f80e7b798d3 __clone (libc.so.6)
(cherry picked from commit aed9cafd5c)
It is possible to have a fetch that is active, but it has been cloned,
so it won't be used when found in the hash table. The fetch options
also prevent matching in the hash table, so add a hexadecimal dump of
the fctx->options to the output.
(cherry picked from commit 23ae5544be)
Instead of creating new nodes for every possible NSEC3 record, only
create them if we are actually going to add a new NSEC3 record.
(cherry picked from commit 6f7abbfaac)
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.
(cherry picked from commit 41159e9062)
The dns_catz_generate_zonecfg() function generates a zone configuration
string to use with a new catalog zone member zone. The buffer for the
string is 512 bytes initially (ISC_BUFFER_INCR), but can be reallocated
when required, when using corresponding isc_buffer functions like
isc_buffer_reserve(), isc_buffer_putstr(), isc_buffer_copyregion(), etc.
However, the dns_name_totext() function, which expects the buffer as an
argument, doesn't automatically resize it if the name doesn't fit there,
but instead just returns ISC_R_NOSPACE.
The chance of this occurring increases when the configuration string is
large due to, for example, long zone name, long list of primary servers
which have keys configured and/or TLS configured.
Use dns_name_format() accompanied with isc_buffer_putstr() instead of
dns_name_totext().
(cherry picked from commit 684d7e008a)
dns_rbtnodechain_prev requires the current node to still be valid
which was not always the case after dereference_iter_node was called.
Move the call to dereference_iter_node to after the dns_rbtnodechain_prev
to preserve the node.
dns_qpiter_{prev,next} requires the current iterator node to still be
valid which might not always the case after dereference_iter_node was
called. Currently, this is ensured via closeversion() mechanism, but it
is not guaranteed to be true in the future.
Move the call to dereference_iter_node to after the dns_qpiter_prev()
and dns_qpiter_next() to prevent a possible use-after-free of the
current iterator node.
(cherry picked from commit 9914bd383e)
