Although 'dns_fetch_t' fetch can have two associated events, one for
each of 'DNS_EVENT_FETCHDONE' and 'DNS_EVENT_TRYSTALE' types, the
dns_resolver_cancelfetch() function is designed in a way that it
expects only one existing event, which it must cancel, and when it
happens so that 'stale-answer-client-timeout' is enabled and there
are two events, only one of them is canceled, and it results in an
assertion in dns_resolver_destroyfetch(), when it finds a dangling
event.
Change the logic of dns_resolver_cancelfetch() function so that it
cancels both the events (if they exist), and in the right order.
(cherry picked from commit ec2098ca35)
dns_db_findext() asserts if RRSIG is passed to it and
query_lookup_stale() failed to map RRSIG to ANY to prevent this. To
avoid cases like this in the future, move the mapping of SIG and RRSIG
to ANY for qctx->type to qctx_init().
(cherry picked from commit 56eae06418)
check allow-update, update-policy, and allow-update-forwarding before
consuming quota slots, so that unauthorized clients can't fill the
quota.
(this moves the access check before the prerequisite check, which
violates the precise wording of RFC 2136. however, RFC co-author Paul
Vixie has stated that the RFC is mistaken on this point; it should have
said that access checking must happen *no later than* the completion of
prerequisite checks, not that it must happen exactly then.)
(cherry picked from commit 964f559edb)
limit the number of simultaneous DNS UPDATE events that can be
processed by adding a quota for update and update forwarding.
this quota currently, arbitrarily, defaults to 100.
also add a statistics counter to record when the update quota
has been exceeded.
(cherry picked from commit 7c47254a14)
The hash table implementation in the current branch does not
support automatic resize operation, so the initial value of
the table should be chosen carefully.
Catalog zones entries hash table's size is currently only 4 bits,
which is very low for a catalog zone with thousands of entries,
and causes CPU consumption spikes when working with the hash
table to add/delete/search entries.
Use 16 bits instead, which should make working with big catalog
zones much faster at the expense of slightly higher memory usage,
i.e. 512 KiB for a 64-bit system for each catalog zone.
As shown in the previous commit, using sizeof(type_t) is a little
bit more error-prone when copy-pasting code, so extracting the
size information from the pointer which is being dealt with seems
like a better alternative.
(cherry picked from commit cf4003fa58)
Free 'sizeof(dns_forwarder_t)' bytes of memory instead of
'sizeof(dns_sockaddr_t)' bytes, because `fwd` is a pointer
to a 'dns_forwarder_t' type structure.
(cherry picked from commit 0cc1b06d98)
The dns_zonemgr_releasezone() function makes a decision to destroy
'zmgr' (based on its references count, after decreasing it) inside
a lock, and then destroys the object outside of the lock.
This causes a race with dns_zonemgr_detach(), which could destroy
the object in the meantime.
Change dns_zonemgr_releasezone() to detach from 'zmgr' and destroy
the object (if needed) using dns_zonemgr_detach(), outside of the
lock.
(cherry picked from commit c1fc212253)
This commit deprecates the "dscp" configuration option and "dscp"
parameters to source-address configuration options (query-source,
transfer-source, etc.
(Note that the DSCP feature has not been fully operational since
the network manager was introduced in 9.16.0; outgoing DSCP values
can be configured, but incoming DSCP values are not detected.)
Arthimetic on NULL pointers is undefined. Avoid arithmetic operations
when 'in' is NULL and require 'in' to be non-NULL if 'inlen' is not zero.
(cherry picked from commit 349c23dbb7)
When node is NULL when calling getparent() et al. they return NULL
but performing arithmetic on the NULL pointer is undefined. Check
if 'node' or 'header' is NULL and skip the adjustment.
Commit 9ffb4a7ba1 causes Clang Static
Analyzer to flag a potential NULL dereference in query_nxdomain():
query.c:9394:26: warning: Dereference of null pointer [core.NullDereference]
if (!qctx->nxrewrite || qctx->rpz_st->m.rpz->addsoa) {
^~~~~~~~~~~~~~~~~~~
1 warning generated.
The warning above is for qctx->rpz_st potentially being a NULL pointer
when query_nxdomain() is called from query_resume(). This is a false
positive because none of the database lookup result codes currently
causing query_nxdomain() to be called (DNS_R_EMPTYWILD, DNS_R_NXDOMAIN)
can be returned by a database lookup following a recursive resolution
attempt. Add a NULL check nevertheless in order to future-proof the
code and silence Clang Static Analyzer.
(cherry picked from commit 07592d1315)
(cherry picked from commit a4547a1093)
With 'stale-answer-enable yes;' and 'stale-answer-client-timeout off;',
consider the following situation:
A CNAME record and its target record are in the cache, then the CNAME
record expires, but the target record is still valid.
When a new query for the CNAME record arrives, and the query fails,
the stale record is used, and then the query "restarts" to follow
the CNAME target. The problem is that the query's multiple stale
options (like DNS_DBFIND_STALEOK) are not reset, so 'query_lookup()'
treats the restarted query as a lookup following a failed lookup,
and returns a SERVFAIL answer when there is no stale data found in the
cache, even if there is valid non-stale data there available.
With this change, query_lookup() now considers non-stale data in the
cache in the first place, and returns it if it is available.
(cherry picked from commit 91a1a8efc5)
The dns_catz_update_from_db() function prints serial number as a signed
number (with "%d" in the format string), but the `vers` variable's type
is 'uint32_t'. This breaks serials bigger than 2^31.
Use PRIu32 instead of "d" in the format string.
(cherry picked from commit 72b1760ea6)
Instead of relying on hash table search when using the keys, implement a
proper reference counting in dns_keyfileio_t objects, and attach/detach
the objects to the zone.
(cherry picked from commit 79115a0c3b)
Due to off-by-one error in zonemgr_keymgmt_delete, unused key file IO
lock objects were never freed and they were kept until the server
shutdown. Adjust the returned value by -1 to accomodate the fact that
the atomic_fetch_*() functions return the value before the operation and
not current value after the operation.
(cherry picked from commit fb1acd6736)
Zero TTL handling does not need to be different for 'rdatasetiter_first'
and 'rdatasetiter_next' and it interacts badly with 'bind_rdatadataset'
which makes different determinations.
(cherry picked from commit 1a39328feb)
'DNS_DB_STALEOK' returns stale rdatasets as well as current rdatasets.
'DNS_DB_EXPIREDOK' returns expired rdatasets as well as current
rdatasets. This option is currently only set when DNS_DB_STALEOK is
also set.
(cherry picked from commit 85048ddeee)
Add an options parameter to control what rdatasets are returned when
iteratating over the node. Specific modes will be added later.
(cherry picked from commit 7695c36a5d)
Send the ns_query_cancel() on the recursing clients when we initiate the
named shutdown for faster shutdown.
When we are shutting down the resolver, we cancel all the outstanding
fetches, and the ISC_R_CANCEL events doesn't propagate to the ns_client
callback.
In the future, the better solution how to fix this would be to look at
the shutdown paths and let them all propagate from bottom (loopmgr) to
top (f.e. ns_client).
(cherry picked from commit d861d403bb9a7912e29a06aba6caf6d502839f1b)
dns_db_updatenotify_unregister needed to be called earlier to ensure
that listener->onupdate_arg always points to a valid object. The
existing lazy cleanup in rbtdb_free did not ensure that.
(cherry picked from commit 35839e91d8)
Duplicate dns_db_updatenotify_register registrations need to be
suppressed to ensure that dns_db_updatenotify_unregister is successful.
(cherry picked from commit f13e71e551)
Commit 9ffb4a7ba1 causes Clang Static
Analyzer to flag a potential NULL dereference in query_nxdomain():
query.c:9394:26: warning: Dereference of null pointer [core.NullDereference]
if (!qctx->nxrewrite || qctx->rpz_st->m.rpz->addsoa) {
^~~~~~~~~~~~~~~~~~~
1 warning generated.
The warning above is for qctx->rpz_st potentially being a NULL pointer
when query_nxdomain() is called from query_resume(). This is a false
positive because none of the database lookup result codes currently
causing query_nxdomain() to be called (DNS_R_EMPTYWILD, DNS_R_NXDOMAIN)
can be returned by a database lookup following a recursive resolution
attempt. Add a NULL check nevertheless in order to future-proof the
code and silence Clang Static Analyzer.
(cherry picked from commit 07592d1315)
With 'stale-answer-enable yes;' and 'stale-answer-client-timeout off;',
consider the following situation:
A CNAME record and its target record are in the cache, then the CNAME
record expires, but the target record is still valid.
When a new query for the CNAME record arrives, and the query fails,
the stale record is used, and then the query "restarts" to follow
the CNAME target. The problem is that the query's multiple stale
options (like DNS_DBFIND_STALEOK) are not reset, so 'query_lookup()'
treats the restarted query as a lookup following a failed lookup,
and returns a SERVFAIL answer when there is no stale data found in the
cache, even if there is valid non-stale data there available.
With this change, query_lookup() now considers non-stale data in the
cache in the first place, and returns it if it is available.
(cherry picked from commit 86a80e723f)
The "final reference detached" message was meant to be DEBUG(1), but was
instead kept at INFO level. Move it to the DEBUG(1) logging level, so
it's not printed under normal operations.
(cherry picked from commit 1816244725)
Deprecate auto-dnssec, add specific log warning to migrate to
dnssec-policy.
Cherry-picking triggered a lot of conflicts, so the changes
were manually picked.
(manually picked from commit f9845dd1)
When using dual-stack-servers the covering namespace to check whether
answers are in scope or not should be fctx->domain. To do this we need
to be able to distingish forwarding due to forwarders clauses and
dual-stack-servers. A new flag FCTX_ADDRINFO_DUALSTACK has been added
to signal this.
(cherry picked from commit dfbffd77f9)
There were a number of places where the zone table should have been
locked, but wasn't, when dns_zt_apply was called.
Added a isc_rwlocktype_t type parameter to dns_zt_apply and adjusted
all calls to using it. Removed locks in callers.
(cherry picked from commit f053d5b414)
The zone_refreshkeys() could run before the zone_shutdown(), but after
the last .erefs has been "detached" causing assertion failure when doing
dns_zone_attach(). Remove the use of .erefs (dns_zone_attach/detach)
and replace it with using the .irefs and additional checks whether the
zone is exiting in the callbacks.
(cherry picked from commit 80e66fbd2d)
There was an exception for dnssec-policy that allowed DNSSEC in the
unsigned version of the zone. This however causes a crash if the
zone switches from dynamic to inline-signing in the case of NSEC3,
because we are now trying to add an NSEC3 record to a non-NSEC3 node.
This is because BIND expects none of the records in the unsigned
version of the zone to be NSEC3.
Remove the exception for dnssec-policy when copying non DNSSEC
records, but do allow for DNSKEY as this may be a published DNSKEY
from a different provider.
(cherry picked from commit 332b98ae49)
When named starts it creates an empty KEYDATA record in the managed-keys
zone as a placeholder, then schedules a key refresh. If key refresh
fails for some reason (e.g. connectivity problems), named will load the
placeholder key into secroots as a trusted key during the next startup,
which will break the chain of trust, and named will never recover from
that state until managed-keys.bind and managed-keys.bind.jnl files are
manually deleted before (re)starting named again.
Before calling load_secroots(), check that we are not dealing with a
placeholder.
(cherry picked from commit 354ae2d7e3)
Because dns_resolver_createfetch() locks the view, it was necessary
to unlock the zone in zone_refreshkeys() before calling it in order
to maintain the lock order, and relock afterward. this permitted a race
with dns_zone_synckeyzone().
This commit moves the call to dns_resolver_createfetch() into a separate
function which is called asynchronously after the zone has been
unlocked.
The keyfetch object now attaches to the zone to ensure that
it won't be shut down before the asynchronous call completes.
This necessitated refactoring dns_zone_detach() so it always runs
unlocked. For managed zones it schedules zone_shutdown() to
run asynchronously; for unmanaged zones there is no task.
ARM states that the "eligibility" TTL is the smallest original TTL
value that is accepted for a record to be eligible for prefetching,
but the code, which implements the condition doesn't behave in that
manner for the edge case when the TTL is equal to the configured
eligibility value.
Fix the code to check that the TTL is greater than, or equal to the
configured eligibility value, instead of just greater than it.
(cherry picked from commit 863f51466e)
For UDP queries, after calling dns_adb_beginudpfetch() in fctx_query(),
make sure that dns_adb_endudpfetch() is also called on error path, in
order to adjust the quota back.
(cherry picked from commit 5da79e2be0)
RPZ rewrites called dns_db_findext() without passing through the
client database options; as as result, if the client set CD=1,
DNS_DBFIND_PENDINGOK was not used as it should have been, and
cache lookups failed, resulting in failure of the rewrite.
(cherry picked from commit 305a50dbe1)
This log happens when BIND checks the parental-agents if the DS has
been published. But if you don't have parental-agents set up, the list
of keys to check will be empty and the result will be ISC_R_NOTFOUND.
This is not an error, so change the log level to debug in this case.
(cherry picked from commit a1d57fc8cb)
Instead of having "arbitrary" (void *)-1 to define non-linked, add a
ISC_LINK_TOMBSTONE(type) macro that replaces the "magic" value with a
define.
(cherry picked from commit 5e20c2ccfb)
Since we are using designated initializers, we were missing initializers
for ISC_LIST and ISC_LINK, add them, so you can do
*foo = (foo_t){ .list = ISC_LIST_INITIALIZER };
Instead of:
*foo = (foo_t){ 0 };
ISC_LIST_INIT(foo->list);
(cherry picked from commit cb3c36b8bf)
The incrementing and decrementing of 'ns_statscounter_recursclients'
were not properly balanced: for example, it would be incremented for
a prefetch query but not decremented if the query failed.
This commit ensures that the recursion quota and the recursive clients
counter are always in sync with each other.
(cherry picked from commit 82991451b4)
The duration_fromtext() function is truncating large numbers
to 32 bits instead of capping or rejecting them, i.e. 64424509445,
which is 0xf00000005, gets parsed as 32-bit value 5 (0x00000005).
Fail parsing a duration if any of its components is bigger than
32 bits. Using those kind of big numbers has no practical use case
for a duration.
The cfg_obj_asduration() function can overflow the 32 bit
seconds variable when calculating the duration from its component
parts.
To avoid that, use 64-bit calculation and return UINT32_MAX if the
calculated value is bigger than UINT32_MAX. Again, a number this big
has no practical use case anyway.
The buffer for the generated duration string is limited to 64 bytes,
which, in theory, is smaller than the longest possible generated
duration string.
Use 80 bytes instead, calculated by the '7 x (10 + 1) + 3' formula,
where '7' is the count of the duration's parts (year, month, etc.), '10'
is their maximum length when printed as a decimal number, '1' is their
indicator character (Y, M, etc.), and 3 is two more indicators (P and T)
and the terminating NUL character.
(cherry picked from commit fddaebb285)
The cfg_print_duration() checks added previously in the 'duration_test'
unit test uncovered a bug in cfg_print_duration().
When calculating the current 'str' pointer of the generated text in the
buffer 'buf', it erroneously adds 1 byte to compensate for that part's
indicator character. For example, to add 12 minutes, it needs to add
2 + 1 = 3 characters, where 2 is the length of "12", and 1 is the length
of "M" (for minute). The mistake was that the length of the indicator
is already included in 'durationlen[i]', so there is no need to
calculate it again.
In the result of this mistake the current pointer can advance further
than needed and end up after the zero-byte instead of right on it, which
essentially cuts off any further generated text. For example, for a
5 minutes and 30 seconds duration, instead of having this:
'P', 'T', '5', 'M', '3', '0', 'S', '\0'
The function generates this:
'P', 'T', '5', 'M', '\0', '3', '0', 'S', '\0'
Fix the bug by adding to 'str' just 'durationlen[i]' instead of
'durationlen[i] + 1'.
(cherry picked from commit dc55f1ebb9)
