Fix stack-use-after-scope in dns_message_checksig test
Previously stack with buffer for test dns message went out of scope
before the message was processed. For fuzz testing its better to avoid
allocation, so let's avoid allocations completely and use simplest
possible static buffer.
Evan Hunt [Wed, 17 Aug 2022 20:39:38 +0000 (13:39 -0700)]
change ISC__BUFFER macros to inline functions
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.
Petr Menšík [Mon, 26 Sep 2022 11:52:20 +0000 (13:52 +0200)]
Simplify allowing warnings during ARM build
RHEL8 Sphinx does not support all features used in ARM building. But
with few emitted warnings it can build the documentation fine. Simplify
warnings acceptance by allowing make doc SPHINX_W=''.
Petr Menšík [Sat, 24 Sep 2022 18:22:20 +0000 (14:22 -0400)]
Compatibility for building ARM on older sphinx
Make documentation building successful even on RHEL9 sphinx 3.4.3. It
does not like case-insensitive matching of terms, so provide lowercase
text description with Uppercase word reference.
Ondřej Surý [Wed, 2 Mar 2022 10:48:26 +0000 (11:48 +0100)]
Add dns_message_checksig() fuzzer
dns_message_checksig is called in a number of scenarios
* on requests and responses
* on multiple opcodes
* with and without signatures
* with TSIG signatures
* with SIG(0) signatures
* with and without configured TSIG keys
* with and without KEY records being present
* signing performed now, in the future and in the past
we use the first two octets of the seed to configure the calling
environment with the remainder of the seed being the rdata of the
TSIG/SIG(0) record.
Ondřej Surý [Wed, 2 Mar 2022 10:48:26 +0000 (11:48 +0100)]
Add the ability specify the signing / verification time
When fuzzing it is useful for all signing operations to happen
at a specific time for reproducability. Add two variables to
the message structure (fuzzing and fuzztime) to specify if a
fixed time should be used and the value of that time.
Switch the CSPRNG function from RAND_bytes() to uv_random()
The RAND_bytes() implementation differs between the OpenSSL versions and
uses the system entropy only for seeding its internal CSPRNG. The
uv_random() on the other hand uses the system provided CSPRNG.
Switch from RAND_bytes() to uv_random() to use system provided CSPRNG.
Mark Andrews [Wed, 21 Sep 2022 01:54:48 +0000 (11:54 +1000)]
Stop passing mctx to dns_rdata_tostruct as it is unnecessary for SIG
dns_rdata_tostruct doesn't need a mctx passed to it for SIG (the signer
is already expanded at this point). About the only time when mctx is
needed is when the structure is to be used after the rdata has been
destroyed.
The comments in CA.cfg file serve as a good tutorial for setting up
a simple PKI for a system test. There is a typo in one of the presented
commands, which results in openssl not exiting with an error message
instead of generating a certificate.
Petr Menšík [Thu, 8 Sep 2022 14:33:38 +0000 (16:33 +0200)]
Remove engine related parts for OpenSSL 3.0
OpenSSL just cannot work with mixing ENGINE_* api mixed with OSSL_PARAM
builders. But it can be built in legacy mode, where deprecated but still
working API would be used.
It can work under OpenSSL 3.0, but only if using legacy code paths
matching OpenSSL 1.1 calls and functions.
Remove fromlabel processing by OpenSSL 3.0 only functions. They can
return later with a proper provider support for pkcs11.
Petr Menšík [Thu, 8 Sep 2022 15:19:20 +0000 (17:19 +0200)]
Do not use OSSL_PARAM when engine API is compiled
OpenSSL has deprecated many things in version 3.0. If pkcs11 engine
should work then no builder from OpenSSL 3.0 API can be used.
Allow switching to OpenSSL 1.1 like calls even on OpenSSL 3.0 when
OPENSSL_API_COMPAT=10100 is defined. It would still compile and allow
working keys loading from the engine passed on command line.
Ondřej Surý [Mon, 29 Aug 2022 08:55:10 +0000 (10:55 +0200)]
Cleanup the asychronous code in the stream implementations
After the loopmgr work has been merged, we can now cleanup the TCP and
TLS protocols a little bit, because there are stronger guarantees that
the sockets will be kept on the respective loops/threads. We only need
asynchronous call for listening sockets (start, stop) and reading from
the TCP (because the isc_nm_read() might be called from read callback
again.
This commit does the following changes (they are intertwined together):
1. Cleanup most of the asynchronous events in the TCP code, and add
comments for the events that needs to be kept asynchronous.
2. Remove isc_nm_resumeread() from the netmgr API, and replace
isc_nm_resumeread() calls with existing isc_nm_read() calls.
3. Remove isc_nm_pauseread() from the netmgr API, and replace
isc_nm_pauseread() calls with a new isc_nm_read_stop() call.
4. Disable the isc_nm_cancelread() for the streaming protocols, only the
datagram-like protocols can use isc_nm_cancelread().
5. Add isc_nmhandle_close() that can be used to shutdown the socket
earlier than after the last detach. Formerly, the socket would be
closed only after all reading and sending would be finished and the
last reference would be detached. The new isc_nmhandle_close() can
be used to close the underlying socket earlier, so all the other
asynchronous calls would call their respective callbacks immediately.
Michał Kępień [Thu, 22 Sep 2022 12:03:17 +0000 (14:03 +0200)]
Ensure required cached glue is rendered
When looking for required glue, dns_message_rendersection() only
processes the first rdataset associated with the first name added to the
ADDITIONAL section. If the DNS_RDATASETATTR_REQUIRED attribute is set
for an rdataset which is located somewhere else (i.e. the name it is
associated with is preceded by another name in the ADDITIONAL section),
it will not be honored, i.e. the TC bit will not be set even if the
rdataset does not fit into the response.
Check the attributes of each processed rdataset while appending names to
a referral response based on a glue cache entry. If a given rdataset is
marked with DNS_RDATASETATTR_REQUIRED, make sure the name it is
associated with is added to the response at the beginning of the
ADDITIONAL section, not its end.
Note that using ISC_LIST_PREPEND() instead of ISC_LIST_APPEND() is not
necessary when associating the rdataset with its owner name because the
dns_name_t structures are initialized just before the glue rdatasets are
associated with them and therefore they are empty at that point, which
means no other (non-required) rdataset can precede the glue rdatasets
within the dns_name_t structure owning them.
Michał Kępień [Thu, 22 Sep 2022 12:03:17 +0000 (14:03 +0200)]
Mark required glue during glue cache processing
If an NS RRset at the parent side of a delegation point only contains
in-bailiwick NS records, at least one glue record should be included in
every referral response sent for such a delegation point or else clients
will need to send follow-up queries in order to determine name server
addresses. In certain edge cases (when the total size of a referral
response without glue records was just below to the UDP packet size
limit), named failed to adhere to that rule by sending non-truncated,
glueless referral responses.
Fix the problem by marking all in-bailiwick NS records processed by
glue_nsdname_cb() (the dns_rdataset_additionaldata() callback used by
RBTDB code while iterating over an NS RRset when dns_rdataset_addglue()
is called) with the DNS_RDATASETATTR_REQUIRED flag. Note that for
simplicity, glue RRsets for all in-bailiwick NS records are marked this
way, even though dns_message_rendersection() only checks the attributes
for the first rdataset associated with the first name added to the
ADDITIONAL section.
Michał Kępień [Thu, 22 Sep 2022 12:03:17 +0000 (14:03 +0200)]
Add tests for broken glueless referrals
If an NS RRset at the parent side of a delegation point only contains
in-bailiwick NS records, at least one glue record should be included in
every referral response sent for such a delegation point or else clients
will need to send follow-up queries in order to determine name server
addresses. In certain edge cases (when the total size of a referral
response without glue records was just below to the UDP packet size
limit), named failed to adhere to that rule by sending non-truncated,
glueless referral responses.
Add tests attempting to trigger that bug in several different scenarios,
covering all possible combinations of the following factors:
- type of zone (signed, unsigned),
- glue record type (A, AAAA, both).
Michał Kępień [Thu, 22 Sep 2022 12:03:17 +0000 (14:03 +0200)]
Clean up the "glue" system test
Bring the "glue" system test up to speed with other system tests: add
check numbering, ensure test artifacts are preserved upon failure,
improve error reporting, make the test fail upon unexpected errors,
address ShellCheck warnings.
Tony Finch [Wed, 21 Sep 2022 16:44:48 +0000 (17:44 +0100)]
A more helpful error when --without-jemalloc is impossible
When jemalloc is the system allocator (on FreeBSD and NetBSD), trying
to build --without-jemalloc caused an obscure compiler error. Instead,
complain at configure time that --without-jemalloc cannot work. (It
needs to remain an error because it is vexing when configure quietly
ignores an explicit direction.)
Instead of expecting that telemetry check has already finished,
wait for it for maximum of three seconds, because named is run with
-tat=3, so the telemetry check must happen with 3 second window.
Require isc_timer to be manipulated on the timer loop
Each isc_timer needs to be created, started and destroyed on the current
loop. The isc_timer_stop() can be run on any loop, but when run from
different loop than the one associated with the timer, the request to
stop the timer will be recorded in atomic variable and the underlying
uv_timer_t will be stopped on next uv_timer_t callback call. This
allows any thread to stop the timer.
Create the negative trust anchor timer on the current loop
Instead of always creating the trust anchor timer (dns_nta_t) on the
main loop, create the timer on the current loop and associate each
dns_nta_t object to the loop it was created on. This simplifies the
timer handling as everything is run on the associated loop.
During the change, the dns_nta_t structure was renamed to dns__nta_t
and changed to be fully internal to the nta.c compilation unit, and the
dns_ntatable_t structure was made opaque. This required no change to
code using the API as dns_nta_t never had any external users and the
dns_ntatable_t was properly accessed only by using function calls.
Create the response policy zones update timer on demand
Instead of creating the response policy zone deferred update timer when
creating the response policy zone object, create it on demand on the
current loop and destroy it as soon as the timer has finished its job.
There's a side-effect - the processing of the response policy zone
update is now done on the current loop - previously, it was always on
the main loop.
Instead of creating the catalog zone deferred update timer when creating
the catalog zone object, create it on demand on the current loop and
destroy it as soon as the timer has finished its job. There's a
side-effect - the processing of the catalog zone update is now done on
the current loop - previously, it was always on the main loop.
Create the spillattimer when needed and destroy it early
Instead of creating dns_resolver .spillattimer when the dns_resolver_t
object is created, create it on the current loop as needed and destroy
it as soon as the timer has finished its job. This avoids the need to
manipulate the timer from a different thread.
Instead of creating the zone timers at the zone creation time (which
could be any thread), create the zone timer from the isc_loop that has
beena assigned to the zone (zone->loop);
In preparation for the on-loop timers, the isc_ratelimiter API was
converted to use the timer on main loop and start and stop the timer
asynchronously on the main loop.
As it sometimes happens that the object using isc_timer_t is destroyed
via detaching all the references with no guarantee that the last thread
will be matching thread, add a helper isc_timer_async_destroy() function
that stops the timer and runs the destroy function via isc_async_run()
on the matching thread.
Evan Hunt [Tue, 16 Aug 2022 23:26:02 +0000 (16:26 -0700)]
additional code cleanups in httpd.c
- 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
Tony Finch [Wed, 21 Sep 2022 11:21:32 +0000 (12:21 +0100)]
Ensure the first random number is non-zero when fuzzing
In fuzzing mode, `isc_random` uses a fixed seed for reproducibility.
The particular seed chosen happened to produce zero as its first
number, however commit bd251de0 introduced an initialization check in
`random_test` that required it to be non-zero. This change adjusts the
seed to avoid spurious test failures.
Also, remove the temporary variable that was used for initialization
because it did not match the type of the thread-local seed array.
Implement TLS transport support for dns_request and dns_dispatch
This change prepares ground for sending DNS requests using DoT,
which, in particular, will be used for forwarding dynamic updates
to TLS-enabled primaries.
Convert xfrin.c:get_create_tlsctx() into a library function
In order to make xfrin.c:get_create_tlsctx() reusable, move the function
into transport.c, and make changes into its prototype to not use the
'dns_xfrin_ctx_t' type, thus making it more universal.
This change prepares ground for adding transport support into the
dispatch manager.
Also, move the typedefs for 'dns_transport_t' and 'dns_transport_list_t'
from transport.h into types.h.
Tony Finch [Fri, 22 Apr 2022 13:35:36 +0000 (14:35 +0100)]
Move random number re-seeding out of the hot path
Instead of checking if we need to re-seed for every isc_random call,
seed the random number generator in the libisc global initializer
and the per-thread initializer.
Disable stringop-overread with gcc-11+ Address Sanitizer
When Address Sanitizer is enabled in gcc-11+, number of false positives
might appear like this:
netmgr/udp.c: In function 'isc__nm_udp_send':
netmgr/udp.c:729:13: warning: 'uv_udp_send' reading 16 bytes from a region of size 8 [-Wstringop-overread]
729 | r = uv_udp_send(&uvreq->uv_req.udp_send, &sock->uv_handle.udp,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
730 | &uvreq->uvbuf, 1, sa, udp_send_cb);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
netmgr/udp.c:729:13: note: referencing argument 3 of type 'const uv_buf_t[0]'
In file included from ./include/isc/uv.h:17,
from ./include/isc/barrier.h:31,
from netmgr/udp.c:17:
/usr/include/uv.h:711:15: note: in a call to function 'uv_udp_send'
711 | UV_EXTERN int uv_udp_send(uv_udp_send_t* req,
| ^~~~~~~~~~~
Disable the warning globally in the autoconf, instead of just locally in
a single CI job, as it might affect people outside our GitLab CI.
Ondřej Surý [Mon, 29 Aug 2022 11:42:14 +0000 (13:42 +0200)]
Add missing isc_refcount_destroy() for isc__nmsocket_t
The destructor for the isc__nmsocket_t was missing call to the
isc_refcount_destroy() on the reference counter, which might lead to
spurious ThreadSanitizer data race warnings if we ever change the
acquire-release memory order in the isc_refcount_decrement().
Ondřej Surý [Mon, 29 Aug 2022 10:11:37 +0000 (12:11 +0200)]
Reorder the uv_close() calls to close the socket immediately
Simplify the closing code - during the loopmgr implementation, it was
discovered that the various lists used by the uv_loop_t aren't FIFO, but
LIFO. See doc/dev/libuv.md for more details.
With this knowledge, we can close the protocol handles (uv_udp_t and
uv_tcp_t) and uv_timer_t at the same time by reordering the uv_close()
calls, and thus making sure that after calling the
isc__nm_stoplistening(), the code will not issue any additional callback
calls (accept, read) on the socket that stopped listening.
This might help with the TLS and DoH shutting down sequence as described
in the [GL #3509] as we now stop the reading, stop the timer and call
the uv_close() as earliest as possible.
Improve the udp_shutdown_read and udp_cancel_read tests
In the udp_shutdown_read unit test, delay the isc_loopmgr_shutdown() to
the send callback, and in the udp_cancel_read test wait for a single
timed out test, then read again, send an UDP packet and cancel the read
from the send callback.
The network manager UDP code was misinterpreting when the libuv called
the udp_recv_cb with nrecv == 0 and addr == NULL -> this doesn't really
mean that the "stream" has ended, but the libuv indicates that the
receive buffer can be freed. This could lead to assertion failure in
the code that calls isc_nm_read() from the network manager read callback
due to the extra spurious callbacks.
Properly handle the extra callback calls from the libuv in the client
read callback, and refactor the UDP isc_nm_read() implementation to be
synchronous, so no datagram is lost between the time that we stop the
reading from the UDP socket and we restart it again in the asychronous
udpread event.
Add a unit test that tests the isc_nm_read() call from the read
callback to receive two datagrams.
projects / thirdparty / bind9.git / log
