Instead of using an extra rarely-used paramater to dns_clientinfo_init()
to set ECS information for a client, this commit adds a function
dns_clientinfo_setecs() which can be called only when ECS is needed.
(cherry picked from commit ff3fdaa424)
when generating a key, if a DH key already existed for the same
name, a spurious warning message was generated saying "bad key
type". this is fixed.
(cherry picked from commit 82503bec99)
the optional 'port' option, when used with notify-source,
transfer-source, etc, is used to set up UDP dispatches with a
particular source port, but when the actual UDP connection was
established the port would be overridden with a random one. this
has been fixed.
(configuring source ports is deprecated in 9.20 and slated for
removal in 9.22, but should still work correctly until then.)
(cherry picked from commit 4d50c912ba)
it was possible for a managed trust anchor needing to send a key
refresh query to be unable to do so because an authoritative zone
was not yet loaded. this has been corrected by delaying the
synchronization of managed-keys zones until after all zones are
loaded.
(cherry-picked from commit bafbbd2465)
Commit 6f998bbe51 added a new parameter,
'options', to the prototype of the 'allrdatasets' function pointer in
struct dns_dbmethods. Handle this new parameter accordingly in
rpsdb_allrdatasets().
(cherry picked from commit f3def4e4ed)
The ADB hashmaps are stored in extra memory contexts, so the hash
tables are excluded from the overmem accounting. The new memory
context was unnamed, give it a proper name.
Same thing has happened with extra memory context used for named
global log context - give the extra memory context a proper name.
(cherry picked from commit 3cda9f9f14)
Set the DS state after issuing 'rndc dnssec -checkds'. If the DS
was published, it should go in RUMOURED state, regardless whether it
is already safe to do so according to the state machine.
Leaving it in HIDDEN (or if it was magically already in OMNIPRESENT or
UNRETENTIVE) would allow for easy shoot in the foot situations.
Similar, if the DS was withdrawn, the state should be set to
UNRETENTIVE. Leaving it in OMNIPRESENT (or RUMOURED/HIDDEN)
would also allow for easy shoot in the foot situations.
(cherry picked from commit ee42f66fbe)
It is allowed to point parental-agents to a resolver. Therefore, the
RD bit should be set on requests.
Upon receiving a DS response, ensure that the message has either the
AA or the RA bit set.
(cherry picked from commit e34722ed43)
The write node lock needs to be held when setting node->wild in
add_wildcard_magic except when being called from loading_addrdataset
which is used to load the zone without locking during its initial
load.
(cherry picked from commit 81c24b8da2)
Return 'isc_result_t' type value instead of 'bool' to indicate
the actual failure. Rename the function to something not suggesting
a boolean type result. Make changes in the places where the API
function is being used to check for the result code instead of
a boolean value.
(cherry picked from commit 41dc48bfd7)
Detaching the views in the zone_shutdown() could lead to
lock-order-inversion between adb->namelocks[bucket], adb->lock,
view->lock and zone->lock. Detach the views outside of the section that
zone-locked.
(cherry picked from commit 978a0ef84c)
The reference counting and isc_timer_attach()/isc_timer_detach()
semantic are actually misleading because it cannot be used under normal
conditions. The usual conditions under which is timer used uses the
object where timer is used as argument to the "timer" itself. This
means that when the caller is using `isc_timer_detach()` it needs the
timer to stop and the isc_timer_detach() does that only if this would be
the last reference. Unfortunately, this also means that if the timer is
attached elsewhere and the timer is fired it will most likely be
use-after-free, because the object used in the timer no longer exists.
Remove the reference counting from the isc_timer unit, remove
isc_timer_attach() function and rename isc_timer_detach() to
isc_timer_destroy() to better reflect how the API needs to be used.
The only caveat is that the already executed event must be destroyed
before the isc_timer_destroy() is called because the timer is no longet
attached to .ev_destroy_arg.
(cherry picked from commit ae01ec2823)
When we are loading the zones, set the quantum to UINT_MAX, which makes
task_run process all tasks at once. After the zone loading is finished
the quantum will be dropped to 1 to not block server when we are loading
new zones after reconfiguration.
(cherry picked from commit 87c4c24cde)
The .view (and possibly .prev_view) would be kept attached to the
removed zone until the zone is fully removed from the memory in
zone_free(). If this process is delayed because server is busy
something else like doing constant `rndc reconfig`, it could take
seconds to detach the view, possibly keeping multiple dead views in the
memory. This could quickly lead to a massive memory bloat.
Release the views early in the zone_shutdown() call, and don't wait
until the zone is freed.
(cherry picked from commit 13bb821280)
During XoT it is important to check for "dot" ALPN tag to be
negotiated (according to the RFC 9103). We were doing that, however, the
situation was not handled properly, leading to non-cancelled zone
transfers that would crash (abort()) BIND on shutdown.
In this particular case 'result' might equal 'ISC_R_SUCCESS'. When
this is the case, the part of the code supposed to handle failures
will not cancel the zone transfer.
This situation cannot happen when BIND is a secondary of other BIND
instance. Only primaries following the RFC not closely enough could
trigger such a behaviour.
(cherry picked from commit 34a1aab1cb)
Include isc_rwlocktype_t type definition in zt.h
See merge request isc-projects/bind9!7376
(cherry picked from commit d7bcdf8bd6)
395d6fca Include isc_rwlocktype_t type definition in zt.h
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)
Previously, an incremental hash table resizing was implemented for the
dns_rbt_t hash table implementation. Using that as a base, also
implement the incremental hash table resizing also for isc_ht API
hashtables:
1. During the resize, allocate the new hash table, but keep the old
table unchanged.
2. In each lookup, delete, or iterator operation, check both tables.
3. Perform insertion operations only in the new table.
4. At each insertion also move <r> elements from the old table to
the new table.
5. When all elements are removed from the old table, deallocate it.
To ensure that the old table is completely copied over before the new
table itself needs to be enlarged, it is necessary to increase the
size of the table by a factor of at least (<r> + 1)/<r> during resizing.
In our implementation <r> is equal to 1.
The downside of this approach is that the old table and the new table
could stay in memory for longer when there are no new insertions into
the hash table for prolonged periods of time as the incremental
rehashing happens only during the insertions.
(cherry picked from commit e42cb1f198)
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)
DSCP has not been fully working since the network manager was
introduced in 9.16, and has been completely broken since 9.18.
This seems to have caused very few difficulties for anyone,
so we have now marked it as obsolete and removed the
implementation.
To ensure that old config files don't fail, the code to parse
dscp key-value pairs is still present, but a warning is logged
that the feature is obsolete and should not be used. Nothing is
done with configured values, and there is no longer any
range checking.
(cherry picked from commit 916ea26ead)
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)
Previously, dns_dispatch_gettcp() could pick a TCP connection created by
different thread - this breaks our contractual promise to DNS dispatch
by using the TCP connection on a different thread than it was created.
Add .tid member to the dns_dispatch_t struct and skip the dispatches
from other threads when looking up a TCP dispatch that we can reuse in
dns_request.
NOTE: This is going to be properly refactored, but this change could be
also backported to 9.18 for better stability and thread-affinity.
(cherry picked from commit 1a999353cd)
If the isc_task_create_bound() fails in the middle of buckets
initialization - the most common case would be shutdown initialized
during reload, not all tasks would be initialized, but the cleanup
code would try to cleanup all buckets.
Make sure that we cleanup only the initialized buckets by setting
ntasks to the number of already initialized tasks on the error path.
The 'localaddr' pointer can be NULL, which causes an assertion failure.
Use '&disp->local' instead when printing a debug log message.
(cherry picked from commit 41ca9d419e)
Additionally to renaming, it changes the function definition so that
it accepts a pointer to pointer instead of returning a pointer to the
new object.
It is mostly done to make it in line with other functions in the
module.
(cherry picked from commit 7962e7f575)
Normally, when a 'resquery_t' object is created in fctx_query(),
we call dns_adb_beginudpfetch() (which increases the ADB quota)
only if it's a UDP query. Then, in fctx_cancelquery(), we call
dns_adb_endudpfetch() to decreases back the ADB quota, again only
if it's a UDP query.
The problem is that a UDP query can become a TCP query, preventing
the quota from adjusting back in fctx_cancelquery() later.
Call dns_adb_beginudpfetch() also when switching the query type
from UDP to TCP.
(cherry picked from commit 53afe1f978)
The dns_request code is very sensitive about calling the connected and
deadlocks when the timing is "right" in several places. Move the call
to the connected callback to the (udp|tcp)_connected() functions, so
they are called asynchronously instead of directly from
the (udp|tcp)_dispentry_cancel() functions.
(cherry picked from commit 9dd8deaf01)
The TCP dispatches are removed from the dispatchmgr->list in the
dispatch_destroy() and there's a brief period of time where
dns_dispatch_gettcp() can find a dispatch in connected state that's
being destroyed.
Set the dispatch state to DNS_DISPATCHSTATE_NONE in the TCP connection
callback if there are no responses waiting, and ignore TCP dispatches
with zero references in dns_dispatch_gettcp().
(cherry picked from commit 3fac4ca57e)
In tcp_connected() a typo has turned a DbC check into an assignment
breaking the state machine and making the dns_dispatch_gettcp() try to
attach to dispatch in process of destruction.
The TCP dispatches in DNS_DISPATCHSTATE_NONE could be either very
fresh or those could be dispatches that failed connecting to the
destination. Ignore them when trying to connect to an existing
TCP dispatch via dns_dispatch_gettcp().
The dispatches are not thread-bound, and used freely between various
threads (see the dns_resolver and dns_request units for details).
This refactoring make sure that all non-const dns_dispatch_t and
dns_dispentry_t members are accessed under a lock, and both object now
track their internal state (NONE, CONNECTING, CONNECTED, CANCELED)
instead of guessing the state from the state of various struct members.
During the refactoring, the artificial limit DNS_DISPATCH_SOCKSQUOTA on
UDP sockets per dispatch was removed as the limiting needs to happen and
happens on in dns_resolver and limiting the number of UDP sockets
artificially in dispatch could lead to unpredictable behaviour in case
one dispatch has the limit exhausted by others are idle.
The TCP artificial limit of DNS_DISPATCH_MAXREQUESTS makes even less
sense as the TCP connections are only reused in the dns_request API
that's not a heavy user of the outgoing connections.
As a side note, the fact that UDP and TCP dispatch pretends to be same
thing, but in fact the connected UDP is handled from dns_dispentry_t and
dns_dispatch_t acts as a broker, but connected TCP is handled from
dns_dispatch_t and dns_dispatchmgr_t acts as a broker doesn't really
help the clarity of this unit.
This refactoring kept to API almost same - only dns_dispatch_cancel()
and dns_dispatch_done() were merged into dns_dispatch_done() as we need
to cancel active netmgr handles in any case to not leave dangling
connections around. The functions handling UDP and TCP have been mostly
split to their matching counterparts and the dns_dispatch_<function>
functions are now thing wrappers that call <udp|tcp>_dispatch_<function>
based on the socket type.
More debugging-level logging was added to the unit to accomodate for
this fact.
(cherry picked from commit 6f317f27ea)
The overmem cleaning in ADB could become overzealous and clean fresh ADB
names and entries. Add a safety check to not clean any ADB names and
entries that are below ADB_CACHE_MINIMUM threshold.
(cherry picked from commit 0b661b6f95)
The ADB overmem accounting would include the memory used by hashtables
thus vastly reducing the space that can be used for ADB names and
entries when the hashtables would grow. Create own memory context for
the ADB names and entries hash tables.
(cherry picked from commit 59dee0b078)
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)
when serve-stale is enabled, NXDOMAIN cache entries are no longer
preserved after the normal negative cache TTL, in order to reduce
unnecessary cache memory consumption.
(cherry picked from commit f1485ca145)
