Ondřej Surý [Wed, 31 Mar 2021 10:14:54 +0000 (12:14 +0200)]
Use generic functions for reading and timers in TCP
The TCP module has been updated to use the generic functions from
netmgr.c instead of its own local copies. This brings the module
mostly up to par with the TCPDNS and TLSDNS modules.
Ondřej Surý [Wed, 31 Mar 2021 09:48:41 +0000 (11:48 +0200)]
Fix and clean up handling of connect callbacks
Serveral problems were discovered and fixed after the change in
the connection timeout in the previous commits:
* In TLSDNS, the connection callback was not called at all under some
circumstances when the TCP connection had been established, but the
TLS handshake hadn't been completed yet. Additional checks have
been put in place so that tls_cycle() will end early when the
nmsocket is invalidated by the isc__nm_tlsdns_shutdown() call.
* In TCP, TCPDNS and TLSDNS, new connections would be established
even when the network manager was shutting down. The new
call isc__nm_closing() has been added and is used to bail out
early even before uv_tcp_connect() is attempted.
Ondřej Surý [Tue, 30 Mar 2021 07:25:09 +0000 (09:25 +0200)]
Make it possible to recover from connect timeouts
Similarly to the read timeout, it's now possible to recover from
ISC_R_TIMEDOUT event by restarting the timer from the connect callback.
The change here also fixes platforms that missing the socket() options
to set the TCP connection timeout, by moving the timeout code into user
space. On platforms that support setting the connect timeout via a
socket option, the timeout has been hardcoded to 2 minutes (the maximum
value of tcp-initial-timeout).
Ondřej Surý [Mon, 29 Mar 2021 08:52:05 +0000 (10:52 +0200)]
Make it possible to recover from read timeouts
Previously, when the client timed out on read, the client socket would
be automatically closed and destroyed when the nmhandle was detached.
This commit changes the logic so that it's possible for the callback to
recover from the ISC_R_TIMEDOUT event by restarting the timer. This is
done by calling isc_nmhandle_settimeout(), which prevents the timeout
handling code from destroying the socket; instead, it continues to wait
for data.
One specific use case for multiple timeouts is serve-stale - the client
socket could be created with shorter timeout (as specified with
stale-answer-client-timeout), so we can serve the requestor with stale
answer, but keep the original query running for a longer time.
Ondřej Surý [Wed, 31 Mar 2021 06:59:03 +0000 (08:59 +0200)]
Disable netmgr tests only when running under CI
The full netmgr test suite is unstable when run in CI due to various
timing issues. Previously, we enabled the full test suite only when
CI_ENABLE_ALL_TESTS environment variable was set, but that went against
original intent of running the full suite when an individual developer
would run it locally.
This change disables the full test suite only when running in the CI and
the CI_ENABLE_ALL_TESTS is not set.
Using "stale-answer-client-timeout" turns out to have unforeseen
negative consequences, and thus it is better to disable the feature
by default for the time being.
Diego Fronza [Tue, 6 Apr 2021 19:29:25 +0000 (16:29 -0300)]
Resolve TSAN data race in zone_maintenance
Fix race between zone_maintenance and dns_zone_notifyreceive functions,
zone_maintenance was attempting to read a zone flag calling
DNS_ZONE_FLAG(zone, flag) while dns_zone_notifyreceive was updating
a flag in the same zone calling DNS_ZONE_SETFLAG(zone, ...).
The code reading the flag in zone_maintenance was not protected by the
zone's lock, to avoid a race the zone's lock is now being acquired
before an attempt to read the zone flag is made.
Michał Kępień [Wed, 7 Apr 2021 09:41:45 +0000 (11:41 +0200)]
Enforce a run time limit on unit test binaries
When a unit test binary hangs, the GitLab CI job in which it is run is
stuck until its run time limit is exceeded. Furthermore, it is not
trivial to determine which test(s) hung in a given GitLab CI job based
on its log. To prevent these issues, enforce a run time limit on every
binary executed by the lib/unit-test-driver.sh script. Use a timeout of
5 minutes for consistency with older BIND 9 branches, which employed
Kyua for running unit tests. Report an exit code of 124 when the run
time limit is exceeded for a unit test binary, for consistency with the
"timeout" tool included in GNU coreutils.
It is mentioned there that when TLS status equals SSL_ERROR_SYSCALL
AND errno == 0 it means that underlying transport layer returned EOF
prematurely. However, we are managing the transport ourselves, so we
should just resume reading from the TCP socket.
It seems that this case has been handled properly on modern versions
of OpenSSL. That being said, the situation goes in line with the
manual: it is briefly mentioned there that SSL_ERROR_SYSCALL might be
returned not only in a case of low-level errors (like system call
failures).
Matthijs Mekking [Tue, 30 Mar 2021 14:05:51 +0000 (16:05 +0200)]
If RPZ config'd, bail stale-answer-client-timeout
When we are recursing, RPZ processing is not allowed. But when we are
performing a lookup due to "stale-answer-client-timeout", we are still
recursing. This effectively means that RPZ processing is disabled on
such a lookup.
In this case, bail the "stale-answer-client-timeout" lookup and wait
for recursion to complete, as we we can't perform the RPZ rewrite
rules reliably.
Matthijs Mekking [Mon, 29 Mar 2021 07:46:08 +0000 (09:46 +0200)]
Rename "staleonly"
The dboption DNS_DBFIND_STALEONLY caused confusion because it implies
we are looking for stale data **only** and ignore any active RRsets in
the cache. Rename it to DNS_DBFIND_STALETIMEOUT as it is more clear
the option is related to a lookup due to "stale-answer-client-timeout".
Rename other usages of "staleonly", instead use "lookup due to...".
Also rename related function and variable names.
Matthijs Mekking [Fri, 26 Mar 2021 14:18:01 +0000 (15:18 +0100)]
Restore the RECURSIONOK attribute after staleonly
When doing a staleonly lookup we don't want to fallback to recursion.
After all, there are obviously problems with recursion, otherwise we
wouldn't do a staleonly lookup.
When resuming from recursion however, we should restore the
RECURSIONOK flag, allowing future required lookups for this client
to recurse.
Matthijs Mekking [Fri, 26 Mar 2021 13:43:07 +0000 (14:43 +0100)]
Remove result exception on staleonly lookup
When implementing "stale-answer-client-timeout", we decided that
we should only return positive answers prematurely to clients. A
negative response is not useful, and in that case it is better to
wait for the recursion to complete.
To do so, we check the result and if it is not ISC_R_SUCCESS, we
decide that it is not good enough. However, there are more return
codes that could lead to a positive answer (e.g. CNAME chains).
This commit removes the exception and now uses the same logic that
other stale lookups use to determine if we found a useful stale
answer (stale_found == true).
This means we can simplify two test cases in the serve-stale system
test: nodata.example is no longer treated differently than data.example.
Matthijs Mekking [Fri, 26 Mar 2021 10:58:12 +0000 (11:58 +0100)]
Remove INSIST on NS_QUERYATTR_ANSWERED
The NS_QUERYATTR_ANSWERED attribute is to prevent sending a response
twice. Without the attribute, this may happen if a staleonly lookup
found a useful answer and sends a response to the client, and later
recursion ends and also tries to send a response.
The attribute was also used to mask adding a duplicate RRset. This is
considered harmful. When we created a response to the client with a
stale only lookup (regardless if we actually have send the response),
we should clear the rdatasets that were added during that lookup.
Mark such rdatasets with the a new attribute,
DNS_RDATASETATTR_STALE_ADDED. Set a query attribute
NS_QUERYATTR_STALEOK if we may have added rdatasets during a stale
only lookup. Before creating a response on a normal lookup, check if
we can expect rdatasets to have been added during a staleonly lookup.
If so, clear the rdatasets from the message with the attribute
DNS_RDATASETATTR_STALE_ADDED set.
Matthijs Mekking [Thu, 25 Mar 2021 17:12:38 +0000 (18:12 +0100)]
Simplify when to detach the client
With stale-answer-client-timeout, we may send a response to the client,
but we may want to hold on to the network manager handle, because
recursion is going on in the background, or we need to refresh a
stale RRset.
Simplify the setting of 'nodetach':
* During a staleonly lookup we should not detach the nmhandle, so just
set it prior to 'query_lookup()'.
* During a staleonly "stalefirst" lookup set the 'nodetach' to true
if we are going to refresh the RRset.
Now there is no longer the need to clear the 'nodetach' if we go
through the "dbfind_stale", "stale_refresh_window", or "stale_only"
paths.
Matthijs Mekking [Thu, 25 Mar 2021 16:25:32 +0000 (17:25 +0100)]
Refactor stale lookups, ignore active RRsets
When doing a staleonly lookup, ignore active RRsets from cache. If we
don't, we may add a duplicate RRset to the message, and hit an
assertion failure in query.c because adding the duplicate RRset to the
ANSWER section failed.
This can happen on a race condition. When a client query is received,
the recursion is started. When 'stale-answer-client-timeout' triggers
around the same time the recursion completes, the following sequence
of events may happen:
1. Queue the "try stale" fetch_callback() event to the client task.
2. Add the RRsets from the authoritative response to the cache.
3. Queue the "fetch complete" fetch_callback() event to the client task.
4. Execute the "try stale" fetch_callback(), which retrieves the
just-inserted RRset from the database.
5. In "ns_query_done()" we are still recursing, but the "staleonly"
query attribute has already been cleared. In other words, the
query will resume when recursion ends (it already has ended but is
still on the task queue).
6. Execute the "fetch complete" fetch_callback(). It finds the answer
from recursion in the cache again and tries to add the duplicate to
the answer section.
This commit changes the logic for finding stale answers in the cache,
such that on "stale_only" lookups actually only stale RRsets are
considered. It refactors the code so that code paths for "dbfind_stale",
"stale_refresh_window", and "stale_only" are more clear.
First we call some generic code that applies in all three cases,
formatting the domain name for logging purposes, increment the
trystale stats, and check if we actually found stale data that we can
use.
The "dbfind_stale" lookup will return SERVFAIL if we didn't found a
usable answer, otherwise we will continue with the lookup
(query_gotanswer()). This is no different as before the introduction of
"stale-answer-client-timeout" and "stale-refresh-time".
The "stale_refresh_window" lookup is similar to the "dbfind_stale"
lookup: return SERVFAIL if we didn't found a usable answer, otherwise
continue with the lookup (query_gotanswer()).
Finally the "stale_only" lookup.
If the "stale_only" lookup was triggered because of an actual client
timeout (stale-answer-client-timeout > 0), and if database lookup
returned a stale usable RRset, trigger a response to the client.
Otherwise return and wait until the recursion completes (or the
resolver query times out).
If the "stale_only" lookup is a "stale-anwer-client-timeout 0" lookup,
preferring stale data over a lookup. In this case if there was no stale
data, or the data was not a positive answer, retry the lookup with the
stale options cleared, a.k.a. a normal lookup. Otherwise, continue
with the lookup (query_gotanswer()) and refresh the stale RRset. This
will trigger a response to the client, but will not detach the handle
because a fetch will be created to refresh the RRset.
Matthijs Mekking [Thu, 25 Mar 2021 13:13:35 +0000 (14:13 +0100)]
Keep track of allow client detach
The stale-answer-client-timeout feature introduced a dependancy on
when a client may be detached from the handle. The dboption
DNS_DBFIND_STALEONLY was reused to track this attribute. This overloads
the meaning of this database option, and actually introduced a bug
because the option was checked in other places. In particular, in
'ns_query_done()' there is a check for 'RECURSING(qctx->client) &&
(!QUERY_STALEONLY(&qctx->client->query) || ...' and the condition is
satisfied because recursion has not completed yet and
DNS_DBFIND_STALEONLY is already cleared by that time (in
query_lookup()), because we found a useful answer and we should detach
the client from the handle after sending the response.
Add a new boolean to the client structure to keep track of client
detach from handle is allowed or not. It is only disallowed if we are
in a staleonly lookup and we didn't found a useful answer.
Artem Boldariev [Wed, 31 Mar 2021 10:23:59 +0000 (13:23 +0300)]
Fix crash (regression) in DIG when handling non-DoH responses
This commit fixes crash in dig when it encounters non-expected header
value. The bug was introduced at some point late in the last DoH
development cycle. Also, refactors the relevant code a little bit to
ensure better incoming data validation for client-side DoH
connections.
Mark Andrews [Mon, 1 Mar 2021 02:58:06 +0000 (13:58 +1100)]
Test dynamic libraries should not be installed
Tag the libraries with check_ to prevent them being installed
by "make install". Additionally make check requires .so to be
create which requires .lai files to be constructed which, in
turn, requires -rpath <dir> as part of "linking" the .la file.
Michal Nowak [Wed, 31 Mar 2021 08:20:35 +0000 (10:20 +0200)]
Run gcc:tarball CI job in web-triggered pipelines
The gcc:tarball CI job may identify problems with tarballs created by
"make dist" of the tarball-create CI job. Enabling the gcc:tarball CI
job in web-triggered pipelines provides developers with a test vector.
Michał Kępień [Mon, 29 Mar 2021 11:06:39 +0000 (13:06 +0200)]
Include all pre-generated man pages in "make dist"
Some man pages (e.g. dnstap-read.1, named-nzd2nzf.1) should only be
installed conditionally (when the relevant features are enabled in a
given BIND 9 build). This is achieved using Automake conditionals.
However, while all source reStructuredText files are included in
tarballs produced by "make dist" (distribution tarballs) as they should
be, the list of pre-generated man pages included in distribution
tarballs incorrectly depends on the ./configure switches used for the
build for which "make dist" is run. Meanwhile, distribution tarballs
should always contain all the files necessary to build any flavor of
BIND 9.
Here is an example scenario which fails to work as intended:
autoreconf -i
./configure --disable-maintainer-mode
make dist
tar --extract --file bind-9.17.11.tar.xz
cd bind-9.17.11
./configure --disable-maintainer-mode --enable-dnstap
make
Fix by always including pre-generated versions of all conditionally
installed man pages in EXTRA_DIST. While this may cause some of them to
appear in EXTRA_DIST more than once (depending on the ./configure
switches used for the build for which "make dist" is run), it seems to
not be a problem for Automake.
Petr Mensik [Fri, 26 Mar 2021 10:01:59 +0000 (11:01 +0100)]
Do not require config.h to use isc/util.h
util.h requires ISC_CONSTRUCTOR definition, which depends on config.h
inclusion. It does not include it from isc/util.h (or any other header).
Using isc/util.h fails hard when isc/util.h is used without including
bind's config.h.
Move the check to c file, where ISC_CONSTRUCTOR is used. Ensure config.h
is included there.
Diego Fronza [Tue, 16 Feb 2021 16:13:07 +0000 (13:13 -0300)]
Added tests for tries=1 and retry=0 on TCP EOF
Added tests to ensure that dig won't retry sending a query over tcp
(+tcp) when a TCP connection is closed prematurely (EOF is read) if
either +tries=1 or retry=0 is specified on the command line.
Diego Fronza [Tue, 16 Feb 2021 15:36:45 +0000 (12:36 -0300)]
Adjusted dig system tests
Now that premature EOF on tcp connections take +tries and +retry into
account, the dig system tests handling TCP EOF with +tries=1 were
expecting dig to do a second attempt in handling the tcp query, which
doesn't happen anymore.
To make the test work as expected +tries value was adjusted to 2, to
make it behave as before after the new update on dig.
Diego Fronza [Mon, 15 Feb 2021 22:28:50 +0000 (19:28 -0300)]
Don't retry +tcp queries on failure if tries=1 or retries=0
Before this commit, a premature EOF (connection closed) on tcp queries
was causing dig to automatically attempt to send the query again, even
if +tries=1 or +retries=0 was provided on command line.
This commit fix the problem by taking into account the no. of retries
specified by the user when processing a premature EOF on tcp
connections.
Matthijs Mekking [Wed, 24 Mar 2021 07:48:23 +0000 (08:48 +0100)]
Add kasp.sh to run.sh.in script
Add kasp.sh to the list of scripts copied from the source directory to
the build directory before any test is run. This will fix
the out-of-tree test failures introduced in commit ecb073bdd6b1ff5ad07293c1db190cf28df6708e on the 'main' branch.
Matthijs Mekking [Thu, 18 Mar 2021 09:47:56 +0000 (10:47 +0100)]
Fix some intermittent kasp failures
When calling "rndc dnssec -checkds", it may take some milliseconds
before the appropriate changes have been written to the state file.
Add retry_quiet mechanisms to allow the write operation to finish.
Also retry_quiet the check for the next key event. A "rndc dnssec"
command may trigger a zone_rekey event and this will write out
a new "next key event" log line, but it may take a bit longer than
than expected in the tests.
Matthijs Mekking [Wed, 17 Mar 2021 14:57:34 +0000 (15:57 +0100)]
Rekey immediately after rndc checkds/rollover
Call 'dns_zone_rekey' after a 'rndc dnssec -checkds' or 'rndc dnssec
-rollover' command is received, because such a command may influence
the next key event. Updating the keys immediately avoids unnecessary
rollover delays.
The kasp system test no longer needs to call 'rndc loadkeys' after
a 'rndc dnssec -checkds' or 'rndc dnssec -rollover' command.
Matthijs Mekking [Wed, 17 Mar 2021 09:04:00 +0000 (10:04 +0100)]
Delete CDS/CDNSKEY records when zone is unsigned
CDS/CDNSKEY DELETE records are only useful if they are signed,
otherwise the parent cannot verify these RRsets anyway. So once the DS
has been removed (and signaled to BIND), we can remove the DNSKEY and
RRSIG records, and at this point we can also remove the CDS/CDNSKEY
records.
Matthijs Mekking [Fri, 19 Mar 2021 11:13:28 +0000 (12:13 +0100)]
Retry quiet check keys
Change the 'check_keys' function to try three times. Some intermittent
kasp test failures are because we are inspecting the key files
before the actual change has happen. The 'retry_quiet' approach allows
for a bit more time to let the write operation finish.
Matthijs Mekking [Fri, 19 Mar 2021 07:29:55 +0000 (08:29 +0100)]
Update copyrights for keymgr2kasp
This MR introduces a new system test 'keymgr2kasp' to test
migration to 'dnssec-policy'. It moves some existing tests from
the 'kasp' system test to here.
Also a common script 'kasp.sh', to be used in kasp specific tests,
is introduced.
Matthijs Mekking [Fri, 26 Feb 2021 08:27:32 +0000 (09:27 +0100)]
Fix keymgr key init bug
The 'keymgr_key_init()' function initializes key states if they have
not been set previously. It looks at the key timing metadata and
determines using the given times whether a state should be set to
RUMOURED or OMNIPRESENT.
However, the DNSKEY and ZRRSIG states were mixed up: When looking
at the Activate timing metadata we should set the ZRRSIG state, and
when looking at the Published timing metadata we should set the
DNSKEY state.
Matthijs Mekking [Thu, 25 Feb 2021 16:51:58 +0000 (17:51 +0100)]
Test keymgr2kasp state from timing metadata
Add two test zones that migrate to dnssec-policy. Test if the key
states are set accordingly given the timing metadata.
The rumoured.kasp zone has its Publish/Active/SyncPublish times set
not too long ago so the key states should be set to RUMOURED. The
omnipresent.kasp zone has its Publish/Active/SyncPublish times set
long enough to set the key states to OMNIPRESENT.
Slightly change the init_migration_keys function to set the
key lifetime to "none" (legacy keys don't have lifetime). Then in the
test case set the expected key lifetime explicitly.
Matthijs Mekking [Wed, 24 Feb 2021 14:11:59 +0000 (15:11 +0100)]
Move kasp migration tests to different directory
The kasp system test was getting pretty large, and more tests are on
the way. Time to split up. Move tests that are related to migrating
to dnssec-policy to a separate directory 'keymgr2kasp'.
Michał Kępień [Mon, 22 Mar 2021 08:36:48 +0000 (09:36 +0100)]
Install man page for named-compilezone
The named-checkzone tool can also be invoked as named-compilezone. Make
sure a man page is installed for that alias. Move and rename the
"man_named-checkzone" label to prevent a Sphinx duplicate label warning
from being raised (see commit 84862e96c1fcff6e7c1ca31884e2fad921afa4f7
for more information).
Michał Kępień [Mon, 22 Mar 2021 08:36:48 +0000 (09:36 +0100)]
Install named-nzd2nzf man page conditionally
The named-nzd2nzf utility is only built and installed for LMDB-enabled
builds. Adjust the relevant Makefile.am file to make sure the
named-nzd2nzf.1 man page is also only built and installed for
LMDB-enabled builds.
Michał Kępień [Mon, 22 Mar 2021 08:36:48 +0000 (09:36 +0100)]
Install dnstap-read man page conditionally
The dnstap-read utility is only built and installed for dnstap-enabled
builds. Adjust the relevant Makefile.am file to make sure the
dnstap-read.1 man page is also only built and installed for
dnstap-enabled builds.
Evan Hunt [Sat, 20 Mar 2021 19:11:11 +0000 (12:11 -0700)]
placeholder for #2575
Issue #2575 was merged to 9.16 only as change 5603, but a placeholder
was not added to CHANGES in the main branch. This commit adds the
placeholder and renumbers the two subsequent changes.
Patrick McLean [Sat, 20 Mar 2021 05:50:51 +0000 (22:50 -0700)]
dig: Use high resolution clocks when microsecond accuracy is requested
The TIME_NOW macro calls isc_time_now which uses CLOCK_REALTIME_COARSE
for getting the current time. This is perfectly fine for millisecond,
however when the user request microsecond resolutiuon, they are going
to get very inaccurate results. This is especially true on a server
class machine where the clock ticks may be set to 100HZ.
This changes dig to use the new TIME_NOW_HIRES macro that uses the
CLOCK_MONOTONIC_RAW that is more expensive, but gets the *actual*
current time rather than the at the last kernel time tick.
Patrick McLean [Sat, 20 Mar 2021 05:48:22 +0000 (22:48 -0700)]
Add isc_time_now_hires function to get current time with high resolution
The current isc_time_now uses CLOCK_REALTIME_COARSE which only updates
on a timer tick. This clock is generally fine for millisecond accuracy,
but on servers with 100hz clocks, this clock is nowhere near accurate
enough for microsecond accuracy.
This commit adds a new isc_time_now_hires function that uses
CLOCK_REALTIME, which gives the current time, though it is somewhat
expensive to call. When microsecond accuracy is required, it may be
required to use extra resources for higher accuracy.
Ondřej Surý [Mon, 15 Mar 2021 09:22:15 +0000 (10:22 +0100)]
Require CMocka >= 1.1.3 to run the unit tests
In CMocka versions << 1.1.3, the skip() function would cause the whole
unit test to abort when CMOCKA_TEST_ABORT is set. As this is problem
only in Debian 9 Stretch and Ubuntu 16.04 Xenial, we just require the
CMocka >= 1.1.3 and disable the unit testing on Debian 9 Stretch until
we can pull the libcmocka-dev from stretch-backports and remove the
Ubuntu 16.04 Xenial from the CI as it is reaching End of Standard
Support at the end of April 2021.
Ondřej Surý [Fri, 29 Jan 2021 12:00:46 +0000 (13:00 +0100)]
Fix compilation with NETMGR_TRACE(_VERBOSE) enabled on non-Linux
When NETMGR_TRACE(_VERBOSE) is enabled, the build would fail on some
non-Linux non-glibc platforms because:
* Use <stdint.h> print macros because uint_fast32_t is not always
unsigned long
* The header <execinfo.h> is not available on non-glibc, thus commit
adds dummy backtrace() and backtrace_symbols_fd() functions for
platforms without HAVE_BACKTRACE
Ondřej Surý [Fri, 29 Jan 2021 12:00:46 +0000 (13:00 +0100)]
Improve reliability of the netmgr unit tests
The netmgr unit tests were designed to push the system limits to maximum
by sending as many queries as possible in the busy loop from multiple
threads. This mostly works with UDP, but in the stateful protocol where
establishing the connection takes more time, it failed quite often in
the CI. On FreeBSD, this happened more often, because the socket() call
would fail spuriosly making the problem even worse.
This commit does several things to improve reliability:
* return value of isc_nm_<proto>connect() is always checked and retried
when scheduling the connection fails
* The busy while loop has been slowed down with usleep(1000); so the
netmgr threads could schedule the work and get executed.
* The isc_thread_yield() was replaced with usleep(1000); also to allow
the other threads to do any work.
* Instead of waiting on just one variable, we wait for multiple
variables to reach the final value
* We are wrapping the netmgr operations (connects, reads, writes,
accepts) with reference counting and waiting for all the callbacks to
be accounted for.
This has two effects:
a) the isc_nm_t is always clean of active sockets and handles when
destroyed, so it will prevent the spurious INSIST(references == 1)
from isc_nm_destroy()
b) the unit test now ensures that all the callbacks are always called
when they should be called, so any stuck test means that there was
a missing callback call and it is always a real bug
These changes allows us to remove the workaround that would not run
certain tests on systems without port load-balancing.
Ondřej Surý [Fri, 19 Mar 2021 14:08:14 +0000 (15:08 +0100)]
Call isc__nm_tlsdns_failed_read on tls_error to cleanup the socket
In tls_error(), we now call isc__nm_tlsdns_failed_read() instead of just
stopping timer and reading from the socket. This allows us to properly
cleanup any pending operation on the socket.
Ondřej Surý [Thu, 18 Mar 2021 20:37:12 +0000 (21:37 +0100)]
Call the isc__nm_failed_connect_cb() early when shutting down
When shutting down, calling the isc__nm_failed_connect_cb() was delayed
until the connect callback would be called. It turned out that the
connect callback might not get called at all when the socket is being
shut down. Call the failed_connect_cb() directly in the
tlsdns_shutdown() instead of waiting for the connect callback to call it.
Ondřej Surý [Thu, 18 Mar 2021 17:14:38 +0000 (18:14 +0100)]
Fix memory accounting bug in TLSDNS
After a partial write the tls.senddata buffer would be rearranged to
contain only the data tha wasn't sent and the len part would be made
shorter, which would lead to attempt to free only part of a socket's
tls.senddata buffer.
Ondřej Surý [Fri, 29 Jan 2021 12:00:46 +0000 (13:00 +0100)]
Fix dangling uvreq when data is sent from tlsdns_cycle()
The tlsdns_cycle() might call uv_write() to write data to the socket,
when this happens and the socket is shutdown before the callback
completes, the uvreq structure was not freed because the callback would
be called with non-zero status code.
Ondřej Surý [Thu, 18 Mar 2021 10:16:45 +0000 (11:16 +0100)]
Change the isc_nm_(get|set)timeouts() to work with milliseconds
The RFC7828 specifies the keepalive interval to be 16-bit, specified in
units of 100 milliseconds and the configuration options tcp-*-timeouts
are following the suit. The units of 100 milliseconds are very
unintuitive and while we can't change the configuration and presentation
format, we should not follow this weird unit in the API.
This commit changes the isc_nm_(get|set)timeouts() functions to work
with milliseconds and convert the values to milliseconds before passing
them to the function, not just internally.
Ondřej Surý [Thu, 18 Mar 2021 08:27:38 +0000 (09:27 +0100)]
Merge the common parts between udp, tcpdns and tlsdns protocol
The udp, tcpdns and tlsdns contained lot of cut&paste code or code that
was very similar making the stack harder to maintain as any change to
one would have to be copied to the the other protocols.
In this commit, we merge the common parts into the common functions
under isc__nm_<foo> namespace and just keep the little differences based
on the socket type.
Ondřej Surý [Tue, 16 Mar 2021 08:03:02 +0000 (09:03 +0100)]
Fix TCPDNS and TLSDNS timers
After the TCPDNS refactoring the initial and idle timers were broken and
only the tcp-initial-timeout was always applied on the whole TCP
connection.
This broke any TCP connection that took longer than tcp-initial-timeout,
most often this would affect large zone AXFRs.
This commit changes the timeout logic in this way:
* On TCP connection accept the tcp-initial-timeout is applied
and the timer is started
* When we are processing and/or sending any DNS message the timer is
stopped
* When we stop processing all DNS messages, the tcp-idle-timeout
is applied and the timer is started again
Ondřej Surý [Wed, 17 Mar 2021 11:20:40 +0000 (12:20 +0100)]
Add TCP timeouts system test
The system tests were missing a test that would test tcp-initial-timeout
and tcp-idle-timeout.
This commit adds new "timeouts" system test that adds:
* Test that waits longer than tcp-initial-timeout and then checks
whether the socket was closed
* Test that sends and receives DNS message then waits longer than
tcp-initial-timeout but shorter time than tcp-idle-timeout than
sends DNS message again than waits longer than tcp-idle-timeout
and checks whether the socket was closed
* Similar test, but bursting 25 DNS messages than waiting longer than
tcp-initial-timeout and shorter than tcp-idle-timeout than do second
25 DNS message burst
* Check whether transfer longer than tcp-initial-timeout succeeds
projects / thirdparty / bind9.git / log
