This commit ensures that 'sock->tls.pending_req' is not getting
nullified during TLS connection timeout callback as it prevents the
connection callback being called when connecting was not successful.
We expect 'isc__nm_failed_connect_cb() to be called from
'isc__nm_tlsdns_shutdown()' when establishing connections was
successful, but with 'sock->tls.pending_req' nullified that will not
happen.
The code removed most likely was required in older iterations of the
NM, but to me it seems that now it does only harm. One of the well
know pronounced effects is leading to irrecoverable zone transfer
hangs via TLS.
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)
libuv support for receiving multiple UDP messages in a single system
call (recvmmsg()) has been tweaked several times between libuv versions
1.35.0 and 1.40.0. Mixing and matching libuv versions within that span
may lead to assertion failures and is therefore considered harmful, so
try to limit potential damage be preventing users from mixing libuv
versions with distinct sets of recvmmsg()-related flags.
(cherry picked from commit 735d09bffe)
The implementation of UDP recvmmsg in libuv 1.35 and 1.36 is
incomplete and could cause assertion failure under certain
circumstances.
Modify the configure and runtime checks to report a fatal error when
trying to compile or run with the affected versions.
(cherry picked from commit 251f411fc3)
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)
This commit ensures that BIND and supplementary tools still can be
built on newer versions of DragonFly BSD. It used to be the case, but
somewhere between versions 6.2 and 6.4 the OS developers rearranged
headers and moved some function definitions around.
Before that the fact that it worked was more like a coincidence, this
time we, at least, looked at the related man pages included with the
OS.
No in depth testing has been done on this OS as we do not really
support this platform - so it is more like a goodwill act. We can,
however, use this platform for testing purposes, too. Also, we know
that the OS users do use BIND, as it is included in its ports
directory.
Building with './configure' and './configure --without-jemalloc' have
been fixed and are known to work at the time the commit is made.
(cherry picked from commit 942569a1bb)
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)
If the OpenSSL SHA1_{Init,Update,Final} API is still available, use it.
The API has been deprecated in OpenSSL 3.0, but it is significantly
faster than EVP_MD API, so make an exception here and keep using it
until we can't.
(cherry picked from commit 25db8d0103)
Instead of going through another layer, use OpenSSL EVP_MD API directly
in the isc_iterated_hash() implementation. This shaves off couple of
microseconds in the microbenchmark.
(cherry picked from commit 36654df732)
as far as I can determine the order of operations is not important.
*** CID 351372: Concurrent data access violations (ATOMICITY)
/lib/isc/timer.c: 227 in timer_purge()
221 LOCK(&timer->lock);
222 if (!purged) {
223 /*
224 * The event has already been executed, but not
225 * yet destroyed.
226 */
>>> CID 351372: Concurrent data access violations (ATOMICITY)
>>> Using an unreliable value of "event" inside the second locked section. If the data that "event" depends on was changed by another thread, this use might be incorrect.
227 timerevent_unlink(timer, event);
228 }
229 }
230 }
231
232 void
(cherry picked from commit 98718b3b4b)
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)
The isc_task_purge() and isc_task_purgerange() were now unused, so sweep
the task.c file. Additionally remove unused ISC_EVENTATTR_NOPURGE event
attribute.
(cherry picked from commit c17eee034b)
Add isc_task_setquantum() function that modifies quantum for the future
isc_task_run() invocations.
NOTE: The current isc_task_run() caches the task->quantum into a local
variable and therefore the current event loop is not affected by any
quantum change.
(cherry picked from commit 15ea6f002f)
Instead of searching for the events to purge, keep the list of scheduled
events on the timer list and purge the events that we have scheduled.
(cherry picked from commit 3f8024b4a2f12fcd28a9dd813b6f1f3f11d506f2)
The isc_task_purgerange() was walking through all events on the task to
find a matching task. Instead use the ISC_LINK_LINKED to find whether
the event is active.
Cleanup the related isc_task_unsend() and isc_task_unsendrange()
functions that were not used anywhere.
(cherry picked from commit 17aed2f895)
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)
Prefer the pthread_barrier implementation on platforms where it is
available over uv_barrier implementation. This also solves the problem
with thread sanitizer builds on macOS that doesn't have pthread barrier.
(cherry picked from commit d07c4a98da)
This reverts commit f17f5e831b that made
following change:
> The TLSDNS transport was not honouring the single read callback for
> TLSDNS client. It would call the read callbacks repeatedly in case the
> single TLS read would result in multiple DNS messages in the decoded
> buffer.
Turns out that this change broke XoT, so we are reverting the change
until we figure out a proper fix that will keep the design promise and
not break XoT at the same time.
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)
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)
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)
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 f102df96b8)
Backport macros that can be used to implement generic attach, detach,
ref, and unref functions, so they don't have to be repeated over and
over in each unit that uses reference counting.
This commit fixes TLS session resumption via session IDs when
client certificates are used. To do so it makes sure that session ID
contexts are set within server TLS contexts. See OpenSSL documentation
for 'SSL_CTX_set_session_id_context()', the "Warnings" section.
(cherry picked from commit 837fef78b1)
This commit ensures that send callbacks are always called from within
the context of its worker thread even in the case of
shuttigdown/inactive socket, just like TCP transport does and with
which TLS attempts to be as compatible as possible.
(cherry picked from commit 2bfc079946)
This commit changes ISC_R_NOTCONNECTED error code to ISC_R_CANCELLED
when attempting to start reading data on the shutting down socket in
order to make its behaviour compatible with that of TCP and not break
the common code in the unit tests.
(cherry picked from commit ef659365ce)
This commit adds a check if 'sock->recv_cb' might have been nullified
during the call to 'sock->recv_cb'. That could happen, e.g. by an
indirect call to 'isc_nmhandle_close()' from within the callback when
wrapping up.
In this case, let's close the TLS connection.
(cherry picked from commit bed5e2bb08)
The TLSDNS transport was not honouring the single read callback for
TLSDNS client. It would call the read callbacks repeatedly in case the
single TLS read would result in multiple DNS messages in the decoded
buffer.
(cherry picked from commit e3c628d562)
Handle the situation in the read and send callbacks when the
httpd->readhandle has been already detached; f.e. from the previous
iteration of the read callback.
Don't detach the httpd->readcallback from the isc_httpdmgr_shutdown()
and the send callback, but rather call isc_nm_cancelread() to shutdown
the http request from the read callback.
The various factors like NS_PER_MS are now defined in a single place
and the names are no longer inconsistent. I chose the _PER_SEC names
rather than _PER_S because it is slightly more clear in isolation;
but the smaller units are always NS, US, and MS.
(cherry picked from commit 00307fe318)
Firefox 90+ apparently sends more than 10 headers, so we need to bump
the number to some higher number. Bump it to 100 just to be on a save
side, this is for internal use only anyway.
(cherry picked from commit e4654d1a6a)
The statschannel truncated test still terminates abruptly sometimes and
it doesn't return the answer for the first query. This might happen
when the second process_request() discovers there's not enough space
before the sending is complete and the connection is terminated before
the client gets the data.
Change the isc_http, so it pauses the reading when it receives the data
and resumes it only after the sending has completed or there's
incomplete request waiting for more data.
This makes the request processing slightly less efficient, but also less
taxing for the server, because previously all requests that has been
received via single TCP read would be processed in the loop and the
sends would be queued after the read callback has processed a full
buffer.
(cherry picked from commit 13959781cb)
Rewrite the isc_httpd to be more robust.
1. Replace the hand-crafted HTTP request parser with picohttpparser for
parsing the whole HTTP/1.0 and HTTP/1.1 requests. Limit the number
of allowed headers to 10 (arbitrary number).
2. Replace the hand-crafted URL parser with isc_url_parse for parsing
the URL from the HTTP request.
3. Increase the receive buffer to match the isc_netmgr buffers, so we
can at least receive two full isc_nm_read()s. This makes the
truncation processing much simpler.
4. Process the received buffer from single isc_nm_read() in a single
loop and schedule the sends to be independent of each other.
The first two changes makes the code simpler and rely on already
existing libraries that we already had (isc_url based on nodejs) or are
used elsewhere (picohttpparser).
The second two changes remove the artificial "truncation" limit on
parsing multiple request. Now only a request that has too many
headers (currently 10) or is too big (so, the receive buffer fills up
without reaching end of the request) will end the connection.
We can be benevolent here with the limites, because the statschannel
channel is by definition private and access must be allowed only to
administrators of the server. There are no timers, no rate-limiting, no
upper limit on the number of requests that can be served, etc.
(cherry picked from commit beecde7120)
PicoHTTPParser is a tiny, primitive, fast HTTP request/response parser.
Unlike most parsers, it is stateless and does not allocate memory by
itself. All it does is accept pointer to buffer and the output
structure, and setups the pointers in the latter to point at the
necessary portions of the buffer.
(cherry picked from commit 3a8884f024)
It was possible that accept callback can be called after listener
shutdown. In such a case the callback pointer equals NULL, leading to
segmentation fault. This commit fixes that.
This commit introduces a primitive isc__nmsocket_stop() which performs
shutting down on a multilayered socket ensuring the proper order of
the operations.
The shared data within the socket object can be destroyed after the
call completed, as it is guaranteed to not be used from within the
context of other worker threads.
(cherry picked from commit 5ab2c0ebb3)
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)
I.e. print the name of the function in BIND that called the system
function that returned an error. Since it was useful for pthreads
code, it seems worthwhile doing so everywhere.
(cherry picked from commit 26ed03a61e)
Mostly generated automatically with the following semantic patch,
except where coccinelle was confused by #ifdef in lib/isc/net.c
@@ expression list args; @@
- UNEXPECTED_ERROR(__FILE__, __LINE__, args)
+ UNEXPECTED_ERROR(args)
@@ expression list args; @@
- FATAL_ERROR(__FILE__, __LINE__, args)
+ FATAL_ERROR(args)
(cherry picked from commit ec50c58f52)
This commit fixes TLS DNS verification error message reporting which
we probably broke during one of the recent networking code
refactorings.
This prevent e.g. dig from producing useful error messages related to
TLS certificates verification.
Ensure that TLS error is empty before calling SSL_get_error() or doing
SSL I/O so that the result will not get affected by prior error
statuses.
In particular, the improper error handling led to intermittent unit
test failure and, thus, could be responsible for some of the system
test failures and other intermittent TLS-related issues.
See here for more details:
https://www.openssl.org/docs/man3.0/man3/SSL_get_error.html
In particular, it mentions the following:
> The current thread's error queue must be empty before the TLS/SSL
> I/O operation is attempted, or SSL_get_error() will not work
> reliably.
As we use the result of SSL_get_error() to decide on I/O operations,
we need to ensure that it works reliably by cleaning the error queue.
TLS DNS: empty error queue before attempting I/O
previously, when ISC_BUFFER_USEINLINE was defined, macros were
used to implement isc_buffer primitives (isc_buffer_init(),
isc_buffer_region(), etc). these macros were missing the DbC
assertions for those primitives, which made it possible for
coding errors to go undetected.
adding the assertions to the macros caused compiler warnings on
some platforms. therefore, this commit converts the ISC__BUFFER
macros to static inline functions instead, with assertions included,
and eliminates the non-inline implementation from buffer.c.
the --enable-buffer-useinline configure option has been removed.
(cherry picked from commit 1926ddc987)
- use isc_buffer functions when appropriate, rather than converting
to and from isc_region unnecessarily
- use the zlib total_out value instead of calculating it
- use c99 struct initialization
(cherry picked from commit 4b7248545e)
