Commit graph

8695 commits

Author SHA1 Message Date
Aram Sargsyan
db45cab546 Fix a lock-order-inversion bug in resolver.c
There is a lock-order-inversion (potential deadlock) in resolver.c,
because in dns_resolver_shutdown() a resolver bucket lock is locked
while the resolver lock itself is already locked, while in
fctx_sendevents() the resolver lock is locked while a bucket lock
is locked before calling that function in fctx__done_detach().

The resolver lock/unlock in dns_resolver_shutdown() was added back in
the 317e36d47e commit to make sure that
the function is finished before the resolver object is destroyed.

Since res->exiting is atomic, it should be possible to remove the
resolver locking in dns_resolver_shutdown() and add it to the
send_shutdown_events() function which requires it.

Also, since 'res->exiting' is now set while unlocked, the 'INSIST'
in spillattimer_countdown() is wrong, and is removed.
2023-06-06 11:02:24 +00:00
Aram Sargsyan
cd47429365 Add ClientQuota statistics channel counter
This counter indicates the number of the resolver's spilled
queries due to reaching the clients per query quota.

(cherry picked from commit 04648d7c2f)
2023-05-31 11:07:08 +00:00
Artem Boldariev
cec8947bc1
ZMGR: TLS contexts cache - properly synchronise access
This commit ensures that access to the TLS context cache within zone
manager is properly synchronised.

Previously there was a possibility for it to get unexpectedly
NULLified for a brief moment by a call to
dns_zonemgr_set_tlsctx_cache() from one thread, while being accessed
from another (e.g. from got_transfer_quota()). This behaviour could
lead to server abort()ing on configuration reload (under very rare
circumstances).

That behaviour has been fixed.

(cherry picked from commit 0b95cf74ff)
2023-05-26 15:24:51 +03:00
Evan Hunt
e9b6991357 fix handling of TCP timeouts
when a TCP dispatch times out, we call tcp_recv() with a result
value of ISC_R_TIMEDOUT; this cancels the oldest dispatch
entry in the dispatch's active queue, plus any additional entries
that have waited longer than their configured timeouts. if, at
that point, there were more dispatch entries still on the active
queue, it resumes reading, but until now it failed to restart
the timer.

this has been corrected: we now calculate a new timeout
based on the oldest dispatch entry still remaining.  this
requires us to initialize the start time of each dispatch entry
when it's first added to the queue.

in order to ensure that the handling of timed-out requests is
consistent, we now calculate the runtime of each dispatch
entry based on the same value for 'now'.

incidentally also fixed a compile error that turned up when
DNS_DISPATCH_TRACE was turned on.

(cherry picked from commit 0e800467ee)
2023-05-26 02:07:02 -07:00
Evan Hunt
afae1b65e8 prevent TSIG keys from being added to multiple rings
it was possible to add a TSIG key to more than one TSIG
keyring at a time, and this was in fact happening with the
session key, which was generated once and then added to the
keyrings for each view as it was configured.

this has been corrected and a REQUIRE added to dns_tsigkeyring_add()
to prevent it from happening again.
2023-05-25 15:08:48 -07:00
Aram Sargsyan
7713d2cb6d Check whether zone->db is a valid pointer before attaching
The zone_resigninc() function does not check the validity of
'zone->db', which can crash named if the zone was unloaded earlier,
for example with "rndc delete".

Check that 'zone->db' is not 'NULL' before attaching to it, like
it is done in zone_sign() and zone_nsec3chain() functions, which
can similarly be called by zone maintenance.

(cherry picked from commit fae0930eb8)
2023-05-15 12:01:23 +00:00
Ondřej Surý
e0620ca208
Honour the source-port when retrying in dns_dispatch
When retrying in the DNS dispatch, the local port would be forgotten on
ISC_R_ADDRINUSE, keep the configured source-port even when retrying.

Additionally, treat ISC_R_NOPERM same as ISC_R_ADDRINUSE.

Closes: #3986
(cherry picked from commit c8e8ccd026)
2023-05-05 15:40:12 +02:00
Mark Andrews
0eee434741 Cleanup orphaned empty-non-terminal NSEC3
When OPTOUT was in use we didn't ensure that NSEC3 records
for orphaned empty-non-terminals where removed.  Check if
there are orphaned empty-non-terminal NSEC3 even if there
wasn't an NSEC3 RRset to be removed in dns_nsec3_delnsec3.

(cherry picked from commit 27160c137f)
2023-04-25 06:46:08 +01:00
Ondřej Surý
fa2cb06c75
Implement maximum global and idle time for incoming XFR
After the dns_xfrin was changed to use network manager, the maximum
global (max-transfer-time-in) and idle (max-transfer-idle-in) times for
incoming transfers were turned inoperational because of missing
implementation.

Restore this functionality by implementing the timers for the incoming
transfers.

(cherry picked from commit d2377f8e04)
2023-04-21 17:21:32 +02:00
Evan Hunt
960e4dadd2
check for invalid protocol when dispatch fails
treat ISC_R_INVALIDPROTO as a networking error when it occurs.

(cherry picked from commit 2269a3e6fb)
2023-04-21 12:47:07 +02:00
Matthijs Mekking
bf5eea515d Add key state init debugging
When debugging an issue it can be useful to see what BIND initially
set the key states to.

(cherry picked from commit e752656a38)
2023-04-17 12:14:06 +02:00
Ondřej Surý
07720989a0
Eliminate the dead code in dst_api.c
In write_public_key() and write_key_state(), there were left-over checks
for result, that were effectively dead code after the last refactoring.
Remove those.

(cherry picked from commit 766366e934)
2023-04-05 09:11:24 +02:00
Ondřej Surý
7e7985c9c7 Attach catzs to catz instead of doing this explicitly
Instead of explicitly adding a reference to catzs (catalog zones) when
calling the update callback, attach the catzs to the catz (catalog zone)
object to keep it referenced for the whole time the catz exists.

(cherry picked from commit 2ded876db2)
2023-04-04 11:47:43 +00:00
Ondřej Surý
2bb4fa34cc
Fix xfrin_connect_done() error paths
The xfrin_connect_done() had several problems:

- it would not add the server to unreachable table in case of the
  failure coming from the dispatch [GL #3989]

- if dns_dispatch_checkperm() disallowed the connection, the xfr would
  be left undetached

- if xfrin_send_request() failed to send the request, the xfr would be
  left undetached

All of these have been fixed in this commit.

(cherry picked from commit 536e439c79)
2023-04-04 10:07:27 +02:00
Aram Sargsyan
7beda284d2 Do not resend TCP requests
The req_response() function is using 'udpcount' variable to resend
the request 'udpcount' times on timeout even for TCP requests,
which does not make sense, as it would use the same connection.

Add a condition to use the resend logic only for UDP requests.

(cherry picked from commit edcdb881da)
2023-04-03 16:18:39 +00:00
Aram Sargsyan
14084d8eac Perform request validation in req_response() before using the pointer
The 'request' pointer is used before it is checked. Perform the check
before using the pointer.

(cherry picked from commit 5b37359697)
2023-04-03 16:18:39 +00:00
Aram Sargsyan
4dc2ff79d6 Synchronize dns_request_createraw() and dns_request_create() UDP timeout
The dns_request_createraw() function, unlike dns_request_create(), when
calculating the UDP timeout value, doesn't check that 'udpretries' is
not zero, and that is the more logical behavior, because the calculation
formula uses division to 'udpretries + 1', where '1' is the first try.

Change the dns_request_create() function to remove the 'udpretries != 0'
condition.

Add a 'REQUIRE(udpretries != UINT_MAX)' check to protect from a division
by zero.

Make the 'request->udpcount' field to represent the number of tries,
instead of the number of retries.

(cherry picked from commit 643abfbba7)
2023-04-03 16:18:39 +00:00
Mark Andrews
39c82bf429
dns_view_untrust modifies dnskey->flags when it shouldn't
Copy the structure and declare dnskey as const.

(cherry picked from commit 21d828241b)
2023-04-03 17:46:13 +02:00
Mark Andrews
972c245065
Handle dns_rdata_fromstruct failure dns_keytable_deletekey
dns_rdata_fromstruct in dns_keytable_deletekey can potentially
fail with ISC_R_NOSPACE.  Handle the error condition.

(cherry picked from commit b5df9b8591)
2023-04-03 17:46:13 +02:00
Mark Andrews
4920fca095
Reduce the number of verifiations required
In selfsigned_dnskey only call dns_dnssec_verify if the signature's
key id matches a revoked key, the trust is pending and the key
matches a trust anchor.  Previously named was calling dns_dnssec_verify
unconditionally resulted in busy work.

(cherry picked from commit e68fecbdaa)
2023-04-03 17:46:13 +02:00
Mark Andrews
59f17be969
Add new view method dns_view_istrusted
dns_view_istrusted determines if the given key is treated as
being trusted by the view.

(cherry picked from commit 7278fff579)
2023-04-03 17:46:13 +02:00
Ondřej Surý
718893ece4
Replace isc_fsaccess API with more secure file creation
The isc_fsaccess API was created to hide the implementation details
between POSIX and Windows APIs.  As we are not supporting the Windows
APIs anymore, it's better to drop this API used in the DST part.

Moreover, the isc_fsaccess was setting the permissions in an insecure
manner - it operated on the filename, and not on the file descriptor
which can lead to all kind of attacks if unpriviledged user has read (or
even worse write) access to key directory.

Replace the code that operates on the private keys with code that uses
mkstemp(), fchmod() and atomic rename() at the end, so at no time the
private key files have insecure permissions.

(cherry picked from commit 263d232c79)
2023-03-31 16:47:15 +02:00
Matthijs Mekking
9874a26686 Fix scan-build issue: initialized value never read
Value stored to 'source' during its initialization is never read.

(cherry picked from commit 4c33277446)
2023-03-29 13:31:49 +00:00
Aram Sargsyan
987f61dbb7 Fix a data race in dns__catz_update_cb()
The dns__catz_update_cb() function was earlier updated (see
d2ecff3c4a) to use a separate
'dns_db_t' object ('catz->updb' instead of 'catz->db') to
avoid a race between the 'dns__catz_update_cb()' and
'dns_catz_dbupdate_callback()' functions, but the 'REQUIRE'
check there still checks the validity of the 'catz->db' object.

Fix the omission.

(cherry picked from commit a2817541b3)
2023-03-27 11:39:19 +00:00
Ondřej Surý
4bf253ffe1
Properly handle ISC_R_SHUTTINGDOWN in resquery_response()
When resquery_response() was called with ISC_R_SHUTTINDOWN, the region
argument would be NULL, but rctx_respinit() would try to pass
region->base and region->len to the isc_buffer_init() leading to
a NULL pointer dereference.  Properly handle non-ISC_R_SUCCESS by
ignoring the provided region.

(cherry picked from commit 93259812dd)
2023-03-23 12:26:09 +01:00
Aram Sargsyan
eef58e9708 Hold a catz reference while the update process is running
This should delay the catalog zone from being destroyed during
shutdown, if the update process is still running.

Doing this should not introduce significant shutdown delays, as
the update function constantly checks the 'shuttingdown' flag
and cancels the process if it is set.

(cherry picked from commit dc2b8bb1c9)
2023-03-21 11:47:17 +00:00
Mark Andrews
7f13c9d3fa When signing with a new algorithm preserve NSEC/NSEC3 chains
If the zone already has existing NSEC/NSEC3 chains then zone_sign
needs to continue to use them.  If there are no chains then use
kasp setting otherwise generate an NSEC chain.

(cherry picked from commit 4b55201459)
2023-03-15 00:18:54 +11:00
Mark Andrews
bc7c9c6ad1 Report key name when removing it
(cherry picked from commit 9f161544fd)
2023-03-14 16:26:56 +11:00
Mark Andrews
92cab3fb5e Add ISC_FORMAT_PRINTF to report's declaration
dns_dnssec_updatekeys's 'report' could be called with invalid arguments
which the compiler should be be able to detect.

(cherry picked from commit 7a0a2fc3e4)
2023-03-14 16:26:56 +11:00
Aram Sargsyan
9c48b6619a Check if catz is active in dns__catz_update_cb()
A reconfiguration can deactivate the catalog zone, while the
offloaded update process was preparing to run.

(cherry picked from commit 6980e3b354)
2023-03-02 18:57:16 +00:00
Aram Sargsyan
2e348627a1 Check if catz is active in dns__catz_timer_cb()
A reconfiguration can deactivate the catalog zone, while the
update process was deferred using a timer.

(cherry picked from commit 67c77aba38)
2023-03-02 18:57:16 +00:00
Aram Sargsyan
fb15a6d6f6 Use catzs->lock in dns_catz_prereconfig()
There can be an update running in another thread, so use a lock,
like it's done in dns_catz_postreconfig().

(cherry picked from commit 3973724d67)
2023-03-02 18:57:16 +00:00
Aram Sargsyan
d6001423af catz: protect db_registered and db callback (un)registration with a lock
Doing this to avoid a race between the 'dns__catz_update_cb()' and
'dns_catz_dbupdate_callback()' functions.

(cherry picked from commit a87859f1fa)
2023-03-02 18:57:16 +00:00
Aram Sargsyan
574682b8c7 catz: use two pairs of dns_db_t and dns_dbversion_t in a catalog zone
As it is done in the RPZ module, use 'db' and 'dbversion' for the
database we are going to update to, and 'updb' and 'updbversion' for
the database we are working on.

Doing this should avoid a race between the 'dns__catz_update_cb()' and
'dns_catz_dbupdate_callback()' functions.

(cherry picked from commit d2ecff3c4a)
2023-03-02 18:57:16 +00:00
Aram Sargsyan
6834ea968e Revert "Process db callbacks in zone_loaddone() after zone_postload()"
This reverts commit a719647023.

The commit introduced a data race, because dns_db_endload() is called
after unfreezing the zone.

(not cherry picked from commit 593dea871a)
2023-03-02 18:55:06 +00:00
Aram Sargsyan
9fdce9948e Add a lock for dns_catz_zone_t
Use a lock for the catalog zones during dns__catz_zones_merge() to
avoid races between 'catz' and 'parentcatz'.

(cherry picked from commit 2ae3bc6e1d)
2023-03-01 17:05:15 +00:00
Aram Sargsyan
6b7d2df6b8 Finish catalog zone 'zone' and 'zones' to 'catz' and 'catzs' renaming
There are leftovers from the previous refactoring effort, which left
some function declarations and comments in the header file unchanged.

Finish the renaming.

(cherry picked from commit 580ef2e18f)
2023-02-28 14:52:35 +00:00
Aram Sargsyan
e9acfbd43a catz: unregister the db update-notify callback before detaching from db
When detaching from the previous version of the database, make sure
that the update-notify callback is unregistered, otherwise there is
an INSIST check which can generate an assertion failure in free_rbtdb(),
which checks that there are no outstanding update listeners in the list.

There is a similar code already in place for RPZ.

(cherry picked from commit cf79692a66)
2023-02-28 13:47:18 +00:00
Aram Sargsyan
00569f62b3 Searching catzs->zones requires a read lock
Lock the catzs->lock mutex before searching in the catzs->zones
hash table.

(cherry picked from commit 0ef0c86632)
2023-02-28 13:47:18 +00:00
Aram Sargsyan
a719647023 Process db callbacks in zone_loaddone() after zone_postload()
The zone_postload() function can fail and unregister the callbacks.

Call dns_db_endload() only after calling zone_postload() to make
sure that the registered update-notify callbacks are not called
when the zone loading has failed during zone_postload().

Also, don't ignore the return value of zone_postload().

(cherry picked from commit ed268b46f1)
2023-02-28 13:47:18 +00:00
Ondřej Surý
8b059b211f Pause the catz dbiterator while processing the zone
The dbiterator read-locks the whole zone and it stayed locked during
whole processing time when catz is being read.  Pause the iterator, so
the updates to catz zone are not being blocked while processing the catz
update.

(cherry picked from commit 4e7187601f)
2023-02-28 11:11:17 +00:00
Ondřej Surý
d13e7472ea Unlock catzs during dns__catz_update_cb()
Instead of holding the catzs->lock the whole time we process the catz
update, only hold it for hash table lookup and then release it.  This
should unblock any other threads that might be processing updates to
catzs triggered by extra incoming transfer.

(cherry picked from commit b1cd4a066a)
2023-02-28 11:11:17 +00:00
Aram Sargsyan
16dc8c3977 Offload catalog zone updates
Offload catalog zone processing so that the network manager threads
are not interrupted by a large catalog zone update.

Introduce a new 'updaterunning' state alongside with 'updatepending',
like it is done in the RPZ module.

Note that the dns__catz_update_cb() function currently holds the
catzs->lock during the whole process, which is far from being optimal,
but the issue is going to be addressed separately.

(cherry picked from commit 0b96c9234f)
2023-02-28 11:11:17 +00:00
Aram Sargsyan
ed942f5536 Add shutdown signaling for catalog zones
This change should make sure that catalog zone update processing
doesn't happen when the catalog zone is being shut down. This
should help avoid races when offloading the catalog zone updates
in the follow-up commit.

(cherry picked from commit 246b7084d6)
2023-02-28 09:50:42 +00:00
Aram Sargsyan
f8663976ff Add reference count tracing for dns_catz_zone_t and dns_catz_zones_t
Tracing can be activated by defining DNS_RPZ_TRACE in catz.h.

(cherry picked from commit 53f0c5a9ac)
2023-02-27 20:43:48 +00:00
Aram Sargsyan
43d99eb8b8 Light refactoring of catz.c
* Change 'dns_catz_new_zones()' function's prototype (the order of the
  arguments) to synchronize it with the similar function in rpz.c.
* Rename 'refs' to 'references' in preparation of ISC_REFCOUNT_*
  macros usage for reference tracking.
* Unify dns_catz_zone_t naming to catz, and dns_catz_zones_t naming to
  catzs, following the logic of similar changes in rpz.c.
* Use C compound literals for structure initialization.
* Synchronize the "new zone version came too soon" log message with the
  one in rpz.c.
* Use more of 'sizeof(*ptr)' style instead of the 'sizeof(type_t)' style
  expressions when allocating or freeing memory for 'ptr'.

(cherry picked from commit 8cb79fec9d)
2023-02-27 20:43:48 +00:00
Aram Sargsyan
9050481d1f Fix a cleanup bug when isc_task_create() fails in dns_catz_new_zones()
Use isc_mem_putanddetach() instead of isc_mem_put() to detach from the
memory context.
2023-02-27 12:11:20 +00:00
Aram Sargsyan
6f8fb0241a Fix a memory leak when isc_timer_create() fails in dns_catz_new_zone()
Destroy 'new_zone->coos' in dns_catz_new_zone() on error path.
2023-02-27 12:04:01 +00:00
Mark Andrews
87602eecd7 Fix dns_kasp_attach / dns_kasp_detach usage
The kasp pointers in dns_zone_t should consistently be changed by
dns_kasp_attach and dns_kasp_detach so the usage is balanced.

(cherry picked from commit b41882cc75)
2023-02-21 16:54:53 +01:00
Mark Andrews
0d56344a8e In hmac_createctx free ctx on isc_hmac_init failure
(cherry picked from commit d22257a370)
2023-02-18 10:26:55 +11:00
Aram Sargsyan
aafe857e48 Fix RPZ reference counting error on shutdown
A dns_rpz_unref_rpzs() call is missing when taking the 'goto unlock;'
path on shutdown, in order to compensate for the earlier
dns_rpz_ref_rpzs() call.

Move the dns_rpz_ref_rpzs() call after the shutdown check.

(cherry picked from commit afbe63565f)
2023-02-14 11:21:40 +00:00
Aram Sargsyan
c3a4e14243 Fix an error path bug in rpz.c:update_nodes()
When dns_db_createiterator() fails, 'updbit' should not be destroyed
for obvious reasons, i.e. it is NULL.

(cherry picked from commit ef4f15d2d1)
2023-02-14 10:26:37 +00:00
Aram Sargsyan
c72b19b5e8 Fix a shutdown and error path bugs in rpz.c:update_nodes()
When shutting down, or when dns_dbiterator_current() fails, 'node'
shouldn't be detached, because it is NULL at that point.

(cherry picked from commit d36728e42f)
2023-02-14 10:26:37 +00:00
Aram Sargsyan
cc4cb67149 Fix a shutdown bug in update_rpz_cb()
When shutting down, the cleanup path should not try to destroy
'newnodes', because it is NULL at that point.

Introduce another label for the "shuttingdown" scenario.

(cherry picked from commit 975d16230b)
2023-02-14 10:26:37 +00:00
Ondřej Surý
5baf3ac3b6 Refactor dns_rpz unit to use single reference counting
The dns_rpz_zones structure was using .refs and .irefs for strong and
weak reference counting.  Rewrite the unit to use just a single
reference counting + shutdown sequence (dns_rpz_destroy_rpzs) that must
be called by the creator of the dns_rpz_zones_t object.  Remove the
reference counting from the dns_rpz_zone structure as it is not needed
because the zone objects are fully embedded into the dns_rpz_zones
structure and dns_rpz_zones_t object must never be destroyed before all
dns_rpz_zone_t objects.

The dns_rps_zones_t reference counting uses the new ISC_REFCOUNT_TRACE
capability - enable by defining DNS_RPZ_TRACE in the dns/rpz.h header.

Additionally, add magic numbers to the dns_rpz_zone and dns_rpz_zones
structures.

(cherry picked from commit 77659e7392)
2023-02-14 09:58:16 +00:00
Mark Andrews
4a7c78b290 Report the key name that failed in retry_keyfetch
When there are multiple managed trust anchors we need to know the
name of the trust anchor that is failing.  Extend the error message
to include the trust anchor name.

(cherry picked from commit fb7b7ac495)
2023-02-14 10:44:39 +11:00
Ondřej Surý
6873cc1c79 Run the RPZ update as offloaded work
Previously, the RPZ updates ran quantized on the main nm_worker loops.
As the quantum was set to 1024, this might lead to service
interruptions when large RPZ update was processed.

Change the RPZ update process to run as the offloaded work.  The update
and cleanup loops were refactored to do as little locking of the
maintenance lock as possible for the shortest periods of time and the db
iterator is being paused for every iteration, so we don't hold the rbtdb
tree lock for prolonged periods of time.

(cherry picked from commit f106d0ed2b)
2023-02-13 11:41:52 +00:00
Ondřej Surý
bb6029db64 Refactor the dns_rpz_add/delete to use local rpz copy
Previously dns_rpz_add() were passed dns_rpz_zones_t and index to .zones
array.  Because we actually attach to dns_rpz_zone_t, we should be using
the local pointer instead of passing the index and "finding" the
dns_rpz_zone_t again.

Additionally, dns_rpz_add() and dns_rpz_delete() were used only inside
rpz.c, so make them static.

(cherry picked from commit b6e885c97f)
2023-02-13 11:41:52 +00:00
Ondřej Surý
74bd205177 General cleanup of dns_rpz implementation
Do a general cleanup of lib/dns/rpz.c style:

 * Removed deprecated and unused functions
 * Unified dns_rpz_zone_t naming to rpz
 * Unified dns_rpz_zones_t naming to rpzs
 * Add and use rpz_attach() and rpz_attach_rpzs() functions
 * Shuffled variables to be more local (cppcheck cleanup)

(cherry picked from commit 840179a247)
2023-02-13 11:41:52 +00:00
Evan Hunt
342286ecdb remove isc_bind9 variable
isc_bind9 was a global bool used to indicate whether the library
was being used internally by BIND or by an external caller. external
use is no longer supported, but the variable was retained for use
by dyndb, which needed it only when being built without libtool.
building without libtool is *also* no longer supported, so the variable
can go away.

(cherry picked from commit 935879ed11)
2023-02-09 10:07:39 -08:00
Mark Andrews
4fd22a2228 Define DNS_RDATASET_INIT for static initialisation
(cherry picked from commit ddc4d1fca4)
2023-02-09 08:58:15 +11:00
Evan Hunt
9f1c6d9744 refactor dns_clientinfo_init(); use separate function to set ECS
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)
2023-02-08 00:13:12 -08:00
Evan Hunt
bc3be6dc29 silence a spurious warning during key generation
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)
2023-02-07 23:30:24 -08:00
Evan Hunt
5508e25376 use configured source ports for UDP requests
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)
2023-02-06 17:02:02 -08:00
Evan Hunt
ac150939d5 delay trust anchor management until zones are loaded
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)
2023-02-06 14:27:16 -08:00
Michał Kępień
41743a3061 Handle iterator options in rpsdb_allrdatasets()
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)
2023-02-01 11:26:31 +01:00
Ondřej Surý
a25e41da02
Properly name ADB hashmap and named log memory contexts
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)
2023-01-30 12:38:13 +01:00
Matthijs Mekking
68f0fc6309 Force set DS state after 'rndc dnssec -checkds'
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)
2023-01-30 09:27:38 +01:00
Michał Kępień
8b4dcc27ef BIND 9.18.11
-----BEGIN PGP SIGNATURE-----
 
 iQJDBAABCgAtFiEENKwGS3ftSQfs1TU17QVz/8hFYQUFAmPAh6gPHG1pY2hhbEBp
 c2Mub3JnAAoJEO0Fc//IRWEFyGsQAJuggfdFRAFzH6QTlE+jYPGGPGGAEp9+lFuP
 ufCdlek5FKN/V/NFpuMfNeyQ3NhK93ofMzaluAg47vM5Cj2/lPxUUFea7w20tHVm
 Nqsxk4Lc+RsnngVNUwWtA6CkwAGHDQA0Rwa3OEjqPkm33KLwCeC3w3ufv6KPlT7m
 MubNOd7BogMBxDg63TnOlSBjcKFi/TzGCNmOVj1cyOj9QP52XeIe6iKol4g47mWG
 erQ8ZKV/vWoIRCwLdPheRgCgO/2KyHLGbtI+uJ53OExiYnrKL18wGnt1Foo8Er9V
 hOkBykzgtWTtgrl8Ljd1lbR6FjZvLgcWWIZ6oM2RXjD25942lNgyWYubQHsRHchi
 /vnFD3qg5SBBbCHuzIzy9QCk2YYwJiDpI8t2RngzhJOexHGcCLYyM99yriqNYnFw
 DHoFkcUbJiHGhtEzzGuhz7LrSySclvqQRYbWLh7qcuUIKGdbPiWB8BmZtAkaFyaN
 fOJYwk8pSlpgvFqaAOicG3hAWTUxcJ5U/wWdBFk7Xg3wZ/K2XLuA88QgxePh2S2L
 kYBwwD81amWMEZct1hq9PW42vFFiWjJtZnTceZjCVARQamJ/+QgjUapMfbnYb1jN
 ry4XQoFz3FhfT4Ow2cKfRUzrh8lrNUJNqMoNiXDnj4jjH1YwIN6NqIYqrXJUGeCU
 yaaBGMu/
 =XdZL
 -----END PGP SIGNATURE-----

Merge tag 'v9_18_11' into v9_18

BIND 9.18.11
2023-01-25 21:26:22 +01:00
Matthijs Mekking
89089817d3 Set RD bit on checkds requests
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)
2023-01-19 16:56:22 +01:00
Mark Andrews
4508b0a1e2 Add missing node lock when setting node->wild in rbtdb.c
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)
2023-01-20 00:46:24 +11:00
Aram Sargsyan
8f209c7dcf Refactor isc_nm_xfr_allowed()
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)
2023-01-19 12:20:10 +00:00
Ondřej Surý
3d3c8a935a
Detach the zone views outside of the zone lock
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)
2023-01-19 10:21:19 +01:00
Ondřej Surý
e26aa4cbb1
Don't use reference counting in isc_timer unit
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)
2023-01-18 22:39:26 +01:00
Ondřej Surý
a7055b01af
Set quantum to infinity for the zone loading task
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)
2023-01-18 18:04:41 +01:00
Ondřej Surý
f8b7188a42
Detach the views in zone_shutdown(), not in zone_free()
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)
2023-01-17 22:47:43 +01:00
Artem Boldariev
dbd15f962e XoT: properly handle the case when checking for ALPN failed
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)
2023-01-17 21:27:44 +02:00
Ondřej Surý
c5c432a36a Merge branch 'feature/main/zt-rwlock.h' into 'main'
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
2023-01-16 11:07:19 +00:00
Aram Sargsyan
a4fc5e5158 Cancel all fetch events in dns_resolver_cancelfetch()
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)
2023-01-12 12:54:02 +01:00
Ondřej Surý
8c31a939c9
Implement incremental hash table resizing in isc_ht
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)
2023-01-11 17:15:33 +01:00
Aram Sargsyan
39cc597022 Use sizeof(*ptr) for allocating/freeing memory in forward.c
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)
2023-01-11 13:42:09 +00:00
Aram Sargsyan
64c8e63942 Fix dns_fwdtable_addfwd() error path cleanup bug
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)
2023-01-11 13:42:09 +00:00
Aram Sargsyan
4121af8f7e Fix a use-after-free bug in dns_zonemgr_releasezone()
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)
2023-01-11 12:16:23 +00:00
Evan Hunt
5fd93c66aa remove nonfunctional DSCP implementation
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)
2023-01-09 14:23:26 -08:00
Matthijs Mekking
271bc20b1c Consider non-stale data when in serve-stale mode
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)
2023-01-09 14:26:02 +01:00
Ondřej Surý
0e263b34c6 Pin the dns_dispatch to threads when reusing
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)
2023-01-05 10:20:47 +01:00
Ondřej Surý
d48f5e253f Don't cleanup uninitialized dns_resolver buckets
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.
2023-01-03 10:33:23 +01:00
Aram Sargsyan
35b5ba2ff4 Don't pass a NULL pointer to isc_sockaddr_format()
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)
2022-12-28 14:19:37 +00:00
Artem Boldariev
bccbf28249 tlsctx_client_session_cache_new() -> tlsctx_client_session_create()
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)
2022-12-23 13:58:14 +02:00
Aram Sargsyan
926f0323b6 Fix an ADB quota management error in the resolver
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)
2022-12-23 10:08:00 +00:00
Aram Sargsyan
650f3098e1 INSIST that active quota is 0 in destroy_adbentry()
This should catch ADB quota management errors in the resolver.

(cherry picked from commit c7ba26c3d6)
2022-12-23 10:08:00 +00:00
Ondřej Surý
be3cf85cfe Call the connected dns_dispatch callback asynchronously
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)
2022-12-21 12:41:15 +00:00
Ondřej Surý
87ad3ecaf0 Ignore TCP dispatches that have zero references
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)
2022-12-21 12:41:15 +00:00
Ondřej Surý
3f4970da1a Fix assignment vs comparison typo in tcp_connected()
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.
2022-12-21 12:41:15 +00:00
Ondřej Surý
b714033731 Don't remove dispatches in CANCELED state from the list
In dns_dispatch_gettcp(), we can't remove canceled dispatches from the
mgr->list because ISC_LIST_NEXT() would fail in the next iteration.
2022-12-21 12:41:15 +00:00
Ondřej Surý
3edccaf0c7 Ignore TCP dispatches in DNS_DISPATCHSTATE_NONE state
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().
2022-12-21 12:41:15 +00:00
Ondřej Surý
5cc12ab92c Fix the thread safety in the dns_dispatch unit
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)
2022-12-21 12:41:15 +00:00
Ondřej Surý
146f93ff46
Don't expire fresh ADB names and entries
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)
2022-12-19 09:26:58 +01:00
Ondřej Surý
ecde82689e
Exclude the ADB hashmaps from ADB overmem accounting
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)
2022-12-16 19:22:43 +01:00
Ondřej Surý
095f634f48
Try next server on resolver timeout
Instead of resending to the same server on the (dispatch) timeout in the
resolver, try the next server.

(cherry picked from commit 5466a48fc9)
2022-12-16 18:37:22 +01:00