Aram Sargsyan [Fri, 25 Feb 2022 08:41:36 +0000 (08:41 +0000)]
Remove EVP_CIPHER_CTX_new() and EVP_CIPHER_CTX_free() shims
LibreSSL 3.5.0 fails to compile with these shims. We could have just
removed the LibreSSL check from the pre-processor condition, but it
seems that these shims are no longer needed because all the supported
versions of OpenSSL and LibreSSL have those functions.
According to EVP_ENCRYPTINIT(3) manual page in LibreSSL,
EVP_CIPHER_CTX_new() and EVP_CIPHER_CTX_free() first appeared in
OpenSSL 0.9.8b, and have been available since OpenBSD 4.5.
Evan Hunt [Thu, 24 Feb 2022 01:36:11 +0000 (17:36 -0800)]
document zone grammar more correctly
the "zone" clause can be documented using, for instance,
`cfg_test --zonegrammar primary", which prints only
options that are valid in primary zones. this was not
the method being used when generating the named.conf
man page; instead, "zone" was documented with all possible
options, and no zone types at all.
this commit removes "zone" from the generic documentation
and adds include statements in named.conf.rst so that
correct zone grammars will be included in the man page.
Mark Andrews [Mon, 28 Feb 2022 00:47:56 +0000 (11:47 +1100)]
Grow the lex token buffer in one more place
when parsing key pairs, if the '=' character fell at max_token
a protective INSIST preventing buffer overrun could be triggered.
Attempt to grow the buffer immediately before the INSIST.
Also removed an unnecessary INSIST on the opening double quote
of key buffer pair.
Mark Andrews [Mon, 28 Feb 2022 02:43:20 +0000 (13:43 +1100)]
Use unsigned arithmetic when shifting by 24
By default C promotes short unsigned values to signed int which
leads to undefined behaviour when the value is shifted by too much.
Force unsigned arithmetic to be perform by explicitly casting to a
unsigned type.
Ondřej Surý [Mon, 28 Feb 2022 09:25:06 +0000 (10:25 +0100)]
Handle TCP sockets in isc__nmsocket_reset()
The isc__nmsocket_reset() was missing a case for raw TCP sockets (used
by RNDC and DoH) which would case a assertion failure when write timeout
would be triggered.
TCP sockets are now also properly handled in isc__nmsocket_reset().
Evan Hunt [Mon, 14 Feb 2022 21:05:03 +0000 (13:05 -0800)]
mem_maybedup() can no longer fail
mem_maybedup() calls isc_mem_allocate() if an mctx is supplied,
but that can no longer fail, so now the only way mem_maybedup()
could return NULL is if it was given a NULL source address by the
caller. this commit adds a REQUIRE to prevent that scenario, and
cleans up all the calling code that previously checked for NULL
return values.
this function is mostly used in rdata tostruct() implementations, so
the documentation for dns_rdata_tostruct() has been updated to
remove 'ISC_R_NOMEMORY' as a possible return value.
Evan Hunt [Sat, 26 Feb 2022 00:33:30 +0000 (16:33 -0800)]
add a CFG_CLAUSEFLAG_NODOC flag for use with outdated terms
"masters" and "default-masters" are now flagged so they will
not be included in the named.conf man page, despite being
accepted as valid options by the parser for backward
compatibiility.
Petr Špaček [Thu, 18 Feb 2021 20:29:33 +0000 (21:29 +0100)]
Add dns_rdata_fromtext() fuzzer
... along with dns_rdataclass_fromtext and dns_rdatatype_fromtext
Most of the test binary is modified named-rrchecker. Main differences:
- reads single RR and exists
- does not refuse meta classes and rr types
We actually do have some fromtext code for meta-things so erroring out
in named-rrchecker would prevent us from testing this code.
Corpus has examples of all currently supported RR types. I did not do
any minimization.
Ondřej Surý [Wed, 23 Feb 2022 23:14:26 +0000 (00:14 +0100)]
Disable inactive uvreqs caching when compiled with sanitizers
When isc__nm_uvreq_t gets deactivated, it could be just put onto array
stack to be reused later to save some initialization time.
Unfortunately, this might hide some use-after-free errors.
Disable the inactive uvreqs caching when compiled with Address or
Thread Sanitizer.
Ondřej Surý [Wed, 23 Feb 2022 08:12:51 +0000 (09:12 +0100)]
Disable inactive handles caching when compiled with sanitizers
When isc_nmhandle_t gets deactivated, it could be just put onto array
stack to be reused later to safe some initialization time.
Unfortunately, this might hide some use-after-free errors.
Disable the inactive handles caching when compiled with Address or
Thread Sanitizer.
Ondřej Surý [Wed, 23 Feb 2022 07:54:49 +0000 (08:54 +0100)]
Remove active handles tracking from isc__nmsocket_t
The isc__nmsocket_t has locked array of isc_nmhandle_t that's not used
for anything. The isc__nmhandle_get() adds the isc_nmhandle_t to the
locked array (and resized if necessary) and removed when
isc_nmhandle_put() finally destroys the handle. That's all it does, so
it serves no useful purpose.
Remove the .ah_handles, .ah_size, and .ah_frees members of the
isc__nmsocket_t and .ah_pos member of the isc_nmhandle_t struct.
Ondřej Surý [Tue, 22 Feb 2022 17:12:18 +0000 (18:12 +0100)]
Delay isc__nm_uvreq_t deallocation to connection callback
When the TCP, TCPDNS or TLSDNS connection times out, the isc__nm_uvreq_t
would be pushed into sock->inactivereqs before the uv_tcp_connect()
callback finishes. Because the isc__nmsocket_t keeps the list of
inactive isc__nm_uvreq_t, this would cause use-after-free only when the
sock->inactivereqs is full (which could never happen because the failure
happens in connection timeout callback) or when the sock->inactivereqs
mechanism is completely removed (f.e. when running under Address or
Thread Sanitizer).
Delay isc__nm_uvreq_t deallocation to the connection callback and only
signal the connection callback should be called by shutting down the
libuv socket from the connection timeout callback.
Michał Kępień [Wed, 23 Feb 2022 13:39:11 +0000 (14:39 +0100)]
Fix more ns_statscounter_recursclients underflows
Commit aab691d51266f552a7923db32686fb9398b1d255 did not fix all possible
scenarios in which the ns_statscounter_recursclients counter underflows.
The solution implemented therein can be ineffective e.g. when CNAME
chaining happens with prefetching enabled.
Here is an example recursive resolution scenario in which the
ns_statscounter_recursclients counter can underflow with the current
logic in effect:
1. Query processing starts, the answer is not found in the cache, so
recursion is started. The NS_CLIENTATTR_RECURSING attribute is set.
ns_statscounter_recursclients is incremented (Δ = +1).
2. Recursion completes, returning a CNAME. client->recursionquota is
non-NULL, so the NS_CLIENTATTR_RECURSING attribute remains set.
ns_statscounter_recursclients is decremented (Δ = 0).
3. Query processing restarts.
4. The current QNAME (the target of the CNAME from step 2) is found in
the cache, with a TTL low enough to trigger a prefetch.
5. query_prefetch() attaches to client->recursionquota.
ns_statscounter_recursclients is not incremented because
query_prefetch() does not do that (Δ = 0).
6. Query processing restarts.
7. The current QNAME (the target of the CNAME from step 4) is not found
in the cache, so recursion is started. client->recursionquota is
already attached to (since step 5) and the NS_CLIENTATTR_RECURSING
attribute is set (since step 1), so ns_statscounter_recursclients is
not incremented (Δ = 0).
8. The prefetch from step 5 completes. client->recursionquota is
detached from in prefetch_done(). ns_statscounter_recursclients is
not decremented because prefetch_done() does not do that (Δ = 0).
9. Recursion for the current QNAME completes. client->recursionquota
is already detached from, i.e. set to NULL (since step 8), and the
NS_CLIENTATTR_RECURSING attribute is set (since step 1), so
ns_statscounter_recursclients is decremented (Δ = -1).
Another possible scenario is that after step 7, recursion for the target
of the CNAME from step 4 completes before the prefetch for the CNAME
itself. fetch_callback() then notices that client->recursionquota is
non-NULL and decrements ns_statscounter_recursclients, even though
client->recursionquota was attached to by query_prefetch() and therefore
not accompanied by an incrementation of ns_statscounter_recursclients.
The net result is also an underflow.
Instead of trying to properly handle all possible orderings of events
set into motion by normal recursion and prefetch-triggered recursion,
adjust ns_statscounter_recursclients whenever the recursive clients
quota is successfully attached to or detached from. Remove the
NS_CLIENTATTR_RECURSING attribute altogether as its only purpose is made
obsolete by this change.
Michał Kępień [Mon, 21 Feb 2022 09:50:38 +0000 (10:50 +0100)]
Add "UNUSED(fctx);" to FCTXTRACE*() macro stubs
Commit b6d40b3c4e4452cee6f7f06d889e4e8d113e9588 removed most uses of the
'fctx' variable from the rctx_dispfail() function: it is now only needed
by the FCTXTRACE3() macro. However, when --enable-querytrace is not in
effect, that macro evaluates to a list of UNUSED() macros that does not
include "UNUSED(fctx);". This triggers the following compilation
warning when building without --enable-querytrace:
resolver.c: In function 'rctx_dispfail':
resolver.c:7888:21: warning: unused variable 'fctx' [-Wunused-variable]
7888 | fetchctx_t *fctx = rctx->fctx;
| ^~~~
Fix by adding "UNUSED(fctx);" lines to all FCTXTRACE*() macros. This is
safe to do because all of those macros use the 'fctx' local variable, so
there is no danger of introducing new errors caused by use of undeclared
identifiers.
Ondřej Surý [Thu, 27 Jan 2022 11:34:15 +0000 (12:34 +0100)]
Downgrade -Wstringop-overread from error to warning
Due to a bug in gcc-11, the build fails when AddressSanitizer is
enabled. Downgrading the -Wstringop-overread to just a warning in the
gcc:asan build allows the code to compile.
Ondřej Surý [Thu, 27 Jan 2022 09:09:07 +0000 (10:09 +0100)]
Remove the keep-response-order ACL map
The keep-response-order option has been obsoleted, and in this commit,
remove the keep-response-order ACL map rendering the option no-op, the
call the isc_nm_sequential() and the now unused isc_nm_sequential()
function itself.
Ondřej Surý [Thu, 27 Jan 2022 09:13:10 +0000 (10:13 +0100)]
Declare the keep-response-order obsolete
The keep-response-order option has been introduced when TCP pipelining
has been introduced to BIND 9 as a failsafe for possibly non-compliant
clients.
Declare the keep-response-order obsolete as all DNS clients should
either support out-of-order processing or don't send more DNS queries
until the DNS response for the previous one has been received.
Ondřej Surý [Thu, 27 Jan 2022 09:08:06 +0000 (10:08 +0100)]
Remove the limit on the number of simultaneous TCP queries
There was an artificial limit of 23 on the number of simultaneous
pipelined queries in the single TCP connection. The new network
managers is capable of handling "unlimited" (limited only by the TCP
read buffer size ) queries similar to "unlimited" handling of the DNS
queries receive over UDP.
Don't limit the number of TCP queries that we can process within a
single TCP read callback.
Ondřej Surý [Tue, 8 Feb 2022 20:32:10 +0000 (21:32 +0100)]
Add XFR max-transfer-time-out and max-tranfer-idle-out system tests
Extend the timeouts system test to ensure that the maximum outgoing
transfer time (max-transfer-time-out) and maximum outgoing transfer idle
time (max-transfer-idle-out) works as expected. This is done by
lowering the limits to 5/1 minutes and testing that the connection has
been dropped while sleeping between the individual XFR messages.
Evan Hunt [Sat, 5 Feb 2022 00:52:22 +0000 (16:52 -0800)]
support $INCLUDE in makejournal
bin/tests/system/makejournal needs to ignore DNS_R_SEENINCLUDE
when calling dns_db_load(), otherwise it cannot generate a journal
for a zone file with a $INCLUDE statement.
Ondřej Surý [Tue, 15 Feb 2022 13:41:15 +0000 (14:41 +0100)]
Reset the TCP connection when garbage is received
When invalid DNS message is received, there was a handling mechanism for
DoH that would be called to return proper HTTP response.
Reuse this mechanism and reset the TCP connection when the client is
blackholed, DNS message is completely bogus or the ns_client receives
response instead of query.
Evan Hunt [Wed, 9 Feb 2022 22:56:04 +0000 (14:56 -0800)]
correct TCP error handling in dispatch and resolver
- certain TCP result codes, including ISC_R_EOF and
ISC_R_CONNECTIONRESET, were being mapped to ISC_R_SHUTTINGDOWN
before calling the response handler in tcp_recv_cancelall().
the result codes should be passed through to the response handler
without being changed.
- the response handlers, resquery_response() and req_response(), had
code to return immediately if encountering ISC_R_EOF, but this is
not the correct behavior; that should only happen in the case of
ISC_R_CANCELED when it was the caller that canceled the operation
- ISC_R_CONNECTIONRESET was not being caught in rctx_dispfail().
- removed code in rctx_dispfail() to retry queries without EDNS
when receiving ISC_R_EOF; this is now treated the same as any
other connection failure.
Ondřej Surý [Thu, 10 Feb 2022 07:42:22 +0000 (08:42 +0100)]
Update writetimeout to be T_IDLE in netmgr_test.c
Use the isc_nmhandle_setwritetimeout() function in the netmgr unit test
to allow more time for writing and reading the responses because some of
the intervals that are used in the unit tests are really small leaving a
little room for any delays.
Ondřej Surý [Wed, 9 Feb 2022 18:48:13 +0000 (19:48 +0100)]
Add isc_nmhandle_setwritetimeout() function
In some situations (unit test and forthcoming XFR timeouts MR), we need
to modify the write timeout independently of the read timeout. Add a
isc_nmhandle_setwritetimeout() function that could be called before
isc_nm_send() to specify a custom write timeout interval.
Ondřej Surý [Wed, 9 Feb 2022 11:46:29 +0000 (12:46 +0100)]
Add TCP write timeout system test
Extend the timeouts system test that bursts the queries for large TXT
record and never read any responses back filling up the server TCP write
buffer. The test should work with the default wmem_max value on
Linux (208k).
Ondřej Surý [Wed, 9 Feb 2022 10:21:04 +0000 (11:21 +0100)]
Add TCP, TCPDNS and TLSDNS write timer
When the outgoing TCP write buffers are full because the other party is
not reading the data, the uv_write() could wait indefinitely on the
uv_loop and never calling the callback. Add a new write timer that uses
the `tcp-idle-timeout` value to interrupt the TCP connection when we are
not able to send data for defined period of time.
Ondřej Surý [Wed, 9 Feb 2022 11:45:37 +0000 (12:45 +0100)]
Add uv_tcp_close_reset compat
The uv_tcp_close_reset() function was added in libuv 1.32.0 and since we
support older libuv releases, we have to add a shim uv_tcp_close_reset()
implementation loosely based on libuv.
Ondřej Surý [Wed, 9 Feb 2022 09:59:08 +0000 (10:59 +0100)]
Rename sock->timer to sock->read_timer
Before adding the write timer, we have to remove the generic sock->timer
to sock->read_timer. We don't touch the function names to limit the
impact of the refactoring.
Evan Hunt [Wed, 16 Feb 2022 23:46:33 +0000 (15:46 -0800)]
negative 'blackhole' ACL match could be treated as positive
There was a bug in the checking of the "blackhole" ACL in
dns_request_create*(), causing an address to be treated as included
in the ACL if it was explicitly *excluded*. Thus, leaving "blackhole"
unset had no effect, but setting it to "none" would cause any
destination addresses to be rejected for dns_request purposes. This
would cause zone transfer requests and SOA queries to fail, among
other things.
The bug has been fixed, and "blackhole { none; };" was added to the
xfer system test as a regression test.
Ondřej Surý [Tue, 15 Feb 2022 13:44:29 +0000 (14:44 +0100)]
Add UV_RUNTIME_CHECK() macro to print uv_strerror()
When libuv functions fail, they return correct return value that could
be useful for more detailed debugging. Currently, we usually just check
whether the return value is 0 and invoke assertion error if it doesn't
throwing away the details why the call has failed. Unfortunately, this
often happen on more exotic platforms.
Add a UV_RUNTIME_CHECK() macro that can be used to print more detailed
error message (via uv_strerror() before ending the execution of the
program abruptly with the assertion.