Ondřej Surý [Thu, 3 Dec 2020 16:58:10 +0000 (17:58 +0100)]
Use sock->nchildren instead of mgr->nworkers when initializing NM
On Windows, we were limiting the number of listening children to just 1,
but we were then iterating on mgr->nworkers. That lead to scheduling
more async_*listen() than actually allocated and out-of-bound read-write
operation on the heap.
Ondřej Surý [Thu, 3 Dec 2020 10:24:57 +0000 (11:24 +0100)]
Configure the system-wide TCP connection timeout on OpenBSD
As there's no TCP connection timeout socket option that we can use, we
need to configure the TCP connection timeout system-wide in the CI, so
the netmgr unit tests doesn't cause assertion failure when there stuck
outgoing TCP connection waiting for 150 second timeout.
Ondřej Surý [Thu, 3 Dec 2020 12:00:33 +0000 (13:00 +0100)]
Fix datarace when UDP/TCP connect fails and we are in nmthread
When we were in nmthread, the isc__nm_async_<proto>connect() function
executes in the same thread as the isc__nm_<proto>connect() and on a
failure, it would block indefinitely because the failure branch was
setting sock->active to false before the condition around the wait had a
chance to skip the WAIT().
This also fixes the zero system test being stuck on FreeBSD 11, so we
re-enable the test in the commit.
Michał Kępień [Thu, 3 Dec 2020 11:48:43 +0000 (12:48 +0100)]
Temporarily disable the "legacy" test on Windows
The current issues with the way dig handles TCP "connection refused"
errors cause the "legacy" system test to consistently fail on Windows
due to the expected strings not being present in dig output.
Temporarily disable the "legacy" system test on Windows by moving it
from the PARALLEL_COMMON list to the PARALLEL_UNIX list until the
situation is rectified.
Ondřej Surý [Wed, 2 Dec 2020 14:37:18 +0000 (15:37 +0100)]
Distribute queries among threads even on platforms without lb sockets
On platforms without load-balancing socket all the queries would be
handle by a single thread. Currently, the support for load-balanced
sockets is present in Linux with SO_REUSEPORT and FreeBSD 12 with
SO_REUSEPORT_LB.
This commit adds workaround for such platforms that:
1. setups single shared listening socket for all listening nmthreads for
UDP, TCP and TCPDNS netmgr transports
2. Calls uv_udp_bind/uv_tcp_bind on the underlying socket just once and
for rest of the nmthreads only copy the internal libuv flags (should
be just UV_HANDLE_BOUND and optionally UV_HANDLE_IPV6).
3. start reading on UDP socket or listening on TCP socket
The load distribution among the nmthreads is uneven, but it's still
better than utilizing just one thread for processing all the incoming
queries
Ondřej Surý [Thu, 3 Dec 2020 07:33:21 +0000 (08:33 +0100)]
Don't use stack allocated buffer for uv_write()
On FreeBSD, the stack is destroyed more aggressively than on Linux and
that revealed a bug where we were allocating the 16-bit len for the
TCPDNS message on the stack and the buffer got garbled before the
uv_write() sendback was executed. Now, the len is part of the uvreq, so
we can safely pass it to the uv_write() as the req gets destroyed after
the sendcb is executed.
Michał Kępień [Wed, 2 Dec 2020 21:36:23 +0000 (22:36 +0100)]
Make netmgr initialize and cleanup Winsock itself
On Windows, WSAStartup() needs to be called to initialize Winsock before
any sockets are created or else socket() calls will return error code
10093 (WSANOTINITIALISED). Since BIND's Network Manager is intended to
work as a reusable networking library, it should take care of calling
WSAStartup() - and its cleanup counterpart, WSACleanup() - itself rather
than relying on external code to do it. Add the necessary WSAStartup()
and WSACleanup() calls to isc_nm_start() and isc_nm_destroy(),
respectively.
Michal Nowak [Wed, 2 Dec 2020 07:57:46 +0000 (08:57 +0100)]
Add uv_wrap.h to libisctest_la_SOURCES
uv_wrap.h is included in tcp_test.c and udp_test.c and therefore should
be listed in lib/isc/tests/Makefile.am, otherwise unit test run from
distribution tarball fails to compile:
tcp_test.c:37:10: fatal error: uv_wrap.h: No such file or directory
#include "uv_wrap.h"
^~~~~~~~~~~
udp_test.c:37:10: fatal error: uv_wrap.h: No such file or directory
#include "uv_wrap.h"
^~~~~~~~~~~
Ondřej Surý [Tue, 1 Dec 2020 17:53:26 +0000 (18:53 +0100)]
Change the default value for nocookie-udp-size back to 4096
The DNS Flag Day 2020 reduced all the EDNS buffer sizes to 1232. In
this commit, we revert the default value for nocookie-udp-size back to
4096 because the option is too obscure and most people don't realize
that they also need to change this configuration option in addition to
max-udp-size.
Ondřej Surý [Wed, 2 Dec 2020 07:54:51 +0000 (08:54 +0100)]
Avoid netievent allocations when the callbacks can be called directly
After turning the users callbacks to be asynchronous, there was a
visible performance drop. This commit prevents the unnecessary
allocations while keeping the code paths same for both asynchronous and
synchronous calls.
The same change was done to the isc__nm_udp_{read,send} as those two
functions are in the hot path.
Ondřej Surý [Tue, 1 Dec 2020 15:08:44 +0000 (16:08 +0100)]
Disable the new netmgr tests on non-Linux platforms
The new netmgr tests are not-yet fine-tuned for non-Linux platforms.
Disable them now, so we can move forward and fix the tests of *BSD
in the next iteration.
This commit will get reverted when we add support for netmgr
multi-threading.
Ondřej Surý [Tue, 1 Dec 2020 14:08:49 +0000 (15:08 +0100)]
The cmocka.h header MUST be included before isc/util.h gets included
The isc/util.h header redefine the DbC checks (REQUIRE, INSIST, ...) to
be cmocka "fake" assertions. However that means that cmocka.h needs to
be included after UNIT_TESTING is defined but before isc/util.h is
included. Because isc/util.h is included in most of the project headers
this means that the sequence MUST be:
#define UNIT_TESTING
#include <cmocka.h>
#include <isc/_anything_.h>
See !2204 for other header requirements for including cmocka.h.
Ondřej Surý [Thu, 12 Nov 2020 09:32:18 +0000 (10:32 +0100)]
Refactor netmgr and add more unit tests
This is a part of the works that intends to make the netmgr stable,
testable, maintainable and tested. It contains a numerous changes to
the netmgr code and unfortunately, it was not possible to split this
into smaller chunks as the work here needs to be committed as a complete
works.
NOTE: There's a quite a lot of duplicated code between udp.c, tcp.c and
tcpdns.c and it should be a subject to refactoring in the future.
The changes that are included in this commit are listed here
(extensively, but not exclusively):
* The netmgr_test unit test was split into individual tests (udp_test,
tcp_test, tcpdns_test and newly added tcp_quota_test)
* The udp_test and tcp_test has been extended to allow programatic
failures from the libuv API. Unfortunately, we can't use cmocka
mock() and will_return(), so we emulate the behaviour with #define and
including the netmgr/{udp,tcp}.c source file directly.
* The netievents that we put on the nm queue have variable number of
members, out of these the isc_nmsocket_t and isc_nmhandle_t always
needs to be attached before enqueueing the netievent_<foo> and
detached after we have called the isc_nm_async_<foo> to ensure that
the socket (handle) doesn't disappear between scheduling the event and
actually executing the event.
* Cancelling the in-flight TCP connection using libuv requires to call
uv_close() on the original uv_tcp_t handle which just breaks too many
assumptions we have in the netmgr code. Instead of using uv_timer for
TCP connection timeouts, we use platform specific socket option.
* Fix the synchronization between {nm,async}_{listentcp,tcpconnect}
When isc_nm_listentcp() or isc_nm_tcpconnect() is called it was
waiting for socket to either end up with error (that path was fine) or
to be listening or connected using condition variable and mutex.
Several things could happen:
0. everything is ok
1. the waiting thread would miss the SIGNAL() - because the enqueued
event would be processed faster than we could start WAIT()ing.
In case the operation would end up with error, it would be ok, as
the error variable would be unchanged.
2. the waiting thread miss the sock->{connected,listening} = `true`
would be set to `false` in the tcp_{listen,connect}close_cb() as
the connection would be so short lived that the socket would be
closed before we could even start WAIT()ing
* The tcpdns has been converted to using libuv directly. Previously,
the tcpdns protocol used tcp protocol from netmgr, this proved to be
very complicated to understand, fix and make changes to. The new
tcpdns protocol is modeled in a similar way how tcp netmgr protocol. Closes: #2194, #2283, #2318, #2266, #2034, #1920
* The tcp and tcpdns is now not using isc_uv_import/isc_uv_export to
pass accepted TCP sockets between netthreads, but instead (similar to
UDP) uses per netthread uv_loop listener. This greatly reduces the
complexity as the socket is always run in the associated nm and uv
loops, and we are also not touching the libuv internals.
There's an unfortunate side effect though, the new code requires
support for load-balanced sockets from the operating system for both
UDP and TCP (see #2137). If the operating system doesn't support the
load balanced sockets (either SO_REUSEPORT on Linux or SO_REUSEPORT_LB
on FreeBSD 12+), the number of netthreads is limited to 1.
* The netmgr has now two debugging #ifdefs:
1. Already existing NETMGR_TRACE prints any dangling nmsockets and
nmhandles before triggering assertion failure. This options would
reduce performance when enabled, but in theory, it could be enabled
on low-performance systems.
2. New NETMGR_TRACE_VERBOSE option has been added that enables
extensive netmgr logging that allows the software engineer to
precisely track any attach/detach operations on the nmsockets and
nmhandles. This is not suitable for any kind of production
machine, only for debugging.
* The tlsdns netmgr protocol has been split from the tcpdns and it still
uses the old method of stacking the netmgr boxes on top of each other.
We will have to refactor the tlsdns netmgr protocol to use the same
approach - build the stack using only libuv and openssl.
* Limit but not assert the tcp buffer size in tcp_alloc_cb Closes: #2061
Mark Andrews [Thu, 26 Nov 2020 04:59:14 +0000 (15:59 +1100)]
Adjust default value of "max-recursion-queries"
Since the queries sent towards root and TLD servers are now included in
the count (as a result of the fix for CVE-2020-8616),
"max-recursion-queries" has a higher chance of being exceeded by
non-attack queries. Increase its default value from 75 to 100.
Michal Nowak [Thu, 19 Nov 2020 11:33:27 +0000 (12:33 +0100)]
Drop bin/tests/headerdep_test.sh.in
The bin/tests/headerdep_test.sh script has not been updated since it was
first created and it cannot be used as-is with the current BIND source
code. Better tools (e.g. "include-what-you-use") emerged since the
script was committed back in 2000, so instead of trying to bring it up
to date, remove it from the source repository.
Michal Nowak [Thu, 19 Nov 2020 09:35:57 +0000 (10:35 +0100)]
Revise OPTIONS.md
- The STD_CDEFINES build-time variable was dropped when the build
system was migrated to Automake. CPPFLAGS is the variable which
should now be used for setting preprocessor macros.
- Sort the list of preprocessor macros which affect BIND behavior.
Remove ISC_BUFFER_USEINLINE from the list as it can be controlled
using its relevant ./configure option (--enable-buffer-useinline).
Rename NS_RUN_PID_DIR to NAMED_RUN_PID_DIR to match the source code.
Mark Andrews [Thu, 12 Nov 2020 22:45:47 +0000 (09:45 +1100)]
Tighten DNS COOKIE response handling
Fallback to TCP when we have already seen a DNS COOKIE response
from the given address and don't have one in this UDP response. This
could be a server that has turned off DNS COOKIE support, a
misconfigured anycast server with partial DNS COOKIE support, or a
spoofed response. Falling back to TCP is the correct behaviour in
all 3 cases.
Michal Nowak [Tue, 24 Nov 2020 16:39:23 +0000 (17:39 +0100)]
Write traceback file to the same directory as core file
The traceback files could overwrite each other on systems which do not
use different core dump file names for different processes. Prevent
that by writing the traceback file to the same directory as the core
dump file.
These changes still do not prevent the operating system from overwriting
a core dump file if the same binary crashes multiple times in the same
directory and core dump files are named identically for different
processes.
Diego Fronza [Wed, 25 Nov 2020 19:01:06 +0000 (16:01 -0300)]
Silence coverity warnings in query.c
Return value of dns_db_getservestalerefresh() and
dns_db_getservestalettl() functions were previously unhandled.
This commit purposefully ignore those return values since there is
no side effect if those results are != ISC_R_SUCCESS, it also supress
Coverity warnings.
Michał Kępień [Thu, 26 Nov 2020 12:10:40 +0000 (13:10 +0100)]
Use proper cmocka macros for pointer checks
Make sure pointer checks in unit tests use cmocka assertion macros
dedicated for use with pointers instead of those dedicated for use with
integers or booleans.
Matthijs Mekking [Fri, 13 Nov 2020 11:26:05 +0000 (12:26 +0100)]
Add NSEC3PARAM unit test, refactor zone.c
Add unit test to ensure the right NSEC3PARAM event is scheduled in
'dns_zone_setnsec3param()'. To avoid scheduling and managing actual
tasks, split up the 'dns_zone_setnsec3param()' function in two parts:
1. 'dns__zone_lookup_nsec3param()' that will check if the requested
NSEC3 parameters already exist, and if a new salt needs to be
generated.
2. The actual scheduling of the new NSEC3PARAM event (if needed).
When generating a new salt, compare it with the previous NSEC3
paremeters to ensure the new parameters are different from the
previous ones.
This moves the salt generation call from 'bin/named/*.s' to
'lib/dns/zone.c'. When setting new NSEC3 parameters, you can set a new
function parameter 'resalt' to enforce a new salt to be generated. A
new salt will also be generated if 'salt' is set to NULL.
Logging salt with zone context can now be done with 'dnssec_log',
removing the need for 'dns_nsec3_log_salt'.
Matthijs Mekking [Fri, 23 Oct 2020 13:02:19 +0000 (15:02 +0200)]
Change nsec3param salt config to saltlen
Upon request from Mark, change the configuration of salt to salt
length.
Introduce a new function 'dns_zone_checknsec3aram' that can be used
upon reconfiguration to check if the existing NSEC3 parameters are
in sync with the configuration. If a salt is used that matches the
configured salt length, don't change the NSEC3 parameters.
Matthijs Mekking [Tue, 13 Oct 2020 15:48:22 +0000 (17:48 +0200)]
Check nsec3param configuration values
Check 'nsec3param' configuration for the number of iterations. The
maximum number of iterations that are allowed are based on the key
size (see https://tools.ietf.org/html/rfc5155#section-10.3).
Check 'nsec3param' configuration for correct salt. If the string is
not "-" or hex-based, this is a bad salt.
Matthijs Mekking [Tue, 13 Oct 2020 12:52:02 +0000 (14:52 +0200)]
Don't use 'rndc signing' with kasp
The 'rndc signing' command allows you to manipulate the private
records that are used to store signing state. Don't use these with
'dnssec-policy' as such manipulations may violate the policy (if you
want to change the NSEC3 parameters, change the policy and reconfig).
Matthijs Mekking [Tue, 13 Oct 2020 12:48:04 +0000 (14:48 +0200)]
Fix a reconfig bug wrt inline-signing
When doing 'rndc reconfig', named may complain about a zone not being
reusable because it has a raw version of the zone, and the new
configuration has not set 'inline-signing'. However, 'inline-signing'
may be implicitly true if a 'dnssec-policy' is used for the zone, and
the zone is not dynamic.
Improve the check in 'named_zone_reusable'. Create a new function for
checking 'inline-signing' configuration that matches existing code in
'bin/named/server.c'.
Matthijs Mekking [Tue, 13 Oct 2020 12:39:21 +0000 (14:39 +0200)]
Support for NSEC3 in dnssec-policy
Implement support for NSEC3 in dnssec-policy. Store the configuration
in kasp objects. When configuring a zone, call 'dns_zone_setnsec3param'
to queue an nsec3param event. This will ensure that any previous
chains will be removed and a chain according to the dnssec-policy is
created.
Add tests for dnssec-policy zones that uses the new 'nsec3param'
option, as well as changing to new values, changing to NSEC, and
changing from NSEC.
Michał Kępień [Wed, 25 Nov 2020 11:45:47 +0000 (12:45 +0100)]
Convert add_quota() to a function
cppcheck 2.2 reports the following false positive:
lib/isc/tests/quota_test.c:71:21: error: Array 'quotas[101]' accessed at index 110, which is out of bounds. [arrayIndexOutOfBounds]
isc_quota_t *quotas[110];
^
The above is not even an array access, so this report is obviously
caused by a cppcheck bug. Yet, it seems to be triggered by the presence
of the add_quota() macro, which should really be a function. Convert
the add_quota() macro to a function in order to make the code cleaner
and to prevent the above cppcheck 2.2 false positive from being
triggered.
projects / thirdparty / bind9.git / log
